Re: [PATCH 5/6] hw/ppc: Always store the decrementer value

[Cédric Le Goater]

Hello Nick,

On 7/26/23 20:22, Nicholas Piggin wrote:
> When writing a value to the decrementer that raises an exception, the
> irq is raised, but the value is not stored so the store doesn't appear
> to have changed the register when it is read again.
>
> Always store the write value to the register.

This change has a serious performance impact when a guest is run under
PowerNV. Could you please take a look ?

Thanks,

C.

PS: We should really introduce avocado tests for nested.
 
> Fixes: e81a982aa53 ("PPC: Clean up DECR implementation")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>   hw/ppc/ppc.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index fa60f76dd4..cd1993e9c1 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -812,6 +812,11 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t 
> *nextp,
>           return;
>       }
>   
> +    /* Calculate the next timer event */
> +    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> +    next = muldiv64(now, tb_env->decr_freq, NANOSECONDS_PER_SECOND) + value;
> +    *nextp = next; /* nextp is in timebase units */
> +
>       /*
>        * Going from 1 -> 0 or 0 -> -1 is the event to generate a DEC interrupt.
>        *
> @@ -833,11 +838,6 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t 
> *nextp,
>           (*lower_excp)(cpu);
>       }
>   
> -    /* Calculate the next timer event */
> -    now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> -    next = muldiv64(now, tb_env->decr_freq, NANOSECONDS_PER_SECOND) + value;
> -    *nextp = next; /* nextp is in timebase units */
> -
>       /* Adjust timer */
>       timer_mod(timer, muldiv64(next, NANOSECONDS_PER_SECOND, 
> tb_env->decr_freq));
>   }