[PATCH 0/9] target/ppc: powerpc_excp improvements (2/n)

[PATCH 0/9] target/ppc: powerpc_excp improvements (2/n)

[Fabiano Rosas]

These are the follow up cleanups from the RFC that touch the top level
of powerpc_excp. Applies on top of the 1/n series.

Patches 1-2: extract software TLB debug into a function;

Patch 3: group the "unimplemented" messages;

Patches 4-8: move ILE code into a separate function and put ILE and
             AIL under a BookS conditional;

Patch 9: the powerpc_excp_legacy wrapper. Subsequent patch series will
         be dedicated to splitting one "cpu family" each. I have 40x
         ready and I'm working on 60x.

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

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

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

Fabiano Rosas (9):
  target/ppc: powerpc_excp: Extract software TLB logging into a function
  target/ppc: powerpc_excp: Keep 60x soft MMU logs active
  target/ppc: powerpc_excp: Group unimplemented exceptions
  target/ppc: Add HV support to ppc_interrupts_little_endian
  target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
  target/ppc: powerpc_excp: Preserve MSR_LE bit
  target/ppc: powerpc_excp: Move ILE setting into a function
  target/ppc: powerpc_excp: Move AIL under a Book3s block
  target/ppc: Introduce a wrapper for powerpc_excp

 target/ppc/arch_dump.c   |   2 +-
 target/ppc/cpu.h         |  21 ++--
 target/ppc/excp_helper.c | 218 ++++++++++++++-------------------------
 3 files changed, 92 insertions(+), 149 deletions(-)

-- 
2.33.1

[PATCH 1/9] target/ppc: powerpc_excp: Extract software TLB logging into a function

[Fabiano Rosas]

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index c7e55800af..002a42261b 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -63,6 +63,41 @@ static inline void dump_hcall(CPUPPCState *env)
                   env->nip);
 }
 
+static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
+{
+#if defined(DEBUG_SOFTWARE_TLB)
+    const char *es;
+    target_ulong *miss, *cmp;
+    int en;
+
+    if (!qemu_log_enabled()) {
+        return;
+    }
+
+    if (excp == POWERPC_EXCP_IFTLB) {
+        es = "I";
+        en = 'I';
+        miss = &env->spr[SPR_IMISS];
+        cmp = &env->spr[SPR_ICMP];
+    } else {
+        if (excp == POWERPC_EXCP_DLTLB) {
+            es = "DL";
+        } else {
+            es = "DS";
+        }
+        en = 'D';
+        miss = &env->spr[SPR_DMISS];
+        cmp = &env->spr[SPR_DCMP];
+    }
+    qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
+             TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
+             TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
+             env->spr[SPR_HASH1], env->spr[SPR_HASH2],
+             env->error_code);
+#endif
+}
+
+
 static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
                                 target_ulong *msr)
 {
@@ -704,34 +739,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
             }
             /* fall through */
         case POWERPC_EXCP_7x5:
-#if defined(DEBUG_SOFTWARE_TLB)
-            if (qemu_log_enabled()) {
-                const char *es;
-                target_ulong *miss, *cmp;
-                int en;
+            ppc_excp_debug_sw_tlb(env, excp);
 
-                if (excp == POWERPC_EXCP_IFTLB) {
-                    es = "I";
-                    en = 'I';
-                    miss = &env->spr[SPR_IMISS];
-                    cmp = &env->spr[SPR_ICMP];
-                } else {
-                    if (excp == POWERPC_EXCP_DLTLB) {
-                        es = "DL";
-                    } else {
-                        es = "DS";
-                    }
-                    en = 'D';
-                    miss = &env->spr[SPR_DMISS];
-                    cmp = &env->spr[SPR_DCMP];
-                }
-                qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
-                         TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
-                         TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
-                         env->spr[SPR_HASH1], env->spr[SPR_HASH2],
-                         env->error_code);
-            }
-#endif
             msr |= env->crf[0] << 28;
             msr |= env->error_code; /* key, D/I, S/L bits */
             /* Set way using a LRU mechanism */
-- 
2.33.1

[PATCH 2/9] target/ppc: powerpc_excp: Keep 60x soft MMU logs active

[Fabiano Rosas]

Remove the compile time definition and make the logging be controlled
by the `-d mmu` option in the cmdline.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 002a42261b..4769abfb0c 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -30,8 +30,6 @@
 #include "exec/cpu_ldst.h"
 #endif
 
-/* #define DEBUG_SOFTWARE_TLB */
-
 /*****************************************************************************/
 /* Exception processing */
 #if !defined(CONFIG_USER_ONLY)
@@ -65,7 +63,6 @@ static inline void dump_hcall(CPUPPCState *env)
 
 static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
 {
-#if defined(DEBUG_SOFTWARE_TLB)
     const char *es;
     target_ulong *miss, *cmp;
     int en;
@@ -89,12 +86,12 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
         miss = &env->spr[SPR_DMISS];
         cmp = &env->spr[SPR_DCMP];
     }
-    qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
-             TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
-             TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
-             env->spr[SPR_HASH1], env->spr[SPR_HASH2],
-             env->error_code);
-#endif
+
+    qemu_log_mask(CPU_LOG_MMU, "6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
+                  TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
+                  TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
+                  env->spr[SPR_HASH1], env->spr[SPR_HASH2],
+                  env->error_code);
 }
 
 
-- 
2.33.1

[PATCH 3/9] target/ppc: powerpc_excp: Group unimplemented exceptions

[Fabiano Rosas]

(I'll alter this to use powerpc_excp_name once it is merged)

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 4769abfb0c..160e06e3a3 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -628,23 +628,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
         env->spr[SPR_BOOKE_ESR] = ESR_SPV;
         break;
-    case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
-        /* XXX: TODO */
-        cpu_abort(cs, "Embedded floating point data exception "
-                  "is not implemented yet !\n");
-        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
-        break;
-    case POWERPC_EXCP_EFPRI:     /* Embedded floating-point round interrupt  */
-        /* XXX: TODO */
-        cpu_abort(cs, "Embedded floating point round exception "
-                  "is not implemented yet !\n");
-        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
-        break;
-    case POWERPC_EXCP_EPERFM:    /* Embedded performance monitor interrupt   */
-        /* XXX: TODO */
-        cpu_abort(cs,
-                  "Performance counter exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
         break;
     case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
@@ -709,19 +692,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
         trace_ppc_excp_print("PIT");
         break;
-    case POWERPC_EXCP_IO:        /* IO error exception                       */
-        /* XXX: TODO */
-        cpu_abort(cs, "601 IO error exception is not implemented yet !\n");
-        break;
-    case POWERPC_EXCP_RUNM:      /* Run mode exception                       */
-        /* XXX: TODO */
-        cpu_abort(cs, "601 run mode exception is not implemented yet !\n");
-        break;
-    case POWERPC_EXCP_EMUL:      /* Emulation trap exception                 */
-        /* XXX: TODO */
-        cpu_abort(cs, "602 emulation trap exception "
-                  "is not implemented yet !\n");
-        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                      */
@@ -748,56 +718,24 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
             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          */
-        /* XXX: TODO */
-        cpu_abort(cs, "Floating point assist exception "
-                  "is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_DABR:      /* Data address breakpoint                  */
-        /* XXX: TODO */
-        cpu_abort(cs, "DABR exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_IABR:      /* Instruction address breakpoint           */
-        /* XXX: TODO */
-        cpu_abort(cs, "IABR exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_SMI:       /* System management interrupt              */
-        /* XXX: TODO */
-        cpu_abort(cs, "SMI exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
-        /* XXX: TODO */
-        cpu_abort(cs, "Thermal management exception "
-                  "is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
-        /* XXX: TODO */
-        cpu_abort(cs,
-                  "Performance counter exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
-        /* XXX: TODO */
-        cpu_abort(cs, "VPU assist exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_SOFTP:     /* Soft patch exception                     */
-        /* XXX: TODO */
-        cpu_abort(cs,
-                  "970 soft-patch exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_MAINT:     /* Maintenance exception                    */
-        /* XXX: TODO */
-        cpu_abort(cs,
-                  "970 maintenance exception is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_MEXTBR:    /* Maskable external breakpoint             */
-        /* XXX: TODO */
-        cpu_abort(cs, "Maskable external exception "
-                  "is not implemented yet !\n");
-        break;
     case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint         */
-        /* XXX: TODO */
-        cpu_abort(cs, "Non maskable external exception "
-                  "is not implemented yet !\n");
+        cpu_abort(cs, "Exception not implemented\n");
         break;
     default:
     excp_invalid:
-- 
2.33.1

[PATCH 4/9] target/ppc: Add HV support to ppc_interrupts_little_endian

[Fabiano Rosas]

The ppc_interrupts_little_endian function could be used for interrupts
delivered in Hypervisor mode, so add support for powernv8 and powernv9
to it.

Also drop the comment because it is inaccurate, all CPUs that can run
little endian can have interrupts in little endian. The point is
whether they can take interrupts in an endianness different from
MSR_LE.

This change has no functional impact.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/arch_dump.c   |  2 +-
 target/ppc/cpu.h         | 21 +++++++++++++--------
 target/ppc/excp_helper.c |  2 +-
 3 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index bb392f6d88..12cde198a3 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -237,7 +237,7 @@ int cpu_get_dump_info(ArchDumpInfo *info,
     info->d_machine = PPC_ELF_MACHINE;
     info->d_class = ELFCLASS;
 
-    if (ppc_interrupts_little_endian(cpu)) {
+    if (ppc_interrupts_little_endian(cpu, false)) {
         info->d_endian = ELFDATA2LSB;
     } else {
         info->d_endian = ELFDATA2MSB;
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index fc66c3561d..a991da4e74 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2723,19 +2723,24 @@ static inline bool ppc_has_spr(PowerPCCPU *cpu, int spr)
     return cpu->env.spr_cb[spr].name != NULL;
 }
 
-static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu)
+static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
 {
     PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);
+    CPUPPCState *env = &cpu->env;
+    bool ile = false;
 
-    /*
-     * Only models that have an LPCR and know about LPCR_ILE can do little
-     * endian.
-     */
-    if (pcc->lpcr_mask & LPCR_ILE) {
-        return !!(cpu->env.spr[SPR_LPCR] & LPCR_ILE);
+    if (hv) {
+        if (is_isa300(pcc)) {
+            ile = !!(env->spr[SPR_HID0] & HID0_POWER9_HILE);
+        } else {
+            ile = !!(env->spr[SPR_HID0] & HID0_HILE);
+        }
+
+    } else if (pcc->lpcr_mask & LPCR_ILE) {
+        ile = !!(env->spr[SPR_LPCR] & LPCR_ILE);
     }
 
-    return false;
+    return ile;
 }
 
 void dump_mmu(CPUPPCState *env);
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 160e06e3a3..0dbadc5d07 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -997,7 +997,7 @@ void ppc_cpu_do_fwnmi_machine_check(CPUState *cs, target_ulong vector)
      */
     msr = (1ULL << MSR_ME);
     msr |= env->msr & (1ULL << MSR_SF);
-    if (ppc_interrupts_little_endian(cpu)) {
+    if (ppc_interrupts_little_endian(cpu, false)) {
         msr |= (1ULL << MSR_LE);
     }
 
-- 
2.33.1

[PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Fabiano Rosas]

The ppc_interrupts_little_endian function is suitable for determining
the endianness of interrupts for all Book3S CPUs.

(I'm keeping the MSR check for the rest of the CPUs, but it will go
away in the next patch.)

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 0dbadc5d07..5d31940426 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
      * CPU, the HV mode, etc...
      */
 #ifdef TARGET_PPC64
-    if (excp_model == POWERPC_EXCP_POWER7) {
-        if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
-            new_msr |= (target_ulong)1 << MSR_LE;
-        }
-    } else if (excp_model == POWERPC_EXCP_POWER8) {
-        if (new_msr & MSR_HVB) {
-            if (env->spr[SPR_HID0] & HID0_HILE) {
-                new_msr |= (target_ulong)1 << MSR_LE;
-            }
-        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
-            new_msr |= (target_ulong)1 << MSR_LE;
-        }
-    } else if (excp_model == POWERPC_EXCP_POWER9 ||
-               excp_model == POWERPC_EXCP_POWER10) {
-        if (new_msr & MSR_HVB) {
-            if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
-                new_msr |= (target_ulong)1 << MSR_LE;
-            }
-        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
+    if (excp_model >= POWERPC_EXCP_970) {
+        if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
             new_msr |= (target_ulong)1 << MSR_LE;
         }
     } else if (msr_ile) {
-- 
2.33.1

[PATCH 6/9] target/ppc: powerpc_excp: Preserve MSR_LE bit

[Fabiano Rosas]

We currently clear MSR_LE when copying bits from env->msr to
new_msr. However, for CPUs that do not have LPCR_ILE we always set
new_msr[LE] according to env->msr[LE]. And for CPUs that do have ILE
support we need to check LPCR/HID0 anyway, so there's no need to clear
the bit when copying.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 5d31940426..e56ddbe5d5 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -348,10 +348,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     }
 
     /*
-     * new interrupt handler msr preserves existing HV and ME unless
-     * explicitly overriden
+     * new interrupt handler msr preserves existing HV, ME and LE
+     * unless explicitly overriden.
      */
-    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
+    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB | MSR_LE);
 
     /* target registers */
     srr0 = SPR_SRR0;
@@ -763,13 +763,9 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     if (excp_model >= POWERPC_EXCP_970) {
         if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
             new_msr |= (target_ulong)1 << MSR_LE;
+        } else {
+            new_msr &= ~((target_ulong)1 << MSR_LE);
         }
-    } else if (msr_ile) {
-        new_msr |= (target_ulong)1 << MSR_LE;
-    }
-#else
-    if (msr_ile) {
-        new_msr |= (target_ulong)1 << MSR_LE;
     }
 #endif
 
-- 
2.33.1

[PATCH 7/9] target/ppc: powerpc_excp: Move ILE setting into a function

[Fabiano Rosas]

Move the ILE code into a separate function similarly to what we do for
AIL.

This leaves the excp_model check behind because it will go away when
we split powerpc_excp.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index e56ddbe5d5..d7e087f2f6 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -290,6 +290,17 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
 #endif
 }
 
+static void ppc_excp_toggle_ile(PowerPCCPU *cpu, target_ulong *new_msr)
+{
+#ifdef TARGET_PPC64
+    if (ppc_interrupts_little_endian(cpu, !!(*new_msr & MSR_HVB))) {
+        *new_msr |= (target_ulong)1 << MSR_LE;
+    } else {
+        *new_msr &= ~((target_ulong)1 << MSR_LE);
+    }
+#endif
+}
+
 static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
                                           target_ulong vector, target_ulong msr)
 {
@@ -756,18 +767,12 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     }
 
     /*
-     * Sort out endianness of interrupt, this differs depending on the
-     * CPU, the HV mode, etc...
+     * We preserve MSR_LE, but some CPUs can take interrupts in a
+     * different endianness.
      */
-#ifdef TARGET_PPC64
     if (excp_model >= POWERPC_EXCP_970) {
-        if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
-            new_msr |= (target_ulong)1 << MSR_LE;
-        } else {
-            new_msr &= ~((target_ulong)1 << MSR_LE);
-        }
+        ppc_excp_toggle_ile(cpu, &new_msr);
     }
-#endif
 
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_BOOKE) {
-- 
2.33.1

[PATCH 8/9] target/ppc: powerpc_excp: Move AIL under a Book3s block

[Fabiano Rosas]

AIL only applies for Book3s CPUs, so move it along with ILE. This
moves ILE further down in the file because the AIL function can alter
vector so we cannot move it up.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index d7e087f2f6..a4787c3ae2 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -766,14 +766,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
         }
     }
 
-    /*
-     * We preserve MSR_LE, but some CPUs can take interrupts in a
-     * different endianness.
-     */
-    if (excp_model >= POWERPC_EXCP_970) {
-        ppc_excp_toggle_ile(cpu, &new_msr);
-    }
-
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_BOOKE) {
         if (env->spr[SPR_BOOKE_EPCR] & EPCR_ICM) {
@@ -799,8 +791,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
         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);
+    if (excp_model >= POWERPC_EXCP_970) {
+        /*
+         * We preserve MSR_LE, but some CPUs can take interrupts in a
+         * different endianness.
+         */
+        ppc_excp_toggle_ile(cpu, &new_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);
 }
-- 
2.33.1

[PATCH 9/9] target/ppc: Introduce a wrapper for powerpc_excp

[Fabiano Rosas]

Next patches will split powerpc_excp in multiple family specific
handlers. This patch adds a wrapper to make the transition clearer.

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index a4787c3ae2..15c492a934 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -336,7 +336,7 @@ static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
  * Note that this function should be greatly optimized when called
  * with a constant excp, from ppc_hw_interrupt
  */
-static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
+static inline void powerpc_excp_legacy(PowerPCCPU *cpu, int excp)
 {
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
@@ -805,6 +805,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
     powerpc_set_excp_state(cpu, vector, new_msr);
 }
 
+static void powerpc_excp(PowerPCCPU *cpu, int excp)
+{
+    CPUPPCState *env = &cpu->env;
+
+    switch (env->excp_model) {
+    default:
+        powerpc_excp_legacy(cpu, excp);
+    }
+}
+
 void ppc_cpu_do_interrupt(CPUState *cs)
 {
     PowerPCCPU *cpu = POWERPC_CPU(cs);
-- 
2.33.1

Re: [PATCH 1/9] target/ppc: powerpc_excp: Extract software TLB logging into a function

[Cédric Le Goater]

Hello Fabiano,

On 1/3/22 23:07, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>   target/ppc/excp_helper.c | 63 +++++++++++++++++++++++-----------------
>   1 file changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index c7e55800af..002a42261b 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -63,6 +63,41 @@ static inline void dump_hcall(CPUPPCState *env)
>                     env->nip);
>   }
>   
> +static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
> +{
> +#if defined(DEBUG_SOFTWARE_TLB)

I would get rid of the define

> +    const char *es;
> +    target_ulong *miss, *cmp;
> +    int en;
> +
> +    if (!qemu_log_enabled()) {
> +        return;
> +    }
> +
> +    if (excp == POWERPC_EXCP_IFTLB) {
> +        es = "I";
> +        en = 'I';
> +        miss = &env->spr[SPR_IMISS];
> +        cmp = &env->spr[SPR_ICMP];
> +    } else {
> +        if (excp == POWERPC_EXCP_DLTLB) {
> +            es = "DL";
> +        } else {
> +            es = "DS";
> +        }
> +        en = 'D';
> +        miss = &env->spr[SPR_DMISS];
> +        cmp = &env->spr[SPR_DCMP];
> +    }

and simply use :

   qemu_log_mask(CPU_LOG_MMU, ...

Thanks,

C.

> +    qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
> +             TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
> +             TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
> +             env->spr[SPR_HASH1], env->spr[SPR_HASH2],
> +             env->error_code);
> +#endif
> +}
> +
> +
>   static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>                                   target_ulong *msr)
>   {
> @@ -704,34 +739,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>               }
>               /* fall through */
>           case POWERPC_EXCP_7x5:
> -#if defined(DEBUG_SOFTWARE_TLB)
> -            if (qemu_log_enabled()) {
> -                const char *es;
> -                target_ulong *miss, *cmp;
> -                int en;
> +            ppc_excp_debug_sw_tlb(env, excp);
>   
> -                if (excp == POWERPC_EXCP_IFTLB) {
> -                    es = "I";
> -                    en = 'I';
> -                    miss = &env->spr[SPR_IMISS];
> -                    cmp = &env->spr[SPR_ICMP];
> -                } else {
> -                    if (excp == POWERPC_EXCP_DLTLB) {
> -                        es = "DL";
> -                    } else {
> -                        es = "DS";
> -                    }
> -                    en = 'D';
> -                    miss = &env->spr[SPR_DMISS];
> -                    cmp = &env->spr[SPR_DCMP];
> -                }
> -                qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
> -                         TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
> -                         TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
> -                         env->spr[SPR_HASH1], env->spr[SPR_HASH2],
> -                         env->error_code);
> -            }
> -#endif
>               msr |= env->crf[0] << 28;
>               msr |= env->error_code; /* key, D/I, S/L bits */
>               /* Set way using a LRU mechanism */
> 

Re: [PATCH 2/9] target/ppc: powerpc_excp: Keep 60x soft MMU logs active

[Cédric Le Goater]

On 1/3/22 23:07, Fabiano Rosas wrote:
> Remove the compile time definition and make the logging be controlled
> by the `-d mmu` option in the cmdline.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.


> ---
>   target/ppc/excp_helper.c | 15 ++++++---------
>   1 file changed, 6 insertions(+), 9 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 002a42261b..4769abfb0c 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -30,8 +30,6 @@
>   #include "exec/cpu_ldst.h"
>   #endif
>   
> -/* #define DEBUG_SOFTWARE_TLB */
> -
>   /*****************************************************************************/
>   /* Exception processing */
>   #if !defined(CONFIG_USER_ONLY)
> @@ -65,7 +63,6 @@ static inline void dump_hcall(CPUPPCState *env)
>   
>   static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
>   {
> -#if defined(DEBUG_SOFTWARE_TLB)
>       const char *es;
>       target_ulong *miss, *cmp;
>       int en;
> @@ -89,12 +86,12 @@ static void ppc_excp_debug_sw_tlb(CPUPPCState *env, int excp)
>           miss = &env->spr[SPR_DMISS];
>           cmp = &env->spr[SPR_DCMP];
>       }
> -    qemu_log("6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
> -             TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
> -             TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
> -             env->spr[SPR_HASH1], env->spr[SPR_HASH2],
> -             env->error_code);
> -#endif
> +
> +    qemu_log_mask(CPU_LOG_MMU, "6xx %sTLB miss: %cM " TARGET_FMT_lx " %cC "
> +                  TARGET_FMT_lx " H1 " TARGET_FMT_lx " H2 "
> +                  TARGET_FMT_lx " %08x\n", es, en, *miss, en, *cmp,
> +                  env->spr[SPR_HASH1], env->spr[SPR_HASH2],
> +                  env->error_code);
>   }
>   
>   
> 

Re: [PATCH 3/9] target/ppc: powerpc_excp: Group unimplemented exceptions

[Cédric Le Goater]

On 1/3/22 23:07, Fabiano Rosas wrote:
> (I'll alter this to use powerpc_excp_name once it is merged)

Sure.

Reviewed-by: Cédric Le Goater <clg@kaod.org>

Thanks,

C.

> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>   target/ppc/excp_helper.c | 76 ++++------------------------------------
>   1 file changed, 7 insertions(+), 69 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 4769abfb0c..160e06e3a3 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -628,23 +628,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>       case POWERPC_EXCP_SPEU:   /* SPE/embedded floating-point unavailable/VPU  */
>           env->spr[SPR_BOOKE_ESR] = ESR_SPV;
>           break;
> -    case POWERPC_EXCP_EFPDI:     /* Embedded floating-point data interrupt   */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Embedded floating point data exception "
> -                  "is not implemented yet !\n");
> -        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
> -        break;
> -    case POWERPC_EXCP_EFPRI:     /* Embedded floating-point round interrupt  */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Embedded floating point round exception "
> -                  "is not implemented yet !\n");
> -        env->spr[SPR_BOOKE_ESR] = ESR_SPV;
> -        break;
> -    case POWERPC_EXCP_EPERFM:    /* Embedded performance monitor interrupt   */
> -        /* XXX: TODO */
> -        cpu_abort(cs,
> -                  "Performance counter exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_DOORI:     /* Embedded doorbell interrupt              */
>           break;
>       case POWERPC_EXCP_DOORCI:    /* Embedded doorbell critical interrupt     */
> @@ -709,19 +692,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>       case POWERPC_EXCP_PIT:       /* Programmable interval timer interrupt    */
>           trace_ppc_excp_print("PIT");
>           break;
> -    case POWERPC_EXCP_IO:        /* IO error exception                       */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "601 IO error exception is not implemented yet !\n");
> -        break;
> -    case POWERPC_EXCP_RUNM:      /* Run mode exception                       */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "601 run mode exception is not implemented yet !\n");
> -        break;
> -    case POWERPC_EXCP_EMUL:      /* Emulation trap exception                 */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "602 emulation trap exception "
> -                  "is not implemented yet !\n");
> -        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                      */
> @@ -748,56 +718,24 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>               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          */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Floating point assist exception "
> -                  "is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_DABR:      /* Data address breakpoint                  */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "DABR exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_IABR:      /* Instruction address breakpoint           */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "IABR exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_SMI:       /* System management interrupt              */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "SMI exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_THERM:     /* Thermal interrupt                        */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Thermal management exception "
> -                  "is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_PERFM:     /* Embedded performance monitor interrupt   */
> -        /* XXX: TODO */
> -        cpu_abort(cs,
> -                  "Performance counter exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_VPUA:      /* Vector assist exception                  */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "VPU assist exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_SOFTP:     /* Soft patch exception                     */
> -        /* XXX: TODO */
> -        cpu_abort(cs,
> -                  "970 soft-patch exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_MAINT:     /* Maintenance exception                    */
> -        /* XXX: TODO */
> -        cpu_abort(cs,
> -                  "970 maintenance exception is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_MEXTBR:    /* Maskable external breakpoint             */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Maskable external exception "
> -                  "is not implemented yet !\n");
> -        break;
>       case POWERPC_EXCP_NMEXTBR:   /* Non maskable external breakpoint         */
> -        /* XXX: TODO */
> -        cpu_abort(cs, "Non maskable external exception "
> -                  "is not implemented yet !\n");
> +        cpu_abort(cs, "Exception not implemented\n");
>           break;
>       default:
>       excp_invalid:
> 

Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Cédric Le Goater]

On 1/3/22 23:07, Fabiano Rosas wrote:
> The ppc_interrupts_little_endian function is suitable for determining
> the endianness of interrupts for all Book3S CPUs.
> 
> (I'm keeping the MSR check for the rest of the CPUs, but it will go
> away in the next patch.)
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>   target/ppc/excp_helper.c | 21 ++-------------------
>   1 file changed, 2 insertions(+), 19 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 0dbadc5d07..5d31940426 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>        * CPU, the HV mode, etc...
>        */
>   #ifdef TARGET_PPC64
> -    if (excp_model == POWERPC_EXCP_POWER7) {
> -        if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
> -            new_msr |= (target_ulong)1 << MSR_LE;
> -        }
> -    } else if (excp_model == POWERPC_EXCP_POWER8) {
> -        if (new_msr & MSR_HVB) {
> -            if (env->spr[SPR_HID0] & HID0_HILE) {
> -                new_msr |= (target_ulong)1 << MSR_LE;
> -            }
> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
> -            new_msr |= (target_ulong)1 << MSR_LE;
> -        }
> -    } else if (excp_model == POWERPC_EXCP_POWER9 ||
> -               excp_model == POWERPC_EXCP_POWER10) {
> -        if (new_msr & MSR_HVB) {
> -            if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
> -                new_msr |= (target_ulong)1 << MSR_LE;
> -            }
> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
> +    if (excp_model >= POWERPC_EXCP_970) {

why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.

Thanks,

C.


> +        if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
>               new_msr |= (target_ulong)1 << MSR_LE;
>           }
>       } else if (msr_ile) {
> 

Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Fabiano Rosas]

Cédric Le Goater <clg@kaod.org> writes:

> On 1/3/22 23:07, Fabiano Rosas wrote:
>> The ppc_interrupts_little_endian function is suitable for determining
>> the endianness of interrupts for all Book3S CPUs.
>> 
>> (I'm keeping the MSR check for the rest of the CPUs, but it will go
>> away in the next patch.)
>> 
>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>> ---
>>   target/ppc/excp_helper.c | 21 ++-------------------
>>   1 file changed, 2 insertions(+), 19 deletions(-)
>> 
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 0dbadc5d07..5d31940426 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>>        * CPU, the HV mode, etc...
>>        */
>>   #ifdef TARGET_PPC64
>> -    if (excp_model == POWERPC_EXCP_POWER7) {
>> -        if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
>> -            new_msr |= (target_ulong)1 << MSR_LE;
>> -        }
>> -    } else if (excp_model == POWERPC_EXCP_POWER8) {
>> -        if (new_msr & MSR_HVB) {
>> -            if (env->spr[SPR_HID0] & HID0_HILE) {
>> -                new_msr |= (target_ulong)1 << MSR_LE;
>> -            }
>> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>> -            new_msr |= (target_ulong)1 << MSR_LE;
>> -        }
>> -    } else if (excp_model == POWERPC_EXCP_POWER9 ||
>> -               excp_model == POWERPC_EXCP_POWER10) {
>> -        if (new_msr & MSR_HVB) {
>> -            if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
>> -                new_msr |= (target_ulong)1 << MSR_LE;
>> -            }
>> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>> +    if (excp_model >= POWERPC_EXCP_970) {
>
> why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
>

Because the 970 exception model covers POWER5P as well which has ILE.

And looking at cpu_init.c, POWER5 uses a bunch of 970 functions. And
POWER7 uses the POWER5 ones. So we kind of have a dependency chain
there. That is why I'm always handling ">= 970" as "books".

Re: [PATCH 5/9] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Cédric Le Goater]

On 1/4/22 15:11, Fabiano Rosas wrote:
> Cédric Le Goater <clg@kaod.org> writes:
> 
>> On 1/3/22 23:07, Fabiano Rosas wrote:
>>> The ppc_interrupts_little_endian function is suitable for determining
>>> the endianness of interrupts for all Book3S CPUs.
>>>
>>> (I'm keeping the MSR check for the rest of the CPUs, but it will go
>>> away in the next patch.)
>>>
>>> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
>>> ---
>>>    target/ppc/excp_helper.c | 21 ++-------------------
>>>    1 file changed, 2 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>>> index 0dbadc5d07..5d31940426 100644
>>> --- a/target/ppc/excp_helper.c
>>> +++ b/target/ppc/excp_helper.c
>>> @@ -760,25 +760,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>>>         * CPU, the HV mode, etc...
>>>         */
>>>    #ifdef TARGET_PPC64
>>> -    if (excp_model == POWERPC_EXCP_POWER7) {
>>> -        if (!(new_msr & MSR_HVB) && (env->spr[SPR_LPCR] & LPCR_ILE)) {
>>> -            new_msr |= (target_ulong)1 << MSR_LE;
>>> -        }
>>> -    } else if (excp_model == POWERPC_EXCP_POWER8) {
>>> -        if (new_msr & MSR_HVB) {
>>> -            if (env->spr[SPR_HID0] & HID0_HILE) {
>>> -                new_msr |= (target_ulong)1 << MSR_LE;
>>> -            }
>>> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>>> -            new_msr |= (target_ulong)1 << MSR_LE;
>>> -        }
>>> -    } else if (excp_model == POWERPC_EXCP_POWER9 ||
>>> -               excp_model == POWERPC_EXCP_POWER10) {
>>> -        if (new_msr & MSR_HVB) {
>>> -            if (env->spr[SPR_HID0] & HID0_POWER9_HILE) {
>>> -                new_msr |= (target_ulong)1 << MSR_LE;
>>> -            }
>>> -        } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>>> +    if (excp_model >= POWERPC_EXCP_970) {
>>
>> why include POWERPC_EXCP_970 ? These CPUs do not support Little Endian.
>>
> 
> Because the 970 exception model covers POWER5P as well which has ILE.
we need to untangle this first.

POWERPC_EXCP_970 is checked in dbcz and the HID5 bits are specific to 970.
May be add a POWERPC_EXCP_POWER5P ?

> And looking at cpu_init.c, POWER5 uses a bunch of 970 functions. And
> POWER7 uses the POWER5 ones. So we kind of have a dependency chain
> there. That is why I'm always handling ">= 970" as "books".
> 

This is a mess. We also have is_book3s_arch2x() but it will not apply
here.

C.

Re: [PATCH 6/9] target/ppc: powerpc_excp: Preserve MSR_LE bit

[Fabiano Rosas]

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

> We currently clear MSR_LE when copying bits from env->msr to
> new_msr. However, for CPUs that do not have LPCR_ILE we always set
> new_msr[LE] according to env->msr[LE]. And for CPUs that do have ILE
> support we need to check LPCR/HID0 anyway, so there's no need to clear
> the bit when copying.
>
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> ---
>  target/ppc/excp_helper.c | 14 +++++---------
>  1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 5d31940426..e56ddbe5d5 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -348,10 +348,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>      }
>  
>      /*
> -     * new interrupt handler msr preserves existing HV and ME unless
> -     * explicitly overriden
> +     * new interrupt handler msr preserves existing HV, ME and LE
> +     * unless explicitly overriden.
>       */
> -    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB);
> +    new_msr = env->msr & (((target_ulong)1 << MSR_ME) | MSR_HVB | MSR_LE);
>  
>      /* target registers */
>      srr0 = SPR_SRR0;
> @@ -763,13 +763,9 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp)
>      if (excp_model >= POWERPC_EXCP_970) {
>          if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
>              new_msr |= (target_ulong)1 << MSR_LE;
> +        } else {
> +            new_msr &= ~((target_ulong)1 << MSR_LE);
>          }
> -    } else if (msr_ile) {
> -        new_msr |= (target_ulong)1 << MSR_LE;
> -    }
> -#else
> -    if (msr_ile) {
> -        new_msr |= (target_ulong)1 << MSR_LE;
>      }
>  #endif

This patch is incorrect, don't bother with it. I misread the msr_ile
macro as msr_le. I'll think of an alternative.