devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/3] arm-smmu: select suitable MSI IOVA
@ 2025-08-06 21:55 Shyam Saini
  2025-08-06 21:55 ` [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions Shyam Saini
                   ` (2 more replies)
  0 siblings, 3 replies; 16+ messages in thread
From: Shyam Saini @ 2025-08-06 21:55 UTC (permalink / raw)
  To: iommu, linux-arm-kernel, devicetree, virtualization
  Cc: jgg, will, jacob.pan, eric.auger, code, eahariha, vijayb,
	bboscaccy

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 reserving faulty MSI_IOVA
and selecting suitable MSI IOVA address for a given platform.

This approach accommodates platforms that do not have the default MSI base address
available for MSI reservation.

[1]: https://lore.kernel.org/lkml/20200914181307.117792-1-vemegava@linux.microsoft.com/

Thanks,
Shyam

---
v3:
- Drop "arm,smmu-faulty-msi-iova" property change
- Fix iommu driver device tree configuration for PCI devices
- Use "iommu-addresses" property to identify reserved MSI IOVA regions,
  and introduce an additional MSI_IOVA_BASE2 macro to select a suitable
  MSI IOVA base if the default overlaps with a reserved region (suggested by Jason)

v2:
- add new dts property to hold faulty MSI IOVA and select appropriate
  MSI IOVA address
Link: https://lore.kernel.org/linux-iommu/20250410225030.2528385-1-shyamsaini@linux.microsoft.com/

v1:
Link: https://lore.kernel.org/linux-iommu/20250116232307.1436693-1-shyamsaini@linux.microsoft.com/

Shyam Saini (3):
  arm-smmu: move MSI_IOVA macro definitions
  iommu/of: fix device tree configuration for PCI devices
  arm-smmu: select suitable MSI IOVA

 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 27 ++++++++++++++-----
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h |  3 ---
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 30 ++++++++++++++-------
 drivers/iommu/of_iommu.c                    | 11 ++++++++
 drivers/iommu/virtio-iommu.c                |  2 --
 include/linux/iommu.h                       | 28 +++++++++++++++++++
 6 files changed, 79 insertions(+), 22 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions
  2025-08-06 21:55 [PATCH v3 0/3] arm-smmu: select suitable MSI IOVA Shyam Saini
@ 2025-08-06 21:55 ` Shyam Saini
  2025-08-07 14:32   ` kernel test robot
  2025-08-06 21:55 ` [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices Shyam Saini
  2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2 siblings, 1 reply; 16+ messages in thread
From: Shyam Saini @ 2025-08-06 21:55 UTC (permalink / raw)
  To: iommu, linux-arm-kernel, devicetree, virtualization
  Cc: jgg, will, jacob.pan, eric.auger, code, eahariha, vijayb,
	bboscaccy

MSI_IOVA* are common among different iommu/smu drivers,
so move them to common iommu.h header file.

Suggested-by: Jacob Pan <jacob.pan@linux.microsoft.com>
Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h | 3 ---
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 3 ---
 drivers/iommu/virtio-iommu.c                | 2 --
 include/linux/iommu.h                       | 3 +++
 4 files changed, 3 insertions(+), 8 deletions(-)

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 ea41d790463ed..4db5714a50f1e 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -503,9 +503,6 @@ static inline unsigned int arm_smmu_cdtab_l2_idx(unsigned int ssid)
 #define ARM_SMMU_POLL_TIMEOUT_US	1000000 /* 1s! */
 #define ARM_SMMU_POLL_SPIN_COUNT	10
 
-#define MSI_IOVA_BASE			0x8000000
-#define MSI_IOVA_LENGTH			0x100000
-
 enum pri_resp {
 	PRI_RESP_DENY = 0,
 	PRI_RESP_FAIL = 1,
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index 4ced4b5bee4df..4a07650911991 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -50,9 +50,6 @@
  */
 #define QCOM_DUMMY_VAL -1
 
-#define MSI_IOVA_BASE			0x8000000
-#define MSI_IOVA_LENGTH			0x100000
-
 static int force_stage;
 module_param(force_stage, int, S_IRUGO);
 MODULE_PARM_DESC(force_stage,
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 532db1de201ba..6b72776f17716 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -24,8 +24,6 @@
 
 #include "dma-iommu.h"
 
-#define MSI_IOVA_BASE			0x8000000
-#define MSI_IOVA_LENGTH			0x100000
 
 #define VIOMMU_REQUEST_VQ		0
 #define VIOMMU_EVENT_VQ			1
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 7073be1d88415..517f908296436 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1508,6 +1508,9 @@ static inline void iommu_debugfs_setup(void) {}
 #endif
 
 #ifdef CONFIG_IOMMU_DMA
+#define MSI_IOVA_BASE        0x8000000
+#define MSI_IOVA_LENGTH      0x100000
+
 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)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-06 21:55 [PATCH v3 0/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2025-08-06 21:55 ` [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions Shyam Saini
@ 2025-08-06 21:55 ` Shyam Saini
  2025-08-08 12:15   ` Jason Gunthorpe
  2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2 siblings, 1 reply; 16+ messages in thread
From: Shyam Saini @ 2025-08-06 21:55 UTC (permalink / raw)
  To: iommu, linux-arm-kernel, devicetree, virtualization
  Cc: jgg, will, jacob.pan, eric.auger, code, eahariha, vijayb,
	bboscaccy

Individual PCI devices lack dedicated device tree nodes, but
their IOMMU configuration (including reserved IOVA regions) is often
defined at the PCI host controller level in the device tree. The
"iommu-addresses" property in reserved-memory nodes specifies IOVA
ranges that should be reserved for specific devices.

Currently, PCI devices cannot access these configurations because their
dev->of_node is NULL, preventing of_iommu_get_resv_regions() from
discovering reserved regions defined in the device tree.

Fix this by assigning the PCI host controller's device tree node to
PCI devices during IOMMU configuration, enabling them to inherit the
host controller's device tree properties. This allows PCI devices to
properly discover and reserve IOVA regions specified in the device tree.

Link: https://lore.kernel.org/linux-iommu/20250529183815.GA236098@ziepe.ca/
Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 drivers/iommu/of_iommu.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
index 6b989a62def20..077482917e3e8 100644
--- a/drivers/iommu/of_iommu.c
+++ b/drivers/iommu/of_iommu.c
@@ -145,6 +145,17 @@ int of_iommu_configure(struct device *dev, struct device_node *master_np,
 		err = pci_for_each_dma_alias(to_pci_dev(dev),
 					     of_pci_iommu_init, &info);
 		of_pci_check_device_ats(dev, master_np);
+
+		/*
+		 * For PCI devices, ensure the device's of_node points to the
+		 * PCI host controller's device tree node so that reserved regions
+		 * and other DT-specific IOMMU configuration can be found.
+		 * PCI devices typically don't have individual DT nodes, but
+		 * their configuration (including reserved regions) is defined
+		 * at the PCI host controller level.
+		 */
+		if (!err && master_np && !dev->of_node)
+			dev->of_node = of_node_get(master_np);
 	} else {
 		err = of_iommu_configure_device(master_np, dev, id);
 	}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA
  2025-08-06 21:55 [PATCH v3 0/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2025-08-06 21:55 ` [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions Shyam Saini
  2025-08-06 21:55 ` [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices Shyam Saini
@ 2025-08-06 21:55 ` Shyam Saini
  2025-08-06 23:58   ` Jason Gunthorpe
                     ` (2 more replies)
  2 siblings, 3 replies; 16+ messages in thread
From: Shyam Saini @ 2025-08-06 21:55 UTC (permalink / raw)
  To: iommu, linux-arm-kernel, devicetree, virtualization
  Cc: jgg, will, jacob.pan, eric.auger, code, eahariha, vijayb,
	bboscaccy

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++) {
+		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;
+	}
 }
 
 /*
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 */
 
 /*
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA
  2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
@ 2025-08-06 23:58   ` Jason Gunthorpe
  2025-08-07 17:55   ` kernel test robot
  2025-08-11 22:34   ` Jacob Pan
  2 siblings, 0 replies; 16+ messages in thread
From: Jason Gunthorpe @ 2025-08-06 23:58 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy

On Wed, Aug 06, 2025 at 02:55:39PM -0700, Shyam Saini wrote:
> --- 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++) {
> +		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;
> +	}
>  }

I think this whole series looks pretty good, but I would suggest to
put this in a helper..

'iommu_set_sw_msi()' perhaps, must be called after iommu_dma_get_resv_regions()

Then maybe the constants can just be placed in the .c file inside the
helper function.

Jsaon

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions
  2025-08-06 21:55 ` [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions Shyam Saini
@ 2025-08-07 14:32   ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-08-07 14:32 UTC (permalink / raw)
  To: Shyam Saini, iommu, linux-arm-kernel, devicetree, virtualization
  Cc: oe-kbuild-all, jgg, will, jacob.pan, eric.auger, code, eahariha,
	vijayb, bboscaccy

Hi Shyam,

kernel test robot noticed the following build errors:

[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.16 next-20250807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shyam-Saini/arm-smmu-move-MSI_IOVA-macro-definitions/20250807-055741
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20250806215539.1240561-2-shyamsaini%40linux.microsoft.com
patch subject: [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions
config: um-randconfig-r071-20250807 (https://download.01.org/0day-ci/archive/20250807/202508072221.zOvuARaV-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 7b8dea265e72c3037b6b1e54d5ab51b7e14f328b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250807/202508072221.zOvuARaV-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508072221.zOvuARaV-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/iommu/arm/arm-smmu/arm-smmu.c:24:
   In file included from include/linux/dma-mapping.h:8:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
         |                                                   ~~~~~~~~~~ ^
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:1603:35: error: use of undeclared identifier 'MSI_IOVA_BASE'
    1603 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                          ^~~~~~~~~~~~~
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1603:50: error: use of undeclared identifier 'MSI_IOVA_LENGTH'
    1603 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                                         ^~~~~~~~~~~~~~~
   1 warning and 2 errors generated.


vim +/MSI_IOVA_BASE +1603 drivers/iommu/arm/arm-smmu/arm-smmu.c

021bb8420d44cf drivers/iommu/arm-smmu.c              Robin Murphy 2016-09-14  1596  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1597  static void arm_smmu_get_resv_regions(struct device *dev,
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1598  				      struct list_head *head)
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1599  {
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1600  	struct iommu_resv_region *region;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1601  	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1602  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19 @1603  	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
0251d0107cfb0b drivers/iommu/arm/arm-smmu/arm-smmu.c Lu Baolu     2022-10-19  1604  					 prot, IOMMU_RESV_SW_MSI, GFP_KERNEL);
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1605  	if (!region)
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1606  		return;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1607  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1608  	list_add_tail(&region->list, head);
273df9635385b2 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1609  
273df9635385b2 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1610  	iommu_dma_get_resv_regions(dev, head);
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1611  }
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1612  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA
  2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2025-08-06 23:58   ` Jason Gunthorpe
@ 2025-08-07 17:55   ` kernel test robot
  2025-08-11 22:34   ` Jacob Pan
  2 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2025-08-07 17:55 UTC (permalink / raw)
  To: Shyam Saini, iommu, linux-arm-kernel, devicetree, virtualization
  Cc: oe-kbuild-all, jgg, will, jacob.pan, eric.auger, code, eahariha,
	vijayb, bboscaccy

Hi Shyam,

kernel test robot noticed the following build errors:

[auto build test ERROR on soc/for-next]
[also build test ERROR on linus/master v6.16 next-20250807]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Shyam-Saini/arm-smmu-move-MSI_IOVA-macro-definitions/20250807-055741
base:   https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git for-next
patch link:    https://lore.kernel.org/r/20250806215539.1240561-4-shyamsaini%40linux.microsoft.com
patch subject: [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA
config: um-randconfig-r071-20250807 (https://download.01.org/0day-ci/archive/20250808/202508080104.M1bnmiMo-lkp@intel.com/config)
compiler: clang version 22.0.0git (https://github.com/llvm/llvm-project 7b8dea265e72c3037b6b1e54d5ab51b7e14f328b)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250808/202508080104.M1bnmiMo-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202508080104.M1bnmiMo-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/iommu/arm/arm-smmu/arm-smmu.c:24:
   In file included from include/linux/dma-mapping.h:8:
   In file included from include/linux/scatterlist.h:9:
   In file included from arch/um/include/asm/io.h:24:
   include/asm-generic/io.h:1175:55: warning: performing pointer arithmetic on a null pointer has undefined behavior [-Wnull-pointer-arithmetic]
    1175 |         return (port > MMIO_UPPER_LIMIT) ? NULL : PCI_IOBASE + port;
         |                                                   ~~~~~~~~~~ ^
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1602:35: error: use of undeclared identifier 'MSI_IOVA_BASE'
    1602 |         static const u64 msi_bases[] = { MSI_IOVA_BASE, MSI_IOVA_BASE2 };
         |                                          ^~~~~~~~~~~~~
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:1602:50: error: use of undeclared identifier 'MSI_IOVA_BASE2'
    1602 |         static const u64 msi_bases[] = { MSI_IOVA_BASE, MSI_IOVA_BASE2 };
         |                                                         ^~~~~~~~~~~~~~
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:1610:23: error: invalid application of 'sizeof' to an incomplete type 'const u64[]' (aka 'const unsigned long long[]')
    1610 |         for (int i = 0; i != ARRAY_SIZE(msi_bases); i++) {
         |                              ^~~~~~~~~~~~~~~~~~~~~
   include/linux/array_size.h:11:32: note: expanded from macro 'ARRAY_SIZE'
      11 | #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof((arr)[0]) + __must_be_array(arr))
         |                                ^~~~~
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1613:44: error: use of undeclared identifier 'MSI_IOVA_LENGTH'
    1613 |                 if (resv_region_intersects(msi_bases[i], MSI_IOVA_LENGTH, head))
         |                                                          ^~~~~~~~~~~~~~~
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1616:50: error: use of undeclared identifier 'MSI_IOVA_LENGTH'
    1616 |                 region = iommu_alloc_resv_region(msi_bases[i], MSI_IOVA_LENGTH, prot,
         |                                                                ^~~~~~~~~~~~~~~
   1 warning and 5 errors generated.


vim +/MSI_IOVA_BASE2 +1602 drivers/iommu/arm/arm-smmu/arm-smmu.c

  1596	
  1597	static void arm_smmu_get_resv_regions(struct device *dev,
  1598					      struct list_head *head)
  1599	{
  1600		int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
  1601	
> 1602		static const u64 msi_bases[] = { MSI_IOVA_BASE, MSI_IOVA_BASE2 };
  1603	
  1604		iommu_dma_get_resv_regions(dev, head);
  1605	
  1606		/*
  1607		 * Use the first msi_base that does not intersect with a platform
  1608		 * reserved region. The SW MSI base selection is entirely arbitrary.
  1609		 */
> 1610		for (int i = 0; i != ARRAY_SIZE(msi_bases); i++) {
  1611			struct iommu_resv_region *region;
  1612	
  1613			if (resv_region_intersects(msi_bases[i], MSI_IOVA_LENGTH, head))
  1614				continue;
  1615	
  1616			region = iommu_alloc_resv_region(msi_bases[i], MSI_IOVA_LENGTH, prot,
  1617							 IOMMU_RESV_SW_MSI, GFP_KERNEL);
  1618			if (!region)
  1619				return;
  1620	
  1621			list_add_tail(&region->list, head);
  1622			return;
  1623		}
  1624	}
  1625	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-06 21:55 ` [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices Shyam Saini
@ 2025-08-08 12:15   ` Jason Gunthorpe
  2025-08-14 23:30     ` Shyam Saini
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2025-08-08 12:15 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy

On Wed, Aug 06, 2025 at 02:55:38PM -0700, Shyam Saini wrote:
> Individual PCI devices lack dedicated device tree nodes, but

This is not the case, you can put PCI devices in the DT as well, there
is a syntax for it. Why not do that and have the FW populate it on the
device node itself?

You should CC all the device tree maintainers on something like this
as well.

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA
  2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
  2025-08-06 23:58   ` Jason Gunthorpe
  2025-08-07 17:55   ` kernel test robot
@ 2025-08-11 22:34   ` Jacob Pan
  2 siblings, 0 replies; 16+ messages in thread
From: Jacob Pan @ 2025-08-11 22:34 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, jgg, will,
	eric.auger, code, eahariha, vijayb, bboscaccy

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 */
>  
>  /*


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-08 12:15   ` Jason Gunthorpe
@ 2025-08-14 23:30     ` Shyam Saini
  2025-08-15  0:39       ` Jason Gunthorpe
  0 siblings, 1 reply; 16+ messages in thread
From: Shyam Saini @ 2025-08-14 23:30 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

Hi Jason,

On Fri, Aug 08, 2025 at 09:15:15AM -0300, Jason Gunthorpe wrote:
> On Wed, Aug 06, 2025 at 02:55:38PM -0700, Shyam Saini wrote:
> > Individual PCI devices lack dedicated device tree nodes, but
> 
> This is not the case, you can put PCI devices in the DT as well, there
> is a syntax for it. Why not do that and have the FW populate it on the
> device node itself?

sorry, I missed that, were you referring to [1] CONFIG_PCI_DYNAMIC_OF_NODES
approach?

I tried a PoC with that and got the expected results. However, it requires
hardcoding PCI device IDs using DECLARE_PCI_FIXUP_FINAL?

or were you referring to [2]?

In that case, the PCI child node data needs to be parsed, which is
currently handled individually by each host controller driver.

This might not be a strong argument, but since firmware updates tend
to happen less frequently than OS updates, any addition or modification
to the FDT would require a firmware update. Wouldn't it be more
maintainable to keep this logic in Linux, which is easier to update
and typically updated more often?
  
> You should CC all the device tree maintainers on something like this
> as well.
> 

sure, Cc'd some more folks

[1] https://lwn.net/Articles/939317/
[2] https://elixir.bootlin.com/linux/v6.16/source/arch/arm64/boot/dts/nvidia/tegra186.dtsi#L1340

Thanks,
Shyam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-14 23:30     ` Shyam Saini
@ 2025-08-15  0:39       ` Jason Gunthorpe
  2025-08-15  2:32         ` Shyam Saini
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2025-08-15  0:39 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> or were you referring to [2]?
> 
> In that case, the PCI child node data needs to be parsed, which is
> currently handled individually by each host controller driver.

Yes, this looks like it may be what I was thinking of, the pci@1,0
specifes the BDF effectively
 
> This might not be a strong argument, but since firmware updates tend
> to happen less frequently than OS updates, any addition or modification
> to the FDT would require a firmware update. Wouldn't it be more
> maintainable to keep this logic in Linux, which is easier to update
> and typically updated more often?

The DT is supposed to describe the HW, if the PCI device has an issue
with its dma ranges then it seems reasonable to me the FW will use the
existing standards based way to describe that issue?

Presumably this is a fixed issue of the platform. You never did
explain how your system has such werdio behavior, or how something
like iommu=pt can function on it...

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-15  0:39       ` Jason Gunthorpe
@ 2025-08-15  2:32         ` Shyam Saini
  2025-08-18 15:53           ` Jason Gunthorpe
  0 siblings, 1 reply; 16+ messages in thread
From: Shyam Saini @ 2025-08-15  2:32 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

On Thu, Aug 14, 2025 at 09:39:58PM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> > or were you referring to [2]?
> > 
> > In that case, the PCI child node data needs to be parsed, which is
> > currently handled individually by each host controller driver.
> 
> Yes, this looks like it may be what I was thinking of, the pci@1,0
> specifes the BDF effectively
> 

In that case, we'll need to parse the child DTS nodes properly
within of_iommu_get_resv_regions(). I'll include this in v4.

> > This might not be a strong argument, but since firmware updates tend
> > to happen less frequently than OS updates, any addition or modification
> > to the FDT would require a firmware update. Wouldn't it be more
> > maintainable to keep this logic in Linux, which is easier to update
> > and typically updated more often?
> 
> The DT is supposed to describe the HW, if the PCI device has an issue
> with its dma ranges then it seems reasonable to me the FW will use the
> existing standards based way to describe that issue?

If that's the preferred approach, I'll revise the next version accordingly

> Presumably this is a fixed issue of the platform. You never did
> explain how your system has such werdio behavior, or how something
> like iommu=pt can function on it...
Yes, this issue is platform-specific. On this platform, the default MSI IOVA
range overlaps with an address region that is reserved for another purpose,
Other than that we haven't observed any obvious issues for DMA operations

Thanks,
Shyam

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-15  2:32         ` Shyam Saini
@ 2025-08-18 15:53           ` Jason Gunthorpe
  2025-08-26  1:15             ` Shyam Saini
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2025-08-18 15:53 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

On Thu, Aug 14, 2025 at 07:32:36PM -0700, Shyam Saini wrote:
> On Thu, Aug 14, 2025 at 09:39:58PM -0300, Jason Gunthorpe wrote:
> > On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> > > or were you referring to [2]?
> > > 
> > > In that case, the PCI child node data needs to be parsed, which is
> > > currently handled individually by each host controller driver.
> > 
> > Yes, this looks like it may be what I was thinking of, the pci@1,0
> > specifes the BDF effectively
> 
> In that case, we'll need to parse the child DTS nodes properly
> within of_iommu_get_resv_regions(). I'll include this in v4.

Kinda surprised this isn't happening already? It would be good to
refer to the original specs and describe how whatetever you propose is
aligned there.

> > Presumably this is a fixed issue of the platform. You never did
> > explain how your system has such werdio behavior, or how something
> > like iommu=pt can function on it...
> Yes, this issue is platform-specific. On this platform, the default MSI IOVA
> range overlaps with an address region that is reserved for another purpose,
> Other than that we haven't observed any obvious issues for DMA operations

Usually DRAM is at the default MSI IOVA address, so if you run
iommu=pt then presumably your DRAM map has a nice hole in it.. But
maybe the memory map told the OS about that.

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-18 15:53           ` Jason Gunthorpe
@ 2025-08-26  1:15             ` Shyam Saini
  2025-08-26 12:36               ` Jason Gunthorpe
  0 siblings, 1 reply; 16+ messages in thread
From: Shyam Saini @ 2025-08-26  1:15 UTC (permalink / raw)
  To: Jason Gunthorpe
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

Hi Jason,

On Mon, Aug 18, 2025 at 12:53:10PM -0300, Jason Gunthorpe wrote:
> On Thu, Aug 14, 2025 at 07:32:36PM -0700, Shyam Saini wrote:
> > On Thu, Aug 14, 2025 at 09:39:58PM -0300, Jason Gunthorpe wrote:
> > > On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> > > > or were you referring to [2]?
> > > > 
> > > > In that case, the PCI child node data needs to be parsed, which is
> > > > currently handled individually by each host controller driver.
> > > 
> > > Yes, this looks like it may be what I was thinking of, the pci@1,0
> > > specifes the BDF effectively
> > 
> > In that case, we'll need to parse the child DTS nodes properly
> > within of_iommu_get_resv_regions(). I'll include this in v4.
> 
> Kinda surprised this isn't happening already? It would be good to
> refer to the original specs and describe how whatetever you propose is
> aligned there.

Just to confirm, does the v3 version of this series look good to you?
If so, I’ll go ahead and respin the series with the iommu_set_sw_msi()
change and address the other review comments from Jacob.

Otherwise having pci devices nodes in the fdt or dts needs additional
handling, let me know your preference :)

> 
> > > Presumably this is a fixed issue of the platform. You never did
> > > explain how your system has such werdio behavior, or how something
> > > like iommu=pt can function on it...
> > Yes, this issue is platform-specific. On this platform, the default MSI IOVA
> > range overlaps with an address region that is reserved for another purpose,
> > Other than that we haven't observed any obvious issues for DMA operations
> 
> Usually DRAM is at the default MSI IOVA address, so if you run
> iommu=pt then presumably your DRAM map has a nice hole in it.. But
> maybe the memory map told the OS about that.
>
 
our DRAM for this platform starts at higher offset, way beyond MSI_IOVA.


Thanks,
Shyam 

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-26  1:15             ` Shyam Saini
@ 2025-08-26 12:36               ` Jason Gunthorpe
  2025-08-26 22:19                 ` Shyam Saini
  0 siblings, 1 reply; 16+ messages in thread
From: Jason Gunthorpe @ 2025-08-26 12:36 UTC (permalink / raw)
  To: Shyam Saini
  Cc: iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy, robh,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

On Mon, Aug 25, 2025 at 06:15:57PM -0700, Shyam Saini wrote:
> Hi Jason,
> 
> On Mon, Aug 18, 2025 at 12:53:10PM -0300, Jason Gunthorpe wrote:
> > On Thu, Aug 14, 2025 at 07:32:36PM -0700, Shyam Saini wrote:
> > > On Thu, Aug 14, 2025 at 09:39:58PM -0300, Jason Gunthorpe wrote:
> > > > On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> > > > > or were you referring to [2]?
> > > > > 
> > > > > In that case, the PCI child node data needs to be parsed, which is
> > > > > currently handled individually by each host controller driver.
> > > > 
> > > > Yes, this looks like it may be what I was thinking of, the pci@1,0
> > > > specifes the BDF effectively
> > > 
> > > In that case, we'll need to parse the child DTS nodes properly
> > > within of_iommu_get_resv_regions(). I'll include this in v4.
> > 
> > Kinda surprised this isn't happening already? It would be good to
> > refer to the original specs and describe how whatetever you propose is
> > aligned there.
> 
> Just to confirm, does the v3 version of this series look good to you?
> If so, I’ll go ahead and respin the series with the iommu_set_sw_msi()
> change and address the other review comments from Jacob.
> 
> Otherwise having pci devices nodes in the fdt or dts needs additional
> handling, let me know your preference :)

I think you need to consult with the DT/OF owners on these questions..

I've forgotton so many of the details here.

But once you settle on whatever the DT representation is it looks like
the right direction to me.

Jason

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices
  2025-08-26 12:36               ` Jason Gunthorpe
@ 2025-08-26 22:19                 ` Shyam Saini
  0 siblings, 0 replies; 16+ messages in thread
From: Shyam Saini @ 2025-08-26 22:19 UTC (permalink / raw)
  To: thierry.reding, robin.murphy, robh, jroedel
  Cc: jgg, iommu, linux-arm-kernel, devicetree, virtualization, will,
	jacob.pan, eric.auger, code, eahariha, vijayb, bboscaccy,
	saravanak, krzk+dt, conor+dt, lizhi.hou, clement.leger

Hi Thiery, Rob, Robin, and Joerg,

On 26 Aug 2025 09:36, Jason Gunthorpe wrote:
> On Mon, Aug 25, 2025 at 06:15:57PM -0700, Shyam Saini wrote:
> > Hi Jason,
> > 
> > On Mon, Aug 18, 2025 at 12:53:10PM -0300, Jason Gunthorpe wrote:
> > > On Thu, Aug 14, 2025 at 07:32:36PM -0700, Shyam Saini wrote:
> > > > On Thu, Aug 14, 2025 at 09:39:58PM -0300, Jason Gunthorpe wrote:
> > > > > On Thu, Aug 14, 2025 at 04:30:18PM -0700, Shyam Saini wrote:
> > > > > > or were you referring to [2]?
> > > > > > 
> > > > > > In that case, the PCI child node data needs to be parsed, which is
> > > > > > currently handled individually by each host controller driver.
> > > > > 
> > > > > Yes, this looks like it may be what I was thinking of, the pci@1,0
> > > > > specifes the BDF effectively
> > > > 
> > > > In that case, we'll need to parse the child DTS nodes properly
> > > > within of_iommu_get_resv_regions(). I'll include this in v4.
> > > 
> > > Kinda surprised this isn't happening already? It would be good to
> > > refer to the original specs and describe how whatetever you propose is
> > > aligned there.
> > 
> > Just to confirm, does the v3 version of this series look good to you?
> > If so, I’ll go ahead and respin the series with the iommu_set_sw_msi()
> > change and address the other review comments from Jacob.
> > 
> > Otherwise having pci devices nodes in the fdt or dts needs additional
> > handling, let me know your preference :)
> 
> I think you need to consult with the DT/OF owners on these questions..
> 
> I've forgotton so many of the details here.
> 
> But once you settle on whatever the DT representation is it looks like
> the right direction to me.
> 

The of_iommu driver currently does not correctly handle parsing
of the memory-region property for individual PCI devices. This
patch addresses the issue by explicitly setting dev->of_node for
PCI devices, enabling proper parsing of the memory-region property
within of_iommu_get_resv_regions().

Could you please share your feedback on this fix?

Thanks,
Shyam

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2025-08-26 22:19 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-06 21:55 [PATCH v3 0/3] arm-smmu: select suitable MSI IOVA Shyam Saini
2025-08-06 21:55 ` [PATCH v3 1/3] arm-smmu: move MSI_IOVA macro definitions Shyam Saini
2025-08-07 14:32   ` kernel test robot
2025-08-06 21:55 ` [PATCH v3 2/3] iommu/of: fix device tree configuration for PCI devices Shyam Saini
2025-08-08 12:15   ` Jason Gunthorpe
2025-08-14 23:30     ` Shyam Saini
2025-08-15  0:39       ` Jason Gunthorpe
2025-08-15  2:32         ` Shyam Saini
2025-08-18 15:53           ` Jason Gunthorpe
2025-08-26  1:15             ` Shyam Saini
2025-08-26 12:36               ` Jason Gunthorpe
2025-08-26 22:19                 ` Shyam Saini
2025-08-06 21:55 ` [PATCH v3 3/3] arm-smmu: select suitable MSI IOVA Shyam Saini
2025-08-06 23:58   ` Jason Gunthorpe
2025-08-07 17:55   ` kernel test robot
2025-08-11 22:34   ` Jacob Pan

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).