| Message ID | 20230530131214.373524-2-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v1,1/2] target/ppc: Fix decrementer time underflow and infinite timer loop | expand |
On 5/30/23 10:12, Nicholas Piggin wrote: > The decrementer store function has logic that short-cuts the timer if a > very small value is stored (0, 1, or 2) and raises an interrupt > directly. There are two problem with this on BookE. > > First is that BookE says a decrementer interrupt should not be raised > on a store of 0, only of a decrement from 1. Second is that raising > the irq directly will bypass the auto-reload logic in the booke decr > timer function, breaking autoreload when 1 or 2 is stored. > > Fix this by removing that small-value special case. It makes this > tricky logic even more difficult to reason about, and it hardly matters > for performance. > > Cc: sdicaro@DDCI.com > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com> And queued. Daniel > These were some booke decrementer corner case issues I saw, probably > a little less important than the first patch so could go later. > > Thanks, > Nick > > hw/ppc/ppc.c | 9 ++------- > 1 file changed, 2 insertions(+), 7 deletions(-) > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c > index d80b0adc6c..1b1220c423 100644 > --- a/hw/ppc/ppc.c > +++ b/hw/ppc/ppc.c > @@ -811,11 +811,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, > } > > /* > - * Going from 2 -> 1, 1 -> 0 or 0 -> -1 is the event to generate a DEC > - * interrupt. > - * > - * If we get a really small DEC value, we can assume that by the time we > - * handled it we should inject an interrupt already. > + * Going from 1 -> 0 or 0 -> -1 is the event to generate a DEC interrupt. > * > * On MSB level based DEC implementations the MSB always means the interrupt > * is pending, so raise it on those. > @@ -823,8 +819,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, > * On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers > * an edge interrupt, so raise it here too. > */ > - if ((value < 3) || > - ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || > + if (((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || > ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0 > && signed_decr >= 0)) { > (*raise_excp)(cpu);
diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c index d80b0adc6c..1b1220c423 100644 --- a/hw/ppc/ppc.c +++ b/hw/ppc/ppc.c @@ -811,11 +811,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, } /* - * Going from 2 -> 1, 1 -> 0 or 0 -> -1 is the event to generate a DEC - * interrupt. - * - * If we get a really small DEC value, we can assume that by the time we - * handled it we should inject an interrupt already. + * Going from 1 -> 0 or 0 -> -1 is the event to generate a DEC interrupt. * * On MSB level based DEC implementations the MSB always means the interrupt * is pending, so raise it on those. @@ -823,8 +819,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp, * On MSB edge based DEC implementations the MSB going from 0 -> 1 triggers * an edge interrupt, so raise it here too. */ - if ((value < 3) || - ((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || + if (((tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL) && signed_value < 0) || ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && signed_value < 0 && signed_decr >= 0)) { (*raise_excp)(cpu);
The decrementer store function has logic that short-cuts the timer if a very small value is stored (0, 1, or 2) and raises an interrupt directly. There are two problem with this on BookE. First is that BookE says a decrementer interrupt should not be raised on a store of 0, only of a decrement from 1. Second is that raising the irq directly will bypass the auto-reload logic in the booke decr timer function, breaking autoreload when 1 or 2 is stored. Fix this by removing that small-value special case. It makes this tricky logic even more difficult to reason about, and it hardly matters for performance. Cc: sdicaro@DDCI.com Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- These were some booke decrementer corner case issues I saw, probably a little less important than the first patch so could go later. Thanks, Nick hw/ppc/ppc.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-)