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

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

[Shyam Saini]

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.

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

---
v2:
- add new dts property to hold faulty MSI IOVA and select appropriate
  MSI IOVA address

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
  arm-smmu: select suitable MSI IOVA
  dt-bindings: iommu: add "arm,smmu-faulty-msi-iova" property

 .../bindings/iommu/arm,smmu-v3.yaml           |  8 +++
 .../devicetree/bindings/iommu/arm,smmu.yaml   |  8 +++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   |  9 +++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 --
 drivers/iommu/arm/arm-smmu/arm-smmu.c         | 10 ++--
 drivers/iommu/virtio-iommu.c                  |  2 -
 include/linux/iommu.h                         | 52 +++++++++++++++++++
 7 files changed, 81 insertions(+), 11 deletions(-)

-- 
2.34.1

[PATCH v2 1/3] arm-smmu: move MSI_IOVA macro definitions

[Shyam Saini]

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 bd9d7c85576a..d1713f6bbe6d 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h
@@ -502,9 +502,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 8f439c265a23..a54dc4608c62 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 b85ce6310ddb..8c8783c8b31b 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 cf8c16ba04a0..8d38d85f23f1 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1530,6 +1530,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

[PATCH v2 2/3] dt-bindings: iommu: add "arm,smmu-faulty-msi-iova" property

[Shyam Saini]

By default ARM SMMU drivers use MSI_IOVA_BASE macro to reserve
PCI MSI IOVA memory range, this assumes that all the platforms have
MSI_IOVA_BASE address available for MSI reservation. However, this
is not always the case, as some platforms may have the default
address reserved for some other purposes and as a consequence ARM SMMU
drivers can't reserve MSI memory for those platforms.

To address this issue, add 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.

Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml | 8 ++++++++
 Documentation/devicetree/bindings/iommu/arm,smmu.yaml    | 8 ++++++++
 2 files changed, 16 insertions(+)

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
index 75fcf4cb52d9..13b615ca38e1 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
@@ -56,6 +56,13 @@ properties:
       NOTE: this only applies to the SMMU itself, not masters connected
       upstream of the SMMU.
 
+  arm,smmu-faulty-msi-iova:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specifies a faulty PCI MSI base I/O Virtual Address. If this is passed
+      via dts then arm SMMU driver appropriately select suitable MSI_IOVA base
+      which does not intersect with faulty MSI IOVA passed in this dts property.
+
   msi-parent: true
 
   hisilicon,broken-prefetch-cmd:
@@ -92,4 +99,5 @@ examples:
             dma-coherent;
             #iommu-cells = <1>;
             msi-parent = <&its 0xff0000>;
+            arm,smmu-faulty-msi-iova = <0x8000000>;
     };
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 7b9d5507d6cc..1aba9d59d495 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -208,6 +208,13 @@ properties:
       NOTE: this only applies to the SMMU itself, not masters connected
       upstream of the SMMU.
 
+  arm,smmu-faulty-msi-iova:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: |
+      Specifies a faulty PCI MSI base I/O Virtual Address. If this is passed
+      via dts then ARM SMMU driver appropriately select suitable MSI_IOVA base
+      which does not intersect with faulty MSI IOVA passed in this dts property.
+
   calxeda,smmu-secure-config-access:
     type: boolean
     description:
@@ -680,6 +687,7 @@ examples:
             #iommu-cells = <1>;
             /* always ignore appended 5-bit TBU number */
             stream-match-mask = <0x7c00>;
+            arm,smmu-faulty-msi-iova = <0x8000000>;
     };
 
     bus {
-- 
2.34.1

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

[Shyam Saini]

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 adding one more MSI_IOVA base address, so that if the
platforms can select suitable PCI MSI IOVA address if SMMU dts node
has "arm,smmu-faulty-msi-iova".

If this property is not found in the dtb for the given platform then
the driver falls back on the default MSI IOVA address.

Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c |  9 +++-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       |  9 +++-
 include/linux/iommu.h                       | 49 +++++++++++++++++++++
 3 files changed, 63 insertions(+), 4 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 358072b4e293..d6ea82ed4530 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -33,6 +33,8 @@
 #include "arm-smmu-v3.h"
 #include "../../dma-iommu.h"
 
+u32 msi_iova_base;
+
 static bool disable_msipolling;
 module_param(disable_msipolling, bool, 0444);
 MODULE_PARM_DESC(disable_msipolling,
@@ -3541,8 +3543,8 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	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);
+	region = iommu_alloc_resv_region(msi_iova_base, MSI_IOVA_LENGTH, prot,
+					 IOMMU_RESV_SW_MSI, GFP_KERNEL);
 	if (!region)
 		return;
 
@@ -4570,6 +4572,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	u32 cells;
 	int ret = -EINVAL;
+	u32 msi_iova_ptr;
+
+	iommu_configure_msi_iova(dev, "arm,smmu-faulty-msi-iova", msi_iova_ptr);
 
 	if (of_property_read_u32(dev->of_node, "#iommu-cells", &cells))
 		dev_err(dev, "missing #iommu-cells property\n");
diff --git a/drivers/iommu/arm/arm-smmu/arm-smmu.c b/drivers/iommu/arm/arm-smmu/arm-smmu.c
index a54dc4608c62..cd8bf2278057 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -50,6 +50,8 @@
  */
 #define QCOM_DUMMY_VAL -1
 
+u32 msi_iova_base;
+
 static int force_stage;
 module_param(force_stage, int, S_IRUGO);
 MODULE_PARM_DESC(force_stage,
@@ -1594,8 +1596,8 @@ static void arm_smmu_get_resv_regions(struct device *dev,
 	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);
+	region = iommu_alloc_resv_region(msi_iova_base, MSI_IOVA_LENGTH, prot,
+					 IOMMU_RESV_SW_MSI, GFP_KERNEL);
 	if (!region)
 		return;
 
@@ -2030,6 +2032,9 @@ static int arm_smmu_device_dt_probe(struct arm_smmu_device *smmu,
 	const struct arm_smmu_match_data *data;
 	struct device *dev = smmu->dev;
 	bool legacy_binding;
+	u32 *msi_iova_ptr = &msi_iova_base;
+
+	iommu_configure_msi_iova(dev, "arm,smmu-faulty-msi-iova", msi_iova_ptr);
 
 	if (of_property_read_u32(dev->of_node, "#global-interrupts", global_irqs))
 		return dev_err_probe(dev, -ENODEV,
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 8d38d85f23f1..cfd047fdf225 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1531,10 +1531,59 @@ 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 u32 select_msi_iova_base(u32 erratic_iova_addr)
+{
+	phys_addr_t start, end, msi_iova_end;
+
+	if (!erratic_iova_addr)
+		return MSI_IOVA_BASE;
+
+	start = erratic_iova_addr;
+	end = start + MSI_IOVA_LENGTH - 1;
+	msi_iova_end = MSI_IOVA_BASE + MSI_IOVA_LENGTH - 1;
+
+	/* return non-overlapping address */
+	return (start > MSI_IOVA_BASE ||
+			end < msi_iova_end) ? MSI_IOVA_BASE : MSI_IOVA_BASE2;
+}
+
+static inline void iommu_configure_msi_iova(struct device *iommu_dev,
+					    const char *faulty_msi_iova_prop,
+					    u32 *msi_iova)
+{
+	static bool is_msi_iova_selected;
+	u32 faulty_msi_iova_from_dt;
+	int rc;
+
+	rc = of_property_read_u32(iommu_dev->of_node, faulty_msi_iova_prop,
+				  &faulty_msi_iova_from_dt);
+	if (!is_msi_iova_selected) {
+		*msi_iova = select_msi_iova_base(rc ? 0 : faulty_msi_iova_from_dt);
+		dev_dbg(iommu_dev, "setting custom MSI IOVA base to 0x%x\n", *msi_iova);
+		is_msi_iova_selected = true;
+		return;
+	}
+
+	dev_dbg(iommu_dev, "custom MSI IOVA base already set to 0x%x\n", *msi_iova);
+}
+
 int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base);
 #else /* CONFIG_IOMMU_DMA */
+
+static inline u32 select_msi_iova_base(u32 erratic_iova_addr)
+{
+}
+
+static inline void iommu_configure_msi_iova(struct device *iommu_dev,
+					    const char *faulty_msi_iova_prop,
+					    u32 *msi_iova)
+{
+}
+
+
 static inline int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 {
 	return -ENODEV;
-- 
2.34.1

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

[Jason Gunthorpe]

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

Re: [PATCH v2 1/3] arm-smmu: move MSI_IOVA macro definitions

[kernel test robot]

Hi Shyam,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.15-rc1 next-20250411]
[cannot apply to soc/for-next]
[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/20250411-070014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250410225030.2528385-2-shyamsaini%40linux.microsoft.com
patch subject: [PATCH v2 1/3] arm-smmu: move MSI_IOVA macro definitions
config: arc-randconfig-001-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120700.1UwOUvMR-lkp@intel.com/config)
compiler: arc-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120700.1UwOUvMR-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/202504120700.1UwOUvMR-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iommu/arm/arm-smmu/arm-smmu.c: In function 'arm_smmu_get_resv_regions':
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1597:42: error: 'MSI_IOVA_BASE' undeclared (first use in this function); did you mean 'PCI_IO_BASE'?
    1597 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                          ^~~~~~~~~~~~~
         |                                          PCI_IO_BASE
   drivers/iommu/arm/arm-smmu/arm-smmu.c:1597:42: note: each undeclared identifier is reported only once for each function it appears in
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:1597:57: error: 'MSI_IOVA_LENGTH' undeclared (first use in this function)
    1597 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                                         ^~~~~~~~~~~~~~~


vim +/MSI_IOVA_LENGTH +1597 drivers/iommu/arm/arm-smmu/arm-smmu.c

021bb8420d44cf5 drivers/iommu/arm-smmu.c              Robin Murphy 2016-09-14  1590  
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1591  static void arm_smmu_get_resv_regions(struct device *dev,
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1592  				      struct list_head *head)
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1593  {
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1594  	struct iommu_resv_region *region;
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1595  	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1596  
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19 @1597  	region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
0251d0107cfb0bb drivers/iommu/arm/arm-smmu/arm-smmu.c Lu Baolu     2022-10-19  1598  					 prot, IOMMU_RESV_SW_MSI, GFP_KERNEL);
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1599  	if (!region)
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1600  		return;
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1601  
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1602  	list_add_tail(&region->list, head);
273df9635385b21 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1603  
273df9635385b21 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1604  	iommu_dma_get_resv_regions(dev, head);
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1605  }
f3ebee80b3131d4 drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1606  

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

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

[kernel test robot]

Hi Shyam,

kernel test robot noticed the following build warnings:

[auto build test WARNING on robh/for-next]
[also build test WARNING on linus/master v6.15-rc1 next-20250411]
[cannot apply to soc/for-next]
[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/20250411-070014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250410225030.2528385-4-shyamsaini%40linux.microsoft.com
patch subject: [PATCH v2 3/3] arm-smmu: select suitable MSI IOVA
config: arm-randconfig-002-20250412 (https://download.01.org/0day-ci/archive/20250412/202504120727.1ZmvnvO2-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 7.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504120727.1ZmvnvO2-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/202504120727.1ZmvnvO2-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from kernel/fork.c:103:0:
   include/linux/iommu.h: In function 'select_msi_iova_base':
>> include/linux/iommu.h:1573:1: warning: no return statement in function returning non-void [-Wreturn-type]
    }
    ^


vim +1573 include/linux/iommu.h

  1570	
  1571	static inline u32 select_msi_iova_base(u32 erratic_iova_addr)
  1572	{
> 1573	}
  1574	

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

Re: [PATCH v2 1/3] arm-smmu: move MSI_IOVA macro definitions

[kernel test robot]

Hi Shyam,

kernel test robot noticed the following build errors:

[auto build test ERROR on robh/for-next]
[also build test ERROR on linus/master v6.15-rc1 next-20250411]
[cannot apply to soc/for-next joro-iommu/next]
[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/20250411-070014
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
patch link:    https://lore.kernel.org/r/20250410225030.2528385-2-shyamsaini%40linux.microsoft.com
patch subject: [PATCH v2 1/3] arm-smmu: move MSI_IOVA macro definitions
config: arm-randconfig-001-20250412 (https://download.01.org/0day-ci/archive/20250412/202504121044.VSL6FbCC-lkp@intel.com/config)
compiler: clang version 21.0.0git (https://github.com/llvm/llvm-project f819f46284f2a79790038e1f6649172789734ae8)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250412/202504121044.VSL6FbCC-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/202504121044.VSL6FbCC-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/iommu/arm/arm-smmu/arm-smmu.c:1597:35: error: use of undeclared identifier 'MSI_IOVA_BASE'
    1597 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                          ^
>> drivers/iommu/arm/arm-smmu/arm-smmu.c:1597:50: error: use of undeclared identifier 'MSI_IOVA_LENGTH'
    1597 |         region = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
         |                                                         ^
   2 errors generated.


vim +/MSI_IOVA_LENGTH +1597 drivers/iommu/arm/arm-smmu/arm-smmu.c

021bb8420d44cf drivers/iommu/arm-smmu.c              Robin Murphy 2016-09-14  1590  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1591  static void arm_smmu_get_resv_regions(struct device *dev,
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1592  				      struct list_head *head)
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1593  {
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1594  	struct iommu_resv_region *region;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1595  	int prot = IOMMU_WRITE | IOMMU_NOEXEC | IOMMU_MMIO;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1596  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19 @1597  	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  1598  					 prot, IOMMU_RESV_SW_MSI, GFP_KERNEL);
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1599  	if (!region)
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1600  		return;
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1601  
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1602  	list_add_tail(&region->list, head);
273df9635385b2 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1603  
273df9635385b2 drivers/iommu/arm-smmu.c              Robin Murphy 2017-03-16  1604  	iommu_dma_get_resv_regions(dev, head);
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1605  }
f3ebee80b3131d drivers/iommu/arm-smmu.c              Eric Auger   2017-01-19  1606  

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

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

[Jacob Pan]

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

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

[Jason Gunthorpe]

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(&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

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 <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(&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

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

[Shyam Saini]

Hi Jason,

apologies for the delayed response.

> 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(&region->list, head);
> +	static const u64 msi_bases[] = { MSI_IOVA_BASE, 0x12340000 };
>  
>  	iommu_dma_get_resv_regions(dev, head);

my understand is, this hook is not called for all the devices, eg: pcie dts node
doesn't use [1] "iommus" property instead it uses "iommu-map" property
as a consequence, [1] while loop exits prematurely and iommu_dma_get_resv_regions()
is not called, so there is no IOVA reservation for the pcie device. 

Did i miss something? Please let me know.

[1] https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/of/device.c#L161

Thanks,
Shyam

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

[Jason Gunthorpe]

On Tue, May 20, 2025 at 03:42:24PM -0700, Shyam Saini wrote:
> Hi Jason,
> 
> apologies for the delayed response.
> 
> > 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(&region->list, head);
> > +	static const u64 msi_bases[] = { MSI_IOVA_BASE, 0x12340000 };
> >  
> >  	iommu_dma_get_resv_regions(dev, head);
> 
> my understand is, this hook is not called for all the devices, eg: pcie dts node
> doesn't use [1] "iommus" property instead it uses "iommu-map" property
> as a consequence, [1] while loop exits prematurely and iommu_dma_get_resv_regions()
> is not called, so there is no IOVA reservation for the pcie device. 

I can't really understand this sentance.

The above is the only place that creates a IOMMU_RESV_SW_MSI so it is
definately called and used, right? If not where does your
IOMMU_RESV_SW_MSI come from?

This function is also the only thing that computes the reserved ranges
that iommu_get_resv_regions() returns.

As above, I've asked a few times now if your resv_regions() is
correct, meaning there is a reserved range covering the address space
that doesn't have working translation. That means
iommu_get_resv_regions() returns such a range.

If you don't have that then you have a bigger platform problem, IMHO,
as vfio/iommufd only respect reserved ranges.

Otherwise, what is the issue you see, exactly? Did you even try it?

Jason

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

[Shyam Saini]

On Sun, May 25, 2025 at 04:07:03PM -0300, Jason Gunthorpe wrote:
> On Tue, May 20, 2025 at 03:42:24PM -0700, Shyam Saini wrote:
> > Hi Jason,
> > 
> > apologies for the delayed response.
> > 
> > > 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(&region->list, head);
> > > +	static const u64 msi_bases[] = { MSI_IOVA_BASE, 0x12340000 };
> > >  
> > >  	iommu_dma_get_resv_regions(dev, head);
> > 
> > my understand is, this hook is not called for all the devices, eg: pcie dts node
> > doesn't use [1] "iommus" property instead it uses "iommu-map" property
> > as a consequence, [1] while loop exits prematurely and iommu_dma_get_resv_regions()
> > is not called, so there is no IOVA reservation for the pcie device. 
> 
> I can't really understand this sentance.
> 
> The above is the only place that creates a IOMMU_RESV_SW_MSI so it is
> definately called and used, right? If not where does your
> IOMMU_RESV_SW_MSI come from?

code tracing and printks in that code path suggests iommu_dma_get_resv_regions()
called by vfio-pci driver, i didn't mention vfio-pci in my last reply since it
doesn't have an associated device tree node, sorry about that

By enabling this [1] dev_dbg message i get this:

vfio-pci 0000:01:00.2: device is behind an iommu

In case of 0000:01:00.2 device, when it invokes iommu_dma_get_resv_regions(),
code hit [2] this path

> 
> This function is also the only thing that computes the reserved ranges
> that iommu_get_resv_regions() returns.
> 
> As above, I've asked a few times now if your resv_regions() is
> correct, meaning there is a reserved range covering the address space
> that doesn't have working translation. That means
> iommu_get_resv_regions() returns such a range.

sorry about missing that, i see msi iova being reserved:

cat /sys/kernel/iommu_groups/*/reserved_regions
0x0000000008000000 0x00000000080fffff msi
0x0000000008000000 0x00000000080fffff msi
0x0000000008000000 0x00000000080fffff msi
0x0000000008000000 0x00000000080fffff msi
[output trimmed]

> 
> If you don't have that then you have a bigger platform problem, IMHO,
> as vfio/iommufd only respect reserved ranges.
> 
> Otherwise, what is the issue you see, exactly? Did you even try it?
> 

Yes, i tried that,

This is how my dts node looked like
reserved-memory {
               faulty_iova: resv_faulty {
                       iommu-addresses = <&pcieX 0x8000000 0x100000>;
               };
               ..
               ..
}

&pcieX {
    memory-region = <&faulty_iova>;
};

I see it working for the devices which are calling iommu_get_resv_regions(), eg if I
specify faulty_iova for dma controller dts node then i see an additional entry
in the related group, say Y: /sys/kernel/iommu_groups/Y/reserved_regions

Did i misunderstood? appreciate your help on this

Thanks,
Shyam

[1] https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/of/device.c#L170
[2] https://elixir.bootlin.com/linux/v6.15-rc7/source/drivers/iommu/of_iommu.c#L145

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

[Jason Gunthorpe]

On Tue, May 27, 2025 at 01:54:28PM -0700, Shyam Saini wrote:
> > The above is the only place that creates a IOMMU_RESV_SW_MSI so it is
> > definately called and used, right? If not where does your
> > IOMMU_RESV_SW_MSI come from?
> 
> code tracing and printks in that code path suggests iommu_dma_get_resv_regions()
> called by vfio-pci driver, 

Yes, I know it is, that is how it setups the SW_MSI.

> > As above, I've asked a few times now if your resv_regions() is
> > correct, meaning there is a reserved range covering the address space
> > that doesn't have working translation. That means
> > iommu_get_resv_regions() returns such a range.
> 
> sorry about missing that, i see msi iova being reserved:
> 
> cat /sys/kernel/iommu_groups/*/reserved_regions
> 0x0000000008000000 0x00000000080fffff msi
> 0x0000000008000000 0x00000000080fffff msi
> 0x0000000008000000 0x00000000080fffff msi
> 0x0000000008000000 0x00000000080fffff msi
> [output trimmed]

But this does not seem correct, you should have a "reserved" region
covering 0x8000000 as well because you say your platform cannot do DMA
to 0x8000000 and this is why you are doing all this.

All IOVA that the platform cannot DMA from should be reported in the
reserved_regions file as "reserved". You must make your platform
achieve this.

> Yes, i tried that,
> 
> This is how my dts node looked like
> reserved-memory {
>                faulty_iova: resv_faulty {
>                        iommu-addresses = <&pcieX 0x8000000 0x100000>;
>                };
>                ..
>                ..
> }
> 
> &pcieX {
>     memory-region = <&faulty_iova>;
> };
> 
> I see it working for the devices which are calling
> iommu_get_resv_regions(), eg if I specify faulty_iova for dma
> controller dts node then i see an additional entry in the related
> group

Exactly, it has to flow from the DT into the reserved_regions, that is
essential.

So what is the problem if you have figured out how to fix up
/sys/kernel/iommu_groups/Y/reserved_regions?

If you found some cases where you can't get /sys/../reserved_regions
to report the right things from the DT then that needs to be addressed
first before you think about fixing SW_MSI.

I very vaguely recall we have some gaps on OF where the DMA-API code
is understanding parts of the DT that don't get mapped into
reserved_regions and nobody has cared to fix it because it only
effects VFIO. You may have landed in the seat that has to fix it :)

But I still don't have a clear sense of what your actual problem is as
you are show DT that seems reasonable and saying that
/sys/../reserved_regions is working..

Jason

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

[Jacob Pan]

Hi Jason,

On Tue, 27 May 2025 21:04:25 -0300
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Tue, May 27, 2025 at 01:54:28PM -0700, Shyam Saini wrote:
> > > The above is the only place that creates a IOMMU_RESV_SW_MSI so
> > > it is definately called and used, right? If not where does your
> > > IOMMU_RESV_SW_MSI come from?  
> > 
> > code tracing and printks in that code path suggests
> > iommu_dma_get_resv_regions() called by vfio-pci driver,   
> 
> Yes, I know it is, that is how it setups the SW_MSI.
> 
> > > As above, I've asked a few times now if your resv_regions() is
> > > correct, meaning there is a reserved range covering the address
> > > space that doesn't have working translation. That means
> > > iommu_get_resv_regions() returns such a range.  
> > 
> > sorry about missing that, i see msi iova being reserved:
> > 
> > cat /sys/kernel/iommu_groups/*/reserved_regions
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > [output trimmed]  
> 
> But this does not seem correct, you should have a "reserved" region
> covering 0x8000000 as well because you say your platform cannot do DMA
> to 0x8000000 and this is why you are doing all this.
> 
> All IOVA that the platform cannot DMA from should be reported in the
> reserved_regions file as "reserved". You must make your platform
> achieve this.
> 
Just to double confirm, the expected reserved region should be marked as
"direct" instead of "msi", right?

> > Yes, i tried that,
> > 
> > This is how my dts node looked like
> > reserved-memory {
> >                faulty_iova: resv_faulty {
> >                        iommu-addresses = <&pcieX 0x8000000
> > 0x100000>; };
> >                ..
> >                ..
> > }
> > 
> > &pcieX {
> >     memory-region = <&faulty_iova>;
> > };
> > 
> > I see it working for the devices which are calling
> > iommu_get_resv_regions(), eg if I specify faulty_iova for dma
> > controller dts node then i see an additional entry in the related
> > group  
> 
> Exactly, it has to flow from the DT into the reserved_regions, that is
> essential.
> 
> So what is the problem if you have figured out how to fix up
> /sys/kernel/iommu_groups/Y/reserved_regions?
> 
> If you found some cases where you can't get /sys/../reserved_regions
> to report the right things from the DT then that needs to be addressed
> first before you think about fixing SW_MSI.
> 
> I very vaguely recall we have some gaps on OF where the DMA-API code
> is understanding parts of the DT that don't get mapped into
> reserved_regions and nobody has cared to fix it because it only
> effects VFIO. You may have landed in the seat that has to fix it :)
> 
> But I still don't have a clear sense of what your actual problem is as
> you are show DT that seems reasonable and saying that
> /sys/../reserved_regions is working..
> 
> Jason

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

[Jason Gunthorpe]

On Wed, May 28, 2025 at 03:42:55PM -0700, Jacob Pan wrote:

> > All IOVA that the platform cannot DMA from should be reported in the
> > reserved_regions file as "reserved". You must make your platform
> > achieve this.
> > 
> Just to double confirm, the expected reserved region should be marked as
> "direct" instead of "msi", right?

I don't know, it depends what is wrong with this platform.

"msi" is the SW_MSI region created by the iommu driver, ignore it.

"reserved" should be from the DT and it says "my platform is broken,
this IOVA doesn't work, don't use it"

"direct" is something entirely different, it also should come from the
DT and it says "must be mapped 1:1 (iova:physical) at all times" which
usually means some FW or other component is controlling the device and
doing DMA using that physical memory space which is unknown to the OS.

Both cases would interfere with the MSI so if the DT/ACPI declares
such regions that overlap with the SW_MSI address then Linux should
select another SW_MSI address.

From a linux perspective "reserved" just ignores those IOVA spaces
while "direct" sets up 1:1 translations there. So FW using direct
incorrectly could become a security problem.

Jason

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

[Shyam Saini]

On Tue, May 27, 2025 at 09:04:25PM -0300, Jason Gunthorpe wrote:
> On Tue, May 27, 2025 at 01:54:28PM -0700, Shyam Saini wrote:
> > > The above is the only place that creates a IOMMU_RESV_SW_MSI so it is
> > > definately called and used, right? If not where does your
> > > IOMMU_RESV_SW_MSI come from?
> > 
> > code tracing and printks in that code path suggests iommu_dma_get_resv_regions()
> > called by vfio-pci driver, 
> 
> Yes, I know it is, that is how it setups the SW_MSI.
> 
> > > As above, I've asked a few times now if your resv_regions() is
> > > correct, meaning there is a reserved range covering the address space
> > > that doesn't have working translation. That means
> > > iommu_get_resv_regions() returns such a range.
> > 
> > sorry about missing that, i see msi iova being reserved:
> > 
> > cat /sys/kernel/iommu_groups/*/reserved_regions
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > 0x0000000008000000 0x00000000080fffff msi
> > [output trimmed]
> 
> But this does not seem correct, you should have a "reserved" region
> covering 0x8000000 as well because you say your platform cannot do DMA
> to 0x8000000 and this is why you are doing all this.
> 
> All IOVA that the platform cannot DMA from should be reported in the
> reserved_regions file as "reserved". You must make your platform
> achieve this.

so should it be for all the iommu groups?

                no_dma_mem {
                       reg = <0x0 0x8000000 0x0 0x100000>;
                        no-map;
                };
 
i think that's how we reserve memory in general, eg: ramoops
but this doesn't show up in:
  /sys/kernel/iommu_groups/*/reserved_regions

 
> > Yes, i tried that,
> > 
> > This is how my dts node looked like
> > reserved-memory {
> >                faulty_iova: resv_faulty {
> >                        iommu-addresses = <&pcieX 0x8000000 0x100000>;
> >                };
> >                ..
> >                ..
> > }
> > 
> > &pcieX {
> >     memory-region = <&faulty_iova>;
> > };
> > 
> > I see it working for the devices which are calling
> > iommu_get_resv_regions(), eg if I specify faulty_iova for dma
> > controller dts node then i see an additional entry in the related
> > group
> 
> Exactly, it has to flow from the DT into the reserved_regions, that is
> essential.
 
> So what is the problem if you have figured out how to fix up
> /sys/kernel/iommu_groups/Y/reserved_regions?

sorry, i haven't yet
 
> If you found some cases where you can't get /sys/../reserved_regions
> to report the right things from the DT then that needs to be addressed
> first before you think about fixing SW_MSI.
> 
> I very vaguely recall we have some gaps on OF where the DMA-API code
> is understanding parts of the DT that don't get mapped into
> reserved_regions and nobody has cared to fix it because it only
> effects VFIO. You may have landed in the seat that has to fix it :)

I think this is the case we are dealing with?

> But I still don't have a clear sense of what your actual problem is as
> you are show DT that seems reasonable and saying that
> /sys/../reserved_regions is working..

/sys/../reserved_regions is working for certain devices like dma controller
but it doesn't work for pcie devices and its vfio-pcie driver calling
iommu_get_resv_regions() but we don't have dts node for vfio.
I have confirmed this about pcie on two different platforms, it seems to be
OF DMA-API gap that you hinted above, happy to work on that :), it would be
great if you can share any other reference discussions to that problem

When i specify this for dma controller:

		faulty_iova: resv_faulty {
			iommu-addresses = <&dmaX 0x8000000 0x100000>;
		};
&dmaX {
	memory-region = <&faulty_iova>;
};

I see following:
$ cat /sys/kernel/iommu_groups/y/reserved_regions 
0x0000000008000000 0x00000000080fffff reserved
0x00000000a0000000 0x00000000a00fffff msi

Clarifying the Issue with MSI and SMMU Faults on Our Platform:

We are encountering SMMU faults when using our userspace tool/driver that
relies on MSI. Specifically, the issue arises when the MSI_IOVA_BASE is set
to the current default value of 0x08000000.

The observed fault is as follows:

arm-smmu 64000000.iommu: Unhandled context fault: fsr=0x402, iova=0x00000040,
fsynr=0x2f0013, cbfrsynra=0x102, cb=15

Upon investigation, our hardware team confirmed that the memory region
containing 0x08000000 is already mapped for other peripherals, making it
unavailable for MSI usage.

eg: using 0xa0000000 as MSI_IOVA_BASE solves our problem.

let me know if you have any other questions

Thanks,
Shyam

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

[Jason Gunthorpe]

On Thu, May 29, 2025 at 11:22:19AM -0700, Shyam Saini wrote:
> > All IOVA that the platform cannot DMA from should be reported in the
> > reserved_regions file as "reserved". You must make your platform
> > achieve this.
> 
> so should it be for all the iommu groups?
> 
>                 no_dma_mem {
>                        reg = <0x0 0x8000000 0x0 0x100000>;
>                         no-map;
>                 };
>  
> i think that's how we reserve memory in general, eg: ramoops
> but this doesn't show up in:
>   /sys/kernel/iommu_groups/*/reserved_regions

I don't know anymore, I've forgotten these details about DT, you will
need to consult with DT experts..

> > But I still don't have a clear sense of what your actual problem is as
> > you are show DT that seems reasonable and saying that
> > /sys/../reserved_regions is working..
> 
> /sys/../reserved_regions is working for certain devices like dma controller
> but it doesn't work for pcie devices and its vfio-pcie driver calling
> iommu_get_resv_regions() but we don't have dts node for vfio.
> I have confirmed this about pcie on two different platforms, it seems to be
> OF DMA-API gap that you hinted above, happy to work on that :), it would be
> great if you can share any other reference discussions to that problem

Some of it was around the dma_mask and the ranges, but I'm not sure
exactly what is your issue here..

It is very important the the dma-iommu.c code also knows not to use
the non-working IOVA or your system will be buggy and eventually fail.
If that isn't happening you have a critical bug beyond the SW_MSI
issue.

The easiest way to fix it is to have reserved_regions report the right
stuff.

> When i specify this for dma controller:
> 
> 		faulty_iova: resv_faulty {
> 			iommu-addresses = <&dmaX 0x8000000 0x100000>;
> 		};
> &dmaX {
> 	memory-region = <&faulty_iova>;
> };
> 
> I see following:
> $ cat /sys/kernel/iommu_groups/y/reserved_regions 
> 0x0000000008000000 0x00000000080fffff reserved
> 0x00000000a0000000 0x00000000a00fffff msi

See, that looks better. You need to make that happen for all the
effected devices.

> Upon investigation, our hardware team confirmed that the memory region
> containing 0x08000000 is already mapped for other peripherals, making it
> unavailable for MSI usage.

See, that's just wrong HW design. The path from PCI to the SMMU should
not have any other decodes on it, isn't that SBSA?

How does your platform work at all? Isn't 0x08000000 physical memory in
your address map?

Jason

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

[Shyam Saini]

On Thu, May 29, 2025 at 03:38:15PM -0300, Jason Gunthorpe wrote:
> On Thu, May 29, 2025 at 11:22:19AM -0700, Shyam Saini wrote:
> > > All IOVA that the platform cannot DMA from should be reported in the
> > > reserved_regions file as "reserved". You must make your platform
> > > achieve this.
> > 
> > so should it be for all the iommu groups?
> > 
> >                 no_dma_mem {
> >                        reg = <0x0 0x8000000 0x0 0x100000>;
> >                         no-map;
> >                 };
> >  
> > i think that's how we reserve memory in general, eg: ramoops
> > but this doesn't show up in:
> >   /sys/kernel/iommu_groups/*/reserved_regions
> 
> I don't know anymore, I've forgotten these details about DT, you will
> need to consult with DT experts..

no worries, added maintainers and the list

> > > But I still don't have a clear sense of what your actual problem is as
> > > you are show DT that seems reasonable and saying that
> > > /sys/../reserved_regions is working..
> > 
> > /sys/../reserved_regions is working for certain devices like dma controller
> > but it doesn't work for pcie devices and its vfio-pcie driver calling
> > iommu_get_resv_regions() but we don't have dts node for vfio.
> > I have confirmed this about pcie on two different platforms, it seems to be
> > OF DMA-API gap that you hinted above, happy to work on that :), it would be
> > great if you can share any other reference discussions to that problem
> 
> Some of it was around the dma_mask and the ranges, but I'm not sure
> exactly what is your issue here..
> 
> It is very important the the dma-iommu.c code also knows not to use
> the non-working IOVA or your system will be buggy and eventually fail.
> If that isn't happening you have a critical bug beyond the SW_MSI
> issue.

thanks, i will look into this

> The easiest way to fix it is to have reserved_regions report the right
> stuff.
> 
> > When i specify this for dma controller:
> > 
> > 		faulty_iova: resv_faulty {
> > 			iommu-addresses = <&dmaX 0x8000000 0x100000>;
> > 		};
> > &dmaX {
> > 	memory-region = <&faulty_iova>;
> > };
> > 
> > I see following:
> > $ cat /sys/kernel/iommu_groups/y/reserved_regions 
> > 0x0000000008000000 0x00000000080fffff reserved
> > 0x00000000a0000000 0x00000000a00fffff msi
> 
> See, that looks better. You need to make that happen for all the
> effected devices.
> 
> > Upon investigation, our hardware team confirmed that the memory region
> > containing 0x08000000 is already mapped for other peripherals, making it
> > unavailable for MSI usage.
> 
> See, that's just wrong HW design. The path from PCI to the SMMU should
> not have any other decodes on it, isn't that SBSA?

I am not 100% sure

> How does your platform work at all? Isn't 0x08000000 physical memory in
> your address map? 

unfornately, this 0x08000000 physical address is not in our address map

Thanks,
Shyam

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

[Jason Gunthorpe]

On Thu, May 29, 2025 at 03:08:39PM -0700, Shyam Saini wrote:
> > How does your platform work at all? Isn't 0x08000000 physical memory in
> > your address map? 
> 
> unfornately, this 0x08000000 physical address is not in our address map

Is your issue PCI related, do you have a PCI switch without ACS mapped
here?

Jason

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

[Shyam Saini]

Hi Jason,

On Fri, May 30, 2025 at 10:13:28AM -0300, Jason Gunthorpe wrote:
> On Thu, May 29, 2025 at 03:08:39PM -0700, Shyam Saini wrote:
> > > How does your platform work at all? Isn't 0x08000000 physical memory in
> > > your address map? 
> > 
> > unfornately, this 0x08000000 physical address is not in our address map
> 
> Is your issue PCI related, do you have a PCI switch without ACS mapped
> here?

this platform that we are discussing doesn't have ACS caps, it relies on pci
ACS quirk.

Thanks,
Shyam