diff mbox series

[v2,4/4] iommu/arm: Add BBM Level 2 smmu feature

Message ID 20250228182403.6269-6-miko.lenczewski@arm.com (mailing list archive)
State New
Headers show
Series Initial BBML2 support for contpte_convert() | expand

Commit Message

Mikołaj Lenczewski Feb. 28, 2025, 6:24 p.m. UTC
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>
---
 arch/arm64/kernel/cpufeature.c                  | 7 +++----
 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     | 4 ++++
 4 files changed, 13 insertions(+), 4 deletions(-)

Comments

Jason Gunthorpe Feb. 28, 2025, 7:32 p.m. UTC | #1
On Fri, Feb 28, 2025 at 06:24:04PM +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>
> ---
>  arch/arm64/kernel/cpufeature.c                  | 7 +++----
>  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     | 4 ++++
>  4 files changed, 13 insertions(+), 4 deletions(-)

This patch looks good, for what it does. However for bisection safety
it should be earlier, before the patches that change the page table
algorithms to be unsafe for the SMMU.

However, I've heard people talking about shipping chips that have CPUs
with BBML2 but SMMUs without.

On such a system it seems like your series would break previously
working SVA support because this patch will end up disabling it?

Though I see your MIDR_REV list is limited, so perhaps that worry
doesn't effect any real chips made with those families? I am trying to
check some NVIDIA products against this list..

Jason
Yang Shi March 1, 2025, 1:32 a.m. UTC | #2
On 2/28/25 10:24 AM, 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>
> ---
>   arch/arm64/kernel/cpufeature.c                  | 7 +++----
>   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     | 4 ++++
>   4 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 63f6d356dc77..1022c63f81b2 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -2223,8 +2223,6 @@ static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int sco
>   			if (!cpu_has_bbml2_noabort(__cpu_read_midr(cpu)))
>   				return false;
>   		}
> -
> -		return true;
>   	} else if (scope & SCOPE_LOCAL_CPU) {
>   		/* We are a hot-plugged CPU, so only need to check our MIDR.
>   		 * If we have the correct MIDR, but the kernel booted on an
> @@ -2232,10 +2230,11 @@ static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int sco
>   		 * we have an incorrect MIDR, but the kernel booted on a
>   		 * sufficient CPU, we will not bring up this CPU.
>   		 */
> -		return cpu_has_bbml2_noabort(read_cpuid_id());
> +		if (!cpu_has_bbml2_noabort(read_cpuid_id()))
> +			return false;
>   	}
>   
> -	return false;
> +	return has_cpuid_feature(caps, scope);

Do we really need this? IIRC, it means the MIDR has to be in the allow 
list *AND* MMFR2 register has to be set too. AmpereOne doesn't have 
MMFR2 register set.

Thanks,
Yang

>   }
>   
>   #ifdef CONFIG_ARM64_PAN
> 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 9ba596430e7c..6ba182572788 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
> @@ -222,6 +222,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 358072b4e293..dcee0bdec924 100644
> --- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> +++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
> @@ -4406,6 +4406,9 @@ static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
>   	if (FIELD_GET(IDR3_RIL, reg))
>   		smmu->features |= ARM_SMMU_FEAT_RANGE_INV;
>   
> +	if (FIELD_GET(IDR3_BBML, reg) == IDR3_BBML2)
> +		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 bd9d7c85576a..85eaf3ab88c2 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,9 @@ struct arm_smmu_device;
>   #define ARM_SMMU_IDR3			0xc
>   #define IDR3_FWB			(1 << 8)
>   #define IDR3_RIL			(1 << 10)
> +#define IDR3_BBML			GENMASK(12, 11)
> +#define IDR3_BBML1			(1 << 11)
> +#define IDR3_BBML2			(2 << 11)
>   
>   #define ARM_SMMU_IDR5			0x14
>   #define IDR5_STALL_MAX			GENMASK(31, 16)
> @@ -754,6 +757,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)
diff mbox series

Patch

diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 63f6d356dc77..1022c63f81b2 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -2223,8 +2223,6 @@  static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int sco
 			if (!cpu_has_bbml2_noabort(__cpu_read_midr(cpu)))
 				return false;
 		}
-
-		return true;
 	} else if (scope & SCOPE_LOCAL_CPU) {
 		/* We are a hot-plugged CPU, so only need to check our MIDR.
 		 * If we have the correct MIDR, but the kernel booted on an
@@ -2232,10 +2230,11 @@  static bool has_bbml2_noabort(const struct arm64_cpu_capabilities *caps, int sco
 		 * we have an incorrect MIDR, but the kernel booted on a
 		 * sufficient CPU, we will not bring up this CPU.
 		 */
-		return cpu_has_bbml2_noabort(read_cpuid_id());
+		if (!cpu_has_bbml2_noabort(read_cpuid_id()))
+			return false;
 	}
 
-	return false;
+	return has_cpuid_feature(caps, scope);
 }
 
 #ifdef CONFIG_ARM64_PAN
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 9ba596430e7c..6ba182572788 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
@@ -222,6 +222,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 358072b4e293..dcee0bdec924 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -4406,6 +4406,9 @@  static int arm_smmu_device_hw_probe(struct arm_smmu_device *smmu)
 	if (FIELD_GET(IDR3_RIL, reg))
 		smmu->features |= ARM_SMMU_FEAT_RANGE_INV;
 
+	if (FIELD_GET(IDR3_BBML, reg) == IDR3_BBML2)
+		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 bd9d7c85576a..85eaf3ab88c2 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,9 @@  struct arm_smmu_device;
 #define ARM_SMMU_IDR3			0xc
 #define IDR3_FWB			(1 << 8)
 #define IDR3_RIL			(1 << 10)
+#define IDR3_BBML			GENMASK(12, 11)
+#define IDR3_BBML1			(1 << 11)
+#define IDR3_BBML2			(2 << 11)
 
 #define ARM_SMMU_IDR5			0x14
 #define IDR5_STALL_MAX			GENMASK(31, 16)
@@ -754,6 +757,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)