[PATCH] armv8: Fix TCR 64-bit writes

Mark Kettenis mark.kettenis at xs4all.nl
Mon Jun 6 23:00:08 CEST 2022


> From: Andre Przywara <andre.przywara at arm.com>
> Date: Mon,  9 May 2022 17:08:49 +0100
> 
> The AArch64 TCR_ELx register is a 64-bit register, and many newer
> architecture features use bits in the upper half. So far U-Boot was
> igorant of those bits, trying to leave them alone.
> However, in an effort to set bit 31 to 1, it failed doing so, because
> the compiler sign-extended "1 << 31", so that all bits[63:31] got set.
> 
> Older ARMv8.0 cores don't define anything dangerous up there, but newer
> architecture revisions do, and setting all those bits will end badly:
> =================
> $ qemu-system-aarch64 -cpu max ....
> U-Boot 2022.07-rc1 (May 09 2022 - 15:21:00 +0100)
> 
> DRAM:  1.5 GiB
> =================  (hangs here)
> 
> Defining TCR_ELx_RSVD to "1U << 31" avoids the sign-extension, so all
> upper bits stay at a safe 0 value. This means no more surprises when
> U-Boot runs on a more capable CPU core.

Hi Andre,

This change breaks U-Boot on Apple M1 systems.  Th reason is
"interesting".  The Apple Icestorm and Firestorm cores implement
FEAT_VHE, but do so in an unusual (non-compiant?) way.  the
HCR_EL2.E2H bit is hardwired to 1, which means that EL2 Host is always
enabled.  As a consequence, TCR_EL2 has a different layout (the same
as TCR_EL1).  This means that the get_tcr() function in
arch/arm/cpu/armv8/cache_v8.c returns the wrong value.  Apparently the
sign-extension that happened before accidentally set the "right" bits
so I didn't notice this issue before.

Below is a possible fix.  This uses #ifdef CONFIG_ARCH_APPLE to avoid
growing the code.  A more generic way of fixing this would involve
checking the value of HCR_EL2.E2H, probably in mmu_setup() and pass
el=1 instead of el=2 to get_tcr() instead.

Thoughts?


diff --git a/arch/arm/cpu/armv8/cache_v8.c b/arch/arm/cpu/armv8/cache_v8.c
index 3de18c7675..ea3a60b2cd 100644
--- a/arch/arm/cpu/armv8/cache_v8.c
+++ b/arch/arm/cpu/armv8/cache_v8.c
@@ -74,7 +74,11 @@ u64 get_tcr(int el, u64 *pips, u64 *pva_bits)
 	if (el == 1) {
 		tcr = TCR_EL1_RSVD | (ips << 32) | TCR_EPD1_DISABLE;
 	} else if (el == 2) {
+#ifdef CONFIG_ARCH_APPLE
+		tcr = TCR_EL1_RSVD | (ips << 32) | TCR_EPD1_DISABLE;
+#else
 		tcr = TCR_EL2_RSVD | (ips << 16);
+#endif
 	} else {
 		tcr = TCR_EL3_RSVD | (ips << 16);
 	}




More information about the U-Boot mailing list