Messages in this thread Patch in this message | | | Date | Wed, 10 Mar 2021 11:16:49 +0000 | From | Will Deacon <> | Subject | Re: [PATCH] arm64: mm: fix runtime fallback to 48-bt VA when 52-bit VA is enabled |
| |
On Tue, Mar 09, 2021 at 07:32:16PM -0500, Mark Salter wrote: > I ran into an early boot soft lockup on a Qualcomm Amberwing using a v5.11 > kernel configured for 52-bit VA. This turned into a panic with a v5.12-rc2 > kernel. > > The problem is that when we fall back to 48-bit VA, idmap_t0sz is not > updated. Later, the kvm hypervisor uses idmap_t0sz to set its tcr_el2 and > hangs (v5.11). After commit 1401bef703a4 ("arm64: mm: Always update TCR_EL1 > from __cpu_set_tcr_t0sz()"), the kernel panics when trying to use the idmap > to call idmap_cpu_replace_ttbr1(). > > Oddly, other systems (thunderX2 and Ampere eMag) which don't support 52-bit > VA seem to handle the setting of an unsupported t0sz without any apparent > problems. Indeed, if one reads back the tcr written with t0sz==12, the > value read has t0sz==16. Not so with Amberwing.
Nice, you have one of those elusive platforms!
> Fixes: 90ec95cda91a ("arm64: mm: Introduce VA_BITS_MIN") > Signed-off-by: Mark Salter <msalter@redhat.com> > --- > arch/arm64/kernel/head.S | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S > index 66b0e0b66e31..2bcbbb26292e 100644 > --- a/arch/arm64/kernel/head.S > +++ b/arch/arm64/kernel/head.S > @@ -291,6 +291,7 @@ SYM_FUNC_START_LOCAL(__create_page_tables) > */ > adrp x0, idmap_pg_dir > adrp x3, __idmap_text_start // __pa(__idmap_text_start) > + mov x4, TCR_T0SZ(VA_BITS) > > #ifdef CONFIG_ARM64_VA_BITS_52 > mrs_s x6, SYS_ID_AA64MMFR2_EL1 > @@ -299,6 +300,13 @@ SYM_FUNC_START_LOCAL(__create_page_tables) > cbnz x6, 1f > #endif > mov x5, #VA_BITS_MIN > +#ifdef CONFIG_ARM64_VA_BITS_52 > + mov x4, TCR_T0SZ(VA_BITS_MIN) > + adr_l x6, idmap_t0sz > + str x4, [x6] > + dmb sy > + dc ivac, x6 // Invalidate potentially stale cache line > +#endif > 1: > adr_l x6, vabits_actual > str x5, [x6] > @@ -319,7 +327,7 @@ SYM_FUNC_START_LOCAL(__create_page_tables) > */ > adrp x5, __idmap_text_end > clz x5, x5 > - cmp x5, TCR_T0SZ(VA_BITS) // default T0SZ small enough? > + cmp x5, x4 // default T0SZ small enough? > b.ge 1f // .. then skip VA range extension
Could we instead have the default value be 48-bit, and then avoid having to update the variable in both cases? e.g. something along the lines of the entirely untested diff below?
Cheers,
Will
--->8
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S index 66b0e0b66e31..fb795123896f 100644 --- a/arch/arm64/kernel/head.S +++ b/arch/arm64/kernel/head.S @@ -319,7 +319,7 @@ SYM_FUNC_START_LOCAL(__create_page_tables) */ adrp x5, __idmap_text_end clz x5, x5 - cmp x5, TCR_T0SZ(VA_BITS) // default T0SZ small enough? + cmp x5, TCR_T0SZ(VA_BITS_MIN) // default T0SZ small enough? b.ge 1f // .. then skip VA range extension adr_l x6, idmap_t0sz diff --git a/arch/arm64/mm/mmu.c b/arch/arm64/mm/mmu.c index 3802cfbdd20d..4c5603c41870 100644 --- a/arch/arm64/mm/mmu.c +++ b/arch/arm64/mm/mmu.c @@ -40,7 +40,7 @@ #define NO_BLOCK_MAPPINGS BIT(0) #define NO_CONT_MAPPINGS BIT(1) -u64 idmap_t0sz = TCR_T0SZ(VA_BITS); +u64 idmap_t0sz = TCR_T0SZ(VA_BITS_MIN); u64 idmap_ptrs_per_pgd = PTRS_PER_PGD; u64 __section(".mmuoff.data.write") vabits_actual;
| |