qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v1 2/2] target/ppc: Decrementer fix BookE semantics


From: Daniel Henrique Barboza
Subject: Re: [PATCH v1 2/2] target/ppc: Decrementer fix BookE semantics
Date: Mon, 5 Jun 2023 10:38:45 -0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0



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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]