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

[Will Deacon]

On Tue, Sep 09, 2025 at 08:45:59AM -0700, Shyam Saini 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 <j...@ziepe.ca>
> Signed-off-by: Shyam Saini <shyamsa...@linux.microsoft.com>
> ---
>  drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 29 +++++++++++++-----
>  drivers/iommu/arm/arm-smmu/arm-smmu.c       | 27 ++++++++++++-----
>  include/linux/iommu.h                       | 33 +++++++++++++++++++++
>  3 files changed, 75 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 2a8b46b948f05..748a5513c5dbb 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,32 @@ 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) {
> +                     pr_warn("IOMMU: Failed to reserve MSI IOVA: No suitable 
> MSI IOVA range available");
> +                     return;
> +             }
> +
> +             list_add_tail(&region->list, head);
> +             return;
> +     }
>  }
>  
>  /*
> 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;
> +     }

Given that we're walking over the reserved regions to see if we have a
collision with MSI_IOVA_BASE, why not allocate the base address
dynamically if we detect a collision rather than having yet another
hard-coded address which we can't guarantee won't be problematic in future?

Will