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

[Omar Elghoul]

On 9/23/26 6:20 PM, Matthew Rosato wrote:
> On 9/22/26 3:51 PM, Omar Elghoul wrote:
> > Introduce the function zpci_fmb_reenable_device() that checks the state
> > of function measurement and ensures it is enabled. Reset the counters to
> > zero, disable, and re-enable the FMB if it was already enabled. Call
> > this function from zpci_reenable_device().
> >
> > Don't free the FMB buffer during disabling and reuse it when re-enabling
> > measurement. Instead, free the buffer upon device teardown, allowing the
> > same buffer to be reused in the enable path and add the bit fmb_enabled
> > to struct zpci_dev. Audit the only consumer of zdev->fmb and update it
> > to reflect the change in semantics.
> >
> > Signed-off-by: Omar Elghoul <oelghoul@xxxxxxxxxxxxx>
>
> [...]
>
> > +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);
>
> [...]
>
> > +	if (zdev->fmb_enabled)
> > +		zpci_fmb_reenable_device(zdev);
> >   
> Besides Gerd's comments, I was looking at this patch in isolation and
> this combination made me wonder why you were adding what appears to be
> dead code (of course, patch 4 adds another caller that doesn't check
> zdev->fmb_enabled before calling)
>
> Maybe you could add a bit to the commit message besides 'Call
> > this function from zpci_reenable_device().' to indicate that this
> function is also being setup for future re-use where we might be going
> disabled->enabled rather than enabled->disabled->enabled.

Sure, how's this for a new commit message? Hopefully this is clearer and
doesn't read as upside down. I also added a blurb about firmware
implicitly disabling FMB when we disable the device.

"Don't free the FMB buffer when disabling measurement in
zpci_fmb_disable_device(). Instead, make the buffer persistent for the
lifetime of the device and reuse it across enable/disable cycles. Defer
freeing the buffer until teardown in zpci_release_device().

To support the persistent buffers, add the fmb_enabled field to struct
zpci_dev to decouple whether FMB is enabled from whether the buffer has
been allocated. Audit the only consumer of zdev->fmb as a liveness check
and update it to reflect this change.

Introduce the function zpci_fmb_reenable_device() to ensure that the FMB
is enabled. If it was already enabled, disable it, zero the counters,
and re-enable it. This allows the function to be used in both first-time
enabling and re-enabling measurement. Call it in zpci_reenable_device()
to preserve the FMB enablement if it had been implicitly disabled by
firmware in zpci_disable_device()."