[PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model

[PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model

[Nicholas Piggin]

Here's a rollup of where this ended up, hopefully it suits everyone's
preference. Thanks for the review and catching several issues.

Patches 1-3 are unchanged except for minor comment and changelog tweaks,
patch 4 contains fixes for the issues Cedric noticed.

Thanks,
Nick

Nicholas Piggin (4):
  target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour
  target/ppc: POWER10 supports scv
  target/ppc: Rework AIL logic in interrupt delivery
  target/ppc: Add POWER10 exception model

 hw/ppc/spapr_hcall.c            |   8 +-
 target/ppc/cpu-qom.h            |   2 +
 target/ppc/cpu.h                |  13 +--
 target/ppc/excp_helper.c        | 199 +++++++++++++++++++++++---------
 target/ppc/translate.c          |   3 +-
 target/ppc/translate_init.c.inc |   6 +-
 6 files changed, 163 insertions(+), 68 deletions(-)

-- 
2.23.0

[PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[Nicholas Piggin]

ISA v3.0 radix guest execution has a quirk in AIL behaviour such that
the LPCR[AIL] value can apply to hypervisor interrupts.

This affects machines that emulate HV=1 mode (i.e., powernv9).

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/excp_helper.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 85de7e6c90..b8881c0f85 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -791,14 +791,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
 #endif
 
     /*
-     * AIL only works if there is no HV transition and we are running
-     * with translations enabled
+     * AIL only works if MSR[IR] and MSR[DR] are both enabled.
      */
-    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1) ||
-        ((new_msr & MSR_HVB) && !(msr & MSR_HVB))) {
+    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
         ail = 0;
     }
 
+    /*
+     * AIL does not work if there is a MSR[HV] 0->1 transition and the
+     * partition is in HPT mode. For radix guests, such interrupts are
+     * allowed to be delivered to the hypervisor in ail mode.
+     */
+    if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
+        if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
+            ail = 0;
+        }
+    }
+
     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
         cpu_abort(cs, "Raised an exception without defined vector %d\n",
-- 
2.23.0

Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[Fabiano Rosas]

Nicholas Piggin <npiggin@gmail.com> writes:

> ISA v3.0 radix guest execution has a quirk in AIL behaviour such that
> the LPCR[AIL] value can apply to hypervisor interrupts.
>
> This affects machines that emulate HV=1 mode (i.e., powernv9).
>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

> ---
>  target/ppc/excp_helper.c | 17 +++++++++++++----
>  1 file changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 85de7e6c90..b8881c0f85 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -791,14 +791,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>  #endif
>  
>      /*
> -     * AIL only works if there is no HV transition and we are running
> -     * with translations enabled
> +     * AIL only works if MSR[IR] and MSR[DR] are both enabled.
>       */
> -    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1) ||
> -        ((new_msr & MSR_HVB) && !(msr & MSR_HVB))) {
> +    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
>          ail = 0;
>      }
>  
> +    /*
> +     * AIL does not work if there is a MSR[HV] 0->1 transition and the
> +     * partition is in HPT mode. For radix guests, such interrupts are
> +     * allowed to be delivered to the hypervisor in ail mode.
> +     */
> +    if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
> +        if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
> +            ail = 0;
> +        }
> +    }
> +
>      vector = env->excp_vectors[excp];
>      if (vector == (target_ulong)-1ULL) {
>          cpu_abort(cs, "Raised an exception without defined vector %d\n",

Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour

[David Gibson]

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

On Thu, Apr 15, 2021 at 09:12:21AM -0300, Fabiano Rosas wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
> 
> > ISA v3.0 radix guest execution has a quirk in AIL behaviour such that
> > the LPCR[AIL] value can apply to hypervisor interrupts.
> >
> > This affects machines that emulate HV=1 mode (i.e., powernv9).
> >
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> Reviewed-by: Fabiano Rosas <farosas@linux.ibm.com>

Applied to ppc-for-6.1.

> 
> > ---
> >  target/ppc/excp_helper.c | 17 +++++++++++++----
> >  1 file changed, 13 insertions(+), 4 deletions(-)
> >
> > diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> > index 85de7e6c90..b8881c0f85 100644
> > --- a/target/ppc/excp_helper.c
> > +++ b/target/ppc/excp_helper.c
> > @@ -791,14 +791,23 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
> >  #endif
> >  
> >      /*
> > -     * AIL only works if there is no HV transition and we are running
> > -     * with translations enabled
> > +     * AIL only works if MSR[IR] and MSR[DR] are both enabled.
> >       */
> > -    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1) ||
> > -        ((new_msr & MSR_HVB) && !(msr & MSR_HVB))) {
> > +    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
> >          ail = 0;
> >      }
> >  
> > +    /*
> > +     * AIL does not work if there is a MSR[HV] 0->1 transition and the
> > +     * partition is in HPT mode. For radix guests, such interrupts are
> > +     * allowed to be delivered to the hypervisor in ail mode.
> > +     */
> > +    if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
> > +        if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
> > +            ail = 0;
> > +        }
> > +    }
> > +
> >      vector = env->excp_vectors[excp];
> >      if (vector == (target_ulong)-1ULL) {
> >          cpu_abort(cs, "Raised an exception without defined vector %d\n",
> 

-- 
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 2/4] target/ppc: POWER10 supports scv

[Nicholas Piggin]

This must have slipped through the cracks between adding POWER10 support
and scv support.

Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 target/ppc/translate_init.c.inc | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index c03a7c4f52..70f9b9b150 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -9323,7 +9323,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
     pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
                  POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
                  POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
-                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
+                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
     pcc->l1_dcache_size = 0x8000;
     pcc->l1_icache_size = 0x8000;
     pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
-- 
2.23.0

Re: [EXTERNAL] [PATCH v2 2/4] target/ppc: POWER10 supports scv

[Cédric Le Goater]

On 4/15/21 7:42 AM, Nicholas Piggin wrote:
> This must have slipped through the cracks between adding POWER10 support
> and scv support.
> 
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

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


> ---
>  target/ppc/translate_init.c.inc | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index c03a7c4f52..70f9b9b150 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -9323,7 +9323,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
>                   POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
>                   POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> -                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
> +                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
>      pcc->l1_dcache_size = 0x8000;
>      pcc->l1_icache_size = 0x8000;
>      pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> 

Re: [EXTERNAL] [PATCH v2 2/4] target/ppc: POWER10 supports scv

[David Gibson]

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

On Thu, Apr 15, 2021 at 09:43:23AM +0200, Cédric Le Goater wrote:
> On 4/15/21 7:42 AM, Nicholas Piggin wrote:
> > This must have slipped through the cracks between adding POWER10 support
> > and scv support.
> > 
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>

Applied to ppc-for-6.1, thanks.

> 
> 
> > ---
> >  target/ppc/translate_init.c.inc | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> > index c03a7c4f52..70f9b9b150 100644
> > --- a/target/ppc/translate_init.c.inc
> > +++ b/target/ppc/translate_init.c.inc
> > @@ -9323,7 +9323,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
> >      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
> >                   POWERPC_FLAG_BE | POWERPC_FLAG_PMM |
> >                   POWERPC_FLAG_BUS_CLK | POWERPC_FLAG_CFAR |
> > -                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM;
> > +                 POWERPC_FLAG_VSX | POWERPC_FLAG_TM | POWERPC_FLAG_SCV;
> >      pcc->l1_dcache_size = 0x8000;
> >      pcc->l1_icache_size = 0x8000;
> >      pcc->interrupts_big_endian = ppc_cpu_interrupts_big_endian_lpcr;
> > 
> 

-- 
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 3/4] target/ppc: Rework AIL logic in interrupt delivery

[Nicholas Piggin]

The AIL logic is becoming unmanageable spread all over powerpc_excp(),
and it is slated to get even worse with POWER10 support.

Move it all to a new helper function.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_hcall.c            |   3 +-
 target/ppc/cpu.h                |   8 --
 target/ppc/excp_helper.c        | 159 ++++++++++++++++++++------------
 target/ppc/translate_init.c.inc |   2 +-
 4 files changed, 102 insertions(+), 70 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 7b5cd3553c..2fbe04a689 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1395,7 +1395,8 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
         return H_P4;
     }
 
-    if (mflags == AIL_RESERVED) {
+    if (mflags == 1) {
+        /* AIL=1 is reserved */
         return H_UNSUPPORTED_FLAG;
     }
 
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index e73416da68..5200a16d23 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -2375,14 +2375,6 @@ enum {
     HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
 };
 
-/* Alternate Interrupt Location (AIL) */
-enum {
-    AIL_NONE                = 0,
-    AIL_RESERVED            = 1,
-    AIL_0001_8000           = 2,
-    AIL_C000_0000_0000_4000 = 3,
-};
-
 /*****************************************************************************/
 
 #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index b8881c0f85..964a58cfdc 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -136,25 +136,105 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
     return POWERPC_EXCP_RESET;
 }
 
-static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
+/*
+ * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
+ * taken with the MMU on, and which uses an alternate location (e.g., so the
+ * kernel/hv can map the vectors there with an effective address).
+ *
+ * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
+ * are delivered in this way. AIL requires the LPCR to be set to enable this
+ * mode, and then a number of conditions have to be true for AIL to apply.
+ *
+ * First of all, SRESET, MCE, and HMI are always delivered without AIL, because
+ * they specifically want to be in real mode (e.g., the MCE might be signaling
+ * a SLB multi-hit which requires SLB flush before the MMU can be enabled).
+ *
+ * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV],
+ * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
+ * radix mode (LPCR[HR]).
+ *
+ * POWER8, POWER9 with LPCR[HR]=0
+ * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +-----------+-------------+---------+-------------+-----+
+ * | a         | 00/01/10    | x       | x           | 0   |
+ * | a         | 11          | 0       | 1           | 0   |
+ * | a         | 11          | 1       | 1           | a   |
+ * | a         | 11          | 0       | 0           | a   |
+ * +-------------------------------------------------------+
+ *
+ * POWER9 with LPCR[HR]=1
+ * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +-----------+-------------+---------+-------------+-----+
+ * | a         | 00/01/10    | x       | x           | 0   |
+ * | a         | 11          | x       | x           | a   |
+ * +-------------------------------------------------------+
+ *
+ * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
+ * the hypervisor in AIL mode if the guest is radix.
+ */
+static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
+                                      target_ulong msr,
+                                      target_ulong *new_msr,
+                                      target_ulong *vector)
 {
-    uint64_t offset = 0;
+#if defined(TARGET_PPC64)
+    CPUPPCState *env = &cpu->env;
+    bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
+    bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
+    int ail = 0;
+
+    if (excp == POWERPC_EXCP_MCHECK ||
+        excp == POWERPC_EXCP_RESET ||
+        excp == POWERPC_EXCP_HV_MAINT) {
+        /* SRESET, MCE, HMI never apply AIL */
+        return;
+    }
 
-    switch (ail) {
-    case AIL_NONE:
-        break;
-    case AIL_0001_8000:
-        offset = 0x18000;
-        break;
-    case AIL_C000_0000_0000_4000:
-        offset = 0xc000000000004000ull;
-        break;
-    default:
-        cpu_abort(cs, "Invalid AIL combination %d\n", ail);
-        break;
+    if (excp_model == POWERPC_EXCP_POWER8 ||
+        excp_model == POWERPC_EXCP_POWER9) {
+        if (!mmu_all_on) {
+            /* AIL only works if MSR[IR] and MSR[DR] are both enabled. */
+            return;
+        }
+        if (hv_escalation && !(env->spr[SPR_LPCR] & LPCR_HR)) {
+            /*
+             * AIL does not work if there is a MSR[HV] 0->1 transition and the
+             * partition is in HPT mode. For radix guests, such interrupts are
+             * allowed to be delivered to the hypervisor in ail mode.
+             */
+            return;
+        }
+
+        ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
+        if (ail != 2 && ail != 3) {
+            /* AIL=1 is reserved */
+            return;
+        }
+    } else {
+        /* Other processors do not support AIL */
+        return;
     }
 
-    return offset;
+    /*
+     * AIL applies, so the new MSR gets IR and DR set, and an offset applied
+     * to the new IP.
+     */
+    *new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
+
+    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
+        if (ail == 2) {
+            *vector |= 0x0000000000018000ull;
+        } else if (ail == 3) {
+            *vector |= 0xc000000000004000ull;
+        }
+    } else {
+        /* scv AIL is a little different */
+        if (ail == 3) {
+            *vector &= ~0x0000000000017000ull; /* Un-apply the base offset */
+            *vector |= 0xc000000000003000ull; /* Apply scv's AIL=3 offset */
+        }
+    }
+#endif
 }
 
 static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
@@ -197,7 +277,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     CPUState *cs = CPU(cpu);
     CPUPPCState *env = &cpu->env;
     target_ulong msr, new_msr, vector;
-    int srr0, srr1, asrr0, asrr1, lev = -1, ail;
+    int srr0, srr1, asrr0, asrr1, lev = -1;
     bool lpes0;
 
     qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
@@ -238,25 +318,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
      *
      * On anything else, we behave as if LPES0 is 1
      * (externals don't alter MSR:HV)
-     *
-     * AIL is initialized here but can be cleared by
-     * selected exceptions
      */
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_POWER7 ||
         excp_model == POWERPC_EXCP_POWER8 ||
         excp_model == POWERPC_EXCP_POWER9) {
         lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
-        if (excp_model != POWERPC_EXCP_POWER7) {
-            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
-        } else {
-            ail = 0;
-        }
     } else
 #endif /* defined(TARGET_PPC64) */
     {
         lpes0 = true;
-        ail = 0;
     }
 
     /*
@@ -315,7 +386,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
              */
             new_msr |= (target_ulong)MSR_HVB;
         }
-        ail = 0;
 
         /* machine check exceptions don't have ME set */
         new_msr &= ~((target_ulong)1 << MSR_ME);
@@ -519,7 +589,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
                           "exception %d with no HV support\n", excp);
             }
         }
-        ail = 0;
         break;
     case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
     case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
@@ -790,24 +859,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
     }
 #endif
 
-    /*
-     * AIL only works if MSR[IR] and MSR[DR] are both enabled.
-     */
-    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
-        ail = 0;
-    }
-
-    /*
-     * AIL does not work if there is a MSR[HV] 0->1 transition and the
-     * partition is in HPT mode. For radix guests, such interrupts are
-     * allowed to be delivered to the hypervisor in ail mode.
-     */
-    if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
-        if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
-            ail = 0;
-        }
-    }
-
     vector = env->excp_vectors[excp];
     if (vector == (target_ulong)-1ULL) {
         cpu_abort(cs, "Raised an exception without defined vector %d\n",
@@ -848,23 +899,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         /* Save MSR */
         env->spr[srr1] = msr;
 
-        /* Handle AIL */
-        if (ail) {
-            new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
-            vector |= ppc_excp_vector_offset(cs, ail);
-        }
-
 #if defined(TARGET_PPC64)
     } else {
-        /* scv AIL is a little different */
-        if (ail) {
-            new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
-        }
-        if (ail == AIL_C000_0000_0000_4000) {
-            vector |= 0xc000000000003000ull;
-        } else {
-            vector |= 0x0000000000017000ull;
-        }
         vector += lev * 0x20;
 
         env->lr = env->nip;
@@ -872,6 +908,9 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
 #endif
     }
 
+    /* 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);
 }
 
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index 70f9b9b150..a82d9ed647 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -3457,7 +3457,7 @@ static void init_excp_POWER9(CPUPPCState *env)
 
 #if !defined(CONFIG_USER_ONLY)
     env->excp_vectors[POWERPC_EXCP_HVIRT]    = 0x00000EA0;
-    env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00000000;
+    env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00017000;
 #endif
 }
 
-- 
2.23.0

Re: [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery

[David Gibson]

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

On Thu, Apr 15, 2021 at 03:42:26PM +1000, Nicholas Piggin wrote:
> The AIL logic is becoming unmanageable spread all over powerpc_excp(),
> and it is slated to get even worse with POWER10 support.
> 
> Move it all to a new helper function.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Tested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Looks like a nice cleanup overall, just a few minor comments.

> ---
>  hw/ppc/spapr_hcall.c            |   3 +-
>  target/ppc/cpu.h                |   8 --
>  target/ppc/excp_helper.c        | 159 ++++++++++++++++++++------------
>  target/ppc/translate_init.c.inc |   2 +-
>  4 files changed, 102 insertions(+), 70 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 7b5cd3553c..2fbe04a689 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1395,7 +1395,8 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>          return H_P4;
>      }
>  
> -    if (mflags == AIL_RESERVED) {
> +    if (mflags == 1) {
> +        /* AIL=1 is reserved */
>          return H_UNSUPPORTED_FLAG;
>      }
>  
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index e73416da68..5200a16d23 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -2375,14 +2375,6 @@ enum {
>      HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
>  };
>  
> -/* Alternate Interrupt Location (AIL) */
> -enum {
> -    AIL_NONE                = 0,
> -    AIL_RESERVED            = 1,
> -    AIL_0001_8000           = 2,
> -    AIL_C000_0000_0000_4000 = 3,
> -};

Yeah, I always thought these particular constants were a but
pointless.

> -
>  /*****************************************************************************/
>  
>  #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index b8881c0f85..964a58cfdc 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -136,25 +136,105 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>      return POWERPC_EXCP_RESET;
>  }
>  
> -static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
> +/*
> + * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
> + * taken with the MMU on, and which uses an alternate location (e.g., so the
> + * kernel/hv can map the vectors there with an effective address).
> + *
> + * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
> + * are delivered in this way. AIL requires the LPCR to be set to enable this
> + * mode, and then a number of conditions have to be true for AIL to apply.
> + *
> + * First of all, SRESET, MCE, and HMI are always delivered without AIL, because
> + * they specifically want to be in real mode (e.g., the MCE might be signaling
> + * a SLB multi-hit which requires SLB flush before the MMU can be enabled).
> + *
> + * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV],
> + * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
> + * radix mode (LPCR[HR]).
> + *
> + * POWER8, POWER9 with LPCR[HR]=0
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +-----------+-------------+---------+-------------+-----+
> + * | a         | 00/01/10    | x       | x           | 0   |
> + * | a         | 11          | 0       | 1           | 0   |
> + * | a         | 11          | 1       | 1           | a   |
> + * | a         | 11          | 0       | 0           | a   |
> + * +-------------------------------------------------------+
> + *
> + * POWER9 with LPCR[HR]=1
> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +-----------+-------------+---------+-------------+-----+
> + * | a         | 00/01/10    | x       | x           | 0   |
> + * | a         | 11          | x       | x           | a   |
> + * +-------------------------------------------------------+
> + *
> + * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
> + * the hypervisor in AIL mode if the guest is radix.
> + */
> +static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
> +                                      target_ulong msr,
> +                                      target_ulong *new_msr,
> +                                      target_ulong *vector)
>  {
> -    uint64_t offset = 0;
> +#if defined(TARGET_PPC64)
> +    CPUPPCState *env = &cpu->env;
> +    bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
> +    bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
> +    int ail = 0;
> +
> +    if (excp == POWERPC_EXCP_MCHECK ||
> +        excp == POWERPC_EXCP_RESET ||
> +        excp == POWERPC_EXCP_HV_MAINT) {
> +        /* SRESET, MCE, HMI never apply AIL */
> +        return;
> +    }
>  
> -    switch (ail) {
> -    case AIL_NONE:
> -        break;
> -    case AIL_0001_8000:
> -        offset = 0x18000;
> -        break;
> -    case AIL_C000_0000_0000_4000:
> -        offset = 0xc000000000004000ull;
> -        break;
> -    default:
> -        cpu_abort(cs, "Invalid AIL combination %d\n", ail);
> -        break;
> +    if (excp_model == POWERPC_EXCP_POWER8 ||
> +        excp_model == POWERPC_EXCP_POWER9) {
> +        if (!mmu_all_on) {
> +            /* AIL only works if MSR[IR] and MSR[DR] are both enabled. */
> +            return;
> +        }
> +        if (hv_escalation && !(env->spr[SPR_LPCR] & LPCR_HR)) {
> +            /*
> +             * AIL does not work if there is a MSR[HV] 0->1 transition and the
> +             * partition is in HPT mode. For radix guests, such interrupts are
> +             * allowed to be delivered to the hypervisor in ail mode.
> +             */
> +            return;
> +        }
> +
> +        ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> +        if (ail != 2 && ail != 3) {
> +            /* AIL=1 is reserved */

So, AIL==0 and AIL==1 are treated the same here, but for kinda
different reasons.  AIL==0 means no offset should be applied.  AIL==1
is invalid, so we're just ignoring AIL in that case.

I wonder if it would make things clearer to filter the AIL==1 case at
LPCR write time, and just assert() it's not the case here.

> +            return;
> +        }
> +    } else {
> +        /* Other processors do not support AIL */
> +        return;
>      }
>  
> -    return offset;
> +    /*
> +     * AIL applies, so the new MSR gets IR and DR set, and an offset applied
> +     * to the new IP.
> +     */
> +    *new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> +
> +    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
> +        if (ail == 2) {
> +            *vector |= 0x0000000000018000ull;
> +        } else if (ail == 3) {
> +            *vector |= 0xc000000000004000ull;
> +        }
> +    } else {
> +        /* scv AIL is a little different */

What happens with AIL==2 and an SCV?  I mean, here it's as if AIL==0,
but is that right?  If so, I think we should comment it to make it
clear that's not an omission.

> +        if (ail == 3) {
> +            *vector &= ~0x0000000000017000ull; /* Un-apply the base offset */
> +            *vector |= 0xc000000000003000ull; /* Apply scv's AIL=3 offset */
> +        }



> +    }
> +#endif
>  }
>  
>  static inline void powerpc_set_excp_state(PowerPCCPU *cpu,
> @@ -197,7 +277,7 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
>      target_ulong msr, new_msr, vector;
> -    int srr0, srr1, asrr0, asrr1, lev = -1, ail;
> +    int srr0, srr1, asrr0, asrr1, lev = -1;
>      bool lpes0;
>  
>      qemu_log_mask(CPU_LOG_INT, "Raise exception at " TARGET_FMT_lx
> @@ -238,25 +318,16 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>       *
>       * On anything else, we behave as if LPES0 is 1
>       * (externals don't alter MSR:HV)
> -     *
> -     * AIL is initialized here but can be cleared by
> -     * selected exceptions
>       */
>  #if defined(TARGET_PPC64)
>      if (excp_model == POWERPC_EXCP_POWER7 ||
>          excp_model == POWERPC_EXCP_POWER8 ||
>          excp_model == POWERPC_EXCP_POWER9) {
>          lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
> -        if (excp_model != POWERPC_EXCP_POWER7) {
> -            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> -        } else {
> -            ail = 0;
> -        }
>      } else
>  #endif /* defined(TARGET_PPC64) */
>      {
>          lpes0 = true;
> -        ail = 0;
>      }
>  
>      /*
> @@ -315,7 +386,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>               */
>              new_msr |= (target_ulong)MSR_HVB;
>          }
> -        ail = 0;
>  
>          /* machine check exceptions don't have ME set */
>          new_msr &= ~((target_ulong)1 << MSR_ME);
> @@ -519,7 +589,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>                            "exception %d with no HV support\n", excp);
>              }
>          }
> -        ail = 0;
>          break;
>      case POWERPC_EXCP_DSEG:      /* Data segment exception                   */
>      case POWERPC_EXCP_ISEG:      /* Instruction segment exception            */
> @@ -790,24 +859,6 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>      }
>  #endif
>  
> -    /*
> -     * AIL only works if MSR[IR] and MSR[DR] are both enabled.
> -     */
> -    if (!((msr >> MSR_IR) & 1) || !((msr >> MSR_DR) & 1)) {
> -        ail = 0;
> -    }
> -
> -    /*
> -     * AIL does not work if there is a MSR[HV] 0->1 transition and the
> -     * partition is in HPT mode. For radix guests, such interrupts are
> -     * allowed to be delivered to the hypervisor in ail mode.
> -     */
> -    if ((new_msr & MSR_HVB) && !(msr & MSR_HVB)) {
> -        if (!(env->spr[SPR_LPCR] & LPCR_HR)) {
> -            ail = 0;
> -        }
> -    }
> -
>      vector = env->excp_vectors[excp];
>      if (vector == (target_ulong)-1ULL) {
>          cpu_abort(cs, "Raised an exception without defined vector %d\n",
> @@ -848,23 +899,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          /* Save MSR */
>          env->spr[srr1] = msr;
>  
> -        /* Handle AIL */
> -        if (ail) {
> -            new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> -            vector |= ppc_excp_vector_offset(cs, ail);
> -        }
> -
>  #if defined(TARGET_PPC64)
>      } else {
> -        /* scv AIL is a little different */
> -        if (ail) {
> -            new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
> -        }
> -        if (ail == AIL_C000_0000_0000_4000) {
> -            vector |= 0xc000000000003000ull;
> -        } else {
> -            vector |= 0x0000000000017000ull;
> -        }
>          vector += lev * 0x20;
>  
>          env->lr = env->nip;
> @@ -872,6 +908,9 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>  #endif
>      }
>  
> +    /* 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);
>  }
>  
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index 70f9b9b150..a82d9ed647 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -3457,7 +3457,7 @@ static void init_excp_POWER9(CPUPPCState *env)
>  
>  #if !defined(CONFIG_USER_ONLY)
>      env->excp_vectors[POWERPC_EXCP_HVIRT]    = 0x00000EA0;
> -    env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00000000;
> +    env->excp_vectors[POWERPC_EXCP_SYSCALL_VECTORED] = 0x00017000;
>  #endif
>  }
>  

-- 
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/4] target/ppc: Rework AIL logic in interrupt delivery

[Nicholas Piggin]

Excerpts from David Gibson's message of April 16, 2021 2:24 pm:
> On Thu, Apr 15, 2021 at 03:42:26PM +1000, Nicholas Piggin wrote:
>> The AIL logic is becoming unmanageable spread all over powerpc_excp(),
>> and it is slated to get even worse with POWER10 support.
>> 
>> Move it all to a new helper function.
>> 
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>> Tested-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> 
> Looks like a nice cleanup overall, just a few minor comments.
> 
>> ---
>>  hw/ppc/spapr_hcall.c            |   3 +-
>>  target/ppc/cpu.h                |   8 --
>>  target/ppc/excp_helper.c        | 159 ++++++++++++++++++++------------
>>  target/ppc/translate_init.c.inc |   2 +-
>>  4 files changed, 102 insertions(+), 70 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 7b5cd3553c..2fbe04a689 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1395,7 +1395,8 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>          return H_P4;
>>      }
>>  
>> -    if (mflags == AIL_RESERVED) {
>> +    if (mflags == 1) {
>> +        /* AIL=1 is reserved */
>>          return H_UNSUPPORTED_FLAG;
>>      }
>>  
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index e73416da68..5200a16d23 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -2375,14 +2375,6 @@ enum {
>>      HMER_XSCOM_STATUS_MASK      = PPC_BITMASK(21, 23),
>>  };
>>  
>> -/* Alternate Interrupt Location (AIL) */
>> -enum {
>> -    AIL_NONE                = 0,
>> -    AIL_RESERVED            = 1,
>> -    AIL_0001_8000           = 2,
>> -    AIL_C000_0000_0000_4000 = 3,
>> -};
> 
> Yeah, I always thought these particular constants were a but
> pointless.
> 
>> -
>>  /*****************************************************************************/
>>  
>>  #define is_isa300(ctx) (!!(ctx->insns_flags2 & PPC2_ISA300))
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index b8881c0f85..964a58cfdc 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -136,25 +136,105 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>>      return POWERPC_EXCP_RESET;
>>  }
>>  
>> -static uint64_t ppc_excp_vector_offset(CPUState *cs, int ail)
>> +/*
>> + * AIL - Alternate Interrupt Location, a mode that allows interrupts to be
>> + * taken with the MMU on, and which uses an alternate location (e.g., so the
>> + * kernel/hv can map the vectors there with an effective address).
>> + *
>> + * An interrupt is considered to be taken "with AIL" or "AIL applies" if they
>> + * are delivered in this way. AIL requires the LPCR to be set to enable this
>> + * mode, and then a number of conditions have to be true for AIL to apply.
>> + *
>> + * First of all, SRESET, MCE, and HMI are always delivered without AIL, because
>> + * they specifically want to be in real mode (e.g., the MCE might be signaling
>> + * a SLB multi-hit which requires SLB flush before the MMU can be enabled).
>> + *
>> + * After that, behaviour depends on the current MSR[IR], MSR[DR], MSR[HV],
>> + * whether or not the interrupt changes MSR[HV] from 0 to 1, and the current
>> + * radix mode (LPCR[HR]).
>> + *
>> + * POWER8, POWER9 with LPCR[HR]=0
>> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
>> + * +-----------+-------------+---------+-------------+-----+
>> + * | a         | 00/01/10    | x       | x           | 0   |
>> + * | a         | 11          | 0       | 1           | 0   |
>> + * | a         | 11          | 1       | 1           | a   |
>> + * | a         | 11          | 0       | 0           | a   |
>> + * +-------------------------------------------------------+
>> + *
>> + * POWER9 with LPCR[HR]=1
>> + * | LPCR[AIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
>> + * +-----------+-------------+---------+-------------+-----+
>> + * | a         | 00/01/10    | x       | x           | 0   |
>> + * | a         | 11          | x       | x           | a   |
>> + * +-------------------------------------------------------+
>> + *
>> + * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
>> + * the hypervisor in AIL mode if the guest is radix.
>> + */
>> +static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>> +                                      target_ulong msr,
>> +                                      target_ulong *new_msr,
>> +                                      target_ulong *vector)
>>  {
>> -    uint64_t offset = 0;
>> +#if defined(TARGET_PPC64)
>> +    CPUPPCState *env = &cpu->env;
>> +    bool mmu_all_on = ((msr >> MSR_IR) & 1) && ((msr >> MSR_DR) & 1);
>> +    bool hv_escalation = !(msr & MSR_HVB) && (*new_msr & MSR_HVB);
>> +    int ail = 0;
>> +
>> +    if (excp == POWERPC_EXCP_MCHECK ||
>> +        excp == POWERPC_EXCP_RESET ||
>> +        excp == POWERPC_EXCP_HV_MAINT) {
>> +        /* SRESET, MCE, HMI never apply AIL */
>> +        return;
>> +    }
>>  
>> -    switch (ail) {
>> -    case AIL_NONE:
>> -        break;
>> -    case AIL_0001_8000:
>> -        offset = 0x18000;
>> -        break;
>> -    case AIL_C000_0000_0000_4000:
>> -        offset = 0xc000000000004000ull;
>> -        break;
>> -    default:
>> -        cpu_abort(cs, "Invalid AIL combination %d\n", ail);
>> -        break;
>> +    if (excp_model == POWERPC_EXCP_POWER8 ||
>> +        excp_model == POWERPC_EXCP_POWER9) {
>> +        if (!mmu_all_on) {
>> +            /* AIL only works if MSR[IR] and MSR[DR] are both enabled. */
>> +            return;
>> +        }
>> +        if (hv_escalation && !(env->spr[SPR_LPCR] & LPCR_HR)) {
>> +            /*
>> +             * AIL does not work if there is a MSR[HV] 0->1 transition and the
>> +             * partition is in HPT mode. For radix guests, such interrupts are
>> +             * allowed to be delivered to the hypervisor in ail mode.
>> +             */
>> +            return;
>> +        }
>> +
>> +        ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>> +        if (ail != 2 && ail != 3) {
>> +            /* AIL=1 is reserved */
> 
> So, AIL==0 and AIL==1 are treated the same here, but for kinda
> different reasons.  AIL==0 means no offset should be applied.  AIL==1
> is invalid, so we're just ignoring AIL in that case.

Could comment that specifically at least.

> I wonder if it would make things clearer to filter the AIL==1 case at
> LPCR write time, and just assert() it's not the case here.

Let's discuss that in the next mail.

> 
>> +            return;
>> +        }
>> +    } else {
>> +        /* Other processors do not support AIL */
>> +        return;
>>      }
>>  
>> -    return offset;
>> +    /*
>> +     * AIL applies, so the new MSR gets IR and DR set, and an offset applied
>> +     * to the new IP.
>> +     */
>> +    *new_msr |= (1 << MSR_IR) | (1 << MSR_DR);
>> +
>> +    if (excp != POWERPC_EXCP_SYSCALL_VECTORED) {
>> +        if (ail == 2) {
>> +            *vector |= 0x0000000000018000ull;
>> +        } else if (ail == 3) {
>> +            *vector |= 0xc000000000004000ull;
>> +        }
>> +    } else {
>> +        /* scv AIL is a little different */
> 
> What happens with AIL==2 and an SCV?  I mean, here it's as if AIL==0,
> but is that right?  If so, I think we should comment it to make it
> clear that's not an omission.

Yes as far as I can tell that's what the ISA specifies (i.e., NIA is
unchanged).

Sure a comment can be added.

Thanks,
Nick

[PATCH v2 4/4] target/ppc: Add POWER10 exception model

[Nicholas Piggin]

POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
and it removes support for the LPCR[AIL]=0b10 mode.

Reviewed-by: Cédric Le Goater <clg@kaod.org>
Tested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
---
 hw/ppc/spapr_hcall.c            |  7 ++++-
 target/ppc/cpu-qom.h            |  2 ++
 target/ppc/cpu.h                |  5 ++--
 target/ppc/excp_helper.c        | 51 +++++++++++++++++++++++++++++++--
 target/ppc/translate.c          |  3 +-
 target/ppc/translate_init.c.inc |  2 +-
 6 files changed, 62 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 2fbe04a689..6802cd4dc8 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1396,7 +1396,12 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
     }
 
     if (mflags == 1) {
-        /* AIL=1 is reserved */
+        /* AIL=1 is reserved in POWER8/POWER9 */
+        return H_UNSUPPORTED_FLAG;
+    }
+
+    if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
+        /* AIL=2 is also reserved in POWER10 (ISA v3.1) */
         return H_UNSUPPORTED_FLAG;
     }
 
diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
index 118baf8d41..06b6571bc9 100644
--- a/target/ppc/cpu-qom.h
+++ b/target/ppc/cpu-qom.h
@@ -116,6 +116,8 @@ enum powerpc_excp_t {
     POWERPC_EXCP_POWER8,
     /* POWER9 exception model           */
     POWERPC_EXCP_POWER9,
+    /* POWER10 exception model           */
+    POWERPC_EXCP_POWER10,
 };
 
 /*****************************************************************************/
diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
index 5200a16d23..9d35cdfa92 100644
--- a/target/ppc/cpu.h
+++ b/target/ppc/cpu.h
@@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
 #define LPCR_PECE_U_SHIFT (63 - 19)
 #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
 #define LPCR_HVEE         PPC_BIT(17) /* Hypervisor Virt Exit Enable */
-#define LPCR_RMLS_SHIFT   (63 - 37)
+#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
 #define LPCR_RMLS         (0xfull << LPCR_RMLS_SHIFT)
+#define LPCR_HAIL         PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
 #define LPCR_ILE          PPC_BIT(38)
-#define LPCR_AIL_SHIFT    (63 - 40)      /* Alternate interrupt location */
+#define LPCR_AIL_SHIFT    (63 - 40)   /* Alternate interrupt location */
 #define LPCR_AIL          (3ull << LPCR_AIL_SHIFT)
 #define LPCR_UPRT         PPC_BIT(41) /* Use Process Table */
 #define LPCR_EVIRT        PPC_BIT(42) /* Enhanced Virtualisation */
diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
index 964a58cfdc..38a1482519 100644
--- a/target/ppc/excp_helper.c
+++ b/target/ppc/excp_helper.c
@@ -170,7 +170,27 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
  * +-------------------------------------------------------+
  *
  * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
- * the hypervisor in AIL mode if the guest is radix.
+ * the hypervisor in AIL mode if the guest is radix. This is good for
+ * performance but allows the guest to influence the AIL of hypervisor
+ * interrupts using its MSR, and also the hypervisor must disallow guest
+ * interrupts (MSR[HV] 0->0) from using AIL if the hypervisor does not want to
+ * use AIL for its MSR[HV] 0->1 interrupts.
+ *
+ * POWER10 addresses those issues with a new LPCR[HAIL] bit that is applied to
+ * interrupts that begin execution with MSR[HV]=1 (so both MSR[HV] 0->1 and
+ * MSR[HV] 1->1).
+ *
+ * HAIL=1 is equivalent to AIL=3, for interrupts delivered with MSR[HV]=1.
+ *
+ * POWER10 behaviour is
+ * | LPCR[AIL] | LPCR[HAIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
+ * +-----------+------------+-------------+---------+-------------+-----+
+ * | a         | h          | 00/01/10    | 0       | 0           | 0   |
+ * | a         | h          | 11          | 0       | 0           | a   |
+ * | a         | h          | x           | 0       | 1           | h   |
+ * | a         | h          | 00/01/10    | 1       | 1           | 0   |
+ * | a         | h          | 11          | 1       | 1           | h   |
+ * +--------------------------------------------------------------------+
  */
 static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
                                       target_ulong msr,
@@ -210,6 +230,29 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
             /* AIL=1 is reserved */
             return;
         }
+
+    } else if (excp_model == POWERPC_EXCP_POWER10) {
+        if (!mmu_all_on && !hv_escalation) {
+            /*
+             * AIL works for HV interrupts even with guest MSR[IR/DR] disabled.
+             * Guest->guest and HV->HV interrupts do require MMU on.
+             */
+            return;
+        }
+
+        if (*new_msr & MSR_HVB) {
+            if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) {
+                /* HV interrupts depend on LPCR[HAIL] */
+                return;
+            }
+            ail = 3; /* HAIL=1 gives AIL=3 behaviour for HV interrupts */
+        } else {
+            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
+        }
+        if (ail != 3) {
+            /* AIL=1 and AIL=2 are reserved */
+            return;
+        }
     } else {
         /* Other processors do not support AIL */
         return;
@@ -322,7 +365,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
 #if defined(TARGET_PPC64)
     if (excp_model == POWERPC_EXCP_POWER7 ||
         excp_model == POWERPC_EXCP_POWER8 ||
-        excp_model == POWERPC_EXCP_POWER9) {
+        excp_model == POWERPC_EXCP_POWER9 ||
+        excp_model == POWERPC_EXCP_POWER10) {
         lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
     } else
 #endif /* defined(TARGET_PPC64) */
@@ -842,7 +886,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
         } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
             new_msr |= (target_ulong)1 << MSR_LE;
         }
-    } else if (excp_model == POWERPC_EXCP_POWER9) {
+    } 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;
diff --git a/target/ppc/translate.c b/target/ppc/translate.c
index 0984ce637b..e9ed001229 100644
--- a/target/ppc/translate.c
+++ b/target/ppc/translate.c
@@ -7731,7 +7731,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
 #if defined(TARGET_PPC64)
     if (env->excp_model == POWERPC_EXCP_POWER7 ||
         env->excp_model == POWERPC_EXCP_POWER8 ||
-        env->excp_model == POWERPC_EXCP_POWER9)  {
+        env->excp_model == POWERPC_EXCP_POWER9 ||
+        env->excp_model == POWERPC_EXCP_POWER10)  {
         qemu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
                      env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
     }
diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
index a82d9ed647..76d82cc2f6 100644
--- a/target/ppc/translate_init.c.inc
+++ b/target/ppc/translate_init.c.inc
@@ -9317,7 +9317,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
     pcc->radix_page_info = &POWER10_radix_page_info;
     pcc->lrg_decr_bits = 56;
 #endif
-    pcc->excp_model = POWERPC_EXCP_POWER9;
+    pcc->excp_model = POWERPC_EXCP_POWER10;
     pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
     pcc->bfd_mach = bfd_mach_ppc64;
     pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |
-- 
2.23.0

Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model

[David Gibson]

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

On Thu, Apr 15, 2021 at 03:42:27PM +1000, Nicholas Piggin wrote:
> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
> and it removes support for the LPCR[AIL]=0b10 mode.
> 
> Reviewed-by: Cédric Le Goater <clg@kaod.org>
> Tested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> ---
>  hw/ppc/spapr_hcall.c            |  7 ++++-
>  target/ppc/cpu-qom.h            |  2 ++
>  target/ppc/cpu.h                |  5 ++--
>  target/ppc/excp_helper.c        | 51 +++++++++++++++++++++++++++++++--
>  target/ppc/translate.c          |  3 +-
>  target/ppc/translate_init.c.inc |  2 +-
>  6 files changed, 62 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 2fbe04a689..6802cd4dc8 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1396,7 +1396,12 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>      }
>  
>      if (mflags == 1) {
> -        /* AIL=1 is reserved */
> +        /* AIL=1 is reserved in POWER8/POWER9 */
> +        return H_UNSUPPORTED_FLAG;
> +    }
> +
> +    if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
> +        /* AIL=2 is also reserved in POWER10 (ISA v3.1) */
>          return H_UNSUPPORTED_FLAG;
>      }
>  
> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
> index 118baf8d41..06b6571bc9 100644
> --- a/target/ppc/cpu-qom.h
> +++ b/target/ppc/cpu-qom.h
> @@ -116,6 +116,8 @@ enum powerpc_excp_t {
>      POWERPC_EXCP_POWER8,
>      /* POWER9 exception model           */
>      POWERPC_EXCP_POWER9,
> +    /* POWER10 exception model           */
> +    POWERPC_EXCP_POWER10,
>  };
>  
>  /*****************************************************************************/
> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
> index 5200a16d23..9d35cdfa92 100644
> --- a/target/ppc/cpu.h
> +++ b/target/ppc/cpu.h
> @@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
>  #define LPCR_PECE_U_SHIFT (63 - 19)
>  #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
>  #define LPCR_HVEE         PPC_BIT(17) /* Hypervisor Virt Exit Enable */
> -#define LPCR_RMLS_SHIFT   (63 - 37)
> +#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
>  #define LPCR_RMLS         (0xfull << LPCR_RMLS_SHIFT)
> +#define LPCR_HAIL         PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
>  #define LPCR_ILE          PPC_BIT(38)
> -#define LPCR_AIL_SHIFT    (63 - 40)      /* Alternate interrupt location */
> +#define LPCR_AIL_SHIFT    (63 - 40)   /* Alternate interrupt location */
>  #define LPCR_AIL          (3ull << LPCR_AIL_SHIFT)
>  #define LPCR_UPRT         PPC_BIT(41) /* Use Process Table */
>  #define LPCR_EVIRT        PPC_BIT(42) /* Enhanced Virtualisation */
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 964a58cfdc..38a1482519 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -170,7 +170,27 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>   * +-------------------------------------------------------+
>   *
>   * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
> - * the hypervisor in AIL mode if the guest is radix.
> + * the hypervisor in AIL mode if the guest is radix. This is good for
> + * performance but allows the guest to influence the AIL of hypervisor
> + * interrupts using its MSR, and also the hypervisor must disallow guest
> + * interrupts (MSR[HV] 0->0) from using AIL if the hypervisor does not want to
> + * use AIL for its MSR[HV] 0->1 interrupts.
> + *
> + * POWER10 addresses those issues with a new LPCR[HAIL] bit that is applied to
> + * interrupts that begin execution with MSR[HV]=1 (so both MSR[HV] 0->1 and
> + * MSR[HV] 1->1).
> + *
> + * HAIL=1 is equivalent to AIL=3, for interrupts delivered with MSR[HV]=1.
> + *
> + * POWER10 behaviour is
> + * | LPCR[AIL] | LPCR[HAIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
> + * +-----------+------------+-------------+---------+-------------+-----+
> + * | a         | h          | 00/01/10    | 0       | 0           | 0   |
> + * | a         | h          | 11          | 0       | 0           | a   |
> + * | a         | h          | x           | 0       | 1           | h   |
> + * | a         | h          | 00/01/10    | 1       | 1           | 0   |
> + * | a         | h          | 11          | 1       | 1           | h   |
> + * +--------------------------------------------------------------------+
>   */
>  static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>                                        target_ulong msr,
> @@ -210,6 +230,29 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>              /* AIL=1 is reserved */
>              return;
>          }
> +
> +    } else if (excp_model == POWERPC_EXCP_POWER10) {
> +        if (!mmu_all_on && !hv_escalation) {
> +            /*
> +             * AIL works for HV interrupts even with guest MSR[IR/DR] disabled.
> +             * Guest->guest and HV->HV interrupts do require MMU on.
> +             */
> +            return;
> +        }
> +
> +        if (*new_msr & MSR_HVB) {
> +            if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) {
> +                /* HV interrupts depend on LPCR[HAIL] */
> +                return;
> +            }
> +            ail = 3; /* HAIL=1 gives AIL=3 behaviour for HV interrupts */
> +        } else {
> +            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
> +        }
> +        if (ail != 3) {
> +            /* AIL=1 and AIL=2 are reserved */
> +            return;

As with POWER9, I wonder if we should actuall filter this at LPCR
write time and assert() here.

On actual hardware, what will happen if you attempt to write a bad AIL
to the LPCR?

> +        }
>      } else {
>          /* Other processors do not support AIL */
>          return;
> @@ -322,7 +365,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>  #if defined(TARGET_PPC64)
>      if (excp_model == POWERPC_EXCP_POWER7 ||
>          excp_model == POWERPC_EXCP_POWER8 ||
> -        excp_model == POWERPC_EXCP_POWER9) {
> +        excp_model == POWERPC_EXCP_POWER9 ||
> +        excp_model == POWERPC_EXCP_POWER10) {
>          lpes0 = !!(env->spr[SPR_LPCR] & LPCR_LPES0);
>      } else
>  #endif /* defined(TARGET_PPC64) */
> @@ -842,7 +886,8 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int excp_model, int excp)
>          } else if (env->spr[SPR_LPCR] & LPCR_ILE) {
>              new_msr |= (target_ulong)1 << MSR_LE;
>          }
> -    } else if (excp_model == POWERPC_EXCP_POWER9) {
> +    } 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;
> diff --git a/target/ppc/translate.c b/target/ppc/translate.c
> index 0984ce637b..e9ed001229 100644
> --- a/target/ppc/translate.c
> +++ b/target/ppc/translate.c
> @@ -7731,7 +7731,8 @@ void ppc_cpu_dump_state(CPUState *cs, FILE *f, int flags)
>  #if defined(TARGET_PPC64)
>      if (env->excp_model == POWERPC_EXCP_POWER7 ||
>          env->excp_model == POWERPC_EXCP_POWER8 ||
> -        env->excp_model == POWERPC_EXCP_POWER9)  {
> +        env->excp_model == POWERPC_EXCP_POWER9 ||
> +        env->excp_model == POWERPC_EXCP_POWER10)  {
>          qemu_fprintf(f, "HSRR0 " TARGET_FMT_lx " HSRR1 " TARGET_FMT_lx "\n",
>                       env->spr[SPR_HSRR0], env->spr[SPR_HSRR1]);
>      }
> diff --git a/target/ppc/translate_init.c.inc b/target/ppc/translate_init.c.inc
> index a82d9ed647..76d82cc2f6 100644
> --- a/target/ppc/translate_init.c.inc
> +++ b/target/ppc/translate_init.c.inc
> @@ -9317,7 +9317,7 @@ POWERPC_FAMILY(POWER10)(ObjectClass *oc, void *data)
>      pcc->radix_page_info = &POWER10_radix_page_info;
>      pcc->lrg_decr_bits = 56;
>  #endif
> -    pcc->excp_model = POWERPC_EXCP_POWER9;
> +    pcc->excp_model = POWERPC_EXCP_POWER10;
>      pcc->bus_model = PPC_FLAGS_INPUT_POWER9;
>      pcc->bfd_mach = bfd_mach_ppc64;
>      pcc->flags = POWERPC_FLAG_VRE | POWERPC_FLAG_SE |

-- 
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/4] target/ppc: Add POWER10 exception model

[Nicholas Piggin]

Excerpts from David Gibson's message of April 16, 2021 2:28 pm:
> On Thu, Apr 15, 2021 at 03:42:27PM +1000, Nicholas Piggin wrote:
>> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
>> and it removes support for the LPCR[AIL]=0b10 mode.
>> 
>> Reviewed-by: Cédric Le Goater <clg@kaod.org>
>> Tested-by: Cédric Le Goater <clg@kaod.org>
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>  hw/ppc/spapr_hcall.c            |  7 ++++-
>>  target/ppc/cpu-qom.h            |  2 ++
>>  target/ppc/cpu.h                |  5 ++--
>>  target/ppc/excp_helper.c        | 51 +++++++++++++++++++++++++++++++--
>>  target/ppc/translate.c          |  3 +-
>>  target/ppc/translate_init.c.inc |  2 +-
>>  6 files changed, 62 insertions(+), 8 deletions(-)
>> 
>> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
>> index 2fbe04a689..6802cd4dc8 100644
>> --- a/hw/ppc/spapr_hcall.c
>> +++ b/hw/ppc/spapr_hcall.c
>> @@ -1396,7 +1396,12 @@ static target_ulong h_set_mode_resource_addr_trans_mode(PowerPCCPU *cpu,
>>      }
>>  
>>      if (mflags == 1) {
>> -        /* AIL=1 is reserved */
>> +        /* AIL=1 is reserved in POWER8/POWER9 */
>> +        return H_UNSUPPORTED_FLAG;
>> +    }
>> +
>> +    if (mflags == 2 && (pcc->insns_flags2 & PPC2_ISA310)) {
>> +        /* AIL=2 is also reserved in POWER10 (ISA v3.1) */
>>          return H_UNSUPPORTED_FLAG;
>>      }
>>  
>> diff --git a/target/ppc/cpu-qom.h b/target/ppc/cpu-qom.h
>> index 118baf8d41..06b6571bc9 100644
>> --- a/target/ppc/cpu-qom.h
>> +++ b/target/ppc/cpu-qom.h
>> @@ -116,6 +116,8 @@ enum powerpc_excp_t {
>>      POWERPC_EXCP_POWER8,
>>      /* POWER9 exception model           */
>>      POWERPC_EXCP_POWER9,
>> +    /* POWER10 exception model           */
>> +    POWERPC_EXCP_POWER10,
>>  };
>>  
>>  /*****************************************************************************/
>> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h
>> index 5200a16d23..9d35cdfa92 100644
>> --- a/target/ppc/cpu.h
>> +++ b/target/ppc/cpu.h
>> @@ -354,10 +354,11 @@ typedef struct ppc_v3_pate_t {
>>  #define LPCR_PECE_U_SHIFT (63 - 19)
>>  #define LPCR_PECE_U_MASK  (0x7ull << LPCR_PECE_U_SHIFT)
>>  #define LPCR_HVEE         PPC_BIT(17) /* Hypervisor Virt Exit Enable */
>> -#define LPCR_RMLS_SHIFT   (63 - 37)
>> +#define LPCR_RMLS_SHIFT   (63 - 37)   /* RMLS (removed in ISA v3.0) */
>>  #define LPCR_RMLS         (0xfull << LPCR_RMLS_SHIFT)
>> +#define LPCR_HAIL         PPC_BIT(37) /* ISA v3.1 HV AIL=3 equivalent */
>>  #define LPCR_ILE          PPC_BIT(38)
>> -#define LPCR_AIL_SHIFT    (63 - 40)      /* Alternate interrupt location */
>> +#define LPCR_AIL_SHIFT    (63 - 40)   /* Alternate interrupt location */
>>  #define LPCR_AIL          (3ull << LPCR_AIL_SHIFT)
>>  #define LPCR_UPRT         PPC_BIT(41) /* Use Process Table */
>>  #define LPCR_EVIRT        PPC_BIT(42) /* Enhanced Virtualisation */
>> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
>> index 964a58cfdc..38a1482519 100644
>> --- a/target/ppc/excp_helper.c
>> +++ b/target/ppc/excp_helper.c
>> @@ -170,7 +170,27 @@ static int powerpc_reset_wakeup(CPUState *cs, CPUPPCState *env, int excp,
>>   * +-------------------------------------------------------+
>>   *
>>   * The difference with POWER9 being that MSR[HV] 0->1 interrupts can be sent to
>> - * the hypervisor in AIL mode if the guest is radix.
>> + * the hypervisor in AIL mode if the guest is radix. This is good for
>> + * performance but allows the guest to influence the AIL of hypervisor
>> + * interrupts using its MSR, and also the hypervisor must disallow guest
>> + * interrupts (MSR[HV] 0->0) from using AIL if the hypervisor does not want to
>> + * use AIL for its MSR[HV] 0->1 interrupts.
>> + *
>> + * POWER10 addresses those issues with a new LPCR[HAIL] bit that is applied to
>> + * interrupts that begin execution with MSR[HV]=1 (so both MSR[HV] 0->1 and
>> + * MSR[HV] 1->1).
>> + *
>> + * HAIL=1 is equivalent to AIL=3, for interrupts delivered with MSR[HV]=1.
>> + *
>> + * POWER10 behaviour is
>> + * | LPCR[AIL] | LPCR[HAIL] | MSR[IR||DR] | MSR[HV] | new MSR[HV] | AIL |
>> + * +-----------+------------+-------------+---------+-------------+-----+
>> + * | a         | h          | 00/01/10    | 0       | 0           | 0   |
>> + * | a         | h          | 11          | 0       | 0           | a   |
>> + * | a         | h          | x           | 0       | 1           | h   |
>> + * | a         | h          | 00/01/10    | 1       | 1           | 0   |
>> + * | a         | h          | 11          | 1       | 1           | h   |
>> + * +--------------------------------------------------------------------+
>>   */
>>  static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>>                                        target_ulong msr,
>> @@ -210,6 +230,29 @@ static inline void ppc_excp_apply_ail(PowerPCCPU *cpu, int excp_model, int excp,
>>              /* AIL=1 is reserved */
>>              return;
>>          }
>> +
>> +    } else if (excp_model == POWERPC_EXCP_POWER10) {
>> +        if (!mmu_all_on && !hv_escalation) {
>> +            /*
>> +             * AIL works for HV interrupts even with guest MSR[IR/DR] disabled.
>> +             * Guest->guest and HV->HV interrupts do require MMU on.
>> +             */
>> +            return;
>> +        }
>> +
>> +        if (*new_msr & MSR_HVB) {
>> +            if (!(env->spr[SPR_LPCR] & LPCR_HAIL)) {
>> +                /* HV interrupts depend on LPCR[HAIL] */
>> +                return;
>> +            }
>> +            ail = 3; /* HAIL=1 gives AIL=3 behaviour for HV interrupts */
>> +        } else {
>> +            ail = (env->spr[SPR_LPCR] & LPCR_AIL) >> LPCR_AIL_SHIFT;
>> +        }
>> +        if (ail != 3) {
>> +            /* AIL=1 and AIL=2 are reserved */
>> +            return;
> 
> As with POWER9, I wonder if we should actuall filter this at LPCR
> write time and assert() here.

Could do. The processor is allowed to read back reserved bits as 0. I 
can't quite see how reserved values in multi bit fields are treated
though. Neither bits are reserved but the value 2 is. If you write 3
then 2, would the second write also clear bit 0, or would it ignore
the write and leave bit 1 set? I don't see either being explicitly
allowed so it might retain the value 2 I suspect.

> On actual hardware, what will happen if you attempt to write a bad AIL
> to the LPCR?

Good question, I don't know. We don't have an easy way to place a
interrupt handler at 0x17000 in Linux I think. I'll see if I can
get some data.

Thanks,
Nick