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

[Omar Elghoul]

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);