Re: [PATCH 09/10] target/ppc: Simplify syscall exception handlers

[BALATON Zoltan <balaton@eik.bme.hu>]

On Wed, 14 Jun 2023, Nicholas Piggin wrote:
> On Mon Jun 12, 2023 at 8:42 AM 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.
>>
>> Signed-off-by: BALATON Zoltan <balaton@eik.bme.hu>
>> ---
>>  target/ppc/excp_helper.c | 26 ++++++++++----------------
>>  1 file changed, 10 insertions(+), 16 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 4f6a6dfb19..089417894e 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -738,26 +738,23 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int excp)
>>          break;
>>      case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
>>      {
>> -        int lev = env->error_code;
>> +        PowerPCCPU *cpu = env_archcpu(env);
>
> I prefer to keep lev. Makes sense to combine the tests though
> I suppose. Although with powernv it's not really clear that we
> want to dump_syscall. dump_hcall is probably better (powernv
> could support a non-PAPR hypervisor with different hcall
> semantics, but also it could support an OS with different
> syscall semantics too). I guess that could be changed back
> when necessary though.

What do you mean changed back? This is not supposed to change when 
dump_hcall and dump_syscall is called. However I've only changed the 
powerpc_excp_7xx() and powerpc_excp_74xx() functions where this is only 
present as a hack for VOF. I've left powerpc_excp_books() where hypercalls 
really exist unchanged because that takes other bits into accound so 
probably we can't combine the tests rhere.

Regards,
BALATON Zoltan

> Thanks,
> Nick
>
>>
>> -        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(env->error_code == 1 && cpu->vhyp)) {
>>              PPCVirtualHypervisorClass *vhc =
>>                  PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
>> +            dump_hcall(env);
>>              vhc->hypercall(cpu->vhyp, cpu);
>>              return;
>> +        } else {
>> +            dump_syscall(env);
>>          }
>> -
>>          break;
>>      }
>>      case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>> @@ -882,26 +879,23 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
>>          break;
>>      case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
>>      {
>> -        int lev = env->error_code;
>> +        PowerPCCPU *cpu = env_archcpu(env);
>>
>> -        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(env->error_code == 1 && cpu->vhyp)) {
>>              PPCVirtualHypervisorClass *vhc =
>>                  PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
>> +            dump_hcall(env);
>>              vhc->hypercall(cpu->vhyp, cpu);
>>              return;
>> +        } else {
>> +            dump_syscall(env);
>>          }
>> -
>>          break;
>>      }
>>      case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>> --
>> 2.30.9
>
>
>