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

[BALATON Zoltan]

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