Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp

qemu-ppc

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helpe

From:	BALATON Zoltan
Subject:	Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
Date:	Tue, 20 Jun 2023 12:50:43 +0200 (CEST)

On Tue, 20 Jun 2023, Nicholas Piggin wrote:

On Fri Jun 16, 2023 at 9:03 AM AEST, BALATON Zoltan wrote:

Use the env_cpu function to get the CPUState for cpu_abort. These are
only needed in case of fatal errors so this allows to avoid casting
and storing CPUState in a local variable wnen not needed.


Eh, this is still replacing less typing with more. It's normal to
define these things up front of a function especially when used
multiple times. 'cs' should be as instantly recognizable as env
when looking at code so my preference is to keep it as is.

You can copy & paste to avoid typing :-) Since it's only used in fatalerror messages that should not normally be executed I see no point instoring a variable for that when it's never needed. Originally this patchwas to allow changing the function to take env instead of cpu but evenwithout that patch I think this simplifies it so I'd like to keep it ifyou don't mind.


Regards,
BALATON Zoltan

Thanks,
Nick

Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
---
 target/ppc/excp_helper.c | 118 +++++++++++++++++++++------------------
 1 file changed, 63 insertions(+), 55 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 79f5ca1034..122e2a6e41 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -424,7 +424,6 @@ static void powerpc_mcheck_checkstop(CPUPPCState *env)

 static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
     int srr0, srr1;
@@ -452,8 +451,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -502,7 +501,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
             env->spr[SPR_40x_ESR] = ESR_PTR;
             break;
         default:
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -529,11 +528,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         trace_ppc_excp_print("PIT");
         break;
     case POWERPC_EXCP_DEBUG:     /* Debug interrupt                          */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -548,7 +548,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)

 static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;

@@ -571,8 +570,8 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -632,7 +631,7 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
             break;
         default:
             /* Should never occur */
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -654,8 +653,9 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
         if (FIELD_EX64(env->msr, MSR, POW)) {
-            cpu_abort(cs, "Trying to deliver power-saving system reset "
-                      "exception %d with no HV support\n", excp);
+            cpu_abort(env_cpu(env),
+                      "Trying to deliver power-saving system reset exception "
+                      "%d with no HV support\n", excp);
         }
         break;
     case POWERPC_EXCP_TRACE:     /* Trace exception                          */
@@ -682,11 +682,12 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_SMI:       /* System management interrupt              */
     case POWERPC_EXCP_MEXTBR:    /* Maskable external breakpoint             */
     case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint         */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -709,7 +710,6 @@ static void powerpc_excp_6xx(PowerPCCPU *cpu, int excp)

 static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;

@@ -732,8 +732,8 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -791,7 +791,7 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
             break;
         default:
             /* Should never occur */
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -832,8 +832,9 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
         if (FIELD_EX64(env->msr, MSR, POW)) {
-            cpu_abort(cs, "Trying to deliver power-saving system reset "
-                      "exception %d with no HV support\n", excp);
+            cpu_abort(env_cpu(env),
+                      "Trying to deliver power-saving system reset exception "
+                      "%d with no HV support\n", excp);
         }
         break;
     case POWERPC_EXCP_TRACE:     /* Trace exception                          */
@@ -853,11 +854,12 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_SMI:       /* System management interrupt              */
     case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
     case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -880,7 +882,6 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)

 static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;

@@ -903,8 +904,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -962,7 +963,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
             break;
         default:
             /* Should never occur */
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -1003,7 +1004,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
         if (FIELD_EX64(env->msr, MSR, POW)) {
-            cpu_abort(cs, "Trying to deliver power-saving system reset "
+            cpu_abort(env_cpu(env),
+                      "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
         break;
@@ -1016,11 +1018,12 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
     case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
     case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -1043,7 +1046,6 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)

 static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
     int srr0, srr1;
@@ -1080,8 +1082,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -1112,6 +1114,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
         break;
     case POWERPC_EXCP_EXTERNAL:  /* External input                           */
         if (env->mpic_proxy) {
+            CPUState *cs = env_cpu(env);
             /* IACK the IRQ on delivery */
             env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
         }
@@ -1150,7 +1153,7 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)
             break;
         default:
             /* Should never occur */
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -1191,7 +1194,8 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int excp)

             /* DBSR already modified by caller */
         } else {
-            cpu_abort(cs, "Debug exception triggered on unsupported model\n");
+            cpu_abort(env_cpu(env),
+                      "Debug exception triggered on unsupported model\n");
         }
         break;
     case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  
*/
@@ -1205,17 +1209,19 @@ static void powerpc_excp_booke(PowerPCCPU *cpu, int 
excp)
         break;
     case POWERPC_EXCP_RESET:     /* System reset exception                   */
         if (FIELD_EX64(env->msr, MSR, POW)) {
-            cpu_abort(cs, "Trying to deliver power-saving system reset "
+            cpu_abort(env_cpu(env),
+                      "Trying to deliver power-saving system reset "
                       "exception %d with no HV support\n", excp);
         }
         break;
     case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
     case POWERPC_EXCP_EFPRI:     /* Embedded floating-point round interrupt  */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -1278,7 +1284,6 @@ static bool books_vhyp_handles_hv_excp(PowerPCCPU *cpu)

 static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
     int srr0, srr1, lev = -1;
@@ -1317,8 +1322,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)

     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
-        cpu_abort(cs, "Raised an exception without defined vector %d\n",
-                  excp);
+        cpu_abort(env_cpu(env),
+                  "Raised an exception without defined vector %d\n", excp);
     }

     vector |= env->excp_prefix;
@@ -1408,7 +1413,7 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
             break;
         default:
             /* Should never occur */
-            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+            cpu_abort(env_cpu(env), "Invalid program exception %d. Aborting\n",
                       env->error_code);
             break;
         }
@@ -1469,7 +1474,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
             new_msr |= (target_ulong)MSR_HVB;
         } else {
             if (FIELD_EX64(env->msr, MSR, POW)) {
-                cpu_abort(cs, "Trying to deliver power-saving system reset "
+                cpu_abort(env_cpu(env),
+                          "Trying to deliver power-saving system reset "
                           "exception %d with no HV support\n", excp);
             }
         }
@@ -1524,11 +1530,12 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int 
excp)
     case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
     case POWERPC_EXCP_MAINT:     /* Maintenance exception                    */
     case POWERPC_EXCP_HV_MAINT:  /* Hypervisor Maintenance exception         */
-        cpu_abort(cs, "%s exception not implemented\n",
+        cpu_abort(env_cpu(env), "%s exception not implemented\n",
                   powerpc_excp_name(excp));
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
         break;
     }

@@ -1561,8 +1568,8 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
     } else {
         /* Sanity check */
         if (!(env->msr_mask & MSR_HVB) && srr0 == SPR_HSRR0) {
-            cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
-                      "no HV support\n", excp);
+            cpu_abort(env_cpu(env), "Trying to deliver HV exception (HSRR) %d "
+                      "with no HV support\n", excp);
         }

         /* This can update new_msr and vector if AIL applies */
@@ -1580,11 +1587,11 @@ static inline void powerpc_excp_books(PowerPCCPU *cpu, 
int excp)

 static void powerpc_excp(PowerPCCPU *cpu, int excp)
 {
-    CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;

     if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
-        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        cpu_abort(env_cpu(env), "Invalid PowerPC exception %d. Aborting\n",
+                  excp);
     }

     qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
@@ -2118,7 +2125,6 @@ void ppc_maybe_interrupt(CPUPPCState *env)
 static void p7_deliver_interrupt(CPUPPCState *env, int interrupt)
 {
     PowerPCCPU *cpu = env_archcpu(env);
-    CPUState *cs = env_cpu(env);

     switch (interrupt) {
     case PPC_INTERRUPT_MCK: /* Machine check exception */
@@ -2162,14 +2168,14 @@ static void p7_deliver_interrupt(CPUPPCState *env, int 
interrupt)
         assert(!env->resume_as_sreset);
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+        cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+                  interrupt);
     }
 }

 static void p8_deliver_interrupt(CPUPPCState *env, int interrupt)
 {
     PowerPCCPU *cpu = env_archcpu(env);
-    CPUState *cs = env_cpu(env);

     switch (interrupt) {
     case PPC_INTERRUPT_MCK: /* Machine check exception */
@@ -2233,7 +2239,8 @@ static void p8_deliver_interrupt(CPUPPCState *env, int 
interrupt)
         assert(!env->resume_as_sreset);
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+        cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+                  interrupt);
     }
 }

@@ -2312,7 +2319,8 @@ static void p9_deliver_interrupt(CPUPPCState *env, int 
interrupt)
         assert(!env->resume_as_sreset);
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+        cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+                  interrupt);
     }
 }
 #endif
@@ -2320,7 +2328,6 @@ static void p9_deliver_interrupt(CPUPPCState *env, int 
interrupt)
 static void ppc_deliver_interrupt_generic(CPUPPCState *env, int interrupt)
 {
     PowerPCCPU *cpu = env_archcpu(env);
-    CPUState *cs = env_cpu(env);

     switch (interrupt) {
     case PPC_INTERRUPT_RESET: /* External reset */
@@ -2417,7 +2424,8 @@ static void ppc_deliver_interrupt_generic(CPUPPCState 
*env, int interrupt)
         assert(!env->resume_as_sreset);
         break;
     default:
-        cpu_abort(cs, "Invalid PowerPC interrupt %d. Aborting\n", interrupt);
+        cpu_abort(env_cpu(env), "Invalid PowerPC interrupt %d. Aborting\n",
+                  interrupt);
     }
 }

--
2.30.9

[Prev in Thread]

Current Thread

[Next in Thread]

[PATCH v3 00/14] Misc clean ups to target/ppc exception handling, BALATON Zoltan, 2023/06/15
- [PATCH v3 01/14] target/ppc: Remove some superfluous parentheses, BALATON Zoltan, 2023/06/15
- [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once, BALATON Zoltan, 2023/06/15
  - Re: [PATCH v3 05/14] target/ppc: Remove some more local CPUState variables only used once, Nicholas Piggin, 2023/06/20
- [PATCH v3 03/14] target/ppc: Move common check in exception handlers to a function, BALATON Zoltan, 2023/06/15
- [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper, BALATON Zoltan, 2023/06/15
  - Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper, Nicholas Piggin, 2023/06/20
    - Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper, BALATON Zoltan <=
- [PATCH v3 07/14] target/ppd: Remove unused define, BALATON Zoltan, 2023/06/15
  - Re: [PATCH v3 07/14] target/ppd: Remove unused define, Bernhard Beschow, 2023/06/18
- [PATCH v3 06/14] target/ppc: Readability improvements in exception handlers, BALATON Zoltan, 2023/06/15
- [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip, BALATON Zoltan, 2023/06/15
  - Re: [PATCH v3 08/14] target/ppc: Fix gen_sc to use correct nip, Nicholas Piggin, 2023/06/20
- [PATCH v3 02/14] target/ppc: Remove unneeded parameter from powerpc_reset_wakeup(), BALATON Zoltan, 2023/06/15
- [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv, BALATON Zoltan, 2023/06/15
  - Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv, Nicholas Piggin, 2023/06/20
    - Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv, BALATON Zoltan, 2023/06/20
    - Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv, Nicholas Piggin, 2023/06/26

Prev by Date: Re: [PATCH v3 09/14] target/ppc: Move patching nip from exception handler to helper_scv
Next by Date: Re: [PATCH v3 12/14] target/ppc: Clean up ifdefs in excp_helper.c, part 1
Previous by thread: Re: [PATCH v3 04/14] target/ppc: Use env_cpu for cpu_abort in excp_helper
Next by thread: [PATCH v3 07/14] target/ppd: Remove unused define
Index(es):
- Date
- Thread