Re: [PATCH 3/4] target/arm: Correctly trap HCR.TID1 registers in v7A

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

Peter Maydell <peter.maydell@linaro.org> writes:

> In v7A HCR.TID1 is defined to trap for TCMTR, TLBTR, REVIDR and AIDR.
> We incorrectly use an accessfn for REVIDR and AIDR that only traps on
> v8A cores.  Fix this by collapsing access_aa64_tid1() and
> access_aa32_tid1() together and never doing a check for v8 vs v7.
>
> The accessfn is also used for SMIDR_EL1, which is fine as this
> register is AArch64 only.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 24 +++++++-----------------
>  1 file changed, 7 insertions(+), 17 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index c4f73eb3f3..0896e90965 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -924,8 +924,8 @@ static uint64_t isr_read(CPUARMState *env, const ARMCPRegInfo *ri)
>      return ret;
>  }
>  
> -static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> -                                       bool isread)
> +static CPAccessResult access_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> +                                  bool isread)
>  {
>      if (arm_current_el(env) == 1 && (arm_hcr_el2_eff(env) & HCR_TID1)) {
>          return CP_ACCESS_TRAP_EL2;
> @@ -934,16 +934,6 @@ static CPAccessResult access_aa64_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
>      return CP_ACCESS_OK;
>  }
>  
> -static CPAccessResult access_aa32_tid1(CPUARMState *env, const ARMCPRegInfo *ri,
> -                                       bool isread)
> -{
> -    if (arm_feature(env, ARM_FEATURE_V8)) {
> -        return access_aa64_tid1(env, ri, isread);
> -    }
> -
> -    return CP_ACCESS_OK;
> -}
> -

This logic makes more sense from the descriptions compared to 2/4.

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

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro