Re: [PATCH v3 11/19] backends/iommufd: Implement HostIOMMUDeviceClass::check_cap() handler

[Cédric Le Goater]

On 4/30/24 12:06, Duan, Zhenzhong wrote:
> > -----Original Message-----
> > From: Cédric Le Goater <clg@redhat.com>
> > Subject: Re: [PATCH v3 11/19] backends/iommufd: Implement
> > HostIOMMUDeviceClass::check_cap() handler
> >
> > On 4/29/24 08:50, Zhenzhong Duan wrote:
> > > Suggested-by: Cédric Le Goater <clg@redhat.com>
> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > > ---
> > >    backends/iommufd.c | 18 ++++++++++++++++++
> > >    1 file changed, 18 insertions(+)
> > >
> > > diff --git a/backends/iommufd.c b/backends/iommufd.c
> > > index d61209788a..28faec528e 100644
> > > --- a/backends/iommufd.c
> > > +++ b/backends/iommufd.c
> > > @@ -233,6 +233,23 @@ int
> > iommufd_backend_get_device_info(IOMMUFDBackend *be, uint32_t devid,
> > >        return ret;
> > >    }
> > >
> > > +static int hiod_iommufd_check_cap(HostIOMMUDevice *hiod, int cap,
> > Error **errp)
> > > +{
> > > +    switch (cap) {
> > > +    case HOST_IOMMU_DEVICE_CAP_IOMMUFD:
> > > +        return 1;
> >
> > I don't understand this value.
>
> 1 means this host iommu device is attached to IOMMUFD backend,
> or else 0 if attached to legacy backend.

Hmm, this looks hacky to me and it is not used anywhere in the patchset.
Let's reconsider when there is actually a use for it. Until then, please
drop. My feeling is that a new HostIOMMUDeviceClass handler/attributed
should be introduced instead.


Thanks,

C.



> Strictly speaking, HOST_IOMMU_DEVICE_CAP_IOMMUFD is not a
> hardware capability, I'm trying to put all(sw/hw) in CAPs checking
> framework just like KVM<->qemu CAPs does.
>
> Thanks
> Zhenzhong
>
> > Thanks,
> >
> > C.
> >
> >
> > > +    default:
> > > +        return host_iommu_device_check_cap_common(hiod, cap, errp);
> > > +    }
> > > +}
> > > +
> > > +static void hiod_iommufd_class_init(ObjectClass *oc, void *data)
> > > +{
> > > +    HostIOMMUDeviceClass *hioc = HOST_IOMMU_DEVICE_CLASS(oc);
> > > +
> > > +    hioc->check_cap = hiod_iommufd_check_cap;
> > > +};
> > > +
> > >    static const TypeInfo types[] = {
> > >        {
> > >            .name = TYPE_IOMMUFD_BACKEND,
> > > @@ -251,6 +268,7 @@ static const TypeInfo types[] = {
> > >            .parent = TYPE_HOST_IOMMU_DEVICE,
> > >            .instance_size = sizeof(HostIOMMUDeviceIOMMUFD),
> > >            .class_size = sizeof(HostIOMMUDeviceIOMMUFDClass),
> > > +        .class_init = hiod_iommufd_class_init,
> > >            .abstract = true,
> > >        }
> > >    };