Re: [PATCH 1/4] PCI: Introduce an API to check if RC/platform can retain device context during suspend

[Manivannan Sadhasivam]

On Thu, Apr 16, 2026 at 02:18:55PM -0500, Bjorn Helgaas wrote:
> [+cc Rafael]
> 
> On Tue, Apr 14, 2026 at 09:29:39PM +0530, Manivannan Sadhasivam via B4 Relay wrote:
> > From: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > 
> > Currently, the PCI endpoint drivers like NVMe checks whether the device
> > context will be retained or not during system suspend, with the help of
> > pm_suspend_via_firmware() API.
> > 
> > But it is possible that the device context might be lost due to some
> > platform limitation as well. Having those checks in the endpoint drivers
> > will not scale and will cause a lot of code duplication.
> > 
> > So introduce an API that acts as a sole point of truth that the endpoint
> > drivers can rely on to check whether they can expect the device context
> > to be retained or not.
> > 
> > If the API returns 'false', then the client drivers need to prepare for
> > context loss by performing actions such as resetting the device, saving
> > the context, shutting it down etc... If it returns 'true', then the drivers
> > do not need to perform any special action and can leave the device in
> > active state.
> > 
> > Right now, this API only incorporates the pm_suspend_via_firmware() check.
> > But will be extended in the future commits.
> > 
> > Signed-off-by: Manivannan Sadhasivam <manivannan.sadhasivam@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/pci/pci.c   | 23 +++++++++++++++++++++++
> >  include/linux/pci.h |  7 +++++++
> >  2 files changed, 30 insertions(+)
> > 
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 8479c2e1f74f..211616467a77 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -33,6 +33,7 @@
> >  #include <asm/dma.h>
> >  #include <linux/aer.h>
> >  #include <linux/bitfield.h>
> > +#include <linux/suspend.h>
> >  #include "pci.h"
> >  
> >  DEFINE_MUTEX(pci_slot_mutex);
> > @@ -2900,6 +2901,28 @@ void pci_config_pm_runtime_put(struct pci_dev *pdev)
> >  		pm_runtime_put_sync(parent);
> >  }
> >  
> > +/**
> > + * pci_dev_suspend_retention_supported - Check if the platform can retain the device
> > + *					 context during system suspend
> > + * @pdev: PCI device to check
> > + *
> > + * Returns true if the platform can guarantee to retain the device context,
> > + * false otherwise.
> > + */
> > +bool pci_dev_suspend_retention_supported(struct pci_dev *pdev)
> 
> This doesn't seem like the right name.  This isn't a property of the
> *device*; that's all determined by the PCI spec (devices must retain
> all internal state in D0, D1, and D2, they retain it in D3hot if
> No_Soft_Reset, and they never do in D3cold).
> 
> So this seems like something to do with the *platform* behavior.  It
> sounds like this is basically a way to learn whether the device might
> be put in D3cold on system suspend.
> 

That's correct. But I wanted to keep it device specific, since apart from
pm_suspend_via_firmware() there could be other issues causing context to be
lost. Like the issue with RC, brought up in the successive patches. There could
be chances that only one hierarchy might be affected. So making it device
specific would give us the granularity.

- Mani

> > +{
> > +	/*
> > +	 * If the platform firmware (like ACPI) is involved at the end of system
> > +	 * suspend, device context may not be retained.
> > +	 */
> > +	if (pm_suspend_via_firmware())
> > +		return false;
> > +
> > +	/* Assume that the context is retained by default */
> > +	return true;
> > +}
> > +EXPORT_SYMBOL_GPL(pci_dev_suspend_retention_supported);
> > +
> >  static const struct dmi_system_id bridge_d3_blacklist[] = {
> >  #ifdef CONFIG_X86
> >  	{
> > diff --git a/include/linux/pci.h b/include/linux/pci.h
> > index 1c270f1d5123..d9bc7ad4eaa4 100644
> > --- a/include/linux/pci.h
> > +++ b/include/linux/pci.h
> > @@ -2080,6 +2080,8 @@ pci_release_mem_regions(struct pci_dev *pdev)
> >  			    pci_select_bars(pdev, IORESOURCE_MEM));
> >  }
> >  
> > +bool pci_dev_suspend_retention_supported(struct pci_dev *pdev);
> > +
> >  #else /* CONFIG_PCI is not enabled */
> >  
> >  static inline void pci_set_flags(int flags) { }
> > @@ -2239,6 +2241,11 @@ pci_alloc_irq_vectors(struct pci_dev *dev, unsigned int min_vecs,
> >  static inline void pci_free_irq_vectors(struct pci_dev *dev)
> >  {
> >  }
> > +
> > +static inline bool pci_dev_suspend_retention_supported(struct pci_dev *pdev)
> > +{
> > +	return true;
> > +}
> >  #endif /* CONFIG_PCI */
> >  
> >  /* Include architecture-dependent settings and functions */
> > 
> > -- 
> > 2.51.0
> > 
> > 

-- 
மணிவண்ணன் சதாசிவம்