> -----Original Message-----
> From: Eric Auger <eric.au...@redhat.com>
> Sent: Monday, March 17, 2025 4:52 PM
> To: Shameerali Kolothum Thodi
> <shameerali.kolothum.th...@huawei.com>; qemu-...@nongnu.org;
> qemu-devel@nongnu.org
> Cc: peter.mayd...@linaro.org; j...@nvidia.com; nicol...@nvidia.com;
> ddut...@redhat.com; berra...@redhat.com; nath...@nvidia.com;
> mo...@nvidia.com; smost...@google.com; Linuxarm
> <linux...@huawei.com>; Wangzhou (B) <wangzh...@hisilicon.com>;
> jiangkunkun <jiangkun...@huawei.com>; Jonathan Cameron
> <jonathan.came...@huawei.com>; zhangfei....@linaro.org
> 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
