[PATCH 0/3] make MSI IOVA base address and its length configurable

[PATCH 0/3] make MSI IOVA base address and its length configurable

[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. In the previous attempt,
Will suggested reserving the MSI IOVA at runtime whenever there is a
conflict with the default MSI_IOVA_BASE. However, dynamically reserving
this address has debuggability concerns, as it becomes difficult to
track IOMMU mapping failures.

This patch series aims to address the issue by introducing a new DTS
property, "arm,smmu-pci-msi-iova-data". This property allows the
configuration of MSI IOVA with a custom MSI base address and a custom
length for IOMMU/SMMU drivers. It 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

Shyam Saini (3):
  dt-bindings: iommu: add "arm,smmu-pci-msi-iova-data" property
  iommu: consolidate MSI_IOVA macro definitions
  arm-smmu: use dts passed MSI IOVA address and length

 .../bindings/iommu/arm,smmu-v3.yaml           | 12 +++++
 .../devicetree/bindings/iommu/arm,smmu.yaml   | 12 +++++
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c   | 10 ++++-
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.h   |  3 --
 drivers/iommu/arm/arm-smmu/arm-smmu.c         | 11 +++--
 drivers/iommu/virtio-iommu.c                  |  8 ++--
 include/linux/iommu.h                         | 44 +++++++++++++++++++
 7 files changed, 86 insertions(+), 14 deletions(-)

-- 
2.34.1

[PATCH 3/3] arm-smmu: use dts passed MSI IOVA address and length

[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 passing PCI MSI IOVA address via dts property
"arm,smmu-pci-msi-iova-data". Likewise this property can be
used to set custom MSI IOVA address length, by default it should be
set to MSI IOVA default length value i.e, 0x100000.

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

Since this change allows configuration of custom MSI IOVA base
address and length, rename existing as MSI_IOVA* macros accordingly.

Signed-off-by: Shyam Saini <shyamsaini@linux.microsoft.com>
---
 drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c | 10 ++++-
 drivers/iommu/arm/arm-smmu/arm-smmu.c       | 10 ++++-
 drivers/iommu/virtio-iommu.c                |  6 +--
 include/linux/iommu.h                       | 45 ++++++++++++++++++++-
 4 files changed, 62 insertions(+), 9 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 0e4cbb2c64d73..91e88025424f9 100644
--- a/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
+++ b/drivers/iommu/arm/arm-smmu-v3/arm-smmu-v3.c
@@ -32,6 +32,8 @@
 #include "arm-smmu-v3.h"
 #include "../../dma-iommu.h"
 
+struct msi_iova_data msi_data_smmu_v3;
+
 static bool disable_msipolling;
 module_param(disable_msipolling, bool, 0444);
 MODULE_PARM_DESC(disable_msipolling,
@@ -3540,8 +3542,9 @@ 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_data_smmu_v3.msi_iova_base,
+					 msi_data_smmu_v3.msi_iova_length, prot,
+					 IOMMU_RESV_SW_MSI, GFP_KERNEL);
 	if (!region)
 		return;
 
@@ -4569,6 +4572,9 @@ static int arm_smmu_device_dt_probe(struct platform_device *pdev,
 	struct device *dev = &pdev->dev;
 	u32 cells;
 	int ret = -EINVAL;
+	struct msi_iova_data *msi_data_ptr = &msi_data_smmu_v3;
+
+	iommu_configure_msi_iova(dev, "arm,smmu-pci-msi-iova-data", msi_data_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 287f8e8d25890..b636ea302cee0 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -49,6 +49,8 @@
  */
 #define QCOM_DUMMY_VAL -1
 
+struct msi_iova_data msi_data;
+
 static int force_stage;
 module_param(force_stage, int, S_IRUGO);
 MODULE_PARM_DESC(force_stage,
@@ -1593,8 +1595,9 @@ 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_data.msi_iova_base,
+					 msi_data.msi_iova_length, prot,
+					 IOMMU_RESV_SW_MSI, GFP_KERNEL);
 	if (!region)
 		return;
 
@@ -2029,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;
+	struct msi_iova_data *msi_data_ptr = &msi_data;
+
+	iommu_configure_msi_iova(dev, "arm,smmu-pci-msi-iova-data", msi_data_ptr);
 
 	if (of_property_read_u32(dev->of_node, "#global-interrupts", global_irqs))
 		return dev_err_probe(dev, -ENODEV,
diff --git a/drivers/iommu/virtio-iommu.c b/drivers/iommu/virtio-iommu.c
index 8c8783c8b31be..ca50dee09f352 100644
--- a/drivers/iommu/virtio-iommu.c
+++ b/drivers/iommu/virtio-iommu.c
@@ -958,9 +958,9 @@ static void viommu_get_resv_regions(struct device *dev, struct list_head *head)
 	 * software-mapped region.
 	 */
 	if (!msi) {
-		msi = iommu_alloc_resv_region(MSI_IOVA_BASE, MSI_IOVA_LENGTH,
-					      prot, IOMMU_RESV_SW_MSI,
-					      GFP_KERNEL);
+		msi = iommu_alloc_resv_region(MSI_IOVA_BASE_DEFAULT,
+					      MSI_IOVA_LENGTH_DEFAULT, prot,
+					      IOMMU_RESV_SW_MSI, GFP_KERNEL);
 		if (!msi)
 			return;
 
diff --git a/include/linux/iommu.h b/include/linux/iommu.h
index 2a26d3e18b24e..412a89200f094 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1505,8 +1505,43 @@ static inline void iommu_debugfs_setup(void) {}
 #ifdef CONFIG_IOMMU_DMA
 #include <linux/msi.h>
 
-#define MSI_IOVA_BASE		0x8000000
-#define MSI_IOVA_LENGTH		0x100000
+/* MSI_IOVA_BASE_DEFAULT address can be overridden by dts specified address */
+#define MSI_IOVA_BASE_DEFAULT		0x8000000
+#define MSI_IOVA_LENGTH_DEFAULT		0x100000
+
+struct msi_iova_data {
+	u32 msi_iova_base;
+	u32 msi_iova_length;
+};
+
+static inline void iommu_configure_msi_iova(struct device *iommu_dev,
+					    const char *msi_iova_prop,
+					    struct msi_iova_data *msi_data)
+{
+	static bool is_msi_base_set_from_dt;
+	u32 msi_data_from_dt[2];
+	int rc;
+
+	rc = of_property_read_u32_array(iommu_dev->of_node, msi_iova_prop,
+					msi_data_from_dt, 2);
+	if (!rc && !is_msi_base_set_from_dt) {
+		msi_data->msi_iova_base = msi_data_from_dt[0];
+		msi_data->msi_iova_length = msi_data_from_dt[1];
+		dev_info(iommu_dev, "setting custom MSI IOVA base to 0x%x\n",
+			 msi_data->msi_iova_base);
+		is_msi_base_set_from_dt = true;
+	} else if (is_msi_base_set_from_dt &&
+		msi_data->msi_iova_base != msi_data_from_dt[0])
+		dev_warn(iommu_dev, "custom MSI IOVA base already set to 0x%x,"
+			 " skip resetting it to 0x%x\n",
+			 msi_data->msi_iova_base, msi_data_from_dt[0]);
+	else if (!is_msi_base_set_from_dt && rc == -EINVAL) {
+		msi_data->msi_iova_base = MSI_IOVA_BASE_DEFAULT;
+		msi_data->msi_iova_length =  MSI_IOVA_LENGTH_DEFAULT;
+		dev_info(iommu_dev, "using default MSI IOVA base: 0x%x\n",
+			 msi_data->msi_iova_base);
+	}
+}
 
 int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base);
 
@@ -1518,6 +1553,12 @@ void iommu_dma_compose_msi_msg(struct msi_desc *desc, struct msi_msg *msg);
 struct msi_desc;
 struct msi_msg;
 
+static inline void iommu_configure_msi_iova(struct device *iommu_dev,
+					    const char *msi_iova_prop,
+					    u32 (*msi_addr)[2])
+{
+}
+
 static inline int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base)
 {
 	return -ENODEV;
-- 
2.34.1

[PATCH 2/3] iommu: consolidate MSI_IOVA macro definitions

[Shyam Saini]

MSI_IOVA* macros are common among different iommu/smu drivers,
so move them to common iommu.h header file.
This doesn't change anything wrt functionality of the IOMMU subsystem.

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 bd9d7c85576a2..d1713f6bbe6d1 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 79afc92e1d8b9..287f8e8d25890 100644
--- a/drivers/iommu/arm/arm-smmu/arm-smmu.c
+++ b/drivers/iommu/arm/arm-smmu/arm-smmu.c
@@ -49,9 +49,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 b85ce6310ddbd..8c8783c8b31be 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 38c65e92ecd09..2a26d3e18b24e 100644
--- a/include/linux/iommu.h
+++ b/include/linux/iommu.h
@@ -1505,6 +1505,9 @@ static inline void iommu_debugfs_setup(void) {}
 #ifdef CONFIG_IOMMU_DMA
 #include <linux/msi.h>
 
+#define MSI_IOVA_BASE		0x8000000
+#define MSI_IOVA_LENGTH		0x100000
+
 int iommu_get_msi_cookie(struct iommu_domain *domain, dma_addr_t base);
 
 int iommu_dma_prepare_msi(struct msi_desc *desc, phys_addr_t msi_addr);
-- 
2.34.1

[PATCH 1/3] dt-bindings: iommu: add "arm,smmu-pci-msi-iova-data" 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-pci-msi-iova-data" which can be used to hold custom
PCI MSI IOVA address and its address length. This property can be passed
to ARM SMMU drivers via device tree to reserve specified memory range
for MSI.

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

diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
index 75fcf4cb52d9f..dbad612886604 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu-v3.yaml
@@ -56,6 +56,17 @@ properties:
       NOTE: this only applies to the SMMU itself, not masters connected
       upstream of the SMMU.
 
+  arm,smmu-pci-msi-iova-data:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      Specifies a custom PCI MSI base I/O Virtual Address and its memory range
+      size for ARM SMMU drivers. This allows setting a custom address and
+      memory size pair if the default MSI_IOVA_BASE_DEFAULT address and
+      MSI_IOVA_LENGTH_DEFAULT size are not suitable for the intended platform.
+    items:
+      - description: MSI IOVA base address
+      - description: MSI IOVA address length
+
   msi-parent: true
 
   hisilicon,broken-prefetch-cmd:
@@ -92,4 +103,5 @@ examples:
             dma-coherent;
             #iommu-cells = <1>;
             msi-parent = <&its 0xff0000>;
+            arm,smmu-pci-msi-iova-data = <0xa0000000 0x100000>;
     };
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
index 032fdc27127bf..d080b13765d1f 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.yaml
@@ -207,6 +207,17 @@ properties:
       NOTE: this only applies to the SMMU itself, not masters connected
       upstream of the SMMU.
 
+  arm,smmu-pci-msi-iova-data:
+    $ref: /schemas/types.yaml#/definitions/uint32-array
+    description: |
+      Specifies a custom PCI MSI base I/O Virtual Address and its memory range
+      size for ARM SMMU drivers. This allows setting a custom address and
+      memory size pair if the default MSI_IOVA_BASE_DEFAULT address and
+      MSI_IOVA_LENGTH_DEFAULT size are not suitable for the intended platform.
+    items:
+      - description: MSI IOVA base address
+      - description: MSI IOVA address length
+
   calxeda,smmu-secure-config-access:
     type: boolean
     description:
@@ -679,6 +690,7 @@ examples:
             #iommu-cells = <1>;
             /* always ignore appended 5-bit TBU number */
             stream-match-mask = <0x7c00>;
+            arm,smmu-pci-msi-iova-data = <0xa0000000 0x100000>;
     };
 
     bus {
-- 
2.34.1

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Jason Gunthorpe]

On Thu, Jan 16, 2025 at 03:23:04PM -0800, 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.

Can you explain this some more? This address is in the kernel
controlled IOVA space, there are few ways a platform can impact this.

How is the platform impacting it? Is the non-functional IOVA always
reflected in the iommu_get_resv_regions()?

Why not avoid this conflict in your platform software?

> There was an [1] attempt to fix this problem by passing the MSI IOVA
> base as a kernel command line parameter. 

Yuk

> In the previous attempt,
> Will suggested reserving the MSI IOVA at runtime whenever there is a
> conflict with the default MSI_IOVA_BASE. However, dynamically reserving
> this address has debuggability concerns, as it becomes difficult to
> track IOMMU mapping failures.

Still, this approach seems like the best to me..

> This patch series aims to address the issue by introducing a new DTS
> property, "arm,smmu-pci-msi-iova-data". This property allows the
> configuration of MSI IOVA with a custom MSI base address and a custom
> length for IOMMU/SMMU drivers. It accommodates platforms that do not
> have the default MSI base address available for MSI reservation.

My understand was using DT to set kernel configurables was frowned
upon? Ultimately MSI_IOVA_BASE is an arbitary choice by kernel
software.

Jason

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Jacob Pan]

Hi Jason,

On Mon, 20 Jan 2025 10:26:43 -0400
Jason Gunthorpe <jgg@ziepe.ca> wrote:

> On Thu, Jan 16, 2025 at 03:23:04PM -0800, 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.  
> 
> Can you explain this some more? This address is in the kernel
> controlled IOVA space, there are few ways a platform can impact this.
> 
> How is the platform impacting it? Is the non-functional IOVA always
> reflected in the iommu_get_resv_regions()?

I don't know the platform impact but just to clarify, are you asking
whether this non-functional IOVA is also under IORT RMR or other FW
tables? I don't think it is.

But this special IOVA is reflected in iommu_get_resv_regions() the same
way as the hardcoded MSI_IOVA_BASE. So each iommu group's
reserved_regions should show.

> Why not avoid this conflict in your platform software?
I had the same question but it seems there is not enough difference
(than the standard smmu) to justify a platform code. i.e. platform
specific iommu_get_resv_regions(), is that what you are suggesting?

> > There was an [1] attempt to fix this problem by passing the MSI IOVA
> > base as a kernel command line parameter.   
> 
> Yuk
> 
> > In the previous attempt,
> > Will suggested reserving the MSI IOVA at runtime whenever there is a
> > conflict with the default MSI_IOVA_BASE. However, dynamically
> > reserving this address has debuggability concerns, as it becomes
> > difficult to track IOMMU mapping failures.  
> 
> Still, this approach seems like the best to me..
> 
> > This patch series aims to address the issue by introducing a new DTS
> > property, "arm,smmu-pci-msi-iova-data". This property allows the
> > configuration of MSI IOVA with a custom MSI base address and a
> > custom length for IOMMU/SMMU drivers. It accommodates platforms
> > that do not have the default MSI base address available for MSI
> > reservation.  
> 
> My understand was using DT to set kernel configurables was frowned
> upon? Ultimately MSI_IOVA_BASE is an arbitary choice by kernel
> software.
> 
> Jason

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Jason Gunthorpe]

On Tue, Jan 21, 2025 at 01:49:10PM -0800, Jacob Pan wrote:

> > On Thu, Jan 16, 2025 at 03:23:04PM -0800, 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.  
> > 
> > Can you explain this some more? This address is in the kernel
> > controlled IOVA space, there are few ways a platform can impact this.
> > 
> > How is the platform impacting it? Is the non-functional IOVA always
> > reflected in the iommu_get_resv_regions()?
> 
> I don't know the platform impact but just to clarify, are you asking
> whether this non-functional IOVA is also under IORT RMR or other FW
> tables? I don't think it is.

No, I'm asking how can you possibly have a HW platform where
MSI_IOVA_BASE is unable to be used for DMA?

MSI_IOVA_BASE is 128M, and most ARM platforms put DRAM starting at
0. Most ARM VMMs put DRAM starting at 0 too.

So a platform saying that DMA to 128M doesn't work is pretty broken,
to the point it is hard to believe there is a HW issue at work here?

> But this special IOVA is reflected in iommu_get_resv_regions() the same
> way as the hardcoded MSI_IOVA_BASE. So each iommu group's
> reserved_regions should show.

That's great

> > Why not avoid this conflict in your platform software?
> I had the same question but it seems there is not enough difference
> (than the standard smmu) to justify a platform code. i.e. platform
> specific iommu_get_resv_regions(), is that what you are suggesting?

And here I mean, why not stop marking it reserved in the ACPI/DT
inside your firwmare or hypervisor?

This smells like some SW component using the same address Linux uses
for some odd purpose. Just change it and let Linux keep using the
address it wants?

Jason

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Shyam Saini]

Hi Jason,

Apologies for delayed reponse,

> On Tue, Jan 21, 2025 at 01:49:10PM -0800, Jacob Pan wrote:
> 
> > > On Thu, Jan 16, 2025 at 03:23:04PM -0800, 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.  
> > > 
> > > Can you explain this some more? This address is in the kernel
> > > controlled IOVA space, there are few ways a platform can impact this.
> > > 
> > > How is the platform impacting it? Is the non-functional IOVA always
> > > reflected in the iommu_get_resv_regions()?
> > 
> > I don't know the platform impact but just to clarify, are you asking
> > whether this non-functional IOVA is also under IORT RMR or other FW
> > tables? I don't think it is.
> 
> No, I'm asking how can you possibly have a HW platform where
> MSI_IOVA_BASE is unable to be used for DMA?
> 
> MSI_IOVA_BASE is 128M, and most ARM platforms put DRAM starting at
> 0. Most ARM VMMs put DRAM starting at 0 too.
> 
> So a platform saying that DMA to 128M doesn't work is pretty broken,
> to the point it is hard to believe there is a HW issue at work here?

Correct, this is limitation with our HW.
Since we can't fix it in hardware, we would need to fix it in Linux.

> > But this special IOVA is reflected in iommu_get_resv_regions() the same
> > way as the hardcoded MSI_IOVA_BASE. So each iommu group's
> > reserved_regions should show.
> 
> That's great
> 
> > > Why not avoid this conflict in your platform software?
> > I had the same question but it seems there is not enough difference
> > (than the standard smmu) to justify a platform code. i.e. platform
> > specific iommu_get_resv_regions(), is that what you are suggesting?
> 
> And here I mean, why not stop marking it reserved in the ACPI/DT
> inside your firwmare or hypervisor?

even if we reserve it in dts we would still need some address reserved for MSI IOVA

> This smells like some SW component using the same address Linux uses
> for some odd purpose. Just change it and let Linux keep using the
> address it wants?

Unfortunately, it is an HW issue.

Are you okay with this passing custom MSI_IOVA via DTS approach ?

Thanks,
Shyam

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Jason Gunthorpe]

On Thu, Jan 30, 2025 at 03:21:37PM -0800, Shyam Saini wrote:

> Unfortunately, it is an HW issue.

Well, that's pretty bad to have built HW that can't DMA to low
addresses at all.. But OK.
 
> Are you okay with this passing custom MSI_IOVA via DTS approach ?

It isn't up to me, but I've understood the DT maintainers would reject
this as it isn't is describing HW but just a random Linux software
knob.

I think you should make selecting the sw_msi dynamic in Linux.

Jason

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Shyam Saini]

Hi Jason,

> On Thu, Jan 30, 2025 at 03:21:37PM -0800, Shyam Saini wrote:
> 
> > Unfortunately, it is an HW issue.
> 
> Well, that's pretty bad to have built HW that can't DMA to low
> addresses at all.. But OK.
>  
> > Are you okay with this passing custom MSI_IOVA via DTS approach ?
> 
> It isn't up to me, but I've understood the DT maintainers would reject
> this as it isn't is describing HW but just a random Linux software
> knob.
> 

If i understood correctly MSI window IOVA is hw property, if yes then it should be accepted
by DTS folks, did i misundertstand that?

> I think you should make selecting the sw_msi dynamic in Linux.

My understanding is that if we have to make it dynamic, we have to use iova allocator
that would need iova_domain as a member of struct arm_smmu_domain,
and allocate iova for MSI window dynamically using alloc_iova() in arm_smmu_get_resv_regions()
is that what you meant when you mentioned selecting the sw_msi dynamically?

later we still have to [1] re-adjust the final iova list to IOVA range - resereved regions (msi)
[1] https://elixir.bootlin.com/linux/v6.13.1/source/drivers/vfio/vfio_iommu_type1.c#L2232

so either static or dynamic, we seems to need MSI allocation

Please let me know what you think about this?

Thanks,
Shyam

Re: [PATCH 0/3] make MSI IOVA base address and its length configurable

[Jason Gunthorpe]

On Thu, Apr 03, 2025 at 12:34:58PM -0700, Shyam Saini wrote:
> Hi Jason,
> 
> > On Thu, Jan 30, 2025 at 03:21:37PM -0800, Shyam Saini wrote:
> > 
> > > Unfortunately, it is an HW issue.
> > 
> > Well, that's pretty bad to have built HW that can't DMA to low
> > addresses at all.. But OK.
> >  
> > > Are you okay with this passing custom MSI_IOVA via DTS approach ?
> > 
> > It isn't up to me, but I've understood the DT maintainers would reject
> > this as it isn't is describing HW but just a random Linux software
> > knob.
> > 
> 
> If i understood correctly MSI window IOVA is hw property, if yes
> then it should be accepted by DTS folks, did i misundertstand that?

It is not a HW property. The MSI window in the SMMU is entirely up to
software.

Yuor HW proprerty is the regions of IOVA that do not function in the
SMMU.

> > I think you should make selecting the sw_msi dynamic in Linux.
> 
> My understanding is that if we have to make it dynamic, we have to
> use iova allocator that would need iova_domain as a member of struct
> arm_smmu_domain, and allocate iova for MSI window dynamically using
> alloc_iova() in arm_smmu_get_resv_regions() is that what you meant
> when you mentioned selecting the sw_msi dynamically?

I would not structure it like that..

The simplest thing would be to have the SMMU driver have a list of
potential addresses and select the first one that does not intersect
with the non-working IOVA ranges.

Jason