Re: [PATCH v4 1/7] target/arm: Improve masking of HCR RES0 bits

[Peter Maydell]

On Tue, 25 Feb 2020 at 18:08, Richard Henderson
<address@hidden> wrote:
>
> Don't merely start with v8.0, handle v7VE as well.
> Notice writes from aarch32 mode, and the bits that
> ought not be settable from there.
>
> Suggested-by: Peter Maydell <address@hidden>
> Signed-off-by: Richard Henderson <address@hidden>
> ---
>  target/arm/helper.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 79db169e04..d65160fdb3 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -5089,8 +5089,13 @@ static const ARMCPRegInfo el3_no_el2_v8_cp_reginfo[] = 
> {
>  static void hcr_write(CPUARMState *env, const ARMCPRegInfo *ri, uint64_t 
> value)
>  {
>      ARMCPU *cpu = env_archcpu(env);
> -    /* Begin with bits defined in base ARMv8.0.  */
> -    uint64_t valid_mask = MAKE_64BIT_MASK(0, 34);
> +    uint64_t valid_mask;
> +
> +    if (arm_feature(env, ARM_FEATURE_V8)) {
> +        valid_mask = MAKE_64BIT_MASK(0, 34);  /* ARMv8.0 */
> +    } else {
> +        valid_mask = MAKE_64BIT_MASK(0, 28);  /* ARMv7VE */
> +    }
>
>      if (arm_feature(env, ARM_FEATURE_EL3)) {
>          valid_mask &= ~HCR_HCD;
> @@ -5114,6 +5119,14 @@ static void hcr_write(CPUARMState *env, const 
> ARMCPRegInfo *ri, uint64_t value)
>          valid_mask |= HCR_API | HCR_APK;
>      }
>
> +    if (ri->state == ARM_CP_STATE_AA32) {
> +        /*
> +         * Writes from aarch32 mode have more RES0 bits.
> +         * This includes TDZ, RW, E2H, and more.
> +         */
> +        valid_mask &= ~0xff80ff8c90000000ull;
> +    }

Isn't bit HCR2 bit 16 (aka bit 32+16==48 here) also RES0 from AArch32 ?
I suppose it's RES0 from AArch64 too, but as far as what we've
implemented goes so are a bunch of other bits.

I'm not really a fan of the hex-number here either, given we
have HCR_* constants.

thanks
-- PMM