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

[Peter Maydell <peter.maydell@linaro.org>]

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