Re: [PATCH v1 1/2] target/ppc: Fix decrementer time underflow and infinite timer loop

[Daniel Henrique Barboza <danielhb413@gmail.com>]

On 5/30/23 10:12, Nicholas Piggin wrote:
> It is possible to store a very large value to the decrementer that it
> does not raise the decrementer exception so the timer is scheduled, but
> the next time value wraps and is treated as in the past.
> 
> This can occur if (u64)-1 is stored on a zero-triggered exception, or
> (u64)-1 is stored twice on an underflow-triggered exception, for
> example.
> 
> If such a value is set in DECAR, it gets stored to the decrementer by
> the timer function, which then immediately causes another timer, which
> hangs QEMU.
> 
> Clamp the decrementer to the implemented width, and use that as the
> value for the timer calculation, effectively preventing this overflow.
> 
> Reported-by: sdicaro@DDCI.com
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

And queued.


Daniel

> sdicaro@DDCI.com debugged and reported this, I just changed their fix
> to extract variable bits so it works with large decrementer. So most
> of the credit goes to them.
> 
> Thanks,
> Nick
> 
>   hw/ppc/ppc.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 4e816c68c7..d80b0adc6c 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -798,6 +798,8 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
>       int64_t signed_decr;
>   
>       /* Truncate value to decr_width and sign extend for simplicity */
> +    value = extract64(value, 0, nr_bits);
> +    decr = extract64(decr, 0, nr_bits);
>       signed_value = sextract64(value, 0, nr_bits);
>       signed_decr = sextract64(decr, 0, nr_bits);
>