[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model
From: |
Nicholas Piggin |
Subject: |
Re: [PATCH v2 4/4] target/ppc: Add POWER10 exception model |
Date: |
Sat, 17 Apr 2021 13:31:13 +1000 |
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
- Re: [PATCH v2 1/4] target/ppc: Fix POWER9 radix guest HV interrupt AIL behaviour, (continued)