* [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
* [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
* [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
* [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 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
* 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
* 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 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 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 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 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
* 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.