[RFC PATCH v3 04/15] hw/arm/smmu-common: Introduce smmu_iommu_ops_by_type() helper

Shameer Kolothum via posted 15 patches 4 months ago
There is a newer version of this series
[RFC PATCH v3 04/15] hw/arm/smmu-common: Introduce smmu_iommu_ops_by_type() helper
Posted by Shameer Kolothum via 4 months ago
Allows to retrieve the PCIIOMMUOps based on the SMMU type. This will be
useful when we add support for accelerated SMMUV3 in subsequent patches
as that requires a different set of callbacks for iommu ops.

No special handling is required for now and returns the default ops
in base SMMU Class.

No functional changes intended.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
---
 hw/arm/smmu-common.c         | 17 +++++++++++++++--
 include/hw/arm/smmu-common.h |  1 +
 2 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 0f1a06cec2..3a1080773a 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -934,6 +934,16 @@ void smmu_inv_notifiers_all(SMMUState *s)
     }
 }
 
+static const PCIIOMMUOps *smmu_iommu_ops_by_type(SMMUState *s)
+{
+    SMMUBaseClass *sbc;
+
+    sbc = ARM_SMMU_CLASS(object_class_by_name(TYPE_ARM_SMMU));
+    assert(sbc->iommu_ops);
+
+    return sbc->iommu_ops;
+}
+
 static void smmu_base_realize(DeviceState *dev, Error **errp)
 {
     SMMUState *s = ARM_SMMU(dev);
@@ -962,6 +972,7 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
      */
     if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
         object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
+        const PCIIOMMUOps  *iommu_ops;
         /*
          * This condition matches either the default pcie.0, pxb-pcie, or
          * pxb-cxl. For both pxb-pcie and pxb-cxl, parent_dev will be set.
@@ -974,10 +985,11 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
             }
         }
 
+        iommu_ops = smmu_iommu_ops_by_type(s);
         if (s->smmu_per_bus) {
-            pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
+            pci_setup_iommu_per_bus(pci_bus, iommu_ops, s);
         } else {
-            pci_setup_iommu(pci_bus, &smmu_ops, s);
+            pci_setup_iommu(pci_bus, iommu_ops, s);
         }
         return;
     }
@@ -1018,6 +1030,7 @@ static void smmu_base_class_init(ObjectClass *klass, const void *data)
     device_class_set_parent_realize(dc, smmu_base_realize,
                                     &sbc->parent_realize);
     rc->phases.exit = smmu_base_reset_exit;
+    sbc->iommu_ops = &smmu_ops;
 }
 
 static const TypeInfo smmu_base_info = {
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index c6f899e403..eb94623555 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -171,6 +171,7 @@ struct SMMUBaseClass {
     /*< public >*/
 
     DeviceRealize parent_realize;
+    const PCIIOMMUOps *iommu_ops;
 
 };
 
-- 
2.34.1


Re: [RFC PATCH v3 04/15] hw/arm/smmu-common: Introduce smmu_iommu_ops_by_type() helper
Posted by Eric Auger 2 months, 1 week ago

On 7/14/25 5:59 PM, Shameer Kolothum wrote:
> Allows to retrieve the PCIIOMMUOps based on the SMMU type. This will be
> useful when we add support for accelerated SMMUV3 in subsequent patches
> as that requires a different set of callbacks for iommu ops.
>
> No special handling is required for now and returns the default ops
> in base SMMU Class.
>
> No functional changes intended.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>

Thanks

Eric
> ---
>  hw/arm/smmu-common.c         | 17 +++++++++++++++--
>  include/hw/arm/smmu-common.h |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
>
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 0f1a06cec2..3a1080773a 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -934,6 +934,16 @@ void smmu_inv_notifiers_all(SMMUState *s)
>      }
>  }
>  
> +static const PCIIOMMUOps *smmu_iommu_ops_by_type(SMMUState *s)
> +{
> +    SMMUBaseClass *sbc;
> +
> +    sbc = ARM_SMMU_CLASS(object_class_by_name(TYPE_ARM_SMMU));
> +    assert(sbc->iommu_ops);
> +
> +    return sbc->iommu_ops;
> +}
> +
>  static void smmu_base_realize(DeviceState *dev, Error **errp)
>  {
>      SMMUState *s = ARM_SMMU(dev);
> @@ -962,6 +972,7 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
>       */
>      if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
>          object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
> +        const PCIIOMMUOps  *iommu_ops;
>          /*
>           * This condition matches either the default pcie.0, pxb-pcie, or
>           * pxb-cxl. For both pxb-pcie and pxb-cxl, parent_dev will be set.
> @@ -974,10 +985,11 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
>              }
>          }
>  
> +        iommu_ops = smmu_iommu_ops_by_type(s);
>          if (s->smmu_per_bus) {
> -            pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> +            pci_setup_iommu_per_bus(pci_bus, iommu_ops, s);
>          } else {
> -            pci_setup_iommu(pci_bus, &smmu_ops, s);
> +            pci_setup_iommu(pci_bus, iommu_ops, s);
>          }
>          return;
>      }
> @@ -1018,6 +1030,7 @@ static void smmu_base_class_init(ObjectClass *klass, const void *data)
>      device_class_set_parent_realize(dc, smmu_base_realize,
>                                      &sbc->parent_realize);
>      rc->phases.exit = smmu_base_reset_exit;
> +    sbc->iommu_ops = &smmu_ops;
>  }
>  
>  static const TypeInfo smmu_base_info = {
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index c6f899e403..eb94623555 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -171,6 +171,7 @@ struct SMMUBaseClass {
>      /*< public >*/
>  
>      DeviceRealize parent_realize;
> +    const PCIIOMMUOps *iommu_ops;
>  
>  };
>  


Re: [RFC PATCH v3 04/15] hw/arm/smmu-common: Introduce smmu_iommu_ops_by_type() helper
Posted by Jonathan Cameron via 4 months ago
On Mon, 14 Jul 2025 16:59:30 +0100
Shameer Kolothum <shameerali.kolothum.thodi@huawei.com> wrote:

> Allows to retrieve the PCIIOMMUOps based on the SMMU type. This will be
> useful when we add support for accelerated SMMUV3 in subsequent patches
> as that requires a different set of callbacks for iommu ops.
> 
> No special handling is required for now and returns the default ops
> in base SMMU Class.
> 
> No functional changes intended.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Trivial inline.

> ---
>  hw/arm/smmu-common.c         | 17 +++++++++++++++--
>  include/hw/arm/smmu-common.h |  1 +
>  2 files changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
> index 0f1a06cec2..3a1080773a 100644
> --- a/hw/arm/smmu-common.c
> +++ b/hw/arm/smmu-common.c
> @@ -934,6 +934,16 @@ void smmu_inv_notifiers_all(SMMUState *s)
>      }
>  }
>  
> +static const PCIIOMMUOps *smmu_iommu_ops_by_type(SMMUState *s)
> +{
> +    SMMUBaseClass *sbc;
> +
> +    sbc = ARM_SMMU_CLASS(object_class_by_name(TYPE_ARM_SMMU));
> +    assert(sbc->iommu_ops);
> +
> +    return sbc->iommu_ops;
> +}
> +
>  static void smmu_base_realize(DeviceState *dev, Error **errp)
>  {
>      SMMUState *s = ARM_SMMU(dev);
> @@ -962,6 +972,7 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
>       */
>      if (pci_bus_is_express(pci_bus) && pci_bus_is_root(pci_bus) &&
>          object_dynamic_cast(OBJECT(pci_bus)->parent, TYPE_PCI_HOST_BRIDGE)) {
> +        const PCIIOMMUOps  *iommu_ops;
Bonus space.

>          /*
>           * This condition matches either the default pcie.0, pxb-pcie, or
>           * pxb-cxl. For both pxb-pcie and pxb-cxl, parent_dev will be set.
> @@ -974,10 +985,11 @@ static void smmu_base_realize(DeviceState *dev, Error **errp)
>              }
>          }
>  
> +        iommu_ops = smmu_iommu_ops_by_type(s);
>          if (s->smmu_per_bus) {
> -            pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
> +            pci_setup_iommu_per_bus(pci_bus, iommu_ops, s);
>          } else {
> -            pci_setup_iommu(pci_bus, &smmu_ops, s);
> +            pci_setup_iommu(pci_bus, iommu_ops, s);
>          }
>          return;
>      }
> @@ -1018,6 +1030,7 @@ static void smmu_base_class_init(ObjectClass *klass, const void *data)
>      device_class_set_parent_realize(dc, smmu_base_realize,
>                                      &sbc->parent_realize);
>      rc->phases.exit = smmu_base_reset_exit;
> +    sbc->iommu_ops = &smmu_ops;
>  }
>  
>  static const TypeInfo smmu_base_info = {
> diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
> index c6f899e403..eb94623555 100644
> --- a/include/hw/arm/smmu-common.h
> +++ b/include/hw/arm/smmu-common.h
> @@ -171,6 +171,7 @@ struct SMMUBaseClass {
>      /*< public >*/
>  
>      DeviceRealize parent_realize;
> +    const PCIIOMMUOps *iommu_ops;
>  
>  };
>  
Re: [RFC PATCH v3 04/15] hw/arm/smmu-common: Introduce smmu_iommu_ops_by_type() helper
Posted by Nicolin Chen via 4 months ago
On Mon, Jul 14, 2025 at 04:59:30PM +0100, Shameer Kolothum wrote:
> Allows to retrieve the PCIIOMMUOps based on the SMMU type. This will be
> useful when we add support for accelerated SMMUV3 in subsequent patches
> as that requires a different set of callbacks for iommu ops.
> 
> No special handling is required for now and returns the default ops
> in base SMMU Class.
> 
> No functional changes intended.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>

Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>

> +static const PCIIOMMUOps *smmu_iommu_ops_by_type(SMMUState *s)
> +{
> +    SMMUBaseClass *sbc;
> +
> +    sbc = ARM_SMMU_CLASS(object_class_by_name(TYPE_ARM_SMMU));
> +    assert(sbc->iommu_ops);

I have an impression that QEMU uses the glib version more, so it
could be g_assert(). But I do see a lots of assert() also in the
existing code. So, just a note here, not a strong suggestion.