Re: [PATCH v1 1/3] s390/pci: Preserve FMB state in device re-enablement

[Niklas Schnelle]

On Tue, 2026-05-05 at 17:48 +0200, Niklas Schnelle wrote:
> On Tue, 2026-05-05 at 17:41 +0200, Niklas Schnelle wrote:
> > On Fri, 2026-05-01 at 15:25 -0400, Omar Elghoul wrote:
> > > Introduce a function zpci_fmb_reenable_device() that checks for the state
> > > of the FMB and reuses the same buffer where appropriate. If FMB was not
> > > previously enabled, it enables it for the device. Call this function during
> > > a zPCI device re-enablement, which in turn implicitly ensures that the FMB
> > > is enabled for host devices during their KVM registration.
> > > 
> > > This function also clears out the software counters, so that a program
> > > resetting an FMB would see all its counters restart from zero as expected.
> > > The function to clear the software counters is also separated into a static
> > > function as it is now reused in both zpci_fmb_enable_device() and
> > > zpci_fmb_reenable_device().
> > > 
> > > Signed-off-by: Omar Elghoul <oelghoul@xxxxxxxxxxxxx>
> > > ---
> > >  arch/s390/include/asm/pci.h |  1 +
> > >  arch/s390/pci/pci.c         | 71 ++++++++++++++++++++++++++++++-------
> > >  2 files changed, 59 insertions(+), 13 deletions(-)
> > > 
> > 
> > Looks good to me. I also gave this a quick sniff test on an LPAR but
> > still have to test with the FMB handling for KVM that this enabled so
> > not enough for a Tested-by yet.
> > 
> > Still feel free to add my:
> > 
> > Reviewed-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
> > 
> > Thanks,
> > Niklas
> 
> Ah, missed one thing, the kernel-test-robot is of course right in that
> with this patch flags and ctrs are now unused in
> zpci_fmb_enable_device() so the declarations should be dropped. The
> same is also true in zpci_fmb_reenable_device() which the test robot
> did miss!
> 
> Thanks,
> Niklas

Missed another issue which thankfully Sashiko found. The use of
zpci_fmb_reenable_device() in zpci_reenable_device() leads to the new
call chain:

recover_store()
   _do_recover()
      zpci_reenable_device()
          zpci_fmb_reenable_device()

Where we don't take the zdev->fmb_lock. I think it makes sense to add a
lockdep_assert_held() to zpci_fmb_reenable_device().

Thanks,
Niklas