Re: [bootwrapper PATCH v2 06/13] aarch64: initialize SCTLR_ELx for the boot-wrapper

[Andre Przywara <andre.przywara@arm.com>]

On Mon, 17 Jan 2022 13:05:54 +0000
Mark Rutland <mark.rutland@arm.com> wrote:

Hi Mark,

> On Mon, Jan 17, 2022 at 12:15:57PM +0000, Mark Rutland wrote:
> > On Fri, Jan 14, 2022 at 06:12:47PM +0000, Andre Przywara wrote:  
> > > On Fri, 14 Jan 2022 10:56:46 +0000
> > > Mark Rutland <mark.rutland@arm.com> wrote:
> > > 
> > > Hi Mark,  
> > 
> > Hi Andre,  
>  
> > > > +
> > > > +#define SCTLR_EL1_RES1		(BIT(29) | BIT(28) | BIT(23) | BIT(22) | \
> > > > +				 BIT(11) | BIT(8) | BIT(7) | BIT(4))  
> 
> > > > -#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)  
> > > 
> > > So I wonder if this actually works? The ARMv7 version of SCTLR
> > > differs in some bits from both the ARMv8 AArch32 version and more
> > > importantly the AArch64 version.  
> 
> > > I had troubles the other day running the
> > > arm32 Linux kernel decompressor with some ARMv8 SCTLR_EL1 reset value. The
> > > decompressor code does only read-modify-write of SCTLR (probably to
> > > cover multiple architecture revisions), so some bits might stay wrong. In
> > > particular I think having bits 28 and 29 set caused problems.
> > > By looking at the ARMv7 ARM and with experimentation I came up
> > > with 0x00c00878 as a safe and working value.  
> 
> Having re-read all of this, I believe you're right; I'll rework the
> AArch32-kernel SCTLR_EL1_KERNEL to not use the AArch64 bit definitions,
> and I'll add some commentary explaining that we're writing to the AArch32
> format.
> 
> > > Shall we have a separate reset value for 32bit?  
> 
> Assuming you meant to alter the SCTLR_ELx_KERNEL definition, yes.
> 
> The idea of splitting the SCTLR_ELx_RESET and SCTLR_ELx_KERNEL
> definitions was that the former would be whatever the boot-wrapper
> needed to run at ELx, and the latter was whatever we must initialize for
> the kernel to run at ELx, so I don't want to put the AArch32 kernel bits
> into SCTLR_EL1_RESET.
> 
> My only remaining concern is exactly what we must initialize. If there's
> any documentation we can refer to, that'd be great, otherwise I'll dig
> through your prior suggestion.

I don't know of any recommendation what to initialise, though the ARMv7
ARM speaks a bit about reset values.
So the SCTLR_KERNEL value we use in arch/aarch32/include/asm/cpu.h seems
to work (although it's not perfect), but we should use the same in the
KERNEL_32 case in arch/aarch64/include/asm/cpu.h.

Going through the SCTLR description in the ARMv8 *and* ARMv7 ARM again,
I think a safe and sane init value would be to set bits
[23,22,18,16,11,5,4,3] to one. This differs slightly from the value I told
you above, which I think was what I observed on an Cortex-A7 when dumping
SCTLR in the decompressor code.

I became aware of the issue when I tried to start an ARM kernel on a
Cortex-A53 board, with U-Boot dropping from AArch64-EL2 to AArch32-EL1.
SCTLR_EL1 there got initialised according to the ARMv8 ARM (bits
[29,28,23,22,20,11] set), but this made the kernel decompressor hang as
soon as the MMU got enabled. I *think* I bisected it down to bits 28 and
29, which are RES1 in AArch64, but enable TEX remap and the Access Flag in
v7, and both reset to 0 there. I haven't tried that on the model with the
boot-wrapper, but I guess we see the same problem there.

In any case it seems to be already broken, so a fix or discussion doesn't
need to block this series.

Cheers,
Andre

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