Hi Eric,
> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: 04 September 2025 15:32
> To: Duan, Zhenzhong <[email protected]>; Nicolin Chen
> <[email protected]>; Shameer Kolothum <[email protected]>
> Cc: [email protected]; [email protected];
> [email protected]; Jason Gunthorpe <[email protected]>;
> [email protected]; [email protected]; Nathan Chen
> <[email protected]>; Matt Ochs <[email protected]>;
> [email protected]; Linuxarm <[email protected]>; Wangzhou (B)
> <[email protected]>; jiangkunkun <[email protected]>;
> Jonathan Cameron <[email protected]>;
> [email protected]; [email protected]
> Subject: Re: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce smmuv3
> accel device
>
> External email: Use caution opening links or attachments
>
>
> Hi Shameer,
>
> On 7/16/25 11:27 AM, Shameerali Kolothum Thodi wrote:
> >
> >> -----Original Message-----
> >> From: Duan, Zhenzhong <[email protected]>
> >> Sent: Wednesday, July 16, 2025 4:39 AM
> >> To: Nicolin Chen <[email protected]>
> >> Cc: Shameerali Kolothum Thodi
> >> <[email protected]>; [email protected];
> >> [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; [email protected]; [email protected];
> >> [email protected]; Linuxarm <[email protected]>; Wangzhou (B)
> >> <[email protected]>; jiangkunkun <[email protected]>;
> >> Jonathan Cameron <[email protected]>;
> >> [email protected]; [email protected]
> >> Subject: RE: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce
> >> smmuv3 accel device
> >>
> >>
> >>
> >>> -----Original Message-----
> >>> From: Nicolin Chen <[email protected]>
> >>> Subject: Re: [RFC PATCH v3 05/15] hw/arm/smmuv3-accel: Introduce
> >>> smmuv3 accel device
> >>>
> >>> On Tue, Jul 15, 2025 at 10:48:31AM +0000, Duan, Zhenzhong wrote:
> >>>>> +static const TypeInfo types[] = {
> >>>>> + {
> >>>>> + .name = TYPE_ARM_SMMUV3_ACCEL,
> >>>>> + .parent = TYPE_ARM_SMMUV3,
> >>>>> + .class_init = smmuv3_accel_class_init,
> >>>>> + }
> >>>> In cover-letter, I see "-device arm-smmuv3", so where is above accel
> >>>> device created so we could use smmuv3_accel_ops?
> >>> The smmu-common.c is the shared file between accel and non-accel
> >>> instances. It has a module property:
> >>> DEFINE_PROP_BOOL("accel", SMMUState, accel, false),
> >> It looks we expose a new TYPE_ARM_SMMUV3_ACCEL type device just for
> >> exporting accel iommu_ops?
> >> What about returning accel iommu_ops directly in
> >> smmu_iommu_ops_by_type() and drop the new type?
> > We are not creating any new device here. Its just a Class object of
> > different
> type.
> > I had a different approach previously and Eric suggested to try this as
> > there
> > are examples in VFIO/IOMMUFD for something like this.
> >
> > https://lore.kernel.org/qemu-devel/1105d100-dd1e-4aca-b518-
> [email protected]/
> Actually I pointed out that usually we don't add methods in states as it
> was done in v2 (in SMMUState) but rather in classes hence my suggestion
> to use a class instead. Now what looks strange is your class does not
> implement any method ;-)
>
> docs/devel/qom.rst says "The #ObjectClass typically holds a table of function
> pointers
> for the virtual methods implemented by this type."
>
> Sorry if I was unclear.
>
> Now if you don't need any "accel" specific methods besides the
> PCIIOMMUOps which have a specific struct, I am not sure we need a class
> anymore. or then you direct embed the PCIIOMMUOps Struct in the method?
Looks like I misinterpreted your suggestion. I don't see a need for any specific
"accel" methods at the moment. And ObjectClass as per the definition above
Is indeed not a candidate for this now.
> Also it's true that's weird that the actual object is never instantiated and
> may
> also appear in qom object list. This is not the case for the example I gave,
> ie.
> the VFIOContainerBase.
True. My bad.
>
> I don't know if anyone has a better/more elegant idea?
How about something like this instead,
diff --git a/hw/arm/smmu-common.c b/hw/arm/smmu-common.c
index 0f1a06cec2..0412ad26f0 100644
--- a/hw/arm/smmu-common.c
+++ b/hw/arm/smmu-common.c
@@ -956,6 +956,9 @@ static void smmu_base_realize(DeviceState *dev, Error
**errp)
return;
}
+ if (!s->iommu_ops) {
+ s->iommu_ops = &smmu_ops;
+ }
/*
* We only allow default PCIe Root Complex(pcie.0) or pxb-pcie based extra
* root complexes to be associated with SMMU.
@@ -975,9 +978,9 @@ static void smmu_base_realize(DeviceState *dev, Error
**errp)
}
if (s->smmu_per_bus) {
- pci_setup_iommu_per_bus(pci_bus, &smmu_ops, s);
+ pci_setup_iommu_per_bus(pci_bus, s->iommu_ops, s);
} else {
- pci_setup_iommu(pci_bus, &smmu_ops, s);
+ pci_setup_iommu(pci_bus, s->iommu_ops, s);
}
return;
}
diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
index bcf8af8dc7..f0c352515b 100644
--- a/hw/arm/smmuv3.c
+++ b/hw/arm/smmuv3.c
@@ -32,6 +32,7 @@
#include "qapi/error.h"
#include "hw/arm/smmuv3.h"
+#include "smmuv3-accel.h"
#include "smmuv3-internal.h"
#include "smmu-internal.h"
@@ -1882,6 +1883,10 @@ static void smmu_realize(DeviceState *d, Error **errp)
SysBusDevice *dev = SYS_BUS_DEVICE(d);
Error *local_err = NULL;
+ if (sys->accel) {
+ smmuv3_accel_set_iommu_ops(sys);
+ }
+
c->parent_realize(d, &local_err);
if (local_err) {
error_propagate(errp, local_err);
diff --git a/include/hw/arm/smmu-common.h b/include/hw/arm/smmu-common.h
index c6f899e403..7f06d95a52 100644
--- a/include/hw/arm/smmu-common.h
+++ b/include/hw/arm/smmu-common.h
@@ -162,6 +162,8 @@ struct SMMUState {
uint8_t bus_num;
PCIBus *primary_bus;
bool smmu_per_bus; /* SMMU is specific to the primary_bus */
+ const PCIIOMMUOps *iommu_ops;
+ bool accel;
};
struct SMMUBaseClass {
diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
new file mode 100644
index 0000000000..7b4b635044
--- /dev/null
+++ b/hw/arm/smmuv3-accel.c
[...]
+
+static const PCIIOMMUOps smmuv3_accel_ops = {
+ .get_address_space = smmuv3_accel_find_add_as,
+};
+
+void smmuv3_accel_set_iommu_ops(SMMUState *s)
+{
+ s->iommu_ops = &smmuv3_accel_ops;
+}
Please let me know.
Thanks,
Shameer
