[PATCH v2 0/3] vfio-pci/zdev: Improved zPCI Function Measurement Support

[PATCH v2 0/3] vfio-pci/zdev: Improved zPCI Function Measurement Support

[Omar Elghoul]

Hi,

This patch series improves support for function measurement for zPCI
passthrough devices on s390x.

Changelog
=========
v1 -> v2:
* Patch 1/3:
  - Address a possible race condition in zpci_reenable_device() caused by
    calling zpci_fmb_reenable_device() without holding fmb_lock
  - Assert that fmb_lock is held within zpci_fmb_reenable_device()

* Patch 3/3:
  - Address a possible race condition in pci_perf_seq_write() caused by
    consuming zdev->kzdev without holding kzdev_lock

Motivation
==========
The firmware on s390x machines allows for tracking a variety of statistics
relating to zPCI devices in a function measurement block (FMB). However,
the kernel currently lacks a structured mechanism of sharing this
information with userspace, beyond /sys/kernel/debug/pci/ID/statistics.
This can lead to shortcomings when running a guest on KVM with PCI
passthrough devices, as QEMU is unable to provide an accurate FMB snapshot
to the guest.

Proposal
========
We propose adding a new VFIO device feature to zPCI passthrough devices,
allowing userspace programs to read the latest FMB snapshot as it is
written by the firmware. We ensure that function measurement enablement is
preserved across device resets on the host. Furthermore, we guard against
host tampering with the FMB via sysfs when the zPCI device is in
passthrough to protect the VM's state.

I'd appreciate some feedback on these patches.

Thanks in advance.

Omar Elghoul (3):
  s390/pci: Preserve FMB state in device re-enablement
  vfio-pci/zdev: Add VFIO FMB device feature
  s390/pci: Fence FMB enable/disable via sysfs for passthrough devices

 arch/s390/include/asm/pci.h      |  1 +
 arch/s390/pci/pci.c              | 75 ++++++++++++++++++++++++-------
 arch/s390/pci/pci_debug.c        | 11 ++++-
 drivers/vfio/pci/vfio_pci_core.c |  2 +
 drivers/vfio/pci/vfio_pci_priv.h |  9 ++++
 drivers/vfio/pci/vfio_pci_zdev.c | 77 ++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h        | 43 ++++++++++++++++++
 7 files changed, 202 insertions(+), 16 deletions(-)

-- 
2.52.0

[PATCH v2 1/3] s390/pci: Preserve FMB state in device re-enablement

[Omar Elghoul]

Introduce a function zpci_fmb_reenable_device() that checks for the state
of the FMB and reuses the same buffer where appropriate. If FMB was not
previously enabled, it enables it for the device. Call this function during
a zPCI device re-enablement, which in turn implicitly ensures that the FMB
is enabled for host devices during their KVM registration.

This function also clears out the software counters, so that a program
resetting an FMB would see all its counters restart from zero as expected.
The function to clear the software counters is also separated into a static
function as it is now reused in both zpci_fmb_enable_device() and
zpci_fmb_reenable_device().

Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
---
 arch/s390/include/asm/pci.h |  1 +
 arch/s390/pci/pci.c         | 75 +++++++++++++++++++++++++++++--------
 2 files changed, 61 insertions(+), 15 deletions(-)

diff --git a/arch/s390/include/asm/pci.h b/arch/s390/include/asm/pci.h
index 5dcf35f0f325..65014e52d559 100644
--- a/arch/s390/include/asm/pci.h
+++ b/arch/s390/include/asm/pci.h
@@ -323,6 +323,7 @@ void zpci_remove_parent_msi_domain(struct zpci_bus *zbus);
 /* FMB */
 int zpci_fmb_enable_device(struct zpci_dev *);
 int zpci_fmb_disable_device(struct zpci_dev *);
+int zpci_fmb_reenable_device(struct zpci_dev *zdev);
 
 /* Debug */
 int zpci_debug_init(void);
diff --git a/arch/s390/pci/pci.c b/arch/s390/pci/pci.c
index 39bd2adfc240..56cabb2dc291 100644
--- a/arch/s390/pci/pci.c
+++ b/arch/s390/pci/pci.c
@@ -164,22 +164,10 @@ int zpci_unregister_ioat(struct zpci_dev *zdev, u8 dmaas)
 	return cc;
 }
 
-/* Modify PCI: Set PCI function measurement parameters */
-int zpci_fmb_enable_device(struct zpci_dev *zdev)
+static void zpci_fmb_clear_iommu_ctrs(struct zpci_dev *zdev)
 {
-	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
 	struct zpci_iommu_ctrs *ctrs;
-	struct zpci_fib fib = {0};
-	unsigned long flags;
-	u8 cc, status;
-
-	if (zdev->fmb || sizeof(*zdev->fmb) < zdev->fmb_length)
-		return -EINVAL;
-
-	zdev->fmb = kmem_cache_zalloc(zdev_fmb_cache, GFP_KERNEL);
-	if (!zdev->fmb)
-		return -ENOMEM;
-	WARN_ON((u64) zdev->fmb & 0xf);
+	unsigned long flags = 0;
 
 	/* reset software counters */
 	spin_lock_irqsave(&zdev->dom_lock, flags);
@@ -192,7 +180,24 @@ int zpci_fmb_enable_device(struct zpci_dev *zdev)
 		atomic64_set(&ctrs->sync_rpcits, 0);
 	}
 	spin_unlock_irqrestore(&zdev->dom_lock, flags);
+}
+
+/* Modify PCI: Set PCI function measurement parameters */
+int zpci_fmb_enable_device(struct zpci_dev *zdev)
+{
+	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
+	struct zpci_fib fib = {0};
+	u8 cc, status;
+
+	if (zdev->fmb || sizeof(*zdev->fmb) < zdev->fmb_length)
+		return -EINVAL;
+
+	zdev->fmb = kmem_cache_zalloc(zdev_fmb_cache, GFP_KERNEL);
+	if (!zdev->fmb)
+		return -ENOMEM;
+	WARN_ON((u64) zdev->fmb & 0xf);
 
+	zpci_fmb_clear_iommu_ctrs(zdev);
 
 	fib.fmb_addr = virt_to_phys(zdev->fmb);
 	fib.gd = zdev->gisa;
@@ -227,6 +232,41 @@ int zpci_fmb_disable_device(struct zpci_dev *zdev)
 	}
 	return cc ? -EIO : 0;
 }
+EXPORT_SYMBOL_GPL(zpci_fmb_disable_device);
+
+int zpci_fmb_reenable_device(struct zpci_dev *zdev)
+{
+	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
+	struct zpci_fib fib = {0};
+	u8 cc, status;
+
+	lockdep_assert_held(&zdev->fmb_lock);
+
+	if (!zdev->fmb)
+		return zpci_fmb_enable_device(zdev);
+
+	fib.gd = zdev->gisa;
+	cc = zpci_mod_fc(req, &fib, &status); /* Disable function measurement */
+
+	/* Unlike in zpci_fmb_disable_device(), cc == 3 is not a valid state here
+	 * because we are re-enabling function measurement for the same function
+	 * handle.
+	 */
+	if (cc)
+		return -EIO;
+
+	zpci_fmb_clear_iommu_ctrs(zdev);
+
+	fib.fmb_addr = virt_to_phys(zdev->fmb);
+	cc = zpci_mod_fc(req, &fib, &status); /* Re-enable function measurement */
+	if (cc) {
+		kmem_cache_free(zdev_fmb_cache, zdev->fmb);
+		zdev->fmb = NULL;
+		return -EIO;
+	}
+	return 0;
+}
+EXPORT_SYMBOL_GPL(zpci_fmb_reenable_device);
 
 static int zpci_cfg_load(struct zpci_dev *zdev, int offset, u32 *val, u8 len)
 {
@@ -729,9 +769,14 @@ int zpci_reenable_device(struct zpci_dev *zdev)
 	}
 
 	rc = zpci_iommu_register_ioat(zdev, &status);
-	if (rc)
+	if (rc) {
 		zpci_disable_device(zdev);
+		return rc;
+	}
 
+	mutex_lock(&zdev->fmb_lock);
+	zpci_fmb_reenable_device(zdev);
+	mutex_unlock(&zdev->fmb_lock);
 	return rc;
 }
 EXPORT_SYMBOL_GPL(zpci_reenable_device);
-- 
2.52.0

Re: [PATCH v2 1/3] s390/pci: Preserve FMB state in device re-enablement

[Niklas Schnelle]

On Tue, 2026-05-19 at 18:42 -0400, Omar Elghoul wrote:
> Introduce a function zpci_fmb_reenable_device() that checks for the state
> of the FMB and reuses the same buffer where appropriate. If FMB was not
                                                             ^ the

> previously enabled, it enables it for the device. Call this function during
> a zPCI device re-enablement, which in turn implicitly ensures that the FMB
> is enabled for host devices during their KVM registration.
> 
> This function also clears out the software counters, so that a program
> resetting an FMB would see all its counters restart from zero as expected.
> The function to clear the software counters is also separated into a static
> function as it is now reused in both zpci_fmb_enable_device() and
> zpci_fmb_reenable_device().

While the commit message starts in the correct imperative voice it then
drifts to a passive voice in the last sentence. "is also separated
into…", "it is now reused".

Better:
"Besides re-enabling the FMB itself in zpci_fmb_reenable_device() also
clear out the software counters, such that a program resetting an FMB
sees all counters start from zero as expected. Separate this clearing
of software counters out into zpci_fmb_clear_iommu_ctrs() and reuse it
in zpci_fmb_enable_device() and zpci_fmb_reenable_device()."

> 
> Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
> ---
>  arch/s390/include/asm/pci.h |  1 +
>  arch/s390/pci/pci.c         | 75 +++++++++++++++++++++++++++++--------
>  2 files changed, 61 insertions(+), 15 deletions(-)
> 
--- snip ---
> +}
> +
> +/* Modify PCI: Set PCI function measurement parameters */
> +int zpci_fmb_enable_device(struct zpci_dev *zdev)
> +{
> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
> +	struct zpci_fib fib = {0};
> +	u8 cc, status;
> +
> +	if (zdev->fmb || sizeof(*zdev->fmb) < zdev->fmb_length)
> +		return -EINVAL;
> +
> +	zdev->fmb = kmem_cache_zalloc(zdev_fmb_cache, GFP_KERNEL);
> +	if (!zdev->fmb)
> +		return -ENOMEM;
> +	WARN_ON((u64) zdev->fmb & 0xf);
>  
> +	zpci_fmb_clear_iommu_ctrs(zdev);
>  
>  	fib.fmb_addr = virt_to_phys(zdev->fmb);

I think there is still some potential for sharing the actual enable
code between zpci_fmb_enable_device() and zpci_fmb_reenable_device()
e.g. using a static zpci_fmb_do_enable(struct zpci_dev *zdev) helper.
I think that still makes it clearer what's happening too.

>  	fib.gd = zdev->gisa;
> @@ -227,6 +232,41 @@ int zpci_fmb_disable_device(struct zpci_dev *zdev)
>  	}
>  	return cc ? -EIO : 0;
>  }
> +EXPORT_SYMBOL_GPL(zpci_fmb_disable_device);
> +
> +int zpci_fmb_reenable_device(struct zpci_dev *zdev)
> +{
> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
> +	struct zpci_fib fib = {0};
> +	u8 cc, status;
> +
> +	lockdep_assert_held(&zdev->fmb_lock);

Sashiko correctly notes a pre-existing issue in that
zpci_fmb_enable_device() is called without holding zdev->fmb_lock in
pcibios_enable_device() and analogously zpci_fmb_disable_device() in
pcibios_disable_device(). This should also be caught by lockdep if we
had lockdep_assert_held() was in zpci_fmb_enable_device() respectively
zpci_fmb_disable_device(). Would you mind adding a minimal fix patch
for this pre-existing issue in your series? The fix should add the
locking in the pcibios calls as well as add lockdep_assert_held(). Also
don't forget a Fixes tag and Cc stable.

> +
> +	if (!zdev->fmb)
> +		return zpci_fmb_enable_device(zdev);
> +
> +	fib.gd = zdev->gisa;
> +	cc = zpci_mod_fc(req, &fib, &status); /* Disable function measurement */
> +
> +	/* Unlike in zpci_fmb_disable_device(), cc == 3 is not a valid state here
> +	 * because we are re-enabling function measurement for the same function
> +	 * handle.
> +	 */
> +	if (cc)
> +		return -EIO;
> +
> +	zpci_fmb_clear_iommu_ctrs(zdev);
> +
> +	fib.fmb_addr = virt_to_phys(zdev->fmb);
> +	cc = zpci_mod_fc(req, &fib, &status); /* Re-enable function measurement */
> +	if (cc) {
> +		kmem_cache_free(zdev_fmb_cache, zdev->fmb);
> +		zdev->fmb = NULL;
> +		return -EIO;
> +	}
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(zpci_fmb_reenable_device);
>  

The change itself makes sense to me.

Thanks,
Niklas

Re: [PATCH v2 1/3] s390/pci: Preserve FMB state in device re-enablement

[Omar Elghoul]

On 6/3/26 6:50 AM, Niklas Schnelle wrote:

> On Tue, 2026-05-19 at 18:42 -0400, Omar Elghoul wrote:
>> Introduce a function zpci_fmb_reenable_device() that checks for the state
>> of the FMB and reuses the same buffer where appropriate. If FMB was not
>                                                               ^ the
Acked
>
>> previously enabled, it enables it for the device. Call this function during
>> a zPCI device re-enablement, which in turn implicitly ensures that the FMB
>> is enabled for host devices during their KVM registration.
>>
>> This function also clears out the software counters, so that a program
>> resetting an FMB would see all its counters restart from zero as expected.
>> The function to clear the software counters is also separated into a static
>> function as it is now reused in both zpci_fmb_enable_device() and
>> zpci_fmb_reenable_device().
> While the commit message starts in the correct imperative voice it then
> drifts to a passive voice in the last sentence. "is also separated
> into…", "it is now reused".
>
> Better:
> "Besides re-enabling the FMB itself in zpci_fmb_reenable_device() also
> clear out the software counters, such that a program resetting an FMB
> sees all counters start from zero as expected. Separate this clearing
> of software counters out into zpci_fmb_clear_iommu_ctrs() and reuse it
> in zpci_fmb_enable_device() and zpci_fmb_reenable_device()."
Noted, I will amend this.
>
>> Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
>> ---
>>   arch/s390/include/asm/pci.h |  1 +
>>   arch/s390/pci/pci.c         | 75 +++++++++++++++++++++++++++++--------
>>   2 files changed, 61 insertions(+), 15 deletions(-)
>>
> --- snip ---
>> +}
>> +
>> +/* Modify PCI: Set PCI function measurement parameters */
>> +int zpci_fmb_enable_device(struct zpci_dev *zdev)
>> +{
>> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
>> +	struct zpci_fib fib = {0};
>> +	u8 cc, status;
>> +
>> +	if (zdev->fmb || sizeof(*zdev->fmb) < zdev->fmb_length)
>> +		return -EINVAL;
>> +
>> +	zdev->fmb = kmem_cache_zalloc(zdev_fmb_cache, GFP_KERNEL);
>> +	if (!zdev->fmb)
>> +		return -ENOMEM;
>> +	WARN_ON((u64) zdev->fmb & 0xf);
>>   
>> +	zpci_fmb_clear_iommu_ctrs(zdev);
>>   
>>   	fib.fmb_addr = virt_to_phys(zdev->fmb);
> I think there is still some potential for sharing the actual enable
> code between zpci_fmb_enable_device() and zpci_fmb_reenable_device()
> e.g. using a static zpci_fmb_do_enable(struct zpci_dev *zdev) helper.
> I think that still makes it clearer what's happening too.
Acked as well, it would definitely be easier to read. Will do in v3.
>
>>   	fib.gd = zdev->gisa;
>> @@ -227,6 +232,41 @@ int zpci_fmb_disable_device(struct zpci_dev *zdev)
>>   	}
>>   	return cc ? -EIO : 0;
>>   }
>> +EXPORT_SYMBOL_GPL(zpci_fmb_disable_device);
>> +
>> +int zpci_fmb_reenable_device(struct zpci_dev *zdev)
>> +{
>> +	u64 req = ZPCI_CREATE_REQ(zdev->fh, 0, ZPCI_MOD_FC_SET_MEASURE);
>> +	struct zpci_fib fib = {0};
>> +	u8 cc, status;
>> +
>> +	lockdep_assert_held(&zdev->fmb_lock);
> Sashiko correctly notes a pre-existing issue in that
> zpci_fmb_enable_device() is called without holding zdev->fmb_lock in
> pcibios_enable_device() and analogously zpci_fmb_disable_device() in
> pcibios_disable_device(). This should also be caught by lockdep if we
> had lockdep_assert_held() was in zpci_fmb_enable_device() respectively
> zpci_fmb_disable_device(). Would you mind adding a minimal fix patch
> for this pre-existing issue in your series? The fix should add the
> locking in the pcibios calls as well as add lockdep_assert_held(). Also
> don't forget a Fixes tag and Cc stable.
Sure thing, I will add a fix patch in v3.

Thanks.
>
>> +
>> +	if (!zdev->fmb)
>> +		return zpci_fmb_enable_device(zdev);
>> +
>> +	fib.gd = zdev->gisa;
>> +	cc = zpci_mod_fc(req, &fib, &status); /* Disable function measurement */
>> +
>> +	/* Unlike in zpci_fmb_disable_device(), cc == 3 is not a valid state here
>> +	 * because we are re-enabling function measurement for the same function
>> +	 * handle.
>> +	 */
>> +	if (cc)
>> +		return -EIO;
>> +
>> +	zpci_fmb_clear_iommu_ctrs(zdev);
>> +
>> +	fib.fmb_addr = virt_to_phys(zdev->fmb);
>> +	cc = zpci_mod_fc(req, &fib, &status); /* Re-enable function measurement */
>> +	if (cc) {
>> +		kmem_cache_free(zdev_fmb_cache, zdev->fmb);
>> +		zdev->fmb = NULL;
>> +		return -EIO;
>> +	}
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(zpci_fmb_reenable_device);
>>   
> The change itself makes sense to me.
>
> Thanks,
> Niklas

[PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Omar Elghoul]

Set up a new VFIO feature for zPCI devices to share the latest FMB snapshot
with userspace. This feature supports the same 4 FMB formats (0 through 3)
that are already supported by the kernel.

With VFIO_DEVICE_FEATURE_GET, allow the user driver to read the latest FMB
snapshot as well as query whether the FMB is currently enabled on the
function, itself indicating whether the FMB snapshot is valid. On the other
hand, with VFIO_DEVICE_FEATURE_SET, the userspace driver can enable or
disable the FMB.

Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
---
 drivers/vfio/pci/vfio_pci_core.c |  2 +
 drivers/vfio/pci/vfio_pci_priv.h |  9 ++++
 drivers/vfio/pci/vfio_pci_zdev.c | 77 ++++++++++++++++++++++++++++++++
 include/uapi/linux/vfio.h        | 43 ++++++++++++++++++
 4 files changed, 131 insertions(+)

diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index 050e7542952e..07e13667d66a 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1569,6 +1569,8 @@ int vfio_pci_core_ioctl_feature(struct vfio_device *device, u32 flags,
 		return vfio_pci_core_feature_token(vdev, flags, arg, argsz);
 	case VFIO_DEVICE_FEATURE_DMA_BUF:
 		return vfio_pci_core_feature_dma_buf(vdev, flags, arg, argsz);
+	case VFIO_DEVICE_FEATURE_ZPCI_FMB:
+		return vfio_pci_zdev_feature_fmb(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 fca9d0dfac90..208e05942b48 100644
--- a/drivers/vfio/pci/vfio_pci_priv.h
+++ b/drivers/vfio/pci/vfio_pci_priv.h
@@ -93,6 +93,8 @@ int vfio_pci_info_zdev_add_caps(struct vfio_pci_core_device *vdev,
 				struct vfio_info_cap *caps);
 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_fmb(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)
@@ -107,6 +109,13 @@ static inline int vfio_pci_zdev_open_device(struct vfio_pci_core_device *vdev)
 
 static inline void vfio_pci_zdev_close_device(struct vfio_pci_core_device *vdev)
 {}
+
+static inline int vfio_pci_zdev_feature_fmb(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 0990fdb146b7..1e9efe2bee69 100644
--- a/drivers/vfio/pci/vfio_pci_zdev.c
+++ b/drivers/vfio/pci/vfio_pci_zdev.c
@@ -167,3 +167,80 @@ 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(struct vfio_pci_core_device *vdev, u32 flags,
+			      void __user *arg, size_t argsz)
+{
+	struct zpci_dev *zdev;
+	struct vfio_device_feature_zpci_fmb fmb = {0};
+	u32 ops = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_SET;
+	int ret;
+
+	ret = vfio_check_feature(flags, argsz, ops, sizeof(fmb));
+	if (ret != 1)
+		return ret;
+
+	zdev = to_zpci(vdev->pdev);
+	if (!zdev)
+		return -ENODEV;
+
+	mutex_lock(&zdev->fmb_lock);
+	if (flags & VFIO_DEVICE_FEATURE_SET) {
+		if (copy_from_user(&fmb, arg, sizeof(fmb))) {
+			ret = -EFAULT;
+			goto release_lock;
+		}
+
+		if (fmb.flags & VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED)
+			ret = zpci_fmb_reenable_device(zdev);
+		else
+			ret = zpci_fmb_disable_device(zdev);
+		goto release_lock;
+	}
+
+	ret = 0;
+	if (zdev->fmb) {
+		fmb.flags |= VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
+	} else {
+		fmb.flags &= ~VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
+		goto release_lock;
+	}
+
+	fmb.format = zdev->fmb->format;
+	fmb.fmt_ind = zdev->fmb->fmt_ind;
+	fmb.samples = zdev->fmb->samples;
+	fmb.last_update = zdev->fmb->last_update;
+	fmb.ld_ops = zdev->fmb->ld_ops;
+	fmb.st_ops = zdev->fmb->st_ops;
+	fmb.stb_ops = zdev->fmb->stb_ops;
+	fmb.rpcit_ops = zdev->fmb->rpcit_ops;
+
+	switch (zdev->fmb->format) {
+	case 0:
+		if (zdev->fmb->fmt_ind & ZPCI_FMB_DMA_COUNTER_VALID) {
+			fmb.fmt0.dma_rbytes = zdev->fmb->fmt0.dma_rbytes;
+			fmb.fmt0.dma_wbytes = zdev->fmb->fmt0.dma_wbytes;
+		}
+		break;
+	case 1:
+		fmb.fmt1.rx_bytes = zdev->fmb->fmt1.rx_bytes;
+		fmb.fmt1.rx_packets = zdev->fmb->fmt1.rx_packets;
+		fmb.fmt1.tx_bytes = zdev->fmb->fmt1.tx_bytes;
+		fmb.fmt1.tx_packets = zdev->fmb->fmt1.tx_packets;
+		break;
+	case 2:
+		fmb.fmt2.consumed_work_units = zdev->fmb->fmt2.consumed_work_units;
+		fmb.fmt2.max_work_units = zdev->fmb->fmt2.max_work_units;
+		break;
+	case 3:
+		fmb.fmt3.tx_bytes = zdev->fmb->fmt3.tx_bytes;
+		break;
+	}
+
+	if (copy_to_user(arg, &fmb, sizeof(fmb)))
+		ret = -EFAULT;
+
+release_lock:
+	mutex_unlock(&zdev->fmb_lock);
+	return ret;
+}
diff --git a/include/uapi/linux/vfio.h b/include/uapi/linux/vfio.h
index 5de618a3a5ee..6cbc34ff063e 100644
--- a/include/uapi/linux/vfio.h
+++ b/include/uapi/linux/vfio.h
@@ -1534,6 +1534,49 @@ struct vfio_device_feature_dma_buf {
  */
 #define VFIO_DEVICE_FEATURE_MIG_PRECOPY_INFOv2  12
 
+/**
+ * Upon VFIO_DEVICE_FEATURE_GET, provide FMB passthrough for VFIO zPCI devices.
+ *
+ * Upon VFIO_DEVICE_FEATURE_SET, only the flags field is read while the
+ * remainder of the structure is ignored. This allows the driver to enable or
+ * disable the FMB while also leaving reserved bits for future flag expansion.
+ * All reserved fields should be zero for future compatibility.
+ */
+#define VFIO_DEVICE_FEATURE_ZPCI_FMB 13
+#define VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED	0x1
+
+struct vfio_device_feature_zpci_fmb {
+	__u64 flags;
+	__u32 format:	8;
+	__u32 fmt_ind:	24;
+	__u32 samples;
+	__u64 last_update;
+	__u64 ld_ops;
+	__u64 st_ops;
+	__u64 stb_ops;
+	__u64 rpcit_ops;
+	union {
+		struct {
+			__u64 dma_rbytes;
+			__u64 dma_wbytes;
+		} fmt0;
+		struct {
+			__u64 rx_bytes;
+			__u64 rx_packets;
+			__u64 tx_bytes;
+			__u64 tx_packets;
+		} fmt1;
+		struct {
+			__u64 consumed_work_units;
+			__u64 max_work_units;
+		} fmt2;
+		struct {
+			__u64 tx_bytes;
+		} fmt3;
+	};
+	__u64 reserved[16];
+};
+
 /* -------- API for Type1 VFIO IOMMU -------- */
 
 /**
-- 
2.52.0

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Alex Williamson]

On Tue, 19 May 2026 18:42:03 -0400
Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
> index 0990fdb146b7..1e9efe2bee69 100644
> --- a/drivers/vfio/pci/vfio_pci_zdev.c
> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
> @@ -167,3 +167,80 @@ 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(struct vfio_pci_core_device *vdev, u32 flags,
> +			      void __user *arg, size_t argsz)
> +{
> +	struct zpci_dev *zdev;
> +	struct vfio_device_feature_zpci_fmb fmb = {0};
> +	u32 ops = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_SET;

Somewhat gratuitous variable usage.

> +	int ret;
> +
> +	ret = vfio_check_feature(flags, argsz, ops, sizeof(fmb));
> +	if (ret != 1)
> +		return ret;
> +
> +	zdev = to_zpci(vdev->pdev);
> +	if (!zdev)
> +		return -ENODEV;
> +
> +	mutex_lock(&zdev->fmb_lock);

Use a guard and avoid the release_lock gotos.

> +	if (flags & VFIO_DEVICE_FEATURE_SET) {
> +		if (copy_from_user(&fmb, arg, sizeof(fmb))) {
> +			ret = -EFAULT;
> +			goto release_lock;
> +		}
> +
> +		if (fmb.flags & VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED)
> +			ret = zpci_fmb_reenable_device(zdev);
> +		else
> +			ret = zpci_fmb_disable_device(zdev);
> +		goto release_lock;

Remaining flag bits are not tested, breaks any future expanded use of
flags.

Why does the user need to be able to control these?

Doesn't allowing the user to disable FMB remove guaranteed host-based
monitoring?

Since this is already provided via debugfs, why not make this a
userspace problem to interact with the existing interface?

Alternatively, couldn't the existing zpci mediation be extended to
support the guest registering a fmb buffer to be written at regular
intervals (the interface here seems to drop the reporting interval).

> +	}
> +
> +	ret = 0;
> +	if (zdev->fmb) {
> +		fmb.flags |= VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
> +	} else {
> +		fmb.flags &= ~VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
> +		goto release_lock;
> +	}

Flag bit is cleared, goto skips copy-to-user, returns success...

Regardless of what's documented in the header, the buffer should be
assumed to be userspace garbage.  Failing to set or clear the entire
flags field precludes any future use.

Why do we need to use flags to indicate the enable state?  Couldn't we
just as easily have success indicate enabled and -ENOMSG indicate
disabled?  Thanks,

Alex

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Omar Elghoul]

On 6/2/26 6:24 PM, Alex Williamson wrote:

> On Tue, 19 May 2026 18:42:03 -0400
> Omar Elghoul <oelghoul@linux.ibm.com> wrote:
>> diff --git a/drivers/vfio/pci/vfio_pci_zdev.c b/drivers/vfio/pci/vfio_pci_zdev.c
>> index 0990fdb146b7..1e9efe2bee69 100644
>> --- a/drivers/vfio/pci/vfio_pci_zdev.c
>> +++ b/drivers/vfio/pci/vfio_pci_zdev.c
>> @@ -167,3 +167,80 @@ 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(struct vfio_pci_core_device *vdev, u32 flags,
>> +			      void __user *arg, size_t argsz)
>> +{
>> +	struct zpci_dev *zdev;
>> +	struct vfio_device_feature_zpci_fmb fmb = {0};
>> +	u32 ops = VFIO_DEVICE_FEATURE_GET | VFIO_DEVICE_FEATURE_SET;
> Somewhat gratuitous variable usage.
Acked, will remove in v3.
>
>> +	int ret;
>> +
>> +	ret = vfio_check_feature(flags, argsz, ops, sizeof(fmb));
>> +	if (ret != 1)
>> +		return ret;
>> +
>> +	zdev = to_zpci(vdev->pdev);
>> +	if (!zdev)
>> +		return -ENODEV;
>> +
>> +	mutex_lock(&zdev->fmb_lock);
> Use a guard and avoid the release_lock gotos.
Acked as well
>
>> +	if (flags & VFIO_DEVICE_FEATURE_SET) {
>> +		if (copy_from_user(&fmb, arg, sizeof(fmb))) {
>> +			ret = -EFAULT;
>> +			goto release_lock;
>> +		}
>> +
>> +		if (fmb.flags & VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED)
>> +			ret = zpci_fmb_reenable_device(zdev);
>> +		else
>> +			ret = zpci_fmb_disable_device(zdev);
>> +		goto release_lock;
> Remaining flag bits are not tested, breaks any future expanded use of
> flags.
Oversight on my part, I will fix this.
>
> Why does the user need to be able to control these?
We want the user (e.g. QEMU) to be able to control these so that when a
guest enables or disables the FMB, this state gets cascaded to the host and
all the way to the firmware.
>
> Doesn't allowing the user to disable FMB remove guaranteed host-based
> monitoring?
Yes it does, but this one isn't an oversight and is intentional behavior
to achieve the functionality mentioned above. The host-based monitoring is
not necessarily guaranteed and is treated as a device-specific state, so it
makes sense in the case of passthrough to have that state reflect the state
of the guest that is actually using the device.
>
> Since this is already provided via debugfs, why not make this a
> userspace problem to interact with the existing interface?
It might be possible but it would undoubtedly be really ugly and harder to
maintain. I think what we'd dislike most about using debugfs is parsing
text data into the FMB structure. If any of the text representations of the
fields were to change, we would need to update them anywhere that uses them
(e.g., in QEMU or any other user driver). The ABI would be super fragile.
>
> Alternatively, couldn't the existing zpci mediation be extended to
> support the guest registering a fmb buffer to be written at regular
> intervals (the interface here seems to drop the reporting interval).
The firmware only writes the FMB into one buffer every firmware-specified
interval. If we wanted to write the FMB directly into guest memory, we
would either 1) lose host access to the FMB or 2) have to run a periodic
worker in the kernel to copy the host FMB into the guest-provided buffer
every time the firmware does an update. I don't believe either of these
approaches are favorable.

WRT reporting interval, I intentionally dropped that one as it is already
provided by VFIO_DEVICE_INFO_CAP_ZPCI_GROUP.
>
>> +	}
>> +
>> +	ret = 0;
>> +	if (zdev->fmb) {
>> +		fmb.flags |= VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
>> +	} else {
>> +		fmb.flags &= ~VFIO_DEVICE_FEATURE_ZPCI_FMB_FLAGS_ENABLED;
>> +		goto release_lock;
>> +	}
> Flag bit is cleared, goto skips copy-to-user, returns success...
I don't know how I missed this one... I'll fix it or remove it entirely by
using the guard like you suggested earlier.
>
> Regardless of what's documented in the header, the buffer should be
> assumed to be userspace garbage.  Failing to set or clear the entire
> flags field precludes any future use.
Acked.
>
> Why do we need to use flags to indicate the enable state?  Couldn't we
> just as easily have success indicate enabled and -ENOMSG indicate
> disabled?  Thanks,
Good point, I had not previously considered using -ENOMSG. It makes sense
and would for sure simplify this feature.

Thanks.
>
> Alex

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Niklas Schnelle]

On Wed, 2026-06-03 at 08:35 -0400, Omar Elghoul wrote:
> On 6/2/26 6:24 PM, Alex Williamson wrote:
> 
> > On Tue, 19 May 2026 18:42:03 -0400
> > Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> > > 
--- snip ---
> > 
> > Since this is already provided via debugfs, why not make this a
> > userspace problem to interact with the existing interface?
>
> It might be possible but it would undoubtedly be really ugly and harder to
> maintain. I think what we'd dislike most about using debugfs is parsing
> text data into the FMB structure. If any of the text representations of the
> fields were to change, we would need to update them anywhere that uses them
> (e.g., in QEMU or any other user driver). The ABI would be super fragile.

I agree with Omar. The documentation for debugfs ([0] first paragraph)
even says that the files are not supposed to be treated as stable so
I'd really like to avoid treating the statistics as stable ourselves.

Thanks,
Niklas

[0] https://www.kernel.org/doc/Documentation/filesystems/debugfs.txt

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Alex Williamson]

On Wed, 3 Jun 2026 08:35:43 -0400
Omar Elghoul <oelghoul@linux.ibm.com> wrote:

> On 6/2/26 6:24 PM, Alex Williamson wrote:
> >
> > Why does the user need to be able to control these?  
> We want the user (e.g. QEMU) to be able to control these so that when a
> guest enables or disables the FMB, this state gets cascaded to the host and
> all the way to the firmware.
> >
> > Doesn't allowing the user to disable FMB remove guaranteed host-based
> > monitoring?  
> Yes it does, but this one isn't an oversight and is intentional behavior
> to achieve the functionality mentioned above. The host-based monitoring is
> not necessarily guaranteed and is treated as a device-specific state, so it
> makes sense in the case of passthrough to have that state reflect the state
> of the guest that is actually using the device.

If we really need a SET for enable/disable, I think it should be a
separate feature.  It really makes no sense to pass a giant structure
into a SET operation to look at the state of one flag bit.

> > Since this is already provided via debugfs, why not make this a
> > userspace problem to interact with the existing interface?  
> It might be possible but it would undoubtedly be really ugly and harder to
> maintain. I think what we'd dislike most about using debugfs is parsing
> text data into the FMB structure. If any of the text representations of the
> fields were to change, we would need to update them anywhere that uses them
> (e.g., in QEMU or any other user driver). The ABI would be super fragile.
> >
> > Alternatively, couldn't the existing zpci mediation be extended to
> > support the guest registering a fmb buffer to be written at regular
> > intervals (the interface here seems to drop the reporting interval).  
> The firmware only writes the FMB into one buffer every firmware-specified
> interval. If we wanted to write the FMB directly into guest memory, we
> would either 1) lose host access to the FMB or 2) have to run a periodic
> worker in the kernel to copy the host FMB into the guest-provided buffer
> every time the firmware does an update. I don't believe either of these
> approaches are favorable.
> 
> WRT reporting interval, I intentionally dropped that one as it is already
> provided by VFIO_DEVICE_INFO_CAP_ZPCI_GROUP.

Hmm, I also see fmb_length in VFIO_DEVICE_INFO_CAP_ZPCI_BASE.  If we
have that, do we really need structured data in the GET feature?  Maybe
GET just provides a user pointer and the raw fmb data is copied to it.
Thanks,

Alex

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Omar Elghoul]

On 6/3/26 11:55 AM, Alex Williamson wrote:
> On Wed, 3 Jun 2026 08:35:43 -0400
> Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> 
>> On 6/2/26 6:24 PM, Alex Williamson wrote:
>>>
>>> Why does the user need to be able to control these?
>> We want the user (e.g. QEMU) to be able to control these so that when a
>> guest enables or disables the FMB, this state gets cascaded to the host and
>> all the way to the firmware.
>>>
>>> Doesn't allowing the user to disable FMB remove guaranteed host-based
>>> monitoring?
>> Yes it does, but this one isn't an oversight and is intentional behavior
>> to achieve the functionality mentioned above. The host-based monitoring is
>> not necessarily guaranteed and is treated as a device-specific state, so it
>> makes sense in the case of passthrough to have that state reflect the state
>> of the guest that is actually using the device.
> 
> If we really need a SET for enable/disable, I think it should be a
> separate feature.  It really makes no sense to pass a giant structure
> into a SET operation to look at the state of one flag bit.
> 
> [...]
> 
> Hmm, I also see fmb_length in VFIO_DEVICE_INFO_CAP_ZPCI_BASE.  If we
> have that, do we really need structured data in the GET feature?  Maybe
> GET just provides a user pointer and the raw fmb data is copied to it.

If we did this and passed just flags, a user ptr, and possibly a buffer
length field, what would you think of leaving them in one feature? This
way, the SET case would have possibly 8 or 16 bytes of overhead rather
than the entire FMB structs, but would still keep the uAPI simple enough
by avoiding multiple VFIO features for the same firmware feature.

Thanks.

(Apologies for the resend - the previous email was rejected for being 
sent in HTML format.)

> Thanks,
> 
> Alex

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Alex Williamson]

On Wed, 3 Jun 2026 14:26:30 -0400
Omar Elghoul <oelghoul@linux.ibm.com> wrote:

> On 6/3/26 11:55 AM, Alex Williamson wrote:
> > On Wed, 3 Jun 2026 08:35:43 -0400
> > Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> >   
> >> On 6/2/26 6:24 PM, Alex Williamson wrote:  
> >>>
> >>> Why does the user need to be able to control these?  
> >> We want the user (e.g. QEMU) to be able to control these so that when a
> >> guest enables or disables the FMB, this state gets cascaded to the host and
> >> all the way to the firmware.  
> >>>
> >>> Doesn't allowing the user to disable FMB remove guaranteed host-based
> >>> monitoring?  
> >> Yes it does, but this one isn't an oversight and is intentional behavior
> >> to achieve the functionality mentioned above. The host-based monitoring is
> >> not necessarily guaranteed and is treated as a device-specific state, so it
> >> makes sense in the case of passthrough to have that state reflect the state
> >> of the guest that is actually using the device.  
> > 
> > If we really need a SET for enable/disable, I think it should be a
> > separate feature.  It really makes no sense to pass a giant structure
> > into a SET operation to look at the state of one flag bit.
> > 
> > [...]
> > 
> > Hmm, I also see fmb_length in VFIO_DEVICE_INFO_CAP_ZPCI_BASE.  If we
> > have that, do we really need structured data in the GET feature?  Maybe
> > GET just provides a user pointer and the raw fmb data is copied to it.  
> 
> If we did this and passed just flags, a user ptr, and possibly a buffer
> length field, what would you think of leaving them in one feature? This
> way, the SET case would have possibly 8 or 16 bytes of overhead rather
> than the entire FMB structs, but would still keep the uAPI simple enough
> by avoiding multiple VFIO features for the same firmware feature.

It doesn't seem the GET needs either flags or buffer length.  The data
is opaque through vfio, so there's nothing to flag.  The buffer size is
at best a sanity check, it has no actual bearing on the copy to user
buffer.  We're not writing to a variable length ioctl buffer, we're
writing out to the user pointer.  The feature only needs to be
consistent that it copies no more than fmb_length.

The combined SET/GET that perform different actions especially stands
out because of the structure, but I don't think making the structure
size more manageable resolves that they do very different things.  I
think the implementation is also much easier if GET simply dumps the
FMB to the user pointer and SET takes only a scalar enable/disable
value, ie. a fundamental type that's handled as a bool.  Thanks,

Alex

Re: [PATCH v2 2/3] vfio-pci/zdev: Add VFIO FMB device feature

[Omar Elghoul]

On 6/3/26 3:24 PM, Alex Williamson wrote:
> On Wed, 3 Jun 2026 14:26:30 -0400
> Omar Elghoul <oelghoul@linux.ibm.com> wrote:
> 
>> On 6/3/26 11:55 AM, Alex Williamson wrote:
>>> On Wed, 3 Jun 2026 08:35:43 -0400
>>> Omar Elghoul <oelghoul@linux.ibm.com> wrote:
>>>    
>>>> On 6/2/26 6:24 PM, Alex Williamson wrote:
>>>>>
>>>>> Why does the user need to be able to control these?
>>>> We want the user (e.g. QEMU) to be able to control these so that when a
>>>> guest enables or disables the FMB, this state gets cascaded to the host and
>>>> all the way to the firmware.
>>>>>
>>>>> Doesn't allowing the user to disable FMB remove guaranteed host-based
>>>>> monitoring?
>>>> Yes it does, but this one isn't an oversight and is intentional behavior
>>>> to achieve the functionality mentioned above. The host-based monitoring is
>>>> not necessarily guaranteed and is treated as a device-specific state, so it
>>>> makes sense in the case of passthrough to have that state reflect the state
>>>> of the guest that is actually using the device.
>>>
>>> If we really need a SET for enable/disable, I think it should be a
>>> separate feature.  It really makes no sense to pass a giant structure
>>> into a SET operation to look at the state of one flag bit.
>>>
>>> [...]
>>>
>>> Hmm, I also see fmb_length in VFIO_DEVICE_INFO_CAP_ZPCI_BASE.  If we
>>> have that, do we really need structured data in the GET feature?  Maybe
>>> GET just provides a user pointer and the raw fmb data is copied to it.
>>
>> If we did this and passed just flags, a user ptr, and possibly a buffer
>> length field, what would you think of leaving them in one feature? This
>> way, the SET case would have possibly 8 or 16 bytes of overhead rather
>> than the entire FMB structs, but would still keep the uAPI simple enough
>> by avoiding multiple VFIO features for the same firmware feature.
> 
> It doesn't seem the GET needs either flags or buffer length.  The data
> is opaque through vfio, so there's nothing to flag.  The buffer size is
> at best a sanity check, it has no actual bearing on the copy to user
> buffer.  We're not writing to a variable length ioctl buffer, we're
> writing out to the user pointer.  The feature only needs to be
> consistent that it copies no more than fmb_length.
> 
> The combined SET/GET that perform different actions especially stands
> out because of the structure, but I don't think making the structure
> size more manageable resolves that they do very different things.  I
> think the implementation is also much easier if GET simply dumps the
> FMB to the user pointer and SET takes only a scalar enable/disable
> value, ie. a fundamental type that's handled as a bool.  Thanks,

Acked, I will keep this in mind for future versions.

Thanks.

> 
> Alex
> 

[PATCH v2 3/3] s390/pci: Fence FMB enable/disable via sysfs for passthrough devices

[Omar Elghoul]

Introduce a fence over enabling or disabling FMB via sysfs when the zPCI
device is associated with a KVM. This will allow a KVM guest to use FMB
passthrough and avoid the edge-case where the host disables FMB while the
guest is still using it, which may cause partial counter resets and
inconsistent reads which have no parallel in the architecture.

With this patch, the userspace driver, likely QEMU, is still able to enable
or disable the FMB using the VFIO device feature introduced in the previous
patch, effectively securing what is associated with the VM state and
isolating it from other processes on the host.

For VFIO devices that are not associated with a KVM (i.e., for userspace
drivers other than QEMU), this fence does not take effect.

Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
---
 arch/s390/pci/pci_debug.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/s390/pci/pci_debug.c b/arch/s390/pci/pci_debug.c
index c7ed7bf254b5..2601614b919b 100644
--- a/arch/s390/pci/pci_debug.c
+++ b/arch/s390/pci/pci_debug.c
@@ -149,9 +149,15 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
 	if (!zdev)
 		return 0;
 
+	mutex_lock(&zdev->kzdev_lock);
+	if (zdev->kzdev) {
+		rc = -EPERM;
+		goto release_kzdev_and_out;
+	}
+
 	rc = kstrtoul_from_user(ubuf, count, 10, &val);
 	if (rc)
-		return rc;
+		goto release_kzdev_and_out;
 
 	mutex_lock(&zdev->fmb_lock);
 	switch (val) {
@@ -163,6 +169,9 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
 		break;
 	}
 	mutex_unlock(&zdev->fmb_lock);
+
+release_kzdev_and_out:
+	mutex_unlock(&zdev->kzdev_lock);
 	return rc ? rc : count;
 }
 
-- 
2.52.0

Re: [PATCH v2 3/3] s390/pci: Fence FMB enable/disable via sysfs for passthrough devices

[Niklas Schnelle]

On Tue, 2026-05-19 at 18:42 -0400, Omar Elghoul wrote:
> Introduce a fence over enabling or disabling FMB via sysfs when the zPCI
> device is associated with a KVM. This will allow a KVM guest to use FMB
> passthrough and avoid the edge-case where the host disables FMB while the
> guest is still using it, which may cause partial counter resets and
> inconsistent reads which have no parallel in the architecture.
> 
> With this patch, the userspace driver, likely QEMU, is still able to enable
> or disable the FMB using the VFIO device feature introduced in the previous
> patch, effectively securing what is associated with the VM state and
> isolating it from other processes on the host.
> 
> For VFIO devices that are not associated with a KVM (i.e., for userspace
> drivers other than QEMU), this fence does not take effect.
> 
> Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
> ---
>  arch/s390/pci/pci_debug.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/s390/pci/pci_debug.c b/arch/s390/pci/pci_debug.c
> index c7ed7bf254b5..2601614b919b 100644
> --- a/arch/s390/pci/pci_debug.c
> +++ b/arch/s390/pci/pci_debug.c
> @@ -149,9 +149,15 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
>  	if (!zdev)
>  		return 0;
>  
> +	mutex_lock(&zdev->kzdev_lock);
> +	if (zdev->kzdev) {
> +		rc = -EPERM;
> +		goto release_kzdev_and_out;

Nit: "release" to me sounds misleading here since it's not about any of
the things called "release" in the kernel but about unlocking a mutex,
I'd probably go with "out_unlock_kzdev" for the label.

> +	}
> +
>  	rc = kstrtoul_from_user(ubuf, count, 10, &val);
>  	if (rc)
> -		return rc;
> +		goto release_kzdev_and_out;
>  
>  	mutex_lock(&zdev->fmb_lock);
>  	switch (val) {
> @@ -163,6 +169,9 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
>  		break;
>  	}
>  	mutex_unlock(&zdev->fmb_lock);
> +
> +release_kzdev_and_out:
> +	mutex_unlock(&zdev->kzdev_lock);
>  	return rc ? rc : count;
>  }
>  

Can't say I love having to fence the host from being able to disable
the FMB via sysfs, but the potential inconsistencies really aren't
covered by the architecture and I don't see a better way.

So with the nit handled feel free to add:

Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>

Thanks,
Niklas

Re: [PATCH v2 3/3] s390/pci: Fence FMB enable/disable via sysfs for passthrough devices

[Omar Elghoul]

On 6/3/26 8:15 AM, Niklas Schnelle wrote:

> On Tue, 2026-05-19 at 18:42 -0400, Omar Elghoul wrote:
>> Introduce a fence over enabling or disabling FMB via sysfs when the zPCI
>> device is associated with a KVM. This will allow a KVM guest to use FMB
>> passthrough and avoid the edge-case where the host disables FMB while the
>> guest is still using it, which may cause partial counter resets and
>> inconsistent reads which have no parallel in the architecture.
>>
>> With this patch, the userspace driver, likely QEMU, is still able to enable
>> or disable the FMB using the VFIO device feature introduced in the previous
>> patch, effectively securing what is associated with the VM state and
>> isolating it from other processes on the host.
>>
>> For VFIO devices that are not associated with a KVM (i.e., for userspace
>> drivers other than QEMU), this fence does not take effect.
>>
>> Signed-off-by: Omar Elghoul <oelghoul@linux.ibm.com>
>> ---
>>   arch/s390/pci/pci_debug.c | 11 ++++++++++-
>>   1 file changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/s390/pci/pci_debug.c b/arch/s390/pci/pci_debug.c
>> index c7ed7bf254b5..2601614b919b 100644
>> --- a/arch/s390/pci/pci_debug.c
>> +++ b/arch/s390/pci/pci_debug.c
>> @@ -149,9 +149,15 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
>>   	if (!zdev)
>>   		return 0;
>>   
>> +	mutex_lock(&zdev->kzdev_lock);
>> +	if (zdev->kzdev) {
>> +		rc = -EPERM;
>> +		goto release_kzdev_and_out;
> Nit: "release" to me sounds misleading here since it's not about any of
> the things called "release" in the kernel but about unlocking a mutex,
> I'd probably go with "out_unlock_kzdev" for the label.
Noted, I will rename it.
>
>> +	}
>> +
>>   	rc = kstrtoul_from_user(ubuf, count, 10, &val);
>>   	if (rc)
>> -		return rc;
>> +		goto release_kzdev_and_out;
>>   
>>   	mutex_lock(&zdev->fmb_lock);
>>   	switch (val) {
>> @@ -163,6 +169,9 @@ static ssize_t pci_perf_seq_write(struct file *file, const char __user *ubuf,
>>   		break;
>>   	}
>>   	mutex_unlock(&zdev->fmb_lock);
>> +
>> +release_kzdev_and_out:
>> +	mutex_unlock(&zdev->kzdev_lock);
>>   	return rc ? rc : count;
>>   }
>>   
> Can't say I love having to fence the host from being able to disable
> the FMB via sysfs, but the potential inconsistencies really aren't
> covered by the architecture and I don't see a better way.
>
> So with the nit handled feel free to add:
>
> Reviewed-by: Niklas Schnelle <schnelle@linux.ibm.com>
Thanks.
>
> Thanks,
> Niklas
>

Re: [PATCH v2 0/3] vfio-pci/zdev: Improved zPCI Function Measurement Support

[Omar Elghoul]

Hi,

Gently pinging the series in case it has fallen out of your radars.

Thanks.

On 5/19/26 6:42 PM, Omar Elghoul wrote:
> Hi,
>
> This patch series improves support for function measurement for zPCI
> passthrough devices on s390x.
>
> Changelog
> =========
> v1 -> v2:
> * Patch 1/3:
>    - Address a possible race condition in zpci_reenable_device() caused by
>      calling zpci_fmb_reenable_device() without holding fmb_lock
>    - Assert that fmb_lock is held within zpci_fmb_reenable_device()
>
> * Patch 3/3:
>    - Address a possible race condition in pci_perf_seq_write() caused by
>      consuming zdev->kzdev without holding kzdev_lock
>
> Motivation
> ==========
> The firmware on s390x machines allows for tracking a variety of statistics
> relating to zPCI devices in a function measurement block (FMB). However,
> the kernel currently lacks a structured mechanism of sharing this
> information with userspace, beyond /sys/kernel/debug/pci/ID/statistics.
> This can lead to shortcomings when running a guest on KVM with PCI
> passthrough devices, as QEMU is unable to provide an accurate FMB snapshot
> to the guest.
>
> Proposal
> ========
> We propose adding a new VFIO device feature to zPCI passthrough devices,
> allowing userspace programs to read the latest FMB snapshot as it is
> written by the firmware. We ensure that function measurement enablement is
> preserved across device resets on the host. Furthermore, we guard against
> host tampering with the FMB via sysfs when the zPCI device is in
> passthrough to protect the VM's state.
>
> I'd appreciate some feedback on these patches.
>
> Thanks in advance.
>
> Omar Elghoul (3):
>    s390/pci: Preserve FMB state in device re-enablement
>    vfio-pci/zdev: Add VFIO FMB device feature
>    s390/pci: Fence FMB enable/disable via sysfs for passthrough devices
>
>   arch/s390/include/asm/pci.h      |  1 +
>   arch/s390/pci/pci.c              | 75 ++++++++++++++++++++++++-------
>   arch/s390/pci/pci_debug.c        | 11 ++++-
>   drivers/vfio/pci/vfio_pci_core.c |  2 +
>   drivers/vfio/pci/vfio_pci_priv.h |  9 ++++
>   drivers/vfio/pci/vfio_pci_zdev.c | 77 ++++++++++++++++++++++++++++++++
>   include/uapi/linux/vfio.h        | 43 ++++++++++++++++++
>   7 files changed, 202 insertions(+), 16 deletions(-)
>