On Mon, Jan 31, 2022 at 04:41:43PM -0700, Alex Williamson wrote:

quoted

+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,.

quoted

+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

quoted

+	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