[bootwrapper PATCH v3 02/15] aarch64: correct SCTLR_EL1_KERNEL for AA32 kernels

[Andre Przywara]

On Tue, 25 Jan 2022 15:00:44 +0000
Mark Rutland <mark.rutland at arm.com> wrote:

Hi,

> Bits [31:0] of the AArch64 SCTLR_EL1 register are architecturally mapped
> to bits [31:0] of the AArch32 SCTLR register. This means that any
> individual bit always has the same value across the two registers.
> 
> Across the two registers, the same bit may have distinct meanings,
> distinct RESx behaviour, and distinct reset behaviour. For example,
> SCTLR_EL1[28] is nTLSMD, which we wich to initialize to 0b1, whereas
> SCTLR[28] is TRE, which we wish to initialize to 0b0.
> 
> To avoid setting bits which we did not intend to, and in preparation for
> configuring more AArch64 RES1 bits, this patch decouples the
> SCTLR_EL1_KERNEL value used for AArch32 kernels from the SCTLR_EL1_RES1
> definitions such that is does not inherit AArch64 SCTLR_EL1 bits which
> would be problematic in the AArch32 SCTLR. For now we open-code a copy
> of the bits defined in arch/aarch64/include/asm/cpu.h.
> 
> Signed-off-by: Mark Rutland <mark.rutland at arm.com>
> Cc: Andre Przywara <andre.przywara at arm.com>
> ---
>  arch/aarch64/include/asm/cpu.h | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/aarch64/include/asm/cpu.h b/arch/aarch64/include/asm/cpu.h
> index e3269b8..49d3f86 100644
> --- a/arch/aarch64/include/asm/cpu.h
> +++ b/arch/aarch64/include/asm/cpu.h
> @@ -36,8 +36,15 @@
>  #define SCTLR_EL1_RES1		(3 << 28 | 3 << 22 | 1 << 11)
>  
>  #ifdef KERNEL_32
> -/* 32-bit kernel decompressor uses CP15 barriers */
> -#define SCTLR_EL1_KERNEL	(SCTLR_EL1_RES1 | SCTLR_EL1_CP15BEN)
> +/*
> + * When booting a 32-bit kernel, EL1 uses AArch32 and registers which are
> + * architecturally mapped must be configured with the AArch32 layout.
> + *
> + * We copy the AArch32 definition of SCTLR_KERNEL here.
> + *
> + * TODO: restructure the headers to share a single definition.
> + */
> +#define SCTLR_EL1_KERNEL	(3 << 22 | 1 << 11 | 1 << 5 | 3 << 4)

Is the verbatim copying of the value, including the redundancy, intended?
Maybe that meant 3 << 3, as bits[4:3] are documented as "Reserved,
RAO/SBOP" in the ARMv7 ARM?
It's fine with me either way, since it copies the aarch32 value, just
wanted to check:

Reviewed-by: Andre Przywara <andre.przywara at arm.com>

Cheers,
Andre

>  #define SPSR_KERNEL		(SPSR_A | SPSR_I | SPSR_F | SPSR_HYP)
>  #else
>  #define SCTLR_EL1_KERNEL	SCTLR_EL1_RES1