Re: [EXTERNAL] [RFC PATCH 2/2] target/ppc: Add POWER10 exception model

[Cédric Le Goater]

On 4/14/21 5:23 AM, Nicholas Piggin wrote:
> POWER10 adds a new bit that modifies interrupt behaviour, LPCR[HAIL],
> and it removes support for the LPCR[AIL]=0b10 mode.

This looks good but it's missing the MSR_LE setting. A part from that : 

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

and 

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

distros using scv on P10 now need your patch to boot :

"powerpc/powernv: Enable HAIL (HV AIL) for ISA v3.1 processors"

I guess it will get merged in time. 

Thanks,

C.


> 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        | 43 +++++++++++++++++++++++++++++++++
>  target/ppc/translate.c          |  3 ++-
>  target/ppc/translate_init.c.inc |  2 +-
>  6 files changed, 57 insertions(+), 5 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 9ff316767c..19931361a0 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -172,6 +172,26 @@ 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 (LPCR[HR]=1).
> + * 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 interrupt that begin execution with MSR[HV]=1 (so both
> + * MSR[HV] 0->1 and 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,
> @@ -211,6 +231,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;
> 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 |
>