* [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix @ 2021-09-20 6:11 Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) Cédric Le Goater ` (3 more replies) 0 siblings, 4 replies; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 6:11 UTC (permalink / raw) To: David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel, 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 ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) 2021-09-20 6:11 [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix Cédric Le Goater @ 2021-09-20 6:12 ` Cédric Le Goater 2021-09-20 6:54 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins Cédric Le Goater ` (2 subsequent siblings) 3 siblings, 1 reply; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 6:12 UTC (permalink / raw) To: David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) 2021-09-20 6:12 ` [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) Cédric Le Goater @ 2021-09-20 6:54 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2021-09-20 6:54 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins 2021-09-20 6:11 [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) Cédric Le Goater @ 2021-09-20 6:12 ` Cédric Le Goater 2021-09-20 6:55 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater 3 siblings, 1 reply; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 6:12 UTC (permalink / raw) To: David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins 2021-09-20 6:12 ` [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins Cédric Le Goater @ 2021-09-20 6:55 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2021-09-20 6:55 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) 2021-09-20 6:11 [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins Cédric Le Goater @ 2021-09-20 6:12 ` Cédric Le Goater 2021-09-20 7:34 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater 3 siblings, 1 reply; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 6:12 UTC (permalink / raw) To: David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) 2021-09-20 6:12 ` [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) Cédric Le Goater @ 2021-09-20 7:34 ` David Gibson 2021-09-20 7:42 ` Cédric Le Goater 0 siblings, 1 reply; 13+ messages in thread From: David Gibson @ 2021-09-20 7:34 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) 2021-09-20 7:34 ` David Gibson @ 2021-09-20 7:42 ` Cédric Le Goater 2021-09-21 1:44 ` David Gibson 0 siblings, 1 reply; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 7:42 UTC (permalink / raw) To: David Gibson; +Cc: qemu-ppc, Greg Kurz, qemu-devel 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" > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) 2021-09-20 7:42 ` Cédric Le Goater @ 2021-09-21 1:44 ` David Gibson 0 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2021-09-21 1:44 UTC (permalink / raw) To: Cédric Le Goater; +Cc: qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer 2021-09-20 6:11 [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix Cédric Le Goater ` (2 preceding siblings ...) 2021-09-20 6:12 ` [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) Cédric Le Goater @ 2021-09-20 6:12 ` Cédric Le Goater 2021-09-20 7:36 ` David Gibson ` (2 more replies) 3 siblings, 3 replies; 13+ messages in thread From: Cédric Le Goater @ 2021-09-20 6:12 UTC (permalink / raw) To: David Gibson, Greg Kurz Cc: Luis Fernando Fujita Pires, qemu-ppc, qemu-devel, 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 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater @ 2021-09-20 7:36 ` David Gibson 2021-09-20 12:20 ` Luis Fernando Fujita Pires 2021-09-21 1:44 ` David Gibson 2 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2021-09-20 7:36 UTC (permalink / raw) To: Cédric Le Goater Cc: Luis Fernando Fujita Pires, qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater 2021-09-20 7:36 ` David Gibson @ 2021-09-20 12:20 ` Luis Fernando Fujita Pires 2021-09-21 1:44 ` David Gibson 2 siblings, 0 replies; 13+ messages in thread From: Luis Fernando Fujita Pires @ 2021-09-20 12:20 UTC (permalink / raw) To: Cédric Le Goater, David Gibson, Greg Kurz; +Cc: qemu-ppc, qemu-devel 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> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater 2021-09-20 7:36 ` David Gibson 2021-09-20 12:20 ` Luis Fernando Fujita Pires @ 2021-09-21 1:44 ` David Gibson 2 siblings, 0 replies; 13+ messages in thread From: David Gibson @ 2021-09-21 1:44 UTC (permalink / raw) To: Cédric Le Goater Cc: Luis Fernando Fujita Pires, qemu-ppc, Greg Kurz, qemu-devel [-- 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 --] ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-09-21 2:59 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-09-20 6:11 [PATCH v4 0/4] target/ppc: debug messages cleanup and decrementer fix Cédric Le Goater 2021-09-20 6:12 ` [PATCH v4 1/4] target/ppc: Convert debug to trace events (exceptions) Cédric Le Goater 2021-09-20 6:54 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 2/4] target/ppc: Replace debug messages by asserts for unknown IRQ pins Cédric Le Goater 2021-09-20 6:55 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 3/4] target/ppc: Convert debug to trace events (decrementer and IRQ) Cédric Le Goater 2021-09-20 7:34 ` David Gibson 2021-09-20 7:42 ` Cédric Le Goater 2021-09-21 1:44 ` David Gibson 2021-09-20 6:12 ` [PATCH v4 4/4] target/ppc: Fix 64-bit decrementer Cédric Le Goater 2021-09-20 7:36 ` David Gibson 2021-09-20 12:20 ` Luis Fernando Fujita Pires 2021-09-21 1:44 ` David Gibson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.