Re: [PATCH v2 6/8] target/ppc: 74xx: System Call exception cleanup

[BALATON Zoltan]

On Thu, 27 Jan 2022, Fabiano Rosas wrote:
Remove the BookE code and add a comment explaining why we need to keep
hypercall support even though this CPU does not have a hypervisor
mode.

Thanks, this should do it. Just some small comments below.

Signed-off-by: Fabiano Rosas <faro...@linux.ibm.com>
---
target/ppc/excp_helper.c | 17 +++++++++++------
1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index bb17b65dc0..396e25ed35 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -556,7 +556,7 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
    CPUState *cs = CPU(cpu);
    CPUPPCState *env = &cpu->env;
    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);
@@ -669,7 +669,8 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
        }
        break;
    case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
-        lev = env->error_code;
+    {
+        int lev = env->error_code;

        if ((lev == 1) && cpu->vhyp) {

In case you'll have another version, maybe drop the extra parenthesis 
around lev == here and below...

            dump_hcall(env);
@@ -683,17 +684,21 @@ static void powerpc_excp_74xx(PowerPCCPU *cpu, int excp)
         */
        env->nip += 4;

-        /* "PAPR mode" built-in hypercall emulation */
+        /*
+         * 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) {

...here. Or maybe even drop the lev local variable and just use 
env->error_code in the ifs directly? Only apprears twice now so could do 
without copying it to a local. But not a big deal so it's not worth a 
respin only for this.

Regards,
BALATON Zoltan

            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_FPU:       /* Floating-point unavailable exception     */
    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
        break;