Re: [PATCHv2 2/2] pci: warn if a running device is unaware of reset

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

On Fri, 2024-10-25 at 15:27 -0700, Keith Busch wrote:
> From: Keith Busch <kbusch@kernel.org>
> 
> If a reset is issued to a running device with a driver that didn't
> register the notification callbacks, the driver may be unaware of this
> event and have an inconsistent view of the device's state. Log a warning
> of this event because there's nothing else indicating the event occured,
> which could be confusing when debugging such situations.
> 
> Signed-off-by: Keith Busch <kbusch@kernel.org>
> ---
>  drivers/pci/pci.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 338dfcd983f1e..bbf12d4998269 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5158,6 +5158,8 @@ static void pci_dev_save_and_disable(struct pci_dev *dev)
>  	 */
>  	if (err_handler && err_handler->reset_prepare)
>  		err_handler->reset_prepare(dev);
> +	else if (dev->driver)
> +		pci_warn(dev, "resetting");
>  
>  	/*
>  	 * Wake-up device prior to save.  PM registers default to D0 after
> @@ -5191,6 +5193,8 @@ static void pci_dev_restore(struct pci_dev *dev)
>  	 */
>  	if (err_handler && err_handler->reset_done)
>  		err_handler->reset_done(dev);
> +	else if (dev->driver)
> +		pci_warn(dev, "reset done");
>  }
>  
>  /* dev->reset_methods[] is a 0-terminated list of indices into this array */

For what it's worth on s390 I think the previous proposal of having the
attribute on the pci_bus would have been better in principle. On s390
the PCI bus probing is done by firmware and Linux doesn't see a pci_dev
for bridges but we do create struct pci_bus for example for a PF and
its child VFs.

Then again we can't really do a reset on this level, other than
manually going through the PCI functions and resetting them one by one.
In fact we may see PCI functions on their own bus while another Linux
instance (LPAR) sees other functions from that bus. So yeah, I guess
it's fair not to have this attribute but still wanted to offer these
thoughts.

Thanks,
Niklas