[PATCH v2] hw/pci/pci: Enforce pci_setup_iommu_per_bus() is called only once per bus

Eric Auger posted 1 patch 1 month, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260603124415.1120808-1-eric.auger@redhat.com
Maintainers: Eric Auger <eric.auger@redhat.com>, Peter Maydell <peter.maydell@linaro.org>, "Michael S. Tsirkin" <mst@redhat.com>
include/hw/pci/pci.h | 16 +++++++++++++++-
hw/arm/smmu-common.c |  4 +++-
hw/pci/pci.c         |  9 +++++++--
3 files changed, 25 insertions(+), 4 deletions(-)
[PATCH v2] hw/pci/pci: Enforce pci_setup_iommu_per_bus() is called only once per bus
Posted by Eric Auger 1 month, 1 week ago
Currently it is possible to attach several arm-smmuv3 devices to the
same bus although it is a wrong setup.

Change the prototype of pci_setup_iommu_per_bus to pass an error
handle. This latter is set when iommu_per_bus is already set and
used by the single caller (smmu_base_realize) to report a useful
error to the end-user.

While at it document pci_setup_iommu_per_bus callback in the header.

Fixes: 66d2f665e163 ("hw/arm/virt: Allow user-creatable SMMUv3 dev instantiation")
Signed-off-by: Eric Auger <eric.auger@redhat.com>
Tested-by: Nathan Chen <nathanc@nvidia.com>
Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

---

v1 -> v2:
- in smmu_base_realize, return if pci_setup_iommu_per_bus returns false
  (Philippe)
---
 include/hw/pci/pci.h | 16 +++++++++++++++-
 hw/arm/smmu-common.c |  4 +++-
 hw/pci/pci.c         |  9 +++++++--
 3 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index 5b179091de..f2448e941a 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -863,7 +863,21 @@ int pci_iommu_unregister_iotlb_notifier(PCIDevice *dev, uint32_t pasid,
  */
 void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *ops, void *opaque);
 
-void pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops, void *opaque);
+/**
+ * pci_setup_iommu_per_bus: Initialize specific IOMMU handlers for a PCIBus
+ *
+ * Similar to pci_setup_iommu but enforces that the iommu only protects
+ * @bus downstream end points and no other bus hierarchy
+ *
+ * @bus: the #PCIBus being updated.
+ * @ops: the #PCIIOMMUOps
+ * @opaque: passed to callbacks of the @ops structure.
+ * @errp: error handle
+ *
+ * Returns false on failure with @errp set, true on success
+ */
+bool pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops,
+                             void *opaque, Error **errp);
 
 pcibus_t pci_bar_address(PCIDevice *d,
                          int reg, uint8_t type, pcibus_t size);
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 58c4452b1f..8e40ba603d 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -981,7 +981,9 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
         }
 
         if (s->smmu_per_bus) {
-            pci_setup_iommu_per_bus(pci_bus, s->iommu_ops, s);
+            if (!pci_setup_iommu_per_bus(pci_bus, s->iommu_ops, s, errp)) {
+                return;
+            }
         } else {
             pci_setup_iommu(pci_bus, s->iommu_ops, s);
         }
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 4298adf5a0..6d54524c9b 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -3307,11 +3307,16 @@ void pci_setup_iommu(PCIBus *bus, const PCIIOMMUOps *ops, void *opaque)
  * IOMMU ops are returned, avoiding the use of the parent’s IOMMU when
  * it's not appropriate.
  */
-void pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops,
-                             void *opaque)
+bool pci_setup_iommu_per_bus(PCIBus *bus, const PCIIOMMUOps *ops,
+                             void *opaque, Error **errp)
 {
+    if (bus->iommu_per_bus) {
+        error_setg(errp, "An iommu is already attached to this bus");
+        return false;
+    }
     pci_setup_iommu(bus, ops, opaque);
     bus->iommu_per_bus = true;
+    return true;
 }
 
 static void pci_dev_get_w64(PCIBus *b, PCIDevice *dev, void *opaque)
-- 
2.53.0


Re: [PATCH v2] hw/pci/pci: Enforce pci_setup_iommu_per_bus() is called only once per bus
Posted by Peter Maydell 3 weeks, 3 days ago
On Wed, 3 Jun 2026 at 13:44, Eric Auger <eric.auger@redhat.com> wrote:
>
> Currently it is possible to attach several arm-smmuv3 devices to the
> same bus although it is a wrong setup.
>
> Change the prototype of pci_setup_iommu_per_bus to pass an error
> handle. This latter is set when iommu_per_bus is already set and
> used by the single caller (smmu_base_realize) to report a useful
> error to the end-user.
>
> While at it document pci_setup_iommu_per_bus callback in the header.
>
> Fixes: 66d2f665e163 ("hw/arm/virt: Allow user-creatable SMMUv3 dev instantiation")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

I'll take this into target-arm.next as it's a fix for a function
that's only used by the arm SMMU.

thanks
-- PMM
Re: [PATCH v2] hw/pci/pci: Enforce pci_setup_iommu_per_bus() is called only once per bus
Posted by Michael S. Tsirkin 3 weeks, 3 days ago
On Tue, Jun 16, 2026 at 09:47:55AM +0100, Peter Maydell wrote:
> On Wed, 3 Jun 2026 at 13:44, Eric Auger <eric.auger@redhat.com> wrote:
> >
> > Currently it is possible to attach several arm-smmuv3 devices to the
> > same bus although it is a wrong setup.
> >
> > Change the prototype of pci_setup_iommu_per_bus to pass an error
> > handle. This latter is set when iommu_per_bus is already set and
> > used by the single caller (smmu_base_realize) to report a useful
> > error to the end-user.
> >
> > While at it document pci_setup_iommu_per_bus callback in the header.
> >
> > Fixes: 66d2f665e163 ("hw/arm/virt: Allow user-creatable SMMUv3 dev instantiation")
> > Signed-off-by: Eric Auger <eric.auger@redhat.com>
> > Tested-by: Nathan Chen <nathanc@nvidia.com>
> > Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> 
> I'll take this into target-arm.next as it's a fix for a function
> that's only used by the arm SMMU.
> 
> thanks
> -- PMM

Pls do.
Acked-by: Michael S. Tsirkin <mst@redhat.com>

-- 
MST
Re: [PATCH v2] hw/pci/pci: Enforce pci_setup_iommu_per_bus() is called only once per bus
Posted by Philippe Mathieu-Daudé 1 month ago
On 3/6/26 14:44, Eric Auger wrote:
> Currently it is possible to attach several arm-smmuv3 devices to the
> same bus although it is a wrong setup.
> 
> Change the prototype of pci_setup_iommu_per_bus to pass an error
> handle. This latter is set when iommu_per_bus is already set and
> used by the single caller (smmu_base_realize) to report a useful
> error to the end-user.
> 
> While at it document pci_setup_iommu_per_bus callback in the header.
> 
> Fixes: 66d2f665e163 ("hw/arm/virt: Allow user-creatable SMMUv3 dev instantiation")
> Signed-off-by: Eric Auger <eric.auger@redhat.com>
> Tested-by: Nathan Chen <nathanc@nvidia.com>
> Reviewed-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
> 
> ---
> 
> v1 -> v2:
> - in smmu_base_realize, return if pci_setup_iommu_per_bus returns false
>    (Philippe)
> ---
>   include/hw/pci/pci.h | 16 +++++++++++++++-
>   hw/arm/smmu-common.c |  4 +++-
>   hw/pci/pci.c         |  9 +++++++--
>   3 files changed, 25 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@mailo.com>