* [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-21 9:10 ` Harsh Prateek Bora
2024-01-18 22:01 ` [PATCH v5 2/9] target/ppc: Readability improvements in exception handlers BALATON Zoltan
` (8 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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 2ec6429e36..b8fd01d04c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -445,7 +445,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;
@@ -473,8 +472,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;
@@ -523,7 +522,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;
}
@@ -550,11 +549,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;
}
@@ -569,7 +569,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;
@@ -592,8 +591,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;
@@ -653,7 +652,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;
}
@@ -675,8 +674,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 */
@@ -703,11 +703,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;
}
@@ -730,7 +731,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;
@@ -753,8 +753,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;
@@ -812,7 +812,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;
}
@@ -854,8 +854,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 */
@@ -875,11 +876,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;
}
@@ -902,7 +904,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;
@@ -925,8 +926,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;
@@ -984,7 +985,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;
}
@@ -1026,7 +1027,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;
@@ -1039,11 +1041,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;
}
@@ -1066,7 +1069,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;
@@ -1103,8 +1105,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;
@@ -1135,6 +1137,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);
}
@@ -1173,7 +1176,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;
}
@@ -1214,7 +1217,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 */
@@ -1228,17 +1232,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;
}
@@ -1367,7 +1373,6 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
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;
@@ -1406,8 +1411,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;
@@ -1503,7 +1508,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;
}
@@ -1569,7 +1574,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);
}
}
@@ -1641,11 +1647,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;
}
@@ -1678,8 +1685,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 */
@@ -1697,11 +1704,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
@@ -2235,7 +2242,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 */
@@ -2279,14 +2285,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 */
@@ -2350,7 +2356,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);
}
}
@@ -2429,7 +2436,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
@@ -2437,7 +2445,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 */
@@ -2534,7 +2541,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] 21+ messages in thread* Re: [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper
2024-01-18 22:01 ` [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
@ 2024-02-21 9:10 ` Harsh Prateek Bora
0 siblings, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-21 9:10 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, 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.
>
I wish the patch could have broader scope to cover whole file and not
just cpu_abort, however, this is good to begin with.
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> 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 2ec6429e36..b8fd01d04c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -445,7 +445,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;
> @@ -473,8 +472,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;
> @@ -523,7 +522,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;
> }
> @@ -550,11 +549,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;
> }
>
> @@ -569,7 +569,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;
>
> @@ -592,8 +591,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;
> @@ -653,7 +652,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;
> }
> @@ -675,8 +674,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 */
> @@ -703,11 +703,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;
> }
>
> @@ -730,7 +731,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;
>
> @@ -753,8 +753,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;
> @@ -812,7 +812,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;
> }
> @@ -854,8 +854,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 */
> @@ -875,11 +876,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;
> }
>
> @@ -902,7 +904,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;
>
> @@ -925,8 +926,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;
> @@ -984,7 +985,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;
> }
> @@ -1026,7 +1027,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;
> @@ -1039,11 +1041,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;
> }
>
> @@ -1066,7 +1069,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;
> @@ -1103,8 +1105,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;
> @@ -1135,6 +1137,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);
> }
> @@ -1173,7 +1176,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;
> }
> @@ -1214,7 +1217,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 */
> @@ -1228,17 +1232,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;
> }
>
> @@ -1367,7 +1373,6 @@ static bool is_prefix_insn_excp(PowerPCCPU *cpu, int excp)
>
> 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;
> @@ -1406,8 +1411,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;
> @@ -1503,7 +1508,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;
> }
> @@ -1569,7 +1574,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);
> }
> }
> @@ -1641,11 +1647,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;
> }
>
> @@ -1678,8 +1685,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 */
> @@ -1697,11 +1704,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
> @@ -2235,7 +2242,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 */
> @@ -2279,14 +2285,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 */
> @@ -2350,7 +2356,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);
> }
> }
>
> @@ -2429,7 +2436,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
> @@ -2437,7 +2445,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 */
> @@ -2534,7 +2541,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);
> }
> }
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 2/9] target/ppc: Readability improvements in exception handlers
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 3/9] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
` (7 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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 | 179 +++++++--------------------------------
2 files changed, 33 insertions(+), 147 deletions(-)
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index f8101ffa29..2f9b610abc 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2902,6 +2902,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 b8fd01d04c..39eefc168a 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -403,9 +403,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;
@@ -447,25 +446,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 overridden.
- */
+ /* 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;
}
@@ -475,7 +464,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) {
@@ -487,7 +475,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;
@@ -558,12 +545,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);
}
@@ -575,16 +558,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 overridden
- */
+ /* 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;
}
@@ -594,7 +571,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) {
@@ -604,7 +580,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]);
@@ -632,11 +607,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;
@@ -712,20 +685,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);
}
@@ -737,16 +701,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 overridden
- */
+ /* 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;
}
@@ -756,7 +714,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) {
@@ -764,7 +721,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]);
@@ -792,11 +748,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;
@@ -865,12 +819,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 */
@@ -885,20 +837,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);
}
@@ -910,16 +853,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 overridden
- */
+ /* 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;
}
@@ -929,7 +866,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) {
@@ -937,7 +873,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]);
@@ -965,11 +900,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;
@@ -1050,20 +983,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);
}
@@ -1071,24 +995,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 overridden
+ * 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;
}
@@ -1108,7 +1026,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) {
@@ -1152,11 +1069,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;
@@ -1257,12 +1172,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);
}
@@ -1375,21 +1286,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 overridden
+ * 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
@@ -1414,7 +1321,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;
if (is_prefix_insn_excp(cpu, excp)) {
@@ -1431,7 +1337,6 @@ 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);
@@ -1449,23 +1354,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 */
@@ -1488,11 +1387,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;
@@ -1656,21 +1553,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;
}
@@ -1679,19 +1568,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] 21+ messages in thread* [PATCH v5 3/9] target/ppc: Fix gen_sc to use correct nip
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 2/9] target/ppc: Readability improvements in exception handlers BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 4/9] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
` (6 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
---
target/ppc/excp_helper.c | 43 ++--------------------------------------
target/ppc/translate.c | 10 ++++++----
2 files changed, 8 insertions(+), 45 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 39eefc168a..1c07a11405 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -116,7 +116,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)
@@ -131,7 +131,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
@@ -516,12 +516,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");
@@ -632,12 +626,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 */
@@ -780,13 +768,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
@@ -932,13 +913,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
@@ -1098,12 +1072,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 */
@@ -1418,13 +1386,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 329da4d518..a80d24143e 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4535,15 +4535,17 @@ static void gen_hrfid(DisasContext *ctx)
#endif
static void gen_sc(DisasContext *ctx)
{
- uint32_t lev;
-
/*
* LEV is a 7-bit field, but the top 6 bits are treated as a reserved
* field (i.e., ignored). ISA v3.1 changes that to 5 bits, but that is
* for Ultravisor which TCG does not support, so just ignore the top 6.
*/
- lev = (ctx->opcode >> 5) & 0x1;
- gen_exception_err(ctx, POWERPC_SYSCALL, lev);
+ uint32_t lev = (ctx->opcode >> 5) & 0x1;
+
+ gen_update_nip(ctx, ctx->base.pc_next);
+ gen_helper_raise_exception_err(tcg_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] 21+ messages in thread* [PATCH v5 4/9] target/ppc: Move patching nip from exception handler to helper_scv
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (2 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 3/9] target/ppc: Fix gen_sc to use correct nip BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-01-18 22:01 ` [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers BALATON Zoltan
` (5 subsequent siblings)
9 siblings, 0 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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>
---
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 1c07a11405..411d67376c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1405,7 +1405,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);
@@ -2528,6 +2527,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 a80d24143e..d8cd34721c 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -4554,7 +4554,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(tcg_env, tcg_constant_i32(lev));
--
2.30.9
^ permalink raw reply related [flat|nested] 21+ messages in thread* [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (3 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 4/9] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-21 9:20 ` Harsh Prateek Bora
2024-02-21 9:42 ` Harsh Prateek Bora
2024-01-18 22:01 ` [PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
` (4 subsequent siblings)
9 siblings, 2 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
After previous changes the hypercall handling in 7xx and 74xx
exception handlers can be folded into one if statement to simpilfy
this code. Also add "unlikely" to mark the less freqiently used branch
for the compiler.
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 411d67376c..035a9fd968 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -762,26 +762,22 @@ 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);
powerpc_reset_excp_state(cpu);
return;
+ } else {
+ dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
@@ -907,26 +903,22 @@ 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);
powerpc_reset_excp_state(cpu);
return;
+ } else {
+ dump_syscall(env);
}
-
break;
}
case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
--
2.30.9
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers
2024-01-18 22:01 ` [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers BALATON Zoltan
@ 2024-02-21 9:20 ` Harsh Prateek Bora
2024-02-21 9:42 ` Harsh Prateek Bora
1 sibling, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-21 9:20 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, 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. Also add "unlikely" to mark the less freqiently used branch
> for the compiler.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> 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 411d67376c..035a9fd968 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -762,26 +762,22 @@ 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);
> powerpc_reset_excp_state(cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> @@ -907,26 +903,22 @@ 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);
> powerpc_reset_excp_state(cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers
2024-01-18 22:01 ` [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers BALATON Zoltan
2024-02-21 9:20 ` Harsh Prateek Bora
@ 2024-02-21 9:42 ` Harsh Prateek Bora
1 sibling, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-21 9:42 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, 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. Also add "unlikely" to mark the less freqiently used branch
> for the compiler.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Nice cleanup.
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> 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 411d67376c..035a9fd968 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -762,26 +762,22 @@ 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);
> powerpc_reset_excp_state(cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
> @@ -907,26 +903,22 @@ 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);
> powerpc_reset_excp_state(cpu);
> return;
> + } else {
> + dump_syscall(env);
> }
> -
> break;
> }
> case POWERPC_EXCP_FPU: /* Floating-point unavailable exception */
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (4 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 5/9] target/ppc: Simplify syscall exception handlers BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-22 9:31 ` Harsh Prateek Bora
2024-01-18 22:01 ` [PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2 BALATON Zoltan
` (3 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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 035a9fd968..d8eab4ff6c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -35,7 +35,7 @@
/*****************************************************************************/
/* Exception processing */
-#if !defined(CONFIG_USER_ONLY)
+#ifndef CONFIG_USER_ONLY
static const char *powerpc_excp_name(int excp)
{
@@ -186,7 +186,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 */
@@ -380,7 +380,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)
{
@@ -1123,7 +1123,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;
@@ -1537,7 +1537,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)
{
@@ -1588,7 +1588,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 | \
@@ -1919,7 +1919,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
return 0;
}
-#endif
+#endif /* TARGET_PPC64 */
static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
{
@@ -2036,7 +2036,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:
@@ -2075,7 +2075,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);
@@ -2277,7 +2277,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)
{
@@ -2386,7 +2386,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;
@@ -2496,9 +2496,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)
{
@@ -2515,7 +2515,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)) {
@@ -2544,7 +2544,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)
{
@@ -2555,7 +2555,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;
@@ -2584,7 +2584,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)
{
/*
@@ -2605,7 +2605,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)
@@ -2682,7 +2682,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 */
@@ -2724,7 +2724,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)
{
@@ -2737,8 +2737,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)
@@ -2853,8 +2853,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 */
@@ -2963,7 +2962,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] 21+ messages in thread* Re: [PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1
2024-01-18 22:01 ` [PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
@ 2024-02-22 9:31 ` Harsh Prateek Bora
0 siblings, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-22 9:31 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, BALATON Zoltan wrote:
> 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>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> 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 035a9fd968..d8eab4ff6c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -35,7 +35,7 @@
>
> /*****************************************************************************/
> /* Exception processing */
> -#if !defined(CONFIG_USER_ONLY)
> +#ifndef CONFIG_USER_ONLY
>
> static const char *powerpc_excp_name(int excp)
> {
> @@ -186,7 +186,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 */
> @@ -380,7 +380,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)
> {
> @@ -1123,7 +1123,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;
> @@ -1537,7 +1537,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)
> {
> @@ -1588,7 +1588,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 | \
> @@ -1919,7 +1919,7 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
>
> return 0;
> }
> -#endif
> +#endif /* TARGET_PPC64 */
>
> static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
> {
> @@ -2036,7 +2036,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:
> @@ -2075,7 +2075,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);
> @@ -2277,7 +2277,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)
> {
> @@ -2386,7 +2386,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;
> @@ -2496,9 +2496,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)
> {
> @@ -2515,7 +2515,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)) {
> @@ -2544,7 +2544,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)
> {
> @@ -2555,7 +2555,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;
> @@ -2584,7 +2584,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)
> {
> /*
> @@ -2605,7 +2605,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)
> @@ -2682,7 +2682,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 */
> @@ -2724,7 +2724,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)
> {
> @@ -2737,8 +2737,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)
> @@ -2853,8 +2853,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 */
> @@ -2963,7 +2962,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);
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (5 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 6/9] target/ppc: Clean up ifdefs in excp_helper.c, part 1 BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-22 9:34 ` Harsh Prateek Bora
2024-01-18 22:01 ` [PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3 BALATON Zoltan
` (2 subsequent siblings)
9 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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 d8eab4ff6c..2d4a72883f 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2607,7 +2607,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;
@@ -2682,7 +2682,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] 21+ messages in thread* Re: [PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2
2024-01-18 22:01 ` [PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2 BALATON Zoltan
@ 2024-02-22 9:34 ` Harsh Prateek Bora
0 siblings, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-22 9:34 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, BALATON Zoltan wrote:
> 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>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> 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 d8eab4ff6c..2d4a72883f 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2607,7 +2607,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;
> @@ -2682,7 +2682,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 */
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (6 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 7/9] target/ppc: Clean up ifdefs in excp_helper.c, part 2 BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-22 9:38 ` Harsh Prateek Bora
2024-01-18 22:01 ` [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions BALATON Zoltan
2024-01-30 21:08 ` [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
9 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
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 2d4a72883f..5124c3e6b5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -2496,10 +2496,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);
@@ -2605,9 +2603,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;
@@ -2707,10 +2703,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)
{
@@ -2738,9 +2732,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;
@@ -2851,11 +2843,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)
{
@@ -3197,5 +3186,5 @@ bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
return false;
}
-#endif /* CONFIG_TCG */
#endif /* !CONFIG_USER_ONLY */
+#endif /* CONFIG_TCG */
--
2.30.9
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3
2024-01-18 22:01 ` [PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3 BALATON Zoltan
@ 2024-02-22 9:38 ` Harsh Prateek Bora
0 siblings, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-22 9:38 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, BALATON Zoltan wrote:
> Concatenate #if blocks that are ending then beginning on the next line
> again.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> ---
> 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 2d4a72883f..5124c3e6b5 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -2496,10 +2496,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);
> @@ -2605,9 +2603,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;
> @@ -2707,10 +2703,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)
> {
> @@ -2738,9 +2732,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;
> @@ -2851,11 +2843,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)
> {
> @@ -3197,5 +3186,5 @@ bool ppc_cpu_debug_check_watchpoint(CPUState *cs, CPUWatchpoint *wp)
> return false;
> }
>
> -#endif /* CONFIG_TCG */
> #endif /* !CONFIG_USER_ONLY */
> +#endif /* CONFIG_TCG */
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (7 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 8/9] target/ppc: Clean up ifdefs in excp_helper.c, part 3 BALATON Zoltan
@ 2024-01-18 22:01 ` BALATON Zoltan
2024-02-22 10:05 ` Harsh Prateek Bora
2024-02-22 10:06 ` Harsh Prateek Bora
2024-01-30 21:08 ` [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
9 siblings, 2 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-18 22:01 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
These wrappers call out to handle POWER7 and newer in separate
functions but reduce to the generic case when TARGET_PPC64 is not
defined. It is easy enough to include the switch in the beginning of
the generic functions to branch out to the specific functions and get
rid of these wrappers. This avoids one indirection and entitely
compiles out the switch without TARGET_PPC64.
Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
target/ppc/excp_helper.c | 70 ++++++++++++++++++----------------------
1 file changed, 31 insertions(+), 39 deletions(-)
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 5124c3e6b5..de51627c4c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1921,8 +1921,21 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
}
#endif /* TARGET_PPC64 */
-static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
+static int ppc_next_unmasked_interrupt(CPUPPCState *env)
{
+#ifdef TARGET_PPC64
+ switch (env->excp_model) {
+ case POWERPC_EXCP_POWER7:
+ return p7_next_unmasked_interrupt(env);
+ case POWERPC_EXCP_POWER8:
+ return p8_next_unmasked_interrupt(env);
+ case POWERPC_EXCP_POWER9:
+ case POWERPC_EXCP_POWER10:
+ return p9_next_unmasked_interrupt(env);
+ default:
+ break;
+ }
+#endif
bool async_deliver;
/* External reset */
@@ -2033,23 +2046,6 @@ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
return 0;
}
-static int ppc_next_unmasked_interrupt(CPUPPCState *env)
-{
- switch (env->excp_model) {
-#ifdef TARGET_PPC64
- case POWERPC_EXCP_POWER7:
- return p7_next_unmasked_interrupt(env);
- case POWERPC_EXCP_POWER8:
- return p8_next_unmasked_interrupt(env);
- case POWERPC_EXCP_POWER9:
- case POWERPC_EXCP_POWER10:
- return p9_next_unmasked_interrupt(env);
-#endif
- default:
- return ppc_next_unmasked_interrupt_generic(env);
- }
-}
-
/*
* Sets CPU_INTERRUPT_HARD if there is at least one unmasked interrupt to be
* delivered and clears CPU_INTERRUPT_HARD otherwise.
@@ -2279,8 +2275,24 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
}
#endif /* TARGET_PPC64 */
-static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
+static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
{
+#ifdef TARGET_PPC64
+ switch (env->excp_model) {
+ case POWERPC_EXCP_POWER7:
+ p7_deliver_interrupt(env, interrupt);
+ return;
+ case POWERPC_EXCP_POWER8:
+ p8_deliver_interrupt(env, interrupt);
+ return;
+ case POWERPC_EXCP_POWER9:
+ case POWERPC_EXCP_POWER10:
+ p9_deliver_interrupt(env, interrupt);
+ return;
+ default:
+ break;
+ }
+#endif
PowerPCCPU *cpu = env_archcpu(env);
switch (interrupt) {
@@ -2383,26 +2395,6 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
}
}
-static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
-{
- switch (env->excp_model) {
-#ifdef TARGET_PPC64
- case POWERPC_EXCP_POWER7:
- p7_deliver_interrupt(env, interrupt);
- break;
- case POWERPC_EXCP_POWER8:
- p8_deliver_interrupt(env, interrupt);
- break;
- case POWERPC_EXCP_POWER9:
- case POWERPC_EXCP_POWER10:
- p9_deliver_interrupt(env, interrupt);
- break;
-#endif
- default:
- ppc_deliver_interrupt_generic(env, interrupt);
- }
-}
-
void ppc_cpu_do_system_reset(CPUState *cs)
{
PowerPCCPU *cpu = POWERPC_CPU(cs);
--
2.30.9
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions
2024-01-18 22:01 ` [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions BALATON Zoltan
@ 2024-02-22 10:05 ` Harsh Prateek Bora
2024-02-22 10:06 ` Harsh Prateek Bora
1 sibling, 0 replies; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-22 10:05 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, BALATON Zoltan wrote:
> These wrappers call out to handle POWER7 and newer in separate
> functions but reduce to the generic case when TARGET_PPC64 is not
> defined. It is easy enough to include the switch in the beginning of
> the generic functions to branch out to the specific functions and get
> rid of these wrappers. This avoids one indirection and entitely
s/entitely/entirely
> compiles out the switch without TARGET_PPC64.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> target/ppc/excp_helper.c | 70 ++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 5124c3e6b5..de51627c4c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1921,8 +1921,21 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
> #endif /* TARGET_PPC64 */
>
> -static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
> +static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> {
> +#ifdef TARGET_PPC64
> + switch (env->excp_model) {
> + case POWERPC_EXCP_POWER7:
> + return p7_next_unmasked_interrupt(env);
> + case POWERPC_EXCP_POWER8:
> + return p8_next_unmasked_interrupt(env);
> + case POWERPC_EXCP_POWER9:
> + case POWERPC_EXCP_POWER10:
> + return p9_next_unmasked_interrupt(env);
> + default:
> + break;
> + }
> +#endif
> bool async_deliver;
>
> /* External reset */
> @@ -2033,23 +2046,6 @@ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
> return 0;
> }
>
> -static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> -{
> - switch (env->excp_model) {
> -#ifdef TARGET_PPC64
> - case POWERPC_EXCP_POWER7:
> - return p7_next_unmasked_interrupt(env);
> - case POWERPC_EXCP_POWER8:
> - return p8_next_unmasked_interrupt(env);
> - case POWERPC_EXCP_POWER9:
> - case POWERPC_EXCP_POWER10:
> - return p9_next_unmasked_interrupt(env);
> -#endif
> - default:
> - return ppc_next_unmasked_interrupt_generic(env);
> - }
> -}
> -
> /*
> * Sets CPU_INTERRUPT_HARD if there is at least one unmasked interrupt to be
> * delivered and clears CPU_INTERRUPT_HARD otherwise.
> @@ -2279,8 +2275,24 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
> }
> #endif /* TARGET_PPC64 */
>
> -static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> +static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
> {
> +#ifdef TARGET_PPC64
> + switch (env->excp_model) {
> + case POWERPC_EXCP_POWER7:
> + p7_deliver_interrupt(env, interrupt);
> + return;
> + case POWERPC_EXCP_POWER8:
> + p8_deliver_interrupt(env, interrupt);
> + return;
> + case POWERPC_EXCP_POWER9:
> + case POWERPC_EXCP_POWER10:
> + p9_deliver_interrupt(env, interrupt);
> + return;
These return statements could be clubbed with the function call itself.
With the suggested fixes,
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> + default:
> + break;
> + }
> +#endif
> PowerPCCPU *cpu = env_archcpu(env);
>
> switch (interrupt) {
> @@ -2383,26 +2395,6 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> }
> }
>
> -static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
> -{
> - switch (env->excp_model) {
> -#ifdef TARGET_PPC64
> - case POWERPC_EXCP_POWER7:
> - p7_deliver_interrupt(env, interrupt);
> - break;
> - case POWERPC_EXCP_POWER8:
> - p8_deliver_interrupt(env, interrupt);
> - break;
> - case POWERPC_EXCP_POWER9:
> - case POWERPC_EXCP_POWER10:
> - p9_deliver_interrupt(env, interrupt);
> - break;
> -#endif
> - default:
> - ppc_deliver_interrupt_generic(env, interrupt);
> - }
> -}
> -
> void ppc_cpu_do_system_reset(CPUState *cs)
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions
2024-01-18 22:01 ` [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions BALATON Zoltan
2024-02-22 10:05 ` Harsh Prateek Bora
@ 2024-02-22 10:06 ` Harsh Prateek Bora
2024-02-22 11:15 ` BALATON Zoltan
1 sibling, 1 reply; 21+ messages in thread
From: Harsh Prateek Bora @ 2024-02-22 10:06 UTC (permalink / raw)
To: BALATON Zoltan, qemu-devel, qemu-ppc
Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On 1/19/24 03:31, BALATON Zoltan wrote:
> These wrappers call out to handle POWER7 and newer in separate
> functions but reduce to the generic case when TARGET_PPC64 is not
> defined. It is easy enough to include the switch in the beginning of
> the generic functions to branch out to the specific functions and get
> rid of these wrappers. This avoids one indirection and entitely
s/entitely/entirely
> compiles out the switch without TARGET_PPC64.
>
> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
> ---
> target/ppc/excp_helper.c | 70 ++++++++++++++++++----------------------
> 1 file changed, 31 insertions(+), 39 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 5124c3e6b5..de51627c4c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1921,8 +1921,21 @@ static int p9_next_unmasked_interrupt(CPUPPCState *env)
> }
> #endif /* TARGET_PPC64 */
>
> -static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
> +static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> {
> +#ifdef TARGET_PPC64
> + switch (env->excp_model) {
> + case POWERPC_EXCP_POWER7:
> + return p7_next_unmasked_interrupt(env);
> + case POWERPC_EXCP_POWER8:
> + return p8_next_unmasked_interrupt(env);
> + case POWERPC_EXCP_POWER9:
> + case POWERPC_EXCP_POWER10:
> + return p9_next_unmasked_interrupt(env);
> + default:
> + break;
> + }
> +#endif
> bool async_deliver;
>
> /* External reset */
> @@ -2033,23 +2046,6 @@ static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
> return 0;
> }
>
> -static int ppc_next_unmasked_interrupt(CPUPPCState *env)
> -{
> - switch (env->excp_model) {
> -#ifdef TARGET_PPC64
> - case POWERPC_EXCP_POWER7:
> - return p7_next_unmasked_interrupt(env);
> - case POWERPC_EXCP_POWER8:
> - return p8_next_unmasked_interrupt(env);
> - case POWERPC_EXCP_POWER9:
> - case POWERPC_EXCP_POWER10:
> - return p9_next_unmasked_interrupt(env);
> -#endif
> - default:
> - return ppc_next_unmasked_interrupt_generic(env);
> - }
> -}
> -
> /*
> * Sets CPU_INTERRUPT_HARD if there is at least one unmasked interrupt to be
> * delivered and clears CPU_INTERRUPT_HARD otherwise.
> @@ -2279,8 +2275,24 @@ static void p9_deliver_interrupt(CPUPPCState *env, int interrupt)
> }
> #endif /* TARGET_PPC64 */
>
> -static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> +static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
> {
> +#ifdef TARGET_PPC64
> + switch (env->excp_model) {
> + case POWERPC_EXCP_POWER7:
> + p7_deliver_interrupt(env, interrupt);
> + return;
> + case POWERPC_EXCP_POWER8:
> + p8_deliver_interrupt(env, interrupt);
> + return;
> + case POWERPC_EXCP_POWER9:
> + case POWERPC_EXCP_POWER10:
> + p9_deliver_interrupt(env, interrupt);
> + return;
These return statements could be clubbed with the function call itself.
With the suggested fixes,
Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
> + default:
> + break;
> + }
> +#endif
> PowerPCCPU *cpu = env_archcpu(env);
>
> switch (interrupt) {
> @@ -2383,26 +2395,6 @@ static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
> }
> }
>
> -static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
> -{
> - switch (env->excp_model) {
> -#ifdef TARGET_PPC64
> - case POWERPC_EXCP_POWER7:
> - p7_deliver_interrupt(env, interrupt);
> - break;
> - case POWERPC_EXCP_POWER8:
> - p8_deliver_interrupt(env, interrupt);
> - break;
> - case POWERPC_EXCP_POWER9:
> - case POWERPC_EXCP_POWER10:
> - p9_deliver_interrupt(env, interrupt);
> - break;
> -#endif
> - default:
> - ppc_deliver_interrupt_generic(env, interrupt);
> - }
> -}
> -
> void ppc_cpu_do_system_reset(CPUState *cs)
> {
> PowerPCCPU *cpu = POWERPC_CPU(cs);
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions
2024-02-22 10:06 ` Harsh Prateek Bora
@ 2024-02-22 11:15 ` BALATON Zoltan
0 siblings, 0 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-02-22 11:15 UTC (permalink / raw)
To: Harsh Prateek Bora
Cc: qemu-devel, qemu-ppc, Nicholas Piggin, Daniel Henrique Barboza,
clg
On Thu, 22 Feb 2024, Harsh Prateek Bora wrote:
> On 1/19/24 03:31, BALATON Zoltan wrote:
>> These wrappers call out to handle POWER7 and newer in separate
>> functions but reduce to the generic case when TARGET_PPC64 is not
>> defined. It is easy enough to include the switch in the beginning of
>> the generic functions to branch out to the specific functions and get
>> rid of these wrappers. This avoids one indirection and entitely
>
> s/entitely/entirely
>
>> compiles out the switch without TARGET_PPC64.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>> target/ppc/excp_helper.c | 70 ++++++++++++++++++----------------------
>> 1 file changed, 31 insertions(+), 39 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 5124c3e6b5..de51627c4c 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1921,8 +1921,21 @@ static int p9_next_unmasked_interrupt(CPUPPCState
>> *env)
>> }
>> #endif /* TARGET_PPC64 */
>> -static int ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
>> +static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>> {
>> +#ifdef TARGET_PPC64
>> + switch (env->excp_model) {
>> + case POWERPC_EXCP_POWER7:
>> + return p7_next_unmasked_interrupt(env);
>> + case POWERPC_EXCP_POWER8:
>> + return p8_next_unmasked_interrupt(env);
>> + case POWERPC_EXCP_POWER9:
>> + case POWERPC_EXCP_POWER10:
>> + return p9_next_unmasked_interrupt(env);
>> + default:
>> + break;
>> + }
>> +#endif
>> bool async_deliver;
>> /* External reset */
>> @@ -2033,23 +2046,6 @@ static int
>> ppc_next_unmasked_interrupt_generic(CPUPPCState *env)
>> return 0;
>> }
>> -static int ppc_next_unmasked_interrupt(CPUPPCState *env)
>> -{
>> - switch (env->excp_model) {
>> -#ifdef TARGET_PPC64
>> - case POWERPC_EXCP_POWER7:
>> - return p7_next_unmasked_interrupt(env);
>> - case POWERPC_EXCP_POWER8:
>> - return p8_next_unmasked_interrupt(env);
>> - case POWERPC_EXCP_POWER9:
>> - case POWERPC_EXCP_POWER10:
>> - return p9_next_unmasked_interrupt(env);
>> -#endif
>> - default:
>> - return ppc_next_unmasked_interrupt_generic(env);
>> - }
>> -}
>> -
>> /*
>> * Sets CPU_INTERRUPT_HARD if there is at least one unmasked interrupt to
>> be
>> * delivered and clears CPU_INTERRUPT_HARD otherwise.
>> @@ -2279,8 +2275,24 @@ static void p9_deliver_interrupt(CPUPPCState *env,
>> int interrupt)
>> }
>> #endif /* TARGET_PPC64 */
>> -static void ppc_deliver_interrupt_generic(CPUPPCState *env, int
>> interrupt)
>> +static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
>> {
>> +#ifdef TARGET_PPC64
>> + switch (env->excp_model) {
>> + case POWERPC_EXCP_POWER7:
>> + p7_deliver_interrupt(env, interrupt);
>> + return;
>> + case POWERPC_EXCP_POWER8:
>> + p8_deliver_interrupt(env, interrupt);
>> + return;
>> + case POWERPC_EXCP_POWER9:
>> + case POWERPC_EXCP_POWER10:
>> + p9_deliver_interrupt(env, interrupt);
>> + return;
>
> These return statements could be clubbed with the function call itself.
These return void so I thought it's cleaner this way but it appears to
work the way you suggest too so I'll change it then.
> With the suggested fixes,
> Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>
Thanks for reviewing these. I'll send an updated version today.
Regards,
BALATON Zoltan
>> + default:
>> + break;
>> + }
>> +#endif
>> PowerPCCPU *cpu = env_archcpu(env);
>> switch (interrupt) {
>> @@ -2383,26 +2395,6 @@ static void
>> ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
>> }
>> }
>> -static void ppc_deliver_interrupt(CPUPPCState *env, int interrupt)
>> -{
>> - switch (env->excp_model) {
>> -#ifdef TARGET_PPC64
>> - case POWERPC_EXCP_POWER7:
>> - p7_deliver_interrupt(env, interrupt);
>> - break;
>> - case POWERPC_EXCP_POWER8:
>> - p8_deliver_interrupt(env, interrupt);
>> - break;
>> - case POWERPC_EXCP_POWER9:
>> - case POWERPC_EXCP_POWER10:
>> - p9_deliver_interrupt(env, interrupt);
>> - break;
>> -#endif
>> - default:
>> - ppc_deliver_interrupt_generic(env, interrupt);
>> - }
>> -}
>> -
>> void ppc_cpu_do_system_reset(CPUState *cs)
>> {
>> PowerPCCPU *cpu = POWERPC_CPU(cs);
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH v5 0/9] Misc clean ups to target/ppc exception handling
2024-01-18 22:01 [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
` (8 preceding siblings ...)
2024-01-18 22:01 ` [PATCH v5 9/9] target/ppc: Remove interrupt handler wrapper functions BALATON Zoltan
@ 2024-01-30 21:08 ` BALATON Zoltan
2024-02-16 13:24 ` BALATON Zoltan
9 siblings, 1 reply; 21+ messages in thread
From: BALATON Zoltan @ 2024-01-30 21:08 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On Thu, 18 Jan 2024, 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. This
> series was submitted before but only partially merged due to freeze
> and conflicting series os thia was postponed then to avoid conflicts.
Ping?
Regards,
BALATON Zoltan
> v5:
> - rebase on master
> - keep logging nip pointing to the sc instruction
> - add another patch
>
> v4: Rebased on master dropping what was merged
>
> BALATON Zoltan (9):
> target/ppc: Use env_cpu for cpu_abort in excp_helper
> target/ppc: Readability improvements in exception handlers
> target/ppc: Fix gen_sc to use correct nip
> target/ppc: Move patching nip from exception handler to helper_scv
> target/ppc: Simplify syscall exception handlers
> 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
> target/ppc: Remove interrupt handler wrapper functions
>
> target/ppc/cpu.h | 1 +
> target/ppc/excp_helper.c | 490 +++++++++++++--------------------------
> target/ppc/translate.c | 16 +-
> 3 files changed, 170 insertions(+), 337 deletions(-)
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [PATCH v5 0/9] Misc clean ups to target/ppc exception handling
2024-01-30 21:08 ` [PATCH v5 0/9] Misc clean ups to target/ppc exception handling BALATON Zoltan
@ 2024-02-16 13:24 ` BALATON Zoltan
0 siblings, 0 replies; 21+ messages in thread
From: BALATON Zoltan @ 2024-02-16 13:24 UTC (permalink / raw)
To: qemu-devel, qemu-ppc; +Cc: Nicholas Piggin, Daniel Henrique Barboza, clg
On Tue, 30 Jan 2024, BALATON Zoltan wrote:
> On Thu, 18 Jan 2024, 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. This
>> series was submitted before but only partially merged due to freeze
>> and conflicting series os thia was postponed then to avoid conflicts.
>
> Ping?
Ping^2?
Regards,
BALATON Zoltan
>> v5:
>> - rebase on master
>> - keep logging nip pointing to the sc instruction
>> - add another patch
>>
>> v4: Rebased on master dropping what was merged
>>
>> BALATON Zoltan (9):
>> target/ppc: Use env_cpu for cpu_abort in excp_helper
>> target/ppc: Readability improvements in exception handlers
>> target/ppc: Fix gen_sc to use correct nip
>> target/ppc: Move patching nip from exception handler to helper_scv
>> target/ppc: Simplify syscall exception handlers
>> 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
>> target/ppc: Remove interrupt handler wrapper functions
>>
>> target/ppc/cpu.h | 1 +
>> target/ppc/excp_helper.c | 490 +++++++++++++--------------------------
>> target/ppc/translate.c | 16 +-
>> 3 files changed, 170 insertions(+), 337 deletions(-)
>>
>>
>
>
^ permalink raw reply [flat|nested] 21+ messages in thread