Re: [PATCH 3/6] target/arm: Honor TCR_ELx.{I}PS

["Alex Bennée" <alex.bennee@linaro.org>]

Richard Henderson <richard.henderson@linaro.org> writes:

> This field controls the output (intermediate) physical address size
> of the translation process.  V8 requires to raise an AddressSize
> fault if the page tables are programmed incorrectly, such that any
> intermediate descriptor address, or the final translated address,
> is out of range.
>
> Add an outputsize field to ARMVAParameters, and fill it in during
> aa64_va_parameters.  Pass the value to check_s2_mmu_setup to use
> instead of the raw PAMax value.  Test the descaddr as extracted
> from TTBR and from page table entries.
>
> Restrict descaddrmask so that we won't raise the fault for v7.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/internals.h |  1 +
>  target/arm/helper.c    | 92 +++++++++++++++++++++++++++++-------------
>  2 files changed, 65 insertions(+), 28 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index 27d2fcd26c..3e801833b4 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -1032,6 +1032,7 @@ static inline uint32_t aarch64_pstate_valid_mask(const ARMISARegisters *id)
>   */
>  typedef struct ARMVAParameters {
>      unsigned tsz    : 8;
> +    unsigned ps     : 3;
>      unsigned select : 1;
>      bool tbi        : 1;
>      bool epd        : 1;
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index fab9ee70d8..568914bd42 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11003,7 +11003,7 @@ do_fault:
>   * false otherwise.
>   */
>  static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
> -                               int inputsize, int stride)
> +                               int inputsize, int stride, int outputsize)
>  {
>      const int grainsize = stride + 3;
>      int startsizecheck;
> @@ -11019,22 +11019,19 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
>      }
>  
>      if (is_aa64) {
> -        CPUARMState *env = &cpu->env;
> -        unsigned int pamax = arm_pamax(cpu);
> -
>          switch (stride) {
>          case 13: /* 64KB Pages.  */
> -            if (level == 0 || (level == 1 && pamax <= 42)) {
> +            if (level == 0 || (level == 1 && outputsize <= 42)) {
>                  return false;
>              }
>              break;
>          case 11: /* 16KB Pages.  */
> -            if (level == 0 || (level == 1 && pamax <= 40)) {
> +            if (level == 0 || (level == 1 && outputsize <= 40)) {
>                  return false;
>              }
>              break;
>          case 9: /* 4KB Pages.  */
> -            if (level == 0 && pamax <= 42) {
> +            if (level == 0 && outputsize <= 42) {
>                  return false;
>              }
>              break;
> @@ -11043,8 +11040,8 @@ static bool check_s2_mmu_setup(ARMCPU *cpu, bool is_aa64, int level,
>          }
>  
>          /* Inputsize checks.  */
> -        if (inputsize > pamax &&
> -            (arm_el_is_aa64(env, 1) || inputsize > 40)) {
> +        if (inputsize > outputsize &&
> +            (arm_el_is_aa64(&cpu->env, 1) || inputsize > 40)) {
>              /* This is CONSTRAINED UNPREDICTABLE and we choose to fault.  */
>              return false;
>          }
> @@ -11090,17 +11087,19 @@ static uint8_t convert_stage2_attrs(CPUARMState *env, uint8_t s2attrs)
>  }
>  #endif /* !CONFIG_USER_ONLY */
>  
> +/* This mapping is common between ID_AA64MMFR0.PARANGE and TCR_ELx.{I}PS. */
> +static const uint8_t pamax_map[] = {
> +    [0] = 32,
> +    [1] = 36,
> +    [2] = 40,
> +    [3] = 42,
> +    [4] = 44,
> +    [5] = 48,
> +};

I note that the IPS encoding includes b110 (6) for 52 bits or 4PB
addressing. I guess that gets introduced later.

Anyway:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée