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@xxxxxxxxxxxxx>
> > ---
> >   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@xxxxxxxxxxxxx>
Thanks.
> Thanks,
> Niklas