On Mon, Jan 31, 2022 at 04:41:43PM -0700, Alex Williamson wrote: > > +int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags, > > + void __user *arg, size_t argsz) > > +{ > > + struct vfio_pci_core_device *vdev = > > + container_of(device, struct vfio_pci_core_device, vdev); > > + uuid_t uuid; > > + int ret; > > Nit, should uuid at least be scoped within the token code? Or token > code pushed to a separate function? Sure, it wasn't done before, but it would be nicer,. > > +static inline int vfio_check_feature(u32 flags, size_t argsz, u32 supported_ops, > > + size_t minsz) > > +{ > > + if ((flags & (VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_SET)) & > > + ~supported_ops) > > + return -EINVAL; > > These look like cases where it would be useful for userspace debugging > to differentiate errnos. I tried to keep it unchanged from what it was today. > -EOPNOTSUPP? This would be my preference, but it would also be the first use in vfio > > + if (flags & VFIO_DEVICE_FEATURE_PROBE) > > + return 0; > > + /* Without PROBE one of GET or SET must be requested */ > > + if (!(flags & (VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_SET))) > > + return -EINVAL; > > + if (argsz < minsz) > > + return -EINVAL; > > -ENOSPC? Do you want to do all of these minsz then? There are lots.. Jason