> -----Original Message-----
> From: Eric Auger <[email protected]>
> Sent: Monday, March 17, 2025 4:52 PM
> To: Shameerali Kolothum Thodi
> <[email protected]>; [email protected];
> [email protected]
> Cc: [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]
> Subject: Re: [RFC PATCH v2 07/20] hw/arm/smmu-common: Introduce
> callbacks for PCIIOMMUOps
> Hi Shameer,
>
> On 3/13/25 9:09 AM, Shameerali Kolothum Thodi wrote:
> > Hi Eric,
> >
> >>> bool accel;
> >>> +
> >>> + AddressSpace * (*get_address_space)(PCIBus *bus, void *opaque,
> int
> >> devfn);
> >>> + bool (*set_iommu_device)(PCIBus *bus, void *opaque, int devfn,
> >>> + HostIOMMUDevice *dev, Error **errp);
> >>> + void (*unset_iommu_device)(PCIBus *bus, void *opaque, int devfn);
> >> I think this should be exposed by a class and only implemented in the
> >> smmuv3 accel device. Adding those cbs directly in the State looks not the
> >> std way.
> > Ok. You mean we can directly place PCIIOMMUOps *ops here then?
> When I first skimmed through the series I assumed you would use 2
> seperate devices, in which case that would use 2 different
> implementations of the same class. You may have a look at
> docs/devel/qom.rst and Methods and class there.
>
> Now as I commented earlier I think the end user shall instantiate the
> same device for non accel and accel. I would advocate for passing an
> option telling whether we want accel modality. Then it rather looks like
> what was done for vfio device with either legacy or iommufd backend.
>
> depending on whether the iommufd option is passed you select the right
> class implementation:
> see hw/vfio/common.c and vfio_attach_device
>
>
> const VFIOIOMMUClass *ops =
> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGA
> CY));
>
> if (vbasedev->iommufd) {
> ops =
> VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUFD)
> );
> }
>
> I would doing something similar for selecting the right ops depending on
> the passed option.
>
> I hope this helps
Thanks Eric. I will take a look.
Shameer
