Re: [Intel-gfx] [PATCH 08/10] vfio-iommufd: Support iommufd for emulated VFIO devices

Tian, Kevin
2022-11-03T04:52:31Z

> From: Jason Gunthorpe <jgg@nvidia.com>
> Sent: Tuesday, November 1, 2022 8:49 PM
> > > ---
> > > drivers/gpu/drm/i915/gvt/kvmgt.c | 3 +
> > > drivers/s390/cio/vfio_ccw_ops.c | 3 +
> > > drivers/s390/crypto/vfio_ap_ops.c | 3 +
> > > drivers/vfio/container.c | 108 ++++++-----------------------
> > > drivers/vfio/iommufd.c | 57 ++++++++++++++++
> > > drivers/vfio/vfio.h | 10 ++-
> > > drivers/vfio/vfio_main.c | 110 +++++++++++++++++++++++++++++-
> > > include/linux/vfio.h | 14 ++++
> > > 8 files changed, 217 insertions(+), 91 deletions(-)
> >
> > mtty, mdpy and mbochs?
>
> They don't call rw or pin_pages, so they don't need to do
> anything:
>
>
> /*
> * If the driver doesn't provide this op then it means the device does
> * not do DMA at all. So nothing to do.
> */
> if (!vdev->ops->bind_iommufd)
> return 0;
>

OK, I see the point of this check here.

btw It'd be good to document in vfio_device_ops that driver must provide
this op if the device does DMA.

> > > +
> > > + /* VFIO historically tries to auto-detect a kthread */
> > > + if (!current->mm)
> > > + flags |= IOMMUFD_ACCESS_RW_KTHREAD;
> >
> > Can you elaborate why this cannot be put in iommufd as the default
> > policy similar to what vfio container does?
>
> Snooping in kernel structs to try to guess the calling execution
> context is bad design. The caller should know its own context and it
> should declare positively what it is. Someday this should be lifted
> out of VFIO as well and into the drivers.
>

with the last sentence it makes more sense. otherwise I didn't see
why putting the guess in vfio makes real difference from doing it
in iommufd as there is no vfio specific state referenced for making
this decision.