Re: [PATCH for-6.1 1/2] accel/tcg: Don't use CF_COUNT_MASK as the max value of icount_decr.u16.low

[Philippe Mathieu-Daudé]

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;
>  
>