| Message ID | 20220630110016.31441-1-joey.gouly@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1] arm64: mm: fix booting with 52-bit address space | expand |
Hi Joey, On Thu, 30 Jun 2022 at 13:00, Joey Gouly <joey.gouly@arm.com> wrote: > > __cpu_setup() used `vabits_actual` before it had been set to the > correct value (in arm64_memblock_init()), if 52-bit address space > was supported. > > Use the register value directly so that TCR_EL1.T1SZ will be set > to the correct value. > > This fixes booting on QEMU and the FVP model. > > Fixes: 0d9b1ffefabe ("arm64: mm: make vabits_actual a build time constant if possible") > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > Cc: Ard Biesheuvel <ardb@kernel.org> > Cc: Will Deacon <will@kernel.org> > Cc: Anshuman Khandual <anshuman.khandual@arm.com> > Cc: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Joey Gouly <joey.gouly@arm.com> > --- > arch/arm64/mm/proc.S | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S > index 605c6640f94b..375448b2de9e 100644 > --- a/arch/arm64/mm/proc.S > +++ b/arch/arm64/mm/proc.S > @@ -466,7 +466,16 @@ SYM_FUNC_START(__cpu_setup) > tcr_clear_errata_bits tcr, x9, x5 > > #ifdef CONFIG_ARM64_VA_BITS_52 > - ldr_l x9, vabits_actual > + /* > + * vabits_actual has not been set at this point, check the > + * ID_AA64MMFR2_EL1 register directly. > + */ > + mrs_s x10, SYS_ID_AA64MMFR2_EL1 > + and x10, x10, #(0xf << ID_AA64MMFR2_LVA_SHIFT) > + mov x9, #52 > + cbnz x10, 1f > + mov x9, #VA_BITS_MIN > +1: > sub x9, xzr, x9 > add x9, x9, #64 > tcr_set_t1sz tcr, x9 Oops, sorry about that! I think this might give the wrong result on heterogeneous systems, e.g., if you boot on a CPU that does not have LVA but have secondaries that do. Given the the issue only affects the boot CPU, perhaps we should initialize vabits_actual to 0x0 statically, and assign the correct value in arm64_memblock_init(). Or alternatively, pass the correct value as a parameter to __cpu_setup(), in which case we can use the above approach for the primary, and load vabits_actual for the secondary. I'm also more than happy to clean up my own mess, and take this as a reported-by.
diff --git a/arch/arm64/mm/proc.S b/arch/arm64/mm/proc.S index 605c6640f94b..375448b2de9e 100644 --- a/arch/arm64/mm/proc.S +++ b/arch/arm64/mm/proc.S @@ -466,7 +466,16 @@ SYM_FUNC_START(__cpu_setup) tcr_clear_errata_bits tcr, x9, x5 #ifdef CONFIG_ARM64_VA_BITS_52 - ldr_l x9, vabits_actual + /* + * vabits_actual has not been set at this point, check the + * ID_AA64MMFR2_EL1 register directly. + */ + mrs_s x10, SYS_ID_AA64MMFR2_EL1 + and x10, x10, #(0xf << ID_AA64MMFR2_LVA_SHIFT) + mov x9, #52 + cbnz x10, 1f + mov x9, #VA_BITS_MIN +1: sub x9, xzr, x9 add x9, x9, #64 tcr_set_t1sz tcr, x9
__cpu_setup() used `vabits_actual` before it had been set to the correct value (in arm64_memblock_init()), if 52-bit address space was supported. Use the register value directly so that TCR_EL1.T1SZ will be set to the correct value. This fixes booting on QEMU and the FVP model. Fixes: 0d9b1ffefabe ("arm64: mm: make vabits_actual a build time constant if possible") Signed-off-by: Joey Gouly <joey.gouly@arm.com> Cc: Ard Biesheuvel <ardb@kernel.org> Cc: Will Deacon <will@kernel.org> Cc: Anshuman Khandual <anshuman.khandual@arm.com> Cc: Mark Rutland <mark.rutland@arm.com> Signed-off-by: Joey Gouly <joey.gouly@arm.com> --- arch/arm64/mm/proc.S | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)