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

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

On 6/9/26 6:52 PM, Alex Williamson wrote:
> On Mon,  8 Jun 2026 13:18:50 -0400
> Omar Elghoul <oelghoul@linux.ibm.com> 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>
>> Reviewed-by: Niklas Schnelle <schnelle@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..a2dc79418c21 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 out_unlock_kzdev;
>> +	}
>> +
>>   	rc = kstrtoul_from_user(ubuf, count, 10, &val);
>>   	if (rc)
>> -		return rc;
>> +		goto out_unlock_kzdev;
>>   
>>   	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);
>> +
>> +out_unlock_kzdev:
>> +	mutex_unlock(&zdev->kzdev_lock);
>>   	return rc ? rc : count;
>>   }
>>   
> 
> Why not also use a guard for the mutex here and avoid the goto
> unlock... also moving the guard below the kstrtoul_from_user()?
> 
> The fmb_lock could switch to a guard too, but that's existing.

 From where I'm standing I don't think there is any particular reason to
do it one way vs the other. Thanks.

> Thanks,
> 
> Alex
>