Re: [PATCH 03/11] target/arm: Add mmu_idx for EL1 and EL2 w/ PAN enabled

[Peter Maydell]

On Tue, 3 Dec 2019 at 22:53, Richard Henderson
<address@hidden> wrote:
>
> To implement PAN, we will want to swap, for short periods
> of time, to a different privileged mmu_idx.  In addition,
> we cannot do this with flushing alone, because the AT*
> instructions have both PAN and PAN-less versions.
>
> Add the ARMMMUIdx*_PAN constants where necessary next to
> the corresponding ARMMMUIdx* constant.
>
> Signed-off-by: Richard Henderson <address@hidden>
> diff --git a/target/arm/cpu.h b/target/arm/cpu.h
> index 22935e4433..22c5706835 100644
> --- a/target/arm/cpu.h
> +++ b/target/arm/cpu.h
> @@ -2715,20 +2715,22 @@ bool write_cpustate_to_list(ARMCPU *cpu, bool 
> kvm_sync);
>   *  5. we want to be able to use the TLB for accesses done as part of a
>   *     stage1 page table walk, rather than having to walk the stage2 page
>   *     table over and over.
> + *  6. we need separate EL1/EL2 mmu_idx for handling the Priviledged Access

"Privileged" (no 'd')

> + *     Never (PAN) bit within PSTATE.
>   *
>   * This gives us the following list of cases:
>   *
>   * NS EL0 (aka NS PL0) EL1&0 stage 1+2
> - * NS EL1 (aka NS PL1) EL1&0 stage 1+2
> + * NS EL1 (aka NS PL1) EL1&0 stage 1+2 (+PAN)
>   * NS EL0 EL2&0
> - * NS EL2 EL2&0
> + * NS EL2 EL2&0 (+PAN)
>   * NS EL2 (aka NS PL2)
>   * S EL0 (aka S PL0)
> - * S EL1 (not used if EL3 is 32 bit)
> + * S EL1 (not used if EL3 is 32 bit) (+PAN)
>   * S EL3 (aka S PL1)
>   * NS EL0&1 stage 2
>   *
> - * for a total of 9 different mmu_idx.
> + * for a total of 12 different mmu_idx.
>   *
> @@ -11886,11 +11924,14 @@ static uint32_t rebuild_hflags_a64(CPUARMState 
> *env, int el, int fp_el,
>      /* TODO: ARMv8.2-UAO */
>      switch (mmu_idx) {
>      case ARMMMUIdx_EL10_1:
> +    case ARMMMUIdx_EL10_1_PAN:
>      case ARMMMUIdx_SE1:
> +    case ARMMMUIdx_SE1_PAN:
>          /* TODO: ARMv8.3-NV */
>          flags = FIELD_DP32(flags, TBFLAG_A64, UNPRIV, 1);
>          break;
>      case ARMMMUIdx_EL20_2:
> +    case ARMMMUIdx_EL20_2_PAN:
>          /* TODO: ARMv8.4-SecEL2 */
>          /*
>           * Note that EL20_2 is gated by HCR_EL2.E2H == 1, but EL20_0 is
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index fe492bea90..b5c7bc2d76 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -124,9 +124,11 @@ static int get_a64_user_mem_index(DisasContext *s)
>           */
>          switch (useridx) {
>          case ARMMMUIdx_EL10_1:
> +        case ARMMMUIdx_EL10_1_PAN:
>              useridx = ARMMMUIdx_EL10_0;
>              break;
>          case ARMMMUIdx_EL20_2:
> +        case ARMMMUIdx_EL20_2_PAN:
>              useridx = ARMMMUIdx_EL20_0;
>              break;
>          case ARMMMUIdx_SE1:

Why doesn't this switch need also a case for ARMMMUIdx_SE1_PAN ?

otherwise
Reviewed-by: Peter Maydell <address@hidden>

thanks
-- PMM