Re: [Intel-gfx] [PATCH v1 5/5] vfio: Check the presence for iommufd callbacks in __vfio_register_dev()

[Jason Gunthorpe <jgg@nvidia.com>]

On Fri, Mar 10, 2023 at 02:15:32AM +0000, Tian, Kevin wrote:
> > From: Liu, Yi L <yi.l.liu@intel.com>
> > Sent: Wednesday, March 8, 2023 9:14 PM
> > 
> > After making the no-DMA drivers (samples/vfio-mdev) providing iommufd
> > callbacks, __vfio_register_dev() should check the presence of the iommufd
> > callbacks if CONFIG_IOMMUFD is enabled.
> > 
> > Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> > ---
> >  drivers/vfio/vfio_main.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c
> > index 43bd6b76e2b6..89497c933490 100644
> > --- a/drivers/vfio/vfio_main.c
> > +++ b/drivers/vfio/vfio_main.c
> > @@ -255,8 +255,9 @@ static int __vfio_register_dev(struct vfio_device
> > *device,
> >  {
> >  	int ret;
> > 
> > -	if (WARN_ON(device->ops->bind_iommufd &&
> > -		    (!device->ops->unbind_iommufd ||
> > +	if (WARN_ON(IS_ENABLED(CONFIG_IOMMUFD) &&
> > +		    (!device->ops->bind_iommufd ||
> > +		     !device->ops->unbind_iommufd ||
> >  		     !device->ops->attach_ioas)))
> >  		return -EINVAL;
> > 
> 
> I don't think IS_ENABLED() check is necessary. those ops are
> defined in the driver side w/o a conditional CONFIG_IOMMUFD.
> 
> We should warn out lacking of those ops to the driver developer
> as early as possible, instead of postponing it until someone
> starts to build kernel with CONFIG_IOMMUFD.

The ops are NULL if !CONFIG_IOMMUFD. The previous code was OK because
it checked for non-null bind before demanding the others be non-null.

Jason