Re: [PATCH 1/1] target/arm: Add raw_writefn to SCR_EL3 register

[Michael Nawrocki]

On 2/2/21 6:29 AM, Peter Maydell wrote:
> On Thu, 28 Jan 2021 at 14:31, Mike Nawrocki
> <michael.nawrocki@gtri.gatech.edu> wrote:
> > Fixes an issue in migration where the reset value of SCR and the value
> > produced by scr_write via the writefn for SCR_EL3 mismatch.
> >
> > Signed-off-by: Mike Nawrocki <michael.nawrocki@gtri.gatech.edu>
> > ---
> >   target/arm/helper.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/target/arm/helper.c b/target/arm/helper.c
> > index d2ead3fcbd..e3c4fe76cb 100644
> > --- a/target/arm/helper.c
> > +++ b/target/arm/helper.c
> > @@ -5785,7 +5785,7 @@ static const ARMCPRegInfo el3_cp_reginfo[] = {
> >       { .name = "SCR_EL3", .state = ARM_CP_STATE_AA64,
> >         .opc0 = 3, .opc1 = 6, .crn = 1, .crm = 1, .opc2 = 0,
> >         .access = PL3_RW, .fieldoffset = offsetof(CPUARMState, cp15.scr_el3),
> > -      .resetvalue = 0, .writefn = scr_write },
> > +      .resetvalue = 0, .writefn = scr_write, .raw_writefn = raw_write },
> >       { .name = "SCR",  .type = ARM_CP_ALIAS | ARM_CP_NEWEL,
> >         .cp = 15, .opc1 = 0, .crn = 1, .crm = 1, .opc2 = 0,
> >         .access = PL1_RW, .accessfn = access_trap_aa32s_el1,
> > --
>
> I think the problem here is not the lack of a raw_writefn,
> but that we're not handling the RES1 bits [5:4] in SCR_EL3 correctly.
> Specifically:
>
>   * if the CPU is AArch64-only then the resetvalue for SCR_EL3 should
>     not be 0, but 0x30 (but if the CPU has AArch32 at all then it should
>     still be 0)
>   * scr_write() should not be saying "force the FW and AW bits to 1 if
>     written as an AArch64 register". It can, but is not obliged to,
>     do this if the CPU is AArch64-only; it must not if the CPU has
>     AArch32, even if the register is being accessed via its AArch64 form.
>
> This is because RES1 has some complicated semantics for bits like
> this which are "RES1 only in some architectural contexts" (this is all
> defined in the Glossary entry in the v8A Arm ARM), which basically
> means that if AArch32 is supported then the bit has to be reads-as-written
> from the AArch64 side.
>
> thanks
> -- PMM

I see what you mean. Does QEMU support AArch64-only CPU models, and if 
so, is there a way to determine if the CPU has AArch32?

Thanks,
Mike