From: Harsh Prateek Bora <harshpb@linux.ibm.com>
To: BALATON Zoltan <balaton@eik.bme.hu>,
qemu-devel@nongnu.org, qemu-ppc@nongnu.org
Cc: Nicholas Piggin <npiggin@gmail.com>,
Daniel Henrique Barboza <danielhb413@gmail.com>,
clg@kaod.org
Subject: Re: [PATCH v5 1/9] target/ppc: Use env_cpu for cpu_abort in excp_helper
Date: Wed, 21 Feb 2024 14:40:37 +0530 [thread overview]
Message-ID: <81762fe1-38a9-4892-a1fb-4ea41fd0d9c6@linux.ibm.com> (raw)
In-Reply-To: <dd4c596af0c12b6a2b4935e4571ef88f866178a6.1705614747.git.balaton@eik.bme.hu>
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);
> }
> }
>
next prev parent reply other threads:[~2024-02-21 15:41 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
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-02-21 9:10 ` Harsh Prateek Bora [this message]
2024-01-18 22:01 ` [PATCH v5 2/9] target/ppc: Readability improvements in exception handlers BALATON Zoltan
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 ` [PATCH v5 4/9] target/ppc: Move patching nip from exception handler to helper_scv BALATON Zoltan
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
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
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
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
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
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=81762fe1-38a9-4892-a1fb-4ea41fd0d9c6@linux.ibm.com \
--to=harshpb@linux.ibm.com \
--cc=balaton@eik.bme.hu \
--cc=clg@kaod.org \
--cc=danielhb413@gmail.com \
--cc=npiggin@gmail.com \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).