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

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

[Fabiano Rosas]

This is version two of the second batch of cleanups to powerpc_excp. I
have started using powerpc_excp_name for the unimplemented messages,
solved the POWERPC_EXCP_970 conundrum by ignoring it and left AIL and
ILE separated because that can be done in the Book3S series where it
will make more sense.

- patch 1,2: unchanged;

- patch 3: group unimplemented messages and use powerpc_excp_name;

- patch 4: add HV support to ppc_interrupts_little_endian but now
           check has_hv_mode in case anyone passes hv=true by mistake;

- patch 5: add MSR_ILE support to ppc_interrupts_little_endian, this
           avoids having to check excp_model in the next patch;

- patch 6: use ppc_interrupts_little_endian in powerpc_excp but this
           time not only for >= 970, but for all CPUs;

- patch 7: unchanged.

this series v1:
https://lists.nongnu.org/archive/html/qemu-ppc/2022-01/msg00054.html

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 [already merged]:
https://mail.gnu.org/archive/html/qemu-ppc/2021-12/msg00696.html

Fabiano Rosas (7):
  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: Add MSR_ILE support to ppc_interrupts_little_endian
  target/ppc: Use ppc_interrupts_little_endian in powerpc_excp
  target/ppc: Introduce a wrapper for powerpc_excp

 target/ppc/arch_dump.c   |   2 +-
 target/ppc/cpu.h         |  25 ++++--
 target/ppc/excp_helper.c | 184 ++++++++++++---------------------------
 3 files changed, 74 insertions(+), 137 deletions(-)

-- 
2.33.1

[PATCH v2 1/7] 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 a779dc936a..2c5d5470de 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -135,6 +135,41 @@ static 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)
 {
@@ -777,34 +812,8 @@ static 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

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

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:23PM -0300, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

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

> ---
>  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 a779dc936a..2c5d5470de 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -135,6 +135,41 @@ static 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)
>  {
> @@ -777,34 +812,8 @@ static 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 */

-- 
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 --]

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

[Richard Henderson]

On 1/5/22 12:40 PM, 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 a779dc936a..2c5d5470de 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -135,6 +135,41 @@ static 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;
> +    }
> +

Because code movement,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

Better is qemu_loglevel_mask(CPU_LOG_MMU)
and perhaps then you can remove the ifdef.


r~

[PATCH v2 2/7] 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>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 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 2c5d5470de..ce86b2ae37 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)
@@ -137,7 +135,6 @@ static 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;
@@ -161,12 +158,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

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

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:24PM -0300, 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>

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

I'm guessing from the text here that this logging is specific to the
60x software TLB implementation rather than (say) the 40x, 44x, 8xx or
freescale implementations.  Changing the function name to reflect that
might be a nice future change.

> ---
>  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 2c5d5470de..ce86b2ae37 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)
> @@ -137,7 +135,6 @@ static 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;
> @@ -161,12 +158,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);
>  }
>  
>  

-- 
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 --]

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

[Richard Henderson]

On 1/5/22 12:40 PM, 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>
> ---
>   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 2c5d5470de..ce86b2ae37 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)
> @@ -137,7 +135,6 @@ static 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;
> @@ -161,12 +158,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);

Ah, then my comment wrt patch 1 applies to this one -- use the proper filter function at 
the top of this one, before all of the data collection for the actual logging.


r~

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

[Fabiano Rosas]

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
Reviewed-by: Cédric Le Goater <clg@kaod.org>
---
 target/ppc/excp_helper.c | 77 +++++-----------------------------------
 1 file changed, 8 insertions(+), 69 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index ce86b2ae37..fa41f8048d 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -701,23 +701,6 @@ static 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     */
@@ -782,19 +765,6 @@ static 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                      */
@@ -821,56 +791,25 @@ static 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, "%s exception not implemented\n",
+                  powerpc_excp_name(excp));
         break;
     default:
     excp_invalid:
-- 
2.33.1

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

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:25PM -0300, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

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

> ---
>  target/ppc/excp_helper.c | 77 +++++-----------------------------------
>  1 file changed, 8 insertions(+), 69 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index ce86b2ae37..fa41f8048d 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -701,23 +701,6 @@ static 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     */
> @@ -782,19 +765,6 @@ static 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                      */
> @@ -821,56 +791,25 @@ static 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, "%s exception not implemented\n",
> +                  powerpc_excp_name(excp));
>          break;
>      default:
>      excp_invalid:

-- 
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 --]

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

[Richard Henderson]

On 1/5/22 12:40 PM, Fabiano Rosas wrote:
> Signed-off-by: Fabiano Rosas<farosas@linux.ibm.com>
> Reviewed-by: Cédric Le Goater<clg@kaod.org>
> ---
>   target/ppc/excp_helper.c | 77 +++++-----------------------------------
>   1 file changed, 8 insertions(+), 69 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v2 4/7] 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         | 23 +++++++++++++++--------
 target/ppc/excp_helper.c |  2 +-
 3 files changed, 17 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 f20d4ffa6d..a6fc857ad4 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2728,20 +2728,27 @@ 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)
+#if !defined(CONFIG_USER_ONLY)
+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 && env->has_hv_mode) {
+        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;
 }
+#endif
 
 void dump_mmu(CPUPPCState *env);
 
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index fa41f8048d..92953af896 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -1071,7 +1071,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

Re: [PATCH v2 4/7] target/ppc: Add HV support to ppc_interrupts_little_endian

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:26PM -0300, Fabiano Rosas wrote:
> 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>

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

With one nit you might want to look at later:

> ---
>  target/ppc/arch_dump.c   |  2 +-
>  target/ppc/cpu.h         | 23 +++++++++++++++--------
>  target/ppc/excp_helper.c |  2 +-
>  3 files changed, 17 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)) {

I'm wondering if using hv==false here is actually correct, and AFAICT
it probably is for spapr, but not for powernv.  So I'm wondering if we
should actually test cpu->vhyp here to get the right value for powernv
as well.

>          info->d_endian = ELFDATA2LSB;
>      } else {
>          info->d_endian = ELFDATA2MSB;
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index f20d4ffa6d..a6fc857ad4 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2728,20 +2728,27 @@ 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)
> +#if !defined(CONFIG_USER_ONLY)
> +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 && env->has_hv_mode) {
> +        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;
>  }
> +#endif
>  
>  void dump_mmu(CPUPPCState *env);
>  
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index fa41f8048d..92953af896 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1071,7 +1071,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);
>      }
>  

-- 
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 --]

Re: [PATCH v2 4/7] target/ppc: Add HV support to ppc_interrupts_little_endian

[Cédric Le Goater]

On 1/6/22 06:30, David Gibson wrote:
> On Wed, Jan 05, 2022 at 05:40:26PM -0300, Fabiano Rosas wrote:
>> 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>
> 
> Reviewed-by: David Gibson <david@gibson.dropbear.id.au>
> 
> With one nit you might want to look at later:
> 
>> ---
>>   target/ppc/arch_dump.c   |  2 +-
>>   target/ppc/cpu.h         | 23 +++++++++++++++--------
>>   target/ppc/excp_helper.c |  2 +-
>>   3 files changed, 17 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)) {
> 
> I'm wondering if using hv==false here is actually correct, and AFAICT
> it probably is for spapr, but not for powernv.  So I'm wondering if we
> should actually test cpu->vhyp here to get the right value for powernv
> as well.

yes. 'cpu->vhyp' or 'env->has_hv_mode' or 'env->msr_mask & MSR_HVB'

The use of 'env->msr_mask & MSR_HVB' would need a cleanup. env->has_hv_mode
is equivalent. May be a helper to rule them both would be better.

Thanks,

C.

> 
>>           info->d_endian = ELFDATA2LSB;
>>       } else {
>>           info->d_endian = ELFDATA2MSB;
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index f20d4ffa6d..a6fc857ad4 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -2728,20 +2728,27 @@ 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)
>> +#if !defined(CONFIG_USER_ONLY)
>> +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 && env->has_hv_mode) {
>> +        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;
>>   }
>> +#endif
>>   
>>   void dump_mmu(CPUPPCState *env);
>>   
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index fa41f8048d..92953af896 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -1071,7 +1071,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);
>>       }
>>   
> 

[PATCH v2 5/7] target/ppc: Add MSR_ILE support to ppc_interrupts_little_endian

[Fabiano Rosas]

Some CPUs set ILE via an MSR bit. We can make
ppc_interrupts_little_endian handle that case as well. Now we have a
centralized way of determining the endianness of interrupts.

This change has no functional impact.

Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>
---
 target/ppc/cpu.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index a6fc857ad4..f99cd0ea92 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2733,7 +2733,7 @@ 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;
+    bool ile;
 
     if (hv && env->has_hv_mode) {
         if (is_isa300(pcc)) {
@@ -2744,6 +2744,8 @@ static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
 
     } else if (pcc->lpcr_mask & LPCR_ILE) {
         ile = !!(env->spr[SPR_LPCR] & LPCR_ILE);
+    } else {
+        ile = !!(msr_ile);
     }
 
     return ile;
-- 
2.33.1

Re: [PATCH v2 5/7] target/ppc: Add MSR_ILE support to ppc_interrupts_little_endian

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:27PM -0300, Fabiano Rosas wrote:
> Some CPUs set ILE via an MSR bit. We can make
> ppc_interrupts_little_endian handle that case as well. Now we have a
> centralized way of determining the endianness of interrupts.
> 
> This change has no functional impact.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

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

> ---
>  target/ppc/cpu.h | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index a6fc857ad4..f99cd0ea92 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2733,7 +2733,7 @@ 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;
> +    bool ile;
>  
>      if (hv && env->has_hv_mode) {
>          if (is_isa300(pcc)) {
> @@ -2744,6 +2744,8 @@ static inline bool ppc_interrupts_little_endian(PowerPCCPU *cpu, bool hv)
>  
>      } else if (pcc->lpcr_mask & LPCR_ILE) {
>          ile = !!(env->spr[SPR_LPCR] & LPCR_ILE);
> +    } else {
> +        ile = !!(msr_ile);
>      }
>  
>      return ile;

-- 
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 --]

[PATCH v2 6/7] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[Fabiano Rosas]

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

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

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 92953af896..d16bdf9283 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -833,36 +833,9 @@ static void powerpc_excp(PowerPCCPU *cpu, int excp)
      * Sort out endianness of interrupt, this differs depending on the
      * 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) {
-            new_msr |= (target_ulong)1 << MSR_LE;
-        }
-    } else if (msr_ile) {
+    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
         new_msr |= (target_ulong)1 << MSR_LE;
     }
-#else
-    if (msr_ile) {
-        new_msr |= (target_ulong)1 << MSR_LE;
-    }
-#endif
 
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_BOOKE) {
-- 
2.33.1

Re: [PATCH v2 6/7] target/ppc: Use ppc_interrupts_little_endian in powerpc_excp

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:28PM -0300, Fabiano Rosas wrote:
> The ppc_interrupts_little_endian function is now suitable for
> determining the endianness of interrupts for all CPUs.
> 
> Signed-off-by: Fabiano Rosas <farosas@linux.ibm.com>

Nice!

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

> ---
>  target/ppc/excp_helper.c | 29 +----------------------------
>  1 file changed, 1 insertion(+), 28 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 92953af896..d16bdf9283 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -833,36 +833,9 @@ static void powerpc_excp(PowerPCCPU *cpu, int excp)
>       * Sort out endianness of interrupt, this differs depending on the
>       * 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) {
> -            new_msr |= (target_ulong)1 << MSR_LE;
> -        }
> -    } else if (msr_ile) {
> +    if (ppc_interrupts_little_endian(cpu, !!(new_msr & MSR_HVB))) {
>          new_msr |= (target_ulong)1 << MSR_LE;
>      }
> -#else
> -    if (msr_ile) {
> -        new_msr |= (target_ulong)1 << MSR_LE;
> -    }
> -#endif
>  
>  #if defined(TARGET_PPC64)
>      if (excp_model == POWERPC_EXCP_BOOKE) {

-- 
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 --]

[PATCH v2 7/7] 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 d16bdf9283..ea854a868e 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -397,7 +397,7 @@ static 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 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;
@@ -868,6 +868,16 @@ static 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 v2 7/7] target/ppc: Introduce a wrapper for powerpc_excp

[David Gibson]

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

On Wed, Jan 05, 2022 at 05:40:29PM -0300, Fabiano Rosas wrote:
> 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>

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

> ---
>  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 d16bdf9283..ea854a868e 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -397,7 +397,7 @@ static 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 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;
> @@ -868,6 +868,16 @@ static 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);

-- 
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 --]