Re: [PATCH v4 19/40] target/arm: Add regime_has_2_ranges

From: "Alex Bennée" <alex.bennee@linaro.org>
To: Richard Henderson <richard.henderson@linaro.org>
Cc: peter.maydell@linaro.org, qemu-devel@nongnu.org
Subject: Re: [PATCH v4 19/40] target/arm: Add regime_has_2_ranges
Date: Wed, 04 Dec 2019 14:16:23 +0000	[thread overview]
Message-ID: <87blso89c8.fsf@linaro.org> (raw)
In-Reply-To: <20191203022937.1474-20-richard.henderson@linaro.org>

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

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>

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

> ---
>  target/arm/internals.h     | 16 ++++++++++++++++
>  target/arm/helper.c        | 23 ++++++-----------------
>  target/arm/translate-a64.c |  3 +--
>  3 files changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index d73615064c..1ca9a7cc78 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -837,6 +837,22 @@ static inline void arm_call_el_change_hook(ARMCPU *cpu)
>      }
>  }
>  
> +/* Return true if this address translation regime has two ranges.  */
> +static inline bool regime_has_2_ranges(ARMMMUIdx mmu_idx)
> +{
> +    switch (mmu_idx) {
> +    case ARMMMUIdx_Stage1_E0:
> +    case ARMMMUIdx_Stage1_E1:
> +    case ARMMMUIdx_EL10_0:
> +    case ARMMMUIdx_EL10_1:
> +    case ARMMMUIdx_EL20_0:
> +    case ARMMMUIdx_EL20_2:
> +        return true;
> +    default:
> +        return false;
> +    }
> +}
> +
>  /* Return true if this address translation regime is secure */
>  static inline bool regime_is_secure(CPUARMState *env, ARMMMUIdx mmu_idx)
>  {
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index f86285ffbe..27adf24fa6 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -8885,15 +8885,8 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
>      }
>  
>      if (is_aa64) {
> -        switch (regime_el(env, mmu_idx)) {
> -        case 1:
> -            if (!is_user) {
> -                xn = pxn || (user_rw & PAGE_WRITE);
> -            }
> -            break;
> -        case 2:
> -        case 3:
> -            break;
> +        if (regime_has_2_ranges(mmu_idx) && !is_user) {
> +            xn = pxn || (user_rw & PAGE_WRITE);
>          }
>      } else if (arm_feature(env, ARM_FEATURE_V7)) {
>          switch (regime_el(env, mmu_idx)) {
> @@ -9427,7 +9420,6 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
>                                          ARMMMUIdx mmu_idx)
>  {
>      uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
> -    uint32_t el = regime_el(env, mmu_idx);
>      bool tbi, tbid, epd, hpd, using16k, using64k;
>      int select, tsz;
>  
> @@ -9437,7 +9429,7 @@ ARMVAParameters aa64_va_parameters_both(CPUARMState *env, uint64_t va,
>       */
>      select = extract64(va, 55, 1);
>  
> -    if (el > 1) {
> +    if (!regime_has_2_ranges(mmu_idx)) {
>          tsz = extract32(tcr, 0, 6);
>          using64k = extract32(tcr, 14, 1);
>          using16k = extract32(tcr, 15, 1);
> @@ -9593,10 +9585,7 @@ static bool get_phys_addr_lpae(CPUARMState *env, target_ulong address,
>          param = aa64_va_parameters(env, address, mmu_idx,
>                                     access_type != MMU_INST_FETCH);
>          level = 0;
> -        /* If we are in 64-bit EL2 or EL3 then there is no TTBR1, so mark it
> -         * invalid.
> -         */
> -        ttbr1_valid = (el < 2);
> +        ttbr1_valid = regime_has_2_ranges(mmu_idx);
>          addrsize = 64 - 8 * param.tbi;
>          inputsize = 64 - param.tsz;
>      } else {
> @@ -11306,8 +11295,8 @@ static uint32_t rebuild_hflags_a64(CPUARMState *env, int el, int fp_el,
>  
>      flags = FIELD_DP32(flags, TBFLAG_ANY, AARCH64_STATE, 1);
>  
> -    /* FIXME: ARMv8.1-VHE S2 translation regime.  */
> -    if (regime_el(env, stage1) < 2) {
> +    /* Get control bits for tagged addresses.  */
> +    if (regime_has_2_ranges(mmu_idx)) {
>          ARMVAParameters p1 = aa64_va_parameters_both(env, -1, stage1);
>          tbid = (p1.tbi << 1) | p0.tbi;
>          tbii = tbid & ~((p1.tbid << 1) | p0.tbid);
> diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
> index 885c99f0c9..d0b65c49e2 100644
> --- a/target/arm/translate-a64.c
> +++ b/target/arm/translate-a64.c
> @@ -175,8 +175,7 @@ static void gen_top_byte_ignore(DisasContext *s, TCGv_i64 dst,
>      if (tbi == 0) {
>          /* Load unmodified address */
>          tcg_gen_mov_i64(dst, src);
> -    } else if (s->current_el >= 2) {
> -        /* FIXME: ARMv8.1-VHE S2 translation regime.  */
> +    } else if (!regime_has_2_ranges(s->mmu_idx)) {
>          /* Force tag byte to all zero */
>          tcg_gen_extract_i64(dst, src, 0, 56);
>      } else {

-- 
Alex Bennée