Re: [PATCH v6 2/4] s390/pci: Reuse FMB buffer and preserve state in device re-enablement

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

On 9/17/26 11:28 AM, Matthew Rosato wrote:
> On 9/17/26 9:32 AM, Omar Elghoul wrote:
>> On 9/16/26 10:21 PM, Matthew Rosato wrote:
>>>
>>>> +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_enabled)
>>>> +        return zpci_fmb_enable_device(zdev);
>>>
>>> Sashiko has a point here, won't all paths that call zpci_reenable_device
>>> now unconditionally turn on FMB here even if it was purposely disabled?
>>>
>>> The pattern of callers for zpci_reenable_device() is:
>>>
>>> zpci_disable_device()
>>> ...
>>> zpci_reenable_device()
>>>
>>> We only care to re-enable the FMB during device re-enable if the device
>>> had FMB enabled at the time zpci_disable_device() was called.
>>
>> The way I thought of it was pcibios_enable_device() already enables
>> measurement, so we might as well ensure it is enabled when the zdev is
> 
> Yes, FMB starts enabled by default but then it could be disabled
> intentionally via sysfs; if a recovery action occurs on the device
> sometime after I manually disabled FMB for a device, I'd expect it to
> still be disabled after the recovery action but AFAICT with this
> codepath it will always get re-enabled.
> 
> Completely removing the device and re-introducing it to the system is a
> different matter; in that case yeah we'd be back to FMB-enabled because
> that's our default starting state via pcibios_enable_device().

Makes sense, I will fix this in the next version.

> 
>> re-enabled. That default state is also helpful for the later patches
>> because kvm_s390_pci_register_kvm() calls zpci_reenable_device().
>>
>> But I guess if we didn't want to conflate the two, we could also add an
>> unconditional zpci_fmb_reenable_device() within the KVM register hook,
>> to run after the zpci_reenable_device(), whichever you'd prefer.
> 
> Do we even want it to be part of the KVM register hook at all, or could
> we leave it off until we get a VFIO_DEVICE_FEATURE_ZPCI_FMB_ENABLE?
> 
> That way if a guest isn't using FMB (we never intercepted the associated
> MPCIFC to enable measurement), QEMU would never issue
> VFIO_DEVICE_FEATURE_ZPCI_FMB_ENABLE and then we just leave the host FMB off.
> 
> In practice, yes, FMB is always on by default for a linux guest today
> but if we were to ever move to a model where we make it opt-in, it would
> be nice to only enable the host FMB when the guest actually wants it.