Re: [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA

[Jacob Pan <jacob.pan@linux.microsoft.com>]

On Wed,  6 Aug 2025 14:55:39 -0700
Shyam Saini <shyamsaini@linux.microsoft.com> wrote:

> Currently ARM SMMU drivers hardcode PCI MSI IOVA address.
> Not all the platform have same memory mappings and some platform
> could have this address already being mapped for something else.
> This can lead to collision and as a consequence the MSI IOVA addr
> range is never reserved.
> 
> Fix this by reserving faulty IOVA range and selecting alternate
> MSI_IOVA suitable for the intended platform.
> 
> Example of reserving faulty IOVA range for PCIE device in the DTS:
> 
> reserved-memory {
> 	#address-cells = <2>;
> 	#size-cells = <2>;
> 	faulty_iova: resv_faulty {
> 		iommu-addresses = <&pcieX 0x0 0x8000000 0x0 0x100000>;
> 	};
> };
> 
> &pcieX {
> 	memory-region = <&faulty_iova>;
> };
> 
> Suggested-by: Jason Gunthorpe <jgg@ziepe.ca>
> Link: https://lore.kernel.org/linux-iommu/20250416181759.GF493866@ziepe.ca/
> Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27
> +++++++++++++++------ drivers/iommu/arm/arm-smmu/arm-smmu.c       |
> 27 +++++++++++++++------ include/linux/iommu.h
> | 25 +++++++++++++++++++ 3 files changed, 65 insertions(+), 14
> deletions(-)
> 
> 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
> 5f46140a8f3fa..0dfb522e173f0 100644 ---
> a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3642,17 +3642,30 @@
> static int arm_smmu_of_xlate(struct device *dev, static void
> arm_smmu_get_resv_regions(struct device *dev, struct list_head *head)
>  {
> -	struct iommu_resv_region *region;
>  	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>  
> -	region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> -					 prot, IOMMU_RESV_SW_MSI,
> GFP_KERNEL);
> -	if (!region)
> -		return;
> -
> -	list_add_tail(&region->list, head);
> +	static const u64 msi_bases[] = { MSI_IOVA_BASE,
> MSI_IOVA_BASE2 }; 
>  	iommu_dma_get_resv_regions(dev, head);
> +
> +	/*
> +	 * Use the first msi_base that does not intersect with a
> platform
> +	 * reserved region. The SW MSI base selection is entirely
> arbitrary.
> +	 */
> +	for (int i = 0; i != ARRAY_SIZE(msi_bases); i++) {
nit: i < ARRAY_SIZE(msi_bases) is more robust

> +		struct iommu_resv_region *region;
> +
> +		if (resv_region_intersects(msi_bases[i],
> MSI_IOVA_LENGTH, head))
> +			continue;
> +
> +		region = iommu_alloc_resv_region(msi_bases[i],
> MSI_IOVA_LENGTH, prot,
> +						 IOMMU_RESV_SW_MSI,
> GFP_KERNEL);
> +		if (!region)
> +			return;
> +
> +		list_add_tail(&region->list, head);
> +		return;
> +	}

Maybe add a warning if both msi_bases are conflicting. Or you can keep
searching until a non-intersecting region is found. e.g. let
resv_region_intersects() return a valid region.

>  }
>  
>  /*
> diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c index
> 4a07650911991..84b74b8519386 100644 ---
> a/drivers/iommu/arm/arm-smmu/arm-smmu.c +++
> b/drivers/iommu/arm/arm-smmu/arm-smmu.c @@ -1600,17 +1600,30 @@
> static int arm_smmu_of_xlate(struct device *dev, static void
> arm_smmu_get_resv_regions(struct device *dev, struct list_head *head)
>  {
> -	struct iommu_resv_region *region;
>  	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
>  
> -	region = iommu_alloc_resv_region(MSI_IOVA_BASE,
> MSI_IOVA_LENGTH,
> -					 prot, IOMMU_RESV_SW_MSI,
> GFP_KERNEL);
> -	if (!region)
> -		return;
> -
> -	list_add_tail(&region->list, head);
> +	static const u64 msi_bases[] = { MSI_IOVA_BASE,
> MSI_IOVA_BASE2 }; 
>  	iommu_dma_get_resv_regions(dev, head);
> +
> +	/*
> +	 * Use the first msi_base that does not intersect with a
> platform
> +	 * reserved region. The SW MSI base selection is entirely
> arbitrary.
> +	 */
> +	for (int i = 0; i != ARRAY_SIZE(msi_bases); i++) {
> +		struct iommu_resv_region *region;
> +
> +		if (resv_region_intersects(msi_bases[i],
> MSI_IOVA_LENGTH, head))
> +			continue;
> +
> +		region = iommu_alloc_resv_region(msi_bases[i],
> MSI_IOVA_LENGTH, prot,
> +						 IOMMU_RESV_SW_MSI,
> GFP_KERNEL);
> +		if (!region)
> +			return;
> +
> +		list_add_tail(&region->list, head);
> +		return;
> +	}
>  }
>  
>  static int arm_smmu_def_domain_type(struct device *dev)
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 517f908296436..6750ecdebaa94 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -1509,14 +1509,39 @@ static inline void iommu_debugfs_setup(void)
> {} 
>  #ifdef CONFIG_IOMMU_DMA
>  #define MSI_IOVA_BASE        0x8000000
> +#define MSI_IOVA_BASE2       0xA0000000
>  #define MSI_IOVA_LENGTH      0x100000
>  
> +static inline bool resv_region_intersects(phys_addr_t msi_base,
> size_t length,
> +					  struct list_head
> *resv_region_list) +{
> +	struct iommu_resv_region *region;
> +	phys_addr_t start, end, resv_region_end;
> +
> +	start = msi_base;
> +	end = start + length - 1;
> +	list_for_each_entry(region, resv_region_list, list) {
> +		resv_region_end = region->start + region->length - 1;
> +		/* overlap detected */
> +		if (!(start > resv_region_end || end <
> region->start))
> +			return true;
> +	}
> +
> +	return false;
> +}
> +
>  int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t
> base); #else /* CONFIG_IOMMU_DMA */
>  static inline int iommu_get_msi_cookie(struct iommu_domain *domain,
> dma_addr_t base) {
>  	return -ENODEV;
>  }
> +
> +static inline bool resv_region_intersects(phys_addr_t msi_base,
> size_t length,
> +					  struct list_head
> *resv_region_list) +{
> +	return false;
> +}
>  #endif	/* CONFIG_IOMMU_DMA */
>  
>  /*