Re: [PATCH v3 4/7] vfio: Introduce VFIO_DEVICE_FEATURE ioctl and first user

From: Cornelia Huck
Date: Thu Mar 19 2020 - 13:33:59 EST


On Wed, 11 Mar 2020 15:59:01 -0600
Alex Williamson <alex.williamson@xxxxxxxxxx> wrote:

> The VFIO_DEVICE_FEATURE ioctl is meant to be a general purpose, device
> agnostic ioctl for setting, retrieving, and probing device features.
> This implementation provides a 16-bit field for specifying a feature
> index, where the data porition of the ioctl is determined by the
> semantics for the given feature. Additional flag bits indicate the
> direction and nature of the operation; SET indicates user data is
> provided into the device feature, GET indicates the device feature is
> written out into user data. The PROBE flag augments determining
> whether the given feature is supported, and if provided, whether the
> given operation on the feature is supported.
>
> The first user of this ioctl is for setting the vfio-pci VF token,
> where the user provides a shared secret key (UUID) on a SR-IOV PF
> device, which users must provide when opening associated VF devices.
>
> Signed-off-by: Alex Williamson <alex.williamson@xxxxxxxxxx>
> ---
> drivers/vfio/pci/vfio_pci.c | 59 +++++++++++++++++++++++++++++++++++++++++++
> include/uapi/linux/vfio.h | 37 +++++++++++++++++++++++++++
> 2 files changed, 96 insertions(+)

(...)

> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
> index 9e843a147ead..aa37f90a2180 100644
> --- a/include/uapi/linux/vfio.h
> +++ b/include/uapi/linux/vfio.h
> @@ -707,6 +707,43 @@ struct vfio_device_ioeventfd {
>
> #define VFIO_DEVICE_IOEVENTFD _IO(VFIO_TYPE, VFIO_BASE + 16)
>
> +/**
> + * VFIO_DEVICE_FEATURE - _IORW(VFIO_TYPE, VFIO_BASE + 17,
> + * struct vfio_device_feature)
> + *
> + * Get, set, or probe feature data of the device. The feature is selected
> + * using the FEATURE_MASK portion of the flags field. Support for a feature
> + * can be probed by setting both the FEATURE_MASK and PROBE bits. A probe
> + * may optionally include the GET and/or SET bits to determine read vs write
> + * access of the feature respectively. Probing a feature will return success
> + * if the feature is supported and all of the optionally indicated GET/SET
> + * methods are supported. The format of the data portion of the structure is
> + * specific to the given feature. The data portion is not required for
> + * probing.

Maybe add

"GET and SET are mutually exclusive, unless PROBE is also specified."

?

> + *
> + * Return 0 on success, -errno on failure.
> + */
> +struct vfio_device_feature {
> + __u32 argsz;
> + __u32 flags;
> +#define VFIO_DEVICE_FEATURE_MASK (0xffff) /* 16-bit feature index */
> +#define VFIO_DEVICE_FEATURE_GET (1 << 16) /* Get feature into data[] */
> +#define VFIO_DEVICE_FEATURE_SET (1 << 17) /* Set feature from data[] */
> +#define VFIO_DEVICE_FEATURE_PROBE (1 << 18) /* Probe feature support */
> + __u8 data[];
> +};
> +
> +#define VFIO_DEVICE_FEATURE _IO(VFIO_TYPE, VFIO_BASE + 17)
> +
> +/*
> + * Provide support for setting a PCI VF Token, which is used as a shared
> + * secret between PF and VF drivers. This feature may only be set on a
> + * PCI SR-IOV PF when SR-IOV is enabled on the PF and there are no existing
> + * open VFs. Data provided when setting this feature is a 16-byte array
> + * (__u8 b[16]), representing a UUID.
> + */
> +#define VFIO_DEVICE_FEATURE_PCI_VF_TOKEN (0)
> +
> /* -------- API for Type1 VFIO IOMMU -------- */
>
> /**
>

Reviewed-by: Cornelia Huck <cohuck@xxxxxxxxxx>