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

[Niklas Schnelle <schnelle@linux.ibm.com>]

On Mon, 2026-10-05 at 11:45 -0400, Omar Elghoul wrote:
> 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 bool 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().

I think this causes a sequencing error in zpci_hot_reset_device().
First the device gets disabled via zpci_disable_device(). This
implicitly disables the FMB but keeps zdev->fmb_enabled set. Then we
call zpci_fmb_reenable_device() in zpci_reenable_device(). Since zdev-
>fmb_enabled is set we don't first enable the FMB and instead go
directly to disabling it but that is wrong since the FMB is already
disabled as a side effect of the CLP Set PCI Function (Disable) in
zpci_disable_device().

Also, and I think Gerd mentioned this before, there is a disconnect in
semantics between zpci_fmb_reenable_device() and zpci_reenable_device()
that is quite confusing. While zpci_reenable_device() re-enables the
device with existing interrupts and I/O address translations, after it
was disabled, zpci_fmb_reenable_device() on the other hand does a
disable and then enable cycle.

I think the idea here is that zdev->fmb_enabled tries to track whether
the FMB is supposed to be enabled rather than if it is enabled.  This
makes some sense since the FMB can get disabled by the device entering
the error state or a zpci_disable_device() and we want to know if we
need to re-enable it at the re-enable of the device.

Importantly, unlike the disablement of a device we always initiate the
enablement. But then we can't try to disable the FMB without knowing if
it was already disabled. I think a possible solution for this would be
to have zpci_fmb_reenable_device() mean that we know that the FMB is
disabled but should be enabled, which we know when we re-enable the
device and zdev->fmb_enabled is set. Of course then it doesn't do a
disable but only an enable despite zdev->fmb_enabled already being set,
Then zpci_fmb_enable_device() on the other hand sets the flag initially
and then uses zpci_fmb_reenable_device() or a shared helper. Of course
we would then have to properly document zdev->fmb_enabled as being a
the target rather than current state.

Thanks,
Niklas