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

Eric Auger posted 1 patch 4 days, 17 hours 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 4 days, 17 hours 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 Philippe Mathieu-Daudé 4 days 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>