diff mbox series

[v6,5/9] target/ppc: Simplify syscall exception handlers

Message ID 5441ced92de39f712378e8aac4346aef809301c7.1708601065.git.balaton@eik.bme.hu
State New
Headers show
Series Misc clean ups to target/ppc exception handling | expand

Commit Message

BALATON Zoltan Feb. 22, 2024, 11:33 a.m. UTC
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(-)

Comments

Philippe Mathieu-Daudé Feb. 22, 2024, 12:09 p.m. UTC | #1
On 22/2/24 12:33, BALATON Zoltan wrote:
> After previous changes the hypercall handling in 7xx and 74xx
> exception handlers can be folded into one if statement to simpilfy

"simplify"

> this code. Also add "unlikely" to mark the less freqiently used branch

"frequently"

> 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(-)
BALATON Zoltan Feb. 22, 2024, 12:20 p.m. UTC | #2
On Thu, 22 Feb 2024, Philippe Mathieu-Daudé wrote:
> On 22/2/24 12:33, BALATON Zoltan wrote:
>> After previous changes the hypercall handling in 7xx and 74xx
>> exception handlers can be folded into one if statement to simpilfy
>
> "simplify"
>
>> this code. Also add "unlikely" to mark the less freqiently used branch
>
> "frequently"

Could these be fixed up when merging please? I'd not resend again unless 
there's some other things need fixing.

Regards,
BALATON Zoltan

>> 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(-)
>
>
>
Nicholas Piggin Feb. 27, 2024, 6:32 a.m. UTC | #3
On Thu Feb 22, 2024 at 9:33 PM AEST, BALATON Zoltan wrote:
> After previous changes the hypercall handling in 7xx and 74xx
> exception handlers can be folded into one if statement to simpilfy
> this code. 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;

You could avoid the else statement for these because the
hcall branch returns.

Actually books could be changed similarly too, I think dump_hcall can be
done in the books_vhyp_handles_hcall() branch. But you don't need to
change that in your patch since it's behaviour change.

Thanks,
Nick
Nicholas Piggin Feb. 27, 2024, 6:36 a.m. UTC | #4
On Thu Feb 22, 2024 at 10:20 PM AEST, BALATON Zoltan wrote:
> On Thu, 22 Feb 2024, Philippe Mathieu-Daudé wrote:
> > On 22/2/24 12:33, BALATON Zoltan wrote:
> >> After previous changes the hypercall handling in 7xx and 74xx
> >> exception handlers can be folded into one if statement to simpilfy
> >
> > "simplify"
> >
> >> this code. Also add "unlikely" to mark the less freqiently used branch
> >
> > "frequently"
>
> Could these be fixed up when merging please? I'd not resend again unless 
> there's some other things need fixing.

Main thing was the gen_exception_err code shraing with sc. If you
wouldn't mind resending the series with all fixups. I'll plan to
get another ppc PR in before soft freeze in ~ 2 weeks so and I'll
grab this if possible.

Thanks,
Nick
diff mbox series

Patch

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     */