* [PATCH v3 01/14] target/ppc: Remove some superfluous parentheses
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 02/14] target/ppc: Remove unneeded parameter from powerpc_reset_wakeup() BALATON Zoltan
` (13 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 12d8a7257b..8298217e78 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1009,7 +1009,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
{
int lev = env->error_code;
- if ((lev == 1) && cpu->vhyp) {
+ if (lev == 1 && cpu->vhyp) {
dump_hcall(env);
} else {
dump_syscall(env);
@@ -1027,7 +1027,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
* uses VOF and the 74xx CPUs, so although the 74xx don't have
* HV mode, we need to keep hypercall support.
*/
- if ((lev == 1) && cpu->vhyp) {
+ if (lev == 1 && cpu->vhyp) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
vhc->hypercall(cpu->vhyp, cpu);
@@ -1481,7 +1481,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception */
lev = env->error_code;
- if ((lev == 1) && cpu->vhyp) {
+ if (lev == 1 && cpu->vhyp) {
dump_hcall(env);
} else {
dump_syscall(env);
@@ -1494,7 +1494,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
env->nip += 4;
/* "PAPR mode" built-in hypercall emulation */
- if ((lev == 1) && books_vhyp_handles_hcall(cpu)) {
+ if (lev == 1 && books_vhyp_handles_hcall(cpu)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
vhc->hypercall(cpu->vhyp, cpu);
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 02/14] target/ppc: Remove unneeded parameter from powerpc_reset_wakeup()
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 01/14] target/ppc: Remove some superfluous parentheses BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 03/14] target/ppc: Move common check in exception handlers to a function BALATON Zoltan
` (12 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
CPUState is rarely needed by this function (only for logging a fatal
error) and it's easy to get from the env parameter so passing it
separately is not necessary.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8298217e78..3783315fdb 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -166,8 +166,7 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
}
#if defined(TARGET_PPC64)
-static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
- target_ulong *msr)
+static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr)
{
/* We no longer are in a PM state */
env->resume_as_sreset = false;
@@ -202,8 +201,8 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
*msr |= SRR1_WAKEHVI;
break;
default:
- cpu_abort(cs, "Unsupported exception %d in Power Save mode\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Unsupported exception %d in Power Save mode\n", excp);
}
return POWERPC_EXCP_RESET;
}
@@ -1353,7 +1352,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
* P7/P8/P9
*/
if (env->resume_as_sreset) {
- excp = powerpc_reset_wakeup(cs, env, excp, &msr);
+ excp = powerpc_reset_wakeup(env, excp, &msr);
}
/*
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 03/14] target/ppc: Move common check in exception handlers to a function
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 01/14] target/ppc: Remove some superfluous parentheses BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 02/14] target/ppc: Remove unneeded parameter from powerpc_reset_wakeup() BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
` (11 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
All powerpc exception handlers share some code when handling machine
check exceptions. Move this to a common function.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 114 +++++++++------------------------------
1 file changed, 25 insertions(+), 89 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 3783315fdb..79f5ca1034 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -403,6 +403,25 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector,
env->reserve_addr = -1;
}
+static void powerpc_mcheck_checkstop(CPUPPCState *env)
+{
+ CPUState *cs = env_cpu(env);
+
+ if (FIELD_EX64(env->msr, MSR, ME)) {
+ return;
+ }
+
+ /* Machine check exception is not enabled. Enter checkstop state. */
+ fprintf(stderr, "Machine check while not allowed. "
+ "Entering checkstop state\n");
+ if (qemu_log_separate()) {
+ qemu_log("Machine check while not allowed. "
+ "Entering checkstop state\n");
+ }
+ cs->halted = 1;
+ cpu_interrupt_exittb(cs);
+}
+
static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
{
CPUState *cs = CPU(cpu);
@@ -445,21 +464,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
srr1 = SPR_40x_SRR3;
break;
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
-
+ powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -576,21 +581,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_CRITICAL: /* Critical input */
break;
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
-
+ powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -749,21 +740,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
-
+ powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -934,21 +911,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
-
+ powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -1129,21 +1092,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
srr1 = SPR_BOOKE_CSRR1;
break;
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
-
+ powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -1376,20 +1325,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
switch (excp) {
case POWERPC_EXCP_MCHECK: /* Machine check exception */
- if (!FIELD_EX64(env->msr, MSR, ME)) {
- /*
- * Machine check exception is not enabled. Enter
- * checkstop state.
- */
- fprintf(stderr, "Machine check while not allowed. "
- "Entering checkstop state\n");
- if (qemu_log_separate()) {
- qemu_log("Machine check while not allowed. "
- "Entering checkstop state\n");
- }
- cs->halted = 1;
- cpu_interrupt_exittb(cs);
- }
+ powerpc_mcheck_checkstop(env);
if (env->msr_mask & MSR_HVB) {
/*
* ISA specifies HV, but can be delivered to guest with HV
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (2 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 03/14] target/ppc: Move common check in exception handlers to a function BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 4:54 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once BALATON Zoltan
` (10 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Use the env_cpu function to get the CPUState for cpu_abort. These are
only needed in case of fatal errors so this allows to avoid casting
and storing CPUState in a local variable wnen not needed.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 118 +++++++++++++++++++++------------------
1 file changed, 63 insertions(+), 55 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 79f5ca1034..122e2a6e41 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -424,7 +424,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
int srr0, srr1;
@@ -452,8 +451,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -502,7 +501,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
env->spr[SPR_40x_ESR] = ESR_PTR;
break;
default:
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -529,11 +528,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
trace_ppc_excp_print("PIT");
break;
case POWERPC_EXCP_DEBUG: /* Debug interrupt */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -548,7 +548,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
@@ -571,8 +570,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -632,7 +631,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
break;
default:
/* Should never occur */
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -654,8 +653,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_RESET: /* System reset exception */
if (FIELD_EX64(env->msr, MSR, POW)) {
- cpu_abort(cs, "Trying to deliver power-saving system reset "
- "exception %d with no HV support\n", excp);
+ cpu_abort(env_cpu(env),
+ "Trying to deliver power-saving system reset exception "
+ "%d with no HV support\n", excp);
}
break;
case POWERPC_EXCP_TRACE: /* Trace exception */
@@ -682,11 +682,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SMI: /* System management interrupt */
case POWERPC_EXCP_MEXTBR: /* Maskable external breakpoint */
case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -709,7 +710,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
@@ -732,8 +732,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -791,7 +791,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
break;
default:
/* Should never occur */
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -832,8 +832,9 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_RESET: /* System reset exception */
if (FIELD_EX64(env->msr, MSR, POW)) {
- cpu_abort(cs, "Trying to deliver power-saving system reset "
- "exception %d with no HV support\n", excp);
+ cpu_abort(env_cpu(env),
+ "Trying to deliver power-saving system reset exception "
+ "%d with no HV support\n", excp);
}
break;
case POWERPC_EXCP_TRACE: /* Trace exception */
@@ -853,11 +854,12 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SMI: /* System management interrupt */
case POWERPC_EXCP_THERM: /* Thermal interrupt */
case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -880,7 +882,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
@@ -903,8 +904,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -962,7 +963,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
break;
default:
/* Should never occur */
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -1003,7 +1004,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_RESET: /* System reset exception */
if (FIELD_EX64(env->msr, MSR, POW)) {
- cpu_abort(cs, "Trying to deliver power-saving system reset "
+ cpu_abort(env_cpu(env),
+ "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
break;
@@ -1016,11 +1018,12 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_THERM: /* Thermal interrupt */
case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
case POWERPC_EXCP_VPUA: /* Vector assist exception */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -1043,7 +1046,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
int srr0, srr1;
@@ -1080,8 +1082,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -1112,6 +1114,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_EXTERNAL: /* External input */
if (env->mpic_proxy) {
+ CPUState *cs = env_cpu(env);
/* IACK the IRQ on delivery */
env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
}
@@ -1150,7 +1153,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
break;
default:
/* Should never occur */
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -1191,7 +1194,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
/* DBSR already modified by caller */
} else {
- cpu_abort(cs, "Debug exception triggered on unsupported model\n");
+ cpu_abort(env_cpu(env),
+ "Debug exception triggered on unsupported model\n");
}
break;
case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable/VPU */
@@ -1205,17 +1209,19 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_RESET: /* System reset exception */
if (FIELD_EX64(env->msr, MSR, POW)) {
- cpu_abort(cs, "Trying to deliver power-saving system reset "
+ cpu_abort(env_cpu(env),
+ "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
break;
case POWERPC_EXCP_EFPDI: /* Embedded floating-point data interrupt */
case POWERPC_EXCP_EFPRI: /* Embedded floating-point round interrupt */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -1278,7 +1284,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
int srr0, srr1, lev = -1;
@@ -1317,8 +1322,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
vector = env->excp_vectors[excp];
if (vector == (target_ulong)-1ULL) {
- cpu_abort(cs, "Raised an exception without defined vector %d\n",
- excp);
+ cpu_abort(env_cpu(env),
+ "Raised an exception without defined vector %d\n", excp);
}
vector |= env->excp_prefix;
@@ -1408,7 +1413,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
break;
default:
/* Should never occur */
- cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+ cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
env->error_code);
break;
}
@@ -1469,7 +1474,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
new_msr |= (target_ulong)MSR_HVB;
} else {
if (FIELD_EX64(env->msr, MSR, POW)) {
- cpu_abort(cs, "Trying to deliver power-saving system reset "
+ cpu_abort(env_cpu(env),
+ "Trying to deliver power-saving system reset "
"exception %d with no HV support\n", excp);
}
}
@@ -1524,11 +1530,12 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_VPUA: /* Vector assist exception */
case POWERPC_EXCP_MAINT: /* Maintenance exception */
case POWERPC_EXCP_HV_MAINT: /* Hypervisor Maintenance exception */
- cpu_abort(cs, "%s exception not implemented\n",
+ cpu_abort(env_cpu(env), "%s exception not implemented\n",
powerpc_excp_name(excp));
break;
default:
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
break;
}
@@ -1561,8 +1568,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
} else {
/* Sanity check */
if (!(env->msr_mask & MSR_HVB) && srr0 == SPR_HSRR0) {
- cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
- "no HV support\n", excp);
+ cpu_abort(env_cpu(env), "Trying to deliver HV exception (HSRR) %d "
+ "with no HV support\n", excp);
}
/* This can update new_msr and vector if AIL applies */
@@ -1580,11 +1587,11 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, int excp)
static void powerpc_excp(PowerPCCPU *cpu, int excp)
{
- CPUState *cs = CPU(cpu);
CPUPPCState *env = &cpu->env;
if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
- cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+ cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+ excp);
}
qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
@@ -2118,7 +2125,6 @@ void ppc_maybe_interrupt(CPUPPCState *env)
static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = env_cpu(env);
switch (interrupt) {
case PPC_INTERRUPT_MCK: /* Machine check exception */
@@ -2162,14 +2168,14 @@ static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
assert(!env->resume_as_sreset);
break;
default:
- cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+ cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+ interrupt);
}
}
static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = env_cpu(env);
switch (interrupt) {
case PPC_INTERRUPT_MCK: /* Machine check exception */
@@ -2233,7 +2239,8 @@ static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
assert(!env->resume_as_sreset);
break;
default:
- cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+ cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+ interrupt);
}
}
@@ -2312,7 +2319,8 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
assert(!env->resume_as_sreset);
break;
default:
- cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+ cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+ interrupt);
}
}
#endif
@@ -2320,7 +2328,6 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = env_cpu(env);
switch (interrupt) {
case PPC_INTERRUPT_RESET: /* External reset */
@@ -2417,7 +2424,8 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
assert(!env->resume_as_sreset);
break;
default:
- cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+ cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+ interrupt);
}
}
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
2023-06-15 23:03 ` [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
@ 2023-06-20 4:54 ` Nicholas Piggin
2023-06-20 10:50 ` BALATON Zoltan
0 siblings, 1 reply; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 4:54 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> Use the env_cpu function to get the CPUState for cpu_abort. These are
> only needed in case of fatal errors so this allows to avoid casting
> and storing CPUState in a local variable wnen not needed.
>
Eh, this is still replacing less typing with more. It's normal to
define these things up front of a function especially when used
multiple times. 'cs' should be as instantly recognizable as env
when looking at code so my preference is to keep it as is.
Thanks,
Nick
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> target/ppc/excp_helper.c | 118 +++++++++++++++++++++------------------
> 1 file changed, 63 insertions(+), 55 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 79f5ca1034..122e2a6e41 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -424,7 +424,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
>
> static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
> int srr0, srr1;
> @@ -452,8 +451,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -502,7 +501,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> env->spr[SPR_40x_ESR] = ESR_PTR;
> break;
> default:
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -529,11 +528,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> trace_ppc_excp_print("PIT");
> break;
> case POWERPC_EXCP_DEBUG: /* Debug interrupt */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -548,7 +548,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>
> static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
>
> @@ -571,8 +570,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -632,7 +631,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
> break;
> default:
> /* Should never occur */
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -654,8 +653,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_RESET: /* System reset exception */
> if (FIELD_EX64(env->msr, MSR, POW)) {
> - cpu_abort(cs, "Trying to deliver power-saving system reset "
> - "exception %d with no HV support\n", excp);
> + cpu_abort(env_cpu(env),
> + "Trying to deliver power-saving system reset exception "
> + "%d with no HV support\n", excp);
> }
> break;
> case POWERPC_EXCP_TRACE: /* Trace exception */
> @@ -682,11 +682,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SMI: /* System management interrupt */
> case POWERPC_EXCP_MEXTBR: /* Maskable external breakpoint */
> case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -709,7 +710,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>
> static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
>
> @@ -732,8 +732,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -791,7 +791,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> break;
> default:
> /* Should never occur */
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -832,8 +832,9 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_RESET: /* System reset exception */
> if (FIELD_EX64(env->msr, MSR, POW)) {
> - cpu_abort(cs, "Trying to deliver power-saving system reset "
> - "exception %d with no HV support\n", excp);
> + cpu_abort(env_cpu(env),
> + "Trying to deliver power-saving system reset exception "
> + "%d with no HV support\n", excp);
> }
> break;
> case POWERPC_EXCP_TRACE: /* Trace exception */
> @@ -853,11 +854,12 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SMI: /* System management interrupt */
> case POWERPC_EXCP_THERM: /* Thermal interrupt */
> case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -880,7 +882,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>
> static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
>
> @@ -903,8 +904,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -962,7 +963,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> break;
> default:
> /* Should never occur */
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -1003,7 +1004,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_RESET: /* System reset exception */
> if (FIELD_EX64(env->msr, MSR, POW)) {
> - cpu_abort(cs, "Trying to deliver power-saving system reset "
> + cpu_abort(env_cpu(env),
> + "Trying to deliver power-saving system reset "
> "exception %d with no HV support\n", excp);
> }
> break;
> @@ -1016,11 +1018,12 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_THERM: /* Thermal interrupt */
> case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
> case POWERPC_EXCP_VPUA: /* Vector assist exception */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -1043,7 +1046,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>
> static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
> int srr0, srr1;
> @@ -1080,8 +1082,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -1112,6 +1114,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_EXTERNAL: /* External input */
> if (env->mpic_proxy) {
> + CPUState *cs = env_cpu(env);
> /* IACK the IRQ on delivery */
> env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
> }
> @@ -1150,7 +1153,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
> break;
> default:
> /* Should never occur */
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -1191,7 +1194,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>
> /* DBSR already modified by caller */
> } else {
> - cpu_abort(cs, "Debug exception triggered on unsupported model\n");
> + cpu_abort(env_cpu(env),
> + "Debug exception triggered on unsupported model\n");
> }
> break;
> case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable/VPU */
> @@ -1205,17 +1209,19 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_RESET: /* System reset exception */
> if (FIELD_EX64(env->msr, MSR, POW)) {
> - cpu_abort(cs, "Trying to deliver power-saving system reset "
> + cpu_abort(env_cpu(env),
> + "Trying to deliver power-saving system reset "
> "exception %d with no HV support\n", excp);
> }
> break;
> case POWERPC_EXCP_EFPDI: /* Embedded floating-point data interrupt */
> case POWERPC_EXCP_EFPRI: /* Embedded floating-point round interrupt */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -1278,7 +1284,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
>
> static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
> target_ulong msr, new_msr, vector;
> int srr0, srr1, lev = -1;
> @@ -1317,8 +1322,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>
> vector = env->excp_vectors[excp];
> if (vector == (target_ulong)-1ULL) {
> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
> - excp);
> + cpu_abort(env_cpu(env),
> + "Raised an exception without defined vector %d\n", excp);
> }
>
> vector |= env->excp_prefix;
> @@ -1408,7 +1413,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> break;
> default:
> /* Should never occur */
> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
> env->error_code);
> break;
> }
> @@ -1469,7 +1474,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> new_msr |= (target_ulong)MSR_HVB;
> } else {
> if (FIELD_EX64(env->msr, MSR, POW)) {
> - cpu_abort(cs, "Trying to deliver power-saving system reset "
> + cpu_abort(env_cpu(env),
> + "Trying to deliver power-saving system reset "
> "exception %d with no HV support\n", excp);
> }
> }
> @@ -1524,11 +1530,12 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_VPUA: /* Vector assist exception */
> case POWERPC_EXCP_MAINT: /* Maintenance exception */
> case POWERPC_EXCP_HV_MAINT: /* Hypervisor Maintenance exception */
> - cpu_abort(cs, "%s exception not implemented\n",
> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
> powerpc_excp_name(excp));
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> break;
> }
>
> @@ -1561,8 +1568,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> } else {
> /* Sanity check */
> if (!(env->msr_mask & MSR_HVB) && srr0 == SPR_HSRR0) {
> - cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
> - "no HV support\n", excp);
> + cpu_abort(env_cpu(env), "Trying to deliver HV exception (HSRR) %d "
> + "with no HV support\n", excp);
> }
>
> /* This can update new_msr and vector if AIL applies */
> @@ -1580,11 +1587,11 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>
> static void powerpc_excp(PowerPCCPU *cpu, int excp)
> {
> - CPUState *cs = CPU(cpu);
> CPUPPCState *env = &cpu->env;
>
> if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
> + excp);
> }
>
> qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> @@ -2118,7 +2125,6 @@ void ppc_maybe_interrupt(CPUPPCState *env)
> static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
> {
> PowerPCCPU *cpu = env_archcpu(env);
> - CPUState *cs = env_cpu(env);
>
> switch (interrupt) {
> case PPC_INTERRUPT_MCK: /* Machine check exception */
> @@ -2162,14 +2168,14 @@ static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
> assert(!env->resume_as_sreset);
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
> + interrupt);
> }
> }
>
> static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
> {
> PowerPCCPU *cpu = env_archcpu(env);
> - CPUState *cs = env_cpu(env);
>
> switch (interrupt) {
> case PPC_INTERRUPT_MCK: /* Machine check exception */
> @@ -2233,7 +2239,8 @@ static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
> assert(!env->resume_as_sreset);
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
> + interrupt);
> }
> }
>
> @@ -2312,7 +2319,8 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
> assert(!env->resume_as_sreset);
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
> + interrupt);
> }
> }
> #endif
> @@ -2320,7 +2328,6 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
> static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> {
> PowerPCCPU *cpu = env_archcpu(env);
> - CPUState *cs = env_cpu(env);
>
> switch (interrupt) {
> case PPC_INTERRUPT_RESET: /* External reset */
> @@ -2417,7 +2424,8 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> assert(!env->resume_as_sreset);
> break;
> default:
> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
> + interrupt);
> }
> }
>
> --
> 2.30.9
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
2023-06-20 4:54 ` Nicholas Piggin
@ 2023-06-20 10:50 ` BALATON Zoltan
0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-20 10:50 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
>> Use the env_cpu function to get the CPUState for cpu_abort. These are
>> only needed in case of fatal errors so this allows to avoid casting
>> and storing CPUState in a local variable wnen not needed.
>>
>
> Eh, this is still replacing less typing with more. It's normal to
> define these things up front of a function especially when used
> multiple times. 'cs' should be as instantly recognizable as env
> when looking at code so my preference is to keep it as is.
You can copy & paste to avoid typing :-) Since it's only used in fatal
error messages that should not normally be executed I see no point in
storing a variable for that when it's never needed. Originally this patch
was to allow changing the function to take env instead of cpu but even
without that patch I think this simplifies it so I'd like to keep it if
you don't mind.
Regards,
BALATON Zoltan
> Thanks,
> Nick
>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> target/ppc/excp_helper.c | 118 +++++++++++++++++++++------------------
>> 1 file changed, 63 insertions(+), 55 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 79f5ca1034..122e2a6e41 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -424,7 +424,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)
>>
>> static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>> int srr0, srr1;
>> @@ -452,8 +451,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -502,7 +501,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>> env->spr[SPR_40x_ESR] = ESR_PTR;
>> break;
>> default:
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -529,11 +528,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>> trace_ppc_excp_print("PIT");
>> break;
>> case POWERPC_EXCP_DEBUG: /* Debug interrupt */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -548,7 +548,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>
>> static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>>
>> @@ -571,8 +570,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -632,7 +631,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>> break;
>> default:
>> /* Should never occur */
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -654,8 +653,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>> break;
>> case POWERPC_EXCP_RESET: /* System reset exception */
>> if (FIELD_EX64(env->msr, MSR, POW)) {
>> - cpu_abort(cs, "Trying to deliver power-saving system reset "
>> - "exception %d with no HV support\n", excp);
>> + cpu_abort(env_cpu(env),
>> + "Trying to deliver power-saving system reset exception "
>> + "%d with no HV support\n", excp);
>> }
>> break;
>> case POWERPC_EXCP_TRACE: /* Trace exception */
>> @@ -682,11 +682,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>> case POWERPC_EXCP_SMI: /* System management interrupt */
>> case POWERPC_EXCP_MEXTBR: /* Maskable external breakpoint */
>> case POWERPC_EXCP_NMEXTBR: /* Non maskable external breakpoint */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -709,7 +710,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
>>
>> static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>>
>> @@ -732,8 +732,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -791,7 +791,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>> break;
>> default:
>> /* Should never occur */
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -832,8 +832,9 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>> break;
>> case POWERPC_EXCP_RESET: /* System reset exception */
>> if (FIELD_EX64(env->msr, MSR, POW)) {
>> - cpu_abort(cs, "Trying to deliver power-saving system reset "
>> - "exception %d with no HV support\n", excp);
>> + cpu_abort(env_cpu(env),
>> + "Trying to deliver power-saving system reset exception "
>> + "%d with no HV support\n", excp);
>> }
>> break;
>> case POWERPC_EXCP_TRACE: /* Trace exception */
>> @@ -853,11 +854,12 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>> case POWERPC_EXCP_SMI: /* System management interrupt */
>> case POWERPC_EXCP_THERM: /* Thermal interrupt */
>> case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -880,7 +882,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>>
>> static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>>
>> @@ -903,8 +904,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -962,7 +963,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>> break;
>> default:
>> /* Should never occur */
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -1003,7 +1004,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>> break;
>> case POWERPC_EXCP_RESET: /* System reset exception */
>> if (FIELD_EX64(env->msr, MSR, POW)) {
>> - cpu_abort(cs, "Trying to deliver power-saving system reset "
>> + cpu_abort(env_cpu(env),
>> + "Trying to deliver power-saving system reset "
>> "exception %d with no HV support\n", excp);
>> }
>> break;
>> @@ -1016,11 +1018,12 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>> case POWERPC_EXCP_THERM: /* Thermal interrupt */
>> case POWERPC_EXCP_PERFM: /* Embedded performance monitor interrupt */
>> case POWERPC_EXCP_VPUA: /* Vector assist exception */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -1043,7 +1046,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>>
>> static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>> int srr0, srr1;
>> @@ -1080,8 +1082,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -1112,6 +1114,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>> break;
>> case POWERPC_EXCP_EXTERNAL: /* External input */
>> if (env->mpic_proxy) {
>> + CPUState *cs = env_cpu(env);
>> /* IACK the IRQ on delivery */
>> env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
>> }
>> @@ -1150,7 +1153,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>> break;
>> default:
>> /* Should never occur */
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -1191,7 +1194,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>>
>> /* DBSR already modified by caller */
>> } else {
>> - cpu_abort(cs, "Debug exception triggered on unsupported model\n");
>> + cpu_abort(env_cpu(env),
>> + "Debug exception triggered on unsupported model\n");
>> }
>> break;
>> case POWERPC_EXCP_SPEU: /* SPE/embedded floating-point unavailable/VPU */
>> @@ -1205,17 +1209,19 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
>> break;
>> case POWERPC_EXCP_RESET: /* System reset exception */
>> if (FIELD_EX64(env->msr, MSR, POW)) {
>> - cpu_abort(cs, "Trying to deliver power-saving system reset "
>> + cpu_abort(env_cpu(env),
>> + "Trying to deliver power-saving system reset "
>> "exception %d with no HV support\n", excp);
>> }
>> break;
>> case POWERPC_EXCP_EFPDI: /* Embedded floating-point data interrupt */
>> case POWERPC_EXCP_EFPRI: /* Embedded floating-point round interrupt */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -1278,7 +1284,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)
>>
>> static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>> target_ulong msr, new_msr, vector;
>> int srr0, srr1, lev = -1;
>> @@ -1317,8 +1322,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>>
>> vector = env->excp_vectors[excp];
>> if (vector == (target_ulong)-1ULL) {
>> - cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> - excp);
>> + cpu_abort(env_cpu(env),
>> + "Raised an exception without defined vector %d\n", excp);
>> }
>>
>> vector |= env->excp_prefix;
>> @@ -1408,7 +1413,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> break;
>> default:
>> /* Should never occur */
>> - cpu_abort(cs, "Invalid program exception %d. Aborting\n",
>> + cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
>> env->error_code);
>> break;
>> }
>> @@ -1469,7 +1474,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> new_msr |= (target_ulong)MSR_HVB;
>> } else {
>> if (FIELD_EX64(env->msr, MSR, POW)) {
>> - cpu_abort(cs, "Trying to deliver power-saving system reset "
>> + cpu_abort(env_cpu(env),
>> + "Trying to deliver power-saving system reset "
>> "exception %d with no HV support\n", excp);
>> }
>> }
>> @@ -1524,11 +1530,12 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> case POWERPC_EXCP_VPUA: /* Vector assist exception */
>> case POWERPC_EXCP_MAINT: /* Maintenance exception */
>> case POWERPC_EXCP_HV_MAINT: /* Hypervisor Maintenance exception */
>> - cpu_abort(cs, "%s exception not implemented\n",
>> + cpu_abort(env_cpu(env), "%s exception not implemented\n",
>> powerpc_excp_name(excp));
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> break;
>> }
>>
>> @@ -1561,8 +1568,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> } else {
>> /* Sanity check */
>> if (!(env->msr_mask & MSR_HVB) && srr0 == SPR_HSRR0) {
>> - cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
>> - "no HV support\n", excp);
>> + cpu_abort(env_cpu(env), "Trying to deliver HV exception (HSRR) %d "
>> + "with no HV support\n", excp);
>> }
>>
>> /* This can update new_msr and vector if AIL applies */
>> @@ -1580,11 +1587,11 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>>
>> static void powerpc_excp(PowerPCCPU *cpu, int excp)
>> {
>> - CPUState *cs = CPU(cpu);
>> CPUPPCState *env = &cpu->env;
>>
>> if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
>> - cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
>> + excp);
>> }
>>
>> qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
>> @@ -2118,7 +2125,6 @@ void ppc_maybe_interrupt(CPUPPCState *env)
>> static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
>> {
>> PowerPCCPU *cpu = env_archcpu(env);
>> - CPUState *cs = env_cpu(env);
>>
>> switch (interrupt) {
>> case PPC_INTERRUPT_MCK: /* Machine check exception */
>> @@ -2162,14 +2168,14 @@ static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
>> assert(!env->resume_as_sreset);
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
>> + interrupt);
>> }
>> }
>>
>> static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
>> {
>> PowerPCCPU *cpu = env_archcpu(env);
>> - CPUState *cs = env_cpu(env);
>>
>> switch (interrupt) {
>> case PPC_INTERRUPT_MCK: /* Machine check exception */
>> @@ -2233,7 +2239,8 @@ static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
>> assert(!env->resume_as_sreset);
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
>> + interrupt);
>> }
>> }
>>
>> @@ -2312,7 +2319,8 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
>> assert(!env->resume_as_sreset);
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
>> + interrupt);
>> }
>> }
>> #endif
>> @@ -2320,7 +2328,6 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
>> static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
>> {
>> PowerPCCPU *cpu = env_archcpu(env);
>> - CPUState *cs = env_cpu(env);
>>
>> switch (interrupt) {
>> case PPC_INTERRUPT_RESET: /* External reset */
>> @@ -2417,7 +2424,8 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
>> assert(!env->resume_as_sreset);
>> break;
>> default:
>> - cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
>> + cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
>> + interrupt);
>> }
>> }
>>
>> --
>> 2.30.9
>
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (3 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 5:01 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 06/14] target/ppc: Readability improvements in exception handlers BALATON Zoltan
` (9 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Some helpers only have a CPUState local to call cpu_interrupt_exittb()
but we can use env_cpu for that and remove the local.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 7 ++-----
1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 122e2a6e41..a175865fa9 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2551,8 +2551,7 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
uint32_t excp = hreg_store_msr(env, val, 0);
if (excp != 0) {
- CPUState *cs = env_cpu(env);
- cpu_interrupt_exittb(cs);
+ cpu_interrupt_exittb(env_cpu(env));
raise_exception(env, excp);
}
}
@@ -2589,8 +2588,6 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn)
static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
{
- CPUState *cs = env_cpu(env);
-
/* MSR:POW cannot be set by any form of rfi */
msr &= ~(1ULL << MSR_POW);
@@ -2614,7 +2611,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
* No need to raise an exception here, as rfi is always the last
* insn of a TB
*/
- cpu_interrupt_exittb(cs);
+ cpu_interrupt_exittb(env_cpu(env));
/* Reset the reservation */
env->reserve_addr = -1;
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once
2023-06-15 23:03 ` [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once BALATON Zoltan
@ 2023-06-20 5:01 ` Nicholas Piggin
0 siblings, 0 replies; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 5:01 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> Some helpers only have a CPUState local to call cpu_interrupt_exittb()
> but we can use env_cpu for that and remove the local.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
I have less issue with this one.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
> ---
> target/ppc/excp_helper.c | 7 ++-----
> 1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 122e2a6e41..a175865fa9 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2551,8 +2551,7 @@ void helper_store_msr(CPUPPCState *env, target_ulong val)
> uint32_t excp = hreg_store_msr(env, val, 0);
>
> if (excp != 0) {
> - CPUState *cs = env_cpu(env);
> - cpu_interrupt_exittb(cs);
> + cpu_interrupt_exittb(env_cpu(env));
> raise_exception(env, excp);
> }
> }
> @@ -2589,8 +2588,6 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn)
>
> static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
> {
> - CPUState *cs = env_cpu(env);
> -
> /* MSR:POW cannot be set by any form of rfi */
> msr &= ~(1ULL << MSR_POW);
>
> @@ -2614,7 +2611,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
> * No need to raise an exception here, as rfi is always the last
> * insn of a TB
> */
> - cpu_interrupt_exittb(cs);
> + cpu_interrupt_exittb(env_cpu(env));
> /* Reset the reservation */
> env->reserve_addr = -1;
>
> --
> 2.30.9
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 06/14] target/ppc: Readability improvements in exception handlers
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (4 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 07/14] target/ppd: Remove unused define BALATON Zoltan
` (8 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Improve readability by shortening some long comments, removing
comments that state the obvious and dropping some empty lines so they
don't distract when reading the code.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/cpu.h | 1 +
target/ppc/excp_helper.c | 180 +++++++--------------------------------
2 files changed, 33 insertions(+), 148 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 0ee2adc105..d7acd65176 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2739,6 +2739,7 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr)
}
#if !defined(CONFIG_USER_ONLY)
+/* Sort out endianness of interrupt. Depends on the CPU, HV mode, etc. */
static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
{
PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a175865fa9..f19a0f2d1d 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -382,9 +382,8 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu, target_ulong vector,
* We don't use hreg_store_msr here as already have treated any
* special case that could occur. Just store MSR and update hflags
*
- * Note: We *MUST* not use hreg_store_msr() as-is anyway because it
- * will prevent setting of the HV bit which some exceptions might need
- * to do.
+ * Note: We *MUST* not use hreg_store_msr() as-is anyway because it will
+ * prevent setting of the HV bit which some exceptions might need to do.
*/
env->nip = vector;
env->msr = msr;
@@ -426,25 +425,15 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
{
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
- int srr0, srr1;
+ int srr0 = SPR_SRR0, srr1 = SPR_SRR1;
/* new srr1 value excluding must-be-zero bits */
msr = env->msr & ~0x783f0000ULL;
- /*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overriden.
- */
+ /* new interrupt handler msr preserves ME unless explicitly overriden */
new_msr = env->msr & (((target_ulong)1 << MSR_ME));
- /* target registers */
- srr0 = SPR_SRR0;
- srr1 = SPR_SRR1;
-
- /*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+ /* HV emu assistance interrupt only exists on server arch 2.05 or later */
if (excp == POWERPC_EXCP_HV_EMU) {
excp = POWERPC_EXCP_PROGRAM;
}
@@ -454,7 +443,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -466,7 +454,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
-
srr0 = SPR_40x_SRR2;
srr1 = SPR_40x_SRR3;
break;
@@ -537,12 +524,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
break;
}
- /* Save PC */
env->spr[srr0] = env->nip;
-
- /* Save MSR */
env->spr[srr1] = msr;
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
@@ -554,16 +537,10 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
/* new srr1 value excluding must-be-zero bits */
msr = env->msr & ~0x783f0000ULL;
- /*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overriden
- */
+ /* new interrupt handler msr preserves ME unless explicitly overriden */
new_msr = env->msr & ((target_ulong)1 << MSR_ME);
- /*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+ /* HV emu assistance interrupt only exists on server arch 2.05 or later */
if (excp == POWERPC_EXCP_HV_EMU) {
excp = POWERPC_EXCP_PROGRAM;
}
@@ -573,7 +550,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -583,7 +559,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
-
break;
case POWERPC_EXCP_DSI: /* Data storage exception */
trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
@@ -611,11 +586,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
powerpc_reset_excp_state(cpu);
return;
}
-
/*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, so always use store_next and claim we are
- * precise in the MSR.
+ * NIP always points to the faulting instruction for FP exceptions,
+ * so always use store_next and claim we are precise in the MSR.
*/
msr |= 0x00100000;
break;
@@ -691,20 +664,11 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
break;
}
- /*
- * Sort out endianness of interrupt, this differs depending on the
- * CPU, the HV mode, etc...
- */
if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
-
- /* Save PC */
env->spr[SPR_SRR0] = env->nip;
-
- /* Save MSR */
env->spr[SPR_SRR1] = msr;
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
@@ -716,16 +680,10 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
/* new srr1 value excluding must-be-zero bits */
msr = env->msr & ~0x783f0000ULL;
- /*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overriden
- */
+ /* new interrupt handler msr preserves ME unless explicitly overriden */
new_msr = env->msr & ((target_ulong)1 << MSR_ME);
- /*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+ /* HV emu assistance interrupt only exists on server arch 2.05 or later */
if (excp == POWERPC_EXCP_HV_EMU) {
excp = POWERPC_EXCP_PROGRAM;
}
@@ -735,7 +693,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -743,7 +700,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
-
break;
case POWERPC_EXCP_DSI: /* Data storage exception */
trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
@@ -771,11 +727,9 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
powerpc_reset_excp_state(cpu);
return;
}
-
/*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, so always use store_next and claim we are
- * precise in the MSR.
+ * NIP always points to the faulting instruction for FP exceptions,
+ * so always use store_next and claim we are precise in the MSR.
*/
msr |= 0x00100000;
break;
@@ -843,12 +797,10 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_DLTLB: /* Data load TLB miss */
case POWERPC_EXCP_DSTLB: /* Data store TLB miss */
ppc_excp_debug_sw_tlb(env, excp);
-
msr |= env->crf[0] << 28;
msr |= env->error_code; /* key, D/I, S/L bits */
/* Set way using a LRU mechanism */
msr |= ((env->last_way + 1) & (env->nb_ways - 1)) << 17;
-
break;
case POWERPC_EXCP_IABR: /* Instruction address breakpoint */
case POWERPC_EXCP_SMI: /* System management interrupt */
@@ -863,20 +815,11 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
break;
}
- /*
- * Sort out endianness of interrupt, this differs depending on the
- * CPU, the HV mode, etc...
- */
if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
-
- /* Save PC */
env->spr[SPR_SRR0] = env->nip;
-
- /* Save MSR */
env->spr[SPR_SRR1] = msr;
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
@@ -888,16 +831,10 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
/* new srr1 value excluding must-be-zero bits */
msr = env->msr & ~0x783f0000ULL;
- /*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overriden
- */
+ /* new interrupt handler msr preserves ME unless explicitly overriden */
new_msr = env->msr & ((target_ulong)1 << MSR_ME);
- /*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+ /* HV emu assistance interrupt only exists on server arch 2.05 or later */
if (excp == POWERPC_EXCP_HV_EMU) {
excp = POWERPC_EXCP_PROGRAM;
}
@@ -907,7 +844,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -915,7 +851,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
powerpc_mcheck_checkstop(env);
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
-
break;
case POWERPC_EXCP_DSI: /* Data storage exception */
trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
@@ -943,11 +878,9 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
powerpc_reset_excp_state(cpu);
return;
}
-
/*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, so always use store_next and claim we are
- * precise in the MSR.
+ * NIP always points to the faulting instruction for FP exceptions,
+ * so always use store_next and claim we are precise in the MSR.
*/
msr |= 0x00100000;
break;
@@ -1027,20 +960,11 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
break;
}
- /*
- * Sort out endianness of interrupt, this differs depending on the
- * CPU, the HV mode, etc...
- */
if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
-
- /* Save PC */
env->spr[SPR_SRR0] = env->nip;
-
- /* Save MSR */
env->spr[SPR_SRR1] = msr;
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
@@ -1048,24 +972,18 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
{
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
- int srr0, srr1;
-
- msr = env->msr;
+ int srr0 = SPR_SRR0, srr1 = SPR_SRR1;
/*
- * new interrupt handler msr preserves existing ME unless
- * explicitly overriden
+ * Book E does not play games with certain bits of xSRR1 being MSR save
+ * bits and others being error status. xSRR1 is the old MSR, period.
*/
- new_msr = env->msr & ((target_ulong)1 << MSR_ME);
+ msr = env->msr;
- /* target registers */
- srr0 = SPR_SRR0;
- srr1 = SPR_SRR1;
+ /* new interrupt handler msr preserves ME unless explicitly overriden */
+ new_msr = env->msr & ((target_ulong)1 << MSR_ME);
- /*
- * Hypervisor emulation assistance interrupt only exists on server
- * arch 2.05 server or later.
- */
+ /* HV emu assistance interrupt only exists on server arch 2.05 or later */
if (excp == POWERPC_EXCP_HV_EMU) {
excp = POWERPC_EXCP_PROGRAM;
}
@@ -1085,7 +1003,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -1129,11 +1046,9 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
powerpc_reset_excp_state(cpu);
return;
}
-
/*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, so always use store_next and claim we are
- * precise in the MSR.
+ * NIP always points to the faulting instruction for FP exceptions,
+ * so always use store_next and claim we are precise in the MSR.
*/
msr |= 0x00100000;
env->spr[SPR_BOOKE_ESR] = ESR_FP;
@@ -1234,12 +1149,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
}
#endif
- /* Save PC */
env->spr[srr0] = env->nip;
-
- /* Save MSR */
env->spr[srr1] = msr;
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
@@ -1286,21 +1197,17 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
{
CPUPPCState *env = &cpu->env;
target_ulong msr, new_msr, vector;
- int srr0, srr1, lev = -1;
+ int srr0 = SPR_SRR0, srr1 = SPR_SRR1, lev = -1;
/* new srr1 value excluding must-be-zero bits */
msr = env->msr & ~0x783f0000ULL;
/*
- * new interrupt handler msr preserves existing HV and ME unless
- * explicitly overriden
+ * new interrupt handler msr preserves HV and ME unless explicitly
+ * overriden
*/
new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
- /* target registers */
- srr0 = SPR_SRR0;
- srr1 = SPR_SRR1;
-
/*
* check for special resume at 0x100 from doze/nap/sleep/winkle on
* P7/P8/P9
@@ -1325,7 +1232,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
cpu_abort(env_cpu(env),
"Raised an exception without defined vector %d\n", excp);
}
-
vector |= env->excp_prefix;
switch (excp) {
@@ -1338,10 +1244,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
*/
new_msr |= (target_ulong)MSR_HVB;
}
-
/* machine check exceptions don't have ME set */
new_msr &= ~((target_ulong)1 << MSR_ME);
-
break;
case POWERPC_EXCP_DSI: /* Data storage exception */
trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
@@ -1354,23 +1258,17 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
{
bool lpes0;
- /*
- * LPES0 is only taken into consideration if we support HV
- * mode for this CPU.
- */
+ /* LPES0 is only taken into consideration if we support HV mode */
if (!env->has_hv_mode) {
break;
}
-
lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
-
if (!lpes0) {
new_msr |= (target_ulong)MSR_HVB;
new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
srr0 = SPR_HSRR0;
srr1 = SPR_HSRR1;
}
-
break;
}
case POWERPC_EXCP_ALIGN: /* Alignment exception */
@@ -1393,11 +1291,9 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
powerpc_reset_excp_state(cpu);
return;
}
-
/*
- * FP exceptions always have NIP pointing to the faulting
- * instruction, so always use store_next and claim we are
- * precise in the MSR.
+ * NIP always points to the faulting instruction for FP exceptions,
+ * so always use store_next and claim we are precise in the MSR.
*/
msr |= 0x00100000;
break;
@@ -1539,21 +1435,13 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
break;
}
- /*
- * Sort out endianness of interrupt, this differs depending on the
- * CPU, the HV mode, etc...
- */
if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
new_msr |= (target_ulong)1 << MSR_LE;
}
-
new_msr |= (target_ulong)1 << MSR_SF;
if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
- /* Save PC */
env->spr[srr0] = env->nip;
-
- /* Save MSR */
env->spr[srr1] = msr;
}
@@ -1562,19 +1450,15 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
/* Deliver interrupt to L1 by returning from the H_ENTER_NESTED call */
vhc->deliver_hv_excp(cpu, excp);
-
powerpc_reset_excp_state(cpu);
-
} else {
/* Sanity check */
if (!(env->msr_mask & MSR_HVB) && srr0 == SPR_HSRR0) {
cpu_abort(env_cpu(env), "Trying to deliver HV exception (HSRR) %d "
"with no HV support\n", excp);
}
-
/* This can update new_msr and vector if AIL applies */
ppc_excp_apply_ail(cpu, excp, msr, &new_msr, &vector);
-
powerpc_set_excp_state(cpu, vector, new_msr);
}
}
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 07/14] target/ppd: Remove unused define
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (5 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 06/14] target/ppc: Readability improvements in exception handlers BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-18 12:48 ` Bernhard Beschow
2023-06-15 23:03 ` [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
` (7 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Commit 7a3fe174b12d removed usage of POWERPC_SYSCALL_VECTORED, drop
the unused define as well.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/translate.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index b591f2e496..a32a9b8a5f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4416,7 +4416,6 @@ static void gen_hrfid(DisasContext *ctx)
#define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL_USER
#else
#define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL
-#define POWERPC_SYSCALL_VECTORED POWERPC_EXCP_SYSCALL_VECTORED
#endif
static void gen_sc(DisasContext *ctx)
{
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 07/14] target/ppd: Remove unused define
2023-06-15 23:03 ` [PATCH v3 07/14] target/ppd: Remove unused define BALATON Zoltan
@ 2023-06-18 12:48 ` Bernhard Beschow
0 siblings, 0 replies; 36+ messages in thread
From: Bernhard Beschow @ 2023-06-18 12:48 UTC (permalink / raw)
To: qemu-devel, BALATON Zoltan, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Typo: s/ppd/ppc/ in the header of the commit message.
Best regards,
Bernhard
Am 15. Juni 2023 23:03:16 UTC schrieb BALATON Zoltan <balaton@eik.bme.hu>:
>Commit 7a3fe174b12d removed usage of POWERPC_SYSCALL_VECTORED, drop
>the unused define as well.
>
>Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>---
> target/ppc/translate.c | 1 -
> 1 file changed, 1 deletion(-)
>
>diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>index b591f2e496..a32a9b8a5f 100644
>--- a/target/ppc/translate.c
>+++ b/target/ppc/translate.c
>@@ -4416,7 +4416,6 @@ static void gen_hrfid(DisasContext *ctx)
> #define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL_USER
> #else
> #define POWERPC_SYSCALL POWERPC_EXCP_SYSCALL
>-#define POWERPC_SYSCALL_VECTORED POWERPC_EXCP_SYSCALL_VECTORED
> #endif
> static void gen_sc(DisasContext *ctx)
> {
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (6 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 07/14] target/ppd: Remove unused define BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 4:03 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
` (6 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Most exceptions are raised with nip pointing to the faulting
instruction but the sc instruction generating a syscall exception
leaves nip pointing to next instruction. Fix gen_sc to not use
gen_exception_err() which sets nip back but correctly set nip to
pc_next so we don't have to patch this in the exception handlers.
This changes the nip logged in dump_syscall and dump_hcall debug
functions but now this matches how nip would be on a real CPU.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 39 ---------------------------------------
target/ppc/translate.c | 8 +++++---
2 files changed, 5 insertions(+), 42 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index f19a0f2d1d..903216c2a6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -495,12 +495,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_SYSCALL: /* System call exception */
dump_syscall(env);
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
break;
case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */
trace_ppc_excp_print("FIT");
@@ -611,12 +605,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_SYSCALL: /* System call exception */
dump_syscall(env);
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
break;
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
case POWERPC_EXCP_DECR: /* Decrementer exception */
@@ -759,13 +747,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
} else {
dump_syscall(env);
}
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
-
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
@@ -910,13 +891,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
} else {
dump_syscall(env);
}
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
-
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
@@ -1075,12 +1049,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
break;
case POWERPC_EXCP_SYSCALL: /* System call exception */
dump_syscall(env);
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
break;
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */
@@ -1322,13 +1290,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
} else {
dump_syscall(env);
}
-
- /*
- * We need to correct the NIP which in this case is supposed
- * to point to the next instruction
- */
- env->nip += 4;
-
/* "PAPR mode" built-in hypercall emulation */
if (lev == 1 && books_vhyp_handles_hcall(cpu)) {
PPCVirtualHypervisorClass *vhc =
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index a32a9b8a5f..4260d3d66f 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4419,10 +4419,12 @@ static void gen_hrfid(DisasContext *ctx)
#endif
static void gen_sc(DisasContext *ctx)
{
- uint32_t lev;
+ uint32_t lev = (ctx->opcode >> 5) & 0x7F;
- lev = (ctx->opcode >> 5) & 0x7F;
- gen_exception_err(ctx, POWERPC_SYSCALL, lev);
+ gen_update_nip(ctx, ctx->base.pc_next);
+ gen_helper_raise_exception_err(cpu_env, tcg_constant_i32(POWERPC_SYSCALL),
+ tcg_constant_i32(lev));
+ ctx->base.is_jmp = DISAS_NORETURN;
}
#if defined(TARGET_PPC64)
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip
2023-06-15 23:03 ` [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
@ 2023-06-20 4:03 ` Nicholas Piggin
0 siblings, 0 replies; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 4:03 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> Most exceptions are raised with nip pointing to the faulting
> instruction but the sc instruction generating a syscall exception
> leaves nip pointing to next instruction. Fix gen_sc to not use
> gen_exception_err() which sets nip back but correctly set nip to
> pc_next so we don't have to patch this in the exception handlers.
>
> This changes the nip logged in dump_syscall and dump_hcall debug
> functions but now this matches how nip would be on a real CPU.
I think this is okay. I'll just send a possible scv change after
this goes in.
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> target/ppc/excp_helper.c | 39 ---------------------------------------
> target/ppc/translate.c | 8 +++++---
> 2 files changed, 5 insertions(+), 42 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index f19a0f2d1d..903216c2a6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -495,12 +495,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_SYSCALL: /* System call exception */
> dump_syscall(env);
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> break;
> case POWERPC_EXCP_FIT: /* Fixed-interval timer interrupt */
> trace_ppc_excp_print("FIT");
> @@ -611,12 +605,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_SYSCALL: /* System call exception */
> dump_syscall(env);
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> break;
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> case POWERPC_EXCP_DECR: /* Decrementer exception */
> @@ -759,13 +747,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> } else {
> dump_syscall(env);
> }
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> -
> /*
> * The Virtual Open Firmware (VOF) relies on the 'sc 1'
> * instruction to communicate with QEMU. The pegasos2 machine
> @@ -910,13 +891,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> } else {
> dump_syscall(env);
> }
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> -
> /*
> * The Virtual Open Firmware (VOF) relies on the 'sc 1'
> * instruction to communicate with QEMU. The pegasos2 machine
> @@ -1075,12 +1049,6 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
> break;
> case POWERPC_EXCP_SYSCALL: /* System call exception */
> dump_syscall(env);
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> break;
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> case POWERPC_EXCP_APU: /* Auxiliary processor unavailable */
> @@ -1322,13 +1290,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> } else {
> dump_syscall(env);
> }
> -
> - /*
> - * We need to correct the NIP which in this case is supposed
> - * to point to the next instruction
> - */
> - env->nip += 4;
> -
> /* "PAPR mode" built-in hypercall emulation */
> if (lev == 1 && books_vhyp_handles_hcall(cpu)) {
> PPCVirtualHypervisorClass *vhc =
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index a32a9b8a5f..4260d3d66f 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4419,10 +4419,12 @@ static void gen_hrfid(DisasContext *ctx)
> #endif
> static void gen_sc(DisasContext *ctx)
> {
> - uint32_t lev;
> + uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>
> - lev = (ctx->opcode >> 5) & 0x7F;
> - gen_exception_err(ctx, POWERPC_SYSCALL, lev);
> + gen_update_nip(ctx, ctx->base.pc_next);
> + gen_helper_raise_exception_err(cpu_env, tcg_constant_i32(POWERPC_SYSCALL),
> + tcg_constant_i32(lev));
> + ctx->base.is_jmp = DISAS_NORETURN;
> }
>
> #if defined(TARGET_PPC64)
> --
> 2.30.9
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (7 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 4:09 ` Nicholas Piggin
2023-06-26 11:28 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 10/14] target/ppc: Simplify syscall exception handlers BALATON Zoltan
` (5 subsequent siblings)
14 siblings, 2 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
From: Nicholas Piggin <npiggin@gmail.com>
Unlike sc, for scv a facility unavailable interrupt must be generated
if FSCR[SCV]=0 so we can't raise the exception with nip set to next
instruction but we can move advancing nip if the FSCR check passes to
helper_scv so the exception handler does not need to change it.
[balaton: added commit message]
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
This needs SoB from Nick
target/ppc/excp_helper.c | 2 +-
target/ppc/translate.c | 6 +++++-
2 files changed, 6 insertions(+), 2 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 903216c2a6..ef363b0285 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */
lev = env->error_code;
dump_syscall(env);
- env->nip += 4;
new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
@@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
void helper_scv(CPUPPCState *env, uint32_t lev)
{
if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
+ env->nip += 4;
raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
} else {
raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 4260d3d66f..0360a17fb3 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx)
{
uint32_t lev = (ctx->opcode >> 5) & 0x7F;
- /* Set the PC back to the faulting instruction. */
+ /*
+ * Set the PC back to the scv instruction (unlike sc), because a facility
+ * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
+ * advances nip if the FSCR check passes.
+ */
gen_update_nip(ctx, ctx->cia);
gen_helper_scv(cpu_env, tcg_constant_i32(lev));
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-15 23:03 ` [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
@ 2023-06-20 4:09 ` Nicholas Piggin
2023-06-20 10:47 ` BALATON Zoltan
2023-06-26 11:28 ` Nicholas Piggin
1 sibling, 1 reply; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 4:09 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
>
> Unlike sc, for scv a facility unavailable interrupt must be generated
> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
> instruction but we can move advancing nip if the FSCR check passes to
> helper_scv so the exception handler does not need to change it.
>
> [balaton: added commit message]
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Ah you sent it, fine, thank you. But actually now I look again,
now we're off by one in the other direction for the dumps.
So... probably your way is still better because it matches the
interrupt semantics of the ISA when executing the instruction,
but it needs this patch:
For my patch you can add
Signed-off-by: Nicholas Piggin <npiggin@gmail.com
Thanks,
Nick
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0e21cb4451..d7f42639c8 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -117,7 +117,7 @@ static void dump_syscall(CPUPPCState *env)
ppc_dump_gpr(env, 0), ppc_dump_gpr(env, 3),
ppc_dump_gpr(env, 4), ppc_dump_gpr(env, 5),
ppc_dump_gpr(env, 6), ppc_dump_gpr(env, 7),
- ppc_dump_gpr(env, 8), env->nip);
+ ppc_dump_gpr(env, 8), env->nip - 4);
}
static void dump_hcall(CPUPPCState *env)
@@ -132,7 +132,7 @@ static void dump_hcall(CPUPPCState *env)
ppc_dump_gpr(env, 7), ppc_dump_gpr(env, 8),
ppc_dump_gpr(env, 9), ppc_dump_gpr(env, 10),
ppc_dump_gpr(env, 11), ppc_dump_gpr(env, 12),
- env->nip);
+ env->nip - 4);
}
#ifdef CONFIG_TCG
> ---
> This needs SoB from Nick
>
> target/ppc/excp_helper.c | 2 +-
> target/ppc/translate.c | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 903216c2a6..ef363b0285 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */
> lev = env->error_code;
> dump_syscall(env);
> - env->nip += 4;
> new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
> new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>
> @@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
> void helper_scv(CPUPPCState *env, uint32_t lev)
> {
> if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
> + env->nip += 4;
> raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
> } else {
> raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 4260d3d66f..0360a17fb3 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx)
> {
> uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>
> - /* Set the PC back to the faulting instruction. */
> + /*
> + * Set the PC back to the scv instruction (unlike sc), because a facility
> + * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
> + * advances nip if the FSCR check passes.
> + */
> gen_update_nip(ctx, ctx->cia);
> gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>
> --
> 2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-20 4:09 ` Nicholas Piggin
@ 2023-06-20 10:47 ` BALATON Zoltan
2023-06-26 11:25 ` Nicholas Piggin
0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-20 10:47 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
>> From: Nicholas Piggin <npiggin@gmail.com>
>>
>> Unlike sc, for scv a facility unavailable interrupt must be generated
>> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
>> instruction but we can move advancing nip if the FSCR check passes to
>> helper_scv so the exception handler does not need to change it.
>>
>> [balaton: added commit message]
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>
> Ah you sent it, fine, thank you. But actually now I look again,
> now we're off by one in the other direction for the dumps.
This is mentioned in the commit message for the patch changing sc. I think
we should not patch nip in the dump so we actually dump what the CPU
should have and match the ISA docs.
> So... probably your way is still better because it matches the
> interrupt semantics of the ISA when executing the instruction,
> but it needs this patch:
OK so then I'm confused why we need nip - 4 in dumps?
> For my patch you can add
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com
Please reply to that patch with this if you're OK with that or I'll just
drop it in v4 and let you send a follow up to avoid confusion.
Regards,
BALATON Zoltan
> Thanks,
> Nick
>
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 0e21cb4451..d7f42639c8 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -117,7 +117,7 @@ static void dump_syscall(CPUPPCState *env)
> ppc_dump_gpr(env, 0), ppc_dump_gpr(env, 3),
> ppc_dump_gpr(env, 4), ppc_dump_gpr(env, 5),
> ppc_dump_gpr(env, 6), ppc_dump_gpr(env, 7),
> - ppc_dump_gpr(env, 8), env->nip);
> + ppc_dump_gpr(env, 8), env->nip - 4);
> }
>
> static void dump_hcall(CPUPPCState *env)
> @@ -132,7 +132,7 @@ static void dump_hcall(CPUPPCState *env)
> ppc_dump_gpr(env, 7), ppc_dump_gpr(env, 8),
> ppc_dump_gpr(env, 9), ppc_dump_gpr(env, 10),
> ppc_dump_gpr(env, 11), ppc_dump_gpr(env, 12),
> - env->nip);
> + env->nip - 4);
> }
>
> #ifdef CONFIG_TCG
>
>
>
>> ---
>> This needs SoB from Nick
>>
>> target/ppc/excp_helper.c | 2 +-
>> target/ppc/translate.c | 6 +++++-
>> 2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 903216c2a6..ef363b0285 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>> case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */
>> lev = env->error_code;
>> dump_syscall(env);
>> - env->nip += 4;
>> new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
>> new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>>
>> @@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
>> void helper_scv(CPUPPCState *env, uint32_t lev)
>> {
>> if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
>> + env->nip += 4;
>> raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
>> } else {
>> raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
>> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
>> index 4260d3d66f..0360a17fb3 100644
>> --- a/target/ppc/translate.c
>> +++ b/target/ppc/translate.c
>> @@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx)
>> {
>> uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>>
>> - /* Set the PC back to the faulting instruction. */
>> + /*
>> + * Set the PC back to the scv instruction (unlike sc), because a facility
>> + * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
>> + * advances nip if the FSCR check passes.
>> + */
>> gen_update_nip(ctx, ctx->cia);
>> gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>>
>> --
>> 2.30.9
>
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-20 10:47 ` BALATON Zoltan
@ 2023-06-26 11:25 ` Nicholas Piggin
2023-06-27 17:40 ` BALATON Zoltan
0 siblings, 1 reply; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-26 11:25 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Tue Jun 20, 2023 at 8:47 PM AEST, BALATON Zoltan wrote:
> On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> > On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> >> From: Nicholas Piggin <npiggin@gmail.com>
> >>
> >> Unlike sc, for scv a facility unavailable interrupt must be generated
> >> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
> >> instruction but we can move advancing nip if the FSCR check passes to
> >> helper_scv so the exception handler does not need to change it.
> >>
> >> [balaton: added commit message]
> >> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >
> > Ah you sent it, fine, thank you. But actually now I look again,
> > now we're off by one in the other direction for the dumps.
>
> This is mentioned in the commit message for the patch changing sc. I think
> we should not patch nip in the dump so we actually dump what the CPU
> should have and match the ISA docs.
>
> > So... probably your way is still better because it matches the
> > interrupt semantics of the ISA when executing the instruction,
> > but it needs this patch:
>
> OK so then I'm confused why we need nip - 4 in dumps?
Sorry I missed your reply here. We want nip - 4 in dumps so the
address of the syscall is the sc instruction itself, not the
random one after it.
Thanks,
Nick
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-26 11:25 ` Nicholas Piggin
@ 2023-06-27 17:40 ` BALATON Zoltan
2023-06-28 1:03 ` Nicholas Piggin
0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-27 17:40 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Mon, 26 Jun 2023, Nicholas Piggin wrote:
> On Tue Jun 20, 2023 at 8:47 PM AEST, BALATON Zoltan wrote:
>> On Tue, 20 Jun 2023, Nicholas Piggin wrote:
>>> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
>>>> From: Nicholas Piggin <npiggin@gmail.com>
>>>>
>>>> Unlike sc, for scv a facility unavailable interrupt must be generated
>>>> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
>>>> instruction but we can move advancing nip if the FSCR check passes to
>>>> helper_scv so the exception handler does not need to change it.
>>>>
>>>> [balaton: added commit message]
>>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>>>
>>> Ah you sent it, fine, thank you. But actually now I look again,
>>> now we're off by one in the other direction for the dumps.
>>
>> This is mentioned in the commit message for the patch changing sc. I think
>> we should not patch nip in the dump so we actually dump what the CPU
>> should have and match the ISA docs.
>>
>>> So... probably your way is still better because it matches the
>>> interrupt semantics of the ISA when executing the instruction,
>>> but it needs this patch:
>>
>> OK so then I'm confused why we need nip - 4 in dumps?
>
> Sorry I missed your reply here. We want nip - 4 in dumps so the
> address of the syscall is the sc instruction itself, not the
> random one after it.
Although that's how it was in QEMU before that's not how it is on real
hardware so I don't think we should keep this and just log what a real CPU
would have and people should know how to interpret that after consulting
the ISA docs.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-27 17:40 ` BALATON Zoltan
@ 2023-06-28 1:03 ` Nicholas Piggin
0 siblings, 0 replies; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-28 1:03 UTC (permalink / raw)
To: BALATON Zoltan
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Wed Jun 28, 2023 at 3:40 AM AEST, BALATON Zoltan wrote:
> On Mon, 26 Jun 2023, Nicholas Piggin wrote:
> > On Tue Jun 20, 2023 at 8:47 PM AEST, BALATON Zoltan wrote:
> >> On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> >>> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> >>>> From: Nicholas Piggin <npiggin@gmail.com>
> >>>>
> >>>> Unlike sc, for scv a facility unavailable interrupt must be generated
> >>>> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
> >>>> instruction but we can move advancing nip if the FSCR check passes to
> >>>> helper_scv so the exception handler does not need to change it.
> >>>>
> >>>> [balaton: added commit message]
> >>>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> >>>
> >>> Ah you sent it, fine, thank you. But actually now I look again,
> >>> now we're off by one in the other direction for the dumps.
> >>
> >> This is mentioned in the commit message for the patch changing sc. I think
> >> we should not patch nip in the dump so we actually dump what the CPU
> >> should have and match the ISA docs.
> >>
> >>> So... probably your way is still better because it matches the
> >>> interrupt semantics of the ISA when executing the instruction,
> >>> but it needs this patch:
> >>
> >> OK so then I'm confused why we need nip - 4 in dumps?
> >
> > Sorry I missed your reply here. We want nip - 4 in dumps so the
> > address of the syscall is the sc instruction itself, not the
> > random one after it.
>
> Although that's how it was in QEMU before
Current upstream QEMU dumps syscall address of sc instruction. After
patch 8 and 9, it will dump the address of the instruction after it.
> that's not how it is on real
> hardware so I don't think we should keep this and just log what a real CPU
> would have and people should know how to interpret that after consulting
> the ISA docs.
I did get the feeling it was nicer your way, OTOH there really is not
anything in the ISA that requires a particular implementation. QEMU is
a real implementation of the ISA anyway. You could argue it's more
consistent for QEMU to keep env->nip as the address of instruction that
caused the interrupt, and then the sc fixup is restricted to setting
SRR0. I'm on the fence about it now.
Thanks,
Nick
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
2023-06-15 23:03 ` [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
2023-06-20 4:09 ` Nicholas Piggin
@ 2023-06-26 11:28 ` Nicholas Piggin
1 sibling, 0 replies; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-26 11:28 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> From: Nicholas Piggin <npiggin@gmail.com>
>
> Unlike sc, for scv a facility unavailable interrupt must be generated
> if FSCR[SCV]=0 so we can't raise the exception with nip set to next
> instruction but we can move advancing nip if the FSCR check passes to
> helper_scv so the exception handler does not need to change it.
>
> [balaton: added commit message]
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
Thanks, sorry for the delay :( Would you be able to resend the series?
You could drop the machine check one for now perhaps until we sort out
what to do with it.
Thanks,
Nick
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> This needs SoB from Nick
>
> target/ppc/excp_helper.c | 2 +-
> target/ppc/translate.c | 6 +++++-
> 2 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 903216c2a6..ef363b0285 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1304,7 +1304,6 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception */
> lev = env->error_code;
> dump_syscall(env);
> - env->nip += 4;
> new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
> new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>
> @@ -2410,6 +2409,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
> void helper_scv(CPUPPCState *env, uint32_t lev)
> {
> if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
> + env->nip += 4;
> raise_exception_err(env, POWERPC_EXCP_SYSCALL_VECTORED, lev);
> } else {
> raise_exception_err(env, POWERPC_EXCP_FU, FSCR_IC_SCV);
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 4260d3d66f..0360a17fb3 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -4433,7 +4433,11 @@ static void gen_scv(DisasContext *ctx)
> {
> uint32_t lev = (ctx->opcode >> 5) & 0x7F;
>
> - /* Set the PC back to the faulting instruction. */
> + /*
> + * Set the PC back to the scv instruction (unlike sc), because a facility
> + * unavailable interrupt must be generated if FSCR[SCV]=0. The helper
> + * advances nip if the FSCR check passes.
> + */
> gen_update_nip(ctx, ctx->cia);
> gen_helper_scv(cpu_env, tcg_constant_i32(lev));
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 10/14] target/ppc: Simplify syscall exception handlers
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (8 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 5:05 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 11/14] target/ppc: Get CPUState in one step BALATON Zoltan
` (4 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
After previous changes the hypercall handling in 7xx and 74xx
exception handlers can be folded into one if statement to simpilfy
this code.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 24 ++++++++----------------
1 file changed, 8 insertions(+), 16 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ef363b0285..a62103b8ac 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -741,25 +741,21 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception */
{
int lev = env->error_code;
-
- if (lev == 1 && cpu->vhyp) {
- dump_hcall(env);
- } else {
- dump_syscall(env);
- }
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 7xx CPUs, so although the 7xx don't have
* HV mode, we need to keep hypercall support.
*/
- if (lev == 1 && cpu->vhyp) {
+ if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
return;
+ } else {
+ dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
@@ -885,25 +881,21 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
case POWERPC_EXCP_SYSCALL: /* System call exception */
{
int lev = env->error_code;
-
- if (lev == 1 && cpu->vhyp) {
- dump_hcall(env);
- } else {
- dump_syscall(env);
- }
/*
* The Virtual Open Firmware (VOF) relies on the 'sc 1'
* instruction to communicate with QEMU. The pegasos2 machine
* uses VOF and the 74xx CPUs, so although the 74xx don't have
* HV mode, we need to keep hypercall support.
*/
- if (lev == 1 && cpu->vhyp) {
+ if (unlikely(lev == 1 && cpu->vhyp)) {
PPCVirtualHypervisorClass *vhc =
PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
+ dump_hcall(env);
vhc->hypercall(cpu->vhyp, cpu);
return;
+ } else {
+ dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 10/14] target/ppc: Simplify syscall exception handlers
2023-06-15 23:03 ` [PATCH v3 10/14] target/ppc: Simplify syscall exception handlers BALATON Zoltan
@ 2023-06-20 5:05 ` Nicholas Piggin
0 siblings, 0 replies; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 5:05 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> After previous changes the hypercall handling in 7xx and 74xx
> exception handlers can be folded into one if statement to simpilfy
> this code.
Simplify and add unlikely?
I like to avoid slipping in improvements with cleanups. Arguably
the changelog is actually more important to describe the stuff
that isn't just rejig of code. Pretty minor in this case, but
good practice otherwise it can get out of hand.
Thanks,
Nick
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> target/ppc/excp_helper.c | 24 ++++++++----------------
> 1 file changed, 8 insertions(+), 16 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index ef363b0285..a62103b8ac 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -741,25 +741,21 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SYSCALL: /* System call exception */
> {
> int lev = env->error_code;
> -
> - if (lev == 1 && cpu->vhyp) {
> - dump_hcall(env);
> - } else {
> - dump_syscall(env);
> - }
> /*
> * The Virtual Open Firmware (VOF) relies on the 'sc 1'
> * instruction to communicate with QEMU. The pegasos2 machine
> * uses VOF and the 7xx CPUs, so although the 7xx don't have
> * HV mode, we need to keep hypercall support.
> */
> - if (lev == 1 && cpu->vhyp) {
> + if (unlikely(lev == 1 && cpu->vhyp)) {
> PPCVirtualHypervisorClass *vhc =
> PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> + dump_hcall(env);
> vhc->hypercall(cpu->vhyp, cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> @@ -885,25 +881,21 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
> case POWERPC_EXCP_SYSCALL: /* System call exception */
> {
> int lev = env->error_code;
> -
> - if (lev == 1 && cpu->vhyp) {
> - dump_hcall(env);
> - } else {
> - dump_syscall(env);
> - }
> /*
> * The Virtual Open Firmware (VOF) relies on the 'sc 1'
> * instruction to communicate with QEMU. The pegasos2 machine
> * uses VOF and the 74xx CPUs, so although the 74xx don't have
> * HV mode, we need to keep hypercall support.
> */
> - if (lev == 1 && cpu->vhyp) {
> + if (unlikely(lev == 1 && cpu->vhyp)) {
> PPCVirtualHypervisorClass *vhc =
> PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> + dump_hcall(env);
> vhc->hypercall(cpu->vhyp, cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> --
> 2.30.9
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 11/14] target/ppc: Get CPUState in one step
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (9 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 10/14] target/ppc: Simplify syscall exception handlers BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
` (3 subsequent siblings)
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
We can get CPUState from env with env_cpu without going through
PowerPCCPU and casting that.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Acked-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 15 +++++++--------
1 file changed, 7 insertions(+), 8 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a62103b8ac..cfb652445a 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1503,8 +1503,8 @@ static int p7_interrupt_powersave(CPUPPCState *env)
static int p7_next_unmasked_interrupt(CPUPPCState *env)
{
- PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = CPU(cpu);
+ CPUState *cs = env_cpu(env);
+
/* Ignore MSR[EE] when coming out of some power management states */
bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
@@ -1593,8 +1593,8 @@ static int p8_interrupt_powersave(CPUPPCState *env)
static int p8_next_unmasked_interrupt(CPUPPCState *env)
{
- PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = CPU(cpu);
+ CPUState *cs = env_cpu(env);
+
/* Ignore MSR[EE] when coming out of some power management states */
bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
@@ -1714,8 +1714,8 @@ static int p9_interrupt_powersave(CPUPPCState *env)
static int p9_next_unmasked_interrupt(CPUPPCState *env)
{
- PowerPCCPU *cpu = env_archcpu(env);
- CPUState *cs = CPU(cpu);
+ CPUState *cs = env_cpu(env);
+
/* Ignore MSR[EE] when coming out of some power management states */
bool msr_ee = FIELD_EX64(env->msr, MSR, EE) || env->resume_as_sreset;
@@ -2410,9 +2410,8 @@ void helper_scv(CPUPPCState *env, uint32_t lev)
void helper_pminsn(CPUPPCState *env, uint32_t insn)
{
- CPUState *cs;
+ CPUState *cs = env_cpu(env);
- cs = env_cpu(env);
cs->halted = 1;
/* Condition for waking up at 0x100 */
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (10 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 11/14] target/ppc: Get CPUState in one step BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-20 5:07 ` Nicholas Piggin
2023-06-15 23:03 ` [PATCH v3 13/14] target/ppc: Clean up ifdefs in excp_helper.c, part 2 BALATON Zoltan
` (2 subsequent siblings)
14 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Use #ifdef, #ifndef for brevity and add comments to #endif that are
more than a few lines apart for clarity.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 49 ++++++++++++++++++++--------------------
1 file changed, 24 insertions(+), 25 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index cfb652445a..858f657128 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -34,7 +34,7 @@
/*****************************************************************************/
/* Exception processing */
-#if !defined(CONFIG_USER_ONLY)
+#ifndef CONFIG_USER_ONLY
static const char *powerpc_excp_name(int excp)
{
@@ -165,7 +165,7 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
env->error_code);
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
static int powerpc_reset_wakeup(CPUPPCState *env, int excp, target_ulong *msr)
{
/* We no longer are in a PM state */
@@ -359,7 +359,7 @@ static void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp, target_ulong msr,
}
}
}
-#endif
+#endif /* TARGET_PPC64 */
static void powerpc_reset_excp_state(PowerPCCPU *cpu)
{
@@ -1100,7 +1100,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
break;
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
/* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
new_msr |= (target_ulong)1 << MSR_CM;
@@ -1419,7 +1419,7 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, int excp)
{
g_assert_not_reached();
}
-#endif
+#endif /* TARGET_PPC64 */
static void powerpc_excp(PowerPCCPU *cpu, int excp)
{
@@ -1470,7 +1470,7 @@ void ppc_cpu_do_interrupt(CPUState *cs)
powerpc_excp(cpu, cs->exception_index);
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
#define P7_UNUSED_INTERRUPTS \
(PPC_INTERRUPT_RESET | PPC_INTERRUPT_HVIRT | PPC_INTERRUPT_CEXT | \
PPC_INTERRUPT_WDT | PPC_INTERRUPT_CDOORBELL | PPC_INTERRUPT_FIT | \
@@ -1801,7 +1801,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
return 0;
}
-#endif
+#endif /* TARGET_PPC64 */
static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
{
@@ -1918,7 +1918,7 @@ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
static int ppc_next_unmasked_interrupt(CPUPPCState *env)
{
switch (env->excp_model) {
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
case POWERPC_EXCP_POWER7:
return p7_next_unmasked_interrupt(env);
case POWERPC_EXCP_POWER8:
@@ -1957,7 +1957,7 @@ void ppc_maybe_interrupt(CPUPPCState *env)
}
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
{
PowerPCCPU *cpu = env_archcpu(env);
@@ -2159,7 +2159,7 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
interrupt);
}
}
-#endif
+#endif /* TARGET_PPC64 */
static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
{
@@ -2268,7 +2268,7 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
{
switch (env->excp_model) {
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
case POWERPC_EXCP_POWER7:
p7_deliver_interrupt(env, interrupt);
break;
@@ -2378,9 +2378,9 @@ void helper_raise_exception(CPUPPCState *env, uint32_t exception)
{
raise_exception_err_ra(env, exception, 0, 0);
}
-#endif
+#endif /* CONFIG_TCG */
-#if !defined(CONFIG_USER_ONLY)
+#ifndef CONFIG_USER_ONLY
#ifdef CONFIG_TCG
void helper_store_msr(CPUPPCState *env, target_ulong val)
{
@@ -2397,7 +2397,7 @@ void helper_ppc_maybe_interrupt(CPUPPCState *env)
ppc_maybe_interrupt(env);
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
void helper_scv(CPUPPCState *env, uint32_t lev)
{
if (env->spr[SPR_FSCR] & (1ull << FSCR_SCV)) {
@@ -2420,7 +2420,7 @@ void helper_pminsn(CPUPPCState *env, uint32_t insn)
ppc_maybe_interrupt(env);
}
-#endif /* defined(TARGET_PPC64) */
+#endif /* TARGET_PPC64 */
static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
{
@@ -2431,7 +2431,7 @@ static void do_rfi(CPUPPCState *env, target_ulong nip, target_ulong msr)
if (env->flags & POWERPC_FLAG_TGPR)
msr &= ~(1ULL << MSR_TGPR);
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
/* Switching to 32-bit ? Crop the nip */
if (!msr_is_64bit(env, msr)) {
nip = (uint32_t)nip;
@@ -2460,7 +2460,7 @@ void helper_rfi(CPUPPCState *env)
do_rfi(env, env->spr[SPR_SRR0], env->spr[SPR_SRR1] & 0xfffffffful);
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
void helper_rfid(CPUPPCState *env)
{
/*
@@ -2481,7 +2481,7 @@ void helper_hrfid(CPUPPCState *env)
{
do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
}
-#endif
+#endif /* TARGET_PPC64 */
#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
void helper_rfebb(CPUPPCState *env, target_ulong s)
@@ -2558,7 +2558,7 @@ void raise_ebb_perfm_exception(CPUPPCState *env)
do_ebb(env, POWERPC_EXCP_PERFM_EBB);
}
-#endif
+#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
/*****************************************************************************/
/* Embedded PowerPC specific helpers */
@@ -2600,7 +2600,7 @@ void helper_tw(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
}
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
uint32_t flags)
{
@@ -2613,8 +2613,8 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
POWERPC_EXCP_TRAP, GETPC());
}
}
-#endif
-#endif
+#endif /* TARGET_PPC64 */
+#endif /* CONFIG_TCG */
#ifdef CONFIG_TCG
static uint32_t helper_SIMON_LIKE_32_64(uint32_t x, uint64_t key, uint32_t lane)
@@ -2729,8 +2729,7 @@ HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
#endif /* CONFIG_TCG */
-#if !defined(CONFIG_USER_ONLY)
-
+#ifndef CONFIG_USER_ONLY
#ifdef CONFIG_TCG
/* Embedded.Processor Control */
@@ -2839,7 +2838,7 @@ void helper_book3s_msgsnd(target_ulong rb)
book3s_msgsnd_common(pir, PPC_INTERRUPT_HDOORBELL);
}
-#if defined(TARGET_PPC64)
+#ifdef TARGET_PPC64
void helper_book3s_msgclrp(CPUPPCState *env, target_ulong rb)
{
helper_hfscr_facility_check(env, HFSCR_MSGP, "msgclrp", HFSCR_IC_MSGP);
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1
2023-06-15 23:03 ` [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
@ 2023-06-20 5:07 ` Nicholas Piggin
2023-06-20 10:52 ` BALATON Zoltan
0 siblings, 1 reply; 36+ messages in thread
From: Nicholas Piggin @ 2023-06-20 5:07 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza
On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
> Use #ifdef, #ifndef for brevity and add comments to #endif that are
> more than a few lines apart for clarity.
These will collide with the SOFTMMU ifdef changes that Philippe is
working on I think? They seem okay but maybe wait until after those
are merged?
Thanks,
Nick
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1
2023-06-20 5:07 ` Nicholas Piggin
@ 2023-06-20 10:52 ` BALATON Zoltan
0 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-20 10:52 UTC (permalink / raw)
To: Nicholas Piggin
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Daniel Henrique Barboza
On Tue, 20 Jun 2023, Nicholas Piggin wrote:
> On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:
>> Use #ifdef, #ifndef for brevity and add comments to #endif that are
>> more than a few lines apart for clarity.
>
> These will collide with the SOFTMMU ifdef changes that Philippe is
> working on I think? They seem okay but maybe wait until after those
> are merged?
Those were just merged and I did not get rebase conflicts. I
think Phillippe replaced SOFTMMU with USER_ONLY or so not touching
exsiring USER_ONLY stuff.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread
* [PATCH v3 13/14] target/ppc: Clean up ifdefs in excp_helper.c, part 2
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (11 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-15 23:03 ` [PATCH v3 14/14] target/ppc: Clean up ifdefs in excp_helper.c, part 3 BALATON Zoltan
2023-06-30 19:52 ` [PATCH v3 00/14] Misc clean ups to target/ppc exception handling Daniel Henrique Barboza
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Remove check for !defined(CONFIG_USER_ONLY) as this is already within
an #ifndef CONFIG_USER_ONLY block.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 858f657128..01e61464ab 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2483,7 +2483,7 @@ void helper_hrfid(CPUPPCState *env)
}
#endif /* TARGET_PPC64 */
-#if defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY)
+#ifdef TARGET_PPC64
void helper_rfebb(CPUPPCState *env, target_ulong s)
{
target_ulong msr = env->msr;
@@ -2558,7 +2558,7 @@ void raise_ebb_perfm_exception(CPUPPCState *env)
do_ebb(env, POWERPC_EXCP_PERFM_EBB);
}
-#endif /* defined(TARGET_PPC64) && !defined(CONFIG_USER_ONLY) */
+#endif /* TARGET_PPC64 */
/*****************************************************************************/
/* Embedded PowerPC specific helpers */
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* [PATCH v3 14/14] target/ppc: Clean up ifdefs in excp_helper.c, part 3
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (12 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 13/14] target/ppc: Clean up ifdefs in excp_helper.c, part 2 BALATON Zoltan
@ 2023-06-15 23:03 ` BALATON Zoltan
2023-06-30 19:52 ` [PATCH v3 00/14] Misc clean ups to target/ppc exception handling Daniel Henrique Barboza
14 siblings, 0 replies; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-15 23:03 UTC (permalink / raw)
To: qemu-devel, qemu-ppc
Cc: clg, Greg Kurz, Daniel Henrique Barboza, Nicholas Piggin
Concatenate #if blocks that are ending then beginning on the next line
again.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 01e61464ab..d909c61202 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2378,10 +2378,8 @@ void helper_raise_exception(CPUPPCState *env, uint32_t exception)
{
raise_exception_err_ra(env, exception, 0, 0);
}
-#endif /* CONFIG_TCG */
#ifndef CONFIG_USER_ONLY
-#ifdef CONFIG_TCG
void helper_store_msr(CPUPPCState *env, target_ulong val)
{
uint32_t excp = hreg_store_msr(env, val, 0);
@@ -2481,9 +2479,7 @@ void helper_hrfid(CPUPPCState *env)
{
do_rfi(env, env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
}
-#endif /* TARGET_PPC64 */
-#ifdef TARGET_PPC64
void helper_rfebb(CPUPPCState *env, target_ulong s)
{
target_ulong msr = env->msr;
@@ -2583,10 +2579,8 @@ void helper_rfmci(CPUPPCState *env)
/* FIXME: choose CSRR1 or MCSRR1 based on cpu type */
do_rfi(env, env->spr[SPR_BOOKE_MCSRR0], env->spr[SPR_BOOKE_MCSRR1]);
}
-#endif /* CONFIG_TCG */
-#endif /* !defined(CONFIG_USER_ONLY) */
+#endif /* !CONFIG_USER_ONLY */
-#ifdef CONFIG_TCG
void helper_tw(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
uint32_t flags)
{
@@ -2614,9 +2608,7 @@ void helper_td(CPUPPCState *env, target_ulong arg1, target_ulong arg2,
}
}
#endif /* TARGET_PPC64 */
-#endif /* CONFIG_TCG */
-#ifdef CONFIG_TCG
static uint32_t helper_SIMON_LIKE_32_64(uint32_t x, uint64_t key, uint32_t lane)
{
const uint16_t c = 0xfffc;
@@ -2727,11 +2719,8 @@ HELPER_HASH(HASHST, env->spr[SPR_HASHKEYR], true, NPHIE)
HELPER_HASH(HASHCHK, env->spr[SPR_HASHKEYR], false, NPHIE)
HELPER_HASH(HASHSTP, env->spr[SPR_HASHPKEYR], true, PHIE)
HELPER_HASH(HASHCHKP, env->spr[SPR_HASHPKEYR], false, PHIE)
-#endif /* CONFIG_TCG */
#ifndef CONFIG_USER_ONLY
-#ifdef CONFIG_TCG
-
/* Embedded.Processor Control */
static int dbell2irq(target_ulong rb)
{
@@ -2898,5 +2887,5 @@ void ppc_cpu_do_unaligned_access(CPUState *cs, vaddr vaddr,
env->error_code = insn & 0x03FF0000;
cpu_loop_exit(cs);
}
-#endif /* CONFIG_TCG */
#endif /* !CONFIG_USER_ONLY */
+#endif /* CONFIG_TCG */
--
2.30.9
^ permalink raw reply related [flat|nested] 36+ messages in thread* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-06-15 23:03 [PATCH v3 00/14] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (13 preceding siblings ...)
2023-06-15 23:03 ` [PATCH v3 14/14] target/ppc: Clean up ifdefs in excp_helper.c, part 3 BALATON Zoltan
@ 2023-06-30 19:52 ` Daniel Henrique Barboza
2023-06-30 22:57 ` BALATON Zoltan
14 siblings, 1 reply; 36+ messages in thread
From: Daniel Henrique Barboza @ 2023-06-30 19:52 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc; +Cc: clg, Greg Kurz, Nicholas Piggin
Zoltan,
Patches 1, 2, 3, 5, 7 and 11 are queued.
If you would be so kind to get the remaining patches, rebase them
on top of my ppc-next and resend, I believe there's more stuff
to be queued.
Thanks,
Daniel
On 6/15/23 20:03, BALATON Zoltan wrote:
> These are some small clean ups for target/ppc/excp_helper.c trying to
> make this code a bit simpler. No functional change is intended.
>
> v2: Patch 3 changes according to review, added tags
> v3: Address more review comments: don't change cpu_interrupt_exittb()
> parameter, add back lev, add scv patch from Nick + add some more
> patches to clean up #ifdefs
>
> Regards,
> BALATON Zoltan
>
> BALATON Zoltan (13):
> target/ppc: Remove some superfluous parentheses
> target/ppc: Remove unneeded parameter from powerpc_reset_wakeup()
> target/ppc: Move common check in exception handlers to a function
> target/ppc: Use env_cpu for cpu_abort in excp_helper
> target/ppc: Remove some more local CPUState variables only used once
> target/ppc: Readability improvements in exception handlers
> target/ppd: Remove unused define
> target/ppc: Fix gen_sc to use correct nip
> target/ppc: Simplify syscall exception handlers
> target/ppc: Get CPUState in one step
> target/ppc: Clean up ifdefs in excp_helper.c, part 1
> target/ppc: Clean up ifdefs in excp_helper.c, part 2
> target/ppc: Clean up ifdefs in excp_helper.c, part 3
>
> Nicholas Piggin (1):
> target/ppc: Move patching nip from exception handler to helper_scv
>
> target/ppc/cpu.h | 1 +
> target/ppc/excp_helper.c | 570 ++++++++++++---------------------------
> target/ppc/translate.c | 15 +-
> 3 files changed, 178 insertions(+), 408 deletions(-)
>
^ permalink raw reply [flat|nested] 36+ messages in thread* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-06-30 19:52 ` [PATCH v3 00/14] Misc clean ups to target/ppc exception handling Daniel Henrique Barboza
@ 2023-06-30 22:57 ` BALATON Zoltan
2023-07-01 6:39 ` Daniel Henrique Barboza
0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-06-30 22:57 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
> Patches 1, 2, 3, 5, 7 and 11 are queued.
>
> If you would be so kind to get the remaining patches, rebase them
> on top of my ppc-next and resend, I believe there's more stuff
> to be queued.
Thanks for taking care of these. I'll do the rebase of remaining patches
once the current queue is merged, they aren't urgent so I can come back to
those later. I'm working on some sam460ex patches but don't know yet when
can I send it so don't wait for me now.
Regards,
BALATON Zoltan
>
>
> Thanks,
>
>
> Daniel
>
> On 6/15/23 20:03, BALATON Zoltan wrote:
>> These are some small clean ups for target/ppc/excp_helper.c trying to
>> make this code a bit simpler. No functional change is intended.
>>
>> v2: Patch 3 changes according to review, added tags
>> v3: Address more review comments: don't change cpu_interrupt_exittb()
>> parameter, add back lev, add scv patch from Nick + add some more
>> patches to clean up #ifdefs
>>
>> Regards,
>> BALATON Zoltan
>>
>> BALATON Zoltan (13):
>> target/ppc: Remove some superfluous parentheses
>> target/ppc: Remove unneeded parameter from powerpc_reset_wakeup()
>> target/ppc: Move common check in exception handlers to a function
>> target/ppc: Use env_cpu for cpu_abort in excp_helper
>> target/ppc: Remove some more local CPUState variables only used once
>> target/ppc: Readability improvements in exception handlers
>> target/ppd: Remove unused define
>> target/ppc: Fix gen_sc to use correct nip
>> target/ppc: Simplify syscall exception handlers
>> target/ppc: Get CPUState in one step
>> target/ppc: Clean up ifdefs in excp_helper.c, part 1
>> target/ppc: Clean up ifdefs in excp_helper.c, part 2
>> target/ppc: Clean up ifdefs in excp_helper.c, part 3
>>
>> Nicholas Piggin (1):
>> target/ppc: Move patching nip from exception handler to helper_scv
>>
>> target/ppc/cpu.h | 1 +
>> target/ppc/excp_helper.c | 570 ++++++++++++---------------------------
>> target/ppc/translate.c | 15 +-
>> 3 files changed, 178 insertions(+), 408 deletions(-)
>>
>
>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-06-30 22:57 ` BALATON Zoltan
@ 2023-07-01 6:39 ` Daniel Henrique Barboza
2023-07-01 9:39 ` BALATON Zoltan
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-01 6:39 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On 6/30/23 19:57, BALATON Zoltan wrote:
> On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
>> Patches 1, 2, 3, 5, 7 and 11 are queued.
>>
>> If you would be so kind to get the remaining patches, rebase them
>> on top of my ppc-next and resend, I believe there's more stuff
>> to be queued.
>
> Thanks for taking care of these. I'll do the rebase of remaining patches once the current queue is merged, they aren't urgent so I can come back to those later. I'm working on some sam460ex patches but don't know yet when can I send it so don't wait for me now.
Got it. Just bear in mind the current release schedule. Code freeze is July 11th:
https://wiki.qemu.org/Planning/8.1
I'll send one last PR before freeze (probably on July 10th) and then it'll be only
bug fixes until end of August.
Thanks,
Daniel
>
> Regards,
> BALATON Zoltan
>
>>
>>
>> Thanks,
>>
>>
>> Daniel
>>
>> On 6/15/23 20:03, BALATON Zoltan wrote:
>>> These are some small clean ups for target/ppc/excp_helper.c trying to
>>> make this code a bit simpler. No functional change is intended.
>>>
>>> v2: Patch 3 changes according to review, added tags
>>> v3: Address more review comments: don't change cpu_interrupt_exittb()
>>> parameter, add back lev, add scv patch from Nick + add some more
>>> patches to clean up #ifdefs
>>>
>>> Regards,
>>> BALATON Zoltan
>>>
>>> BALATON Zoltan (13):
>>> target/ppc: Remove some superfluous parentheses
>>> target/ppc: Remove unneeded parameter from powerpc_reset_wakeup()
>>> target/ppc: Move common check in exception handlers to a function
>>> target/ppc: Use env_cpu for cpu_abort in excp_helper
>>> target/ppc: Remove some more local CPUState variables only used once
>>> target/ppc: Readability improvements in exception handlers
>>> target/ppd: Remove unused define
>>> target/ppc: Fix gen_sc to use correct nip
>>> target/ppc: Simplify syscall exception handlers
>>> target/ppc: Get CPUState in one step
>>> target/ppc: Clean up ifdefs in excp_helper.c, part 1
>>> target/ppc: Clean up ifdefs in excp_helper.c, part 2
>>> target/ppc: Clean up ifdefs in excp_helper.c, part 3
>>>
>>> Nicholas Piggin (1):
>>> target/ppc: Move patching nip from exception handler to helper_scv
>>>
>>> target/ppc/cpu.h | 1 +
>>> target/ppc/excp_helper.c | 570 ++++++++++++---------------------------
>>> target/ppc/translate.c | 15 +-
>>> 3 files changed, 178 insertions(+), 408 deletions(-)
>>>
>>
>>
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-07-01 6:39 ` Daniel Henrique Barboza
@ 2023-07-01 9:39 ` BALATON Zoltan
2023-07-01 9:59 ` Daniel Henrique Barboza
0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-07-01 9:39 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
> On 6/30/23 19:57, BALATON Zoltan wrote:
>> On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
>>> Patches 1, 2, 3, 5, 7 and 11 are queued.
>>>
>>> If you would be so kind to get the remaining patches, rebase them
>>> on top of my ppc-next and resend, I believe there's more stuff
>>> to be queued.
>>
>> Thanks for taking care of these. I'll do the rebase of remaining patches
>> once the current queue is merged, they aren't urgent so I can come back to
>> those later. I'm working on some sam460ex patches but don't know yet when
>> can I send it so don't wait for me now.
>
> Got it. Just bear in mind the current release schedule. Code freeze is July
> 11th:
>
> https://wiki.qemu.org/Planning/8.1
>
> I'll send one last PR before freeze (probably on July 10th) and then it'll be
> only bug fixes until end of August.
Do you mean one more last PR after merging the current queue or the
current queue will only be in that last PR? I hoped there would be a PR
now on which I can rebase the outstanding patches for the last PR so I
don't have to rebase on next but if the only PR you plan is the last on
10th then I may need to move to ppc-next now.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-07-01 9:39 ` BALATON Zoltan
@ 2023-07-01 9:59 ` Daniel Henrique Barboza
2023-07-02 12:36 ` BALATON Zoltan
0 siblings, 1 reply; 36+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-01 9:59 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On 7/1/23 06:39, BALATON Zoltan wrote:
> On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
>> On 6/30/23 19:57, BALATON Zoltan wrote:
>>> On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
>>>> Patches 1, 2, 3, 5, 7 and 11 are queued.
>>>>
>>>> If you would be so kind to get the remaining patches, rebase them
>>>> on top of my ppc-next and resend, I believe there's more stuff
>>>> to be queued.
>>>
>>> Thanks for taking care of these. I'll do the rebase of remaining patches once the current queue is merged, they aren't urgent so I can come back to those later. I'm working on some sam460ex patches but don't know yet when can I send it so don't wait for me now.
>>
>> Got it. Just bear in mind the current release schedule. Code freeze is July 11th:
>>
>> https://wiki.qemu.org/Planning/8.1
>>
>> I'll send one last PR before freeze (probably on July 10th) and then it'll be only bug fixes until end of August.
>
> Do you mean one more last PR after merging the current queue or the current queue will only be in that last PR? I hoped there would be a PR now on which I can rebase the outstanding patches for the last PR so I don't have to rebase on next but if the only PR you plan is the last on 10th then I may need to move to ppc-next now.
Just use ppc-next right now.
Even if I send a PR today with what we have, Peter/Richard has no obligation of
merging it quickly on Monday (there's an US holiday July 4th, and some people will
also skip July 3rd). If you wait for such PR to merge upstream, then start rebasing
your stuff, you'll have less time to work with.
Daniel
>
> Regards,
> BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-07-01 9:59 ` Daniel Henrique Barboza
@ 2023-07-02 12:36 ` BALATON Zoltan
2023-07-03 10:09 ` Daniel Henrique Barboza
0 siblings, 1 reply; 36+ messages in thread
From: BALATON Zoltan @ 2023-07-02 12:36 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
> On 7/1/23 06:39, BALATON Zoltan wrote:
>> On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
>>> On 6/30/23 19:57, BALATON Zoltan wrote:
>>>> On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
>>>>> Patches 1, 2, 3, 5, 7 and 11 are queued.
>>>>>
>>>>> If you would be so kind to get the remaining patches, rebase them
>>>>> on top of my ppc-next and resend, I believe there's more stuff
>>>>> to be queued.
>>>>
>>>> Thanks for taking care of these. I'll do the rebase of remaining patches
>>>> once the current queue is merged, they aren't urgent so I can come back
>>>> to those later. I'm working on some sam460ex patches but don't know yet
>>>> when can I send it so don't wait for me now.
>>>
>>> Got it. Just bear in mind the current release schedule. Code freeze is
>>> July 11th:
>>>
>>> https://wiki.qemu.org/Planning/8.1
>>>
>>> I'll send one last PR before freeze (probably on July 10th) and then it'll
>>> be only bug fixes until end of August.
>>
>> Do you mean one more last PR after merging the current queue or the current
>> queue will only be in that last PR? I hoped there would be a PR now on
>> which I can rebase the outstanding patches for the last PR so I don't have
>> to rebase on next but if the only PR you plan is the last on 10th then I
>> may need to move to ppc-next now.
>
> Just use ppc-next right now.
>
> Even if I send a PR today with what we have, Peter/Richard has no
> obligation of merging it quickly on Monday (there's an US holiday July
> 4th, and some people will also skip July 3rd). If you wait for such PR
> to merge upstream, then start rebasing your stuff, you'll have less time
> to work with.
They may not be in the US though. Anyway, I've tried to rebase the
remaining patches on your ppc-next branch and they applied without changes
so not sure what you need from me now. I think Nick had alternative
versions of the checkstop and sc patches that may cause a rebase when then
go in first. Do you want me to merge them and submit that as a series with
my patches? I don't have much time for now and these are just clean ups so
I can live with these missing the next release and can come back to it
later. I may try to do some sam460ex clean ups instead that should not
clash with anything else.
Regards,
BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread
* Re: [PATCH v3 00/14] Misc clean ups to target/ppc exception handling
2023-07-02 12:36 ` BALATON Zoltan
@ 2023-07-03 10:09 ` Daniel Henrique Barboza
0 siblings, 0 replies; 36+ messages in thread
From: Daniel Henrique Barboza @ 2023-07-03 10:09 UTC (permalink / raw)
To: BALATON Zoltan; +Cc: qemu-devel, qemu-ppc, clg, Greg Kurz, Nicholas Piggin
On 7/2/23 09:36, BALATON Zoltan wrote:
> On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
>> On 7/1/23 06:39, BALATON Zoltan wrote:
>>> On Sat, 1 Jul 2023, Daniel Henrique Barboza wrote:
>>>> On 6/30/23 19:57, BALATON Zoltan wrote:
>>>>> On Fri, 30 Jun 2023, Daniel Henrique Barboza wrote:
>>>>>> Patches 1, 2, 3, 5, 7 and 11 are queued.
>>>>>>
>>>>>> If you would be so kind to get the remaining patches, rebase them
>>>>>> on top of my ppc-next and resend, I believe there's more stuff
>>>>>> to be queued.
>>>>>
>>>>> Thanks for taking care of these. I'll do the rebase of remaining patches once the current queue is merged, they aren't urgent so I can come back to those later. I'm working on some sam460ex patches but don't know yet when can I send it so don't wait for me now.
>>>>
>>>> Got it. Just bear in mind the current release schedule. Code freeze is July 11th:
>>>>
>>>> https://wiki.qemu.org/Planning/8.1
>>>>
>>>> I'll send one last PR before freeze (probably on July 10th) and then it'll be only bug fixes until end of August.
>>>
>>> Do you mean one more last PR after merging the current queue or the current queue will only be in that last PR? I hoped there would be a PR now on which I can rebase the outstanding patches for the last PR so I don't have to rebase on next but if the only PR you plan is the last on 10th then I may need to move to ppc-next now.
>>
>> Just use ppc-next right now.
>>
>> Even if I send a PR today with what we have, Peter/Richard has no obligation of merging it quickly on Monday (there's an US holiday July 4th, and some people will also skip July 3rd). If you wait for such PR to merge upstream, then start rebasing your stuff, you'll have less time to work with.
>
> They may not be in the US though. Anyway, I've tried to rebase the remaining patches on your ppc-next branch and they applied without changes so not sure what you need from me now. I think Nick had alternative versions of the checkstop and sc patches that may cause a rebase when then go in first. Do you want me to merge them and submit that as a series with my patches? I don't have much time for now and these are just clean ups so I can live with these missing the next release and can come back to it later. I may try to do some sam460ex clean ups instead that should not clash with anything else.
I'm fine with leaving some of these cleanups for the next release. I might review
some patches by myself and queueing them during this week.
Daniel
>
> Regards,
> BALATON Zoltan
^ permalink raw reply [flat|nested] 36+ messages in thread