Re: [boot-wrapper 2/3] aarch64: Enable access into SCTLR2_ELx registers from EL2 and below

[Mark Rutland <mark.rutland@arm.com>]

On Fri, Jul 26, 2024 at 12:25:14PM +0530, Anshuman Khandual wrote:
> On 7/25/24 14:10, Mark Rutland wrote:
> > On Tue, Jul 23, 2024 at 04:36:29PM +0530, Anshuman Khandual wrote:
> >> diff --git a/arch/aarch64/init.c b/arch/aarch64/init.c
> >> index 7d9d0d9..5b21cb8 100644
> >> --- a/arch/aarch64/init.c
> >> +++ b/arch/aarch64/init.c
> >> @@ -92,6 +92,9 @@ void cpu_init_el3(void)
> >>  	if (mrs_field(ID_AA64MMFR3_EL1, D128))
> >>  		scr |= SCR_EL3_D128En;
> >>  
> >> +	if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX))
> >> +		scr |= SCR_EL3_SCTLR2En;
> >> +
> > 
> > The SCTLR2_ELx registers reset to UNKNOWN values when the highest
> > implemented exception level is not ELx, so we need to initialize those
> > to safe values. Otherwise a kernel which is not aware of SCTLR2_ELx will
> > be subject to arbitrary behaviour as a result of the SCTLR2_ELx bits
> > which it will not have configured.
> 
> Both SCTLR2_EL1 and SCTLR2_EL2 has the same register fields layout
> except the very last bit i.e SCTLR2_EL2.EMEC which is available in
> SCTLR2_EL2 but not in SCTLR2_EL1.
> 
> AFAICT all the above register fields are applicable for newer arch
> features which the current kernel is not even aware about. So even
> if the kernel is not ware about SCTLR2_EL2 or SCTLR2_EL1 registers,
> there will not be any difference in behaviour related to these new
> arch features.

There several are changes to existing behaviours. Looking at ARM DDI
0487K.a:

* EASE changes the way external aborts are routed, which could surprise
  the exception handling code.

* NMEA causes SError to be taken regardless of PSTATE.A. This *will*
  break exception handling.

... and regardless we have no idea how any of the RES0 bits will be used
in future.

Looking at DDI 0601 ID070124 from:

  https://developer.arm.com/documentation/ddi0601/2024-06/?lang=en

... there are other bits that would be problematic too. Consider how
EnPACM0 works with a kernel that is not PACM-aware but a userspace that
is, especially if CPUs have mismatched reset values.

> Search for the registers in the current mainline kernel.
> 
> $git grep SCTLR2_EL
> 
> arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL2                  sys_reg(3, 4, 1, 0, 3)
> arch/arm64/include/asm/sysreg.h:#define SYS_SCTLR2_EL12                 sys_reg(3, 5, 1, 0, 3)
> arch/arm64/kvm/emulate-nested.c:        SR_TRAP(SYS_SCTLR2_EL2,         CGT_HCR_NV),
> 
> $git grep SCTLR2En
> arch/arm64/kvm/nested.c:                res0 |= HCRX_EL2_SCTLR2En;
> arch/arm64/tools/sysreg:Field   15      SCTLR2En
> 
> Although if we are looking for safer values, guess resetting these
> two registers might be sufficient here ?
> 
> +       if (mrs_field(ID_AA64MMFR3_EL1, SCTLRX)) {
> +               scr |= SCR_EL3_SCTLR2En;
> +               msr(SCTLR2_EL2, 0);
> +               msr(SCTLR2_EL1, 0);
> +       }

Using zero for both looks fine to me.

Mark.