Re: [PATCH v7 4/4] vfio-pci/zdev: Add VFIO FMB device features

[Omar Elghoul <oelghoul@linux.ibm.com>]

On 9/30/26 2:57 PM, Alex Williamson wrote:
> On Tue, 22 Sep 2026 15:51:41 -0400
> Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> 
>> Introduce new VFIO features for zPCI devices to provide FMB passthrough to
>> userspace.
>>
>> Allow the user to enable or disable the FMB using the SET-only feature
>> VFIO_DEVICE_FEATURE_ZPCI_FMB_ENABLE. Likewise allow the user to read the
>> latest FMB using the GET-only feature VFIO_DEVICE_FEATURE_ZPCI_FMB_READ
>> in the case when the FMB is enabled.
>>
>> Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
>> ---
>>   drivers/vfio/pci/vfio_pci_core.c |  4 +++
>>   drivers/vfio/pci/vfio_pci_priv.h | 18 ++++++++++
>>   drivers/vfio/pci/vfio_pci_zdev.c | 60 ++++++++++++++++++++++++++++++++
>>   include/uapi/linux/vfio.h        | 29 +++++++++++++++
>>   4 files changed, 111 insertions(+)
>>
>> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> index 6757054e9d87..3c827a77725b 100644
>> --- a/drivers/vfio/pci/vfio_pci_core.c
>> +++ b/drivers/vfio/pci/vfio_pci_core.c
>> @@ -1627,6 +1627,10 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
>>   		return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
>>   	case VFIO_DEVICE_FEATURE_ZPCI_ERROR:
>>   		return vfio_pci_zdev_feature_err(device, flags, arg, argsz);
>> +	case VFIO_DEVICE_FEATURE_ZPCI_FMB_ENABLE:
>> +		return vfio_pci_zdev_feature_fmb_enable(vdev, flags, arg, argsz);
>> +	case VFIO_DEVICE_FEATURE_ZPCI_FMB_READ:
>> +		return vfio_pci_zdev_feature_fmb_read(vdev, flags, arg, argsz);
>>   	default:
>>   		return -ENOTTY;
>>   	}
>> diff --git a/drivers/vfio/pci/vfio_pci_priv.h b/drivers/vfio/pci/vfio_pci_priv.h
>> index 4e7162234a2e..e04d7e9d0c30 100644
>> --- a/drivers/vfio/pci/vfio_pci_priv.h
>> +++ b/drivers/vfio/pci/vfio_pci_priv.h
>> @@ -95,6 +95,10 @@ int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev);
>>   void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev);
>>   int vfio_pci_zdev_feature_err(struct vfio_device *device, u32 flags,
>>   			      void __user *arg, size_t argsz);
>> +int vfio_pci_zdev_feature_fmb_enable(struct vfio_pci_core_device *vdev, u32 flags,
>> +				     void __user *arg, size_t argsz);
>> +int vfio_pci_zdev_feature_fmb_read(struct vfio_pci_core_device *vdev, u32 flags,
>> +				   void __user *arg, size_t argsz);
>>   #else
>>   static inline int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev,
>>   					      struct vfio_info_cap *caps)
>> @@ -116,6 +120,20 @@ static inline int vfio_pci_zdev_feature_err(struct vfio_device *device,
>>   {
>>   	return -ENOTTY;
>>   }
>> +
>> +static inline int vfio_pci_zdev_feature_fmb_enable(struct vfio_pci_core_device *vdev,
>> +						   u32 flags, void __user *arg,
>> +						   size_t argsz)
>> +{
>> +	return -ENOTTY;
>> +}
>> +
>> +static inline int vfio_pci_zdev_feature_fmb_read(struct vfio_pci_core_device *vdev,
>> +						 u32 flags, void __user *arg,
>> +						 size_t argsz)
>> +{
>> +	return -ENOTTY;
>> +}
>>   #endif
>>   
>>   static inline bool vfio_pci_is_vga(struct pci_dev *pdev)
>> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
>> index f47f36314a1c..b028de8e08a4 100644
>> --- a/drivers/vfio/pci/vfio_pci_zdev.c
>> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
>> @@ -219,3 +219,63 @@ void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev)
>>   	if (zpci_kvm_hook.kvm_unregister)
>>   		zpci_kvm_hook.kvm_unregister(zdev);
>>   }
>> +
>> +int vfio_pci_zdev_feature_fmb_enable(struct vfio_pci_core_device *vdev, u32 flags,
>> +				     void __user *arg, size_t argsz)
>> +{
>> +	struct zpci_dev *zdev;
>> +	struct vfio_device_feature_zpci_fmb_enable fmb_enable;
>> +	int ret;
>> +
>> +	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_SET, sizeof(fmb_enable));
>> +	if (ret != 1)
>> +		return ret;
>> +
>> +	zdev = to_zpci(vdev->pdev);
>> +	if (!zdev)
>> +		return -ENODEV;
>> +
>> +	if (copy_from_user(&fmb_enable, arg, sizeof(fmb_enable)))
>> +		return -EFAULT;
>> +
>> +	guard(mutex)(&zdev->fmb_lock);
>> +
>> +	if (fmb_enable.enabled)
>> +		return zpci_fmb_reenable_device(zdev);
>> +	return zpci_fmb_disable_device(zdev);
>> +}
>> +
>> +int vfio_pci_zdev_feature_fmb_read(struct vfio_pci_core_device *vdev, u32 flags,
>> +				   void __user *arg, size_t argsz)
>> +{
>> +	struct zpci_dev *zdev;
>> +	struct vfio_device_feature_zpci_fmb_read fmb_read;
>> +	struct zpci_fmb fmb_bounce;
>> +	int ret;
>> +
>> +	ret = vfio_check_feature(flags, argsz, VFIO_DEVICE_FEATURE_GET, sizeof(fmb_read));
>> +	if (ret != 1)
>> +		return ret;
>> +
>> +	zdev = to_zpci(vdev->pdev);
>> +	if (!zdev)
>> +		return -ENODEV;
>> +
>> +	if (copy_from_user(&fmb_read, arg, sizeof(fmb_read)))
>> +		return -EFAULT;
>> +	if (!fmb_read.data)
>> +		return -EINVAL;
>> +
>> +	mutex_lock(&zdev->fmb_lock);
>> +	if (!zdev->fmb_enabled) {
>> +		mutex_unlock(&zdev->fmb_lock);
>> +		return -ENOMSG;
>> +	}
>> +
>> +	memcpy(&fmb_bounce, zdev->fmb, zdev->fmb_length);
>> +	mutex_unlock(&zdev->fmb_lock);
> 
> A scoped guard would be preferable for consistency with the enable
> path.

I thought of that too, but then we'd do copy_to_user() while holding the
mutex for longer than is necessary, which I think was pointed out in an
earlier version.

Thanks

> 
>> +
>> +	if (copy_to_user(u64_to_user_ptr(fmb_read.data), &fmb_bounce, zdev->fmb_length))
>> +		return -EFAULT;
>> +	return 0;
>> +}
>> diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
>> index e41437fa17ad..a8d501e42ef9 100644
>> --- a/include/uapi/linux/vfio.h
>> +++ b/include/uapi/linux/vfio.h
>> @@ -1555,6 +1555,35 @@ struct vfio_device_feature_zpci_err {
>>   
>>   #define VFIO_DEVICE_FEATURE_ZPCI_ERROR 13
>>   
>> +/**
>> + * Upon VFIO_DEVICE_FEATURE_SET, enable or disable FMB for the VFIO zPCI device.
>> + *
>> + * enabled is treated as a bool, so any non-zero value evaluates to true. Nested
>> + * enabling resets the FMB counters to zero, and nested disabling fails.
>> + *
>> + * Returns: 0 on success, -1 and errno set appropriately on error.
>> + */
>> +#define VFIO_DEVICE_FEATURE_ZPCI_FMB_ENABLE 14
>> +
>> +struct vfio_device_feature_zpci_fmb_enable {
>> +	__u8 enabled;
>> +};
>> +
>> +/**
>> + * Upon VFIO_DEVICE_FEATURE_GET, provide FMB passthrough for VFIO zPCI devices.
>> + *
>> + * The user-provided buffer must be at least fmb_length large, where fmb_length
>> + * is reported in VFIO_DEVICE_INFO_CAP_ZPCI_BASE.
>> + *
>> + * Returns: 0 on success, -1 and errno set appropriately on error. errno==ENOMSG
>> + * when the FMB is not enabled.
>> + */
>> +#define VFIO_DEVICE_FEATURE_ZPCI_FMB_READ 15
>> +
>> +struct vfio_device_feature_zpci_fmb_read {
>> +	__aligned_u64 data;
>> +};
>> +
> 
> As usual, feature numbers need to be coordinated with the vfio tree.
> What's the merge plan for this?  Thanks,
> 
> Alex