arm64: some 52-bit cleanups

PATCH In review Ard Biesheuvel 2 participants 13 messages Mar 10, 2021 – Mar 11, 2021 arm64

Share & tools

Ard Biesheuvel

author

2021-03-10 17:16 UTC #0 lore raw

cover arm64: some 52-bit cleanups

This work was triggered by Mark Salter's report about a regression regarding the programmed size of the ID map page tables.

The first patch switches the early ID map population code to default to a 48-bit range on kernels built with 52-bit VA support, and only switch to 52-bit if the placement of the kernel requires it. This should address the regressions reported by Mark. This patch should be suitable for being backported to -stable.

The remaining patches are cleanups that remove and/or simplify the 52-bit VA support, and disable some parts in configurations that don't need it.

Patch #5 adds back the logic to use a 52-bit VA ID map on 52-bit VA capable hardware, which we might consider if it is worthwhile for some reason to elide the TCR_EL1 updates that result from having ID map and user space VA ranges of different size.

Cc Mark Salter <[email protected]>

Cc Will Deacon <[email protected]>

Cc James Morse <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Steve Capper <[email protected]>

Cc Anshuman Khandual <[email protected]>

Ard Biesheuvel (5):
arm64: mm: use a 48-bit ID map when possible on 52-bit VA builds
arm64: mm: remove unused __cpu_uses_extended_idmap[_level()]
arm64: mm: use a compile time constant for vabits_actual when possible
arm64: mm: get rid of idmap_ptrs_per_pgd handling
arm64: mm: switch to 52-bit ID map on 52-bit VA capable systems

 arch/arm64/include/asm/memory.h      |  4 +++
 arch/arm64/include/asm/mmu_context.h | 18 --------------
 arch/arm64/kernel/head.S             | 26 +++++++++-----------
 arch/arm64/mm/mmu.c                  |  5 ++--
 4 files changed, 18 insertions(+), 35 deletions(-)

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-10 17:16 UTC #1 lore raw

PATCH 1/5 arm64: mm: use a 48-bit ID map when possible on 52-bit VA builds

52-bit VA kernels can run on hardware that is only 48-bit capable, but configure the ID map as 52-bit by default. This was not a problem until recently, because the special T0SZ value for a 52-bit VA space was never programmed into the TCR register anwyay, and because a 52-bit ID map happens to use the same number of translation levels as a 48-bit one.

This behavior was changed by commit 1401bef703a4 ("arm64: mm: Always update TCR_EL1 from __cpu_set_tcr_t0sz()"), which causes the unsupported T0SZ value for a 52-bit VA to be programmed into TCR_EL1. While some hardware simply ignores this, Mark reports that Amberwing systems choke on this, resulting in a broken boot. But even before that commit, the unsupported idmap_t0sz value was exposed to KVM and used to program TCR_EL2 incorrectly as well.

Given that we already have to deal with address spaces being either 48-bit or 52-bit in size, the cleanest approach seems to be to simply default to a 48-bit VA ID map, and only switch to a 52-bit one if the placement of the kernel in DRAM requires it. This is guaranteed not to happen unless the system is actually 52-bit VA capable.

Fixes 90ec95cda91a ("arm64: mm: Introduce VA_BITS_MIN")

Reported-by Mark Salter <[email protected]>

Link http://lore.kernel.org/r/[email protected] open here

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/mmu_context.h | 5 +----
 arch/arm64/kernel/head.S             | 2 +-
 arch/arm64/mm/mmu.c                  | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)

arch/arm64/include/asm/mmu_context.h +1 -4

index 70ce8c1d2b07..0f467d550f27 100644
@@ -65,10 +65,7 @@ extern u64 idmap_ptrs_per_pgd;
 
 static inline bool __cpu_uses_extended_idmap(void)
 {
-	if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52))
-		return false;
-
-	return unlikely(idmap_t0sz != TCR_T0SZ(VA_BITS));
+	return unlikely(idmap_t0sz != TCR_T0SZ(vabits_actual));
 }
 
 /*

arch/arm64/kernel/head.S +1 -1

index 66b0e0b66e31..0b0387644dc0 100644
@@ -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

arch/arm64/mm/mmu.c +1 -1

index 3802cfbdd20d..4c5603c41870 100644
@@ -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;

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Will Deacon

maintainer

2021-03-11 09:48 UTC #2 lore raw

Re: [PATCH 1/5] arm64: mm: use a 48-bit ID map when possible on 52-bit VA builds

Quoted 35 lines · Ard Biesheuvel

52-bit VA kernels can run on hardware that is only 48-bit capable, but configure the ID map as 52-bit by default. This was not a problem until recently, because the special T0SZ value for a 52-bit VA space was never programmed into the TCR register anwyay, and because a 52-bit ID map happens to use the same number of translation levels as a 48-bit one.
This behavior was changed by commit 1401bef703a4 ("arm64: mm: Always update TCR_EL1 from __cpu_set_tcr_t0sz()"), which causes the unsupported T0SZ value for a 52-bit VA to be programmed into TCR_EL1. While some hardware simply ignores this, Mark reports that Amberwing systems choke on this, resulting in a broken boot. But even before that commit, the unsupported idmap_t0sz value was exposed to KVM and used to program TCR_EL2 incorrectly as well.
Given that we already have to deal with address spaces being either 48-bit or 52-bit in size, the cleanest approach seems to be to simply default to a 48-bit VA ID map, and only switch to a 52-bit one if the placement of the kernel in DRAM requires it. This is guaranteed not to happen unless the system is actually 52-bit VA capable.
Fixes 90ec95cda91a ("arm64: mm: Introduce VA_BITS_MIN")
Reported-by Mark Salter <[email protected]>
Link http://lore.kernel.org/r/[email protected] open here
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/mmu_context.h | 5 +----
 arch/arm64/kernel/head.S             | 2 +-
 arch/arm64/mm/mmu.c                  | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)
arch/arm64/include/asm/mmu_context.h +0 -0
index 70ce8c1d2b07..0f467d550f27 100644

arch/arm64/include/asm/mmu_context.h extern u64 idmap_ptrs_per_pgd; · line 68

Quoted 8 lines

arch/arm64/include/asm/mmu_context.h +1 -4

@@ -65,10 +65,7 @@ extern u64 idmap_ptrs_per_pgd;
 
 static inline bool __cpu_uses_extended_idmap(void)
 {
-	if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52))
-		return false;
-
-	return unlikely(idmap_t0sz != TCR_T0SZ(VA_BITS));
+	return unlikely(idmap_t0sz != TCR_T0SZ(vabits_actual));

I'm a bit confused by this: if the idmap is using 48-bit VAs but vabits_actual is 52, then we'll return true despite not using the extended idmap. Have I missed something, or should that be < instead of != ?

Quoted 8 lines

arch/arm64/include/asm/mmu_context.h +0 -0
 }
 
 /*
arch/arm64/kernel/head.S +0 -0
index 66b0e0b66e31..0b0387644dc0 100644

arch/arm64/kernel/head.S SYM_FUNC_START_LOCAL(__create_page_tables) · line 323

Quoted 6 lines

arch/arm64/kernel/head.S +1 -1

@@ -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

(nit: the comment immediately before this talks about VA_BITS and should probably be updated)

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-11 12:02 UTC #3 lore raw

Re: [PATCH 1/5] arm64: mm: use a 48-bit ID map when possible on 52-bit VA builds

Quoted 38 lines · Will Deacon

Quoted 36 lines · Ard Biesheuvel
52-bit VA kernels can run on hardware that is only 48-bit capable, but configure the ID map as 52-bit by default. This was not a problem until recently, because the special T0SZ value for a 52-bit VA space was never programmed into the TCR register anwyay, and because a 52-bit ID map happens to use the same number of translation levels as a 48-bit one.
This behavior was changed by commit 1401bef703a4 ("arm64: mm: Always update TCR_EL1 from __cpu_set_tcr_t0sz()"), which causes the unsupported T0SZ value for a 52-bit VA to be programmed into TCR_EL1. While some hardware simply ignores this, Mark reports that Amberwing systems choke on this, resulting in a broken boot. But even before that commit, the unsupported idmap_t0sz value was exposed to KVM and used to program TCR_EL2 incorrectly as well.
Given that we already have to deal with address spaces being either 48-bit or 52-bit in size, the cleanest approach seems to be to simply default to a 48-bit VA ID map, and only switch to a 52-bit one if the placement of the kernel in DRAM requires it. This is guaranteed not to happen unless the system is actually 52-bit VA capable.
Fixes 90ec95cda91a ("arm64: mm: Introduce VA_BITS_MIN")
Reported-by Mark Salter <[email protected]>
Link http://lore.kernel.org/r/[email protected] open here
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/mmu_context.h | 5 +----
 arch/arm64/kernel/head.S             | 2 +-
 arch/arm64/mm/mmu.c                  | 2 +-
 3 files changed, 3 insertions(+), 6 deletions(-)
arch/arm64/include/asm/mmu_context.h +0 -0
index 70ce8c1d2b07..0f467d550f27 100644
@@ -65,10 +65,7 @@ extern u64 idmap_ptrs_per_pgd;
 

Quoted 12 lines

arch/arm64/include/asm/mmu_context.h extern u64 idmap_ptrs_per_pgd; · line 68
Quoted 7 lines
arch/arm64/include/asm/mmu_context.h +1 -4
 static inline bool __cpu_uses_extended_idmap(void)
 {
-     if (IS_ENABLED(CONFIG_ARM64_VA_BITS_52))
-             return false;
-
-     return unlikely(idmap_t0sz != TCR_T0SZ(VA_BITS));
+     return unlikely(idmap_t0sz != TCR_T0SZ(vabits_actual));
I'm a bit confused by this: if the idmap is using 48-bit VAs but vabits_actual is 52, then we'll return true despite not using the extended idmap. Have I missed something, or should that be < instead of != ?

Yeah, this is because before, we only ever extended the idmap, but in this case, we are reducing it: the ID map is always 48 bits, and the user space page tables could be 48 or 52. In the latter case, this means we need to update TCR when we switch to the ID map or vice versa, similar to how we switch to a 48-bit VA ID map on a 39-bit VA kernel when physical DRAM cannot be covered with 39 bits.

The good news is that this function is dropped entirely in the next patch. This hunk is only there for backports of this patch to -stable. Therefore, I don't see the point to renaming it to sth like '__cpu_has_different_idmap_size()'

Quoted 18 lines

Quoted 8 lines
arch/arm64/include/asm/mmu_context.h +0 -0
 }
 
 /*
arch/arm64/kernel/head.S +0 -0
index 66b0e0b66e31..0b0387644dc0 100644
arch/arm64/kernel/head.S SYM_FUNC_START_LOCAL(__create_page_tables) · line 323
Quoted 6 lines
arch/arm64/kernel/head.S +1 -1
@@ -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
(nit: the comment immediately before this talks about VA_BITS and should probably be updated)

Quoted 1 line

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-10 17:16 UTC #4 lore raw

PATCH 2/5 arm64: mm: remove unused __cpu_uses_extended_idmap[_level()]

In reply to Ard Biesheuvel

These routines lost all existing users during the latest merge window so we can remove them. This avoids the need to fix them in the context of fixing a regression related to the ID map on 52-bit VA kernels.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/mmu_context.h | 14 --------------
 1 file changed, 14 deletions(-)

arch/arm64/include/asm/mmu_context.h +0 ~~-14~~

index 0f467d550f27..bd02e99b1a4c 100644
@@ -63,20 +63,6 @@ static inline void cpu_switch_mm(pgd_t *pgd, struct mm_struct *mm)
 extern u64 idmap_t0sz;
 extern u64 idmap_ptrs_per_pgd;
 
-static inline bool __cpu_uses_extended_idmap(void)
-{
-	return unlikely(idmap_t0sz != TCR_T0SZ(vabits_actual));
-}
-
-/*
- * True if the extended ID map requires an extra level of translation table
- * to be configured.
- */
-static inline bool __cpu_uses_extended_idmap_level(void)
-{
-	return ARM64_HW_PGTABLE_LEVELS(64 - idmap_t0sz) > CONFIG_PGTABLE_LEVELS;
-}
-
 /*
  * Ensure TCR.T0SZ is set to the provided value.
  */

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-10 17:17 UTC #5 lore raw

PATCH 3/5 arm64: mm: use a compile time constant for vabits_actual when possible

In reply to Ard Biesheuvel

The size of the kernel VA space is a compile time constant unless the kernel is built to support 52-bit virtual addressing, which today is only supported on 64k page size kernels (although this has recently changed in the architecture).

This means that in many configurations, vabits_actual can never deviate from its build time default, making it rather pointless to carry this value in a variable. So use a compile time constant for vabits_actual unless it can really assume different values.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/memory.h |  4 ++++
 arch/arm64/kernel/head.S        | 12 ++++++------
 arch/arm64/mm/mmu.c             |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)

arch/arm64/include/asm/memory.h +4 -0

index c759faf7a1ff..501c5c87ec0a 100644
@@ -179,7 +179,11 @@
 #include <linux/types.h>
 #include <asm/bug.h>
 
+#ifdef CONFIG_ARM64_VA_BITS_52
 extern u64			vabits_actual;
+#else
+#define vabits_actual		((u64)VA_BITS)
+#endif
 
 extern s64			memstart_addr;
 /* PHYS_OFFSET - the physical address of the start of memory. */

arch/arm64/kernel/head.S +6 -6

index 0b0387644dc0..f65d17a90204 100644
@@ -294,16 +294,16 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 
 #ifdef CONFIG_ARM64_VA_BITS_52
 	mrs_s	x6, SYS_ID_AA64MMFR2_EL1
-	and	x6, x6, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
-	mov	x5, #52
-	cbnz	x6, 1f
-#endif
-	mov	x5, #VA_BITS_MIN
-1:
+	tst	x6, #(0xf << ID_AA64MMFR2_LVA_SHIFT)
+	mov	x5, #VA_BITS
+	mov	x6, #VA_BITS_MIN
+	csel	x5, x5, x6, ne
+
 	adr_l	x6, vabits_actual
 	str	x5, [x6]
 	dmb	sy
 	dc	ivac, x6		// Invalidate potentially stale cache line
+#endif
 
 	/*
 	 * VA_BITS may be too small to allow for an ID mapping to be created

arch/arm64/mm/mmu.c +2 -0

index 4c5603c41870..338658cd1bea 100644
@@ -43,8 +43,10 @@
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS_MIN);
 u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
 
+#ifdef CONFIG_ARM64_VA_BITS_52
 u64 __section(".mmuoff.data.write") vabits_actual;
 EXPORT_SYMBOL(vabits_actual);
+#endif
 
 u64 kimage_voffset __ro_after_init;
 EXPORT_SYMBOL(kimage_voffset);

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Will Deacon

maintainer

2021-03-11 09:50 UTC #6 lore raw

Re: [PATCH 3/5] arm64: mm: use a compile time constant for vabits_actual when possible

Quoted 22 lines · Ard Biesheuvel

The size of the kernel VA space is a compile time constant unless the kernel is built to support 52-bit virtual addressing, which today is only supported on 64k page size kernels (although this has recently changed in the architecture).
This means that in many configurations, vabits_actual can never deviate from its build time default, making it rather pointless to carry this value in a variable. So use a compile time constant for vabits_actual unless it can really assume different values.
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/memory.h |  4 ++++
 arch/arm64/kernel/head.S        | 12 ++++++------
 arch/arm64/mm/mmu.c             |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)
arch/arm64/include/asm/memory.h +0 -0
index c759faf7a1ff..501c5c87ec0a 100644

arch/arm64/include/asm/memory.h line 186

Quoted 8 lines

arch/arm64/include/asm/memory.h +4 -0

@@ -179,7 +179,11 @@
 #include <linux/types.h>
 #include <asm/bug.h>
 
+#ifdef CONFIG_ARM64_VA_BITS_52
 extern u64			vabits_actual;
+#else
+#define vabits_actual		((u64)VA_BITS)
+#endif

Maybe we should have VA_BITS_MIN, VA_BITS and VA_BITS_MAX instead of the current VA_BITS_MIN, VA_BITS and vabits_actual? The current naming is definitely a source of confusion for me.

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-11 17:25 UTC #7 lore raw

Re: [PATCH 3/5] arm64: mm: use a compile time constant for vabits_actual when possible

Quoted 27 lines · Will Deacon

Quoted 25 lines · Ard Biesheuvel
The size of the kernel VA space is a compile time constant unless the kernel is built to support 52-bit virtual addressing, which today is only supported on 64k page size kernels (although this has recently changed in the architecture).
This means that in many configurations, vabits_actual can never deviate from its build time default, making it rather pointless to carry this value in a variable. So use a compile time constant for vabits_actual unless it can really assume different values.
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/memory.h |  4 ++++
 arch/arm64/kernel/head.S        | 12 ++++++------
 arch/arm64/mm/mmu.c             |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)
arch/arm64/include/asm/memory.h +0 -0
index c759faf7a1ff..501c5c87ec0a 100644
@@ -179,7 +179,11 @@
 #include <linux/types.h>
 #include <asm/bug.h>
 

Quoted 10 lines

arch/arm64/include/asm/memory.h line 186
Quoted 5 lines
arch/arm64/include/asm/memory.h +4 -0
+#ifdef CONFIG_ARM64_VA_BITS_52
 extern u64                   vabits_actual;
+#else
+#define vabits_actual                ((u64)VA_BITS)
+#endif
Maybe we should have VA_BITS_MIN, VA_BITS and VA_BITS_MAX instead of the current VA_BITS_MIN, VA_BITS and vabits_actual? The current naming is definitely a source of confusion for me.

We will still need build time and runtime versions, and at build time, we sometimes need the minimum value (e.g, for sizing the vmalloc region) and sometimes the maximum value (e.g., in the definition of PTRS_PER_PGD). We could rename VA_BITS to VA_BITS_MAX, as you suggest, and find another name for vabits_actual, but we definitely need all these three quantities in one way or another.

However, renaming vabits_actual to VA_BITS is likely to cause confusion as well, due to the change in meaning, as well as the fact that all-caps identifiers are usually build time constants.

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Will Deacon

maintainer

2021-03-11 18:46 UTC #8 lore raw

Re: [PATCH 3/5] arm64: mm: use a compile time constant for vabits_actual when possible

Quoted 41 lines · Ard Biesheuvel

Quoted 27 lines · Will Deacon
Quoted 25 lines · Ard Biesheuvel
The size of the kernel VA space is a compile time constant unless the kernel is built to support 52-bit virtual addressing, which today is only supported on 64k page size kernels (although this has recently changed in the architecture).
This means that in many configurations, vabits_actual can never deviate from its build time default, making it rather pointless to carry this value in a variable. So use a compile time constant for vabits_actual unless it can really assume different values.
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/memory.h |  4 ++++
 arch/arm64/kernel/head.S        | 12 ++++++------
 arch/arm64/mm/mmu.c             |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)
arch/arm64/include/asm/memory.h +0 -0
index c759faf7a1ff..501c5c87ec0a 100644
@@ -179,7 +179,11 @@
 #include <linux/types.h>
 #include <asm/bug.h>
 
Quoted 10 lines
arch/arm64/include/asm/memory.h line 186
Quoted 5 lines
arch/arm64/include/asm/memory.h +4 -0
+#ifdef CONFIG_ARM64_VA_BITS_52
 extern u64                   vabits_actual;
+#else
+#define vabits_actual                ((u64)VA_BITS)
+#endif
Maybe we should have VA_BITS_MIN, VA_BITS and VA_BITS_MAX instead of the current VA_BITS_MIN, VA_BITS and vabits_actual? The current naming is definitely a source of confusion for me.

Quoted 6 lines

We will still need build time and runtime versions, and at build time, we sometimes need the minimum value (e.g, for sizing the vmalloc region) and sometimes the maximum value (e.g., in the definition of PTRS_PER_PGD). We could rename VA_BITS to VA_BITS_MAX, as you suggest, and find another name for vabits_actual, but we definitely need all these three quantities in one way or another.

Right, I was thinking that for !52-bit VA we've have:

#define VA_BITS CONFIG_ARM64_VA_BITS
#define VA_BITS_MIN VA_BITS_MIN
#define VA_BITS_MAX VA_BITS_MIN

and then for 52-bit VA we'd have:

#define VA_BITS (vabits_actual)
#define VA_BITS_MIN 48
#define VA_BITS_MAX 52

Quoted 3 lines

However, renaming vabits_actual to VA_BITS is likely to cause confusion as well, due to the change in meaning, as well as the fact that all-caps identifiers are usually build time constants.

Yeah, fair enough, but it reads a tonne better imo.

Will

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-11 18:54 UTC #9 lore raw

Re: [PATCH 3/5] arm64: mm: use a compile time constant for vabits_actual when possible

Quoted 62 lines · Will Deacon

Quoted 41 lines · Ard Biesheuvel
Quoted 27 lines · Will Deacon
Quoted 25 lines · Ard Biesheuvel
The size of the kernel VA space is a compile time constant unless the kernel is built to support 52-bit virtual addressing, which today is only supported on 64k page size kernels (although this has recently changed in the architecture).
This means that in many configurations, vabits_actual can never deviate from its build time default, making it rather pointless to carry this value in a variable. So use a compile time constant for vabits_actual unless it can really assume different values.
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/memory.h |  4 ++++
 arch/arm64/kernel/head.S        | 12 ++++++------
 arch/arm64/mm/mmu.c             |  2 ++
 3 files changed, 12 insertions(+), 6 deletions(-)
arch/arm64/include/asm/memory.h +0 -0
index c759faf7a1ff..501c5c87ec0a 100644
@@ -179,7 +179,11 @@
 #include <linux/types.h>
 #include <asm/bug.h>
 
Quoted 10 lines
arch/arm64/include/asm/memory.h line 186
Quoted 5 lines
arch/arm64/include/asm/memory.h +4 -0
+#ifdef CONFIG_ARM64_VA_BITS_52
 extern u64                   vabits_actual;
+#else
+#define vabits_actual                ((u64)VA_BITS)
+#endif
Maybe we should have VA_BITS_MIN, VA_BITS and VA_BITS_MAX instead of the current VA_BITS_MIN, VA_BITS and vabits_actual? The current naming is definitely a source of confusion for me.
Quoted 6 lines
We will still need build time and runtime versions, and at build time, we sometimes need the minimum value (e.g, for sizing the vmalloc region) and sometimes the maximum value (e.g., in the definition of PTRS_PER_PGD). We could rename VA_BITS to VA_BITS_MAX, as you suggest, and find another name for vabits_actual, but we definitely need all these three quantities in one way or another.
Right, I was thinking that for !52-bit VA we've have:
#define VA_BITS         CONFIG_ARM64_VA_BITS
#define VA_BITS_MIN     VA_BITS_MIN
#define VA_BITS_MAX     VA_BITS_MIN
and then for 52-bit VA we'd have:
#define VA_BITS         (vabits_actual)
#define VA_BITS_MIN     48
#define VA_BITS_MAX     52

Quoted 6 lines

Quoted 3 lines
However, renaming vabits_actual to VA_BITS is likely to cause confusion as well, due to the change in meaning, as well as the fact that all-caps identifiers are usually build time constants.
Yeah, fair enough, but it reads a tonne better imo.

Can't we just stop using VA_BITS altogether, and only keep the _MIN and _MAX variants?

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-10 17:17 UTC #10 lore raw

PATCH 4/5 arm64: mm: get rid of idmap_ptrs_per_pgd handling

In reply to Ard Biesheuvel

idmap_ptrs_per_pgd is stored and re-loaded in the very early boot path, and never referenced again. So a variable is not needed here.

Furthermore, given that on the code path in question, PTRS_PER_PGD and '1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)' are guaranteed to be equal unless
- the kernel was built for 48-bit VAs and 52-bit PAs, and
- the kernel was loaded outside of the 48-bit addressable physical memory space (which is not supported by EFI to begin with),
it seems reasonable to simply drop this code path altogether.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/include/asm/mmu_context.h | 1 -
 arch/arm64/kernel/head.S             | 9 +--------
 arch/arm64/mm/mmu.c                  | 1 -
 3 files changed, 1 insertion(+), 10 deletions(-)

arch/arm64/include/asm/mmu_context.h +0 -1

index bd02e99b1a4c..1307f5e27a5a 100644
@@ -61,7 +61,6 @@ static inline void cpu_switch_mm(pgd_t *pgd, struct mm_struct *mm)
  * physical memory, in which case it will be smaller.
  */
 extern u64 idmap_t0sz;
-extern u64 idmap_ptrs_per_pgd;
 
 /*
  * Ensure TCR.T0SZ is set to the provided value.

arch/arm64/kernel/head.S +1 -8

index f65d17a90204..da6e99fa4e08 100644
@@ -344,16 +344,9 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 
 	mov	x4, EXTRA_PTRS
 	create_table_entry x0, x3, EXTRA_SHIFT, x4, x5, x6
-#else
-	/*
-	 * If VA_BITS == 48, we don't have to configure an additional
-	 * translation level, but the top-level table has more entries.
-	 */
-	mov	x4, #1 << (PHYS_MASK_SHIFT - PGDIR_SHIFT)
-	str_l	x4, idmap_ptrs_per_pgd, x5
 #endif
 1:
-	ldr_l	x4, idmap_ptrs_per_pgd
+	mov	x4, PTRS_PER_PGD
 	mov	x5, x3				// __pa(__idmap_text_start)
 	adr_l	x6, __idmap_text_end		// __pa(__idmap_text_end)

arch/arm64/mm/mmu.c +0 -1

index 338658cd1bea..569cf07ddb91 100644
@@ -41,7 +41,6 @@
 #define NO_CONT_MAPPINGS	BIT(1)
 
 u64 idmap_t0sz = TCR_T0SZ(VA_BITS_MIN);
-u64 idmap_ptrs_per_pgd = PTRS_PER_PGD;
 
 #ifdef CONFIG_ARM64_VA_BITS_52
 u64 __section(".mmuoff.data.write") vabits_actual;

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Ard Biesheuvel

author

2021-03-10 17:17 UTC #11 lore raw

PATCH 5/5 arm64: mm: switch to 52-bit ID map on 52-bit VA capable systems

In reply to Ard Biesheuvel

Now that we cleaned up the code a bit, add back the code path to use a 52-bit VA ID map on hardware that is 52-bit VA capable. This removes a TCR update from the cpuidle path.

At the same time, make the 52-bit VA handling mutually exclusive with the extended ID map code: a 52-bit VA kernel running on 48-bit VA only hardware never has a need for it.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/arm64/kernel/head.S | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

arch/arm64/kernel/head.S +4 -1

index da6e99fa4e08..06f1ddeef821 100644
@@ -303,8 +303,10 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 	str	x5, [x6]
 	dmb	sy
 	dc	ivac, x6		// Invalidate potentially stale cache line
-#endif
 
+	mov	x5, TCR_T0SZ(VA_BITS)
+	b.eq	1f			// skip VA range extension on !LVA hardware
+#else
 	/*
 	 * VA_BITS may be too small to allow for an ID mapping to be created
 	 * that covers system RAM if that is located sufficiently high in the

@@ -321,6 +323,7 @@ SYM_FUNC_START_LOCAL(__create_page_tables)
 	clz	x5, x5
 	cmp	x5, TCR_T0SZ(VA_BITS_MIN) // default T0SZ small enough?
 	b.ge	1f			// .. then skip VA range extension
+#endif
 
 	adr_l	x6, idmap_t0sz
 	str	x5, [x6]

Signature · 7 lines

2.30.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Will Deacon

maintainer

2021-03-11 13:28 UTC #12 lore raw

In reply to Ard Biesheuvel

Quoted 10 lines · Ard Biesheuvel

This work was triggered by Mark Salter's report about a regression regarding the programmed size of the ID map page tables.
The first patch switches the early ID map population code to default to a 48-bit range on kernels built with 52-bit VA support, and only switch to 52-bit if the placement of the kernel requires it. This should address the regressions reported by Mark. This patch should be suitable for being backported to -stable.
[...]

Applied the first two patches to arm64 (for-next/fixes), thanks!

[1/5] arm64: mm: use a 48-bit ID map when possible on 52-bit VA builds

      https://git.kernel.org/arm64/c/7ba8f2b2d652
[2/5] arm64: mm: remove unused __cpu_uses_extended_idmap[_level()]
      https://git.kernel.org/arm64/c/30b2675761b8

Cheers,

Signature · 10 lines

Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel