Re: [PATCH rfcv1 4/6] vfio: initialize IOMMUFDDevice and pass to vIOMMU

[Yi Liu]

On 2024/1/18 16:17, Duan, Zhenzhong wrote:
> > -----Original Message-----
> > From: Joao Martins <joao.m.martins@oracle.com>
> > Subject: Re: [PATCH rfcv1 4/6] vfio: initialize IOMMUFDDevice and pass to
> > vIOMMU
> >
> > On 15/01/2024 10:13, Zhenzhong Duan wrote:
> > > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c
> > > index 9bfddc1360..cbd035f148 100644
> > > --- a/hw/vfio/iommufd.c
> > > +++ b/hw/vfio/iommufd.c
> > > @@ -309,6 +309,7 @@ static int iommufd_cdev_attach(const char *name,
> > VFIODevice *vbasedev,
> > >       VFIOContainerBase *bcontainer;
> > >       VFIOIOMMUFDContainer *container;
> > >       VFIOAddressSpace *space;
> > > +    IOMMUFDDevice *idev = &vbasedev->idev;
> > >       struct vfio_device_info dev_info = { .argsz = sizeof(dev_info) };
> > >       int ret, devfd;
> > >       uint32_t ioas_id;
> > > @@ -428,6 +429,7 @@ found_container:
> > >       QLIST_INSERT_HEAD(&bcontainer->device_list, vbasedev,
> > container_next);
> > >       QLIST_INSERT_HEAD(&vfio_device_list, vbasedev, global_next);
> > >
> > > +    iommufd_device_init(idev, sizeof(*idev), container->be, vbasedev-
> > > devid);
> > >       trace_iommufd_cdev_device_info(vbasedev->name, devfd, vbasedev-
> > > num_irqs,
> > >                                      vbasedev->num_regions, vbasedev->flags);
> > >       return 0;
> >
> > In the dirty tracking series, I'll need to fetch out_capabilities from device
> > and do a bunch of stuff that is used when allocating hwpt to ask for dirty
> > tracking. And this means having iommufd_device_init() be called before we
> > call
> > iommufd_cdev_attach_container().
> >
> > Here's what it looks based on an earlier version of your patch:
> >
> > https://github.com/jpemartins/qemu/commit/433f97a05e0cdd8e3b8563a
> > a20e4f22d107219b5
> >
> > I can move the call earlier in my series, unless there's something specifically
> > when you call it here?
>
> I think it's safe to move it earlier, just remember to do the same for existing
> container.

yes, as long as the input of iommufd_device_init() are available in the new 
place. And remember to destroy it if the code failed after initializing
iommufd_device.

--
Regards,
Yi Liu