| Message ID | 20250410225030.2528385-1-shyamsaini@linux.microsoft.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | arm-smmu: select suitable IOVA | expand |
On Thu, Apr 10, 2025 at 03:50:27PM -0700, Shyam Saini wrote: > > Hi, > > Currently, the MSI_IOVA_BASE address is hard-coded to 0x80000000, > assuming that all platforms have this address available for MSI IOVA > reservation. However, this is not always the case, as some platforms > reserve this address for other purposes. Consequently, these platforms > cannot reserve the MSI_IOVA_BASE address for MSI. > > There was an [1] attempt to fix this problem by passing the MSI IOVA > base as a kernel command line parameter. > > This patch series aims to address the issue by introducing a new DTS > property, "arm,smmu-faulty-msi-iova" which can be used to hold faulty > MSI IOVA address. This property can be passed to ARM SMMU drivers > via device tree so that the drivers can select appropriate MSI IOVA base > address which doesn't intersect with the faulty MSI IOVA address. I thought we already talked about this and you were not going to be doing a DT proposal for a software knob? And then you didn't even link to the recent discussion :( https://lore.kernel.org/linux-iommu/20250403232619.GA681099@ziepe.ca/ It is easily solved in the smmuv3 driver without out any DT. Please do that. Jason
Hi Jason, On Thu, 10 Apr 2025 20:00:08 -0300 Jason Gunthorpe <jgg@ziepe.ca> wrote: > On Thu, Apr 10, 2025 at 03:50:27PM -0700, Shyam Saini wrote: > > > > Hi, > > > > Currently, the MSI_IOVA_BASE address is hard-coded to 0x80000000, > > assuming that all platforms have this address available for MSI IOVA > > reservation. However, this is not always the case, as some platforms > > reserve this address for other purposes. Consequently, these > > platforms cannot reserve the MSI_IOVA_BASE address for MSI. > > The fundamental question remains: why is using IOVA 0x80000000, an arbitrarily chosen virtual address, a hardware issue? > > There was an [1] attempt to fix this problem by passing the MSI IOVA > > base as a kernel command line parameter. > > > > This patch series aims to address the issue by introducing a new DTS > > property, "arm,smmu-faulty-msi-iova" which can be used to hold > > faulty MSI IOVA address. This property can be passed to ARM SMMU > > drivers via device tree so that the drivers can select appropriate > > MSI IOVA base address which doesn't intersect with the faulty MSI > > IOVA address. > > I thought we already talked about this and you were not going to be > doing a DT proposal for a software knob? > > And then you didn't even link to the recent discussion :( > > https://lore.kernel.org/linux-iommu/20250403232619.GA681099@ziepe.ca/ > > It is easily solved in the smmuv3 driver without out any DT. Please > do that. Given the above fundamental question answered, then we do have a platform specific IOVA range to avoid, IMHO it must be enumerated via DT or some other means to avoid conflict. 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"? Thanks, Jacob
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.. 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: 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(®ion->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(®ion->list, head); + return; + } } static int arm_smmu_dev_enable_feature(struct device *dev, Jason
Hi Jason, On Wed, 16 Apr 2025 15:17:59 -0300 Jason Gunthorpe <jgg@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(®ion->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(®ion->list, head); > + return; > + } > } > > static int arm_smmu_dev_enable_feature(struct device *dev, > > Jason