All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n)
@ 2022-01-10 18:15 Fabiano Rosas
  2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
                   ` (8 more replies)
  0 siblings, 9 replies; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

This is the first series of the exception model-specific changes. I
intend to keep this same structure for the rest of the exception
models.

== preparation ==

First few patches are cleanups specific to this model. This comes
first because I'm using some of these changes to help validate what
should be removed in the subsequent patches.

== new powerpc_excp function ==

One patch copies the powerpc_excp function as is and renames it to the
exception model in question. Doing this first facilitates spotting
what changed from the _legacy version.

The following patch changes everything that needs to be changed at the
top level (MSR mask, SF, ILE, AIL, SRRs, etc.) and removes exceptions
that are not used by this processor family.

== exceptions cleanup ==

After the new powerpc_excp function has been made specific to that one
exception model, go through every exception removing anything that
does not apply to this model.

Based on legoater/ppc-7.0

Cleanups 2/n [in ppc-7.0]:
https://lists.nongnu.org/archive/html/qemu-ppc/2022-01/msg00252.html

Cleanups 1/n [already merged]:
https://mail.gnu.org/archive/html/qemu-ppc/2021-12/msg00696.html

RFC v2:
https://lists.nongnu.org/archive/html/qemu-ppc/2021-12/msg00542.html

RFC v1:
https://lists.nongnu.org/archive/html/qemu-ppc/2021-06/msg00026.html

Fabiano Rosas (8):
  target/ppc: 405: Add missing MSR bits to msr_mask
  target/ppc: 405: Add missing exception handlers
  target/ppc: Introduce powerpc_excp_40x
  squash target/ppc: Introduce powerpc_excp_40x
  target/ppc: 405: Critical exceptions cleanup
  target/ppc: 405: Machine check exception cleanup
  target/ppc: 405: External exception cleanup
  target/ppc: 405: System call exception cleanup

 target/ppc/cpu_init.c    |   8 +-
 target/ppc/excp_helper.c | 246 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 253 insertions(+), 1 deletion(-)

-- 
2.33.1



^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:04   ` David Gibson
  2022-01-17 21:12   ` Fabiano Rosas
  2022-01-10 18:15 ` [PATCH 2/8] target/ppc: 405: Add missing exception handlers Fabiano Rosas
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

Some bits described in the user manual are missing from msr_mask. Add
them.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/cpu_init.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index e30e86fe9d..a50ddaeaae 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
                        PPC_MEM_SYNC | PPC_MEM_EIEIO |
                        PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC |
                        PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP;
-    pcc->msr_mask = (1ull << MSR_POW) |
+    pcc->msr_mask = (1ull << MSR_AP) |
+                    (1ull << MSR_POW) |
                     (1ull << MSR_CE) |
                     (1ull << MSR_EE) |
                     (1ull << MSR_PR) |
                     (1ull << MSR_FP) |
+                    (1ull << MSR_ME) |
                     (1ull << MSR_DWE) |
                     (1ull << MSR_DE) |
+                    (1ull << MSR_FE1) |
                     (1ull << MSR_IR) |
                     (1ull << MSR_DR);
+
     pcc->mmu_model = POWERPC_MMU_SOFT_4xx;
     pcc->excp_model = POWERPC_EXCP_40x;
     pcc->bus_model = PPC_FLAGS_INPUT_405;
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/8] target/ppc: 405: Add missing exception handlers
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
  2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:10   ` David Gibson
  2022-01-10 18:15 ` [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x Fabiano Rosas
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/cpu_init.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
index a50ddaeaae..9097948e67 100644
--- a/target/ppc/cpu_init.c
+++ b/target/ppc/cpu_init.c
@@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
     env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
     env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
     env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
+    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
     env->excp_vectors[POWERPC_EXCP_SYSCALL]  = 0x00000C00;
+    env->excp_vectors[POWERPC_EXCP_APU]      = 0x00000F20;
     env->excp_vectors[POWERPC_EXCP_PIT]      = 0x00001000;
     env->excp_vectors[POWERPC_EXCP_FIT]      = 0x00001010;
     env->excp_vectors[POWERPC_EXCP_WDT]      = 0x00001020;
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
  2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
  2022-01-10 18:15 ` [PATCH 2/8] target/ppc: 405: Add missing exception handlers Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:20   ` David Gibson
  2022-01-10 18:15 ` [PATCH 4/8] squash " Fabiano Rosas
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
I copied powerpc_excp_legacy verbatim in this commit so the next
one has a clean diff. We can squash both commits before merging.
---
 target/ppc/excp_helper.c | 474 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 474 insertions(+)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index bc646c67a0..12ab5e1b34 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -392,6 +392,477 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu,
     check_tlb_flush(env, false);
 }
 
+static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
+{
+    CPUState *cs = CPU(cpu);
+    CPUPPCState *env = &cpu->env;
+    int excp_model = env->excp_model;
+    target_ulong msr, new_msr, vector;
+    int srr0, srr1, lev = -1;
+
+    if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
+        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+    }
+
+    qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
+                  " => %s (%d) error=%02x\n", env->nip, powerpc_excp_name(excp),
+                  excp, env->error_code);
+
+    /* new srr1 value excluding must-be-zero bits */
+    if (excp_model == POWERPC_EXCP_BOOKE) {
+        msr = env->msr;
+    } else {
+        msr = env->msr & ~0x783f0000ULL;
+    }
+
+    /*
+     * new interrupt handler msr preserves existing HV and ME unless
+     * explicitly overriden
+     */
+    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
+
+    /* target registers */
+    srr0 = SPR_SRR0;
+    srr1 = SPR_SRR1;
+
+    /*
+     * check for special resume at 0x100 from doze/nap/sleep/winkle on
+     * P7/P8/P9
+     */
+    if (env->resume_as_sreset) {
+        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
+    }
+
+    /*
+     * Hypervisor emulation assistance interrupt only exists on server
+     * arch 2.05 server or later. We also don't want to generate it if
+     * we don't have HVB in msr_mask (PAPR mode).
+     */
+    if (excp == POWERPC_EXCP_HV_EMU
+#if defined(TARGET_PPC64)
+        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
+#endif /* defined(TARGET_PPC64) */
+
+    ) {
+        excp = POWERPC_EXCP_PROGRAM;
+    }
+
+#ifdef TARGET_PPC64
+    /*
+     * SPEU and VPU share the same IVOR but they exist in different
+     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
+     */
+    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
+        excp = POWERPC_EXCP_SPEU;
+    }
+#endif
+
+    vector = env->excp_vectors[excp];
+    if (vector == (target_ulong)-1ULL) {
+        cpu_abort(cs, "Raised an exception without defined vector %d\n",
+                  excp);
+    }
+
+    vector |= env->excp_prefix;
+
+    switch (excp) {
+    case POWERPC_EXCP_CRITICAL:    /* Critical input                         */
+        switch (excp_model) {
+        case POWERPC_EXCP_40x:
+            srr0 = SPR_40x_SRR2;
+            srr1 = SPR_40x_SRR3;
+            break;
+        case POWERPC_EXCP_BOOKE:
+            srr0 = SPR_BOOKE_CSRR0;
+            srr1 = SPR_BOOKE_CSRR1;
+            break;
+        case POWERPC_EXCP_G2:
+            break;
+        default:
+            goto excp_invalid;
+        }
+        break;
+    case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
+        if (msr_me == 0) {
+            /*
+             * Machine check exception is not enabled.  Enter
+             * checkstop state.
+             */
+            fprintf(stderr, "Machine check while not allowed. "
+                    "Entering checkstop state\n");
+            if (qemu_log_separate()) {
+                qemu_log("Machine check while not allowed. "
+                        "Entering checkstop state\n");
+            }
+            cs->halted = 1;
+            cpu_interrupt_exittb(cs);
+        }
+        if (env->msr_mask & MSR_HVB) {
+            /*
+             * ISA specifies HV, but can be delivered to guest with HV
+             * clear (e.g., see FWNMI in PAPR).
+             */
+            new_msr |= (target_ulong)MSR_HVB;
+        }
+
+        /* machine check exceptions don't have ME set */
+        new_msr &= ~((target_ulong)1 << MSR_ME);
+
+        /* XXX: should also have something loaded in DAR / DSISR */
+        switch (excp_model) {
+        case POWERPC_EXCP_40x:
+            srr0 = SPR_40x_SRR2;
+            srr1 = SPR_40x_SRR3;
+            break;
+        case POWERPC_EXCP_BOOKE:
+            /* FIXME: choose one or the other based on CPU type */
+            srr0 = SPR_BOOKE_MCSRR0;
+            srr1 = SPR_BOOKE_MCSRR1;
+
+            env->spr[SPR_BOOKE_CSRR0] = env->nip;
+            env->spr[SPR_BOOKE_CSRR1] = msr;
+            break;
+        default:
+            break;
+        }
+        break;
+    case POWERPC_EXCP_DSI:       /* Data storage exception                   */
+        trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
+        break;
+    case POWERPC_EXCP_ISI:       /* Instruction storage exception            */
+        trace_ppc_excp_isi(msr, env->nip);
+        msr |= env->error_code;
+        break;
+    case POWERPC_EXCP_EXTERNAL:  /* External input                           */
+    {
+        bool lpes0;
+
+        cs = CPU(cpu);
+
+        /*
+         * Exception targeting modifiers
+         *
+         * LPES0 is supported on POWER7/8/9
+         * LPES1 is not supported (old iSeries mode)
+         *
+         * On anything else, we behave as if LPES0 is 1
+         * (externals don't alter MSR:HV)
+         */
+#if defined(TARGET_PPC64)
+        if (excp_model == POWERPC_EXCP_POWER7 ||
+            excp_model == POWERPC_EXCP_POWER8 ||
+            excp_model == POWERPC_EXCP_POWER9 ||
+            excp_model == POWERPC_EXCP_POWER10) {
+            lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
+        } else
+#endif /* defined(TARGET_PPC64) */
+        {
+            lpes0 = true;
+        }
+
+        if (!lpes0) {
+            new_msr |= (target_ulong)MSR_HVB;
+            new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+            srr0 = SPR_HSRR0;
+            srr1 = SPR_HSRR1;
+        }
+        if (env->mpic_proxy) {
+            /* IACK the IRQ on delivery */
+            env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
+        }
+        break;
+    }
+    case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
+        /* Get rS/rD and rA from faulting opcode */
+        /*
+         * Note: the opcode fields will not be set properly for a
+         * direct store load/store, but nobody cares as nobody
+         * actually uses direct store segments.
+         */
+        env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
+        break;
+    case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
+        switch (env->error_code & ~0xF) {
+        case POWERPC_EXCP_FP:
+            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
+                trace_ppc_excp_fp_ignore();
+                cs->exception_index = POWERPC_EXCP_NONE;
+                env->error_code = 0;
+                return;
+            }
+
+            /*
+             * FP exceptions always have NIP pointing to the faulting
+             * instruction, so always use store_next and claim we are
+             * precise in the MSR.
+             */
+            msr |= 0x00100000;
+            env->spr[SPR_BOOKE_ESR] = ESR_FP;
+            break;
+        case POWERPC_EXCP_INVAL:
+            trace_ppc_excp_inval(env->nip);
+            msr |= 0x00080000;
+            env->spr[SPR_BOOKE_ESR] = ESR_PIL;
+            break;
+        case POWERPC_EXCP_PRIV:
+            msr |= 0x00040000;
+            env->spr[SPR_BOOKE_ESR] = ESR_PPR;
+            break;
+        case POWERPC_EXCP_TRAP:
+            msr |= 0x00020000;
+            env->spr[SPR_BOOKE_ESR] = ESR_PTR;
+            break;
+        default:
+            /* Should never occur */
+            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
+                      env->error_code);
+            break;
+        }
+        break;
+    case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
+        lev = env->error_code;
+
+        if ((lev == 1) && cpu->vhyp) {
+            dump_hcall(env);
+        } else {
+            dump_syscall(env);
+        }
+
+        /*
+         * We need to correct the NIP which in this case is supposed
+         * to point to the next instruction
+         */
+        env->nip += 4;
+
+        /* "PAPR mode" built-in hypercall emulation */
+        if ((lev == 1) && cpu->vhyp) {
+            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_SYSCALL_VECTORED: /* scv exception                     */
+        lev = env->error_code;
+        dump_syscall(env);
+        env->nip += 4;
+        new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
+        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+
+        vector += lev * 0x20;
+
+        env->lr = env->nip;
+        env->ctr = msr;
+        break;
+    case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
+    case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
+    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
+        break;
+    case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
+        /* FIT on 4xx */
+        trace_ppc_excp_print("FIT");
+        break;
+    case POWERPC_EXCP_WDT:       /* Watchdog timer interrupt                 */
+        trace_ppc_excp_print("WDT");
+        switch (excp_model) {
+        case POWERPC_EXCP_BOOKE:
+            srr0 = SPR_BOOKE_CSRR0;
+            srr1 = SPR_BOOKE_CSRR1;
+            break;
+        default:
+            break;
+        }
+        break;
+    case POWERPC_EXCP_DTLB:      /* Data TLB error                           */
+    case POWERPC_EXCP_ITLB:      /* Instruction TLB error                    */
+        break;
+    case POWERPC_EXCP_DEBUG:     /* Debug interrupt                          */
+        if (env->flags & POWERPC_FLAG_DE) {
+            /* FIXME: choose one or the other based on CPU type */
+            srr0 = SPR_BOOKE_DSRR0;
+            srr1 = SPR_BOOKE_DSRR1;
+
+            env->spr[SPR_BOOKE_CSRR0] = env->nip;
+            env->spr[SPR_BOOKE_CSRR1] = msr;
+
+            /* DBSR already modified by caller */
+        } else {
+            cpu_abort(cs, "Debug exception triggered on unsupported model\n");
+        }
+        break;
+    case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
+        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
+        break;
+    case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
+        break;
+    case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
+        srr0 = SPR_BOOKE_CSRR0;
+        srr1 = SPR_BOOKE_CSRR1;
+        break;
+    case POWERPC_EXCP_RESET:     /* System reset exception                   */
+        /* A power-saving exception sets ME, otherwise it is unchanged */
+        if (msr_pow) {
+            /* indicate that we resumed from power save mode */
+            msr |= 0x10000;
+            new_msr |= ((target_ulong)1 << MSR_ME);
+        }
+        if (env->msr_mask & MSR_HVB) {
+            /*
+             * ISA specifies HV, but can be delivered to guest with HV
+             * clear (e.g., see FWNMI in PAPR, NMI injection in QEMU).
+             */
+            new_msr |= (target_ulong)MSR_HVB;
+        } else {
+            if (msr_pow) {
+                cpu_abort(cs, "Trying to deliver power-saving system reset "
+                          "exception %d with no HV support\n", excp);
+            }
+        }
+        break;
+    case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
+    case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
+    case POWERPC_EXCP_TRACE:     /* Trace exception                          */
+        break;
+    case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
+        msr |= env->error_code;
+        /* fall through */
+    case POWERPC_EXCP_HDECR:     /* Hypervisor decrementer exception         */
+    case POWERPC_EXCP_HDSI:      /* Hypervisor data storage exception        */
+    case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
+    case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
+    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
+    case POWERPC_EXCP_HV_EMU:
+    case POWERPC_EXCP_HVIRT:     /* Hypervisor virtualization                */
+        srr0 = SPR_HSRR0;
+        srr1 = SPR_HSRR1;
+        new_msr |= (target_ulong)MSR_HVB;
+        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+        break;
+    case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
+    case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
+    case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
+#ifdef TARGET_PPC64
+        env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56);
+#endif
+        break;
+    case POWERPC_EXCP_HV_FU:     /* Hypervisor Facility Unavailable Exception */
+#ifdef TARGET_PPC64
+        env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS);
+        srr0 = SPR_HSRR0;
+        srr1 = SPR_HSRR1;
+        new_msr |= (target_ulong)MSR_HVB;
+        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
+#endif
+        break;
+    case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
+        trace_ppc_excp_print("PIT");
+        break;
+    case POWERPC_EXCP_IFTLB:     /* Instruction fetch TLB error              */
+    case POWERPC_EXCP_DLTLB:     /* Data load TLB miss                       */
+    case POWERPC_EXCP_DSTLB:     /* Data store TLB miss                      */
+        switch (excp_model) {
+        case POWERPC_EXCP_602:
+        case POWERPC_EXCP_603:
+        case POWERPC_EXCP_G2:
+            /* Swap temporary saved registers with GPRs */
+            if (!(new_msr & ((target_ulong)1 << MSR_TGPR))) {
+                new_msr |= (target_ulong)1 << MSR_TGPR;
+                hreg_swap_gpr_tgpr(env);
+            }
+            /* fall through */
+        case POWERPC_EXCP_7x5:
+            ppc_excp_debug_sw_tlb(env, excp);
+
+            msr |= env->crf[0] << 28;
+            msr |= env->error_code; /* key, D/I, S/L bits */
+            /* Set way using a LRU mechanism */
+            msr |= ((env->last_way + 1) & (env->nb_ways - 1)) << 17;
+            break;
+        default:
+            cpu_abort(cs, "Invalid TLB miss exception\n");
+            break;
+        }
+        break;
+    case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
+    case POWERPC_EXCP_EFPRI:     /* Embedded floating-point round interrupt  */
+    case POWERPC_EXCP_EPERFM:    /* Embedded performance monitor interrupt   */
+    case POWERPC_EXCP_IO:        /* IO error exception                       */
+    case POWERPC_EXCP_RUNM:      /* Run mode exception                       */
+    case POWERPC_EXCP_EMUL:      /* Emulation trap exception                 */
+    case POWERPC_EXCP_FPA:       /* Floating-point assist exception          */
+    case POWERPC_EXCP_DABR:      /* Data address breakpoint                  */
+    case POWERPC_EXCP_IABR:      /* Instruction address breakpoint           */
+    case POWERPC_EXCP_SMI:       /* System management interrupt              */
+    case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
+    case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
+    case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
+    case POWERPC_EXCP_SOFTP:     /* Soft patch exception                     */
+    case POWERPC_EXCP_MAINT:     /* Maintenance exception                    */
+    case POWERPC_EXCP_MEXTBR:    /* Maskable external breakpoint             */
+    case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint         */
+        cpu_abort(cs, "%s exception not implemented\n",
+                  powerpc_excp_name(excp));
+        break;
+    default:
+    excp_invalid:
+        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
+        break;
+    }
+
+    /* Sanity check */
+    if (!(env->msr_mask & MSR_HVB)) {
+        if (new_msr & MSR_HVB) {
+            cpu_abort(cs, "Trying to deliver HV exception (MSR) %d with "
+                      "no HV support\n", excp);
+        }
+        if (srr0 == SPR_HSRR0) {
+            cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
+                      "no HV support\n", excp);
+        }
+    }
+
+    /*
+     * Sort out endianness of interrupt, this differs depending on the
+     * CPU, the HV mode, etc...
+     */
+    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
+        new_msr |= (target_ulong)1 << MSR_LE;
+    }
+
+#if defined(TARGET_PPC64)
+    if (excp_model == POWERPC_EXCP_BOOKE) {
+        if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
+            /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
+            new_msr |= (target_ulong)1 << MSR_CM;
+        } else {
+            vector = (uint32_t)vector;
+        }
+    } else {
+        if (!msr_isf && !mmu_is_64bit(env->mmu_model)) {
+            vector = (uint32_t)vector;
+        } else {
+            new_msr |= (target_ulong)1 << MSR_SF;
+        }
+    }
+#endif
+
+    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
+        /* Save PC */
+        env->spr[srr0] = env->nip;
+
+        /* Save MSR */
+        env->spr[srr1] = msr;
+    }
+
+    /* This can update new_msr and vector if AIL applies */
+    ppc_excp_apply_ail(cpu, excp_model, excp, msr, &new_msr, &vector);
+
+    powerpc_set_excp_state(cpu, vector, new_msr);
+}
+
 /*
  * Note that this function should be greatly optimized when called
  * with a constant excp, from ppc_hw_interrupt
@@ -872,6 +1343,9 @@ static void powerpc_excp(PowerPCCPU *cpu, int excp)
     CPUPPCState *env = &cpu->env;
 
     switch (env->excp_model) {
+    case POWERPC_EXCP_40x:
+        powerpc_excp_40x(cpu, excp);
+        break;
     default:
         powerpc_excp_legacy(cpu, excp);
     }
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/8] squash target/ppc: Introduce powerpc_excp_40x
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (2 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-10 18:51   ` BALATON Zoltan
  2022-01-10 18:15 ` [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup Fabiano Rosas
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

Introduce a new exception dispatcher for 40x CPUs.

Differences from the generic powerpc_excp code:

- Not BookE, so some MSR bits are cleared at interrupt dispatch;
- No MSR_HV or MSR_LE;
- No power saving states;
- No Hypervisor Emulation Assistance;
- Not 64 bits;
- No System call vectored;
- No Interrupts Little Endian;
- No Alternate Interrupt Location.

Exceptions used:

POWERPC_EXCP_ALIGN
POWERPC_EXCP_APU
POWERPC_EXCP_CRITICAL
POWERPC_EXCP_DEBUG
POWERPC_EXCP_DSI
POWERPC_EXCP_DTLB
POWERPC_EXCP_EXTERNAL
POWERPC_EXCP_FIT
POWERPC_EXCP_FPU
POWERPC_EXCP_ISI
POWERPC_EXCP_ITLB
POWERPC_EXCP_MCHECK
POWERPC_EXCP_PIT
POWERPC_EXCP_PROGRAM
POWERPC_EXCP_SYSCALL
POWERPC_EXCP_WDT

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 165 +++------------------------------------
 1 file changed, 13 insertions(+), 152 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 12ab5e1b34..1d997c4d6b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -409,54 +409,18 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
                   excp, env->error_code);
 
     /* new srr1 value excluding must-be-zero bits */
-    if (excp_model == POWERPC_EXCP_BOOKE) {
-        msr = env->msr;
-    } else {
-        msr = env->msr & ~0x783f0000ULL;
-    }
+    msr = env->msr & ~0x783f0000ULL;
 
     /*
-     * new interrupt handler msr preserves existing HV and ME unless
-     * explicitly overriden
+     * new interrupt handler msr preserves existing ME unless
+     * explicitly overriden.
      */
-    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
+    new_msr = env->msr & (((target_ulong)1 << MSR_ME));
 
     /* target registers */
     srr0 = SPR_SRR0;
     srr1 = SPR_SRR1;
 
-    /*
-     * check for special resume at 0x100 from doze/nap/sleep/winkle on
-     * P7/P8/P9
-     */
-    if (env->resume_as_sreset) {
-        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
-    }
-
-    /*
-     * Hypervisor emulation assistance interrupt only exists on server
-     * arch 2.05 server or later. We also don't want to generate it if
-     * we don't have HVB in msr_mask (PAPR mode).
-     */
-    if (excp == POWERPC_EXCP_HV_EMU
-#if defined(TARGET_PPC64)
-        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
-#endif /* defined(TARGET_PPC64) */
-
-    ) {
-        excp = POWERPC_EXCP_PROGRAM;
-    }
-
-#ifdef TARGET_PPC64
-    /*
-     * SPEU and VPU share the same IVOR but they exist in different
-     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
-     */
-    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
-        excp = POWERPC_EXCP_SPEU;
-    }
-#endif
-
     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
         cpu_abort(cs, "Raised an exception without defined vector %d\n",
@@ -581,6 +545,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
          */
         env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
         break;
+    case POWERPC_EXCP_HV_EMU:
+        /*
+         * Hypervisor emulation assistance interrupt only exists on server
+         * arch 2.05 server or later.
+         */
     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
         switch (env->error_code & ~0xF) {
         case POWERPC_EXCP_FP:
@@ -645,22 +614,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
             new_msr |= (target_ulong)MSR_HVB;
         }
         break;
-    case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception                     */
-        lev = env->error_code;
-        dump_syscall(env);
-        env->nip += 4;
-        new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
-        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
-
-        vector += lev * 0x20;
-
-        env->lr = env->nip;
-        env->ctr = msr;
-        break;
     case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
     case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
-    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
-        break;
     case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
         /* FIT on 4xx */
         trace_ppc_excp_print("FIT");
@@ -693,70 +648,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
             cpu_abort(cs, "Debug exception triggered on unsupported model\n");
         }
         break;
-    case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
-        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
-        break;
-    case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
-        break;
-    case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
-        srr0 = SPR_BOOKE_CSRR0;
-        srr1 = SPR_BOOKE_CSRR1;
-        break;
-    case POWERPC_EXCP_RESET:     /* System reset exception                   */
-        /* A power-saving exception sets ME, otherwise it is unchanged */
-        if (msr_pow) {
-            /* indicate that we resumed from power save mode */
-            msr |= 0x10000;
-            new_msr |= ((target_ulong)1 << MSR_ME);
-        }
-        if (env->msr_mask & MSR_HVB) {
-            /*
-             * ISA specifies HV, but can be delivered to guest with HV
-             * clear (e.g., see FWNMI in PAPR, NMI injection in QEMU).
-             */
-            new_msr |= (target_ulong)MSR_HVB;
-        } else {
-            if (msr_pow) {
-                cpu_abort(cs, "Trying to deliver power-saving system reset "
-                          "exception %d with no HV support\n", excp);
-            }
-        }
-        break;
-    case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
-    case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
-    case POWERPC_EXCP_TRACE:     /* Trace exception                          */
-        break;
-    case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
-        msr |= env->error_code;
-        /* fall through */
-    case POWERPC_EXCP_HDECR:     /* Hypervisor decrementer exception         */
-    case POWERPC_EXCP_HDSI:      /* Hypervisor data storage exception        */
-    case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
-    case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
-    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
-    case POWERPC_EXCP_HV_EMU:
-    case POWERPC_EXCP_HVIRT:     /* Hypervisor virtualization                */
-        srr0 = SPR_HSRR0;
-        srr1 = SPR_HSRR1;
-        new_msr |= (target_ulong)MSR_HVB;
-        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
-        break;
-    case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
-    case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
-    case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
-#ifdef TARGET_PPC64
-        env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56);
-#endif
-        break;
-    case POWERPC_EXCP_HV_FU:     /* Hypervisor Facility Unavailable Exception */
-#ifdef TARGET_PPC64
-        env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS);
-        srr0 = SPR_HSRR0;
-        srr1 = SPR_HSRR1;
-        new_msr |= (target_ulong)MSR_HVB;
-        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
-#endif
-        break;
     case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
         trace_ppc_excp_print("PIT");
         break;
@@ -824,41 +715,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         }
     }
 
-    /*
-     * Sort out endianness of interrupt, this differs depending on the
-     * CPU, the HV mode, etc...
-     */
-    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
-        new_msr |= (target_ulong)1 << MSR_LE;
-    }
+    /* Save PC */
+    env->spr[srr0] = env->nip;
 
-#if defined(TARGET_PPC64)
-    if (excp_model == POWERPC_EXCP_BOOKE) {
-        if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
-            /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
-            new_msr |= (target_ulong)1 << MSR_CM;
-        } else {
-            vector = (uint32_t)vector;
-        }
-    } else {
-        if (!msr_isf && !mmu_is_64bit(env->mmu_model)) {
-            vector = (uint32_t)vector;
-        } else {
-            new_msr |= (target_ulong)1 << MSR_SF;
-        }
-    }
-#endif
-
-    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
-        /* Save PC */
-        env->spr[srr0] = env->nip;
-
-        /* Save MSR */
-        env->spr[srr1] = msr;
-    }
-
-    /* This can update new_msr and vector if AIL applies */
-    ppc_excp_apply_ail(cpu, excp_model, excp, msr, &new_msr, &vector);
+    /* Save MSR */
+    env->spr[srr1] = msr;
 
     powerpc_set_excp_state(cpu, vector, new_msr);
 }
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (3 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 4/8] squash " Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:26   ` David Gibson
  2022-01-10 18:15 ` [PATCH 6/8] target/ppc: 405: Machine check exception cleanup Fabiano Rosas
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

In powerpc_excp_40x the Critical exception is now for 405 only, so we
can remove the BookE and G2 blocks.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 1d997c4d6b..fecf4d5a4e 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -431,20 +431,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
 
     switch (excp) {
     case POWERPC_EXCP_CRITICAL:    /* Critical input                         */
-        switch (excp_model) {
-        case POWERPC_EXCP_40x:
-            srr0 = SPR_40x_SRR2;
-            srr1 = SPR_40x_SRR3;
-            break;
-        case POWERPC_EXCP_BOOKE:
-            srr0 = SPR_BOOKE_CSRR0;
-            srr1 = SPR_BOOKE_CSRR1;
-            break;
-        case POWERPC_EXCP_G2:
-            break;
-        default:
-            goto excp_invalid;
-        }
+        srr0 = SPR_40x_SRR2;
+        srr1 = SPR_40x_SRR3;
         break;
     case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
         if (msr_me == 0) {
@@ -698,7 +686,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
                   powerpc_excp_name(excp));
         break;
     default:
-    excp_invalid:
         cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
         break;
     }
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 6/8] target/ppc: 405: Machine check exception cleanup
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (4 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:26   ` David Gibson
  2022-01-10 18:15 ` [PATCH 7/8] target/ppc: 405: External " Fabiano Rosas
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

powerpc_excp_40x applies only to the 405, so remove HV code and
references to BookE.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 26 ++------------------------
 1 file changed, 2 insertions(+), 24 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index fecf4d5a4e..82ade5d7bd 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -449,34 +449,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
             cs->halted = 1;
             cpu_interrupt_exittb(cs);
         }
-        if (env->msr_mask & MSR_HVB) {
-            /*
-             * ISA specifies HV, but can be delivered to guest with HV
-             * clear (e.g., see FWNMI in PAPR).
-             */
-            new_msr |= (target_ulong)MSR_HVB;
-        }
 
         /* machine check exceptions don't have ME set */
         new_msr &= ~((target_ulong)1 << MSR_ME);
 
-        /* XXX: should also have something loaded in DAR / DSISR */
-        switch (excp_model) {
-        case POWERPC_EXCP_40x:
-            srr0 = SPR_40x_SRR2;
-            srr1 = SPR_40x_SRR3;
-            break;
-        case POWERPC_EXCP_BOOKE:
-            /* FIXME: choose one or the other based on CPU type */
-            srr0 = SPR_BOOKE_MCSRR0;
-            srr1 = SPR_BOOKE_MCSRR1;
-
-            env->spr[SPR_BOOKE_CSRR0] = env->nip;
-            env->spr[SPR_BOOKE_CSRR1] = msr;
-            break;
-        default:
-            break;
-        }
+        srr0 = SPR_40x_SRR2;
+        srr1 = SPR_40x_SRR3;
         break;
     case POWERPC_EXCP_DSI:       /* Data storage exception                   */
         trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 7/8] target/ppc: 405: External exception cleanup
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (5 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 6/8] target/ppc: 405: Machine check exception cleanup Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:28   ` David Gibson
  2022-01-10 18:15 ` [PATCH 8/8] target/ppc: 405: System call " Fabiano Rosas
  2022-01-11  8:37 ` [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Cédric Le Goater
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

405 has no MSR_HV and EPR is BookE only so we can remove it all.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 37 -------------------------------------
 1 file changed, 37 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 82ade5d7bd..f7b9af5065 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -464,44 +464,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         msr |= env->error_code;
         break;
     case POWERPC_EXCP_EXTERNAL:  /* External input                           */
-    {
-        bool lpes0;
-
-        cs = CPU(cpu);
-
-        /*
-         * Exception targeting modifiers
-         *
-         * LPES0 is supported on POWER7/8/9
-         * LPES1 is not supported (old iSeries mode)
-         *
-         * On anything else, we behave as if LPES0 is 1
-         * (externals don't alter MSR:HV)
-         */
-#if defined(TARGET_PPC64)
-        if (excp_model == POWERPC_EXCP_POWER7 ||
-            excp_model == POWERPC_EXCP_POWER8 ||
-            excp_model == POWERPC_EXCP_POWER9 ||
-            excp_model == POWERPC_EXCP_POWER10) {
-            lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
-        } else
-#endif /* defined(TARGET_PPC64) */
-        {
-            lpes0 = true;
-        }
-
-        if (!lpes0) {
-            new_msr |= (target_ulong)MSR_HVB;
-            new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
-            srr0 = SPR_HSRR0;
-            srr1 = SPR_HSRR1;
-        }
-        if (env->mpic_proxy) {
-            /* IACK the IRQ on delivery */
-            env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
-        }
         break;
-    }
     case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
         /* Get rS/rD and rA from faulting opcode */
         /*
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 8/8] target/ppc: 405: System call exception cleanup
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (6 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 7/8] target/ppc: 405: External " Fabiano Rosas
@ 2022-01-10 18:15 ` Fabiano Rosas
  2022-01-11  2:31   ` David Gibson
  2022-01-11  8:37 ` [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Cédric Le Goater
  8 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 18:15 UTC (permalink / raw)
  To: qemu-devel; +Cc: richard.henderson, danielhb413, qemu-ppc, clg, david

There's no sc 1.

We also only used env->nip because of the vhyp code, so change to
'vector' now.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/excp_helper.c | 23 +++--------------------
 1 file changed, 3 insertions(+), 20 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index f7b9af5065..ab298d0d8f 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -398,7 +398,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
     CPUPPCState *env = &cpu->env;
     int excp_model = env->excp_model;
     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);
@@ -518,30 +518,13 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
         }
         break;
     case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
-        lev = env->error_code;
-
-        if ((lev == 1) && cpu->vhyp) {
-            dump_hcall(env);
-        } else {
-            dump_syscall(env);
-        }
+        dump_syscall(env);
 
         /*
          * We need to correct the NIP which in this case is supposed
          * to point to the next instruction
          */
-        env->nip += 4;
-
-        /* "PAPR mode" built-in hypercall emulation */
-        if ((lev == 1) && cpu->vhyp) {
-            PPCVirtualHypervisorClass *vhc =
-                PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
-            vhc->hypercall(cpu->vhyp, cpu);
-            return;
-        }
-        if (lev == 1) {
-            new_msr |= (target_ulong)MSR_HVB;
-        }
+        vector += 4;
         break;
     case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
     case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
-- 
2.33.1



^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/8] squash target/ppc: Introduce powerpc_excp_40x
  2022-01-10 18:15 ` [PATCH 4/8] squash " Fabiano Rosas
@ 2022-01-10 18:51   ` BALATON Zoltan
  2022-01-10 19:00     ` Fabiano Rosas
  0 siblings, 1 reply; 25+ messages in thread
From: BALATON Zoltan @ 2022-01-10 18:51 UTC (permalink / raw)
  To: Fabiano Rosas
  Cc: danielhb413, richard.henderson, qemu-devel, qemu-ppc, clg, david

On Mon, 10 Jan 2022, Fabiano Rosas wrote:
> Introduce a new exception dispatcher for 40x CPUs.
>
> Differences from the generic powerpc_excp code:
>
> - Not BookE, so some MSR bits are cleared at interrupt dispatch;
> - No MSR_HV or MSR_LE;
> - No power saving states;
> - No Hypervisor Emulation Assistance;
> - Not 64 bits;
> - No System call vectored;
> - No Interrupts Little Endian;
> - No Alternate Interrupt Location.
>
> Exceptions used:
>
> POWERPC_EXCP_ALIGN
> POWERPC_EXCP_APU
> POWERPC_EXCP_CRITICAL
> POWERPC_EXCP_DEBUG
> POWERPC_EXCP_DSI
> POWERPC_EXCP_DTLB
> POWERPC_EXCP_EXTERNAL
> POWERPC_EXCP_FIT
> POWERPC_EXCP_FPU
> POWERPC_EXCP_ISI
> POWERPC_EXCP_ITLB
> POWERPC_EXCP_MCHECK
> POWERPC_EXCP_PIT
> POWERPC_EXCP_PROGRAM
> POWERPC_EXCP_SYSCALL
> POWERPC_EXCP_WDT
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> target/ppc/excp_helper.c | 165 +++------------------------------------
> 1 file changed, 13 insertions(+), 152 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 12ab5e1b34..1d997c4d6b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -409,54 +409,18 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>                   excp, env->error_code);
>
>     /* new srr1 value excluding must-be-zero bits */
> -    if (excp_model == POWERPC_EXCP_BOOKE) {
> -        msr = env->msr;
> -    } else {
> -        msr = env->msr & ~0x783f0000ULL;
> -    }
> +    msr = env->msr & ~0x783f0000ULL;
>
>     /*
> -     * new interrupt handler msr preserves existing HV and ME unless
> -     * explicitly overriden
> +     * new interrupt handler msr preserves existing ME unless
> +     * explicitly overriden.
>      */
> -    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
> +    new_msr = env->msr & (((target_ulong)1 << MSR_ME));
>
>     /* target registers */
>     srr0 = SPR_SRR0;
>     srr1 = SPR_SRR1;
>
> -    /*
> -     * check for special resume at 0x100 from doze/nap/sleep/winkle on
> -     * P7/P8/P9
> -     */
> -    if (env->resume_as_sreset) {
> -        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
> -    }
> -
> -    /*
> -     * Hypervisor emulation assistance interrupt only exists on server
> -     * arch 2.05 server or later. We also don't want to generate it if
> -     * we don't have HVB in msr_mask (PAPR mode).
> -     */
> -    if (excp == POWERPC_EXCP_HV_EMU
> -#if defined(TARGET_PPC64)
> -        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
> -#endif /* defined(TARGET_PPC64) */
> -
> -    ) {
> -        excp = POWERPC_EXCP_PROGRAM;
> -    }
> -
> -#ifdef TARGET_PPC64
> -    /*
> -     * SPEU and VPU share the same IVOR but they exist in different
> -     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
> -     */
> -    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
> -        excp = POWERPC_EXCP_SPEU;
> -    }
> -#endif
> -
>     vector = env->excp_vectors[excp];
>     if (vector == (target_ulong)-1ULL) {
>         cpu_abort(cs, "Raised an exception without defined vector %d\n",
> @@ -581,6 +545,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>          */
>         env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
>         break;
> +    case POWERPC_EXCP_HV_EMU:
> +        /*
> +         * Hypervisor emulation assistance interrupt only exists on server
> +         * arch 2.05 server or later.
> +         */
>     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
>         switch (env->error_code & ~0xF) {
>         case POWERPC_EXCP_FP:
> @@ -645,22 +614,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>             new_msr |= (target_ulong)MSR_HVB;
>         }
>         break;
> -    case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception                     */
> -        lev = env->error_code;
> -        dump_syscall(env);
> -        env->nip += 4;
> -        new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
> -        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> -
> -        vector += lev * 0x20;
> -
> -        env->lr = env->nip;
> -        env->ctr = msr;
> -        break;
>     case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>     case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
> -    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
> -        break;

Removing the break here makes FPU and APU fall through to FIT. Is that 
intentional?

Regards,
BALATON Zoltan

>     case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
>         /* FIT on 4xx */
>         trace_ppc_excp_print("FIT");
> @@ -693,70 +648,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>             cpu_abort(cs, "Debug exception triggered on unsupported model\n");
>         }
>         break;
> -    case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
> -        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
> -        break;
> -    case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
> -        break;
> -    case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
> -        srr0 = SPR_BOOKE_CSRR0;
> -        srr1 = SPR_BOOKE_CSRR1;
> -        break;
> -    case POWERPC_EXCP_RESET:     /* System reset exception                   */
> -        /* A power-saving exception sets ME, otherwise it is unchanged */
> -        if (msr_pow) {
> -            /* indicate that we resumed from power save mode */
> -            msr |= 0x10000;
> -            new_msr |= ((target_ulong)1 << MSR_ME);
> -        }
> -        if (env->msr_mask & MSR_HVB) {
> -            /*
> -             * ISA specifies HV, but can be delivered to guest with HV
> -             * clear (e.g., see FWNMI in PAPR, NMI injection in QEMU).
> -             */
> -            new_msr |= (target_ulong)MSR_HVB;
> -        } else {
> -            if (msr_pow) {
> -                cpu_abort(cs, "Trying to deliver power-saving system reset "
> -                          "exception %d with no HV support\n", excp);
> -            }
> -        }
> -        break;
> -    case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
> -    case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
> -    case POWERPC_EXCP_TRACE:     /* Trace exception                          */
> -        break;
> -    case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
> -        msr |= env->error_code;
> -        /* fall through */
> -    case POWERPC_EXCP_HDECR:     /* Hypervisor decrementer exception         */
> -    case POWERPC_EXCP_HDSI:      /* Hypervisor data storage exception        */
> -    case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
> -    case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
> -    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
> -    case POWERPC_EXCP_HV_EMU:
> -    case POWERPC_EXCP_HVIRT:     /* Hypervisor virtualization                */
> -        srr0 = SPR_HSRR0;
> -        srr1 = SPR_HSRR1;
> -        new_msr |= (target_ulong)MSR_HVB;
> -        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> -        break;
> -    case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
> -    case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
> -    case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
> -#ifdef TARGET_PPC64
> -        env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56);
> -#endif
> -        break;
> -    case POWERPC_EXCP_HV_FU:     /* Hypervisor Facility Unavailable Exception */
> -#ifdef TARGET_PPC64
> -        env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS);
> -        srr0 = SPR_HSRR0;
> -        srr1 = SPR_HSRR1;
> -        new_msr |= (target_ulong)MSR_HVB;
> -        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> -#endif
> -        break;
>     case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
>         trace_ppc_excp_print("PIT");
>         break;
> @@ -824,41 +715,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>         }
>     }
>
> -    /*
> -     * Sort out endianness of interrupt, this differs depending on the
> -     * CPU, the HV mode, etc...
> -     */
> -    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
> -        new_msr |= (target_ulong)1 << MSR_LE;
> -    }
> +    /* Save PC */
> +    env->spr[srr0] = env->nip;
>
> -#if defined(TARGET_PPC64)
> -    if (excp_model == POWERPC_EXCP_BOOKE) {
> -        if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
> -            /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
> -            new_msr |= (target_ulong)1 << MSR_CM;
> -        } else {
> -            vector = (uint32_t)vector;
> -        }
> -    } else {
> -        if (!msr_isf && !mmu_is_64bit(env->mmu_model)) {
> -            vector = (uint32_t)vector;
> -        } else {
> -            new_msr |= (target_ulong)1 << MSR_SF;
> -        }
> -    }
> -#endif
> -
> -    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
> -        /* Save PC */
> -        env->spr[srr0] = env->nip;
> -
> -        /* Save MSR */
> -        env->spr[srr1] = msr;
> -    }
> -
> -    /* This can update new_msr and vector if AIL applies */
> -    ppc_excp_apply_ail(cpu, excp_model, excp, msr, &new_msr, &vector);
> +    /* Save MSR */
> +    env->spr[srr1] = msr;
>
>     powerpc_set_excp_state(cpu, vector, new_msr);
> }
>


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/8] squash target/ppc: Introduce powerpc_excp_40x
  2022-01-10 18:51   ` BALATON Zoltan
@ 2022-01-10 19:00     ` Fabiano Rosas
  0 siblings, 0 replies; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-10 19:00 UTC (permalink / raw)
  To: BALATON Zoltan
  Cc: danielhb413, richard.henderson, qemu-devel, qemu-ppc, clg, david

BALATON Zoltan <balaton@eik.bme.hu> writes:

> On Mon, 10 Jan 2022, Fabiano Rosas wrote:
>> Introduce a new exception dispatcher for 40x CPUs.
>>
>> Differences from the generic powerpc_excp code:
>>
>> - Not BookE, so some MSR bits are cleared at interrupt dispatch;
>> - No MSR_HV or MSR_LE;
>> - No power saving states;
>> - No Hypervisor Emulation Assistance;
>> - Not 64 bits;
>> - No System call vectored;
>> - No Interrupts Little Endian;
>> - No Alternate Interrupt Location.
>>
>> Exceptions used:
>>
>> POWERPC_EXCP_ALIGN
>> POWERPC_EXCP_APU
>> POWERPC_EXCP_CRITICAL
>> POWERPC_EXCP_DEBUG
>> POWERPC_EXCP_DSI
>> POWERPC_EXCP_DTLB
>> POWERPC_EXCP_EXTERNAL
>> POWERPC_EXCP_FIT
>> POWERPC_EXCP_FPU
>> POWERPC_EXCP_ISI
>> POWERPC_EXCP_ITLB
>> POWERPC_EXCP_MCHECK
>> POWERPC_EXCP_PIT
>> POWERPC_EXCP_PROGRAM
>> POWERPC_EXCP_SYSCALL
>> POWERPC_EXCP_WDT
>>
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>> target/ppc/excp_helper.c | 165 +++------------------------------------
>> 1 file changed, 13 insertions(+), 152 deletions(-)
>>
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 12ab5e1b34..1d997c4d6b 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -409,54 +409,18 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>                   excp, env->error_code);
>>
>>     /* new srr1 value excluding must-be-zero bits */
>> -    if (excp_model == POWERPC_EXCP_BOOKE) {
>> -        msr = env->msr;
>> -    } else {
>> -        msr = env->msr & ~0x783f0000ULL;
>> -    }
>> +    msr = env->msr & ~0x783f0000ULL;
>>
>>     /*
>> -     * new interrupt handler msr preserves existing HV and ME unless
>> -     * explicitly overriden
>> +     * new interrupt handler msr preserves existing ME unless
>> +     * explicitly overriden.
>>      */
>> -    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
>> +    new_msr = env->msr & (((target_ulong)1 << MSR_ME));
>>
>>     /* target registers */
>>     srr0 = SPR_SRR0;
>>     srr1 = SPR_SRR1;
>>
>> -    /*
>> -     * check for special resume at 0x100 from doze/nap/sleep/winkle on
>> -     * P7/P8/P9
>> -     */
>> -    if (env->resume_as_sreset) {
>> -        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
>> -    }
>> -
>> -    /*
>> -     * Hypervisor emulation assistance interrupt only exists on server
>> -     * arch 2.05 server or later. We also don't want to generate it if
>> -     * we don't have HVB in msr_mask (PAPR mode).
>> -     */
>> -    if (excp == POWERPC_EXCP_HV_EMU
>> -#if defined(TARGET_PPC64)
>> -        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
>> -#endif /* defined(TARGET_PPC64) */
>> -
>> -    ) {
>> -        excp = POWERPC_EXCP_PROGRAM;
>> -    }
>> -
>> -#ifdef TARGET_PPC64
>> -    /*
>> -     * SPEU and VPU share the same IVOR but they exist in different
>> -     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
>> -     */
>> -    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
>> -        excp = POWERPC_EXCP_SPEU;
>> -    }
>> -#endif
>> -
>>     vector = env->excp_vectors[excp];
>>     if (vector == (target_ulong)-1ULL) {
>>         cpu_abort(cs, "Raised an exception without defined vector %d\n",
>> @@ -581,6 +545,11 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>          */
>>         env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
>>         break;
>> +    case POWERPC_EXCP_HV_EMU:
>> +        /*
>> +         * Hypervisor emulation assistance interrupt only exists on server
>> +         * arch 2.05 server or later.
>> +         */
>>     case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
>>         switch (env->error_code & ~0xF) {
>>         case POWERPC_EXCP_FP:
>> @@ -645,22 +614,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>>             new_msr |= (target_ulong)MSR_HVB;
>>         }
>>         break;
>> -    case POWERPC_EXCP_SYSCALL_VECTORED: /* scv exception                     */
>> -        lev = env->error_code;
>> -        dump_syscall(env);
>> -        env->nip += 4;
>> -        new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
>> -        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
>> -
>> -        vector += lev * 0x20;
>> -
>> -        env->lr = env->nip;
>> -        env->ctr = msr;
>> -        break;
>>     case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>>     case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
>> -    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
>> -        break;
>
> Removing the break here makes FPU and APU fall through to FIT. Is that 
> intentional?

No, that is a mistake indeed. Thanks.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
  2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
@ 2022-01-11  2:04   ` David Gibson
  2022-01-11  2:07     ` David Gibson
  2022-01-17 21:12   ` Fabiano Rosas
  1 sibling, 1 reply; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:04 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 1890 bytes --]

On Mon, Jan 10, 2022 at 03:15:39PM -0300, Fabiano Rosas wrote:
> Some bits described in the user manual are missing from msr_mask. Add
> them.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index e30e86fe9d..a50ddaeaae 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
>                         PPC_MEM_SYNC | PPC_MEM_EIEIO |
>                         PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC |
>                         PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP;
> -    pcc->msr_mask = (1ull << MSR_POW) |
> +    pcc->msr_mask = (1ull << MSR_AP) |
> +                    (1ull << MSR_POW) |

If I'm looking at things correctly, the "MSR_POW" bit on 405 is
actually called "MSR_WE", which appears related, but not quite
identical.  I think it would be good to introduce a new define to get
it a name matching the user manual.

>                      (1ull << MSR_CE) |
>                      (1ull << MSR_EE) |
>                      (1ull << MSR_PR) |
>                      (1ull << MSR_FP) |
> +                    (1ull << MSR_ME) |
>                      (1ull << MSR_DWE) |
>                      (1ull << MSR_DE) |
> +                    (1ull << MSR_FE1) |
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);
> +
>      pcc->mmu_model = POWERPC_MMU_SOFT_4xx;
>      pcc->excp_model = POWERPC_EXCP_40x;
>      pcc->bus_model = PPC_FLAGS_INPUT_405;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
  2022-01-11  2:04   ` David Gibson
@ 2022-01-11  2:07     ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:07 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 2150 bytes --]

On Tue, Jan 11, 2022 at 01:04:14PM +1100, David Gibson wrote:
> On Mon, Jan 10, 2022 at 03:15:39PM -0300, Fabiano Rosas wrote:
> > Some bits described in the user manual are missing from msr_mask. Add
> > them.
> > 
> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> >  target/ppc/cpu_init.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index e30e86fe9d..a50ddaeaae 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
> >                         PPC_MEM_SYNC | PPC_MEM_EIEIO |
> >                         PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC |
> >                         PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP;
> > -    pcc->msr_mask = (1ull << MSR_POW) |
> > +    pcc->msr_mask = (1ull << MSR_AP) |
> > +                    (1ull << MSR_POW) |
> 
> If I'm looking at things correctly, the "MSR_POW" bit on 405 is
> actually called "MSR_WE", which appears related, but not quite
> identical.  I think it would be good to introduce a new define to get
> it a name matching the user manual.

Also, it looks like this is still missing the MSR[APE] bit (in the
same place as the MSR_KEY bit on 603e).

> >                      (1ull << MSR_CE) |
> >                      (1ull << MSR_EE) |
> >                      (1ull << MSR_PR) |
> >                      (1ull << MSR_FP) |
> > +                    (1ull << MSR_ME) |
> >                      (1ull << MSR_DWE) |
> >                      (1ull << MSR_DE) |
> > +                    (1ull << MSR_FE1) |
> >                      (1ull << MSR_IR) |
> >                      (1ull << MSR_DR);
> > +
> >      pcc->mmu_model = POWERPC_MMU_SOFT_4xx;
> >      pcc->excp_model = POWERPC_EXCP_40x;
> >      pcc->bus_model = PPC_FLAGS_INPUT_405;
> 



-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] target/ppc: 405: Add missing exception handlers
  2022-01-10 18:15 ` [PATCH 2/8] target/ppc: 405: Add missing exception handlers Fabiano Rosas
@ 2022-01-11  2:10   ` David Gibson
  2022-01-14 21:46     ` Fabiano Rosas
  0 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:10 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 1642 bytes --]

On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index a50ddaeaae..9097948e67 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;

I have a vague recollection from my days of working on 405 that there
may have been something funky with FP emulation on there: e.g. FP
instructions causing 0x700 program interrupts instead of FP unavailble
interrupts or something.

I might be remembering incorrectly - the manual does seem to imply
that 0x800 FP unavailable is there as normal, but it might be worth
double checking this (against real hardware, if possible).

>      env->excp_vectors[POWERPC_EXCP_SYSCALL]  = 0x00000C00;
> +    env->excp_vectors[POWERPC_EXCP_APU]      = 0x00000F20;
>      env->excp_vectors[POWERPC_EXCP_PIT]      = 0x00001000;
>      env->excp_vectors[POWERPC_EXCP_FIT]      = 0x00001010;
>      env->excp_vectors[POWERPC_EXCP_WDT]      = 0x00001020;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x
  2022-01-10 18:15 ` [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x Fabiano Rosas
@ 2022-01-11  2:20   ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:20 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 20811 bytes --]

On Mon, Jan 10, 2022 at 03:15:41PM -0300, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
> I copied powerpc_excp_legacy verbatim in this commit so the next
> one has a clean diff. We can squash both commits before merging.

I think copying it verbatim then simplifying is a good idea, even for
the final version, but the commit messages would need to be updated a
bit.  I'd suggest:

 * "Introduce powerpc_excp_40x" with a message saying it's just a copy
   of the legacy function for now

then

 * "Simplify "powerpc_excp_40x", where you cut out the non-405 stuff

> ---
>  target/ppc/excp_helper.c | 474 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 474 insertions(+)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index bc646c67a0..12ab5e1b34 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -392,6 +392,477 @@ static void powerpc_set_excp_state(PowerPCCPU *cpu,
>      check_tlb_flush(env, false);
>  }
>  
> +static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
> +{
> +    CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
> +    int excp_model = env->excp_model;
> +    target_ulong msr, new_msr, vector;
> +    int srr0, srr1, lev = -1;
> +
> +    if (excp <= POWERPC_EXCP_NONE || excp >= POWERPC_EXCP_NB) {
> +        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> +    }
> +
> +    qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> +                  " => %s (%d) error=%02x\n", env->nip, powerpc_excp_name(excp),
> +                  excp, env->error_code);
> +
> +    /* new srr1 value excluding must-be-zero bits */
> +    if (excp_model == POWERPC_EXCP_BOOKE) {
> +        msr = env->msr;
> +    } else {
> +        msr = env->msr & ~0x783f0000ULL;
> +    }
> +
> +    /*
> +     * new interrupt handler msr preserves existing HV and ME unless
> +     * explicitly overriden
> +     */
> +    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
> +
> +    /* target registers */
> +    srr0 = SPR_SRR0;
> +    srr1 = SPR_SRR1;
> +
> +    /*
> +     * check for special resume at 0x100 from doze/nap/sleep/winkle on
> +     * P7/P8/P9
> +     */
> +    if (env->resume_as_sreset) {
> +        excp = powerpc_reset_wakeup(cs, env, excp, &msr);
> +    }
> +
> +    /*
> +     * Hypervisor emulation assistance interrupt only exists on server
> +     * arch 2.05 server or later. We also don't want to generate it if
> +     * we don't have HVB in msr_mask (PAPR mode).
> +     */
> +    if (excp == POWERPC_EXCP_HV_EMU
> +#if defined(TARGET_PPC64)
> +        && !(mmu_is_64bit(env->mmu_model) && (env->msr_mask & MSR_HVB))
> +#endif /* defined(TARGET_PPC64) */
> +
> +    ) {
> +        excp = POWERPC_EXCP_PROGRAM;
> +    }
> +
> +#ifdef TARGET_PPC64
> +    /*
> +     * SPEU and VPU share the same IVOR but they exist in different
> +     * processors. SPEU is e500v1/2 only and VPU is e6500 only.
> +     */
> +    if (excp_model == POWERPC_EXCP_BOOKE && excp == POWERPC_EXCP_VPU) {
> +        excp = POWERPC_EXCP_SPEU;
> +    }
> +#endif
> +
> +    vector = env->excp_vectors[excp];
> +    if (vector == (target_ulong)-1ULL) {
> +        cpu_abort(cs, "Raised an exception without defined vector %d\n",
> +                  excp);
> +    }
> +
> +    vector |= env->excp_prefix;
> +
> +    switch (excp) {
> +    case POWERPC_EXCP_CRITICAL:    /* Critical input                         */
> +        switch (excp_model) {
> +        case POWERPC_EXCP_40x:
> +            srr0 = SPR_40x_SRR2;
> +            srr1 = SPR_40x_SRR3;
> +            break;
> +        case POWERPC_EXCP_BOOKE:
> +            srr0 = SPR_BOOKE_CSRR0;
> +            srr1 = SPR_BOOKE_CSRR1;
> +            break;
> +        case POWERPC_EXCP_G2:
> +            break;
> +        default:
> +            goto excp_invalid;
> +        }
> +        break;
> +    case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
> +        if (msr_me == 0) {
> +            /*
> +             * Machine check exception is not enabled.  Enter
> +             * checkstop state.
> +             */
> +            fprintf(stderr, "Machine check while not allowed. "
> +                    "Entering checkstop state\n");
> +            if (qemu_log_separate()) {
> +                qemu_log("Machine check while not allowed. "
> +                        "Entering checkstop state\n");
> +            }
> +            cs->halted = 1;
> +            cpu_interrupt_exittb(cs);
> +        }
> +        if (env->msr_mask & MSR_HVB) {
> +            /*
> +             * ISA specifies HV, but can be delivered to guest with HV
> +             * clear (e.g., see FWNMI in PAPR).
> +             */
> +            new_msr |= (target_ulong)MSR_HVB;
> +        }
> +
> +        /* machine check exceptions don't have ME set */
> +        new_msr &= ~((target_ulong)1 << MSR_ME);
> +
> +        /* XXX: should also have something loaded in DAR / DSISR */
> +        switch (excp_model) {
> +        case POWERPC_EXCP_40x:
> +            srr0 = SPR_40x_SRR2;
> +            srr1 = SPR_40x_SRR3;
> +            break;
> +        case POWERPC_EXCP_BOOKE:
> +            /* FIXME: choose one or the other based on CPU type */
> +            srr0 = SPR_BOOKE_MCSRR0;
> +            srr1 = SPR_BOOKE_MCSRR1;
> +
> +            env->spr[SPR_BOOKE_CSRR0] = env->nip;
> +            env->spr[SPR_BOOKE_CSRR1] = msr;
> +            break;
> +        default:
> +            break;
> +        }
> +        break;
> +    case POWERPC_EXCP_DSI:       /* Data storage exception                   */
> +        trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);
> +        break;
> +    case POWERPC_EXCP_ISI:       /* Instruction storage exception            */
> +        trace_ppc_excp_isi(msr, env->nip);
> +        msr |= env->error_code;
> +        break;
> +    case POWERPC_EXCP_EXTERNAL:  /* External input                           */
> +    {
> +        bool lpes0;
> +
> +        cs = CPU(cpu);
> +
> +        /*
> +         * Exception targeting modifiers
> +         *
> +         * LPES0 is supported on POWER7/8/9
> +         * LPES1 is not supported (old iSeries mode)
> +         *
> +         * On anything else, we behave as if LPES0 is 1
> +         * (externals don't alter MSR:HV)
> +         */
> +#if defined(TARGET_PPC64)
> +        if (excp_model == POWERPC_EXCP_POWER7 ||
> +            excp_model == POWERPC_EXCP_POWER8 ||
> +            excp_model == POWERPC_EXCP_POWER9 ||
> +            excp_model == POWERPC_EXCP_POWER10) {
> +            lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> +        } else
> +#endif /* defined(TARGET_PPC64) */
> +        {
> +            lpes0 = true;
> +        }
> +
> +        if (!lpes0) {
> +            new_msr |= (target_ulong)MSR_HVB;
> +            new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +            srr0 = SPR_HSRR0;
> +            srr1 = SPR_HSRR1;
> +        }
> +        if (env->mpic_proxy) {
> +            /* IACK the IRQ on delivery */
> +            env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
> +        }
> +        break;
> +    }
> +    case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
> +        /* Get rS/rD and rA from faulting opcode */
> +        /*
> +         * Note: the opcode fields will not be set properly for a
> +         * direct store load/store, but nobody cares as nobody
> +         * actually uses direct store segments.
> +         */
> +        env->spr[SPR_DSISR] |= (env->error_code & 0x03FF0000) >> 16;
> +        break;
> +    case POWERPC_EXCP_PROGRAM:   /* Program exception                        */
> +        switch (env->error_code & ~0xF) {
> +        case POWERPC_EXCP_FP:
> +            if ((msr_fe0 == 0 && msr_fe1 == 0) || msr_fp == 0) {
> +                trace_ppc_excp_fp_ignore();
> +                cs->exception_index = POWERPC_EXCP_NONE;
> +                env->error_code = 0;
> +                return;
> +            }
> +
> +            /*
> +             * FP exceptions always have NIP pointing to the faulting
> +             * instruction, so always use store_next and claim we are
> +             * precise in the MSR.
> +             */
> +            msr |= 0x00100000;
> +            env->spr[SPR_BOOKE_ESR] = ESR_FP;
> +            break;
> +        case POWERPC_EXCP_INVAL:
> +            trace_ppc_excp_inval(env->nip);
> +            msr |= 0x00080000;
> +            env->spr[SPR_BOOKE_ESR] = ESR_PIL;
> +            break;
> +        case POWERPC_EXCP_PRIV:
> +            msr |= 0x00040000;
> +            env->spr[SPR_BOOKE_ESR] = ESR_PPR;
> +            break;
> +        case POWERPC_EXCP_TRAP:
> +            msr |= 0x00020000;
> +            env->spr[SPR_BOOKE_ESR] = ESR_PTR;
> +            break;
> +        default:
> +            /* Should never occur */
> +            cpu_abort(cs, "Invalid program exception %d. Aborting\n",
> +                      env->error_code);
> +            break;
> +        }
> +        break;
> +    case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
> +        lev = env->error_code;
> +
> +        if ((lev == 1) && cpu->vhyp) {
> +            dump_hcall(env);
> +        } else {
> +            dump_syscall(env);
> +        }
> +
> +        /*
> +         * We need to correct the NIP which in this case is supposed
> +         * to point to the next instruction
> +         */
> +        env->nip += 4;
> +
> +        /* "PAPR mode" built-in hypercall emulation */
> +        if ((lev == 1) && cpu->vhyp) {
> +            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_SYSCALL_VECTORED: /* scv exception                     */
> +        lev = env->error_code;
> +        dump_syscall(env);
> +        env->nip += 4;
> +        new_msr |= env->msr & ((target_ulong)1 << MSR_EE);
> +        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +
> +        vector += lev * 0x20;
> +
> +        env->lr = env->nip;
> +        env->ctr = msr;
> +        break;
> +    case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
> +    case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */
> +    case POWERPC_EXCP_DECR:      /* Decrementer exception                    */
> +        break;
> +    case POWERPC_EXCP_FIT:       /* Fixed-interval timer interrupt           */
> +        /* FIT on 4xx */
> +        trace_ppc_excp_print("FIT");
> +        break;
> +    case POWERPC_EXCP_WDT:       /* Watchdog timer interrupt                 */
> +        trace_ppc_excp_print("WDT");
> +        switch (excp_model) {
> +        case POWERPC_EXCP_BOOKE:
> +            srr0 = SPR_BOOKE_CSRR0;
> +            srr1 = SPR_BOOKE_CSRR1;
> +            break;
> +        default:
> +            break;
> +        }
> +        break;
> +    case POWERPC_EXCP_DTLB:      /* Data TLB error                           */
> +    case POWERPC_EXCP_ITLB:      /* Instruction TLB error                    */
> +        break;
> +    case POWERPC_EXCP_DEBUG:     /* Debug interrupt                          */
> +        if (env->flags & POWERPC_FLAG_DE) {
> +            /* FIXME: choose one or the other based on CPU type */
> +            srr0 = SPR_BOOKE_DSRR0;
> +            srr1 = SPR_BOOKE_DSRR1;
> +
> +            env->spr[SPR_BOOKE_CSRR0] = env->nip;
> +            env->spr[SPR_BOOKE_CSRR1] = msr;
> +
> +            /* DBSR already modified by caller */
> +        } else {
> +            cpu_abort(cs, "Debug exception triggered on unsupported model\n");
> +        }
> +        break;
> +    case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
> +        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
> +        break;
> +    case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
> +        break;
> +    case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
> +        srr0 = SPR_BOOKE_CSRR0;
> +        srr1 = SPR_BOOKE_CSRR1;
> +        break;
> +    case POWERPC_EXCP_RESET:     /* System reset exception                   */
> +        /* A power-saving exception sets ME, otherwise it is unchanged */
> +        if (msr_pow) {
> +            /* indicate that we resumed from power save mode */
> +            msr |= 0x10000;
> +            new_msr |= ((target_ulong)1 << MSR_ME);
> +        }
> +        if (env->msr_mask & MSR_HVB) {
> +            /*
> +             * ISA specifies HV, but can be delivered to guest with HV
> +             * clear (e.g., see FWNMI in PAPR, NMI injection in QEMU).
> +             */
> +            new_msr |= (target_ulong)MSR_HVB;
> +        } else {
> +            if (msr_pow) {
> +                cpu_abort(cs, "Trying to deliver power-saving system reset "
> +                          "exception %d with no HV support\n", excp);
> +            }
> +        }
> +        break;
> +    case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
> +    case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
> +    case POWERPC_EXCP_TRACE:     /* Trace exception                          */
> +        break;
> +    case POWERPC_EXCP_HISI:      /* Hypervisor instruction storage exception */
> +        msr |= env->error_code;
> +        /* fall through */
> +    case POWERPC_EXCP_HDECR:     /* Hypervisor decrementer exception         */
> +    case POWERPC_EXCP_HDSI:      /* Hypervisor data storage exception        */
> +    case POWERPC_EXCP_HDSEG:     /* Hypervisor data segment exception        */
> +    case POWERPC_EXCP_HISEG:     /* Hypervisor instruction segment exception */
> +    case POWERPC_EXCP_SDOOR_HV:  /* Hypervisor Doorbell interrupt            */
> +    case POWERPC_EXCP_HV_EMU:
> +    case POWERPC_EXCP_HVIRT:     /* Hypervisor virtualization                */
> +        srr0 = SPR_HSRR0;
> +        srr1 = SPR_HSRR1;
> +        new_msr |= (target_ulong)MSR_HVB;
> +        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +        break;
> +    case POWERPC_EXCP_VPU:       /* Vector unavailable exception             */
> +    case POWERPC_EXCP_VSXU:       /* VSX unavailable exception               */
> +    case POWERPC_EXCP_FU:         /* Facility unavailable exception          */
> +#ifdef TARGET_PPC64
> +        env->spr[SPR_FSCR] |= ((target_ulong)env->error_code << 56);
> +#endif
> +        break;
> +    case POWERPC_EXCP_HV_FU:     /* Hypervisor Facility Unavailable Exception */
> +#ifdef TARGET_PPC64
> +        env->spr[SPR_HFSCR] |= ((target_ulong)env->error_code << FSCR_IC_POS);
> +        srr0 = SPR_HSRR0;
> +        srr1 = SPR_HSRR1;
> +        new_msr |= (target_ulong)MSR_HVB;
> +        new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> +#endif
> +        break;
> +    case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
> +        trace_ppc_excp_print("PIT");
> +        break;
> +    case POWERPC_EXCP_IFTLB:     /* Instruction fetch TLB error              */
> +    case POWERPC_EXCP_DLTLB:     /* Data load TLB miss                       */
> +    case POWERPC_EXCP_DSTLB:     /* Data store TLB miss                      */
> +        switch (excp_model) {
> +        case POWERPC_EXCP_602:
> +        case POWERPC_EXCP_603:
> +        case POWERPC_EXCP_G2:
> +            /* Swap temporary saved registers with GPRs */
> +            if (!(new_msr & ((target_ulong)1 << MSR_TGPR))) {
> +                new_msr |= (target_ulong)1 << MSR_TGPR;
> +                hreg_swap_gpr_tgpr(env);
> +            }
> +            /* fall through */
> +        case POWERPC_EXCP_7x5:
> +            ppc_excp_debug_sw_tlb(env, excp);
> +
> +            msr |= env->crf[0] << 28;
> +            msr |= env->error_code; /* key, D/I, S/L bits */
> +            /* Set way using a LRU mechanism */
> +            msr |= ((env->last_way + 1) & (env->nb_ways - 1)) << 17;
> +            break;
> +        default:
> +            cpu_abort(cs, "Invalid TLB miss exception\n");
> +            break;
> +        }
> +        break;
> +    case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
> +    case POWERPC_EXCP_EFPRI:     /* Embedded floating-point round interrupt  */
> +    case POWERPC_EXCP_EPERFM:    /* Embedded performance monitor interrupt   */
> +    case POWERPC_EXCP_IO:        /* IO error exception                       */
> +    case POWERPC_EXCP_RUNM:      /* Run mode exception                       */
> +    case POWERPC_EXCP_EMUL:      /* Emulation trap exception                 */
> +    case POWERPC_EXCP_FPA:       /* Floating-point assist exception          */
> +    case POWERPC_EXCP_DABR:      /* Data address breakpoint                  */
> +    case POWERPC_EXCP_IABR:      /* Instruction address breakpoint           */
> +    case POWERPC_EXCP_SMI:       /* System management interrupt              */
> +    case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
> +    case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
> +    case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
> +    case POWERPC_EXCP_SOFTP:     /* Soft patch exception                     */
> +    case POWERPC_EXCP_MAINT:     /* Maintenance exception                    */
> +    case POWERPC_EXCP_MEXTBR:    /* Maskable external breakpoint             */
> +    case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint         */
> +        cpu_abort(cs, "%s exception not implemented\n",
> +                  powerpc_excp_name(excp));
> +        break;
> +    default:
> +    excp_invalid:
> +        cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
> +        break;
> +    }
> +
> +    /* Sanity check */
> +    if (!(env->msr_mask & MSR_HVB)) {
> +        if (new_msr & MSR_HVB) {
> +            cpu_abort(cs, "Trying to deliver HV exception (MSR) %d with "
> +                      "no HV support\n", excp);
> +        }
> +        if (srr0 == SPR_HSRR0) {
> +            cpu_abort(cs, "Trying to deliver HV exception (HSRR) %d with "
> +                      "no HV support\n", excp);
> +        }
> +    }
> +
> +    /*
> +     * Sort out endianness of interrupt, this differs depending on the
> +     * CPU, the HV mode, etc...
> +     */
> +    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
> +        new_msr |= (target_ulong)1 << MSR_LE;
> +    }
> +
> +#if defined(TARGET_PPC64)
> +    if (excp_model == POWERPC_EXCP_BOOKE) {
> +        if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
> +            /* Cat.64-bit: EPCR.ICM is copied to MSR.CM */
> +            new_msr |= (target_ulong)1 << MSR_CM;
> +        } else {
> +            vector = (uint32_t)vector;
> +        }
> +    } else {
> +        if (!msr_isf && !mmu_is_64bit(env->mmu_model)) {
> +            vector = (uint32_t)vector;
> +        } else {
> +            new_msr |= (target_ulong)1 << MSR_SF;
> +        }
> +    }
> +#endif
> +
> +    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
> +        /* Save PC */
> +        env->spr[srr0] = env->nip;
> +
> +        /* Save MSR */
> +        env->spr[srr1] = msr;
> +    }
> +
> +    /* This can update new_msr and vector if AIL applies */
> +    ppc_excp_apply_ail(cpu, excp_model, excp, msr, &new_msr, &vector);
> +
> +    powerpc_set_excp_state(cpu, vector, new_msr);
> +}
> +
>  /*
>   * Note that this function should be greatly optimized when called
>   * with a constant excp, from ppc_hw_interrupt
> @@ -872,6 +1343,9 @@ static void powerpc_excp(PowerPCCPU *cpu, int excp)
>      CPUPPCState *env = &cpu->env;
>  
>      switch (env->excp_model) {
> +    case POWERPC_EXCP_40x:
> +        powerpc_excp_40x(cpu, excp);
> +        break;
>      default:
>          powerpc_excp_legacy(cpu, excp);
>      }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup
  2022-01-10 18:15 ` [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup Fabiano Rosas
@ 2022-01-11  2:26   ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:26 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]

On Mon, Jan 10, 2022 at 03:15:43PM -0300, Fabiano Rosas wrote:
> In powerpc_excp_40x the Critical exception is now for 405 only, so we
> can remove the BookE and G2 blocks.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/excp_helper.c | 17 ++---------------
>  1 file changed, 2 insertions(+), 15 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 1d997c4d6b..fecf4d5a4e 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -431,20 +431,8 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>  
>      switch (excp) {
>      case POWERPC_EXCP_CRITICAL:    /* Critical input                         */
> -        switch (excp_model) {
> -        case POWERPC_EXCP_40x:
> -            srr0 = SPR_40x_SRR2;
> -            srr1 = SPR_40x_SRR3;
> -            break;
> -        case POWERPC_EXCP_BOOKE:
> -            srr0 = SPR_BOOKE_CSRR0;
> -            srr1 = SPR_BOOKE_CSRR1;
> -            break;
> -        case POWERPC_EXCP_G2:
> -            break;
> -        default:
> -            goto excp_invalid;
> -        }
> +        srr0 = SPR_40x_SRR2;
> +        srr1 = SPR_40x_SRR3;
>          break;
>      case POWERPC_EXCP_MCHECK:    /* Machine check exception                  */
>          if (msr_me == 0) {
> @@ -698,7 +686,6 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>                    powerpc_excp_name(excp));
>          break;
>      default:
> -    excp_invalid:
>          cpu_abort(cs, "Invalid PowerPC exception %d. Aborting\n", excp);
>          break;
>      }

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 6/8] target/ppc: 405: Machine check exception cleanup
  2022-01-10 18:15 ` [PATCH 6/8] target/ppc: 405: Machine check exception cleanup Fabiano Rosas
@ 2022-01-11  2:26   ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:26 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 2196 bytes --]

On Mon, Jan 10, 2022 at 03:15:44PM -0300, Fabiano Rosas wrote:
> powerpc_excp_40x applies only to the 405, so remove HV code and
> references to BookE.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/excp_helper.c | 26 ++------------------------
>  1 file changed, 2 insertions(+), 24 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index fecf4d5a4e..82ade5d7bd 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -449,34 +449,12 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>              cs->halted = 1;
>              cpu_interrupt_exittb(cs);
>          }
> -        if (env->msr_mask & MSR_HVB) {
> -            /*
> -             * ISA specifies HV, but can be delivered to guest with HV
> -             * clear (e.g., see FWNMI in PAPR).
> -             */
> -            new_msr |= (target_ulong)MSR_HVB;
> -        }
>  
>          /* machine check exceptions don't have ME set */
>          new_msr &= ~((target_ulong)1 << MSR_ME);
>  
> -        /* XXX: should also have something loaded in DAR / DSISR */
> -        switch (excp_model) {
> -        case POWERPC_EXCP_40x:
> -            srr0 = SPR_40x_SRR2;
> -            srr1 = SPR_40x_SRR3;
> -            break;
> -        case POWERPC_EXCP_BOOKE:
> -            /* FIXME: choose one or the other based on CPU type */
> -            srr0 = SPR_BOOKE_MCSRR0;
> -            srr1 = SPR_BOOKE_MCSRR1;
> -
> -            env->spr[SPR_BOOKE_CSRR0] = env->nip;
> -            env->spr[SPR_BOOKE_CSRR1] = msr;
> -            break;
> -        default:
> -            break;
> -        }
> +        srr0 = SPR_40x_SRR2;
> +        srr1 = SPR_40x_SRR3;
>          break;
>      case POWERPC_EXCP_DSI:       /* Data storage exception                   */
>          trace_ppc_excp_dsi(env->spr[SPR_DSISR], env->spr[SPR_DAR]);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 7/8] target/ppc: 405: External exception cleanup
  2022-01-10 18:15 ` [PATCH 7/8] target/ppc: 405: External " Fabiano Rosas
@ 2022-01-11  2:28   ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:28 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]

On Mon, Jan 10, 2022 at 03:15:45PM -0300, Fabiano Rosas wrote:
> 405 has no MSR_HV and EPR is BookE only so we can remove it all.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  target/ppc/excp_helper.c | 37 -------------------------------------
>  1 file changed, 37 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 82ade5d7bd..f7b9af5065 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -464,44 +464,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>          msr |= env->error_code;
>          break;
>      case POWERPC_EXCP_EXTERNAL:  /* External input                           */
> -    {
> -        bool lpes0;
> -
> -        cs = CPU(cpu);
> -
> -        /*
> -         * Exception targeting modifiers
> -         *
> -         * LPES0 is supported on POWER7/8/9
> -         * LPES1 is not supported (old iSeries mode)
> -         *
> -         * On anything else, we behave as if LPES0 is 1
> -         * (externals don't alter MSR:HV)
> -         */
> -#if defined(TARGET_PPC64)
> -        if (excp_model == POWERPC_EXCP_POWER7 ||
> -            excp_model == POWERPC_EXCP_POWER8 ||
> -            excp_model == POWERPC_EXCP_POWER9 ||
> -            excp_model == POWERPC_EXCP_POWER10) {
> -            lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        } else
> -#endif /* defined(TARGET_PPC64) */
> -        {
> -            lpes0 = true;
> -        }
> -
> -        if (!lpes0) {
> -            new_msr |= (target_ulong)MSR_HVB;
> -            new_msr |= env->msr & ((target_ulong)1 << MSR_RI);
> -            srr0 = SPR_HSRR0;
> -            srr1 = SPR_HSRR1;
> -        }
> -        if (env->mpic_proxy) {
> -            /* IACK the IRQ on delivery */
> -            env->spr[SPR_BOOKE_EPR] = ldl_phys(cs->as, env->mpic_iack);
> -        }
>          break;
> -    }
>      case POWERPC_EXCP_ALIGN:     /* Alignment exception                      */
>          /* Get rS/rD and rA from faulting opcode */
>          /*

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] target/ppc: 405: System call exception cleanup
  2022-01-10 18:15 ` [PATCH 8/8] target/ppc: 405: System call " Fabiano Rosas
@ 2022-01-11  2:31   ` David Gibson
  2022-01-11 12:48     ` Fabiano Rosas
  0 siblings, 1 reply; 25+ messages in thread
From: David Gibson @ 2022-01-11  2:31 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: richard.henderson, danielhb413, qemu-ppc, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 2594 bytes --]

On Mon, Jan 10, 2022 at 03:15:46PM -0300, Fabiano Rosas wrote:
> There's no sc 1.
> 
> We also only used env->nip because of the vhyp code, so change to
> 'vector' now.

I don't think this is right.  The point with the env->nip change is
changing the PC as it appeared *before* saving it to SRR0, so that
we'll eventually return to the right place.  'vector' is the address
for the interrupt vector itself.


> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/excp_helper.c | 23 +++--------------------
>  1 file changed, 3 insertions(+), 20 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index f7b9af5065..ab298d0d8f 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -398,7 +398,7 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>      CPUPPCState *env = &cpu->env;
>      int excp_model = env->excp_model;
>      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);
> @@ -518,30 +518,13 @@ static void powerpc_excp_40x(PowerPCCPU *cpu, int excp)
>          }
>          break;
>      case POWERPC_EXCP_SYSCALL:   /* System call exception                    */
> -        lev = env->error_code;
> -
> -        if ((lev == 1) && cpu->vhyp) {
> -            dump_hcall(env);
> -        } else {
> -            dump_syscall(env);
> -        }
> +        dump_syscall(env);
>  
>          /*
>           * We need to correct the NIP which in this case is supposed
>           * to point to the next instruction
>           */
> -        env->nip += 4;
> -
> -        /* "PAPR mode" built-in hypercall emulation */
> -        if ((lev == 1) && cpu->vhyp) {
> -            PPCVirtualHypervisorClass *vhc =
> -                PPC_VIRTUAL_HYPERVISOR_GET_CLASS(cpu->vhyp);
> -            vhc->hypercall(cpu->vhyp, cpu);
> -            return;
> -        }
> -        if (lev == 1) {
> -            new_msr |= (target_ulong)MSR_HVB;
> -        }
> +        vector += 4;
>          break;
>      case POWERPC_EXCP_FPU:       /* Floating-point unavailable exception     */
>      case POWERPC_EXCP_APU:       /* Auxiliary processor unavailable          */

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n)
  2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
                   ` (7 preceding siblings ...)
  2022-01-10 18:15 ` [PATCH 8/8] target/ppc: 405: System call " Fabiano Rosas
@ 2022-01-11  8:37 ` Cédric Le Goater
  8 siblings, 0 replies; 25+ messages in thread
From: Cédric Le Goater @ 2022-01-11  8:37 UTC (permalink / raw)
  To: Fabiano Rosas, qemu-devel
  Cc: richard.henderson, danielhb413, qemu-ppc, Christophe Leroy, david

On 1/10/22 19:15, Fabiano Rosas wrote:
> This is the first series of the exception model-specific changes. I
> intend to keep this same structure for the rest of the exception
> models.
> 
> == preparation ==
> 
> First few patches are cleanups specific to this model. This comes
> first because I'm using some of these changes to help validate what
> should be removed in the subsequent patches.
> 
> == new powerpc_excp function ==
> 
> One patch copies the powerpc_excp function as is and renames it to the
> exception model in question. Doing this first facilitates spotting
> what changed from the _legacy version.
> 
> The following patch changes everything that needs to be changed at the
> top level (MSR mask, SF, ILE, AIL, SRRs, etc.) and removes exceptions
> that are not used by this processor family.
> 
> == exceptions cleanup ==
> 
> After the new powerpc_excp function has been made specific to that one
> exception model, go through every exception removing anything that
> does not apply to this model.
> 
> Based on legoater/ppc-7.0
> 
> Cleanups 2/n [in ppc-7.0]:
> https://lists.nongnu.org/archive/html/qemu-ppc/2022-01/msg00252.html
> 
> Cleanups 1/n [already merged]:
> https://mail.gnu.org/archive/html/qemu-ppc/2021-12/msg00696.html
> 
> RFC v2:
> https://lists.nongnu.org/archive/html/qemu-ppc/2021-12/msg00542.html
> 
> RFC v1:
> https://lists.nongnu.org/archive/html/qemu-ppc/2021-06/msg00026.html
> 
> Fabiano Rosas (8):
>    target/ppc: 405: Add missing MSR bits to msr_mask
>    target/ppc: 405: Add missing exception handlers
>    target/ppc: Introduce powerpc_excp_40x
>    squash target/ppc: Introduce powerpc_excp_40x
>    target/ppc: 405: Critical exceptions cleanup
>    target/ppc: 405: Machine check exception cleanup
>    target/ppc: 405: External exception cleanup
>    target/ppc: 405: System call exception cleanup
> 
>   target/ppc/cpu_init.c    |   8 +-
>   target/ppc/excp_helper.c | 246 +++++++++++++++++++++++++++++++++++++++
>   2 files changed, 253 insertions(+), 1 deletion(-)
> 

The 405 CPU emulation is not "fully" functional today. the latest kernel
boots, see instructions here :

   https://github.com/legoater/qemu/wiki/ref405ep

but user space segfaults after a while. I suspect some registers (r31)
being clobbered. The same user space image boots correctly under the
QEMU bamboo machine using a 440EP CPU.

That said, the 405 MMU, DECR, SYSCALL exceptions seem to work fine.
So it might be the kernel in some dark corner when restoring user
context.

It would be nice to fix to get a test better coverage for this patchset.
Anyhow, these are good cleanups and they won't be gated by 405 user
space being broken.

Thanks,

C.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 8/8] target/ppc: 405: System call exception cleanup
  2022-01-11  2:31   ` David Gibson
@ 2022-01-11 12:48     ` Fabiano Rosas
  0 siblings, 0 replies; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-11 12:48 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, danielhb413, richard.henderson, qemu-devel, clg

David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Jan 10, 2022 at 03:15:46PM -0300, Fabiano Rosas wrote:
>> There's no sc 1.
>> 
>> We also only used env->nip because of the vhyp code, so change to
>> 'vector' now.
>
> I don't think this is right.  The point with the env->nip change is
> changing the PC as it appeared *before* saving it to SRR0, so that
> we'll eventually return to the right place.  'vector' is the address
> for the interrupt vector itself.

Ugh, In the RFC version I introduced a 'nip' that holds env->nip. So it
would be correct to use 'nip' instead of 'env->nip' here. I knew
'vector' was looking off... Thanks for catching that.



^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] target/ppc: 405: Add missing exception handlers
  2022-01-11  2:10   ` David Gibson
@ 2022-01-14 21:46     ` Fabiano Rosas
  2022-01-15  7:05       ` David Gibson
  0 siblings, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-14 21:46 UTC (permalink / raw)
  To: David Gibson; +Cc: qemu-ppc, danielhb413, richard.henderson, qemu-devel, clg

David Gibson <david@gibson.dropbear.id.au> writes:

> On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>  target/ppc/cpu_init.c | 2 ++
>>  1 file changed, 2 insertions(+)
>> 
>> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
>> index a50ddaeaae..9097948e67 100644
>> --- a/target/ppc/cpu_init.c
>> +++ b/target/ppc/cpu_init.c
>> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
>>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
>>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
>>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
>> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
>
> I have a vague recollection from my days of working on 405 that there
> may have been something funky with FP emulation on there: e.g. FP
> instructions causing 0x700 program interrupts instead of FP unavailble
> interrupts or something.

Maybe this (from the manual):

  Program - causing conditions:
  
  Attempted execution of illegal instructions, TRAP instruction,
  privileged instruction in problem state, or auxiliary processor (APU)
  instruction, or unimplemented FPU instruction, or unimplemented APU
  instruction, or APU interrupt, or FPU interrupt
  
  FPU Unavailable - causing conditions:
  
  Attempted execution of an FPU instruction when MSR[FP]=0.

There's also this bit:

  Attempted execution of an APU instruction while the APUc405exception
  signal is asserted) results in a program interrupt. Similarly, attempted
  execution of an FPU instruction whilethe FPUc405exception signal is
  asserted) also results in a program interrupt. The following also result
  in program interrupts: attempted execution of an APU instruction while
  APUc405DcdAPUOp is asserted but APUC405DcdValidOp is deasserted; and
  attempted execution of an FPU instruction while APUc405DcdFpuOp but
  APUC405DcdValidOp is deasserted.

> I might be remembering incorrectly - the manual does seem to imply
> that 0x800 FP unavailable is there as normal, but it might be worth
> double checking this (against real hardware, if possible).

The Linux kernel has the vectors that I'm adding disabled:

  EXCEPTION(0x0800, Trap_08, unknown_exception) <-- FPU
  EXCEPTION(0x0900, Trap_09, unknown_exception)
  EXCEPTION(0x0A00, Trap_0A, unknown_exception) 
  EXCEPTION(0x0B00, Trap_0B, unknown_exception)
  ...
  EXCEPTION(0x0F00, Trap_0F, unknown_exception) <-- APU

(0xf20 would probably cause a crash as we'd jump to the middle of the
exception prologue)

Maybe I should drop this patch then? That way future developers won't
feel tempted to raise one of these.

It seems mostly inconsequential either way, what do you think?


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/8] target/ppc: 405: Add missing exception handlers
  2022-01-14 21:46     ` Fabiano Rosas
@ 2022-01-15  7:05       ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-15  7:05 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: qemu-ppc, danielhb413, richard.henderson, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 3585 bytes --]

On Fri, Jan 14, 2022 at 06:46:10PM -0300, Fabiano Rosas wrote:
> David Gibson <david@gibson.dropbear.id.au> writes:
> 
> > On Mon, Jan 10, 2022 at 03:15:40PM -0300, Fabiano Rosas wrote:
> >> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> >> ---
> >>  target/ppc/cpu_init.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> >> index a50ddaeaae..9097948e67 100644
> >> --- a/target/ppc/cpu_init.c
> >> +++ b/target/ppc/cpu_init.c
> >> @@ -1951,7 +1951,9 @@ static void init_excp_4xx_softmmu(CPUPPCState *env)
> >>      env->excp_vectors[POWERPC_EXCP_EXTERNAL] = 0x00000500;
> >>      env->excp_vectors[POWERPC_EXCP_ALIGN]    = 0x00000600;
> >>      env->excp_vectors[POWERPC_EXCP_PROGRAM]  = 0x00000700;
> >> +    env->excp_vectors[POWERPC_EXCP_FPU]      = 0x00000800;
> >
> > I have a vague recollection from my days of working on 405 that there
> > may have been something funky with FP emulation on there: e.g. FP
> > instructions causing 0x700 program interrupts instead of FP unavailble
> > interrupts or something.
> 
> Maybe this (from the manual):
> 
>   Program - causing conditions:
>   
>   Attempted execution of illegal instructions, TRAP instruction,
>   privileged instruction in problem state, or auxiliary processor (APU)
>   instruction, or unimplemented FPU instruction, or unimplemented APU
>   instruction, or APU interrupt, or FPU interrupt
>   
>   FPU Unavailable - causing conditions:
>   
>   Attempted execution of an FPU instruction when MSR[FP]=0.
> 
> There's also this bit:
> 
>   Attempted execution of an APU instruction while the APUc405exception
>   signal is asserted) results in a program interrupt. Similarly, attempted
>   execution of an FPU instruction whilethe FPUc405exception signal is
>   asserted) also results in a program interrupt. The following also result
>   in program interrupts: attempted execution of an APU instruction while
>   APUc405DcdAPUOp is asserted but APUC405DcdValidOp is deasserted; and
>   attempted execution of an FPU instruction while APUc405DcdFpuOp but
>   APUC405DcdValidOp is deasserted.

Right... those do seem to suggest that FP comes in as a 0x700 rather
than 0x800.  Really hard to be sure without checking an actual chip.

> > I might be remembering incorrectly - the manual does seem to imply
> > that 0x800 FP unavailable is there as normal, but it might be worth
> > double checking this (against real hardware, if possible).
> 
> The Linux kernel has the vectors that I'm adding disabled:
> 
>   EXCEPTION(0x0800, Trap_08, unknown_exception) <-- FPU
>   EXCEPTION(0x0900, Trap_09, unknown_exception)
>   EXCEPTION(0x0A00, Trap_0A, unknown_exception) 
>   EXCEPTION(0x0B00, Trap_0B, unknown_exception)
>   ...
>   EXCEPTION(0x0F00, Trap_0F, unknown_exception) <-- APU
> 
> (0xf20 would probably cause a crash as we'd jump to the middle of the
> exception prologue)

Right.. that's fairly strong evidence that those vectors don't operate
in practice.

> Maybe I should drop this patch then? That way future developers won't
> feel tempted to raise one of these.

Maybe.  Better yet would be to verify on a chip then drop a comment in
there explicitly describing the situation

> It seems mostly inconsequential either way, what do you think?

Well, yes.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
  2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
  2022-01-11  2:04   ` David Gibson
@ 2022-01-17 21:12   ` Fabiano Rosas
  2022-01-18  8:40     ` David Gibson
  1 sibling, 1 reply; 25+ messages in thread
From: Fabiano Rosas @ 2022-01-17 21:12 UTC (permalink / raw)
  To: qemu-devel; +Cc: qemu-ppc, danielhb413, richard.henderson, clg, david

Fabiano Rosas <farosas@linux.ibm.com> writes:

> Some bits described in the user manual are missing from msr_mask. Add
> them.
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/cpu_init.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> index e30e86fe9d..a50ddaeaae 100644
> --- a/target/ppc/cpu_init.c
> +++ b/target/ppc/cpu_init.c
> @@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
>                         PPC_MEM_SYNC | PPC_MEM_EIEIO |
>                         PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC |
>                         PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP;
> -    pcc->msr_mask = (1ull << MSR_POW) |
> +    pcc->msr_mask = (1ull << MSR_AP) |
> +                    (1ull << MSR_POW) |
>                      (1ull << MSR_CE) |
>                      (1ull << MSR_EE) |
>                      (1ull << MSR_PR) |
>                      (1ull << MSR_FP) |
> +                    (1ull << MSR_ME) |
>                      (1ull << MSR_DWE) |
>                      (1ull << MSR_DE) |
> +                    (1ull << MSR_FE1) |
>                      (1ull << MSR_IR) |
>                      (1ull << MSR_DR);

This patch brings an unexpected complication:

MSR_AP here is not correct, it is defined as:

#define MSR_AP   23 /* Access privilege state on 602 */

That is bit 8. While MSR_AP in the 405 is bit 6. So I would need to
introduce a new MSR_AP_405 defined as:

#define MSR_AP_405   25 /* Auxiliar processor available on 405 */

But 25 is the same as MSR_SPE, so it triggers this code in
init_ppc_proc:

    /* MSR bits & flags consistency checks */
    if (env->msr_mask & (1 << 25)) {
        switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
        case POWERPC_FLAG_SPE:
        case POWERPC_FLAG_VRE:
            break;
        default:
            fprintf(stderr, "PowerPC MSR definition inconsistency\n"
                    "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n");
            exit(1);
        }
     ...

The commit that introduced that sanity check is 25ba3a6812 ("Remove
synonymous in PowerPC MSR bits definitions..."), which sort of assumes
that MSR bits will not have different purposes between any of the (now
47) CPUs, while itself leaving other duplicated bits around.
   
So my idea is to drop this patch and only include the MSR_ME that is of
practical effect at patch 6. I think going into the rabbit hole of
disambiguating MSR bits falls out of the scope of the exception series.


^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask
  2022-01-17 21:12   ` Fabiano Rosas
@ 2022-01-18  8:40     ` David Gibson
  0 siblings, 0 replies; 25+ messages in thread
From: David Gibson @ 2022-01-18  8:40 UTC (permalink / raw)
  To: Fabiano Rosas; +Cc: qemu-ppc, danielhb413, richard.henderson, qemu-devel, clg

[-- Attachment #1: Type: text/plain, Size: 3434 bytes --]

On Mon, Jan 17, 2022 at 06:12:20PM -0300, Fabiano Rosas wrote:
> Fabiano Rosas <farosas@linux.ibm.com> writes:
> 
> > Some bits described in the user manual are missing from msr_mask. Add
> > them.
> >
> > Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> > ---
> >  target/ppc/cpu_init.c | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> >
> > diff --git a/target/ppc/cpu_init.c b/target/ppc/cpu_init.c
> > index e30e86fe9d..a50ddaeaae 100644
> > --- a/target/ppc/cpu_init.c
> > +++ b/target/ppc/cpu_init.c
> > @@ -2535,15 +2535,19 @@ POWERPC_FAMILY(405)(ObjectClass *oc, void *data)
> >                         PPC_MEM_SYNC | PPC_MEM_EIEIO |
> >                         PPC_40x_TLB | PPC_MEM_TLBIA | PPC_MEM_TLBSYNC |
> >                         PPC_4xx_COMMON | PPC_405_MAC | PPC_40x_EXCP;
> > -    pcc->msr_mask = (1ull << MSR_POW) |
> > +    pcc->msr_mask = (1ull << MSR_AP) |
> > +                    (1ull << MSR_POW) |
> >                      (1ull << MSR_CE) |
> >                      (1ull << MSR_EE) |
> >                      (1ull << MSR_PR) |
> >                      (1ull << MSR_FP) |
> > +                    (1ull << MSR_ME) |
> >                      (1ull << MSR_DWE) |
> >                      (1ull << MSR_DE) |
> > +                    (1ull << MSR_FE1) |
> >                      (1ull << MSR_IR) |
> >                      (1ull << MSR_DR);
> 
> This patch brings an unexpected complication:
> 
> MSR_AP here is not correct, it is defined as:
> 
> #define MSR_AP   23 /* Access privilege state on 602 */
> 
> That is bit 8. While MSR_AP in the 405 is bit 6. So I would need to

Uh oh...

> introduce a new MSR_AP_405 defined as:
> 
> #define MSR_AP_405   25 /* Auxiliar processor available on 405 */
> 
> But 25 is the same as MSR_SPE, so it triggers this code in
> init_ppc_proc:
> 
>     /* MSR bits & flags consistency checks */
>     if (env->msr_mask & (1 << 25)) {
>         switch (env->flags & (POWERPC_FLAG_SPE | POWERPC_FLAG_VRE)) {
>         case POWERPC_FLAG_SPE:
>         case POWERPC_FLAG_VRE:
>             break;
>         default:
>             fprintf(stderr, "PowerPC MSR definition inconsistency\n"
>                     "Should define POWERPC_FLAG_SPE or POWERPC_FLAG_VRE\n");
>             exit(1);
>         }
>      ...

Ah.  Which suggests this section itself should probably be taken out
of the common path and moved to code specific to the cpu families with
SPE.

> The commit that introduced that sanity check is 25ba3a6812 ("Remove
> synonymous in PowerPC MSR bits definitions..."), which sort of assumes
> that MSR bits will not have different purposes between any of the (now
> 47) CPUs, while itself leaving other duplicated bits around.

Ah, oops.  Even in 2007 I could have told you that wasn't a safe
assumption.  Oh well.

> So my idea is to drop this patch and only include the MSR_ME that is of
> practical effect at patch 6. I think going into the rabbit hole of
> disambiguating MSR bits falls out of the scope of the exception series.

That seems fair for the time being.  I suspect splitting the exception
paths will make cleaning up MSR collisions like this easier.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2022-01-18  9:11 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-10 18:15 [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Fabiano Rosas
2022-01-10 18:15 ` [PATCH 1/8] target/ppc: 405: Add missing MSR bits to msr_mask Fabiano Rosas
2022-01-11  2:04   ` David Gibson
2022-01-11  2:07     ` David Gibson
2022-01-17 21:12   ` Fabiano Rosas
2022-01-18  8:40     ` David Gibson
2022-01-10 18:15 ` [PATCH 2/8] target/ppc: 405: Add missing exception handlers Fabiano Rosas
2022-01-11  2:10   ` David Gibson
2022-01-14 21:46     ` Fabiano Rosas
2022-01-15  7:05       ` David Gibson
2022-01-10 18:15 ` [PATCH 3/8] target/ppc: Introduce powerpc_excp_40x Fabiano Rosas
2022-01-11  2:20   ` David Gibson
2022-01-10 18:15 ` [PATCH 4/8] squash " Fabiano Rosas
2022-01-10 18:51   ` BALATON Zoltan
2022-01-10 19:00     ` Fabiano Rosas
2022-01-10 18:15 ` [PATCH 5/8] target/ppc: 405: Critical exceptions cleanup Fabiano Rosas
2022-01-11  2:26   ` David Gibson
2022-01-10 18:15 ` [PATCH 6/8] target/ppc: 405: Machine check exception cleanup Fabiano Rosas
2022-01-11  2:26   ` David Gibson
2022-01-10 18:15 ` [PATCH 7/8] target/ppc: 405: External " Fabiano Rosas
2022-01-11  2:28   ` David Gibson
2022-01-10 18:15 ` [PATCH 8/8] target/ppc: 405: System call " Fabiano Rosas
2022-01-11  2:31   ` David Gibson
2022-01-11 12:48     ` Fabiano Rosas
2022-01-11  8:37 ` [PATCH 0/8] target/ppc: powerpc_excp improvements [40x] (3/n) Cédric Le Goater

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.