arm64: Clean up and simplify PA space size handling

PATCH Superseded Ard Biesheuvel 4 participants 14 messages Nov 11, 2024 – Nov 15, 2024

Subsystems (4)

Stable branch arm64 KVM arm64 GDB kernel debugging helper scripts

lore

Share & tools

lore

Ard Biesheuvel

author

2024-11-11 08:38 UTC #0 lore raw

cover arm64: Clean up and simplify PA space size handling

From: Ard Biesheuvel <[email protected]>

This series addresses a number of buglets related to how we handle the size of the physical address space when building LPA2 capable kernels:

- reject 52-bit physical addressess in the mapping routines when LPA2 is configured but not available at runtime
- ensure that TCR.IPS is not set to 52-bits if LPA2 is not supported
- ensure that TCR_EL2.PS and DS match the host, regardless of whether LPA2 is available at stage 2
- don't rely on kvm_get_parange() and invalid physical addresses as control flags in the pKVM page donation APIs

Finally, the configurable 48-bit physical address space limit is dropped entirely, as it doesn't buy us a lot now that all the PARange and {I}PS handling is done at runtime.

Cc Catalin Marinas <[email protected]>

Cc Will Deacon <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Ryan Roberts <[email protected]>

Cc Anshuman Khandual <[email protected]>

Cc Kees Cook <[email protected]>

Ard Biesheuvel (6):
arm64/mm: Reduce PA space to 48 bits when LPA2 is not enabled arm64/mm: Override PARange for !LPA2 and use it consistently arm64/kvm: Configure HYP TCR.PS/DS based on host stage1 arm64/kvm: Avoid invalid physical addresses to signal owner updates
arm64: Kconfig: force ARM64_PAN=y when enabling TTBR0 sw PAN arm64/mm: Drop configurable 48-bit physical address space limit

 arch/arm64/Kconfig                     | 37 ++------------------
 arch/arm64/include/asm/assembler.h     | 14 +++-----
 arch/arm64/include/asm/cpufeature.h    |  3 +-
 arch/arm64/include/asm/kvm_pgtable.h   |  3 +-
 arch/arm64/include/asm/pgtable-hwdef.h | 12 +------
 arch/arm64/include/asm/pgtable-prot.h  |  7 ++++
 arch/arm64/include/asm/pgtable.h       | 11 +-----
 arch/arm64/include/asm/sparsemem.h     |  4 ++-
 arch/arm64/include/asm/sysreg.h        |  6 ----
 arch/arm64/kernel/cpufeature.c         |  2 +-
 arch/arm64/kernel/pi/idreg-override.c  |  9 +++++
 arch/arm64/kernel/pi/map_kernel.c      |  6 ++++
 arch/arm64/kvm/arm.c                   |  8 ++---
 arch/arm64/kvm/hyp/pgtable.c           | 37 ++++++++------------
 arch/arm64/mm/init.c                   |  2 +-
 arch/arm64/mm/pgd.c                    |  9 ++---
 arch/arm64/mm/proc.S                   |  2 --
 scripts/gdb/linux/constants.py.in      |  1 -
 tools/arch/arm64/include/asm/sysreg.h  |  6 ----
 19 files changed, 62 insertions(+), 117 deletions(-)

Signature · 1 line

2.47.0.277.g8800431eea-goog

Ard Biesheuvel

author

2024-11-11 08:35 UTC #1 lore raw

PATCH 1/6 arm64/mm: Reduce PA space to 48 bits when LPA2 is not enabled

From: Ard Biesheuvel <[email protected]>

Currently, LPA2 support implies support for up to 52 bits of physical addressing, and this is reflected in global definitions such as PHYS_MASK_SHIFT and MAX_PHYSMEM_BITS.

This is potentially problematic, given that LPA2 support is modeled as a CPU feature which can be overridden, and with LPA2 support turned off, attempting to map physical regions with address bits [51:48] set (which may exist on LPA2 capable systems booting with arm64.nolva) will result in corrupted mappings with a truncated output address and bogus shareability attributes.

This means that the accepted physical address range in the mapping routines should be at most 48 bits wide when LPA2 is supported but not enabled.

Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")

Cc <[email protected]>

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

---
 arch/arm64/include/asm/pgtable-hwdef.h | 6 ------
 arch/arm64/include/asm/pgtable-prot.h  | 7 +++++++
 arch/arm64/include/asm/sparsemem.h     | 4 +++-
 3 files changed, 10 insertions(+), 7 deletions(-)

arch/arm64/include/asm/pgtable-hwdef.h +0 -6

index fd330c1db289..a970def932aa 100644
@@ -218,12 +218,6 @@
  */
 #define S1_TABLE_AP		(_AT(pmdval_t, 3) << 61)
 
-/*
- * Highest possible physical address supported.
- */
-#define PHYS_MASK_SHIFT		(CONFIG_ARM64_PA_BITS)
-#define PHYS_MASK		((UL(1) << PHYS_MASK_SHIFT) - 1)
-
 #define TTBR_CNP_BIT		(UL(1) << 0)
 
 /*

arch/arm64/include/asm/pgtable-prot.h +7 -0

index 9f9cf13bbd95..a95f1f77bb39 100644
@@ -81,6 +81,7 @@ extern unsigned long prot_ns_shared;
 #define lpa2_is_enabled()	false
 #define PTE_MAYBE_SHARED	PTE_SHARED
 #define PMD_MAYBE_SHARED	PMD_SECT_S
+#define PHYS_MASK_SHIFT		(CONFIG_ARM64_PA_BITS)
 #else
 static inline bool __pure lpa2_is_enabled(void)
 {

@@ -89,8 +90,14 @@ static inline bool __pure lpa2_is_enabled(void)
 
 #define PTE_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PTE_SHARED)
 #define PMD_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PMD_SECT_S)
+#define PHYS_MASK_SHIFT		(lpa2_is_enabled() ? CONFIG_ARM64_PA_BITS : 48)
 #endif
 
+/*
+ * Highest possible physical address supported.
+ */
+#define PHYS_MASK		((UL(1) << PHYS_MASK_SHIFT) - 1)
+
 /*
  * If we have userspace only BTI we don't want to mark kernel pages
  * guarded even if the system does support BTI.

arch/arm64/include/asm/sparsemem.h +3 -1

index 8a8acc220371..035e0ca74e88 100644
@@ -5,7 +5,9 @@
 #ifndef __ASM_SPARSEMEM_H
 #define __ASM_SPARSEMEM_H
 
-#define MAX_PHYSMEM_BITS	CONFIG_ARM64_PA_BITS
+#include <asm/pgtable-prot.h>
+
+#define MAX_PHYSMEM_BITS	PHYS_MASK_SHIFT
 
 /*
  * Section size must be at least 512MB for 64K base

Signature · 1 line

2.47.0.277.g8800431eea-goog

Anshuman Khandual

Reviewed-by on v1 1/6

2024-11-15 05:05 UTC #2 lore raw

Re: [PATCH 1/6] arm64/mm: Reduce PA space to 48 bits when LPA2 is not enabled

Quoted 75 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>
Currently, LPA2 support implies support for up to 52 bits of physical addressing, and this is reflected in global definitions such as PHYS_MASK_SHIFT and MAX_PHYSMEM_BITS.
This is potentially problematic, given that LPA2 support is modeled as a CPU feature which can be overridden, and with LPA2 support turned off, attempting to map physical regions with address bits [51:48] set (which may exist on LPA2 capable systems booting with arm64.nolva) will result in corrupted mappings with a truncated output address and bogus shareability attributes.
This means that the accepted physical address range in the mapping routines should be at most 48 bits wide when LPA2 is supported but not enabled.
Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")
Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/pgtable-hwdef.h | 6 ------
 arch/arm64/include/asm/pgtable-prot.h  | 7 +++++++
 arch/arm64/include/asm/sparsemem.h     | 4 +++-
 3 files changed, 10 insertions(+), 7 deletions(-)
arch/arm64/include/asm/pgtable-hwdef.h +0 -6
index fd330c1db289..a970def932aa 100644
@@ -218,12 +218,6 @@
  */
 #define S1_TABLE_AP		(_AT(pmdval_t, 3) << 61)
 
-/*
- * Highest possible physical address supported.
- */
-#define PHYS_MASK_SHIFT		(CONFIG_ARM64_PA_BITS)
-#define PHYS_MASK		((UL(1) << PHYS_MASK_SHIFT) - 1)
-
 #define TTBR_CNP_BIT		(UL(1) << 0)
 
 /*
arch/arm64/include/asm/pgtable-prot.h +7 -0
index 9f9cf13bbd95..a95f1f77bb39 100644
@@ -81,6 +81,7 @@ extern unsigned long prot_ns_shared;
 #define lpa2_is_enabled()	false
 #define PTE_MAYBE_SHARED	PTE_SHARED
 #define PMD_MAYBE_SHARED	PMD_SECT_S
+#define PHYS_MASK_SHIFT		(CONFIG_ARM64_PA_BITS)
 #else
 static inline bool __pure lpa2_is_enabled(void)
 {
@@ -89,8 +90,14 @@ static inline bool __pure lpa2_is_enabled(void)
 
 #define PTE_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PTE_SHARED)
 #define PMD_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PMD_SECT_S)
+#define PHYS_MASK_SHIFT		(lpa2_is_enabled() ? CONFIG_ARM64_PA_BITS : 48)
 #endif
 
+/*
+ * Highest possible physical address supported.
+ */
+#define PHYS_MASK		((UL(1) << PHYS_MASK_SHIFT) - 1)
+
 /*
  * If we have userspace only BTI we don't want to mark kernel pages
  * guarded even if the system does support BTI.
arch/arm64/include/asm/sparsemem.h +0 -0
index 8a8acc220371..035e0ca74e88 100644

arch/arm64/include/asm/sparsemem.h line 13

Quoted 10 lines

arch/arm64/include/asm/sparsemem.h +3 -1

@@ -5,7 +5,9 @@
 #ifndef __ASM_SPARSEMEM_H
 #define __ASM_SPARSEMEM_H
 
-#define MAX_PHYSMEM_BITS	CONFIG_ARM64_PA_BITS
+#include <asm/pgtable-prot.h>
+
+#define MAX_PHYSMEM_BITS	PHYS_MASK_SHIFT
 
 /*
  * Section size must be at least 512MB for 64K base

Reviewed-by Anshuman Khandual <[email protected]>

Ard Biesheuvel

author

2024-11-11 08:36 UTC #3 lore raw

PATCH 2/6 arm64/mm: Override PARange for !LPA2 and use it consistently

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.

Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.

So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.

Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")

Cc <[email protected]>

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

---
 arch/arm64/include/asm/assembler.h    | 5 +++++
 arch/arm64/kernel/cpufeature.c        | 2 +-
 arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++
 arch/arm64/kernel/pi/map_kernel.c     | 6 ++++++
 arch/arm64/mm/init.c                  | 2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)

arch/arm64/include/asm/assembler.h +5 -0

index 3d8d534a7a77..ad63457a05c5 100644
@@ -343,6 +343,11 @@ alternative_cb_end
 	// Narrow PARange to fit the PS field in TCR_ELx
 	ubfx	\tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
 	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
+#ifdef CONFIG_ARM64_LPA2
+alternative_if_not ARM64_HAS_VA52
+	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
+alternative_else_nop_endif
+#endif
 	cmp	\tmp0, \tmp1
 	csel	\tmp0, \tmp1, \tmp0, hi
 	bfi	\tcr, \tmp0, \pos, #3

arch/arm64/kernel/cpufeature.c +1 -1

index 37e4c02e0272..6f5137040ff6 100644
@@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void)
 		return;
 
 	safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
-	mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+	mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
 	mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 
 	/* Verify VMID bits */

arch/arm64/kernel/pi/idreg-override.c +9 -0

index 22159251eb3a..c6b185b885f7 100644
@@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val)
 		id_aa64mmfr0_override.val |=
 			(ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
 		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
+
+		/*
+		 * Override PARange to 48 bits - the override will just be
+		 * ignored if the actual PARange is smaller, but this is
+		 * unlikely to be the case for LPA2 capable silicon.
+		 */
+		id_aa64mmfr0_override.val |=
+			ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
+		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
 	}
 #endif
 	return true;

arch/arm64/kernel/pi/map_kernel.c +6 -0

index f374a3e5a5fe..e57b043f324b 100644
@@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
 {
 	u64 sctlr = read_sysreg(sctlr_el1);
 	u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
+	u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
+	u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
+							   ID_AA64MMFR0_EL1_PARANGE_SHIFT);
+
+	tcr &= ~TCR_IPS_MASK;
+	tcr |= parange << TCR_IPS_SHIFT;
 
 	asm("	msr	sctlr_el1, %0		;"
 	    "	isb				;"

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

index d21f67d67cf5..4db9887b2aef 100644
@@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
 		extern u16 memstart_offset_seed;
-		u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+		u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
 		int parange = cpuid_feature_extract_unsigned_field(
 					mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT);
 		s64 range = linear_region_size -

Signature · 1 line

2.47.0.277.g8800431eea-goog

Anshuman Khandual

2024-11-15 06:05 UTC #4 lore raw

Re: [PATCH 2/6] arm64/mm: Override PARange for !LPA2 and use it consistently

Quoted 31 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")
Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/assembler.h    | 5 +++++
 arch/arm64/kernel/cpufeature.c        | 2 +-
 arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++
 arch/arm64/kernel/pi/map_kernel.c     | 6 ++++++
 arch/arm64/mm/init.c                  | 2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)
arch/arm64/include/asm/assembler.h +0 -0
index 3d8d534a7a77..ad63457a05c5 100644

arch/arm64/include/asm/assembler.h alternative_cb_end · line 350

Quoted 8 lines

arch/arm64/include/asm/assembler.h +5 -0

@@ -343,6 +343,11 @@ alternative_cb_end
 	// Narrow PARange to fit the PS field in TCR_ELx
 	ubfx	\tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
 	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
+#ifdef CONFIG_ARM64_LPA2
+alternative_if_not ARM64_HAS_VA52
+	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
+alternative_else_nop_endif
+#endif

I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.

arch/arm64/kernel/cpufeature.c static void verify_hyp_capabilities(void) · line 3402

Quoted 13 lines

arch/arm64/include/asm/assembler.h +0 -0

 	cmp	\tmp0, \tmp1
 	csel	\tmp0, \tmp1, \tmp0, hi
 	bfi	\tcr, \tmp0, \pos, #3

arch/arm64/kernel/cpufeature.c +1 -1

index 37e4c02e0272..6f5137040ff6 100644
@@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void)
 		return;
 
 	safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
-	mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+	mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);

Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?

Quoted 10 lines

arch/arm64/kernel/cpufeature.c +0 -0

 	mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 
 	/* Verify VMID bits */

arch/arm64/kernel/pi/idreg-override.c +0 -0

index 22159251eb3a..c6b185b885f7 100644
@@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val)
 		id_aa64mmfr0_override.val |=
 			(ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT;

arch/arm64/kernel/pi/idreg-override.c static bool __init mmfr2_varange_filter(u64 val) · line 94

Quoted 10 lines

arch/arm64/kernel/pi/idreg-override.c +9 -0

 		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
+
+		/*
+		 * Override PARange to 48 bits - the override will just be
+		 * ignored if the actual PARange is smaller, but this is
+		 * unlikely to be the case for LPA2 capable silicon.
+		 */
+		id_aa64mmfr0_override.val |=
+			ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
+		id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;

Could these be used instead ?

SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?

Quoted 8 lines

arch/arm64/kernel/pi/idreg-override.c +0 -0
 	}
 #endif
 	return true;
arch/arm64/kernel/pi/map_kernel.c +0 -0
index f374a3e5a5fe..e57b043f324b 100644

arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 143

Quoted 8 lines

arch/arm64/kernel/pi/map_kernel.c +5 -0

@@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
 {
 	u64 sctlr = read_sysreg(sctlr_el1);
 	u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
+	u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
+	u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
+							   ID_AA64MMFR0_EL1_PARANGE_SHIFT);
+
+	tcr &= ~TCR_IPS_MASK;

Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.

arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 144

Quoted 1 line

arch/arm64/kernel/pi/map_kernel.c +1 -0
+	tcr |= parange << TCR_IPS_SHIFT;

Wondering if FIELD_PREP() could be used here.

arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 283

Quoted 13 lines

arch/arm64/kernel/pi/map_kernel.c +0 -0

 	asm("	msr	sctlr_el1, %0		;"
 	    "	isb				;"

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

index d21f67d67cf5..4db9887b2aef 100644
@@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
 
 	if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
 		extern u16 memstart_offset_seed;
-		u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+		u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);

Could this have a comment explaining the need for sanitized value ?

arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 286

Quoted 3 lines

arch/arm64/mm/init.c +0 -0

 		int parange = cpuid_feature_extract_unsigned_field(
 					mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT);
 		s64 range = linear_region_size -

Otherwise LGTM.

Ard Biesheuvel

2024-11-15 09:00 UTC #5 lore raw

Re: [PATCH 2/6] arm64/mm: Override PARange for !LPA2 and use it consistently

Hi Anshuman,

Quoted 37 lines · Anshuman Khandual

Quoted 35 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")
Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/assembler.h    | 5 +++++
 arch/arm64/kernel/cpufeature.c        | 2 +-
 arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++
 arch/arm64/kernel/pi/map_kernel.c     | 6 ++++++
 arch/arm64/mm/init.c                  | 2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)
arch/arm64/include/asm/assembler.h +1 -0
index 3d8d534a7a77..ad63457a05c5 100644
@@ -343,6 +343,11 @@ alternative_cb_end
      // Narrow PARange to fit the PS field in TCR_ELx
      ubfx    \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
      mov     \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
+#ifdef CONFIG_ARM64_LPA2

Quoted 9 lines

arch/arm64/include/asm/assembler.h alternative_cb_end · line 350
Quoted 4 lines
arch/arm64/include/asm/assembler.h +4 -0
+alternative_if_not ARM64_HAS_VA52
+     mov     \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
+alternative_else_nop_endif
+#endif
I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.

It is the other way around, actually. This limit will always be applied on primary boot, which is why IPS is updated again in set_ttbr0_for_lpa2() [below]. Before booting the secondaries (or other subsequent invocations of this code, e.g., in the resume path), this MOV will be NOPed out if LPA2 support is enabled.

Quoted 10 lines

arch/arm64/include/asm/assembler.h +0 -0

      cmp     \tmp0, \tmp1
      csel    \tmp0, \tmp1, \tmp0, hi
      bfi     \tcr, \tmp0, \pos, #3

arch/arm64/kernel/cpufeature.c +0 -0

index 37e4c02e0272..6f5137040ff6 100644
@@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void)
              return;

Quoted 6 lines

arch/arm64/kernel/cpufeature.c static void verify_hyp_capabilities(void) · line 3402
Quoted 3 lines
arch/arm64/kernel/cpufeature.c +1 -1
      safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
-     mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+     mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?

safe_mmfr1 exists because there is also mmfr1 in the same scope. No such distinction exists for mmfr0, so I opted for keeping the name.

Quoted 22 lines

Quoted 10 lines

arch/arm64/kernel/cpufeature.c +0 -0

      mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 
      /* Verify VMID bits */

arch/arm64/kernel/pi/idreg-override.c +0 -0

index 22159251eb3a..c6b185b885f7 100644
@@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val)
              id_aa64mmfr0_override.val |=
                      (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT;

arch/arm64/kernel/pi/idreg-override.c static bool __init mmfr2_varange_filter(u64 val) · line 94

Quoted 10 lines

arch/arm64/kernel/pi/idreg-override.c +9 -0

              id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
+
+             /*
+              * Override PARange to 48 bits - the override will just be
+              * ignored if the actual PARange is smaller, but this is
+              * unlikely to be the case for LPA2 capable silicon.
+              */
+             id_aa64mmfr0_override.val |=
+                     ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
+             id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;

Could these be used instead ?

Quoted 3 lines

SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?

Yes, but 2 lines before, there is another occurrence of this idiom, and I did not want to deviate from that.

We could update both, or update the other one first in a separate patch, I suppose.

Quoted 12 lines

arch/arm64/kernel/pi/idreg-override.c +0 -0

      }
 #endif
      return true;

arch/arm64/kernel/pi/map_kernel.c +0 -0

index f374a3e5a5fe..e57b043f324b 100644
@@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
 {
      u64 sctlr = read_sysreg(sctlr_el1);
      u64 tcr = read_sysreg(tcr_el1) | TCR_DS;

Quoted 9 lines

arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 143
Quoted 5 lines
arch/arm64/kernel/pi/map_kernel.c +5 -0
+     u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
+     u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
+                                                        ID_AA64MMFR0_EL1_PARANGE_SHIFT);
+
+     tcr &= ~TCR_IPS_MASK;
Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.

As explained above, TCR.IPS will be capped at 48 bits up to this point.

Quoted 4 lines

arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 144
Quoted 1 line
arch/arm64/kernel/pi/map_kernel.c +1 -0
+     tcr |= parange << TCR_IPS_SHIFT;
Wondering if FIELD_PREP() could be used here.

AIUI we'd end up with

tcr &= ~TCR_IPS_MASK;
tcr |= FIELD_PREP(TCR_IPS_MASK, parange);

Is that really so much better?

Quoted 9 lines

arch/arm64/kernel/pi/map_kernel.c +0 -0

      asm("   msr     sctlr_el1, %0           ;"
          "   isb                             ;"

arch/arm64/mm/init.c +0 -0

index d21f67d67cf5..4db9887b2aef 100644
@@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)

Quoted 7 lines

arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 283
Quoted 4 lines
arch/arm64/mm/init.c +1 -1
      if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
              extern u16 memstart_offset_seed;
-             u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+             u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Could this have a comment explaining the need for sanitized value ?

Sure. Actually, it shouldn't make any difference here (unless we allow PARange to be narrowed even further, which might make sense if we care about enabling randomization of the linear map on systems where PARange is much larger than the size of the physical address space that is actually populated). However, for consistency, it is better to avoid the 52-bit PARange if LPA2 is disabled.

Quoted 5 lines

arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 286

Quoted 3 lines

arch/arm64/mm/init.c +0 -0

              int parange = cpuid_feature_extract_unsigned_field(
                                      mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT);
              s64 range = linear_region_size -

Otherwise LGTM.

Thanks!

Anshuman Khandual

2024-11-15 11:47 UTC #6 lore raw

Re: [PATCH 2/6] arm64/mm: Override PARange for !LPA2 and use it consistently

Quoted 51 lines · Ard Biesheuvel

Hi Anshuman,
Quoted 37 lines · Anshuman Khandual
Quoted 35 lines · Ard Biesheuvel
From: Ard Biesheuvel <[email protected]>
When FEAT_LPA{,2} are not implemented, the ID_AA64MMFR0_EL1.PARange and TCR.IPS values corresponding with 52-bit physical addressing are reserved.
Setting the TCR.IPS field to 0b110 (52-bit physical addressing) has side effects, such as how the TTBRn_ELx.BADDR fields are interpreted, and so it is important that disabling FEAT_LPA2 (by overriding the ID_AA64MMFR0.TGran fields) also presents a PARange field consistent with that.
So limit the field to 48 bits unless LPA2 is enabled, and update existing references to use the override consistently.
Fixes 352b0395b505 ("arm64: Enable 52-bit virtual addressing for 4k and 16k granule configs")
Cc <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/include/asm/assembler.h    | 5 +++++
 arch/arm64/kernel/cpufeature.c        | 2 +-
 arch/arm64/kernel/pi/idreg-override.c | 9 +++++++++
 arch/arm64/kernel/pi/map_kernel.c     | 6 ++++++
 arch/arm64/mm/init.c                  | 2 +-
 5 files changed, 22 insertions(+), 2 deletions(-)
arch/arm64/include/asm/assembler.h +1 -0
index 3d8d534a7a77..ad63457a05c5 100644
@@ -343,6 +343,11 @@ alternative_cb_end
      // Narrow PARange to fit the PS field in TCR_ELx
      ubfx    \tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
      mov     \tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
+#ifdef CONFIG_ARM64_LPA2
Quoted 9 lines
arch/arm64/include/asm/assembler.h alternative_cb_end · line 350
Quoted 4 lines
arch/arm64/include/asm/assembler.h +4 -0
+alternative_if_not ARM64_HAS_VA52
+     mov     \tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
+alternative_else_nop_endif
+#endif
I guess this will only take effect after cpu features have been finalized but will not be applicable for __cpu_setup() during primary and secondary cpu bring up during boot.

Quoted 1 line

It is the other way around, actually. This limit will always be

Right, missed the '_if_not' part.

Quoted 4 lines

applied on primary boot, which is why IPS is updated again in set_ttbr0_for_lpa2() [below]. Before booting the secondaries (or other subsequent invocations of this code, e.g., in the resume path), this MOV will be NOPed out if LPA2 support is enabled.

Understood.

Quoted 19 lines

Quoted 12 lines
Quoted 12 lines
arch/arm64/include/asm/assembler.h +0 -0
      cmp     \tmp0, \tmp1
      csel    \tmp0, \tmp1, \tmp0, hi
      bfi     \tcr, \tmp0, \pos, #3
arch/arm64/kernel/cpufeature.c +0 -0
index 37e4c02e0272..6f5137040ff6 100644
@@ -3399,7 +3399,7 @@ static void verify_hyp_capabilities(void)
              return;
 
Quoted 6 lines
arch/arm64/kernel/cpufeature.c static void verify_hyp_capabilities(void) · line 3402
Quoted 3 lines
arch/arm64/kernel/cpufeature.c +1 -1
      safe_mmfr1 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
-     mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+     mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Small nit, should be renamed as safe_mmfr0 to be consistent with safe_mmfr1 ?

Quoted 2 lines

safe_mmfr1 exists because there is also mmfr1 in the same scope. No such distinction exists for mmfr0, so I opted for keeping the name.

Fair enough.

Quoted 27 lines

Quoted 23 lines

Quoted 11 lines

arch/arm64/kernel/cpufeature.c +0 -0

      mmfr1 = read_cpuid(ID_AA64MMFR1_EL1);
 
      /* Verify VMID bits */

arch/arm64/kernel/pi/idreg-override.c +0 -0

index 22159251eb3a..c6b185b885f7 100644
@@ -83,6 +83,15 @@ static bool __init mmfr2_varange_filter(u64 val)
              id_aa64mmfr0_override.val |=
                      (ID_AA64MMFR0_EL1_TGRAN_LPA2 - 1) << ID_AA64MMFR0_EL1_TGRAN_SHIFT;

arch/arm64/kernel/pi/idreg-override.c static bool __init mmfr2_varange_filter(u64 val) · line 94

Quoted 10 lines

arch/arm64/kernel/pi/idreg-override.c +9 -0

              id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_TGRAN_SHIFT;
+
+             /*
+              * Override PARange to 48 bits - the override will just be
+              * ignored if the actual PARange is smaller, but this is
+              * unlikely to be the case for LPA2 capable silicon.
+              */
+             id_aa64mmfr0_override.val |=
+                     ID_AA64MMFR0_EL1_PARANGE_48 << ID_AA64MMFR0_EL1_PARANGE_SHIFT;
+             id_aa64mmfr0_override.mask |= 0xfU << ID_AA64MMFR0_EL1_PARANGE_SHIFT;

Could these be used instead ?

Quoted 3 lines

SYS_FIELD_PREP_ENUM(ID_AA64MMFR0_EL1, PARANGE, 48) ID_AA64MMFR0_EL1_PARANGE_MASK ?

Quoted 5 lines

Yes, but 2 lines before, there is another occurrence of this idiom, and I did not want to deviate from that.
We could update both, or update the other one first in a separate patch, I suppose.

Sure, have your choice, don't have a strong view on either method.

Quoted 24 lines

Quoted 14 lines
Quoted 14 lines
arch/arm64/kernel/pi/idreg-override.c +0 -0
      }
 #endif
      return true;
arch/arm64/kernel/pi/map_kernel.c +0 -0
index f374a3e5a5fe..e57b043f324b 100644
@@ -136,6 +136,12 @@ static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr)
 {
      u64 sctlr = read_sysreg(sctlr_el1);
      u64 tcr = read_sysreg(tcr_el1) | TCR_DS;
Quoted 9 lines
arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 143
Quoted 5 lines
arch/arm64/kernel/pi/map_kernel.c +5 -0
+     u64 mmfr0 = read_sysreg(id_aa64mmfr0_el1);
+     u64 parange = cpuid_feature_extract_unsigned_field(mmfr0,
+                                                        ID_AA64MMFR0_EL1_PARANGE_SHIFT);
+
+     tcr &= ~TCR_IPS_MASK;
Could there be a different IPS value in TCR ? OR is this just a normal clean up instead.

Quoted 1 line

As explained above, TCR.IPS will be capped at 48 bits up to this point.

Alright

Quoted 6 lines

Quoted 5 lines
arch/arm64/kernel/pi/map_kernel.c static void noinline __section(".idmap.text") set_ttbr0_for_lpa2(u64 ttbr) · line 144
Quoted 2 lines
arch/arm64/kernel/pi/map_kernel.c +1 -0
+     tcr |= parange << TCR_IPS_SHIFT;
Wondering if FIELD_PREP() could be used here.

Quoted 6 lines

AIUI we'd end up with
tcr &= ~TCR_IPS_MASK;
tcr |= FIELD_PREP(TCR_IPS_MASK, parange);
Is that really so much better?

Not really, can be left unchanged.

Quoted 19 lines

Quoted 11 lines
Quoted 11 lines
arch/arm64/kernel/pi/map_kernel.c +0 -0
      asm("   msr     sctlr_el1, %0           ;"
          "   isb                             ;"
arch/arm64/mm/init.c +0 -0
index d21f67d67cf5..4db9887b2aef 100644
@@ -280,7 +280,7 @@ void __init arm64_memblock_init(void)
 
Quoted 7 lines
arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 283
Quoted 4 lines
arch/arm64/mm/init.c +1 -1
      if (IS_ENABLED(CONFIG_RANDOMIZE_BASE)) {
              extern u16 memstart_offset_seed;
-             u64 mmfr0 = read_cpuid(ID_AA64MMFR0_EL1);
+             u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
Could this have a comment explaining the need for sanitized value ?

Quoted 6 lines

Sure. Actually, it shouldn't make any difference here (unless we allow PARange to be narrowed even further, which might make sense if we care about enabling randomization of the linear map on systems where PARange is much larger than the size of the physical address space that is actually populated). However, for consistency, it is better to avoid the 52-bit PARange if LPA2 is disabled.

Got it, thanks for the explanation.

Quoted 9 lines

Quoted 7 lines

arch/arm64/mm/init.c void __init arm64_memblock_init(void) · line 286

Quoted 5 lines

arch/arm64/mm/init.c +0 -0

              int parange = cpuid_feature_extract_unsigned_field(
                                      mmfr0, ID_AA64MMFR0_EL1_PARANGE_SHIFT);
              s64 range = linear_region_size -

Otherwise LGTM.

Thanks!

Ard Biesheuvel

author

2024-11-11 08:36 UTC #7 lore raw

PATCH 3/6 arm64/kvm: Configure HYP TCR.PS/DS based on host stage1

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

When the host stage1 is configured for LPA2, the value currently being programmed into TCR_EL2.T0SZ may be invalid unless LPA2 is configured at HYP as well. This means kvm_lpa2_is_enabled() is not the right condition to test when setting TCR_EL2.DS, as it will return false if LPA2 is only available for stage 1 but not for stage 2.

Similary, programming TCR_EL2.PS based on a limited IPA range due to lack of stage2 LPA2 support could potentially result in problems.

So use lpa2_is_enabled() instead, and set the PS field according to the host's IPS, which is capped at 48 bits if LPA2 support is absent or disabled. Whether or not we can make meaningful use of such a configuration is a different question.

Cc <[email protected]>

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

---
 arch/arm64/kvm/arm.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

arch/arm64/kvm/arm.c +4 -4

index a0d01c46e408..1d20d86bb9f5 100644
@@ -2005,8 +2005,7 @@ static int kvm_init_vector_slots(void)
 static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
 {
 	struct kvm_nvhe_init_params *params = per_cpu_ptr_nvhe_sym(kvm_init_params, cpu);
-	u64 mmfr0 = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
-	unsigned long tcr;
+	unsigned long tcr, ips;
 
 	/*
 	 * Calculate the raw per-cpu offset without a translation from the

@@ -2020,6 +2019,7 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
 	params->mair_el2 = read_sysreg(mair_el1);
 
 	tcr = read_sysreg(tcr_el1);
+	ips = FIELD_GET(TCR_IPS_MASK, tcr);
 	if (cpus_have_final_cap(ARM64_KVM_HVHE)) {
 		tcr |= TCR_EPD1_MASK;
 	} else {

@@ -2029,8 +2029,8 @@ static void __init cpu_prepare_hyp_mode(int cpu, u32 hyp_va_bits)
 	tcr &= ~TCR_T0SZ_MASK;
 	tcr |= TCR_T0SZ(hyp_va_bits);
 	tcr &= ~TCR_EL2_PS_MASK;
-	tcr |= FIELD_PREP(TCR_EL2_PS_MASK, kvm_get_parange(mmfr0));
-	if (kvm_lpa2_is_enabled())
+	tcr |= FIELD_PREP(TCR_EL2_PS_MASK, ips);
+	if (lpa2_is_enabled())
 		tcr |= TCR_EL2_DS;
 	params->tcr_el2 = tcr;

Signature · 1 line

2.47.0.277.g8800431eea-goog

Ard Biesheuvel

author

2024-11-11 08:45 UTC #8 lore raw

PATCH 4/6 arm64/kvm: Avoid invalid physical addresses to signal owner updates

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

The pKVM stage2 mapping code relies on an invalid physical address to signal to the internal API that only the owner_id fields of descriptors should be updated, which are stored in the high bits of invalid descriptors covering memory that has been donated to protected guests, and is therefore unmapped from the host stage-2 page tables.

Given that these invalid PAs are never stored into the descriptors, it is better to rely on an explicit flag, to clarify the API and to avoid confusion regarding whether or not the output address of a descriptor can ever be invalid to begin with (which is not the case with LPA2).

That removes a dependency on the logic that reasons about the maximum PA range, which differs on LPA2 capable CPUs based on whether LPA2 is enabled or not, and will be further clarified in subsequent patches.

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

---
 arch/arm64/kvm/hyp/pgtable.c | 37 ++++++++------------
 1 file changed, 14 insertions(+), 23 deletions(-)

arch/arm64/kvm/hyp/pgtable.c +14 ~~-23~~

index b11bcebac908..4bf618b2cba7 100644
@@ -35,14 +35,6 @@ static bool kvm_pgtable_walk_skip_cmo(const struct kvm_pgtable_visit_ctx *ctx)
 	return unlikely(ctx->flags & KVM_PGTABLE_WALK_SKIP_CMO);
 }
 
-static bool kvm_phys_is_valid(u64 phys)
-{
-	u64 parange_max = kvm_get_parange_max();
-	u8 shift = id_aa64mmfr0_parange_to_phys_shift(parange_max);
-
-	return phys < BIT(shift);
-}
-
 static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx, u64 phys)
 {
 	u64 granule = kvm_granule_size(ctx->level);

@@ -53,7 +45,7 @@ static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx,
 	if (granule > (ctx->end - ctx->addr))
 		return false;
 
-	if (kvm_phys_is_valid(phys) && !IS_ALIGNED(phys, granule))
+	if (!IS_ALIGNED(phys, granule))
 		return false;
 
 	return IS_ALIGNED(ctx->addr, granule);

@@ -587,6 +579,9 @@ struct stage2_map_data {
 
 	/* Force mappings to page granularity */
 	bool				force_pte;
+
+	/* Walk should update owner_id only */
+	bool				owner_update;
 };
 
 u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)

@@ -885,18 +880,7 @@ static u64 stage2_map_walker_phys_addr(const struct kvm_pgtable_visit_ctx *ctx,
 {
 	u64 phys = data->phys;
 
-	/*
-	 * Stage-2 walks to update ownership data are communicated to the map
-	 * walker using an invalid PA. Avoid offsetting an already invalid PA,
-	 * which could overflow and make the address valid again.
-	 */
-	if (!kvm_phys_is_valid(phys))
-		return phys;
-
-	/*
-	 * Otherwise, work out the correct PA based on how far the walk has
-	 * gotten.
-	 */
+	/* Work out the correct PA based on how far the walk has gotten */
 	return phys + (ctx->addr - ctx->start);
 }

@@ -908,6 +892,13 @@ static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx,
 	if (data->force_pte && ctx->level < KVM_PGTABLE_LAST_LEVEL)
 		return false;
 
+	/*
+	 * Pass a value that is aligned to any block size when updating
+	 * only the owner_id on invalid mappings.
+	 */
+	if (data->owner_update)
+		phys = 0;
+
 	return kvm_block_mapping_supported(ctx, phys);
 }

@@ -923,7 +914,7 @@ static int stage2_map_walker_try_leaf(const struct kvm_pgtable_visit_ctx *ctx,
 	if (!stage2_leaf_mapping_allowed(ctx, data))
 		return -E2BIG;
 
-	if (kvm_phys_is_valid(phys))
+	if (!data->owner_update)
 		new = kvm_init_valid_leaf_pte(phys, data->attr, ctx->level);
 	else
 		new = kvm_init_invalid_leaf_owner(data->owner_id);

@@ -1085,11 +1076,11 @@ int kvm_pgtable_stage2_set_owner(struct kvm_pgtable *pgt, u64 addr, u64 size,
 {
 	int ret;
 	struct stage2_map_data map_data = {
-		.phys		= KVM_PHYS_INVALID,
 		.mmu		= pgt->mmu,
 		.memcache	= mc,
 		.owner_id	= owner_id,
 		.force_pte	= true,
+		.owner_update	= true,
 	};
 	struct kvm_pgtable_walker walker = {
 		.cb		= stage2_map_walker,

Signature · 1 line

2.47.0.277.g8800431eea-goog

Will Deacon

maintainer

2024-11-11 17:29 UTC #9 lore raw

Re: [PATCH 4/6] arm64/kvm: Avoid invalid physical addresses to signal owner updates

Quoted 81 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>
The pKVM stage2 mapping code relies on an invalid physical address to signal to the internal API that only the owner_id fields of descriptors should be updated, which are stored in the high bits of invalid descriptors covering memory that has been donated to protected guests, and is therefore unmapped from the host stage-2 page tables.
Given that these invalid PAs are never stored into the descriptors, it is better to rely on an explicit flag, to clarify the API and to avoid confusion regarding whether or not the output address of a descriptor can ever be invalid to begin with (which is not the case with LPA2).
That removes a dependency on the logic that reasons about the maximum PA range, which differs on LPA2 capable CPUs based on whether LPA2 is enabled or not, and will be further clarified in subsequent patches.
Signed-off-by Ard Biesheuvel <[email protected]>
---
 arch/arm64/kvm/hyp/pgtable.c | 37 ++++++++------------
 1 file changed, 14 insertions(+), 23 deletions(-)
arch/arm64/kvm/hyp/pgtable.c +5 ~~-21~~
index b11bcebac908..4bf618b2cba7 100644
@@ -35,14 +35,6 @@ static bool kvm_pgtable_walk_skip_cmo(const struct kvm_pgtable_visit_ctx *ctx)
 	return unlikely(ctx->flags & KVM_PGTABLE_WALK_SKIP_CMO);
 }
 
-static bool kvm_phys_is_valid(u64 phys)
-{
-	u64 parange_max = kvm_get_parange_max();
-	u8 shift = id_aa64mmfr0_parange_to_phys_shift(parange_max);
-
-	return phys < BIT(shift);
-}
-
 static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx, u64 phys)
 {
 	u64 granule = kvm_granule_size(ctx->level);
@@ -53,7 +45,7 @@ static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx,
 	if (granule > (ctx->end - ctx->addr))
 		return false;
 
-	if (kvm_phys_is_valid(phys) && !IS_ALIGNED(phys, granule))
+	if (!IS_ALIGNED(phys, granule))
 		return false;
 
 	return IS_ALIGNED(ctx->addr, granule);
@@ -587,6 +579,9 @@ struct stage2_map_data {
 
 	/* Force mappings to page granularity */
 	bool				force_pte;
+
+	/* Walk should update owner_id only */
+	bool				owner_update;
 };
 
 u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)
@@ -885,18 +880,7 @@ static u64 stage2_map_walker_phys_addr(const struct kvm_pgtable_visit_ctx *ctx,
 {
 	u64 phys = data->phys;
 
-	/*
-	 * Stage-2 walks to update ownership data are communicated to the map
-	 * walker using an invalid PA. Avoid offsetting an already invalid PA,
-	 * which could overflow and make the address valid again.
-	 */
-	if (!kvm_phys_is_valid(phys))
-		return phys;
-
-	/*
-	 * Otherwise, work out the correct PA based on how far the walk has
-	 * gotten.
-	 */
+	/* Work out the correct PA based on how far the walk has gotten */
 	return phys + (ctx->addr - ctx->start);
 }
 

arch/arm64/kvm/hyp/pgtable.c static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx, · line 901

Quoted 10 lines

arch/arm64/kvm/hyp/pgtable.c +7 -0

@@ -908,6 +892,13 @@ static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx,
 	if (data->force_pte && ctx->level < KVM_PGTABLE_LAST_LEVEL)
 		return false;
 
+	/*
+	 * Pass a value that is aligned to any block size when updating
+	 * only the owner_id on invalid mappings.
+	 */
+	if (data->owner_update)
+		phys = 0;
+

Hmm, isn't this a change in behaviour? Previously we would always store the owner annotations at the leaf, but now this will take place at a higher-level. I think that probably goes horribly wrong if we later try to change the owner for a sub-range; the block will be replaced with a table and the old ownership information will be lost rather than propagated to the leaves.

In other words, I think we should return false here.

Will

Ard Biesheuvel

2024-11-11 18:12 UTC #10 lore raw

Re: [PATCH 4/6] arm64/kvm: Avoid invalid physical addresses to signal owner updates

Quoted 86 lines · Will Deacon

Quoted 84 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

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

---
 arch/arm64/kvm/hyp/pgtable.c | 37 ++++++++------------
 1 file changed, 14 insertions(+), 23 deletions(-)

arch/arm64/kvm/hyp/pgtable.c +5 ~~-21~~

index b11bcebac908..4bf618b2cba7 100644
@@ -35,14 +35,6 @@ static bool kvm_pgtable_walk_skip_cmo(const struct kvm_pgtable_visit_ctx *ctx)
      return unlikely(ctx->flags & KVM_PGTABLE_WALK_SKIP_CMO);
 }
 
-static bool kvm_phys_is_valid(u64 phys)
-{
-     u64 parange_max = kvm_get_parange_max();
-     u8 shift = id_aa64mmfr0_parange_to_phys_shift(parange_max);
-
-     return phys < BIT(shift);
-}
-
 static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx, u64 phys)
 {
      u64 granule = kvm_granule_size(ctx->level);

@@ -53,7 +45,7 @@ static bool kvm_block_mapping_supported(const struct kvm_pgtable_visit_ctx *ctx,
      if (granule > (ctx->end - ctx->addr))
              return false;
 
-     if (kvm_phys_is_valid(phys) && !IS_ALIGNED(phys, granule))
+     if (!IS_ALIGNED(phys, granule))
              return false;
 
      return IS_ALIGNED(ctx->addr, granule);

@@ -587,6 +579,9 @@ struct stage2_map_data {
 
      /* Force mappings to page granularity */
      bool                            force_pte;
+
+     /* Walk should update owner_id only */
+     bool                            owner_update;
 };
 
 u64 kvm_get_vtcr(u64 mmfr0, u64 mmfr1, u32 phys_shift)

@@ -885,18 +880,7 @@ static u64 stage2_map_walker_phys_addr(const struct kvm_pgtable_visit_ctx *ctx,
 {
      u64 phys = data->phys;
 
-     /*
-      * Stage-2 walks to update ownership data are communicated to the map
-      * walker using an invalid PA. Avoid offsetting an already invalid PA,
-      * which could overflow and make the address valid again.
-      */
-     if (!kvm_phys_is_valid(phys))
-             return phys;
-
-     /*
-      * Otherwise, work out the correct PA based on how far the walk has
-      * gotten.
-      */
+     /* Work out the correct PA based on how far the walk has gotten */
      return phys + (ctx->addr - ctx->start);
 }

@@ -908,6 +892,13 @@ static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx,
      if (data->force_pte && ctx->level < KVM_PGTABLE_LAST_LEVEL)
              return false;

Quoted 15 lines

arch/arm64/kvm/hyp/pgtable.c static bool stage2_leaf_mapping_allowed(const struct kvm_pgtable_visit_ctx *ctx, · line 901
Quoted 7 lines
arch/arm64/kvm/hyp/pgtable.c +7 -0
+     /*
+      * Pass a value that is aligned to any block size when updating
+      * only the owner_id on invalid mappings.
+      */
+     if (data->owner_update)
+             phys = 0;
+
Hmm, isn't this a change in behaviour? Previously we would always store the owner annotations at the leaf, but now this will take place at a higher-level. I think that probably goes horribly wrong if we later try to change the owner for a sub-range; the block will be replaced with a table and the old ownership information will be lost rather than propagated to the leaves.

The data->force_pte check preceding this will take care of that, afaict.

Ard Biesheuvel

author

2024-11-11 08:47 UTC #11 lore raw

PATCH 5/6 arm64: Kconfig: force ARM64_PAN=y when enabling TTBR0 sw PAN

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

We have a couple of instances of Kconfig constraints where PAN must be enabled too if TTBR0 sw PAN is enabled, primarily to avoid dealing with the modified TTBR0_EL1 sysreg format that is used when 52-bit physical addressing and/or CnP are enabled (support for either implies support for hardware PAN as well, which will supersede PAN emulation if both are available)

Let's simplify this, and always enable ARM64_PAN when enabling TTBR0 sw PAN. This decouples the PAN configuration from the VA size selection, permitting us to simplify the latter in subsequent patches. (Note that PAN and TTBR0 sw PAN can still be disabled after this patch, but not independently)

To avoid a convoluted circular Kconfig dependency involving KCSAN, make ARM64_MTE select ARM64_PAN too, instead of depending on it.

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

---
 arch/arm64/Kconfig | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

arch/arm64/Kconfig +2 -4

index 232dcade2783..ec218ef22f2b 100644
@@ -1368,7 +1368,6 @@ config ARM64_VA_BITS_48
 
 config ARM64_VA_BITS_52
 	bool "52-bit"
-	depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN
 	help
 	  Enable 52-bit virtual addressing for userspace when explicitly
 	  requested via a hint to mmap(). The kernel will also use 52-bit

@@ -1420,7 +1419,6 @@ config ARM64_PA_BITS_48
 config ARM64_PA_BITS_52
 	bool "52-bit"
 	depends on ARM64_64K_PAGES || ARM64_VA_BITS_52
-	depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN
 	help
 	  Enable support for a 52-bit physical address space, introduced as
 	  part of the ARMv8.2-LPA extension.

@@ -1667,6 +1665,7 @@ config RODATA_FULL_DEFAULT_ENABLED
 config ARM64_SW_TTBR0_PAN
 	bool "Emulate Privileged Access Never using TTBR0_EL1 switching"
 	depends on !KCSAN
+	select ARM64_PAN
 	help
 	  Enabling this option prevents the kernel from accessing
 	  user-space memory directly by pointing TTBR0_EL1 to a reserved

@@ -1923,7 +1922,6 @@ config ARM64_RAS_EXTN
 config ARM64_CNP
 	bool "Enable support for Common Not Private (CNP) translations"
 	default y
-	depends on ARM64_PAN || !ARM64_SW_TTBR0_PAN
 	help
 	  Common Not Private (CNP) allows translation table entries to
 	  be shared between different PEs in the same inner shareable

@@ -2118,7 +2116,7 @@ config ARM64_MTE
 	depends on AS_HAS_ARMV8_5
 	depends on AS_HAS_LSE_ATOMICS
 	# Required for tag checking in the uaccess routines
-	depends on ARM64_PAN
+	select ARM64_PAN
 	select ARCH_HAS_SUBPAGE_FAULTS
 	select ARCH_USES_HIGH_VMA_FLAGS
 	select ARCH_USES_PG_ARCH_2

Signature · 1 line

2.47.0.277.g8800431eea-goog

Ard Biesheuvel

author

2024-11-11 08:49 UTC #12 lore raw

PATCH 6/6 arm64/mm: Drop configurable 48-bit physical address space limit

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

Currently, the maximum supported physical address space can be configured as either 48 bits or 52 bits. The only remaining difference between these in practice is that the former omits the masking and shifting required to construct TTBR and PTE values, which carry bits #48 and higher disjoint from the rest of the physical address.

The overhead of performing these additional calculations is negligible, and so there is little reason to retain support for two different configurations, and we can simply support whatever the hardware supports.

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

---
 arch/arm64/Kconfig                     | 31 +-------------------
 arch/arm64/include/asm/assembler.h     | 13 ++------
 arch/arm64/include/asm/cpufeature.h    |  3 +-
 arch/arm64/include/asm/kvm_pgtable.h   |  3 +-
 arch/arm64/include/asm/pgtable-hwdef.h |  6 +---
 arch/arm64/include/asm/pgtable-prot.h  |  4 +--
 arch/arm64/include/asm/pgtable.h       | 11 +------
 arch/arm64/include/asm/sysreg.h        |  6 ----
 arch/arm64/mm/pgd.c                    |  9 +++---
 arch/arm64/mm/proc.S                   |  2 --
 scripts/gdb/linux/constants.py.in      |  1 -
 tools/arch/arm64/include/asm/sysreg.h  |  6 ----
 12 files changed, 14 insertions(+), 81 deletions(-)

arch/arm64/Kconfig +1 ~~-30~~

index ec218ef22f2b..769af2da4db5 100644
@@ -1405,38 +1405,9 @@ config ARM64_VA_BITS
 	default 48 if ARM64_VA_BITS_48
 	default 52 if ARM64_VA_BITS_52
 
-choice
-	prompt "Physical address space size"
-	default ARM64_PA_BITS_48
-	help
-	  Choose the maximum physical address range that the kernel will
-	  support.
-
-config ARM64_PA_BITS_48
-	bool "48-bit"
-	depends on ARM64_64K_PAGES || !ARM64_VA_BITS_52
-
-config ARM64_PA_BITS_52
-	bool "52-bit"
-	depends on ARM64_64K_PAGES || ARM64_VA_BITS_52
-	help
-	  Enable support for a 52-bit physical address space, introduced as
-	  part of the ARMv8.2-LPA extension.
-
-	  With this enabled, the kernel will also continue to work on CPUs that
-	  do not support ARMv8.2-LPA, but with some added memory overhead (and
-	  minor performance overhead).
-
-endchoice
-
-config ARM64_PA_BITS
-	int
-	default 48 if ARM64_PA_BITS_48
-	default 52 if ARM64_PA_BITS_52
-
 config ARM64_LPA2
 	def_bool y
-	depends on ARM64_PA_BITS_52 && !ARM64_64K_PAGES
+	depends on !ARM64_64K_PAGES
 
 choice
 	prompt "Endianness"

arch/arm64/include/asm/assembler.h +2 ~~-11~~

index ad63457a05c5..01a1e3c16283 100644
@@ -342,14 +342,13 @@ alternative_cb_end
 	mrs	\tmp0, ID_AA64MMFR0_EL1
 	// Narrow PARange to fit the PS field in TCR_ELx
 	ubfx	\tmp0, \tmp0, #ID_AA64MMFR0_EL1_PARANGE_SHIFT, #3
-	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_MAX
 #ifdef CONFIG_ARM64_LPA2
 alternative_if_not ARM64_HAS_VA52
 	mov	\tmp1, #ID_AA64MMFR0_EL1_PARANGE_48
-alternative_else_nop_endif
-#endif
 	cmp	\tmp0, \tmp1
 	csel	\tmp0, \tmp1, \tmp0, hi
+alternative_else_nop_endif
+#endif
 	bfi	\tcr, \tmp0, \pos, #3
 	.endm

@@ -599,21 +598,13 @@ alternative_endif
  * 	ttbr:	returns the TTBR value
  */
 	.macro	phys_to_ttbr, ttbr, phys
-#ifdef CONFIG_ARM64_PA_BITS_52
 	orr	\ttbr, \phys, \phys, lsr #46
 	and	\ttbr, \ttbr, #TTBR_BADDR_MASK_52
-#else
-	mov	\ttbr, \phys
-#endif
 	.endm
 
 	.macro	phys_to_pte, pte, phys
-#ifdef CONFIG_ARM64_PA_BITS_52
 	orr	\pte, \phys, \phys, lsr #PTE_ADDR_HIGH_SHIFT
 	and	\pte, \pte, #PHYS_TO_PTE_ADDR_MASK
-#else
-	mov	\pte, \phys
-#endif
 	.endm
 
 /*

arch/arm64/include/asm/cpufeature.h +1 -2

index e889e12b7648..3852ef120484 100644
@@ -862,9 +862,8 @@ static inline u32 id_aa64mmfr0_parange_to_phys_shift(int parange)
 	 * However, by the "D10.1.4 Principles of the ID scheme
 	 * for fields in ID registers", ARM DDI 0487C.a, any new
 	 * value is guaranteed to be higher than what we know already.
-	 * As a safe limit, we return the limit supported by the kernel.
 	 */
-	default: return CONFIG_ARM64_PA_BITS;
+	default: return 52;
 	}
 }

arch/arm64/include/asm/kvm_pgtable.h +1 -2

index 03f4c3d7839c..c98643542597 100644
@@ -30,8 +30,7 @@
 
 static inline u64 kvm_get_parange_max(void)
 {
-	if (kvm_lpa2_is_enabled() ||
-	   (IS_ENABLED(CONFIG_ARM64_PA_BITS_52) && PAGE_SHIFT == 16))
+	if (kvm_lpa2_is_enabled() || PAGE_SHIFT == 16)
 		return ID_AA64MMFR0_EL1_PARANGE_52;
 	else
 		return ID_AA64MMFR0_EL1_PARANGE_48;

arch/arm64/include/asm/pgtable-hwdef.h +1 -5

index a970def932aa..7ad52dc4c16e 100644
@@ -172,7 +172,6 @@
 #define PTE_SWBITS_MASK		_AT(pteval_t, (BIT(63) | GENMASK(58, 55)))
 
 #define PTE_ADDR_LOW		(((_AT(pteval_t, 1) << (50 - PAGE_SHIFT)) - 1) << PAGE_SHIFT)
-#ifdef CONFIG_ARM64_PA_BITS_52
 #ifdef CONFIG_ARM64_64K_PAGES
 #define PTE_ADDR_HIGH		(_AT(pteval_t, 0xf) << 12)
 #define PTE_ADDR_HIGH_SHIFT	36

@@ -182,7 +181,6 @@
 #define PTE_ADDR_HIGH_SHIFT	42
 #define PHYS_TO_PTE_ADDR_MASK	GENMASK_ULL(49, 8)
 #endif
-#endif
 
 /*
  * AttrIndx[2:0] encoding (mapping attributes defined in the MAIR* registers).

@@ -323,12 +321,10 @@
 /*
  * TTBR.
  */
-#ifdef CONFIG_ARM64_PA_BITS_52
 /*
- * TTBR_ELx[1] is RES0 in this configuration.
+ * TTBR_ELx[1] is RES0 when using 52-bit physical addressing
  */
 #define TTBR_BADDR_MASK_52	GENMASK_ULL(47, 2)
-#endif
 
 #ifdef CONFIG_ARM64_VA_BITS_52
 /* Must be at least 64-byte aligned to prevent corruption of the TTBR */

arch/arm64/include/asm/pgtable-prot.h +2 -2

index a95f1f77bb39..b73acf25341f 100644
@@ -81,7 +81,7 @@ extern unsigned long prot_ns_shared;
 #define lpa2_is_enabled()	false
 #define PTE_MAYBE_SHARED	PTE_SHARED
 #define PMD_MAYBE_SHARED	PMD_SECT_S
-#define PHYS_MASK_SHIFT		(CONFIG_ARM64_PA_BITS)
+#define PHYS_MASK_SHIFT		(52)
 #else
 static inline bool __pure lpa2_is_enabled(void)
 {

@@ -90,7 +90,7 @@ static inline bool __pure lpa2_is_enabled(void)
 
 #define PTE_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PTE_SHARED)
 #define PMD_MAYBE_SHARED	(lpa2_is_enabled() ? 0 : PMD_SECT_S)
-#define PHYS_MASK_SHIFT		(lpa2_is_enabled() ? CONFIG_ARM64_PA_BITS : 48)
+#define PHYS_MASK_SHIFT		(lpa2_is_enabled() ? 52 : 48)
 #endif
 
 /*

arch/arm64/include/asm/pgtable.h +1 ~~-10~~

index dd5dcf7ae056..dd4f1ad9ffca 100644
@@ -69,10 +69,9 @@ extern unsigned long empty_zero_page[PAGE_SIZE / sizeof(unsigned long)];
 	pr_err("%s:%d: bad pte %016llx.\n", __FILE__, __LINE__, pte_val(e))
 
 /*
- * Macros to convert between a physical address and its placement in a
+ * Helpers to convert between a physical address and its placement in a
  * page table entry, taking care of 52-bit addresses.
  */
-#ifdef CONFIG_ARM64_PA_BITS_52
 static inline phys_addr_t __pte_to_phys(pte_t pte)
 {
 	pte_val(pte) &= ~PTE_MAYBE_SHARED;

@@ -83,10 +82,6 @@ static inline pteval_t __phys_to_pte_val(phys_addr_t phys)
 {
 	return (phys | (phys >> PTE_ADDR_HIGH_SHIFT)) & PHYS_TO_PTE_ADDR_MASK;
 }
-#else
-#define __pte_to_phys(pte)	(pte_val(pte) & PTE_ADDR_LOW)
-#define __phys_to_pte_val(phys)	(phys)
-#endif
 
 #define pte_pfn(pte)		(__pte_to_phys(pte) >> PAGE_SHIFT)
 #define pfn_pte(pfn,prot)	\

@@ -1493,11 +1488,7 @@ static inline void update_mmu_cache_range(struct vm_fault *vmf,
 	update_mmu_cache_range(NULL, vma, addr, ptep, 1)
 #define update_mmu_cache_pmd(vma, address, pmd) do { } while (0)
 
-#ifdef CONFIG_ARM64_PA_BITS_52
 #define phys_to_ttbr(addr)	(((addr) | ((addr) >> 46)) & TTBR_BADDR_MASK_52)
-#else
-#define phys_to_ttbr(addr)	(addr)
-#endif
 
 /*
  * On arm64 without hardware Access Flag, copying from user will fail because

arch/arm64/include/asm/sysreg.h +0 -6

index 9c98ff448bd9..11af40e501ae 100644
@@ -928,12 +928,6 @@
 #define ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_LPA2		0x3
 #define ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_MAX		0x7
 
-#ifdef CONFIG_ARM64_PA_BITS_52
-#define ID_AA64MMFR0_EL1_PARANGE_MAX	ID_AA64MMFR0_EL1_PARANGE_52
-#else
-#define ID_AA64MMFR0_EL1_PARANGE_MAX	ID_AA64MMFR0_EL1_PARANGE_48
-#endif
-
 #if defined(CONFIG_ARM64_4K_PAGES)
 #define ID_AA64MMFR0_EL1_TGRAN_SHIFT		ID_AA64MMFR0_EL1_TGRAN4_SHIFT
 #define ID_AA64MMFR0_EL1_TGRAN_LPA2		ID_AA64MMFR0_EL1_TGRAN4_52_BIT

arch/arm64/mm/pgd.c +5 -4

index 0c501cabc238..8722ab6d4b1c 100644
@@ -48,20 +48,21 @@ void pgd_free(struct mm_struct *mm, pgd_t *pgd)
 
 void __init pgtable_cache_init(void)
 {
+	unsigned int pgd_size = PGD_SIZE;
+
 	if (pgdir_is_page_size())
 		return;
 
-#ifdef CONFIG_ARM64_PA_BITS_52
 	/*
 	 * With 52-bit physical addresses, the architecture requires the
 	 * top-level table to be aligned to at least 64 bytes.
 	 */
-	BUILD_BUG_ON(PGD_SIZE < 64);
-#endif
+	if (PHYS_MASK_SHIFT >= 52)
+		pgd_size = max(pgd_size, 64);
 
 	/*
 	 * Naturally aligned pgds required by the architecture.
 	 */
-	pgd_cache = kmem_cache_create("pgd_cache", PGD_SIZE, PGD_SIZE,
+	pgd_cache = kmem_cache_create("pgd_cache", pgd_size, pgd_size,
 				      SLAB_PANIC, NULL);
 }

arch/arm64/mm/proc.S +0 -2

index 8abdc7fed321..84657d04dc51 100644
@@ -197,10 +197,8 @@ SYM_FUNC_ALIAS(__pi_idmap_cpu_replace_ttbr1, idmap_cpu_replace_ttbr1)
 
 	.macro	pte_to_phys, phys, pte
 	and	\phys, \pte, #PTE_ADDR_LOW
-#ifdef CONFIG_ARM64_PA_BITS_52
 	and	\pte, \pte, #PTE_ADDR_HIGH
 	orr	\phys, \phys, \pte, lsl #PTE_ADDR_HIGH_SHIFT
-#endif
 	.endm
 
 	.macro	kpti_mk_tbl_ng, type, num_entries

scripts/gdb/linux/constants.py.in +0 -1

index fd6bd69c5096..05034c0b8fd7 100644
@@ -141,7 +141,6 @@ LX_CONFIG(CONFIG_ARM64_4K_PAGES)
 LX_CONFIG(CONFIG_ARM64_16K_PAGES)
 LX_CONFIG(CONFIG_ARM64_64K_PAGES)
 if IS_BUILTIN(CONFIG_ARM64):
-    LX_VALUE(CONFIG_ARM64_PA_BITS)
     LX_VALUE(CONFIG_ARM64_VA_BITS)
     LX_VALUE(CONFIG_PAGE_SHIFT)
     LX_VALUE(CONFIG_ARCH_FORCE_MAX_ORDER)

tools/arch/arm64/include/asm/sysreg.h +0 -6

index cd8420e8c3ad..daeecb1a5366 100644
@@ -574,12 +574,6 @@
 #define ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_MIN		0x2
 #define ID_AA64MMFR0_EL1_TGRAN_2_SUPPORTED_MAX		0x7
 
-#ifdef CONFIG_ARM64_PA_BITS_52
-#define ID_AA64MMFR0_EL1_PARANGE_MAX	ID_AA64MMFR0_EL1_PARANGE_52
-#else
-#define ID_AA64MMFR0_EL1_PARANGE_MAX	ID_AA64MMFR0_EL1_PARANGE_48
-#endif
-
 #if defined(CONFIG_ARM64_4K_PAGES)
 #define ID_AA64MMFR0_EL1_TGRAN_SHIFT		ID_AA64MMFR0_EL1_TGRAN4_SHIFT
 #define ID_AA64MMFR0_EL1_TGRAN_SUPPORTED_MIN	ID_AA64MMFR0_EL1_TGRAN4_SUPPORTED_MIN

Signature · 1 line

2.47.0.277.g8800431eea-goog

Ard Biesheuvel

2024-11-11 08:56 UTC #13 lore raw

In reply to Ard Biesheuvel

(cc Quentin)

Quoted 58 lines · Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>
This series addresses a number of buglets related to how we handle the size of the physical address space when building LPA2 capable kernels:
- reject 52-bit physical addressess in the mapping routines when LPA2 is configured but not available at runtime
- ensure that TCR.IPS is not set to 52-bits if LPA2 is not supported
- ensure that TCR_EL2.PS and DS match the host, regardless of whether LPA2 is available at stage 2
- don't rely on kvm_get_parange() and invalid physical addresses as control flags in the pKVM page donation APIs
Finally, the configurable 48-bit physical address space limit is dropped entirely, as it doesn't buy us a lot now that all the PARange and {I}PS handling is done at runtime.
Cc Catalin Marinas <[email protected]>
Cc Will Deacon <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Ryan Roberts <[email protected]>
Cc Anshuman Khandual <[email protected]>
Cc Kees Cook <[email protected]>
Ard Biesheuvel (6):
arm64/mm: Reduce PA space to 48 bits when LPA2 is not enabled arm64/mm: Override PARange for !LPA2 and use it consistently arm64/kvm: Configure HYP TCR.PS/DS based on host stage1 arm64/kvm: Avoid invalid physical addresses to signal owner updates
arm64: Kconfig: force ARM64_PAN=y when enabling TTBR0 sw PAN arm64/mm: Drop configurable 48-bit physical address space limit
 arch/arm64/Kconfig                     | 37 ++------------------
 arch/arm64/include/asm/assembler.h     | 14 +++-----
 arch/arm64/include/asm/cpufeature.h    |  3 +-
 arch/arm64/include/asm/kvm_pgtable.h   |  3 +-
 arch/arm64/include/asm/pgtable-hwdef.h | 12 +------
 arch/arm64/include/asm/pgtable-prot.h  |  7 ++++
 arch/arm64/include/asm/pgtable.h       | 11 +-----
 arch/arm64/include/asm/sparsemem.h     |  4 ++-
 arch/arm64/include/asm/sysreg.h        |  6 ----
 arch/arm64/kernel/cpufeature.c         |  2 +-
 arch/arm64/kernel/pi/idreg-override.c  |  9 +++++
 arch/arm64/kernel/pi/map_kernel.c      |  6 ++++
 arch/arm64/kvm/arm.c                   |  8 ++---
 arch/arm64/kvm/hyp/pgtable.c           | 37 ++++++++------------
 arch/arm64/mm/init.c                   |  2 +-
 arch/arm64/mm/pgd.c                    |  9 ++---
 arch/arm64/mm/proc.S                   |  2 --
 scripts/gdb/linux/constants.py.in      |  1 -
 tools/arch/arm64/include/asm/sysreg.h  |  6 ----
 19 files changed, 62 insertions(+), 117 deletions(-)
--
2.47.0.277.g8800431eea-goog