[PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix

[PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix

[Cédric Le Goater]

Hello,

This series converts custom PPC debug messages in trace events. Last
patch is fix for the decrementer when using a 64-bit width.

Thanks,

C. 


Cédric Le Goater (4):
  target/ppc: Convert debug to trace events (exceptions)
  target/ppc: Replace debug messages by asserts for unknown IRQ pins
  target/ppc: Convert debug to trace events (decrementer and IRQ)
  target/ppc: Fix 64-bit decrementer

 hw/ppc/ppc.c             | 211 ++++++++++++++-------------------------
 target/ppc/excp_helper.c |  38 ++-----
 hw/ppc/trace-events      |  22 +++-
 target/ppc/trace-events  |   8 ++
 4 files changed, 114 insertions(+), 165 deletions(-)

-- 
2.31.1

[PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions)

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/excp_helper.c | 38 ++++++++++----------------------------
 target/ppc/trace-events  |  8 ++++++++
 2 files changed, 18 insertions(+), 28 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d7e32ee107e0..b7d176792098 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -23,20 +23,14 @@
 #include "internal.h"
 #include "helper_regs.h"
 
+#include "trace.h"
+
 #ifdef CONFIG_TCG
 #include "exec/helper-proto.h"
 #include "exec/cpu_ldst.h"
 #endif
 
-/* #define DEBUG_OP */
 /* #define DEBUG_SOFTWARE_TLB */
-/* #define DEBUG_EXCEPTIONS */
-
-#ifdef DEBUG_EXCEPTIONS
-#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_EXCP(...) do { } while (0)
-#endif
 
 /*****************************************************************************/
 /* Exception processing */
@@ -414,12 +408,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         }
         break;
     case POWERPC_EXCP_DSI:       /* Data storage exception                   */
-        LOG_EXCP("DSI exception: DSISR=" TARGET_FMT_lx" DAR=" TARGET_FMT_lx
-                 "\n", env->spr[SPR_DSISR], env->spr[SPR_DAR]);
+        trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
         break;
     case POWERPC_EXCP_ISI:       /* Instruction storage exception            */
-        LOG_EXCP("ISI exception: msr=" TARGET_FMT_lx ", nip=" TARGET_FMT_lx
-                 "\n", msr, env->nip);
+        trace_ppc_excp_isi(msr, env->nip);
         msr |= env->error_code;
         break;
     case POWERPC_EXCP_EXTERNAL:  /* External input                           */
@@ -474,7 +466,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
             if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
-                LOG_EXCP("Ignore floating point exception\n");
+                trace_ppc_excp_fp_ignore();
                 cs->exception_index = POWERPC_EXCP_NONE;
                 env->error_code = 0;
                 return;
@@ -489,7 +481,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
             env->spr[SPR_BOOKE_ESR] = ESR_FP;
             break;
         case POWERPC_EXCP_INVAL:
-            LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip);
+            trace_ppc_excp_inval(env->nip);
             msr |= 0x00080000;
             env->spr[SPR_BOOKE_ESR] = ESR_PIL;
             break;
@@ -547,10 +539,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         break;
     case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
         /* FIT on 4xx */
-        LOG_EXCP("FIT exception\n");
+        trace_ppc_excp_print("FIT");
         break;
     case POWERPC_EXCP_WDT:       /* Watchdog timer interrupt                 */
-        LOG_EXCP("WDT exception\n");
+        trace_ppc_excp_print("WDT");
         switch (excp_model) {
         case POWERPC_EXCP_BOOKE:
             srr0 = SPR_BOOKE_CSRR0;
@@ -657,7 +649,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
 #endif
         break;
     case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
-        LOG_EXCP("PIT exception\n");
+        trace_ppc_excp_print("PIT");
         break;
     case POWERPC_EXCP_IO:        /* IO error exception                       */
         /* XXX: TODO */
@@ -1115,14 +1107,6 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
 
 #endif /* !CONFIG_USER_ONLY */
 
-#if defined(DEBUG_OP)
-static void cpu_dump_rfi(target_ulong RA, target_ulong msr)
-{
-    qemu_log("Return from exception at " TARGET_FMT_lx " with flags "
-             TARGET_FMT_lx "\n", RA, msr);
-}
-#endif
-
 /*****************************************************************************/
 /* Exceptions processing helpers */
 
@@ -1221,9 +1205,7 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
     /* XXX: beware: this is false if VLE is supported */
     env->nip = nip & ~((target_ulong)0x00000003);
     hreg_store_msr(env, msr, 1);
-#if defined(DEBUG_OP)
-    cpu_dump_rfi(env->nip, env->msr);
-#endif
+    trace_ppc_excp_rfi(env->nip, env->msr);
     /*
      * No need to raise an exception here, as rfi is always the last
      * insn of a TB
diff --git a/target/ppc/trace-events b/target/ppc/trace-events
index c88cfccf8d19..53b107f56eb6 100644
--- a/target/ppc/trace-events
+++ b/target/ppc/trace-events
@@ -28,3 +28,11 @@ kvm_handle_epr(void) "handle epr"
 kvm_handle_watchdog_expiry(void) "handle watchdog expiry"
 kvm_handle_debug_exception(void) "handle debug exception"
 kvm_handle_nmi_exception(void) "handle NMI exception"
+
+# excp_helper.c
+ppc_excp_rfi(uint64_t nip, uint64_t msr) "Return from exception at 0x%" PRIx64 " with flags 0x%016" PRIx64
+ppc_excp_dsi(uint64_t dsisr, uint64_t dar) "DSI exception: DSISR=0x%" PRIx64 " DAR=0x%" PRIx64
+ppc_excp_isi(uint64_t msr, uint64_t nip) "ISI exception: msr=0x%016" PRIx64 " nip=0x%" PRIx64
+ppc_excp_fp_ignore(void) "Ignore floating point exception"
+ppc_excp_inval(uint64_t nip) "Invalid instruction at 0x%" PRIx64
+ppc_excp_print(const char *excp) "%s exception"
-- 
2.31.1

Re: [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 6043 bytes --]

On Mon, Sep 20, 2021 at 08:12:00AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.

> ---
>  target/ppc/excp_helper.c | 38 ++++++++++----------------------------
>  target/ppc/trace-events  |  8 ++++++++
>  2 files changed, 18 insertions(+), 28 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index d7e32ee107e0..b7d176792098 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -23,20 +23,14 @@
>  #include "internal.h"
>  #include "helper_regs.h"
>  
> +#include "trace.h"
> +
>  #ifdef CONFIG_TCG
>  #include "exec/helper-proto.h"
>  #include "exec/cpu_ldst.h"
>  #endif
>  
> -/* #define DEBUG_OP */
>  /* #define DEBUG_SOFTWARE_TLB */
> -/* #define DEBUG_EXCEPTIONS */
> -
> -#ifdef DEBUG_EXCEPTIONS
> -#  define LOG_EXCP(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_EXCP(...) do { } while (0)
> -#endif
>  
>  /*****************************************************************************/
>  /* Exception processing */
> @@ -414,12 +408,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          }
>          break;
>      case POWERPC_EXCP_DSI:       /* Data storage exception                   */
> -        LOG_EXCP("DSI exception: DSISR=" TARGET_FMT_lx" DAR=" TARGET_FMT_lx
> -                 "\n", env->spr[SPR_DSISR], env->spr[SPR_DAR]);
> +        trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
>          break;
>      case POWERPC_EXCP_ISI:       /* Instruction storage exception            */
> -        LOG_EXCP("ISI exception: msr=" TARGET_FMT_lx ", nip=" TARGET_FMT_lx
> -                 "\n", msr, env->nip);
> +        trace_ppc_excp_isi(msr, env->nip);
>          msr |= env->error_code;
>          break;
>      case POWERPC_EXCP_EXTERNAL:  /* External input                           */
> @@ -474,7 +466,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          switch (env->error_code & ~0xF) {
>          case POWERPC_EXCP_FP:
>              if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
> -                LOG_EXCP("Ignore floating point exception\n");
> +                trace_ppc_excp_fp_ignore();
>                  cs->exception_index = POWERPC_EXCP_NONE;
>                  env->error_code = 0;
>                  return;
> @@ -489,7 +481,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>              env->spr[SPR_BOOKE_ESR] = ESR_FP;
>              break;
>          case POWERPC_EXCP_INVAL:
> -            LOG_EXCP("Invalid instruction at " TARGET_FMT_lx "\n", env->nip);
> +            trace_ppc_excp_inval(env->nip);
>              msr |= 0x00080000;
>              env->spr[SPR_BOOKE_ESR] = ESR_PIL;
>              break;
> @@ -547,10 +539,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          break;
>      case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
>          /* FIT on 4xx */
> -        LOG_EXCP("FIT exception\n");
> +        trace_ppc_excp_print("FIT");
>          break;
>      case POWERPC_EXCP_WDT:       /* Watchdog timer interrupt                 */
> -        LOG_EXCP("WDT exception\n");
> +        trace_ppc_excp_print("WDT");
>          switch (excp_model) {
>          case POWERPC_EXCP_BOOKE:
>              srr0 = SPR_BOOKE_CSRR0;
> @@ -657,7 +649,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>  #endif
>          break;
>      case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
> -        LOG_EXCP("PIT exception\n");
> +        trace_ppc_excp_print("PIT");
>          break;
>      case POWERPC_EXCP_IO:        /* IO error exception                       */
>          /* XXX: TODO */
> @@ -1115,14 +1107,6 @@ bool ppc_cpu_exec_interrupt(CPUState *cs, int interrupt_request)
>  
>  #endif /* !CONFIG_USER_ONLY */
>  
> -#if defined(DEBUG_OP)
> -static void cpu_dump_rfi(target_ulong RA, target_ulong msr)
> -{
> -    qemu_log("Return from exception at " TARGET_FMT_lx " with flags "
> -             TARGET_FMT_lx "\n", RA, msr);
> -}
> -#endif
> -
>  /*****************************************************************************/
>  /* Exceptions processing helpers */
>  
> @@ -1221,9 +1205,7 @@ static inline void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
>      /* XXX: beware: this is false if VLE is supported */
>      env->nip = nip & ~((target_ulong)0x00000003);
>      hreg_store_msr(env, msr, 1);
> -#if defined(DEBUG_OP)
> -    cpu_dump_rfi(env->nip, env->msr);
> -#endif
> +    trace_ppc_excp_rfi(env->nip, env->msr);
>      /*
>       * No need to raise an exception here, as rfi is always the last
>       * insn of a TB
> diff --git a/target/ppc/trace-events b/target/ppc/trace-events
> index c88cfccf8d19..53b107f56eb6 100644
> --- a/target/ppc/trace-events
> +++ b/target/ppc/trace-events
> @@ -28,3 +28,11 @@ kvm_handle_epr(void) "handle epr"
>  kvm_handle_watchdog_expiry(void) "handle watchdog expiry"
>  kvm_handle_debug_exception(void) "handle debug exception"
>  kvm_handle_nmi_exception(void) "handle NMI exception"
> +
> +# excp_helper.c
> +ppc_excp_rfi(uint64_t nip, uint64_t msr) "Return from exception at 0x%" PRIx64 " with flags 0x%016" PRIx64
> +ppc_excp_dsi(uint64_t dsisr, uint64_t dar) "DSI exception: DSISR=0x%" PRIx64 " DAR=0x%" PRIx64
> +ppc_excp_isi(uint64_t msr, uint64_t nip) "ISI exception: msr=0x%016" PRIx64 " nip=0x%" PRIx64
> +ppc_excp_fp_ignore(void) "Ignore floating point exception"
> +ppc_excp_inval(uint64_t nip) "Invalid instruction at 0x%" PRIx64
> +ppc_excp_print(const char *excp) "%s exception"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins

[Cédric Le Goater]

If an unknown pin of the IRQ controller is raised, something is very
wrong in the QEMU model. It is better to abort.

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc.c | 24 ++++++------------------
 1 file changed, 6 insertions(+), 18 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index 7375bf4fa910..a327206a0a1e 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -165,9 +165,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
             ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
             break;
         default:
-            /* Unknown pin - do nothing */
-            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-            return;
+            g_assert_not_reached();
         }
         if (level)
             env->irq_input_state |= 1 << pin;
@@ -252,9 +250,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
             /* XXX: TODO */
             break;
         default:
-            /* Unknown pin - do nothing */
-            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-            return;
+            g_assert_not_reached();
         }
         if (level)
             env->irq_input_state |= 1 << pin;
@@ -287,9 +283,7 @@ static void power7_set_irq(void *opaque, int pin, int level)
         ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
         break;
     default:
-        /* Unknown pin - do nothing */
-        LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-        return;
+        g_assert_not_reached();
     }
 }
 
@@ -323,9 +317,7 @@ static void power9_set_irq(void *opaque, int pin, int level)
         ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
         break;
     default:
-        /* Unknown pin - do nothing */
-        LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-        return;
+        g_assert_not_reached();
     }
 }
 
@@ -459,9 +451,7 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
             ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
             break;
         default:
-            /* Unknown pin - do nothing */
-            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-            return;
+            g_assert_not_reached();
         }
         if (level)
             env->irq_input_state |= 1 << pin;
@@ -523,9 +513,7 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
             ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
             break;
         default:
-            /* Unknown pin - do nothing */
-            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
-            return;
+            g_assert_not_reached();
         }
         if (level)
             env->irq_input_state |= 1 << pin;
-- 
2.31.1

Re: [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 3236 bytes --]

On Mon, Sep 20, 2021 at 08:12:01AM +0200, Cédric Le Goater wrote:
> If an unknown pin of the IRQ controller is raised, something is very
> wrong in the QEMU model. It is better to abort.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.

> ---
>  hw/ppc/ppc.c | 24 ++++++------------------
>  1 file changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index 7375bf4fa910..a327206a0a1e 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -165,9 +165,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
>              break;
>          default:
> -            /* Unknown pin - do nothing */
> -            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -            return;
> +            g_assert_not_reached();
>          }
>          if (level)
>              env->irq_input_state |= 1 << pin;
> @@ -252,9 +250,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>              /* XXX: TODO */
>              break;
>          default:
> -            /* Unknown pin - do nothing */
> -            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -            return;
> +            g_assert_not_reached();
>          }
>          if (level)
>              env->irq_input_state |= 1 << pin;
> @@ -287,9 +283,7 @@ static void power7_set_irq(void *opaque, int pin, int level)
>          ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>          break;
>      default:
> -        /* Unknown pin - do nothing */
> -        LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -        return;
> +        g_assert_not_reached();
>      }
>  }
>  
> @@ -323,9 +317,7 @@ static void power9_set_irq(void *opaque, int pin, int level)
>          ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
>          break;
>      default:
> -        /* Unknown pin - do nothing */
> -        LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -        return;
> +        g_assert_not_reached();
>      }
>  }
>  
> @@ -459,9 +451,7 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
>              ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>              break;
>          default:
> -            /* Unknown pin - do nothing */
> -            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -            return;
> +            g_assert_not_reached();
>          }
>          if (level)
>              env->irq_input_state |= 1 << pin;
> @@ -523,9 +513,7 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
>              ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>              break;
>          default:
> -            /* Unknown pin - do nothing */
> -            LOG_IRQ("%s: unknown IRQ pin %d\n", __func__, pin);
> -            return;
> +            g_assert_not_reached();
>          }
>          if (level)
>              env->irq_input_state |= 1 << pin;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ)

[Cédric Le Goater]

Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc.c        | 169 ++++++++++++++++----------------------------
 hw/ppc/trace-events |  22 +++++-
 2 files changed, 82 insertions(+), 109 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index a327206a0a1e..b813ef732ec1 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -37,22 +37,6 @@
 #include "migration/vmstate.h"
 #include "trace.h"
 
-//#define PPC_DEBUG_IRQ
-//#define PPC_DEBUG_TB
-
-#ifdef PPC_DEBUG_IRQ
-#  define LOG_IRQ(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
-#else
-#  define LOG_IRQ(...) do { } while (0)
-#endif
-
-
-#ifdef PPC_DEBUG_TB
-#  define LOG_TB(...) qemu_log(__VA_ARGS__)
-#else
-#  define LOG_TB(...) do { } while (0)
-#endif
-
 static void cpu_ppc_tb_stop (CPUPPCState *env);
 static void cpu_ppc_tb_start (CPUPPCState *env);
 
@@ -86,9 +70,8 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
     }
 
 
-    LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
-                "req %08x\n", __func__, env, n_IRQ, level,
-                env->pending_interrupts, CPU(cpu)->interrupt_request);
+    trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
+                           CPU(cpu)->interrupt_request);
 
     if (locked) {
         qemu_mutex_unlock_iothread();
@@ -102,8 +85,8 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
     CPUPPCState *env = &cpu->env;
     int cur_level;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+    trace_ppc_irq_set(env, pin, level);
+
     cur_level = (env->irq_input_state >> pin) & 1;
     /* Don't generate spurious events */
     if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
@@ -112,8 +95,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
         switch (pin) {
         case PPC6xx_INPUT_TBEN:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: %s the time base\n",
-                        __func__, level ? "start" : "stop");
+            trace_ppc_irq_set_state("time base", level);
             if (level) {
                 cpu_ppc_tb_start(env);
             } else {
@@ -122,14 +104,12 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
             break;
         case PPC6xx_INPUT_INT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the external IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("external IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
             break;
         case PPC6xx_INPUT_SMI:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the SMI IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("SMI IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_SMI, level);
             break;
         case PPC6xx_INPUT_MCP:
@@ -138,8 +118,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
              *            603/604/740/750: check HID0[EMCP]
              */
             if (cur_level == 1 && level == 0) {
-                LOG_IRQ("%s: raise machine check state\n",
-                            __func__);
+                trace_ppc_irq_set_state("machine check", 1);
                 ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
             }
             break;
@@ -148,20 +127,19 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
             /* XXX: TODO: relay the signal to CKSTP_OUT pin */
             /* XXX: Note that the only way to restart the CPU is to reset it */
             if (level) {
-                LOG_IRQ("%s: stop the CPU\n", __func__);
+                trace_ppc_irq_cpu("stop");
                 cs->halted = 1;
             }
             break;
         case PPC6xx_INPUT_HRESET:
             /* Level sensitive - active low */
             if (level) {
-                LOG_IRQ("%s: reset the CPU\n", __func__);
+                trace_ppc_irq_reset("CPU");
                 cpu_interrupt(cs, CPU_INTERRUPT_RESET);
             }
             break;
         case PPC6xx_INPUT_SRESET:
-            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("RESET IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
             break;
         default:
@@ -190,8 +168,8 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
     CPUPPCState *env = &cpu->env;
     int cur_level;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+    trace_ppc_irq_set(env, pin, level);
+
     cur_level = (env->irq_input_state >> pin) & 1;
     /* Don't generate spurious events */
     if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
@@ -200,14 +178,12 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
         switch (pin) {
         case PPC970_INPUT_INT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the external IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("external IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
             break;
         case PPC970_INPUT_THINT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the SMI IRQ state to %d\n", __func__,
-                        level);
+            trace_ppc_irq_set_state("SMI IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_THERM, level);
             break;
         case PPC970_INPUT_MCP:
@@ -216,8 +192,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
              *            603/604/740/750: check HID0[EMCP]
              */
             if (cur_level == 1 && level == 0) {
-                LOG_IRQ("%s: raise machine check state\n",
-                            __func__);
+                trace_ppc_irq_set_state("machine check", 1);
                 ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
             }
             break;
@@ -225,10 +200,10 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
             /* Level sensitive - active low */
             /* XXX: TODO: relay the signal to CKSTP_OUT pin */
             if (level) {
-                LOG_IRQ("%s: stop the CPU\n", __func__);
+                trace_ppc_irq_cpu("stop");
                 cs->halted = 1;
             } else {
-                LOG_IRQ("%s: restart the CPU\n", __func__);
+                trace_ppc_irq_cpu("restart");
                 cs->halted = 0;
                 qemu_cpu_kick(cs);
             }
@@ -240,13 +215,11 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
             }
             break;
         case PPC970_INPUT_SRESET:
-            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("RESET IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
             break;
         case PPC970_INPUT_TBEN:
-            LOG_IRQ("%s: set the TBEN state to %d\n", __func__,
-                        level);
+            trace_ppc_irq_set_state("TBEN IRQ", level);
             /* XXX: TODO */
             break;
         default:
@@ -272,14 +245,12 @@ static void power7_set_irq(void *opaque, int pin, int level)
 {
     PowerPCCPU *cpu = opaque;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-            &cpu->env, pin, level);
+    trace_ppc_irq_set(&cpu->env, pin, level);
 
     switch (pin) {
     case POWER7_INPUT_INT:
         /* Level sensitive - active high */
-        LOG_IRQ("%s: set the external IRQ state to %d\n",
-                __func__, level);
+        trace_ppc_irq_set_state("external IRQ", level);
         ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
         break;
     default:
@@ -300,24 +271,22 @@ static void power9_set_irq(void *opaque, int pin, int level)
 {
     PowerPCCPU *cpu = opaque;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-            &cpu->env, pin, level);
+    trace_ppc_irq_set(&cpu->env, pin, level);
 
     switch (pin) {
     case POWER9_INPUT_INT:
         /* Level sensitive - active high */
-        LOG_IRQ("%s: set the external IRQ state to %d\n",
-                __func__, level);
+        trace_ppc_irq_set_state("external IRQ", level);
         ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
         break;
     case POWER9_INPUT_HINT:
         /* Level sensitive - active high */
-        LOG_IRQ("%s: set the external IRQ state to %d\n",
-                __func__, level);
+        trace_ppc_irq_set_state("HV external IRQ", level);
         ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
         break;
     default:
         g_assert_not_reached();
+        return;
     }
 }
 
@@ -393,8 +362,8 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
     CPUPPCState *env = &cpu->env;
     int cur_level;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+    trace_ppc_irq_set(env, pin, level);
+
     cur_level = (env->irq_input_state >> pin) & 1;
     /* Don't generate spurious events */
     if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
@@ -403,51 +372,47 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
         switch (pin) {
         case PPC40x_INPUT_RESET_SYS:
             if (level) {
-                LOG_IRQ("%s: reset the PowerPC system\n",
-                            __func__);
+                trace_ppc_irq_reset("system");
                 ppc40x_system_reset(cpu);
             }
             break;
         case PPC40x_INPUT_RESET_CHIP:
             if (level) {
-                LOG_IRQ("%s: reset the PowerPC chip\n", __func__);
+                trace_ppc_irq_reset("chip");
                 ppc40x_chip_reset(cpu);
             }
             break;
         case PPC40x_INPUT_RESET_CORE:
             /* XXX: TODO: update DBSR[MRR] */
             if (level) {
-                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
+                trace_ppc_irq_reset("core");
                 ppc40x_core_reset(cpu);
             }
             break;
         case PPC40x_INPUT_CINT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the critical IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("critical IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
             break;
         case PPC40x_INPUT_INT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the external IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("external IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
             break;
         case PPC40x_INPUT_HALT:
             /* Level sensitive - active low */
             if (level) {
-                LOG_IRQ("%s: stop the CPU\n", __func__);
+                trace_ppc_irq_cpu("stop");
                 cs->halted = 1;
             } else {
-                LOG_IRQ("%s: restart the CPU\n", __func__);
+                trace_ppc_irq_cpu("restart");
                 cs->halted = 0;
                 qemu_cpu_kick(cs);
             }
             break;
         case PPC40x_INPUT_DEBUG:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the debug pin state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("debug pin", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
             break;
         default:
@@ -475,41 +440,37 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
     CPUPPCState *env = &cpu->env;
     int cur_level;
 
-    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
-                env, pin, level);
+    trace_ppc_irq_set(env, pin, level);
+
     cur_level = (env->irq_input_state >> pin) & 1;
     /* Don't generate spurious events */
     if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
         switch (pin) {
         case PPCE500_INPUT_MCK:
             if (level) {
-                LOG_IRQ("%s: reset the PowerPC system\n",
-                            __func__);
+                trace_ppc_irq_reset("system");
                 qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
             }
             break;
         case PPCE500_INPUT_RESET_CORE:
             if (level) {
-                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
+                trace_ppc_irq_reset("core");
                 ppc_set_irq(cpu, PPC_INTERRUPT_MCK, level);
             }
             break;
         case PPCE500_INPUT_CINT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the critical IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("critical IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
             break;
         case PPCE500_INPUT_INT:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the core IRQ state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("core IRQ", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
             break;
         case PPCE500_INPUT_DEBUG:
             /* Level sensitive - active high */
-            LOG_IRQ("%s: set the debug pin state to %d\n",
-                        __func__, level);
+            trace_ppc_irq_set_state("debug pin", level);
             ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
             break;
         default:
@@ -564,7 +525,7 @@ uint64_t cpu_ppc_load_tbl (CPUPPCState *env)
     }
 
     tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
-    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
+    trace_ppc_tb_load(tb);
 
     return tb;
 }
@@ -575,7 +536,7 @@ static inline uint32_t _cpu_ppc_load_tbu(CPUPPCState *env)
     uint64_t tb;
 
     tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
-    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
+    trace_ppc_tb_load(tb);
 
     return tb >> 32;
 }
@@ -595,8 +556,7 @@ static inline void cpu_ppc_store_tb(ppc_tb_t *tb_env, uint64_t vmclk,
     *tb_offsetp = value -
         muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND);
 
-    LOG_TB("%s: tb %016" PRIx64 " offset %08" PRIx64 "\n",
-                __func__, value, *tb_offsetp);
+    trace_ppc_tb_store(value, *tb_offsetp);
 }
 
 void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value)
@@ -632,7 +592,7 @@ uint64_t cpu_ppc_load_atbl (CPUPPCState *env)
     uint64_t tb;
 
     tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
-    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
+    trace_ppc_tb_load(tb);
 
     return tb;
 }
@@ -643,7 +603,7 @@ uint32_t cpu_ppc_load_atbu (CPUPPCState *env)
     uint64_t tb;
 
     tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
-    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
+    trace_ppc_tb_load(tb);
 
     return tb >> 32;
 }
@@ -762,7 +722,7 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
     }  else {
         decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
     }
-    LOG_TB("%s: %016" PRIx64 "\n", __func__, decr);
+    trace_ppc_decr_load(decr);
 
     return decr;
 }
@@ -821,7 +781,7 @@ uint64_t cpu_ppc_load_purr (CPUPPCState *env)
 static inline void cpu_ppc_decr_excp(PowerPCCPU *cpu)
 {
     /* Raise it */
-    LOG_TB("raise decrementer exception\n");
+    trace_ppc_decr_excp("raise");
     ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 1);
 }
 
@@ -835,7 +795,7 @@ static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
     CPUPPCState *env = &cpu->env;
 
     /* Raise it */
-    LOG_TB("raise hv decrementer exception\n");
+    trace_ppc_decr_excp("raise HV");
 
     /* The architecture specifies that we don't deliver HDEC
      * interrupts in a PM state. Not only they don't cause a
@@ -870,8 +830,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
         value |= (0xFFFFFFFFULL << nr_bits);
     }
 
-    LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
-                decr, value);
+    trace_ppc_decr_store(nr_bits, decr, value);
 
     if (kvm_enabled()) {
         /* KVM handles decrementer exceptions, we don't need our own timer */
@@ -1199,9 +1158,8 @@ static void cpu_4xx_fit_cb (void *opaque)
     if ((env->spr[SPR_40x_TCR] >> 23) & 0x1) {
         ppc_set_irq(cpu, PPC_INTERRUPT_FIT, 1);
     }
-    LOG_TB("%s: ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
-           (int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
-           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
+    trace_ppc4xx_fit((int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
+                         env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
 }
 
 /* Programmable interval timer */
@@ -1215,11 +1173,10 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
         !((env->spr[SPR_40x_TCR] >> 26) & 0x1) ||
         (is_excp && !((env->spr[SPR_40x_TCR] >> 22) & 0x1))) {
         /* Stop PIT */
-        LOG_TB("%s: stop PIT\n", __func__);
+        trace_ppc4xx_pit_stop();
         timer_del(tb_env->decr_timer);
     } else {
-        LOG_TB("%s: start PIT %016" PRIx64 "\n",
-                    __func__, ppc40x_timer->pit_reload);
+        trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
         now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
         next = now + muldiv64(ppc40x_timer->pit_reload,
                               NANOSECONDS_PER_SECOND, tb_env->decr_freq);
@@ -1248,9 +1205,7 @@ static void cpu_4xx_pit_cb (void *opaque)
         ppc_set_irq(cpu, ppc40x_timer->decr_excp, 1);
     }
     start_stop_pit(env, tb_env, 1);
-    LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
-           "%016" PRIx64 "\n", __func__,
-           (int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
+    trace_ppc4xx_pit((int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
            (int)((env->spr[SPR_40x_TCR] >> 26) & 0x1),
            env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
            ppc40x_timer->pit_reload);
@@ -1290,8 +1245,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
     next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
     if (next == now)
         next++;
-    LOG_TB("%s: TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
-           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
+    trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
     switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
     case 0x0:
     case 0x1:
@@ -1334,7 +1288,7 @@ void store_40x_pit (CPUPPCState *env, target_ulong val)
 
     tb_env = env->tb_env;
     ppc40x_timer = tb_env->opaque;
-    LOG_TB("%s val" TARGET_FMT_lx "\n", __func__, val);
+    trace_ppc40x_store_pit(val);
     ppc40x_timer->pit_reload = val;
     start_stop_pit(env, tb_env, 0);
 }
@@ -1349,8 +1303,7 @@ static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
     CPUPPCState *env = opaque;
     ppc_tb_t *tb_env = env->tb_env;
 
-    LOG_TB("%s set new frequency to %" PRIu32 "\n", __func__,
-                freq);
+    trace_ppc40x_set_tb_clk(freq);
     tb_env->tb_freq = freq;
     tb_env->decr_freq = freq;
     /* XXX: we should also update all timers */
@@ -1369,7 +1322,7 @@ clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
     tb_env->tb_freq = freq;
     tb_env->decr_freq = freq;
     tb_env->opaque = ppc40x_timer;
-    LOG_TB("%s freq %" PRIu32 "\n", __func__, freq);
+    trace_ppc40x_timers_init(freq);
     if (ppc40x_timer != NULL) {
         /* We use decr timer for PIT */
         tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, env);
diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
index da6e74b80dc1..3bf43fa340fe 100644
--- a/hw/ppc/trace-events
+++ b/hw/ppc/trace-events
@@ -97,7 +97,27 @@ vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx6
 
 # ppc.c
 ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
-
+ppc_tb_load(uint64_t tb) "tb 0x%016" PRIx64
+ppc_tb_store(uint64_t tb, uint64_t offset) "tb 0x%016" PRIx64 " offset 0x%08" PRIx64
+
+ppc_decr_load(uint64_t tb) "decr 0x%016" PRIx64
+ppc_decr_excp(const char *action) "%s decrementer"
+ppc_decr_store(uint32_t nr_bits, uint64_t decr, uint64_t value) "%d-bit 0x%016" PRIx64 " => 0x%016" PRIx64
+
+ppc4xx_fit(uint32_t ir, uint64_t tcr, uint64_t tsr) "ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64
+ppc4xx_pit_stop(void) ""
+ppc4xx_pit_start(uint64_t reload) "PIT 0x%016" PRIx64
+ppc4xx_pit(uint32_t ar, uint32_t ir, uint64_t tcr, uint64_t tsr, uint64_t reload) "ar %d ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64 " PIT 0x%016" PRIx64
+ppc4xx_wdt(uint64_t tcr, uint64_t tsr) "TCR 0x%" PRIx64 " TSR 0x%" PRIx64
+ppc40x_store_pit(uint64_t value) "val 0x%" PRIx64
+ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
+ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
+
+ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
+ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
+ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
+ppc_irq_reset(const char *name) "%s"
+ppc_irq_cpu(const char *action) "%s"
 
 # prep_systemio.c
 prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
-- 
2.31.1

Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 24002 bytes --]

On Mon, Sep 20, 2021 at 08:12:02AM +0200, Cédric Le Goater wrote:
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

So.. all the functions here are called *set_irq*, but you've named the
tracepoints *irq_set*.  It's not a big deal, but it seems like it
would be less confusing if you flipped that around to match the
function names.

> ---
>  hw/ppc/ppc.c        | 169 ++++++++++++++++----------------------------
>  hw/ppc/trace-events |  22 +++++-
>  2 files changed, 82 insertions(+), 109 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index a327206a0a1e..b813ef732ec1 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -37,22 +37,6 @@
>  #include "migration/vmstate.h"
>  #include "trace.h"
>  
> -//#define PPC_DEBUG_IRQ
> -//#define PPC_DEBUG_TB
> -
> -#ifdef PPC_DEBUG_IRQ
> -#  define LOG_IRQ(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
> -#else
> -#  define LOG_IRQ(...) do { } while (0)
> -#endif
> -
> -
> -#ifdef PPC_DEBUG_TB
> -#  define LOG_TB(...) qemu_log(__VA_ARGS__)
> -#else
> -#  define LOG_TB(...) do { } while (0)
> -#endif
> -
>  static void cpu_ppc_tb_stop (CPUPPCState *env);
>  static void cpu_ppc_tb_start (CPUPPCState *env);
>  
> @@ -86,9 +70,8 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
>      }
>  
>  
> -    LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
> -                "req %08x\n", __func__, env, n_IRQ, level,
> -                env->pending_interrupts, CPU(cpu)->interrupt_request);
> +    trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
> +                           CPU(cpu)->interrupt_request);
>  
>      if (locked) {
>          qemu_mutex_unlock_iothread();
> @@ -102,8 +85,8 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>      CPUPPCState *env = &cpu->env;
>      int cur_level;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -                env, pin, level);
> +    trace_ppc_irq_set(env, pin, level);
> +
>      cur_level = (env->irq_input_state >> pin) & 1;
>      /* Don't generate spurious events */
>      if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> @@ -112,8 +95,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>          switch (pin) {
>          case PPC6xx_INPUT_TBEN:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: %s the time base\n",
> -                        __func__, level ? "start" : "stop");
> +            trace_ppc_irq_set_state("time base", level);
>              if (level) {
>                  cpu_ppc_tb_start(env);
>              } else {
> @@ -122,14 +104,12 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              break;
>          case PPC6xx_INPUT_INT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("external IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>              break;
>          case PPC6xx_INPUT_SMI:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the SMI IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("SMI IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_SMI, level);
>              break;
>          case PPC6xx_INPUT_MCP:
> @@ -138,8 +118,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>               *            603/604/740/750: check HID0[EMCP]
>               */
>              if (cur_level == 1 && level == 0) {
> -                LOG_IRQ("%s: raise machine check state\n",
> -                            __func__);
> +                trace_ppc_irq_set_state("machine check", 1);
>                  ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
>              }
>              break;
> @@ -148,20 +127,19 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>              /* XXX: TODO: relay the signal to CKSTP_OUT pin */
>              /* XXX: Note that the only way to restart the CPU is to reset it */
>              if (level) {
> -                LOG_IRQ("%s: stop the CPU\n", __func__);
> +                trace_ppc_irq_cpu("stop");
>                  cs->halted = 1;
>              }
>              break;
>          case PPC6xx_INPUT_HRESET:
>              /* Level sensitive - active low */
>              if (level) {
> -                LOG_IRQ("%s: reset the CPU\n", __func__);
> +                trace_ppc_irq_reset("CPU");
>                  cpu_interrupt(cs, CPU_INTERRUPT_RESET);
>              }
>              break;
>          case PPC6xx_INPUT_SRESET:
> -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("RESET IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
>              break;
>          default:
> @@ -190,8 +168,8 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>      CPUPPCState *env = &cpu->env;
>      int cur_level;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -                env, pin, level);
> +    trace_ppc_irq_set(env, pin, level);
> +
>      cur_level = (env->irq_input_state >> pin) & 1;
>      /* Don't generate spurious events */
>      if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> @@ -200,14 +178,12 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>          switch (pin) {
>          case PPC970_INPUT_INT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("external IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>              break;
>          case PPC970_INPUT_THINT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the SMI IRQ state to %d\n", __func__,
> -                        level);
> +            trace_ppc_irq_set_state("SMI IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_THERM, level);
>              break;
>          case PPC970_INPUT_MCP:
> @@ -216,8 +192,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>               *            603/604/740/750: check HID0[EMCP]
>               */
>              if (cur_level == 1 && level == 0) {
> -                LOG_IRQ("%s: raise machine check state\n",
> -                            __func__);
> +                trace_ppc_irq_set_state("machine check", 1);
>                  ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
>              }
>              break;
> @@ -225,10 +200,10 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>              /* Level sensitive - active low */
>              /* XXX: TODO: relay the signal to CKSTP_OUT pin */
>              if (level) {
> -                LOG_IRQ("%s: stop the CPU\n", __func__);
> +                trace_ppc_irq_cpu("stop");
>                  cs->halted = 1;
>              } else {
> -                LOG_IRQ("%s: restart the CPU\n", __func__);
> +                trace_ppc_irq_cpu("restart");
>                  cs->halted = 0;
>                  qemu_cpu_kick(cs);
>              }
> @@ -240,13 +215,11 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>              }
>              break;
>          case PPC970_INPUT_SRESET:
> -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("RESET IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
>              break;
>          case PPC970_INPUT_TBEN:
> -            LOG_IRQ("%s: set the TBEN state to %d\n", __func__,
> -                        level);
> +            trace_ppc_irq_set_state("TBEN IRQ", level);
>              /* XXX: TODO */
>              break;
>          default:
> @@ -272,14 +245,12 @@ static void power7_set_irq(void *opaque, int pin, int level)
>  {
>      PowerPCCPU *cpu = opaque;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -            &cpu->env, pin, level);
> +    trace_ppc_irq_set(&cpu->env, pin, level);
>  
>      switch (pin) {
>      case POWER7_INPUT_INT:
>          /* Level sensitive - active high */
> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                __func__, level);
> +        trace_ppc_irq_set_state("external IRQ", level);
>          ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>          break;
>      default:
> @@ -300,24 +271,22 @@ static void power9_set_irq(void *opaque, int pin, int level)
>  {
>      PowerPCCPU *cpu = opaque;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -            &cpu->env, pin, level);
> +    trace_ppc_irq_set(&cpu->env, pin, level);
>  
>      switch (pin) {
>      case POWER9_INPUT_INT:
>          /* Level sensitive - active high */
> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                __func__, level);
> +        trace_ppc_irq_set_state("external IRQ", level);
>          ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>          break;
>      case POWER9_INPUT_HINT:
>          /* Level sensitive - active high */
> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                __func__, level);
> +        trace_ppc_irq_set_state("HV external IRQ", level);
>          ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
>          break;
>      default:
>          g_assert_not_reached();
> +        return;
>      }
>  }
>  
> @@ -393,8 +362,8 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
>      CPUPPCState *env = &cpu->env;
>      int cur_level;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -                env, pin, level);
> +    trace_ppc_irq_set(env, pin, level);
> +
>      cur_level = (env->irq_input_state >> pin) & 1;
>      /* Don't generate spurious events */
>      if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> @@ -403,51 +372,47 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
>          switch (pin) {
>          case PPC40x_INPUT_RESET_SYS:
>              if (level) {
> -                LOG_IRQ("%s: reset the PowerPC system\n",
> -                            __func__);
> +                trace_ppc_irq_reset("system");
>                  ppc40x_system_reset(cpu);
>              }
>              break;
>          case PPC40x_INPUT_RESET_CHIP:
>              if (level) {
> -                LOG_IRQ("%s: reset the PowerPC chip\n", __func__);
> +                trace_ppc_irq_reset("chip");
>                  ppc40x_chip_reset(cpu);
>              }
>              break;
>          case PPC40x_INPUT_RESET_CORE:
>              /* XXX: TODO: update DBSR[MRR] */
>              if (level) {
> -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
> +                trace_ppc_irq_reset("core");
>                  ppc40x_core_reset(cpu);
>              }
>              break;
>          case PPC40x_INPUT_CINT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("critical IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
>              break;
>          case PPC40x_INPUT_INT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("external IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>              break;
>          case PPC40x_INPUT_HALT:
>              /* Level sensitive - active low */
>              if (level) {
> -                LOG_IRQ("%s: stop the CPU\n", __func__);
> +                trace_ppc_irq_cpu("stop");
>                  cs->halted = 1;
>              } else {
> -                LOG_IRQ("%s: restart the CPU\n", __func__);
> +                trace_ppc_irq_cpu("restart");
>                  cs->halted = 0;
>                  qemu_cpu_kick(cs);
>              }
>              break;
>          case PPC40x_INPUT_DEBUG:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the debug pin state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("debug pin", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>              break;
>          default:
> @@ -475,41 +440,37 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
>      CPUPPCState *env = &cpu->env;
>      int cur_level;
>  
> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> -                env, pin, level);
> +    trace_ppc_irq_set(env, pin, level);
> +
>      cur_level = (env->irq_input_state >> pin) & 1;
>      /* Don't generate spurious events */
>      if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
>          switch (pin) {
>          case PPCE500_INPUT_MCK:
>              if (level) {
> -                LOG_IRQ("%s: reset the PowerPC system\n",
> -                            __func__);
> +                trace_ppc_irq_reset("system");
>                  qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>              }
>              break;
>          case PPCE500_INPUT_RESET_CORE:
>              if (level) {
> -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
> +                trace_ppc_irq_reset("core");
>                  ppc_set_irq(cpu, PPC_INTERRUPT_MCK, level);
>              }
>              break;
>          case PPCE500_INPUT_CINT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("critical IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
>              break;
>          case PPCE500_INPUT_INT:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the core IRQ state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("core IRQ", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>              break;
>          case PPCE500_INPUT_DEBUG:
>              /* Level sensitive - active high */
> -            LOG_IRQ("%s: set the debug pin state to %d\n",
> -                        __func__, level);
> +            trace_ppc_irq_set_state("debug pin", level);
>              ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>              break;
>          default:
> @@ -564,7 +525,7 @@ uint64_t cpu_ppc_load_tbl (CPUPPCState *env)
>      }
>  
>      tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> +    trace_ppc_tb_load(tb);
>  
>      return tb;
>  }
> @@ -575,7 +536,7 @@ static inline uint32_t _cpu_ppc_load_tbu(CPUPPCState *env)
>      uint64_t tb;
>  
>      tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> +    trace_ppc_tb_load(tb);
>  
>      return tb >> 32;
>  }
> @@ -595,8 +556,7 @@ static inline void cpu_ppc_store_tb(ppc_tb_t *tb_env, uint64_t vmclk,
>      *tb_offsetp = value -
>          muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND);
>  
> -    LOG_TB("%s: tb %016" PRIx64 " offset %08" PRIx64 "\n",
> -                __func__, value, *tb_offsetp);
> +    trace_ppc_tb_store(value, *tb_offsetp);
>  }
>  
>  void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value)
> @@ -632,7 +592,7 @@ uint64_t cpu_ppc_load_atbl (CPUPPCState *env)
>      uint64_t tb;
>  
>      tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> +    trace_ppc_tb_load(tb);
>  
>      return tb;
>  }
> @@ -643,7 +603,7 @@ uint32_t cpu_ppc_load_atbu (CPUPPCState *env)
>      uint64_t tb;
>  
>      tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> +    trace_ppc_tb_load(tb);
>  
>      return tb >> 32;
>  }
> @@ -762,7 +722,7 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
>      }  else {
>          decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
>      }
> -    LOG_TB("%s: %016" PRIx64 "\n", __func__, decr);
> +    trace_ppc_decr_load(decr);
>  
>      return decr;
>  }
> @@ -821,7 +781,7 @@ uint64_t cpu_ppc_load_purr (CPUPPCState *env)
>  static inline void cpu_ppc_decr_excp(PowerPCCPU *cpu)
>  {
>      /* Raise it */
> -    LOG_TB("raise decrementer exception\n");
> +    trace_ppc_decr_excp("raise");
>      ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 1);
>  }
>  
> @@ -835,7 +795,7 @@ static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
>      CPUPPCState *env = &cpu->env;
>  
>      /* Raise it */
> -    LOG_TB("raise hv decrementer exception\n");
> +    trace_ppc_decr_excp("raise HV");
>  
>      /* The architecture specifies that we don't deliver HDEC
>       * interrupts in a PM state. Not only they don't cause a
> @@ -870,8 +830,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
>          value |= (0xFFFFFFFFULL << nr_bits);
>      }
>  
> -    LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
> -                decr, value);
> +    trace_ppc_decr_store(nr_bits, decr, value);
>  
>      if (kvm_enabled()) {
>          /* KVM handles decrementer exceptions, we don't need our own timer */
> @@ -1199,9 +1158,8 @@ static void cpu_4xx_fit_cb (void *opaque)
>      if ((env->spr[SPR_40x_TCR] >> 23) & 0x1) {
>          ppc_set_irq(cpu, PPC_INTERRUPT_FIT, 1);
>      }
> -    LOG_TB("%s: ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
> -           (int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
> -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> +    trace_ppc4xx_fit((int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
> +                         env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>  }
>  
>  /* Programmable interval timer */
> @@ -1215,11 +1173,10 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
>          !((env->spr[SPR_40x_TCR] >> 26) & 0x1) ||
>          (is_excp && !((env->spr[SPR_40x_TCR] >> 22) & 0x1))) {
>          /* Stop PIT */
> -        LOG_TB("%s: stop PIT\n", __func__);
> +        trace_ppc4xx_pit_stop();
>          timer_del(tb_env->decr_timer);
>      } else {
> -        LOG_TB("%s: start PIT %016" PRIx64 "\n",
> -                    __func__, ppc40x_timer->pit_reload);
> +        trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
>          now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>          next = now + muldiv64(ppc40x_timer->pit_reload,
>                                NANOSECONDS_PER_SECOND, tb_env->decr_freq);
> @@ -1248,9 +1205,7 @@ static void cpu_4xx_pit_cb (void *opaque)
>          ppc_set_irq(cpu, ppc40x_timer->decr_excp, 1);
>      }
>      start_stop_pit(env, tb_env, 1);
> -    LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
> -           "%016" PRIx64 "\n", __func__,
> -           (int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
> +    trace_ppc4xx_pit((int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
>             (int)((env->spr[SPR_40x_TCR] >> 26) & 0x1),
>             env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
>             ppc40x_timer->pit_reload);
> @@ -1290,8 +1245,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
>      next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
>      if (next == now)
>          next++;
> -    LOG_TB("%s: TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
> -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> +    trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>      switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
>      case 0x0:
>      case 0x1:
> @@ -1334,7 +1288,7 @@ void store_40x_pit (CPUPPCState *env, target_ulong val)
>  
>      tb_env = env->tb_env;
>      ppc40x_timer = tb_env->opaque;
> -    LOG_TB("%s val" TARGET_FMT_lx "\n", __func__, val);
> +    trace_ppc40x_store_pit(val);
>      ppc40x_timer->pit_reload = val;
>      start_stop_pit(env, tb_env, 0);
>  }
> @@ -1349,8 +1303,7 @@ static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
>      CPUPPCState *env = opaque;
>      ppc_tb_t *tb_env = env->tb_env;
>  
> -    LOG_TB("%s set new frequency to %" PRIu32 "\n", __func__,
> -                freq);
> +    trace_ppc40x_set_tb_clk(freq);
>      tb_env->tb_freq = freq;
>      tb_env->decr_freq = freq;
>      /* XXX: we should also update all timers */
> @@ -1369,7 +1322,7 @@ clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
>      tb_env->tb_freq = freq;
>      tb_env->decr_freq = freq;
>      tb_env->opaque = ppc40x_timer;
> -    LOG_TB("%s freq %" PRIu32 "\n", __func__, freq);
> +    trace_ppc40x_timers_init(freq);
>      if (ppc40x_timer != NULL) {
>          /* We use decr timer for PIT */
>          tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, env);
> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> index da6e74b80dc1..3bf43fa340fe 100644
> --- a/hw/ppc/trace-events
> +++ b/hw/ppc/trace-events
> @@ -97,7 +97,27 @@ vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx6
>  
>  # ppc.c
>  ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
> -
> +ppc_tb_load(uint64_t tb) "tb 0x%016" PRIx64
> +ppc_tb_store(uint64_t tb, uint64_t offset) "tb 0x%016" PRIx64 " offset 0x%08" PRIx64
> +
> +ppc_decr_load(uint64_t tb) "decr 0x%016" PRIx64
> +ppc_decr_excp(const char *action) "%s decrementer"
> +ppc_decr_store(uint32_t nr_bits, uint64_t decr, uint64_t value) "%d-bit 0x%016" PRIx64 " => 0x%016" PRIx64
> +
> +ppc4xx_fit(uint32_t ir, uint64_t tcr, uint64_t tsr) "ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64
> +ppc4xx_pit_stop(void) ""
> +ppc4xx_pit_start(uint64_t reload) "PIT 0x%016" PRIx64
> +ppc4xx_pit(uint32_t ar, uint32_t ir, uint64_t tcr, uint64_t tsr, uint64_t reload) "ar %d ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64 " PIT 0x%016" PRIx64
> +ppc4xx_wdt(uint64_t tcr, uint64_t tsr) "TCR 0x%" PRIx64 " TSR 0x%" PRIx64
> +ppc40x_store_pit(uint64_t value) "val 0x%" PRIx64
> +ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
> +ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
> +
> +ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
> +ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
> +ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
> +ppc_irq_reset(const char *name) "%s"
> +ppc_irq_cpu(const char *action) "%s"
>  
>  # prep_systemio.c
>  prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ)

[Cédric Le Goater]

On 9/20/21 09:34, David Gibson wrote:
> On Mon, Sep 20, 2021 at 08:12:02AM +0200, Cédric Le Goater wrote:
>> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> 
> So.. all the functions here are called *set_irq*, but you've named the
> tracepoints *irq_set*.


Yes. I did that on purpose to identify easily the PPC IRQ subsystem,
and to be able to use '-trace ppc_irq*' from the command line and in
the monitor.

> It's not a big deal, but it seems like it
> would be less confusing if you flipped that around to match the
> function names.

but you would loose the ability to do what I described above.

Thanks,

C.
  
>> ---
>>   hw/ppc/ppc.c        | 169 ++++++++++++++++----------------------------
>>   hw/ppc/trace-events |  22 +++++-
>>   2 files changed, 82 insertions(+), 109 deletions(-)
>>
>> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
>> index a327206a0a1e..b813ef732ec1 100644
>> --- a/hw/ppc/ppc.c
>> +++ b/hw/ppc/ppc.c
>> @@ -37,22 +37,6 @@
>>   #include "migration/vmstate.h"
>>   #include "trace.h"
>>   
>> -//#define PPC_DEBUG_IRQ
>> -//#define PPC_DEBUG_TB
>> -
>> -#ifdef PPC_DEBUG_IRQ
>> -#  define LOG_IRQ(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
>> -#else
>> -#  define LOG_IRQ(...) do { } while (0)
>> -#endif
>> -
>> -
>> -#ifdef PPC_DEBUG_TB
>> -#  define LOG_TB(...) qemu_log(__VA_ARGS__)
>> -#else
>> -#  define LOG_TB(...) do { } while (0)
>> -#endif
>> -
>>   static void cpu_ppc_tb_stop (CPUPPCState *env);
>>   static void cpu_ppc_tb_start (CPUPPCState *env);
>>   
>> @@ -86,9 +70,8 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
>>       }
>>   
>>   
>> -    LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
>> -                "req %08x\n", __func__, env, n_IRQ, level,
>> -                env->pending_interrupts, CPU(cpu)->interrupt_request);
>> +    trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
>> +                           CPU(cpu)->interrupt_request);
>>   
>>       if (locked) {
>>           qemu_mutex_unlock_iothread();
>> @@ -102,8 +85,8 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>>       CPUPPCState *env = &cpu->env;
>>       int cur_level;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -                env, pin, level);
>> +    trace_ppc_irq_set(env, pin, level);
>> +
>>       cur_level = (env->irq_input_state >> pin) & 1;
>>       /* Don't generate spurious events */
>>       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
>> @@ -112,8 +95,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>>           switch (pin) {
>>           case PPC6xx_INPUT_TBEN:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: %s the time base\n",
>> -                        __func__, level ? "start" : "stop");
>> +            trace_ppc_irq_set_state("time base", level);
>>               if (level) {
>>                   cpu_ppc_tb_start(env);
>>               } else {
>> @@ -122,14 +104,12 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>>               break;
>>           case PPC6xx_INPUT_INT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("external IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>               break;
>>           case PPC6xx_INPUT_SMI:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the SMI IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("SMI IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_SMI, level);
>>               break;
>>           case PPC6xx_INPUT_MCP:
>> @@ -138,8 +118,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>>                *            603/604/740/750: check HID0[EMCP]
>>                */
>>               if (cur_level == 1 && level == 0) {
>> -                LOG_IRQ("%s: raise machine check state\n",
>> -                            __func__);
>> +                trace_ppc_irq_set_state("machine check", 1);
>>                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
>>               }
>>               break;
>> @@ -148,20 +127,19 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
>>               /* XXX: TODO: relay the signal to CKSTP_OUT pin */
>>               /* XXX: Note that the only way to restart the CPU is to reset it */
>>               if (level) {
>> -                LOG_IRQ("%s: stop the CPU\n", __func__);
>> +                trace_ppc_irq_cpu("stop");
>>                   cs->halted = 1;
>>               }
>>               break;
>>           case PPC6xx_INPUT_HRESET:
>>               /* Level sensitive - active low */
>>               if (level) {
>> -                LOG_IRQ("%s: reset the CPU\n", __func__);
>> +                trace_ppc_irq_reset("CPU");
>>                   cpu_interrupt(cs, CPU_INTERRUPT_RESET);
>>               }
>>               break;
>>           case PPC6xx_INPUT_SRESET:
>> -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("RESET IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
>>               break;
>>           default:
>> @@ -190,8 +168,8 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>>       CPUPPCState *env = &cpu->env;
>>       int cur_level;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -                env, pin, level);
>> +    trace_ppc_irq_set(env, pin, level);
>> +
>>       cur_level = (env->irq_input_state >> pin) & 1;
>>       /* Don't generate spurious events */
>>       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
>> @@ -200,14 +178,12 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>>           switch (pin) {
>>           case PPC970_INPUT_INT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("external IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>               break;
>>           case PPC970_INPUT_THINT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the SMI IRQ state to %d\n", __func__,
>> -                        level);
>> +            trace_ppc_irq_set_state("SMI IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_THERM, level);
>>               break;
>>           case PPC970_INPUT_MCP:
>> @@ -216,8 +192,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>>                *            603/604/740/750: check HID0[EMCP]
>>                */
>>               if (cur_level == 1 && level == 0) {
>> -                LOG_IRQ("%s: raise machine check state\n",
>> -                            __func__);
>> +                trace_ppc_irq_set_state("machine check", 1);
>>                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
>>               }
>>               break;
>> @@ -225,10 +200,10 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>>               /* Level sensitive - active low */
>>               /* XXX: TODO: relay the signal to CKSTP_OUT pin */
>>               if (level) {
>> -                LOG_IRQ("%s: stop the CPU\n", __func__);
>> +                trace_ppc_irq_cpu("stop");
>>                   cs->halted = 1;
>>               } else {
>> -                LOG_IRQ("%s: restart the CPU\n", __func__);
>> +                trace_ppc_irq_cpu("restart");
>>                   cs->halted = 0;
>>                   qemu_cpu_kick(cs);
>>               }
>> @@ -240,13 +215,11 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
>>               }
>>               break;
>>           case PPC970_INPUT_SRESET:
>> -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("RESET IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
>>               break;
>>           case PPC970_INPUT_TBEN:
>> -            LOG_IRQ("%s: set the TBEN state to %d\n", __func__,
>> -                        level);
>> +            trace_ppc_irq_set_state("TBEN IRQ", level);
>>               /* XXX: TODO */
>>               break;
>>           default:
>> @@ -272,14 +245,12 @@ static void power7_set_irq(void *opaque, int pin, int level)
>>   {
>>       PowerPCCPU *cpu = opaque;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -            &cpu->env, pin, level);
>> +    trace_ppc_irq_set(&cpu->env, pin, level);
>>   
>>       switch (pin) {
>>       case POWER7_INPUT_INT:
>>           /* Level sensitive - active high */
>> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                __func__, level);
>> +        trace_ppc_irq_set_state("external IRQ", level);
>>           ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>           break;
>>       default:
>> @@ -300,24 +271,22 @@ static void power9_set_irq(void *opaque, int pin, int level)
>>   {
>>       PowerPCCPU *cpu = opaque;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -            &cpu->env, pin, level);
>> +    trace_ppc_irq_set(&cpu->env, pin, level);
>>   
>>       switch (pin) {
>>       case POWER9_INPUT_INT:
>>           /* Level sensitive - active high */
>> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                __func__, level);
>> +        trace_ppc_irq_set_state("external IRQ", level);
>>           ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>           break;
>>       case POWER9_INPUT_HINT:
>>           /* Level sensitive - active high */
>> -        LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                __func__, level);
>> +        trace_ppc_irq_set_state("HV external IRQ", level);
>>           ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
>>           break;
>>       default:
>>           g_assert_not_reached();
>> +        return;
>>       }
>>   }
>>   
>> @@ -393,8 +362,8 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
>>       CPUPPCState *env = &cpu->env;
>>       int cur_level;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -                env, pin, level);
>> +    trace_ppc_irq_set(env, pin, level);
>> +
>>       cur_level = (env->irq_input_state >> pin) & 1;
>>       /* Don't generate spurious events */
>>       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
>> @@ -403,51 +372,47 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
>>           switch (pin) {
>>           case PPC40x_INPUT_RESET_SYS:
>>               if (level) {
>> -                LOG_IRQ("%s: reset the PowerPC system\n",
>> -                            __func__);
>> +                trace_ppc_irq_reset("system");
>>                   ppc40x_system_reset(cpu);
>>               }
>>               break;
>>           case PPC40x_INPUT_RESET_CHIP:
>>               if (level) {
>> -                LOG_IRQ("%s: reset the PowerPC chip\n", __func__);
>> +                trace_ppc_irq_reset("chip");
>>                   ppc40x_chip_reset(cpu);
>>               }
>>               break;
>>           case PPC40x_INPUT_RESET_CORE:
>>               /* XXX: TODO: update DBSR[MRR] */
>>               if (level) {
>> -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
>> +                trace_ppc_irq_reset("core");
>>                   ppc40x_core_reset(cpu);
>>               }
>>               break;
>>           case PPC40x_INPUT_CINT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("critical IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
>>               break;
>>           case PPC40x_INPUT_INT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the external IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("external IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>               break;
>>           case PPC40x_INPUT_HALT:
>>               /* Level sensitive - active low */
>>               if (level) {
>> -                LOG_IRQ("%s: stop the CPU\n", __func__);
>> +                trace_ppc_irq_cpu("stop");
>>                   cs->halted = 1;
>>               } else {
>> -                LOG_IRQ("%s: restart the CPU\n", __func__);
>> +                trace_ppc_irq_cpu("restart");
>>                   cs->halted = 0;
>>                   qemu_cpu_kick(cs);
>>               }
>>               break;
>>           case PPC40x_INPUT_DEBUG:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the debug pin state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("debug pin", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>>               break;
>>           default:
>> @@ -475,41 +440,37 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
>>       CPUPPCState *env = &cpu->env;
>>       int cur_level;
>>   
>> -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
>> -                env, pin, level);
>> +    trace_ppc_irq_set(env, pin, level);
>> +
>>       cur_level = (env->irq_input_state >> pin) & 1;
>>       /* Don't generate spurious events */
>>       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
>>           switch (pin) {
>>           case PPCE500_INPUT_MCK:
>>               if (level) {
>> -                LOG_IRQ("%s: reset the PowerPC system\n",
>> -                            __func__);
>> +                trace_ppc_irq_reset("system");
>>                   qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
>>               }
>>               break;
>>           case PPCE500_INPUT_RESET_CORE:
>>               if (level) {
>> -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
>> +                trace_ppc_irq_reset("core");
>>                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, level);
>>               }
>>               break;
>>           case PPCE500_INPUT_CINT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("critical IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
>>               break;
>>           case PPCE500_INPUT_INT:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the core IRQ state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("core IRQ", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
>>               break;
>>           case PPCE500_INPUT_DEBUG:
>>               /* Level sensitive - active high */
>> -            LOG_IRQ("%s: set the debug pin state to %d\n",
>> -                        __func__, level);
>> +            trace_ppc_irq_set_state("debug pin", level);
>>               ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
>>               break;
>>           default:
>> @@ -564,7 +525,7 @@ uint64_t cpu_ppc_load_tbl (CPUPPCState *env)
>>       }
>>   
>>       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
>> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
>> +    trace_ppc_tb_load(tb);
>>   
>>       return tb;
>>   }
>> @@ -575,7 +536,7 @@ static inline uint32_t _cpu_ppc_load_tbu(CPUPPCState *env)
>>       uint64_t tb;
>>   
>>       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
>> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
>> +    trace_ppc_tb_load(tb);
>>   
>>       return tb >> 32;
>>   }
>> @@ -595,8 +556,7 @@ static inline void cpu_ppc_store_tb(ppc_tb_t *tb_env, uint64_t vmclk,
>>       *tb_offsetp = value -
>>           muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND);
>>   
>> -    LOG_TB("%s: tb %016" PRIx64 " offset %08" PRIx64 "\n",
>> -                __func__, value, *tb_offsetp);
>> +    trace_ppc_tb_store(value, *tb_offsetp);
>>   }
>>   
>>   void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value)
>> @@ -632,7 +592,7 @@ uint64_t cpu_ppc_load_atbl (CPUPPCState *env)
>>       uint64_t tb;
>>   
>>       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
>> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
>> +    trace_ppc_tb_load(tb);
>>   
>>       return tb;
>>   }
>> @@ -643,7 +603,7 @@ uint32_t cpu_ppc_load_atbu (CPUPPCState *env)
>>       uint64_t tb;
>>   
>>       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
>> -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
>> +    trace_ppc_tb_load(tb);
>>   
>>       return tb >> 32;
>>   }
>> @@ -762,7 +722,7 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
>>       }  else {
>>           decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
>>       }
>> -    LOG_TB("%s: %016" PRIx64 "\n", __func__, decr);
>> +    trace_ppc_decr_load(decr);
>>   
>>       return decr;
>>   }
>> @@ -821,7 +781,7 @@ uint64_t cpu_ppc_load_purr (CPUPPCState *env)
>>   static inline void cpu_ppc_decr_excp(PowerPCCPU *cpu)
>>   {
>>       /* Raise it */
>> -    LOG_TB("raise decrementer exception\n");
>> +    trace_ppc_decr_excp("raise");
>>       ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 1);
>>   }
>>   
>> @@ -835,7 +795,7 @@ static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
>>       CPUPPCState *env = &cpu->env;
>>   
>>       /* Raise it */
>> -    LOG_TB("raise hv decrementer exception\n");
>> +    trace_ppc_decr_excp("raise HV");
>>   
>>       /* The architecture specifies that we don't deliver HDEC
>>        * interrupts in a PM state. Not only they don't cause a
>> @@ -870,8 +830,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
>>           value |= (0xFFFFFFFFULL << nr_bits);
>>       }
>>   
>> -    LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
>> -                decr, value);
>> +    trace_ppc_decr_store(nr_bits, decr, value);
>>   
>>       if (kvm_enabled()) {
>>           /* KVM handles decrementer exceptions, we don't need our own timer */
>> @@ -1199,9 +1158,8 @@ static void cpu_4xx_fit_cb (void *opaque)
>>       if ((env->spr[SPR_40x_TCR] >> 23) & 0x1) {
>>           ppc_set_irq(cpu, PPC_INTERRUPT_FIT, 1);
>>       }
>> -    LOG_TB("%s: ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
>> -           (int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
>> -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>> +    trace_ppc4xx_fit((int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
>> +                         env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>>   }
>>   
>>   /* Programmable interval timer */
>> @@ -1215,11 +1173,10 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
>>           !((env->spr[SPR_40x_TCR] >> 26) & 0x1) ||
>>           (is_excp && !((env->spr[SPR_40x_TCR] >> 22) & 0x1))) {
>>           /* Stop PIT */
>> -        LOG_TB("%s: stop PIT\n", __func__);
>> +        trace_ppc4xx_pit_stop();
>>           timer_del(tb_env->decr_timer);
>>       } else {
>> -        LOG_TB("%s: start PIT %016" PRIx64 "\n",
>> -                    __func__, ppc40x_timer->pit_reload);
>> +        trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
>>           now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
>>           next = now + muldiv64(ppc40x_timer->pit_reload,
>>                                 NANOSECONDS_PER_SECOND, tb_env->decr_freq);
>> @@ -1248,9 +1205,7 @@ static void cpu_4xx_pit_cb (void *opaque)
>>           ppc_set_irq(cpu, ppc40x_timer->decr_excp, 1);
>>       }
>>       start_stop_pit(env, tb_env, 1);
>> -    LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
>> -           "%016" PRIx64 "\n", __func__,
>> -           (int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
>> +    trace_ppc4xx_pit((int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
>>              (int)((env->spr[SPR_40x_TCR] >> 26) & 0x1),
>>              env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
>>              ppc40x_timer->pit_reload);
>> @@ -1290,8 +1245,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
>>       next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
>>       if (next == now)
>>           next++;
>> -    LOG_TB("%s: TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
>> -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>> +    trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
>>       switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
>>       case 0x0:
>>       case 0x1:
>> @@ -1334,7 +1288,7 @@ void store_40x_pit (CPUPPCState *env, target_ulong val)
>>   
>>       tb_env = env->tb_env;
>>       ppc40x_timer = tb_env->opaque;
>> -    LOG_TB("%s val" TARGET_FMT_lx "\n", __func__, val);
>> +    trace_ppc40x_store_pit(val);
>>       ppc40x_timer->pit_reload = val;
>>       start_stop_pit(env, tb_env, 0);
>>   }
>> @@ -1349,8 +1303,7 @@ static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
>>       CPUPPCState *env = opaque;
>>       ppc_tb_t *tb_env = env->tb_env;
>>   
>> -    LOG_TB("%s set new frequency to %" PRIu32 "\n", __func__,
>> -                freq);
>> +    trace_ppc40x_set_tb_clk(freq);
>>       tb_env->tb_freq = freq;
>>       tb_env->decr_freq = freq;
>>       /* XXX: we should also update all timers */
>> @@ -1369,7 +1322,7 @@ clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
>>       tb_env->tb_freq = freq;
>>       tb_env->decr_freq = freq;
>>       tb_env->opaque = ppc40x_timer;
>> -    LOG_TB("%s freq %" PRIu32 "\n", __func__, freq);
>> +    trace_ppc40x_timers_init(freq);
>>       if (ppc40x_timer != NULL) {
>>           /* We use decr timer for PIT */
>>           tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, env);
>> diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
>> index da6e74b80dc1..3bf43fa340fe 100644
>> --- a/hw/ppc/trace-events
>> +++ b/hw/ppc/trace-events
>> @@ -97,7 +97,27 @@ vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx6
>>   
>>   # ppc.c
>>   ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
>> -
>> +ppc_tb_load(uint64_t tb) "tb 0x%016" PRIx64
>> +ppc_tb_store(uint64_t tb, uint64_t offset) "tb 0x%016" PRIx64 " offset 0x%08" PRIx64
>> +
>> +ppc_decr_load(uint64_t tb) "decr 0x%016" PRIx64
>> +ppc_decr_excp(const char *action) "%s decrementer"
>> +ppc_decr_store(uint32_t nr_bits, uint64_t decr, uint64_t value) "%d-bit 0x%016" PRIx64 " => 0x%016" PRIx64
>> +
>> +ppc4xx_fit(uint32_t ir, uint64_t tcr, uint64_t tsr) "ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64
>> +ppc4xx_pit_stop(void) ""
>> +ppc4xx_pit_start(uint64_t reload) "PIT 0x%016" PRIx64
>> +ppc4xx_pit(uint32_t ar, uint32_t ir, uint64_t tcr, uint64_t tsr, uint64_t reload) "ar %d ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64 " PIT 0x%016" PRIx64
>> +ppc4xx_wdt(uint64_t tcr, uint64_t tsr) "TCR 0x%" PRIx64 " TSR 0x%" PRIx64
>> +ppc40x_store_pit(uint64_t value) "val 0x%" PRIx64
>> +ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
>> +ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
>> +
>> +ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
>> +ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
>> +ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
>> +ppc_irq_reset(const char *name) "%s"
>> +ppc_irq_cpu(const char *action) "%s"
>>   
>>   # prep_systemio.c
>>   prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
> 

Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ)

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 26577 bytes --]

On Mon, Sep 20, 2021 at 09:42:25AM +0200, Cédric Le Goater wrote:
> On 9/20/21 09:34, David Gibson wrote:
> > On Mon, Sep 20, 2021 at 08:12:02AM +0200, Cédric Le Goater wrote:
> > > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> > 
> > So.. all the functions here are called *set_irq*, but you've named the
> > tracepoints *irq_set*.
> 
> 
> Yes. I did that on purpose to identify easily the PPC IRQ subsystem,
> and to be able to use '-trace ppc_irq*' from the command line and in
> the monitor.
> 
> > It's not a big deal, but it seems like it
> > would be less confusing if you flipped that around to match the
> > function names.
> 
> but you would loose the ability to do what I described above.

Good point.  Applied to ppc-for-6.2.

> 
> Thanks,
> 
> C.
> > > ---
> > >   hw/ppc/ppc.c        | 169 ++++++++++++++++----------------------------
> > >   hw/ppc/trace-events |  22 +++++-
> > >   2 files changed, 82 insertions(+), 109 deletions(-)
> > > 
> > > diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> > > index a327206a0a1e..b813ef732ec1 100644
> > > --- a/hw/ppc/ppc.c
> > > +++ b/hw/ppc/ppc.c
> > > @@ -37,22 +37,6 @@
> > >   #include "migration/vmstate.h"
> > >   #include "trace.h"
> > > -//#define PPC_DEBUG_IRQ
> > > -//#define PPC_DEBUG_TB
> > > -
> > > -#ifdef PPC_DEBUG_IRQ
> > > -#  define LOG_IRQ(...) qemu_log_mask(CPU_LOG_INT, ## __VA_ARGS__)
> > > -#else
> > > -#  define LOG_IRQ(...) do { } while (0)
> > > -#endif
> > > -
> > > -
> > > -#ifdef PPC_DEBUG_TB
> > > -#  define LOG_TB(...) qemu_log(__VA_ARGS__)
> > > -#else
> > > -#  define LOG_TB(...) do { } while (0)
> > > -#endif
> > > -
> > >   static void cpu_ppc_tb_stop (CPUPPCState *env);
> > >   static void cpu_ppc_tb_start (CPUPPCState *env);
> > > @@ -86,9 +70,8 @@ void ppc_set_irq(PowerPCCPU *cpu, int n_IRQ, int level)
> > >       }
> > > -    LOG_IRQ("%s: %p n_IRQ %d level %d => pending %08" PRIx32
> > > -                "req %08x\n", __func__, env, n_IRQ, level,
> > > -                env->pending_interrupts, CPU(cpu)->interrupt_request);
> > > +    trace_ppc_irq_set_exit(env, n_IRQ, level, env->pending_interrupts,
> > > +                           CPU(cpu)->interrupt_request);
> > >       if (locked) {
> > >           qemu_mutex_unlock_iothread();
> > > @@ -102,8 +85,8 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> > >       CPUPPCState *env = &cpu->env;
> > >       int cur_level;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -                env, pin, level);
> > > +    trace_ppc_irq_set(env, pin, level);
> > > +
> > >       cur_level = (env->irq_input_state >> pin) & 1;
> > >       /* Don't generate spurious events */
> > >       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> > > @@ -112,8 +95,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> > >           switch (pin) {
> > >           case PPC6xx_INPUT_TBEN:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: %s the time base\n",
> > > -                        __func__, level ? "start" : "stop");
> > > +            trace_ppc_irq_set_state("time base", level);
> > >               if (level) {
> > >                   cpu_ppc_tb_start(env);
> > >               } else {
> > > @@ -122,14 +104,12 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> > >               break;
> > >           case PPC6xx_INPUT_INT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("external IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >               break;
> > >           case PPC6xx_INPUT_SMI:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the SMI IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("SMI IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_SMI, level);
> > >               break;
> > >           case PPC6xx_INPUT_MCP:
> > > @@ -138,8 +118,7 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> > >                *            603/604/740/750: check HID0[EMCP]
> > >                */
> > >               if (cur_level == 1 && level == 0) {
> > > -                LOG_IRQ("%s: raise machine check state\n",
> > > -                            __func__);
> > > +                trace_ppc_irq_set_state("machine check", 1);
> > >                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
> > >               }
> > >               break;
> > > @@ -148,20 +127,19 @@ static void ppc6xx_set_irq(void *opaque, int pin, int level)
> > >               /* XXX: TODO: relay the signal to CKSTP_OUT pin */
> > >               /* XXX: Note that the only way to restart the CPU is to reset it */
> > >               if (level) {
> > > -                LOG_IRQ("%s: stop the CPU\n", __func__);
> > > +                trace_ppc_irq_cpu("stop");
> > >                   cs->halted = 1;
> > >               }
> > >               break;
> > >           case PPC6xx_INPUT_HRESET:
> > >               /* Level sensitive - active low */
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the CPU\n", __func__);
> > > +                trace_ppc_irq_reset("CPU");
> > >                   cpu_interrupt(cs, CPU_INTERRUPT_RESET);
> > >               }
> > >               break;
> > >           case PPC6xx_INPUT_SRESET:
> > > -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("RESET IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
> > >               break;
> > >           default:
> > > @@ -190,8 +168,8 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
> > >       CPUPPCState *env = &cpu->env;
> > >       int cur_level;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -                env, pin, level);
> > > +    trace_ppc_irq_set(env, pin, level);
> > > +
> > >       cur_level = (env->irq_input_state >> pin) & 1;
> > >       /* Don't generate spurious events */
> > >       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> > > @@ -200,14 +178,12 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
> > >           switch (pin) {
> > >           case PPC970_INPUT_INT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("external IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >               break;
> > >           case PPC970_INPUT_THINT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the SMI IRQ state to %d\n", __func__,
> > > -                        level);
> > > +            trace_ppc_irq_set_state("SMI IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_THERM, level);
> > >               break;
> > >           case PPC970_INPUT_MCP:
> > > @@ -216,8 +192,7 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
> > >                *            603/604/740/750: check HID0[EMCP]
> > >                */
> > >               if (cur_level == 1 && level == 0) {
> > > -                LOG_IRQ("%s: raise machine check state\n",
> > > -                            __func__);
> > > +                trace_ppc_irq_set_state("machine check", 1);
> > >                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, 1);
> > >               }
> > >               break;
> > > @@ -225,10 +200,10 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
> > >               /* Level sensitive - active low */
> > >               /* XXX: TODO: relay the signal to CKSTP_OUT pin */
> > >               if (level) {
> > > -                LOG_IRQ("%s: stop the CPU\n", __func__);
> > > +                trace_ppc_irq_cpu("stop");
> > >                   cs->halted = 1;
> > >               } else {
> > > -                LOG_IRQ("%s: restart the CPU\n", __func__);
> > > +                trace_ppc_irq_cpu("restart");
> > >                   cs->halted = 0;
> > >                   qemu_cpu_kick(cs);
> > >               }
> > > @@ -240,13 +215,11 @@ static void ppc970_set_irq(void *opaque, int pin, int level)
> > >               }
> > >               break;
> > >           case PPC970_INPUT_SRESET:
> > > -            LOG_IRQ("%s: set the RESET IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("RESET IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_RESET, level);
> > >               break;
> > >           case PPC970_INPUT_TBEN:
> > > -            LOG_IRQ("%s: set the TBEN state to %d\n", __func__,
> > > -                        level);
> > > +            trace_ppc_irq_set_state("TBEN IRQ", level);
> > >               /* XXX: TODO */
> > >               break;
> > >           default:
> > > @@ -272,14 +245,12 @@ static void power7_set_irq(void *opaque, int pin, int level)
> > >   {
> > >       PowerPCCPU *cpu = opaque;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -            &cpu->env, pin, level);
> > > +    trace_ppc_irq_set(&cpu->env, pin, level);
> > >       switch (pin) {
> > >       case POWER7_INPUT_INT:
> > >           /* Level sensitive - active high */
> > > -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                __func__, level);
> > > +        trace_ppc_irq_set_state("external IRQ", level);
> > >           ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >           break;
> > >       default:
> > > @@ -300,24 +271,22 @@ static void power9_set_irq(void *opaque, int pin, int level)
> > >   {
> > >       PowerPCCPU *cpu = opaque;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -            &cpu->env, pin, level);
> > > +    trace_ppc_irq_set(&cpu->env, pin, level);
> > >       switch (pin) {
> > >       case POWER9_INPUT_INT:
> > >           /* Level sensitive - active high */
> > > -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                __func__, level);
> > > +        trace_ppc_irq_set_state("external IRQ", level);
> > >           ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >           break;
> > >       case POWER9_INPUT_HINT:
> > >           /* Level sensitive - active high */
> > > -        LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                __func__, level);
> > > +        trace_ppc_irq_set_state("HV external IRQ", level);
> > >           ppc_set_irq(cpu, PPC_INTERRUPT_HVIRT, level);
> > >           break;
> > >       default:
> > >           g_assert_not_reached();
> > > +        return;
> > >       }
> > >   }
> > > @@ -393,8 +362,8 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
> > >       CPUPPCState *env = &cpu->env;
> > >       int cur_level;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -                env, pin, level);
> > > +    trace_ppc_irq_set(env, pin, level);
> > > +
> > >       cur_level = (env->irq_input_state >> pin) & 1;
> > >       /* Don't generate spurious events */
> > >       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> > > @@ -403,51 +372,47 @@ static void ppc40x_set_irq(void *opaque, int pin, int level)
> > >           switch (pin) {
> > >           case PPC40x_INPUT_RESET_SYS:
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the PowerPC system\n",
> > > -                            __func__);
> > > +                trace_ppc_irq_reset("system");
> > >                   ppc40x_system_reset(cpu);
> > >               }
> > >               break;
> > >           case PPC40x_INPUT_RESET_CHIP:
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the PowerPC chip\n", __func__);
> > > +                trace_ppc_irq_reset("chip");
> > >                   ppc40x_chip_reset(cpu);
> > >               }
> > >               break;
> > >           case PPC40x_INPUT_RESET_CORE:
> > >               /* XXX: TODO: update DBSR[MRR] */
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
> > > +                trace_ppc_irq_reset("core");
> > >                   ppc40x_core_reset(cpu);
> > >               }
> > >               break;
> > >           case PPC40x_INPUT_CINT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("critical IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
> > >               break;
> > >           case PPC40x_INPUT_INT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the external IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("external IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >               break;
> > >           case PPC40x_INPUT_HALT:
> > >               /* Level sensitive - active low */
> > >               if (level) {
> > > -                LOG_IRQ("%s: stop the CPU\n", __func__);
> > > +                trace_ppc_irq_cpu("stop");
> > >                   cs->halted = 1;
> > >               } else {
> > > -                LOG_IRQ("%s: restart the CPU\n", __func__);
> > > +                trace_ppc_irq_cpu("restart");
> > >                   cs->halted = 0;
> > >                   qemu_cpu_kick(cs);
> > >               }
> > >               break;
> > >           case PPC40x_INPUT_DEBUG:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the debug pin state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("debug pin", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
> > >               break;
> > >           default:
> > > @@ -475,41 +440,37 @@ static void ppce500_set_irq(void *opaque, int pin, int level)
> > >       CPUPPCState *env = &cpu->env;
> > >       int cur_level;
> > > -    LOG_IRQ("%s: env %p pin %d level %d\n", __func__,
> > > -                env, pin, level);
> > > +    trace_ppc_irq_set(env, pin, level);
> > > +
> > >       cur_level = (env->irq_input_state >> pin) & 1;
> > >       /* Don't generate spurious events */
> > >       if ((cur_level == 1 && level == 0) || (cur_level == 0 && level != 0)) {
> > >           switch (pin) {
> > >           case PPCE500_INPUT_MCK:
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the PowerPC system\n",
> > > -                            __func__);
> > > +                trace_ppc_irq_reset("system");
> > >                   qemu_system_reset_request(SHUTDOWN_CAUSE_GUEST_RESET);
> > >               }
> > >               break;
> > >           case PPCE500_INPUT_RESET_CORE:
> > >               if (level) {
> > > -                LOG_IRQ("%s: reset the PowerPC core\n", __func__);
> > > +                trace_ppc_irq_reset("core");
> > >                   ppc_set_irq(cpu, PPC_INTERRUPT_MCK, level);
> > >               }
> > >               break;
> > >           case PPCE500_INPUT_CINT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the critical IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("critical IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_CEXT, level);
> > >               break;
> > >           case PPCE500_INPUT_INT:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the core IRQ state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("core IRQ", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_EXT, level);
> > >               break;
> > >           case PPCE500_INPUT_DEBUG:
> > >               /* Level sensitive - active high */
> > > -            LOG_IRQ("%s: set the debug pin state to %d\n",
> > > -                        __func__, level);
> > > +            trace_ppc_irq_set_state("debug pin", level);
> > >               ppc_set_irq(cpu, PPC_INTERRUPT_DEBUG, level);
> > >               break;
> > >           default:
> > > @@ -564,7 +525,7 @@ uint64_t cpu_ppc_load_tbl (CPUPPCState *env)
> > >       }
> > >       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
> > > -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> > > +    trace_ppc_tb_load(tb);
> > >       return tb;
> > >   }
> > > @@ -575,7 +536,7 @@ static inline uint32_t _cpu_ppc_load_tbu(CPUPPCState *env)
> > >       uint64_t tb;
> > >       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->tb_offset);
> > > -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> > > +    trace_ppc_tb_load(tb);
> > >       return tb >> 32;
> > >   }
> > > @@ -595,8 +556,7 @@ static inline void cpu_ppc_store_tb(ppc_tb_t *tb_env, uint64_t vmclk,
> > >       *tb_offsetp = value -
> > >           muldiv64(vmclk, tb_env->tb_freq, NANOSECONDS_PER_SECOND);
> > > -    LOG_TB("%s: tb %016" PRIx64 " offset %08" PRIx64 "\n",
> > > -                __func__, value, *tb_offsetp);
> > > +    trace_ppc_tb_store(value, *tb_offsetp);
> > >   }
> > >   void cpu_ppc_store_tbl (CPUPPCState *env, uint32_t value)
> > > @@ -632,7 +592,7 @@ uint64_t cpu_ppc_load_atbl (CPUPPCState *env)
> > >       uint64_t tb;
> > >       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
> > > -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> > > +    trace_ppc_tb_load(tb);
> > >       return tb;
> > >   }
> > > @@ -643,7 +603,7 @@ uint32_t cpu_ppc_load_atbu (CPUPPCState *env)
> > >       uint64_t tb;
> > >       tb = cpu_ppc_get_tb(tb_env, qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL), tb_env->atb_offset);
> > > -    LOG_TB("%s: tb %016" PRIx64 "\n", __func__, tb);
> > > +    trace_ppc_tb_load(tb);
> > >       return tb >> 32;
> > >   }
> > > @@ -762,7 +722,7 @@ static inline int64_t _cpu_ppc_load_decr(CPUPPCState *env, uint64_t next)
> > >       }  else {
> > >           decr = -muldiv64(-diff, tb_env->decr_freq, NANOSECONDS_PER_SECOND);
> > >       }
> > > -    LOG_TB("%s: %016" PRIx64 "\n", __func__, decr);
> > > +    trace_ppc_decr_load(decr);
> > >       return decr;
> > >   }
> > > @@ -821,7 +781,7 @@ uint64_t cpu_ppc_load_purr (CPUPPCState *env)
> > >   static inline void cpu_ppc_decr_excp(PowerPCCPU *cpu)
> > >   {
> > >       /* Raise it */
> > > -    LOG_TB("raise decrementer exception\n");
> > > +    trace_ppc_decr_excp("raise");
> > >       ppc_set_irq(cpu, PPC_INTERRUPT_DECR, 1);
> > >   }
> > > @@ -835,7 +795,7 @@ static inline void cpu_ppc_hdecr_excp(PowerPCCPU *cpu)
> > >       CPUPPCState *env = &cpu->env;
> > >       /* Raise it */
> > > -    LOG_TB("raise hv decrementer exception\n");
> > > +    trace_ppc_decr_excp("raise HV");
> > >       /* The architecture specifies that we don't deliver HDEC
> > >        * interrupts in a PM state. Not only they don't cause a
> > > @@ -870,8 +830,7 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
> > >           value |= (0xFFFFFFFFULL << nr_bits);
> > >       }
> > > -    LOG_TB("%s: " TARGET_FMT_lx " => " TARGET_FMT_lx "\n", __func__,
> > > -                decr, value);
> > > +    trace_ppc_decr_store(nr_bits, decr, value);
> > >       if (kvm_enabled()) {
> > >           /* KVM handles decrementer exceptions, we don't need our own timer */
> > > @@ -1199,9 +1158,8 @@ static void cpu_4xx_fit_cb (void *opaque)
> > >       if ((env->spr[SPR_40x_TCR] >> 23) & 0x1) {
> > >           ppc_set_irq(cpu, PPC_INTERRUPT_FIT, 1);
> > >       }
> > > -    LOG_TB("%s: ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
> > > -           (int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
> > > -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> > > +    trace_ppc4xx_fit((int)((env->spr[SPR_40x_TCR] >> 23) & 0x1),
> > > +                         env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> > >   }
> > >   /* Programmable interval timer */
> > > @@ -1215,11 +1173,10 @@ static void start_stop_pit (CPUPPCState *env, ppc_tb_t *tb_env, int is_excp)
> > >           !((env->spr[SPR_40x_TCR] >> 26) & 0x1) ||
> > >           (is_excp && !((env->spr[SPR_40x_TCR] >> 22) & 0x1))) {
> > >           /* Stop PIT */
> > > -        LOG_TB("%s: stop PIT\n", __func__);
> > > +        trace_ppc4xx_pit_stop();
> > >           timer_del(tb_env->decr_timer);
> > >       } else {
> > > -        LOG_TB("%s: start PIT %016" PRIx64 "\n",
> > > -                    __func__, ppc40x_timer->pit_reload);
> > > +        trace_ppc4xx_pit_start(ppc40x_timer->pit_reload);
> > >           now = qemu_clock_get_ns(QEMU_CLOCK_VIRTUAL);
> > >           next = now + muldiv64(ppc40x_timer->pit_reload,
> > >                                 NANOSECONDS_PER_SECOND, tb_env->decr_freq);
> > > @@ -1248,9 +1205,7 @@ static void cpu_4xx_pit_cb (void *opaque)
> > >           ppc_set_irq(cpu, ppc40x_timer->decr_excp, 1);
> > >       }
> > >       start_stop_pit(env, tb_env, 1);
> > > -    LOG_TB("%s: ar %d ir %d TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx " "
> > > -           "%016" PRIx64 "\n", __func__,
> > > -           (int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
> > > +    trace_ppc4xx_pit((int)((env->spr[SPR_40x_TCR] >> 22) & 0x1),
> > >              (int)((env->spr[SPR_40x_TCR] >> 26) & 0x1),
> > >              env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR],
> > >              ppc40x_timer->pit_reload);
> > > @@ -1290,8 +1245,7 @@ static void cpu_4xx_wdt_cb (void *opaque)
> > >       next = now + muldiv64(next, NANOSECONDS_PER_SECOND, tb_env->decr_freq);
> > >       if (next == now)
> > >           next++;
> > > -    LOG_TB("%s: TCR " TARGET_FMT_lx " TSR " TARGET_FMT_lx "\n", __func__,
> > > -           env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> > > +    trace_ppc4xx_wdt(env->spr[SPR_40x_TCR], env->spr[SPR_40x_TSR]);
> > >       switch ((env->spr[SPR_40x_TSR] >> 30) & 0x3) {
> > >       case 0x0:
> > >       case 0x1:
> > > @@ -1334,7 +1288,7 @@ void store_40x_pit (CPUPPCState *env, target_ulong val)
> > >       tb_env = env->tb_env;
> > >       ppc40x_timer = tb_env->opaque;
> > > -    LOG_TB("%s val" TARGET_FMT_lx "\n", __func__, val);
> > > +    trace_ppc40x_store_pit(val);
> > >       ppc40x_timer->pit_reload = val;
> > >       start_stop_pit(env, tb_env, 0);
> > >   }
> > > @@ -1349,8 +1303,7 @@ static void ppc_40x_set_tb_clk (void *opaque, uint32_t freq)
> > >       CPUPPCState *env = opaque;
> > >       ppc_tb_t *tb_env = env->tb_env;
> > > -    LOG_TB("%s set new frequency to %" PRIu32 "\n", __func__,
> > > -                freq);
> > > +    trace_ppc40x_set_tb_clk(freq);
> > >       tb_env->tb_freq = freq;
> > >       tb_env->decr_freq = freq;
> > >       /* XXX: we should also update all timers */
> > > @@ -1369,7 +1322,7 @@ clk_setup_cb ppc_40x_timers_init (CPUPPCState *env, uint32_t freq,
> > >       tb_env->tb_freq = freq;
> > >       tb_env->decr_freq = freq;
> > >       tb_env->opaque = ppc40x_timer;
> > > -    LOG_TB("%s freq %" PRIu32 "\n", __func__, freq);
> > > +    trace_ppc40x_timers_init(freq);
> > >       if (ppc40x_timer != NULL) {
> > >           /* We use decr timer for PIT */
> > >           tb_env->decr_timer = timer_new_ns(QEMU_CLOCK_VIRTUAL, &cpu_4xx_pit_cb, env);
> > > diff --git a/hw/ppc/trace-events b/hw/ppc/trace-events
> > > index da6e74b80dc1..3bf43fa340fe 100644
> > > --- a/hw/ppc/trace-events
> > > +++ b/hw/ppc/trace-events
> > > @@ -97,7 +97,27 @@ vof_claimed(uint64_t start, uint64_t end, uint64_t size) "0x%"PRIx64"..0x%"PRIx6
> > >   # ppc.c
> > >   ppc_tb_adjust(uint64_t offs1, uint64_t offs2, int64_t diff, int64_t seconds) "adjusted from 0x%"PRIx64" to 0x%"PRIx64", diff %"PRId64" (%"PRId64"s)"
> > > -
> > > +ppc_tb_load(uint64_t tb) "tb 0x%016" PRIx64
> > > +ppc_tb_store(uint64_t tb, uint64_t offset) "tb 0x%016" PRIx64 " offset 0x%08" PRIx64
> > > +
> > > +ppc_decr_load(uint64_t tb) "decr 0x%016" PRIx64
> > > +ppc_decr_excp(const char *action) "%s decrementer"
> > > +ppc_decr_store(uint32_t nr_bits, uint64_t decr, uint64_t value) "%d-bit 0x%016" PRIx64 " => 0x%016" PRIx64
> > > +
> > > +ppc4xx_fit(uint32_t ir, uint64_t tcr, uint64_t tsr) "ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64
> > > +ppc4xx_pit_stop(void) ""
> > > +ppc4xx_pit_start(uint64_t reload) "PIT 0x%016" PRIx64
> > > +ppc4xx_pit(uint32_t ar, uint32_t ir, uint64_t tcr, uint64_t tsr, uint64_t reload) "ar %d ir %d TCR 0x%" PRIx64 " TSR 0x%" PRIx64 " PIT 0x%016" PRIx64
> > > +ppc4xx_wdt(uint64_t tcr, uint64_t tsr) "TCR 0x%" PRIx64 " TSR 0x%" PRIx64
> > > +ppc40x_store_pit(uint64_t value) "val 0x%" PRIx64
> > > +ppc40x_set_tb_clk(uint32_t value) "new frequency %" PRIu32
> > > +ppc40x_timers_init(uint32_t value) "frequency %" PRIu32
> > > +
> > > +ppc_irq_set(void *env, uint32_t pin, uint32_t level) "env [%p] pin %d level %d"
> > > +ppc_irq_set_exit(void *env, uint32_t n_IRQ, uint32_t level, uint32_t pending, uint32_t request) "env [%p] n_IRQ %d level %d => pending 0x%08" PRIx32 " req 0x%08" PRIx32
> > > +ppc_irq_set_state(const char *name, uint32_t level) "\"%s\" level %d"
> > > +ppc_irq_reset(const char *name) "%s"
> > > +ppc_irq_cpu(const char *action) "%s"
> > >   # prep_systemio.c
> > >   prep_systemio_read(uint32_t addr, uint32_t val) "read addr=0x%x val=0x%x"
> > 
> 
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[PATCH v4 4/4] target/ppc: Fix 64-bit decrementer

[Cédric Le Goater]

The current way the mask is built can overflow with a 64-bit decrementer.
Use sextract64() to extract the signed values and remove the logic to
handle negative values which has become useless.

Cc: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
Signed-off-by: Cédric Le Goater <clg@kaod.org>
---
 hw/ppc/ppc.c | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
index b813ef732ec1..f5d012f860af 100644
--- a/hw/ppc/ppc.c
+++ b/hw/ppc/ppc.c
@@ -821,14 +821,12 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
     CPUPPCState *env = &cpu->env;
     ppc_tb_t *tb_env = env->tb_env;
     uint64_t now, next;
-    bool negative;
+    int64_t signed_value;
+    int64_t signed_decr;
 
     /* Truncate value to decr_width and sign extend for simplicity */
-    value &= ((1ULL << nr_bits) - 1);
-    negative = !!(value & (1ULL << (nr_bits - 1)));
-    if (negative) {
-        value |= (0xFFFFFFFFULL << nr_bits);
-    }
+    signed_value = sextract64(value, 0, nr_bits);
+    signed_decr = sextract64(decr, 0, nr_bits);
 
     trace_ppc_decr_store(nr_bits, decr, value);
 
@@ -850,16 +848,16 @@ 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) && negative) ||
-        ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && negative
-          && !(decr & (1ULL << (nr_bits - 1))))) {
+    if ((signed_value < 3) ||
+        ((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);
         return;
     }
 
     /* On MSB level based systems a 0 for the MSB stops interrupt delivery */
-    if (!negative && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
+    if (signed_value >= 0 && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
         (*lower_excp)(cpu);
     }
 
-- 
2.31.1

Re: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2774 bytes --]

On Mon, Sep 20, 2021 at 08:12:03AM +0200, Cédric Le Goater wrote:
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use sextract64() to extract the signed values and remove the logic to
> handle negative values which has become useless.
> 
> Cc: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

LGTM, but not applying because of the comments on 3/4.

> ---
>  hw/ppc/ppc.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index b813ef732ec1..f5d012f860af 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -821,14 +821,12 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
>      CPUPPCState *env = &cpu->env;
>      ppc_tb_t *tb_env = env->tb_env;
>      uint64_t now, next;
> -    bool negative;
> +    int64_t signed_value;
> +    int64_t signed_decr;
>  
>      /* Truncate value to decr_width and sign extend for simplicity */
> -    value &= ((1ULL << nr_bits) - 1);
> -    negative = !!(value & (1ULL << (nr_bits - 1)));
> -    if (negative) {
> -        value |= (0xFFFFFFFFULL << nr_bits);
> -    }
> +    signed_value = sextract64(value, 0, nr_bits);
> +    signed_decr = sextract64(decr, 0, nr_bits);
>  
>      trace_ppc_decr_store(nr_bits, decr, value);
>  
> @@ -850,16 +848,16 @@ 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) && negative) ||
> -        ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && negative
> -          && !(decr & (1ULL << (nr_bits - 1))))) {
> +    if ((signed_value < 3) ||
> +        ((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);
>          return;
>      }
>  
>      /* On MSB level based systems a 0 for the MSB stops interrupt delivery */
> -    if (!negative && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
> +    if (signed_value >= 0 && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
>          (*lower_excp)(cpu);
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

RE: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer

[Luis Fernando Fujita Pires]

From: Cédric Le Goater <clg@kaod.org>
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use sextract64() to extract the signed values and remove the logic to handle
> negative values which has become useless.
> 
> Cc: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for
> TCG")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Reviewed-by: Luis Pires <luis.pires@eldorado.org.br>

Thanks,

--
Luis Pires
Instituto de Pesquisas ELDORADO
Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html>

Re: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 2690 bytes --]

On Mon, Sep 20, 2021 at 08:12:03AM +0200, Cédric Le Goater wrote:
> The current way the mask is built can overflow with a 64-bit decrementer.
> Use sextract64() to extract the signed values and remove the logic to
> handle negative values which has become useless.
> 
> Cc: Luis Fernando Fujita Pires <luis.pires@eldorado.org.br>
> Fixes: a8dafa525181 ("target/ppc: Implement large decrementer support for TCG")
> Signed-off-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.2, thanks.
> ---
>  hw/ppc/ppc.c | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/hw/ppc/ppc.c b/hw/ppc/ppc.c
> index b813ef732ec1..f5d012f860af 100644
> --- a/hw/ppc/ppc.c
> +++ b/hw/ppc/ppc.c
> @@ -821,14 +821,12 @@ static void __cpu_ppc_store_decr(PowerPCCPU *cpu, uint64_t *nextp,
>      CPUPPCState *env = &cpu->env;
>      ppc_tb_t *tb_env = env->tb_env;
>      uint64_t now, next;
> -    bool negative;
> +    int64_t signed_value;
> +    int64_t signed_decr;
>  
>      /* Truncate value to decr_width and sign extend for simplicity */
> -    value &= ((1ULL << nr_bits) - 1);
> -    negative = !!(value & (1ULL << (nr_bits - 1)));
> -    if (negative) {
> -        value |= (0xFFFFFFFFULL << nr_bits);
> -    }
> +    signed_value = sextract64(value, 0, nr_bits);
> +    signed_decr = sextract64(decr, 0, nr_bits);
>  
>      trace_ppc_decr_store(nr_bits, decr, value);
>  
> @@ -850,16 +848,16 @@ 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) && negative) ||
> -        ((tb_env->flags & PPC_DECR_UNDERFLOW_TRIGGERED) && negative
> -          && !(decr & (1ULL << (nr_bits - 1))))) {
> +    if ((signed_value < 3) ||
> +        ((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);
>          return;
>      }
>  
>      /* On MSB level based systems a 0 for the MSB stops interrupt delivery */
> -    if (!negative && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
> +    if (signed_value >= 0 && (tb_env->flags & PPC_DECR_UNDERFLOW_LEVEL)) {
>          (*lower_excp)(cpu);
>      }
>  

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]