[PATCH v2 08/14] target/ppc: 405: System call exception cleanup

Fabiano Rosas posted 14 patches 4 years ago
Maintainers: "Cédric Le Goater" <clg@kaod.org>, Greg Kurz <groug@kaod.org>, Daniel Henrique Barboza <danielhb413@gmail.com>, David Gibson <david@gibson.dropbear.id.au>
[PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by Fabiano Rosas 4 years ago
There's no sc 1.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 21 ++-------------------
 1 file changed, 2 insertions(+), 19 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 8fae8aa0be..9a6f8365d6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -398,7 +398,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
     CPUPPCState *env = &cpu->env;
     int excp_model = env->excp_model;
     target_ulong msr, new_msr, vector;
-    int srr0, srr1, lev = -1;
+    int srr0, srr1;
 
     if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
         cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
@@ -521,30 +521,13 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         }
         break;
     case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
-        lev = env->error_code;
-
-        if ((lev == 1) && cpu->vhyp) {
-            dump_hcall(env);
-        } else {
-            dump_syscall(env);
-        }
+        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) && cpu->vhyp) {
-            PPCVirtualHypervisorClass *vhc =
-                PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
-            vhc->hypercall(cpu->vhyp, cpu);
-            return;
-        }
-        if (lev == 1) {
-            new_msr |= (target_ulong)MSR_HVB;
-        }
         break;
     case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
         trace_ppc_excp_print("FIT");
-- 
2.33.1


Re: [PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by David Gibson 4 years ago
On Tue, Jan 18, 2022 at 03:44:42PM -0300, Fabiano Rosas wrote:
> There's no sc 1.

No... but what exactly should and will happen if you attempt to
execute an "sc 1" on 40x.  Will it be treated as an "sc 0", or will it
cause a 0x700?  If it's a 0x700, better double check that that is
generated at translation time, if you're removing the check on level
here.

> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/excp_helper.c | 21 ++-------------------
>  1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 8fae8aa0be..9a6f8365d6 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -398,7 +398,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>      CPUPPCState *env = &cpu->env;
>      int excp_model = env->excp_model;
>      target_ulong msr, new_msr, vector;
> -    int srr0, srr1, lev = -1;
> +    int srr0, srr1;
>  
>      if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
>          cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> @@ -521,30 +521,13 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>          }
>          break;
>      case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
> -        lev = env->error_code;
> -
> -        if ((lev == 1) && cpu->vhyp) {
> -            dump_hcall(env);
> -        } else {
> -            dump_syscall(env);
> -        }
> +        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) && cpu->vhyp) {
> -            PPCVirtualHypervisorClass *vhc =
> -                PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> -            vhc->hypercall(cpu->vhyp, cpu);
> -            return;
> -        }
> -        if (lev == 1) {
> -            new_msr |= (target_ulong)MSR_HVB;
> -        }
>          break;
>      case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
>          trace_ppc_excp_print("FIT");

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson
Re: [PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by Cédric Le Goater 4 years ago
On 1/19/22 07:09, David Gibson wrote:
> On Tue, Jan 18, 2022 at 03:44:42PM -0300, Fabiano Rosas wrote:
>> There's no sc 1.
> 
> No... but what exactly should and will happen if you attempt to
> execute an "sc 1" on 40x.  Will it be treated as an "sc 0", or will it
> cause a 0x700?  If it's a 0x700, better double check that that is
> generated at translation time, if you're removing the check on level
> here.

A Program Interrupt with the illegal instruction error code should be
generated at translation time but it is not the case today. It never
was correctly implemented AFAICT :

   /* Top bit of opc2 corresponds with low bit of LEV, so use two handlers */
   GEN_HANDLER(sc, 0x11, 0x11, 0xFF, 0x03FFF01D, PPC_FLOW),
   GEN_HANDLER(sc, 0x11, 0x01, 0xFF, 0x03FFF01D, PPC_FLOW),

We would need a simple 'sc' instruction for the PPC405 and other
processors. Let's add that to the TODO list.

Thanks,

C.


> 
>>
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>   target/ppc/excp_helper.c | 21 ++-------------------
>>   1 file changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 8fae8aa0be..9a6f8365d6 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -398,7 +398,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>       CPUPPCState *env = &cpu->env;
>>       int excp_model = env->excp_model;
>>       target_ulong msr, new_msr, vector;
>> -    int srr0, srr1, lev = -1;
>> +    int srr0, srr1;
>>   
>>       if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
>>           cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>> @@ -521,30 +521,13 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>           }
>>           break;
>>       case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
>> -        lev = env->error_code;
>> -
>> -        if ((lev == 1) && cpu->vhyp) {
>> -            dump_hcall(env);
>> -        } else {
>> -            dump_syscall(env);
>> -        }
>> +        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) && cpu->vhyp) {
>> -            PPCVirtualHypervisorClass *vhc =
>> -                PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
>> -            vhc->hypercall(cpu->vhyp, cpu);
>> -            return;
>> -        }
>> -        if (lev == 1) {
>> -            new_msr |= (target_ulong)MSR_HVB;
>> -        }
>>           break;
>>       case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
>>           trace_ppc_excp_print("FIT");
> 


Re: [PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by Cédric Le Goater 4 years ago
On 1/25/22 09:18, Cédric Le Goater wrote:
> On 1/19/22 07:09, David Gibson wrote:
>> On Tue, Jan 18, 2022 at 03:44:42PM -0300, Fabiano Rosas wrote:
>>> There's no sc 1.
>>
>> No... but what exactly should and will happen if you attempt to
>> execute an "sc 1" on 40x.  Will it be treated as an "sc 0", or will it
>> cause a 0x700?  If it's a 0x700, better double check that that is
>> generated at translation time, if you're removing the check on level
>> here.
> 
> A Program Interrupt with the illegal instruction error code should be
> generated at translation time but it is not the case today. It never
> was correctly implemented AFAICT :
> 
>    /* Top bit of opc2 corresponds with low bit of LEV, so use two handlers */
>    GEN_HANDLER(sc, 0x11, 0x11, 0xFF, 0x03FFF01D, PPC_FLOW),
>    GEN_HANDLER(sc, 0x11, 0x01, 0xFF, 0x03FFF01D, PPC_FLOW),
> 
> We would need a simple 'sc' instruction for the PPC405 and other
> processors. Let's add that to the TODO list.

The ref405ep machine now boots a mainline Linux with a buildroot user space.
Let's get this series merged first.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

Re: [PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by BALATON Zoltan 4 years ago
On Tue, 25 Jan 2022, Cédric Le Goater wrote:
> On 1/19/22 07:09, David Gibson wrote:
>> On Tue, Jan 18, 2022 at 03:44:42PM -0300, Fabiano Rosas wrote:
>>> There's no sc 1.
>> 
>> No... but what exactly should and will happen if you attempt to
>> execute an "sc 1" on 40x.  Will it be treated as an "sc 0", or will it
>> cause a 0x700?  If it's a 0x700, better double check that that is
>> generated at translation time, if you're removing the check on level
>> here.
>
> A Program Interrupt with the illegal instruction error code should be
> generated at translation time but it is not the case today. It never
> was correctly implemented AFAICT :
>
>  /* Top bit of opc2 corresponds with low bit of LEV, so use two handlers */
>  GEN_HANDLER(sc, 0x11, 0x11, 0xFF, 0x03FFF01D, PPC_FLOW),
>  GEN_HANDLER(sc, 0x11, 0x01, 0xFF, 0x03FFF01D, PPC_FLOW),
>
> We would need a simple 'sc' instruction for the PPC405 and other
> processors. Let's add that to the TODO list.

Not directly related to this but as a reminder: if I remember correctly 
VOF uses sc 1 for hypercalls and I use VOF on pegasos2 which has a G4 or 
G3 CPU that does not have this instruction but we emulate that anyway so 
this works now at least with TCG. AFAICT changes so far did not break this 
but please consider this when getting there. We could use a different 
method for hypercalls in VOF but that would either result in different VOF 
binary for different machines or needing more changes to spap,r neither of 
which is desirable, so we chose this solution for now to allow hypercalls 
on 32bit PPC if the vhyp is set. This was in commit 5e994fc019862.

Regards,
BALATON Zoltan
Re: [PATCH v2 08/14] target/ppc: 405: System call exception cleanup
Posted by Richard Henderson 4 years ago
On 1/19/22 5:44 AM, Fabiano Rosas wrote:
> There's no sc 1.
> 
> Signed-off-by: Fabiano Rosas<farosas@linux.ibm.com>
> ---
>   target/ppc/excp_helper.c | 21 ++-------------------
>   1 file changed, 2 insertions(+), 19 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~