Re: [PATCH] target/arm: Do hflags rebuild in cpsr_write()

[Peter Maydell]

On Tue, 17 Aug 2021 at 21:18, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> Currently we rely on all the callsites of cpsr_write() to rebuild the
> cached hflags if they change one of the CPSR bits which we use as a
> TB flag and cache in hflags.  This is a bit awkward when we want to
> change the set of CPSR bits that we cache, because it means we need
> to re-audit all the cpsr_write() callsites to see which flags they
> are writing and whether they now need to rebuild the hflags.
>
> Switch instead to making cpsr_write() call arm_rebuild_hflags()
> itself if one of the bits being changed is a cached bit.
>
> We don't do the rebuild for the CPSRWriteRaw write type,

Doh. I said this, but then...

> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 201ecf8c67f..cdd6e0858fc 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -9215,6 +9215,7 @@ void cpsr_write(CPUARMState *env, uint32_t val, 
> uint32_t mask,
>                  CPSRWriteType write_type)
>  {
>      uint32_t changed_daif;
> +    bool rebuild_hflags = mask & (CPSR_M | CPSR_E | CPSR_IL);

...forgot to actually check the write type.

Should be:

    bool rebuild_hflags = (write_type != CPSRWriteRaw) &&
        (mask & (CPSR_M | CPSR_E | CPSR_IL));

>      if (mask & CPSR_NZCV) {
>          env->ZF = (~val) & CPSR_Z;
> @@ -9334,6 +9335,9 @@ void cpsr_write(CPUARMState *env, uint32_t val, 
> uint32_t mask,
>      }
>      mask &= ~CACHED_CPSR_BITS;
>      env->uncached_cpsr = (env->uncached_cpsr & ~mask) | (val & mask);
> +    if (rebuild_hflags) {
> +        arm_rebuild_hflags(env);
> +    }
>  }
>
>  /* Sign/zero extend */
> --

-- PMM