qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: BALATON Zoltan
Subject: Re: [PATCH v2 09/10] target/ppc: Simplify syscall exception handlers
Date: Fri, 16 Jun 2023 01:02:24 +0200 (CEST)

On Thu, 15 Jun 2023, Nicholas Piggin wrote:
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...

Originally I had another patch that chnaged these functions to take an env pointer and since cpu is only needed here this was declared local to where it's used. Now that we don't have those patches that change more functions parameters to env then it's indeed not needed. I've added back lev in v3 instead.

Regards,
BALATON Zoltan



reply via email to

[Prev in Thread] Current Thread [Next in Thread]