* [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model
@ 2021-04-15 5:42 Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour Nicholas Piggin
` (3 more replies)
0 siblings, 4 replies; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-15 5:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Fabiano Rosas, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour
2021-04-15 5:42 [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
@ 2021-04-15 5:42 ` Nicholas Piggin
2021-04-15 12:12 ` Fabiano Rosas
2021-04-15 5:42 ` [PATCH v2 2/4] target/ppc: POWER10 supports scv Nicholas Piggin
` (2 subsequent siblings)
3 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-15 5:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Fabiano Rosas, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 2/4] target/ppc: POWER10 supports scv
2021-04-15 5:42 [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour Nicholas Piggin
@ 2021-04-15 5:42 ` Nicholas Piggin
2021-04-15 7:43 ` [EXTERNAL] " Cédric Le Goater
2021-04-15 5:42 ` [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 4/4] target/ppc: Add POWER10 exception model Nicholas Piggin
3 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-15 5:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Fabiano Rosas, Cédric Le Goater, qemu-devel,
Nicholas Piggin, David Gibson
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery
2021-04-15 5:42 [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 2/4] target/ppc: POWER10 supports scv Nicholas Piggin
@ 2021-04-15 5:42 ` Nicholas Piggin
2021-04-16 4:24 ` David Gibson
2021-04-15 5:42 ` [PATCH v2 4/4] target/ppc: Add POWER10 exception model Nicholas Piggin
3 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-15 5:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Fabiano Rosas, qemu-devel, Nicholas Piggin,
Cédric Le Goater, Cédric Le Goater, David Gibson
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH v2 4/4] target/ppc: Add POWER10 exception model
2021-04-15 5:42 [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
` (2 preceding siblings ...)
2021-04-15 5:42 ` [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery Nicholas Piggin
@ 2021-04-15 5:42 ` Nicholas Piggin
2021-04-16 4:28 ` David Gibson
3 siblings, 1 reply; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-15 5:42 UTC (permalink / raw)
To: qemu-ppc
Cc: Fabiano Rosas, qemu-devel, Nicholas Piggin,
Cédric Le Goater, Cédric Le Goater, David Gibson
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
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [EXTERNAL] [PATCH v2 2/4] target/ppc: POWER10 supports scv
2021-04-15 5:42 ` [PATCH v2 2/4] target/ppc: POWER10 supports scv Nicholas Piggin
@ 2021-04-15 7:43 ` Cédric Le Goater
2021-04-16 4:15 ` David Gibson
0 siblings, 1 reply; 13+ messages in thread
From: Cédric Le Goater @ 2021-04-15 7:43 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc; +Cc: Fabiano Rosas, qemu-devel, David Gibson
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;
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour
2021-04-15 5:42 ` [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour Nicholas Piggin
@ 2021-04-15 12:12 ` Fabiano Rosas
2021-04-16 4:13 ` David Gibson
0 siblings, 1 reply; 13+ messages in thread
From: Fabiano Rosas @ 2021-04-15 12:12 UTC (permalink / raw)
To: Nicholas Piggin, qemu-ppc
Cc: Cédric Le Goater, qemu-devel, Nicholas Piggin, David Gibson
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",
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour
2021-04-15 12:12 ` Fabiano Rosas
@ 2021-04-16 4:13 ` David Gibson
0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-04-16 4:13 UTC (permalink / raw)
To: Fabiano Rosas
Cc: Cédric Le Goater, qemu-ppc, qemu-devel, Nicholas Piggin
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [EXTERNAL] [PATCH v2 2/4] target/ppc: POWER10 supports scv
2021-04-15 7:43 ` [EXTERNAL] " Cédric Le Goater
@ 2021-04-16 4:15 ` David Gibson
0 siblings, 0 replies; 13+ messages in thread
From: David Gibson @ 2021-04-16 4:15 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-ppc, qemu-devel, Nicholas Piggin, Fabiano Rosas
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery
2021-04-15 5:42 ` [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery Nicholas Piggin
@ 2021-04-16 4:24 ` David Gibson
2021-04-17 3:17 ` Nicholas Piggin
0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2021-04-16 4:24 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Cédric Le Goater, Cédric Le Goater, qemu-ppc,
qemu-devel, Fabiano Rosas
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model
2021-04-15 5:42 ` [PATCH v2 4/4] target/ppc: Add POWER10 exception model Nicholas Piggin
@ 2021-04-16 4:28 ` David Gibson
2021-04-17 3:31 ` Nicholas Piggin
0 siblings, 1 reply; 13+ messages in thread
From: David Gibson @ 2021-04-16 4:28 UTC (permalink / raw)
To: Nicholas Piggin
Cc: Cédric Le Goater, Cédric Le Goater, qemu-ppc,
qemu-devel, Fabiano Rosas
[-- 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 --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery
2021-04-16 4:24 ` David Gibson
@ 2021-04-17 3:17 ` Nicholas Piggin
0 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-17 3:17 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, Cédric Le Goater, qemu-ppc,
Cédric Le Goater, Fabiano Rosas
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model
2021-04-16 4:28 ` David Gibson
@ 2021-04-17 3:31 ` Nicholas Piggin
0 siblings, 0 replies; 13+ messages in thread
From: Nicholas Piggin @ 2021-04-17 3:31 UTC (permalink / raw)
To: David Gibson
Cc: qemu-devel, Cédric Le Goater, qemu-ppc,
Cédric Le Goater, Fabiano Rosas
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
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2021-04-17 3:32 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-15 5:42 [PATCH v2 0/4] ppc: rework AIL logic, add POWER10 exception model Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour Nicholas Piggin
2021-04-15 12:12 ` Fabiano Rosas
2021-04-16 4:13 ` David Gibson
2021-04-15 5:42 ` [PATCH v2 2/4] target/ppc: POWER10 supports scv Nicholas Piggin
2021-04-15 7:43 ` [EXTERNAL] " Cédric Le Goater
2021-04-16 4:15 ` David Gibson
2021-04-15 5:42 ` [PATCH v2 3/4] target/ppc: Rework AIL logic in interrupt delivery Nicholas Piggin
2021-04-16 4:24 ` David Gibson
2021-04-17 3:17 ` Nicholas Piggin
2021-04-15 5:42 ` [PATCH v2 4/4] target/ppc: Add POWER10 exception model Nicholas Piggin
2021-04-16 4:28 ` David Gibson
2021-04-17 3:31 ` Nicholas Piggin
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).