Re: [PATCH v2 0/3] arm-smmu: select suitable IOVA

[Jacob Pan]

Hi Jason,

On Wed, 16 Apr 2025 15:17:59 -0300
Jason Gunthorpe <j...@ziepe.ca> wrote:

> On Wed, Apr 16, 2025 at 11:04:27AM -0700, Jacob Pan wrote:
> 
> > Per last discussion "SMMU driver have a list of potential addresses
> > and select the first one that does not intersect with the
> > non-working IOVA ranges.". If we don't know what the "non-working
> > IOVA" is, how do we know it does not intersect the "potential
> > addresses"?  
> 
> I had understood from previous discussions that this platform is
> properly creating IOMMU_RESV_RESERVED regions for the IOVA that
> doesn't work. Otherwise everything is broken..
> 
Agree, but I thought the previous attempt was to make
iommu_ops.get_resv_regions() report properly instead of at the platform
level (DT or IORT). Maybe Shayam can show the current reserved regions
in sysfs.

> Presumably that happens through iommu_dma_get_resv_regions() calling
> of_iommu_get_resv_regions() on a DT platform. There is a schema
> describing how to do this, so platform firmware should be able to do
> it..
> 
> So the fix seems trivial enough to me:
> 
Make sense below, probably good to have this regardless of the
current issue. IMHO, FW reserved region should take precedence.

> 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
> b4c21aaed1266a..ebba18579151bc 100644 ---
> a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c +++
> b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c @@ -3562,17 +3562,29 @@
> 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, 0x12340000 };
>  
>       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
> arbitary.
> +      */
> +     for (i = 0; i != ARRAY_SIZE(msi_bases); i++) {
> +             struct iommu_resv_region *region;
> +
> +             if (resv_intersects(msi_bases[i], MSI_IOVA_LENGTH))
> +                     continue;
> +
> +             region = iommu_alloc_resv_region(msi_bases[i],
> MSI_IOVA_LENGTH,
> +                                              IOMMU_WRITE |
> IOMMU_NOEXEC |
> +                                                      IOMMU_MMIO,
> +                                              IOMMU_RESV_SW_MSI,
> GFP_KERNEL);
> +             if (!region)
> +                     return;
> +             list_add_tail(&region->list, head);
> +             return;
> +     }
>  }
>  
>  static int arm_smmu_dev_enable_feature(struct device *dev,
> 
> Jason