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

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

On Wed, Jan 26, 2022 at 03:03:38PM +0000, Andre Przywara wrote:
> On Tue, 25 Jan 2022 15:00:44 +0000
> Mark Rutland <mark.rutland@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@arm.com>
> > Cc: Andre Przywara <andre.przywara@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?

Yup, the intent is to have the exact same definition, overlapping bits and all.

I would like to clean that up in future, but for now I want this to clearly be
the same.

> 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@arm.com>

Thanks!

Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel