On Wed, Nov 05, 2025 at 04:56:00PM -0400, Jason Gunthorpe wrote: > On Wed, Nov 05, 2025 at 01:48:29PM -0700, Alex Williamson wrote: > > On Mon, 3 Nov 2025 07:39:37 +0000 > > Pranjal Shrivastava <[email protected]> wrote: > > > > > On Thu, Oct 23, 2025 at 08:09:28PM -0300, Jason Gunthorpe wrote: > > > > Remove the fallback through the ioctl callback, no drivers use this now. > > > > > > > > Signed-off-by: Jason Gunthorpe <[email protected]> > > > > --- > > > > drivers/vfio/vfio_main.c | 8 ++++---- > > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > > > diff --git a/drivers/vfio/vfio_main.c b/drivers/vfio/vfio_main.c > > > > index a390163ce706c4..f056e82ba35075 100644 > > > > --- a/drivers/vfio/vfio_main.c > > > > +++ b/drivers/vfio/vfio_main.c > > > > @@ -1297,13 +1297,13 @@ static long vfio_device_fops_unl_ioctl(struct > > > > file *filep, > > > > break; > > > > > > > > case VFIO_DEVICE_GET_REGION_INFO: > > > > - if (!device->ops->get_region_info) > > > > - goto ioctl_fallback; > > > > - ret = device->ops->get_region_info(device, uptr); > > > > + if (unlikely(!device->ops->get_region_info)) > > > > + ret = -EINVAL; > > > > Nit, typically I would have expected a success oriented flow, ie. > > > > if (likely(device->ops->get_region_info)) > > ret = device->ops->get_region_info(device, uptr); > > else > > ret = -EINVAL; > > > > But it goes away in the next patch, so *shrug*. > > Yeah, still I can edit it.. > > > > Nit: Let's also add a warn/err log here highliting that the device > > > doesn't populate the get_region_info op? > > > > Are devices required to implement regions? If so, it'd be more > > appropriate to fail the device registration in __vfio_register_dev() > > for the missing op than wait for an ioctl. However, here in the device > > agnostic layer of vfio, I think the answer leans more towards no, we > > could theoretically have a device with no regions. We also want to be > > careful not to introduce a WARN_ON that's user trigger'able. Thanks, > > Yeah, I had the same thought, so lets leave this. If we do want a warn > it should be in registration. > > A device without regions does not seem useful, but also it doesn't > malfunction if someone does want to do that. >
I agree with the both of you.. I just thought if we should remind the user with a log that the dev doesn't expose a region. But I guess we don't need to worry about that. > Thanks, > Jason Thanks, Praan
