Re: [PATCH v3 13/19] vfio: Create host IOMMU device instance

[Cédric Le Goater]

On 4/30/24 12:16, Duan, Zhenzhong wrote:
> > -----Original Message-----
> > From: Cédric Le Goater <clg@redhat.com>
> > Subject: Re: [PATCH v3 13/19] vfio: Create host IOMMU device instance
> >
> > On 4/29/24 08:50, Zhenzhong Duan wrote:
> > > Create host IOMMU device instance in vfio_attach_device() and call
> > > .realize() to initialize it further.
> > >
> > > Suggested-by: Cédric Le Goater <clg@redhat.com>
> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> > > ---
> > >    include/hw/vfio/vfio-common.h |  1 +
> > >    hw/vfio/common.c              | 18 +++++++++++++++++-
> > >    2 files changed, 18 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-
> > common.h
> > > index 0943add3bc..b204b93a55 100644
> > > --- a/include/hw/vfio/vfio-common.h
> > > +++ b/include/hw/vfio/vfio-common.h
> > > @@ -126,6 +126,7 @@ typedef struct VFIODevice {
> > >        OnOffAuto pre_copy_dirty_page_tracking;
> > >        bool dirty_pages_supported;
> > >        bool dirty_tracking;
> > > +    HostIOMMUDevice *hiod;
> > >        int devid;
> > >        IOMMUFDBackend *iommufd;
> > >    } VFIODevice;
> > > diff --git a/hw/vfio/common.c b/hw/vfio/common.c
> > > index 8f9cbdc026..0be8b70ebd 100644
> > > --- a/hw/vfio/common.c
> > > +++ b/hw/vfio/common.c
> > > @@ -1497,6 +1497,8 @@ int vfio_attach_device(char *name, VFIODevice
> > *vbasedev,
> > >    {
> > >        const VFIOIOMMUClass *ops =
> > VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_LEGACY));
> > > +    HostIOMMUDevice *hiod;
> > > +    int ret;
> > >
> > >        if (vbasedev->iommufd) {
> > >            ops =
> > VFIO_IOMMU_CLASS(object_class_by_name(TYPE_VFIO_IOMMU_IOMMUF
> > D));
> > > @@ -1504,7 +1506,20 @@ int vfio_attach_device(char *name,
> > VFIODevice *vbasedev,
> > >        assert(ops);
> > >
> > > -    return ops->attach_device(name, vbasedev, as, errp);
> > > +    ret = ops->attach_device(name, vbasedev, as, errp);
> > > +    if (ret < 0) {
> > > +        return ret;
> >
> >
> > hmm, I wonder if we should change the return value of vfio_attach_device()
> > to be a bool.
>
> I see, also VFIOIOMMUClass:: setup and VFIOIOMMUClass::add_window.
> I can add cleanup patches to fix them if you have no other plan.

Yes please. I don't have plans for changes in that area. The only pending
patches are from the series "[v4] migration: Improve error reporting" [*].

Thanks,

C.


[*] https://lore.kernel.org/qemu-devel/20240306133441.2351700-1-clg@redhat.com/