Re: [PATCH v2 1/1] target/arm: Fix SCR RES1 handling

[Peter Maydell]

On Wed, 3 Feb 2021 at 18:12, Rémi Denis-Courmont <remi@remlab.net> wrote:
>
> Le keskiviikkona 3. helmikuuta 2021, 18.55.52 EET michael. nawrocki--- via a
> écrit :
> > The FW and AW bits of SCR_EL3 are RES1 only in some contexts. Force them
> > to 1 only when there is no support for AArch32 at EL1 or above.
> >
> > The reset value will be 0x30 only if the CPU is AArch64-only; if there
> > is support for AArch32 at EL1 or above, it will be reset to 0.
> >
> > Also adds helper function isar_feature_aa64_aa32_el1 to check if AArch32
> > is supported at EL1 or above.
> >
> > Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>

> > @@ -2024,7 +2024,10 @@ static void scr_write(CPUARMState *env, const
> > ARMCPRegInfo *ri, uint64_t value) ARMCPU *cpu = env_archcpu(env);
> >
> >      if (ri->state == ARM_CP_STATE_AA64) {
> > -        value |= SCR_FW | SCR_AW;   /* these two bits are RES1.  */
> > +        if (arm_feature(env, ARM_FEATURE_AARCH64) &&
>
> AFAICT, this is a tautology in this context.

Normally, yes, but:

> > +            !cpu_isar_feature(aa64_aa32_el1, cpu)) {
> > +                value |= SCR_FW | SCR_AW;   /* these two bits are RES1.  */
> > +        }
> >          valid_mask &= ~SCR_NET;
> >
> >          if (cpu_isar_feature(aa64_lor, cpu)) {
> > @@ -2063,6 +2066,15 @@ static void scr_write(CPUARMState *env, const
> > ARMCPRegInfo *ri, uint64_t value) raw_write(env, ri, value);
> >  }
> >
> > +static void scr_reset(CPUARMState *env, const ARMCPRegInfo *ri)
> > +{
> > +    /*
> > +     * scr_write will set the RES1 bits on an AArch64-only CPU.
> > +     * The reset value will be 0x30 on an AArch64-only CPU and 0 otherwise.
> > +     */
> > +    scr_write(env, ri, 0);
> > +}

...the new scr_reset() function introduces a code path where
scr_write() will be called on the STATE_AA64 reginfo even on
a 32-bit only CPU. (The AArch32 SCR reginfo is an alias, so it
relies on the AA64 reginfo being present-but-not-guest-accessible
for the reset value.)

thanks
-- PMM
j