[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers
From: |
Nicholas Piggin |
Subject: |
Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers |
Date: |
Thu, 15 Jun 2023 21:46:41 +1000 |
On Thu Jun 15, 2023 at 7:25 PM AEST, BALATON Zoltan wrote:
> On Thu, 15 Jun 2023, Nicholas Piggin wrote:
> > On Thu Jun 15, 2023 at 7:34 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 1682b988ba..662457f342 100644
> >> --- a/target/ppc/excp_helper.c
> >> +++ b/target/ppc/excp_helper.c
> >> @@ -740,26 +740,23 @@ static void powerpc_excp_7xx(PowerPCCPU *cpu, int
> >> excp)
> >> break;
> >> case POWERPC_EXCP_SYSCALL: /* System call exception
> >> */
> >> {
> >> - int lev = env->error_code;
> >
> > I would still keep lev. Self documenting and consistent with books
> > handler.
>
> lev is still there in the books version, but probably not really needed in
> these 7xx versions which does not really have level parameter. This hack
> should likely go away and replaced with something else on the long run as
> this won't work with KVM but that needs some support from VOF or compiling
> a different version for pegasos2 which wasn't considered so far. I can add
> the local back if you really insist but I don't think it really makes much
> sense in these cases for 7xx and 74xx.
It is using the sc 1 instruction which does have a lev field though? The
hardware might not have such a thing but what is being emulatd here
does, so I think lev makes sense.
Removing this would be fine, but while you have it yes please just leave
it as lev.
> >> + PowerPCCPU *cpu = env_archcpu(env);
> >
> > Is this necessary?
>
> Yes, for cpu->vhyp below.
cpu->vhyp was there before your patch...
Thanks,
Nick
- [PATCH v2 05/10] target/ppc: Change parameter of cpu_interrupt_exittb() to an env pointer, (continued)
[PATCH v2 06/10] target/ppc: Readability improvements in exception handlers, BALATON Zoltan, 2023/06/14
[PATCH v2 07/10] target/ppd: Remove unused define, BALATON Zoltan, 2023/06/14
[PATCH v2 09/10] target/ppc: Simplify syscall exception handlers, BALATON Zoltan, 2023/06/14
[PATCH v2 08/10] target/ppc: Fix gen_sc to use correct nip, BALATON Zoltan, 2023/06/14
[PATCH v2 10/10] target/ppc: Get CPUState in one step, BALATON Zoltan, 2023/06/14