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/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
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.

Thanks

> 
> Thanks,
> Matt
> 
>> +
>> +	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;
>> +
>> +	zdev->fmb_enabled = 0;
>> +	return zpci_fmb_enable_device(zdev);
>> +}
>> +EXPORT_SYMBOL_GPL(zpci_fmb_reenable_device);
>