| Message ID | 20210725174405.24568-2-peter.maydell@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | accel/tcg: Fix hang when running in icount mode | expand |
On 7/25/21 7:44 PM, Peter Maydell wrote: > In cpu_loop_exec_tb() we were bounding the number of insns we might > try to execute in a TB using CF_COUNT_MASK. This is incorrect, > because we can validly put up to 0xffff into icount_decr.u16.low. In > particular, since commit 78ff82bb1b67c0d7 reduced CF_COUNT_MASK to > 511 this meant that we would incorrectly only try to execute 511 > instructions in a 512-instruction TB, which could result in QEMU > hanging when in icount mode. Nice catch... > Use the actual maximum value, which is 0xffff. (This brings this code > in to line with the similar logic in icount_prepare_for_run() in > tcg-accel-ops-icount.c.) > > Fixes: 78ff82bb1b67c0d7 > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/499 > Signed-off-by: Peter Maydell <peter.maydell@linaro.org> > --- > accel/tcg/cpu-exec.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c > index fc895cf51e4..6e8dc291197 100644 > --- a/accel/tcg/cpu-exec.c > +++ b/accel/tcg/cpu-exec.c > @@ -834,7 +834,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb, > /* Ensure global icount has gone forward */ > icount_update(cpu); > /* Refill decrementer and continue execution. */ > - insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); > + insns_left = MIN(0xffff, cpu->icount_budget); Or UINT16_MAX. Regardless: Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org> > cpu_neg(cpu)->icount_decr.u16.low = insns_left; > cpu->icount_extra = cpu->icount_budget - insns_left; > >
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c index fc895cf51e4..6e8dc291197 100644 --- a/accel/tcg/cpu-exec.c +++ b/accel/tcg/cpu-exec.c @@ -834,7 +834,7 @@ static inline void cpu_loop_exec_tb(CPUState *cpu, TranslationBlock *tb, /* Ensure global icount has gone forward */ icount_update(cpu); /* Refill decrementer and continue execution. */ - insns_left = MIN(CF_COUNT_MASK, cpu->icount_budget); + insns_left = MIN(0xffff, cpu->icount_budget); cpu_neg(cpu)->icount_decr.u16.low = insns_left; cpu->icount_extra = cpu->icount_budget - insns_left;
In cpu_loop_exec_tb() we were bounding the number of insns we might try to execute in a TB using CF_COUNT_MASK. This is incorrect, because we can validly put up to 0xffff into icount_decr.u16.low. In particular, since commit 78ff82bb1b67c0d7 reduced CF_COUNT_MASK to 511 this meant that we would incorrectly only try to execute 511 instructions in a 512-instruction TB, which could result in QEMU hanging when in icount mode. Use the actual maximum value, which is 0xffff. (This brings this code in to line with the similar logic in icount_prepare_for_run() in tcg-accel-ops-icount.c.) Fixes: 78ff82bb1b67c0d7 Resolves: https://gitlab.com/qemu-project/qemu/-/issues/499 Signed-off-by: Peter Maydell <peter.maydell@linaro.org> --- accel/tcg/cpu-exec.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)