[PATCH v7 0/4] Initial BBML2 support for contpte_convert()

[PATCH v7 0/4] Initial BBML2 support for contpte_convert()

[Mikołaj Lenczewski]

Hi All,

This patch series extends the cpufeature framework to add support for
easily matching against all early cpus, and builds on this to add initial
support for eliding Break-Before-Make requirements on systems that support
BBML2 and additionally guarantee to never raise a conflict abort.

This support reorders and elides both a TLB invalidation and a DSB in
contpte_convert(), when BBML2 is supported. This leads to a 12%
improvement when executing a microbenchmark designed to force the
pathological path where contpte_convert() gets called. This represents
an 80% reduction in the cost of calling contpte_convert().

We clarify both the correctness and performance benefits of this elision
with respect to the relevant Arm ARM passages, via substantial comments
in the contpte_convert() source.

This series is based on v6.16-rc2 (e04c78d86a96).

Notes
======

Patch 1 extends the cpufeature framework machinery as discussed in [1],
to allows checking for the presence of a feature on all early boot cpus
(and on all late cpus when they are brought online).

Patch 2 implements an allow-list of cpus that support BBML2, but with
the additional constraint of never causing TLB conflict aborts. We
settled on this constraint because we will use the feature for kernel
mappings in the future, for which we cannot handle conflict aborts
safely.

Patch 2 implements a MIDR check but does not implement a AA64_ID_MMFR2
check, as the BBML2_NOABORT MIDR check is a strict superset of the feature
register check (all platforms that pass the MIDR check will also pass the
feature register check).

Yang Shi has a series at [2] that aims to use BBML2 to enable splitting
the linear map at runtime. This series partially overlaps with it to add
the cpu feature. We believe this series is fully compatible with Yang's
requirements and could go first.

[1]:
  https://lore.kernel.org/all/aCSHESk1DzShD4vt@arm.com/

[2]:
  https://lore.kernel.org/linux-arm-kernel/20250304222018.615808-1-yang@os.amperecomputing.com/

Changelog
=========

v7:
  - fix up some minor spelling and formatting nits
  - integrate cpufeature framework patch to each bbml2 detection
  - avoid making feature register check, rely on MIDR check
  - rebase onto v6.16-rc2

v6:
  - https://lore.kernel.org/all/20250428153514.55772-2-miko.lenczewski@arm.com/
  - clarify correctness and performance of elision of __tlb_flush_range()
  - rebase onto v6.15-rc3

v5:
  - https://lore.kernel.org/all/20250325093625.55184-1-miko.lenczewski@arm.com/
  - fixup coding style nits
  - document motivation for kernel commandline parameter

v4:
  - https://lore.kernel.org/all/20250319150533.37440-2-miko.lenczewski@arm.com/
  - rebase onto v6.14-rc5
  - switch from arm64 sw feature override to hw feature override
  - reintroduce has_cpuid_feature() check in addition to MIDR check

v3:
  - https://lore.kernel.org/all/20250313104111.24196-2-miko.lenczewski@arm.com/
  - rebase onto v6.14-rc4
  - add arm64.nobbml2 commandline override
  - squash "delay tlbi" and "elide tlbi" patches

v2:
  - https://lore.kernel.org/all/20250228182403.6269-2-miko.lenczewski@arm.com/
  - fix buggy MIDR check to properly account for all boot+late cpus
  - add smmu bbml2 feature check

v1:
  - https://lore.kernel.org/all/20250219143837.44277-3-miko.lenczewski@arm.com/
  - rebase onto v6.14-rc3
  - remove kvm bugfix patches from series
  - strip out conflict abort handler code
  - switch from blocklist to allowlist of bmml2+noabort implementations
  - remove has_cpuid_feature() in favour of MIDR check

rfc-v1:
  - https://lore.kernel.org/all/20241211154611.40395-1-miko.lenczewski@arm.com/
  - https://lore.kernel.org/all/20241211160218.41404-1-miko.lenczewski@arm.com/

Catalin Marinas (1):
  arm64: cpufeature: Introduce MATCH_ALL_EARLY_CPUS capability type

Mikołaj Lenczewski (3):
  arm64: Add BBM Level 2 cpu feature
  iommu/arm: Add BBM Level 2 smmu feature
  arm64/mm: Elide tlbi in contpte_convert() under BBML2

 arch/arm64/include/asm/cpufeature.h           |  28 ++++
 arch/arm64/kernel/cpufeature.c                | 100 +++++++++++--
 arch/arm64/mm/contpte.c                       | 139 +++++++++++++++++-
 arch/arm64/tools/cpucaps                      |   1 +
 .../iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c   |   3 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |   3 +
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |   2 +
 7 files changed, 264 insertions(+), 12 deletions(-)

-- 
2.49.0

[PATCH v7 1/4] arm64: cpufeature: Introduce MATCH_ALL_EARLY_CPUS capability type

[Mikołaj Lenczewski]

From: Catalin Marinas <catalin.marinas@arm.com>

For system-wide capabilities, the kernel has the SCOPE_SYSTEM type. Such
capabilities are checked once the SMP boot has completed using the
sanitised ID registers. However, there is a need for a new capability
type similar in scope to the system one but with checking performed
locally on each CPU during boot (e.g. based on MIDR_EL1 which is not a
sanitised register).

Introduce ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS which, together with
ARM64_CPUCAP_SCOPE_LOCAL_CPU, ensures that such capability is enabled
only if all early CPUs have it. For ease of use, define
ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE which combines SCOPE_LOCAL_CPU,
PERMITTED_FOR_LATE_CPUS and MATCH_ALL_EARLY_CPUS.

Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
---
 arch/arm64/include/asm/cpufeature.h | 23 +++++++++++
 arch/arm64/kernel/cpufeature.c      | 60 +++++++++++++++++++++++------
 2 files changed, 72 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index c4326f1cb917..155ebd040c55 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -275,6 +275,14 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
 #define ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU	((u16)BIT(5))
 /* Panic when a conflict is detected */
 #define ARM64_CPUCAP_PANIC_ON_CONFLICT		((u16)BIT(6))
+/*
+ * When paired with SCOPE_LOCAL_CPU, all early CPUs must satisfy the
+ * condition. This is different from SCOPE_SYSTEM where the check is performed
+ * only once at the end of the SMP boot on the sanitised ID registers.
+ * SCOPE_SYSTEM is not suitable for cases where the capability depends on
+ * properties local to a CPU like MIDR_EL1.
+ */
+#define ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS	((u16)BIT(7))
 
 /*
  * CPU errata workarounds that need to be enabled at boot time if one or
@@ -304,6 +312,16 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
 	(ARM64_CPUCAP_SCOPE_LOCAL_CPU		|	\
 	 ARM64_CPUCAP_OPTIONAL_FOR_LATE_CPU	|	\
 	 ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU)
+/*
+ * CPU feature detected at boot time and present on all early CPUs. Late CPUs
+ * are permitted to have the feature even if it hasn't been enabled, although
+ * the feature will not be used by Linux in this case. If all early CPUs have
+ * the feature, then every late CPU must have it.
+ */
+#define ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE		\
+	 (ARM64_CPUCAP_SCOPE_LOCAL_CPU		|	\
+	  ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU	|	\
+	  ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS)
 
 /*
  * CPU feature detected at boot time, on one or more CPUs. A late CPU
@@ -391,6 +409,11 @@ static inline int cpucap_default_scope(const struct arm64_cpu_capabilities *cap)
 	return cap->type & ARM64_CPUCAP_SCOPE_MASK;
 }
 
+static inline bool cpucap_match_all_early_cpus(const struct arm64_cpu_capabilities *cap)
+{
+	return cap->type & ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS;
+}
+
 /*
  * Generic helper for handling capabilities with multiple (match,enable) pairs
  * of call backs, sharing the same capability bit.
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index b34044e20128..f9c947166322 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -3370,18 +3370,49 @@ static void update_cpu_capabilities(u16 scope_mask)
 
 	scope_mask &= ARM64_CPUCAP_SCOPE_MASK;
 	for (i = 0; i < ARM64_NCAPS; i++) {
+		bool match_all = false;
+		bool caps_set = false;
+		bool boot_cpu = false;
+
 		caps = cpucap_ptrs[i];
-		if (!caps || !(caps->type & scope_mask) ||
-		    cpus_have_cap(caps->capability) ||
-		    !caps->matches(caps, cpucap_default_scope(caps)))
+		if (!caps || !(caps->type & scope_mask))
 			continue;
 
-		if (caps->desc && !caps->cpus)
+		match_all = cpucap_match_all_early_cpus(caps);
+		caps_set = cpus_have_cap(caps->capability);
+		boot_cpu = scope_mask & SCOPE_BOOT_CPU;
+
+		/*
+		 * Unless it's a match-all CPUs feature, avoid probing if
+		 * already detected.
+		 */
+		if (!match_all && caps_set)
+			continue;
+
+		/*
+		 * A match-all CPUs capability is only set when probing the
+		 * boot CPU. It may be cleared subsequently if not detected on
+		 * secondary ones.
+		 */
+		if (match_all && !caps_set && !boot_cpu)
+			continue;
+
+		if (!caps->matches(caps, cpucap_default_scope(caps))) {
+			if (match_all)
+				__clear_bit(caps->capability, system_cpucaps);
+			continue;
+		}
+
+		/*
+		 * Match-all CPUs capabilities are logged later when the
+		 * system capabilities are finalised.
+		 */
+		if (!match_all && caps->desc && !caps->cpus)
 			pr_info("detected: %s\n", caps->desc);
 
 		__set_bit(caps->capability, system_cpucaps);
 
-		if ((scope_mask & SCOPE_BOOT_CPU) && (caps->type & SCOPE_BOOT_CPU))
+		if (boot_cpu && (caps->type & SCOPE_BOOT_CPU))
 			set_bit(caps->capability, boot_cpucaps);
 	}
 }
@@ -3782,17 +3813,24 @@ static void __init setup_system_capabilities(void)
 	enable_cpu_capabilities(SCOPE_ALL & ~SCOPE_BOOT_CPU);
 	apply_alternatives_all();
 
-	/*
-	 * Log any cpucaps with a cpumask as these aren't logged by
-	 * update_cpu_capabilities().
-	 */
 	for (int i = 0; i < ARM64_NCAPS; i++) {
 		const struct arm64_cpu_capabilities *caps = cpucap_ptrs[i];
 
-		if (caps && caps->cpus && caps->desc &&
-			cpumask_any(caps->cpus) < nr_cpu_ids)
+		if (!caps || !caps->desc)
+			continue;
+
+		/*
+		 * Log any cpucaps with a cpumask as these aren't logged by
+		 * update_cpu_capabilities().
+		 */
+		if (caps->cpus && cpumask_any(caps->cpus) < nr_cpu_ids)
 			pr_info("detected: %s on CPU%*pbl\n",
 				caps->desc, cpumask_pr_args(caps->cpus));
+
+		/* Log match-all CPUs capabilities */
+		if (cpucap_match_all_early_cpus(caps) &&
+		    cpus_have_cap(caps->capability))
+			pr_info("detected: %s\n", caps->desc);
 	}
 
 	/*
-- 
2.49.0

[PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Mikołaj Lenczewski]

The Break-Before-Make cpu feature supports multiple levels (levels 0-2),
and this commit adds a dedicated BBML2 cpufeature to test against
support for.

To support BBML2 in as wide a range of contexts as we can, we want not
only the architectural guarantees that BBML2 makes, but additionally
want BBML2 to not create TLB conflict aborts. Not causing aborts avoids
us having to prove that no recursive faults can be induced in any path
that uses BBML2, allowing its use for arbitrary kernel mappings.

This feature builds on the previous ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
as all early cpus must support BBML2 for us to enable it (and any later
cpus must also support it to be onlined).

Not onlining late cpus that do not support BBML2 is unavoidable, as we
might currently be using BBML2 semantics for kernel memory regions. This
could cause faults in the late cpus, and would be difficult to unwind,
so let us avoid the case altogether.

Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
---
 arch/arm64/include/asm/cpufeature.h |  5 ++++
 arch/arm64/kernel/cpufeature.c      | 40 +++++++++++++++++++++++++++++
 arch/arm64/tools/cpucaps            |  1 +
 3 files changed, 46 insertions(+)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 155ebd040c55..bf13d676aae2 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -871,6 +871,11 @@ static inline bool system_supports_pmuv3(void)
 	return cpus_have_final_cap(ARM64_HAS_PMUV3);
 }
 
+static inline bool system_supports_bbml2_noabort(void)
+{
+	return alternative_has_cap_unlikely(ARM64_HAS_BBML2_NOABORT);
+}
+
 int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
 bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
 
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f9c947166322..2e80ff237b96 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2213,6 +2213,41 @@ static bool hvhe_possible(const struct arm64_cpu_capabilities *entry,
 	return arm64_test_sw_feature_override(ARM64_SW_FEATURE_OVERRIDE_HVHE);
 }
 
+static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int scope)
+{
+	/*
+	 * We want to allow usage of BBML2 in as wide a range of kernel contexts
+	 * as possible. This list is therefore an allow-list of known-good
+	 * implementations that both support BBML2 and additionally, fulfill the
+	 * extra constraint of never generating TLB conflict aborts when using
+	 * the relaxed BBML2 semantics (such aborts make use of BBML2 in certain
+	 * kernel contexts difficult to prove safe against recursive aborts).
+	 *
+	 * Note that implementations can only be considered "known-good" if their
+	 * implementors attest to the fact that the implementation never raises
+	 * TLBI conflict aborts for BBML2 mapping granularity changes.
+	 */
+	static const struct midr_range supports_bbml2_noabort_list[] = {
+		MIDR_REV_RANGE(MIDR_CORTEX_X4, 0, 3, 0xf),
+		MIDR_REV_RANGE(MIDR_NEOVERSE_V3, 0, 2, 0xf),
+		{}
+	};
+
+	/* Does our cpu guarantee to never raise TLB conflict aborts? */
+	if (!is_midr_in_range_list(supports_bbml2_noabort_list))
+		return false;
+
+	/*
+	 * We currently ignore the AA64_ID_MMFR2 register, and only care about
+	 * whether the MIDR check passes. This is because we specifically
+	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
+	 * and so the MMFR2 check is pointless (all implementations passing the
+	 * MIDR check should also pass the MMFR2 check).
+	 */
+
+	return true;
+}
+
 #ifdef CONFIG_ARM64_PAN
 static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
 {
@@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
 		.matches = has_cpuid_feature,
 		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
 	},
+	{
+		.capability = ARM64_HAS_BBML2_NOABORT,
+		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
+		.matches = has_bbml2_noabort,
+	},
 	{
 		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
 		.capability = ARM64_HAS_LPA2,
diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
index 10effd4cff6b..2bd2bfaeddcd 100644
--- a/arch/arm64/tools/cpucaps
+++ b/arch/arm64/tools/cpucaps
@@ -45,6 +45,7 @@ HAS_LPA2
 HAS_LSE_ATOMICS
 HAS_MOPS
 HAS_NESTED_VIRT
+HAS_BBML2_NOABORT
 HAS_PAN
 HAS_PMUV3
 HAS_S1PIE
-- 
2.49.0

[PATCH v7 3/4] iommu/arm: Add BBM Level 2 smmu feature

[Mikołaj Lenczewski]

For supporting BBM Level 2 for userspace mappings, we want to ensure
that the smmu also supports its own version of BBM Level 2. Luckily, the
smmu spec (IHI 0070G 3.21.1.3) is stricter than the aarch64 spec (DDI
0487K.a D8.16.2), so already guarantees that no aborts are raised when
BBM level 2 is claimed.

Add the feature and testing for it under arm_smmu_sva_supported().

Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
Reviewed-by: Robin Murphy <robin.murphy@arm.com>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c | 3 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c     | 3 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h     | 2 ++
 3 files changed, 8 insertions(+)

diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
index 0601dece0a0d..59a480974d80 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3-sva.c
@@ -220,6 +220,9 @@ bool arm_smmu_sva_supported(struct arm_smmu_device *smmu)
 		feat_mask |= ARM_SMMU_FEAT_VAX;
 	}
 
+	if (system_supports_bbml2_noabort())
+		feat_mask |= ARM_SMMU_FEAT_BBML2;
+
 	if ((smmu->features & feat_mask) != feat_mask)
 		return false;
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
index 10cc6dc26b7b..39e933086f8f 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4457,6 +4457,9 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	if (FIELD_GET(IDR3_FWB, reg))
 		smmu->features |= ARM_SMMU_FEAT_S2FWB;
 
+	if (FIELD_GET(IDR3_BBM, reg) == 2)
+		smmu->features |= ARM_SMMU_FEAT_BBML2;
+
 	/* IDR5 */
 	reg = readl_relaxed(smmu->base + ARM_SMMU_IDR5);
 
diff --git a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
index ea41d790463e..a33bf520ba97 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -60,6 +60,7 @@ struct arm_smmu_device;
 #define ARM_SMMU_IDR3			0xc
 #define IDR3_FWB			(1 << 8)
 #define IDR3_RIL			(1 << 10)
+#define IDR3_BBM			GENMASK(12, 11)
 
 #define ARM_SMMU_IDR5			0x14
 #define IDR5_STALL_MAX			GENMASK(31, 16)
@@ -755,6 +756,7 @@ struct arm_smmu_device {
 #define ARM_SMMU_FEAT_HA		(1 << 21)
 #define ARM_SMMU_FEAT_HD		(1 << 22)
 #define ARM_SMMU_FEAT_S2FWB		(1 << 23)
+#define ARM_SMMU_FEAT_BBML2		(1 << 24)
 	u32				features;
 
 #define ARM_SMMU_OPT_SKIP_PREFETCH	(1 << 0)
-- 
2.49.0

[PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Mikołaj Lenczewski]

When converting a region via contpte_convert() to use mTHP, we have two
different goals. We have to mark each entry as contiguous, and we would
like to smear the dirty and young (access) bits across all entries in
the contiguous block. Currently, we do this by first accumulating the
dirty and young bits in the block, using an atomic
__ptep_get_and_clear() and the relevant pte_{dirty,young}() calls,
performing a tlbi, and finally smearing the correct bits across the
block using __set_ptes().

This approach works fine for BBM level 0, but with support for BBM level
2 we are allowed to reorder the tlbi to after setting the pagetable
entries. We expect the time cost of a tlbi to be much greater than the
cost of clearing and resetting the PTEs. As such, this reordering of the
tlbi outside the window where our PTEs are invalid greatly reduces the
duration the PTE are visibly invalid for other threads. This reduces the
likelyhood of a concurrent page walk finding an invalid PTE, reducing
the likelyhood of a fault in other threads, and improving performance
(more so when there are more threads).

Because we support via allowlist only bbml2 implementations that never
raise conflict aborts and instead invalidate the tlb entries
automatically in hardware, we can avoid the final flush altogether.

However, avoiding the intermediate tlbi+dsb must be carefully considered
to ensure that we remain both correct and performant. We document our
reasoning and the expected interactions further in the contpte_convert()
source. To do so we rely on the aarch64 spec (DDI 0487L.a D8.7.1.1)
requirements RNGLXZ and RJQQTC to provide guarantees that the elision is
correct.

Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>
---
 arch/arm64/mm/contpte.c | 139 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 138 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
index bcac4f55f9c1..203357061d0a 100644
--- a/arch/arm64/mm/contpte.c
+++ b/arch/arm64/mm/contpte.c
@@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
 			pte = pte_mkyoung(pte);
 	}
 
-	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
+	/*
+	 * On eliding the __tlb_flush_range() under BBML2+noabort:
+	 *
+	 * NOTE: Instead of using N=16 as the contiguous block length, we use
+	 *       N=4 for clarity.
+	 *
+	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
+	 *       unset and set, respectively.
+	 *
+	 * We worry about two cases where contiguous bit is used:
+	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
+	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
+	 *
+	 * Currently, the BBML0 folding case looks as follows:
+	 *
+	 *  0) Initial page-table layout:
+	 *
+	 *   +----+----+----+----+
+	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
+	 *   +----+----+----+----+
+	 *
+	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
+	 *
+	 *   +----+----+----+----+
+	 *   |  0 |  0 |  0 |  0 |
+	 *   +----+----+----+----+
+	 *
+	 *  2) __flush_tlb_range():
+	 *
+	 *   |____ tlbi + dsb ____|
+	 *
+	 *  3) __set_ptes() to repaint contiguous block:
+	 *
+	 *   +----+----+----+----+
+	 *   |RO,c|RO,c|RO,c|RO,c|
+	 *   +----+----+----+----+
+	 *
+	 *  4) The kernel will eventually __flush_tlb() for changed page:
+	 *
+	 *                  |____| <--- tlbi + dsb
+	 *
+	 * As expected, the intermediate tlbi+dsb ensures that other PEs
+	 * only ever see an invalid (0) entry, or the new contiguous TLB entry.
+	 * The final tlbi+dsb will always throw away the newly installed
+	 * contiguous TLB entry, which is a micro-optimisation opportunity,
+	 * but does not affect correctness.
+	 *
+	 * In the BBML2 case, the change is avoiding the intermediate tlbi+dsb.
+	 * This means a few things, but notably other PEs will still "see" any
+	 * stale cached TLB entries. This could lead to a "contiguous bit
+	 * misprogramming" issue until the final tlbi+dsb of the changed page,
+	 * which would clear out both the stale (RW,n) entry and the new (RO,c)
+	 * contiguous entry installed in its place.
+	 *
+	 * What this is saying, is the following:
+	 *
+	 *  +----+----+----+----+
+	 *  |RO,n|RO,n|RO,n|RW,n| <--- old page tables, all non-contiguous
+	 *  +----+----+----+----+
+	 *
+	 *  +----+----+----+----+
+	 *  |RO,c|RO,c|RO,c|RO,c| <--- new page tables, all contiguous
+	 *  +----+----+----+----+
+	 *   /\
+	 *   ||
+	 *
+	 *  If both the old single (RW,n) and new contiguous (RO,c) TLB entries
+	 *  are present, and a write is made to this address, do we fault or
+	 *  is the write permitted (via amalgamation)?
+	 *
+	 * The relevant Arm ARM DDI 0487L.a requirements are RNGLXZ and RJQQTC,
+	 * and together state that when BBML1 or BBML2 are implemented, either
+	 * a TLB conflict abort is raised (which we expressly forbid), or will
+	 * "produce an OA, access permissions, and memory attributes that are
+	 * consistent with any of the programmed translation table values".
+	 *
+	 * That is to say, will either raise a TLB conflict, or produce one of
+	 * the cached TLB entries, but never amalgamate.
+	 *
+	 * Thus, as the page tables are only considered "consistent" after
+	 * the final tlbi+dsb (which evicts both the single stale (RW,n) TLB
+	 * entry as well as the new contiguous (RO,c) TLB entry), omitting the
+	 * initial tlbi+dsb is correct.
+	 *
+	 * It is also important to note that at the end of the BBML2 folding
+	 * case, we are still left with potentially all N TLB entries still
+	 * cached (the N-1 non-contiguous ptes, and the single contiguous
+	 * block). However, over time, natural TLB pressure will cause the
+	 * non-contiguous pte TLB entries to be flushed, leaving only the
+	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
+	 * not only correct, but also keeps our eventual performance benefits.
+	 *
+	 * For the unfolding case, BBML0 looks as follows:
+	 *
+	 *  0) Initial page-table layout:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,c|RW,c|RW,c|RW,c| <--- last page being set as RO
+	 *   +----+----+----+----+
+	 *
+	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
+	 *
+	 *   +----+----+----+----+
+	 *   |  0 |  0 |  0 |  0 |
+	 *   +----+----+----+----+
+	 *
+	 *  2) __flush_tlb_range():
+	 *
+	 *   |____ tlbi + dsb ____|
+	 *
+	 *  3) __set_ptes() to repaint as non-contiguous:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,n|RW,n|RW,n|RW,n|
+	 *   +----+----+----+----+
+	 *
+	 *  4) Update changed page permissions:
+	 *
+	 *   +----+----+----+----+
+	 *   |RW,n|RW,n|RW,n|RO,n| <--- last page permissions set
+	 *   +----+----+----+----+
+	 *
+	 *  5) The kernel will eventually __flush_tlb() for changed page:
+	 *
+	 *                  |____| <--- tlbi + dsb
+	 *
+	 * For BBML2, we again remove the intermediate tlbi+dsb. Here, there
+	 * are no issues, as the final tlbi+dsb covering the changed page is
+	 * guaranteed to remove the original large contiguous (RW,c) TLB entry,
+	 * as well as the intermediate (RW,n) TLB entry; the next access will
+	 * install the new (RO,n) TLB entry and the page tables are only
+	 * considered "consistent" after the final tlbi+dsb, so software must
+	 * be prepared for this inconsistency prior to finishing the mm dance
+	 * regardless.
+	 */
+
+	if (!system_supports_bbml2_noabort())
+		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
 
 	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
 }
-- 
2.49.0

Re: [PATCH v7 1/4] arm64: cpufeature: Introduce MATCH_ALL_EARLY_CPUS capability type

[Suzuki K Poulose]

On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
> From: Catalin Marinas <catalin.marinas@arm.com>
> 
> For system-wide capabilities, the kernel has the SCOPE_SYSTEM type. Such
> capabilities are checked once the SMP boot has completed using the
> sanitised ID registers. However, there is a need for a new capability
> type similar in scope to the system one but with checking performed
> locally on each CPU during boot (e.g. based on MIDR_EL1 which is not a
> sanitised register).
> 
> Introduce ARM64_CPUCAP_MATCH_ALL_EARLY_CPUS which, together with
> ARM64_CPUCAP_SCOPE_LOCAL_CPU, ensures that such capability is enabled
> only if all early CPUs have it. For ease of use, define
> ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE which combines SCOPE_LOCAL_CPU,
> PERMITTED_FOR_LATE_CPUS and MATCH_ALL_EARLY_CPUS.
> 
> Signed-off-by: Catalin Marinas <catalin.marinas@arm.com>
> Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
> Cc: Suzuki K Poulose <Suzuki.Poulose@arm.com>
> ---

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Suzuki K Poulose]

On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
> The Break-Before-Make cpu feature supports multiple levels (levels 0-2),
> and this commit adds a dedicated BBML2 cpufeature to test against
> support for.
> 
> To support BBML2 in as wide a range of contexts as we can, we want not
> only the architectural guarantees that BBML2 makes, but additionally
> want BBML2 to not create TLB conflict aborts. Not causing aborts avoids
> us having to prove that no recursive faults can be induced in any path
> that uses BBML2, allowing its use for arbitrary kernel mappings.
> 
> This feature builds on the previous ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> as all early cpus must support BBML2 for us to enable it (and any later
> cpus must also support it to be onlined).
> 
> Not onlining late cpus that do not support BBML2 is unavoidable, as we
> might currently be using BBML2 semantics for kernel memory regions. This
> could cause faults in the late cpus, and would be difficult to unwind,
> so let us avoid the case altogether.
> 
> Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>

Reviewed-by: Suzuki K Poulose <suzuki.poulose@arm.com>

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Ryan Roberts]

On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
> The Break-Before-Make cpu feature supports multiple levels (levels 0-2),
> and this commit adds a dedicated BBML2 cpufeature to test against
> support for.
> 
> To support BBML2 in as wide a range of contexts as we can, we want not
> only the architectural guarantees that BBML2 makes, but additionally
> want BBML2 to not create TLB conflict aborts. Not causing aborts avoids
> us having to prove that no recursive faults can be induced in any path
> that uses BBML2, allowing its use for arbitrary kernel mappings.
> 
> This feature builds on the previous ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> as all early cpus must support BBML2 for us to enable it (and any later
> cpus must also support it to be onlined).
> 
> Not onlining late cpus that do not support BBML2 is unavoidable, as we
> might currently be using BBML2 semantics for kernel memory regions. This
> could cause faults in the late cpus, and would be difficult to unwind,
> so let us avoid the case altogether.
> 
> Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
> ---
>  arch/arm64/include/asm/cpufeature.h |  5 ++++
>  arch/arm64/kernel/cpufeature.c      | 40 +++++++++++++++++++++++++++++
>  arch/arm64/tools/cpucaps            |  1 +
>  3 files changed, 46 insertions(+)
> 
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 155ebd040c55..bf13d676aae2 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -871,6 +871,11 @@ static inline bool system_supports_pmuv3(void)
>  	return cpus_have_final_cap(ARM64_HAS_PMUV3);
>  }
>  
> +static inline bool system_supports_bbml2_noabort(void)
> +{
> +	return alternative_has_cap_unlikely(ARM64_HAS_BBML2_NOABORT);
> +}
> +
>  int do_emulate_mrs(struct pt_regs *regs, u32 sys_reg, u32 rt);
>  bool try_emulate_mrs(struct pt_regs *regs, u32 isn);
>  
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f9c947166322..2e80ff237b96 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2213,6 +2213,41 @@ static bool hvhe_possible(const struct arm64_cpu_capabilities *entry,
>  	return arm64_test_sw_feature_override(ARM64_SW_FEATURE_OVERRIDE_HVHE);
>  }
>  
> +static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int scope)
> +{
> +	/*
> +	 * We want to allow usage of BBML2 in as wide a range of kernel contexts
> +	 * as possible. This list is therefore an allow-list of known-good
> +	 * implementations that both support BBML2 and additionally, fulfill the
> +	 * extra constraint of never generating TLB conflict aborts when using
> +	 * the relaxed BBML2 semantics (such aborts make use of BBML2 in certain
> +	 * kernel contexts difficult to prove safe against recursive aborts).
> +	 *
> +	 * Note that implementations can only be considered "known-good" if their
> +	 * implementors attest to the fact that the implementation never raises
> +	 * TLBI conflict aborts for BBML2 mapping granularity changes.
> +	 */
> +	static const struct midr_range supports_bbml2_noabort_list[] = {
> +		MIDR_REV_RANGE(MIDR_CORTEX_X4, 0, 3, 0xf),
> +		MIDR_REV_RANGE(MIDR_NEOVERSE_V3, 0, 2, 0xf),
> +		{}
> +	};
> +
> +	/* Does our cpu guarantee to never raise TLB conflict aborts? */
> +	if (!is_midr_in_range_list(supports_bbml2_noabort_list))
> +		return false;
> +
> +	/*
> +	 * We currently ignore the AA64_ID_MMFR2 register, and only care about
> +	 * whether the MIDR check passes. This is because we specifically
> +	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
> +	 * and so the MMFR2 check is pointless (all implementations passing the
> +	 * MIDR check should also pass the MMFR2 check).
> +	 */
> +
> +	return true;
> +}
> +
>  #ifdef CONFIG_ARM64_PAN
>  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
>  {
> @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
>  	},
> +	{
> +		.capability = ARM64_HAS_BBML2_NOABORT,
> +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> +		.matches = has_bbml2_noabort,

Is there a reason you have removed the .desc from this? Without it, the kernel
won't print a "detected" line when it enables the feature. Previously you had:

		.desc = "BBM Level 2 without conflict abort",

> +	},
>  	{
>  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
>  		.capability = ARM64_HAS_LPA2,
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 10effd4cff6b..2bd2bfaeddcd 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -45,6 +45,7 @@ HAS_LPA2
>  HAS_LSE_ATOMICS
>  HAS_MOPS
>  HAS_NESTED_VIRT
> +HAS_BBML2_NOABORT
>  HAS_PAN
>  HAS_PMUV3
>  HAS_S1PIE

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Mikołaj Lenczewski]

On Wed, Jun 18, 2025 at 03:07:41PM +0100, Ryan Roberts wrote:
> On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
> >  #ifdef CONFIG_ARM64_PAN
> >  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
> >  {
> > @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> >  		.matches = has_cpuid_feature,
> >  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
> >  	},
> > +	{
> > +		.capability = ARM64_HAS_BBML2_NOABORT,
> > +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> > +		.matches = has_bbml2_noabort,
> 
> Is there a reason you have removed the .desc from this? Without it, the kernel
> won't print a "detected" line when it enables the feature. Previously you had:
> 
> 		.desc = "BBM Level 2 without conflict abort",
> 
> > +	},
> >  	{
> >  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
> >  		.capability = ARM64_HAS_LPA2, 
>

Damn it! I completely forgot to birng that back. Yet another remenant of
the NO_BBML2_NOABORT version we had discussed in v6, because a
description field would have been confusing due to the double negation:

https://lore.kernel.org/all/83d1f7af-3dc7-45f9-94f3-8a0917c051d2@arm.com/

Thank you very much for spotting this Ryan! Ill return the description,
and respin on Monday (unless it is possible to do this as a RESEND or
similar, but I doubt it since this is technically a functional change) :/

-- 
Kind regards,
Mikołaj Lenczewski

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Ryan Roberts]

On 19/06/2025 09:57, Mikołaj Lenczewski wrote:
> On Wed, Jun 18, 2025 at 03:07:41PM +0100, Ryan Roberts wrote:
>> On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
>>>  #ifdef CONFIG_ARM64_PAN
>>>  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
>>>  {
>>> @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>>>  		.matches = has_cpuid_feature,
>>>  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
>>>  	},
>>> +	{
>>> +		.capability = ARM64_HAS_BBML2_NOABORT,
>>> +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
>>> +		.matches = has_bbml2_noabort,
>>
>> Is there a reason you have removed the .desc from this? Without it, the kernel
>> won't print a "detected" line when it enables the feature. Previously you had:
>>
>> 		.desc = "BBM Level 2 without conflict abort",
>>
>>> +	},
>>>  	{
>>>  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
>>>  		.capability = ARM64_HAS_LPA2, 
>>
> 
> Damn it! I completely forgot to birng that back. Yet another remenant of
> the NO_BBML2_NOABORT version we had discussed in v6, because a
> description field would have been confusing due to the double negation:
> 
> https://lore.kernel.org/all/83d1f7af-3dc7-45f9-94f3-8a0917c051d2@arm.com/
> 
> Thank you very much for spotting this Ryan! Ill return the description,
> and respin on Monday (unless it is possible to do this as a RESEND or
> similar, but I doubt it since this is technically a functional change) :/
> 

I think probably just send out a new version.

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Catalin Marinas]

On Tue, Jun 17, 2025 at 09:51:02AM +0000, Mikołaj Lenczewski wrote:
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index f9c947166322..2e80ff237b96 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2213,6 +2213,41 @@ static bool hvhe_possible(const struct arm64_cpu_capabilities *entry,
>  	return arm64_test_sw_feature_override(ARM64_SW_FEATURE_OVERRIDE_HVHE);
>  }
>  
> +static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int scope)
> +{
> +	/*
> +	 * We want to allow usage of BBML2 in as wide a range of kernel contexts
> +	 * as possible. This list is therefore an allow-list of known-good
> +	 * implementations that both support BBML2 and additionally, fulfill the
> +	 * extra constraint of never generating TLB conflict aborts when using
> +	 * the relaxed BBML2 semantics (such aborts make use of BBML2 in certain
> +	 * kernel contexts difficult to prove safe against recursive aborts).
> +	 *
> +	 * Note that implementations can only be considered "known-good" if their
> +	 * implementors attest to the fact that the implementation never raises
> +	 * TLBI conflict aborts for BBML2 mapping granularity changes.

s/TLBI/TLB/

> +	 */
> +	static const struct midr_range supports_bbml2_noabort_list[] = {
> +		MIDR_REV_RANGE(MIDR_CORTEX_X4, 0, 3, 0xf),
> +		MIDR_REV_RANGE(MIDR_NEOVERSE_V3, 0, 2, 0xf),
> +		{}
> +	};
> +
> +	/* Does our cpu guarantee to never raise TLB conflict aborts? */
> +	if (!is_midr_in_range_list(supports_bbml2_noabort_list))
> +		return false;
> +
> +	/*
> +	 * We currently ignore the AA64_ID_MMFR2 register, and only care about

s/AA64_ID_MMFR2/ID_AA64MMFR2_EL1/

> +	 * whether the MIDR check passes. This is because we specifically
> +	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
> +	 * and so the MMFR2 check is pointless (all implementations passing the
> +	 * MIDR check should also pass the MMFR2 check).

I think there's at least one implementation that behaves as
BBML2-noabort but does not have the ID field advertising BBML2.

> +	 */
> +
> +	return true;
> +}
> +
>  #ifdef CONFIG_ARM64_PAN
>  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
>  {
> @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
>  		.matches = has_cpuid_feature,
>  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
>  	},
> +	{
> +		.capability = ARM64_HAS_BBML2_NOABORT,
> +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> +		.matches = has_bbml2_noabort,
> +	},
>  	{
>  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
>  		.capability = ARM64_HAS_LPA2,
> diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> index 10effd4cff6b..2bd2bfaeddcd 100644
> --- a/arch/arm64/tools/cpucaps
> +++ b/arch/arm64/tools/cpucaps
> @@ -45,6 +45,7 @@ HAS_LPA2
>  HAS_LSE_ATOMICS
>  HAS_MOPS
>  HAS_NESTED_VIRT
> +HAS_BBML2_NOABORT
>  HAS_PAN
>  HAS_PMUV3
>  HAS_S1PIE

Otherwise it looks fine.

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v7 3/4] iommu/arm: Add BBM Level 2 smmu feature

[Catalin Marinas]

On Tue, Jun 17, 2025 at 09:51:03AM +0000, Mikołaj Lenczewski wrote:
> For supporting BBM Level 2 for userspace mappings, we want to ensure
> that the smmu also supports its own version of BBM Level 2. Luckily, the
> smmu spec (IHI 0070G 3.21.1.3) is stricter than the aarch64 spec (DDI
> 0487K.a D8.16.2), so already guarantees that no aborts are raised when
> BBM level 2 is claimed.
> 
> Add the feature and testing for it under arm_smmu_sva_supported().
> 
> Signed-off-by: Mikołaj Lenczewski <miko.lenczewski@arm.com>
> Reviewed-by: Robin Murphy <robin.murphy@arm.com>
> Reviewed-by: Ryan Roberts <ryan.roberts@arm.com>

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Mikołaj Lenczewski]

On Thu, Jun 19, 2025 at 12:05:05PM +0100, Catalin Marinas wrote:
> On Tue, Jun 17, 2025 at 09:51:02AM +0000, Mikołaj Lenczewski wrote:
> > diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> > index f9c947166322..2e80ff237b96 100644
> > --- a/arch/arm64/kernel/cpufeature.c
> > +++ b/arch/arm64/kernel/cpufeature.c
> > @@ -2213,6 +2213,41 @@ static bool hvhe_possible(const struct arm64_cpu_capabilities *entry,
> >  	return arm64_test_sw_feature_override(ARM64_SW_FEATURE_OVERRIDE_HVHE);
> >  }
> >  
> > +static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int scope)
> > +{
> > +	/*
> > +	 * We want to allow usage of BBML2 in as wide a range of kernel contexts
> > +	 * as possible. This list is therefore an allow-list of known-good
> > +	 * implementations that both support BBML2 and additionally, fulfill the
> > +	 * extra constraint of never generating TLB conflict aborts when using
> > +	 * the relaxed BBML2 semantics (such aborts make use of BBML2 in certain
> > +	 * kernel contexts difficult to prove safe against recursive aborts).
> > +	 *
> > +	 * Note that implementations can only be considered "known-good" if their
> > +	 * implementors attest to the fact that the implementation never raises
> > +	 * TLBI conflict aborts for BBML2 mapping granularity changes.
> 
> s/TLBI/TLB/
> 

ACK

> > +	 */
> > +	static const struct midr_range supports_bbml2_noabort_list[] = {
> > +		MIDR_REV_RANGE(MIDR_CORTEX_X4, 0, 3, 0xf),
> > +		MIDR_REV_RANGE(MIDR_NEOVERSE_V3, 0, 2, 0xf),
> > +		{}
> > +	};
> > +
> > +	/* Does our cpu guarantee to never raise TLB conflict aborts? */
> > +	if (!is_midr_in_range_list(supports_bbml2_noabort_list))
> > +		return false;
> > +
> > +	/*
> > +	 * We currently ignore the AA64_ID_MMFR2 register, and only care about
> 
> s/AA64_ID_MMFR2/ID_AA64MMFR2_EL1/
> 

ACK

> > +	 * whether the MIDR check passes. This is because we specifically
> > +	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
> > +	 * and so the MMFR2 check is pointless (all implementations passing the
> > +	 * MIDR check should also pass the MMFR2 check).
> 
> I think there's at least one implementation that behaves as
> BBML2-noabort but does not have the ID field advertising BBML2.
> 

Yes, I put "should" instead of "will" because of the AmpereOne
situation, but I didn't want to "name and shame". Should I explicitly
call this out? Or would you like me to soften the vocabulary here to imply
that as long as the MIDR passes, the MMFR2 check is not important?

> > +	 */
> > +
> > +	return true;
> > +}
> > +
> >  #ifdef CONFIG_ARM64_PAN
> >  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
> >  {
> > @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> >  		.matches = has_cpuid_feature,
> >  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
> >  	},
> > +	{
> > +		.capability = ARM64_HAS_BBML2_NOABORT,
> > +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> > +		.matches = has_bbml2_noabort,
> > +	},
> >  	{
> >  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
> >  		.capability = ARM64_HAS_LPA2,
> > diff --git a/arch/arm64/tools/cpucaps b/arch/arm64/tools/cpucaps
> > index 10effd4cff6b..2bd2bfaeddcd 100644
> > --- a/arch/arm64/tools/cpucaps
> > +++ b/arch/arm64/tools/cpucaps
> > @@ -45,6 +45,7 @@ HAS_LPA2
> >  HAS_LSE_ATOMICS
> >  HAS_MOPS
> >  HAS_NESTED_VIRT
> > +HAS_BBML2_NOABORT
> >  HAS_PAN
> >  HAS_PMUV3
> >  HAS_S1PIE
> 
> Otherwise it looks fine.
> 
> Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

-- 
Kind regards,
Mikołaj Lenczewski

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Mikołaj Lenczewski]

On Thu, Jun 19, 2025 at 10:20:14AM +0100, Ryan Roberts wrote:
> On 19/06/2025 09:57, Mikołaj Lenczewski wrote:
> > On Wed, Jun 18, 2025 at 03:07:41PM +0100, Ryan Roberts wrote:
> >> On 17/06/2025 10:51, Mikołaj Lenczewski wrote:
> >>>  #ifdef CONFIG_ARM64_PAN
> >>>  static void cpu_enable_pan(const struct arm64_cpu_capabilities *__unused)
> >>>  {
> >>> @@ -2980,6 +3015,11 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> >>>  		.matches = has_cpuid_feature,
> >>>  		ARM64_CPUID_FIELDS(ID_AA64MMFR2_EL1, EVT, IMP)
> >>>  	},
> >>> +	{
> >>> +		.capability = ARM64_HAS_BBML2_NOABORT,
> >>> +		.type = ARM64_CPUCAP_EARLY_LOCAL_CPU_FEATURE,
> >>> +		.matches = has_bbml2_noabort,
> >>
> >> Is there a reason you have removed the .desc from this? Without it, the kernel
> >> won't print a "detected" line when it enables the feature. Previously you had:
> >>
> >> 		.desc = "BBM Level 2 without conflict abort",
> >>
> >>> +	},
> >>>  	{
> >>>  		.desc = "52-bit Virtual Addressing for KVM (LPA2)",
> >>>  		.capability = ARM64_HAS_LPA2, 
> >>
> > 
> > Damn it! I completely forgot to birng that back. Yet another remenant of
> > the NO_BBML2_NOABORT version we had discussed in v6, because a
> > description field would have been confusing due to the double negation:
> > 
> > https://lore.kernel.org/all/83d1f7af-3dc7-45f9-94f3-8a0917c051d2@arm.com/
> > 
> > Thank you very much for spotting this Ryan! Ill return the description,
> > and respin on Monday (unless it is possible to do this as a RESEND or
> > similar, but I doubt it since this is technically a functional change) :/
> > 
> 
> I think probably just send out a new version.
> 

ACK

-- 
Kind regards,
Mikołaj Lenczewski

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Catalin Marinas]

On Thu, Jun 19, 2025 at 12:51:32PM +0100, Mikołaj Lenczewski wrote:
> On Thu, Jun 19, 2025 at 12:05:05PM +0100, Catalin Marinas wrote:
> > On Tue, Jun 17, 2025 at 09:51:02AM +0000, Mikołaj Lenczewski wrote:
> > > +	 * whether the MIDR check passes. This is because we specifically
> > > +	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
> > > +	 * and so the MMFR2 check is pointless (all implementations passing the
> > > +	 * MIDR check should also pass the MMFR2 check).
> > 
> > I think there's at least one implementation that behaves as
> > BBML2-noabort but does not have the ID field advertising BBML2.
> > 
> 
> Yes, I put "should" instead of "will" because of the AmpereOne
> situation, but I didn't want to "name and shame". Should I explicitly
> call this out? Or would you like me to soften the vocabulary here to imply
> that as long as the MIDR passes, the MMFR2 check is not important?

I missed the "should" part. Anyway, I would just drop the explanation
from "This is because...".

-- 
Catalin

Re: [PATCH v7 2/4] arm64: Add BBM Level 2 cpu feature

[Mikołaj Lenczewski]

On Thu, Jun 19, 2025 at 02:46:01PM +0100, Catalin Marinas wrote:
> On Thu, Jun 19, 2025 at 12:51:32PM +0100, Mikołaj Lenczewski wrote:
> > On Thu, Jun 19, 2025 at 12:05:05PM +0100, Catalin Marinas wrote:
> > > On Tue, Jun 17, 2025 at 09:51:02AM +0000, Mikołaj Lenczewski wrote:
> > > > +	 * whether the MIDR check passes. This is because we specifically
> > > > +	 * care only about a stricter form of BBML2 (one guaranteeing noabort),
> > > > +	 * and so the MMFR2 check is pointless (all implementations passing the
> > > > +	 * MIDR check should also pass the MMFR2 check).
> > > 
> > > I think there's at least one implementation that behaves as
> > > BBML2-noabort but does not have the ID field advertising BBML2.
> > > 
> > 
> > Yes, I put "should" instead of "will" because of the AmpereOne
> > situation, but I didn't want to "name and shame". Should I explicitly
> > call this out? Or would you like me to soften the vocabulary here to imply
> > that as long as the MIDR passes, the MMFR2 check is not important?
> 
> I missed the "should" part. Anyway, I would just drop the explanation
> from "This is because...".
> 
> -- 
> Catalin

OK, not a problem! Will drop it.

-- 
Kind regards,
Mikołaj Lenczewski

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Catalin Marinas]

On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
> index bcac4f55f9c1..203357061d0a 100644
> --- a/arch/arm64/mm/contpte.c
> +++ b/arch/arm64/mm/contpte.c
> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>  			pte = pte_mkyoung(pte);
>  	}
>  
> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
> +	/*
> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
> +	 *
> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
> +	 *       N=4 for clarity.
> +	 *
> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
> +	 *       unset and set, respectively.
> +	 *
> +	 * We worry about two cases where contiguous bit is used:
> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
> +	 *
> +	 * Currently, the BBML0 folding case looks as follows:
> +	 *
> +	 *  0) Initial page-table layout:
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
> +	 *   +----+----+----+----+
> +	 *
> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |  0 |  0 |  0 |  0 |
> +	 *   +----+----+----+----+
> +	 *
> +	 *  2) __flush_tlb_range():
> +	 *
> +	 *   |____ tlbi + dsb ____|
> +	 *
> +	 *  3) __set_ptes() to repaint contiguous block:
> +	 *
> +	 *   +----+----+----+----+
> +	 *   |RO,c|RO,c|RO,c|RO,c|
> +	 *   +----+----+----+----+

From the initial layout to point (3), we are also changing the
permission. Given the rules you mentioned in the Arm ARM, I think that's
safe (hardware seeing either the old or the new attributes). The
FEAT_BBM description, however, only talks about change between larger
and smaller blocks but no mention of also changing the attributes at the
same time. Hopefully the microarchitects claiming certain CPUs don't
generate conflict aborts understood what Linux does.

> +	 *
> +	 *  4) The kernel will eventually __flush_tlb() for changed page:
> +	 *
> +	 *                  |____| <--- tlbi + dsb
[...]
> +	 * It is also important to note that at the end of the BBML2 folding
> +	 * case, we are still left with potentially all N TLB entries still
> +	 * cached (the N-1 non-contiguous ptes, and the single contiguous
> +	 * block). However, over time, natural TLB pressure will cause the
> +	 * non-contiguous pte TLB entries to be flushed, leaving only the
> +	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
> +	 * not only correct, but also keeps our eventual performance benefits.

Step 4 above implies some TLB flushing from the core code eventually.
What is the situation mentioned in the paragraph above? Is it only until
we get the TLB flushing from the core code?

[...]
> +	if (!system_supports_bbml2_noabort())
> +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>  
>  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);

Eliding the TLBI here is all good but looking at the overall set_ptes(),
why do we bother with unfold+fold for BBML2? Can we not just change
them in place without going through __ptep_get_and_clear()?

-- 
Catalin

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Catalin Marinas]

On Thu, Jun 19, 2025 at 08:29:24PM +0100, Catalin Marinas wrote:
> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
> > +	if (!system_supports_bbml2_noabort())
> > +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
> >  
> >  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
> 
> Eliding the TLBI here is all good but looking at the overall set_ptes(),
> why do we bother with unfold+fold for BBML2? Can we not just change
> them in place without going through __ptep_get_and_clear()?

Ah, it's unlikely that we'd be able to fold them back if only one pte in
the range was modified, so this optimisation would very rarely/never
trigger. So, for this patch:

Reviewed-by: Catalin Marinas <catalin.marinas@arm.com>

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Ryan Roberts]

On 19/06/2025 20:29, Catalin Marinas wrote:
> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>> index bcac4f55f9c1..203357061d0a 100644
>> --- a/arch/arm64/mm/contpte.c
>> +++ b/arch/arm64/mm/contpte.c
>> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>>  			pte = pte_mkyoung(pte);
>>  	}
>>  
>> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>> +	/*
>> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
>> +	 *
>> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
>> +	 *       N=4 for clarity.
>> +	 *
>> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
>> +	 *       unset and set, respectively.
>> +	 *
>> +	 * We worry about two cases where contiguous bit is used:
>> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
>> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
>> +	 *
>> +	 * Currently, the BBML0 folding case looks as follows:
>> +	 *
>> +	 *  0) Initial page-table layout:
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
>> +	 *   +----+----+----+----+
>> +	 *
>> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |  0 |  0 |  0 |  0 |
>> +	 *   +----+----+----+----+
>> +	 *
>> +	 *  2) __flush_tlb_range():
>> +	 *
>> +	 *   |____ tlbi + dsb ____|
>> +	 *
>> +	 *  3) __set_ptes() to repaint contiguous block:
>> +	 *
>> +	 *   +----+----+----+----+
>> +	 *   |RO,c|RO,c|RO,c|RO,c|
>> +	 *   +----+----+----+----+
> 
> From the initial layout to point (3), we are also changing the
> permission. Given the rules you mentioned in the Arm ARM, I think that's
> safe (hardware seeing either the old or the new attributes). The
> FEAT_BBM description, however, only talks about change between larger
> and smaller blocks but no mention of also changing the attributes at the
> same time. Hopefully the microarchitects claiming certain CPUs don't
> generate conflict aborts understood what Linux does.
> 
>> +	 *
>> +	 *  4) The kernel will eventually __flush_tlb() for changed page:
>> +	 *
>> +	 *                  |____| <--- tlbi + dsb
> [...]
>> +	 * It is also important to note that at the end of the BBML2 folding
>> +	 * case, we are still left with potentially all N TLB entries still
>> +	 * cached (the N-1 non-contiguous ptes, and the single contiguous
>> +	 * block). However, over time, natural TLB pressure will cause the
>> +	 * non-contiguous pte TLB entries to be flushed, leaving only the
>> +	 * contiguous block TLB entry. This means that omitting the tlbi+dsb is
>> +	 * not only correct, but also keeps our eventual performance benefits.
> 
> Step 4 above implies some TLB flushing from the core code eventually.
> What is the situation mentioned in the paragraph above? Is it only until
> we get the TLB flushing from the core code?

I think the point that Miko is making here is that at some point between step 3
and 4, we could end up with up to any 3 of the original small entries as well as
the large entry in the TLB. Then the flush at step 4 will only remove the small
entry at the last entry and the large entry - i.e. it will only remove the
entries that overlap the region that the tlbi targets, and up to 3 of the small
entries may be left in the TLB after step 4. Our rationale here was that this is
safe due to BBML2 and it will naturally sort itself out over time with natural
eviction.

> 
> [...]
>> +	if (!system_supports_bbml2_noabort())
>> +		__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>  
>>  	__set_ptes(mm, start_addr, start_ptep, pte, CONT_PTES);
> 
> Eliding the TLBI here is all good but looking at the overall set_ptes(),
> why do we bother with unfold+fold for BBML2? Can we not just change
> them in place without going through __ptep_get_and_clear()?

We need to collect the access and dirty bits so we can apply the aggregate
across all entries. We can only accurately collect the access and dirty bits if
we go via invalid, otherwise the HW could update after we have read the old
entry but before we have written the new entry.

Thanks,
Ryan

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Ryan Roberts]

On 20/06/2025 17:10, Ryan Roberts wrote:
> On 19/06/2025 20:29, Catalin Marinas wrote:
>> On Tue, Jun 17, 2025 at 09:51:04AM +0000, Mikołaj Lenczewski wrote:
>>> diff --git a/arch/arm64/mm/contpte.c b/arch/arm64/mm/contpte.c
>>> index bcac4f55f9c1..203357061d0a 100644
>>> --- a/arch/arm64/mm/contpte.c
>>> +++ b/arch/arm64/mm/contpte.c
>>> @@ -68,7 +68,144 @@ static void contpte_convert(struct mm_struct *mm, unsigned long addr,
>>>  			pte = pte_mkyoung(pte);
>>>  	}
>>>  
>>> -	__flush_tlb_range(&vma, start_addr, addr, PAGE_SIZE, true, 3);
>>> +	/*
>>> +	 * On eliding the __tlb_flush_range() under BBML2+noabort:
>>> +	 *
>>> +	 * NOTE: Instead of using N=16 as the contiguous block length, we use
>>> +	 *       N=4 for clarity.
>>> +	 *
>>> +	 * NOTE: 'n' and 'c' are used to denote the "contiguous bit" being
>>> +	 *       unset and set, respectively.
>>> +	 *
>>> +	 * We worry about two cases where contiguous bit is used:
>>> +	 *  - When folding N smaller non-contiguous ptes as 1 contiguous block.
>>> +	 *  - When unfolding a contiguous block into N smaller non-contiguous ptes.
>>> +	 *
>>> +	 * Currently, the BBML0 folding case looks as follows:
>>> +	 *
>>> +	 *  0) Initial page-table layout:
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |RO,n|RO,n|RO,n|RW,n| <--- last page being set as RO
>>> +	 *   +----+----+----+----+
>>> +	 *
>>> +	 *  1) Aggregate AF + dirty flags using __ptep_get_and_clear():
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |  0 |  0 |  0 |  0 |
>>> +	 *   +----+----+----+----+
>>> +	 *
>>> +	 *  2) __flush_tlb_range():
>>> +	 *
>>> +	 *   |____ tlbi + dsb ____|
>>> +	 *
>>> +	 *  3) __set_ptes() to repaint contiguous block:
>>> +	 *
>>> +	 *   +----+----+----+----+
>>> +	 *   |RO,c|RO,c|RO,c|RO,c|
>>> +	 *   +----+----+----+----+
>>
>> From the initial layout to point (3), we are also changing the
>> permission. Given the rules you mentioned in the Arm ARM, I think that's
>> safe (hardware seeing either the old or the new attributes). The
>> FEAT_BBM description, however, only talks about change between larger
>> and smaller blocks but no mention of also changing the attributes at the
>> same time. Hopefully the microarchitects claiming certain CPUs don't
>> generate conflict aborts understood what Linux does.

I think what you are saying is that despite going via invalid, the HW may see
this direct transition:

+----+----+----+----+
|RO,n|RO,n|RO,n|RW,n|
+----+----+----+----+
to:
+----+----+----+----+
|RO,c|RO,c|RO,c|RO,c|
+----+----+----+----+

There are 2 logical operations here. The first is changing the permissions of
the last entry. The second is changing the size of the entry.

As I understand it, it's permissible in the architecture to update the
permissions of the a PTE without break-before-make and without issuing a tlbi
afterwards; in that case the HW may apply either the old permissions or the new
permissions up until a future tlbi (after which the new permissions are
guarranteed). That's the first logical operation.

The second logical operation is permitted by FEAT_BBM level 2.

So both logical operations are permitted and the Arm ARM doesn't mention any
requirement to "separate" these operations with a tlbi or anything else, as far
as I can see.

So I would interpret that combining these 2 in the way we have should be safe.
RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
this isn't spelled out super clearly.

Perhaps we can move forwards based on this understanding, and I will seek some
clarifying words to be added to the Arm ARM?

Thanks,
Ryan

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Jason Gunthorpe]

On Wed, Jun 25, 2025 at 02:07:23PM +0100, Ryan Roberts wrote:
> I think what you are saying is that despite going via invalid, the HW may see
> this direct transition:
> 
> +----+----+----+----+
> |RO,n|RO,n|RO,n|RW,n|
> +----+----+----+----+
> to:
> +----+----+----+----+
> |RO,c|RO,c|RO,c|RO,c|
> +----+----+----+----+
> 
> There are 2 logical operations here. The first is changing the permissions of
> the last entry. The second is changing the size of the entry.
> 
> As I understand it, it's permissible in the architecture to update the
> permissions of the a PTE without break-before-make and without issuing a tlbi
> afterwards; in that case the HW may apply either the old permissions or the new
> permissions up until a future tlbi (after which the new permissions are
> guarranteed). That's the first logical operation.
> 
> The second logical operation is permitted by FEAT_BBM level 2.
> 
> So both logical operations are permitted and the Arm ARM doesn't mention any
> requirement to "separate" these operations with a tlbi or anything else, as far
> as I can see.
> 
> So I would interpret that combining these 2 in the way we have should be safe.
> RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
> this isn't spelled out super clearly.
> 
> Perhaps we can move forwards based on this understanding, and I will seek some
> clarifying words to be added to the Arm ARM?

FWIW this matches my understanding as well.

AFAIK the actual physical issue for BBM < 2 is the TLB lookup HW
cannot tolerate having two entries that overlap in address because of
different sizes. Every lookup must return exactly one result. If more
results are returned the HW fails in some way.

For BBM 2 if the HW lookup gets more than one result the HW selects
one unpredictably and uses that.

Permissions shouldn't have any impact because they do not effect how
the lookup is performed, they are the result of the lookup.

I've been looking at implementing BBM 2 support on the iommu side and
I didn't see anything that said we cannot do arbitary transformations
under BBM 2? For instance when degrading from a contiguous range of
block descriptors down to single page descriptors I'm expecting to go
like:
    16x2M -> 2M -> 4k -> FLUSH

Jason

Re: [PATCH v7 4/4] arm64/mm: Elide tlbi in contpte_convert() under BBML2

[Ryan Roberts]

On 25/06/2025 14:37, Jason Gunthorpe wrote:
> On Wed, Jun 25, 2025 at 02:07:23PM +0100, Ryan Roberts wrote:
>> I think what you are saying is that despite going via invalid, the HW may see
>> this direct transition:
>>
>> +----+----+----+----+
>> |RO,n|RO,n|RO,n|RW,n|
>> +----+----+----+----+
>> to:
>> +----+----+----+----+
>> |RO,c|RO,c|RO,c|RO,c|
>> +----+----+----+----+
>>
>> There are 2 logical operations here. The first is changing the permissions of
>> the last entry. The second is changing the size of the entry.
>>
>> As I understand it, it's permissible in the architecture to update the
>> permissions of the a PTE without break-before-make and without issuing a tlbi
>> afterwards; in that case the HW may apply either the old permissions or the new
>> permissions up until a future tlbi (after which the new permissions are
>> guarranteed). That's the first logical operation.
>>
>> The second logical operation is permitted by FEAT_BBM level 2.
>>
>> So both logical operations are permitted and the Arm ARM doesn't mention any
>> requirement to "separate" these operations with a tlbi or anything else, as far
>> as I can see.
>>
>> So I would interpret that combining these 2 in the way we have should be safe.
>> RNGLXZ and RJQQTC give further insight into the spirit of the spec. But I agree
>> this isn't spelled out super clearly.
>>
>> Perhaps we can move forwards based on this understanding, and I will seek some
>> clarifying words to be added to the Arm ARM?
> 
> FWIW this matches my understanding as well.
> 
> AFAIK the actual physical issue for BBM < 2 is the TLB lookup HW
> cannot tolerate having two entries that overlap in address because of
> different sizes. Every lookup must return exactly one result. If more
> results are returned the HW fails in some way.
> 
> For BBM 2 if the HW lookup gets more than one result the HW selects
> one unpredictably and uses that.
> 
> Permissions shouldn't have any impact because they do not effect how
> the lookup is performed, they are the result of the lookup.

Totally agree with all of this! But ideally we want the Arm ARM to be
unambiguous so that we can implement against that rather than against our
understanding of the HW issues and what the Arm ARM "almost certainly intended".

> 
> I've been looking at implementing BBM 2 support on the iommu side and
> I didn't see anything that said we cannot do arbitary transformations
> under BBM 2? For instance when degrading from a contiguous range of
> block descriptors down to single page descriptors I'm expecting to go
> like:
>     16x2M -> 2M -> 4k -> FLUSH

If this is just changing sizes, it's definitely, explcitly safe in the Arm ARM
(I haven't read the equivalent SMMU text). And there isn't a requirement to
flush at the end. You only need to flush the (sub-)ranges where permissions or
whatever have changed.

> 
> Jason