Re: [PATCH v6 06/12] PCI: liveupdate: Auto-preserve upstream bridges across Live Update

[David Matlack <dmatlack@google.com>]

On 2026-06-06 10:15 PM, Pranjal Shrivastava wrote:
> On Fri, May 22, 2026 at 08:24:04PM +0000, David Matlack wrote:
> > When a PCI device is preserved across a Live Update, all of its upstream
> > bridges up to the root port must also be preserved. This enables the PCI
> > core and any drivers bound to the bridges to manage bridges correctly
> > across a Live Update.
> > 
> > Notably, this will be used in subsequent commits to ensure that
> > preserved devices can continue performing memory transactions without a
> > disruption or change in routing.
> > 
> > To preserve bridges, the PCI core tracks the number of downstream
> > devices preserved under each bridge using a reference count in struct
> > pci_dev_ser. This allows a bridge to remain preserved until all its
> > downstream preserved devices are unpreserved or finish their
> > participation in the Live Update.
> > 
> > Signed-off-by: David Matlack <dmatlack@google.com>
> > ---
> >  drivers/pci/liveupdate.c    | 136 +++++++++++++++++++++++++++++++-----
> >  include/linux/kho/abi/pci.h |   5 +-
> >  2 files changed, 122 insertions(+), 19 deletions(-)
> > 
> 
> [...]
> 
> > +
> > +#define for_each_pci_dev_in_path(_d, _start, _end) \
> > +	for ((_d) = (_start); (_d) != (_end); (_d) = (_d)->bus->self)
> > +
> > +static void __pci_liveupdate_unpreserve_path(struct pci_ser *ser,
> > +					     struct pci_dev *start,
> > +					     struct pci_dev *end)
> > +{
> > +	struct pci_dev *dev;
> > +
> > +	for_each_pci_dev_in_path(dev, start, end) {
> > +		if (pci_liveupdate_unpreserve_device(ser, dev))
> 
> I might be reading this wrong but are we leaking some upstream devs if 
> an intermediate node fails?
> 
> 			  EP0
> 			/
> Assume we have: RC -> B1 -> B2 
> 				\
> 				 EP1
> 
> and EP0 & EP1 were preserved successfully.
> 
> And then we try unpreserving EP1, we follow:
> 
> unpreserve EP1 -> unpreserve B2 failed due to a corruption.
> 
> This aborts the loop, skipping B1 and RC completely?
> Their refcounts remain elevated, effectively leaking them as preserved 
> state permanently? (i.e. if we unpreserve EP0 after this, B1 & RC will
> still get preserved).

Yes, but that would only happen if there is some sort of kernel bug or
silent data corruption. I guess we could proceed with trying to
unpreserve the bridges upstream. But I opted to log a big warning and
bail immediately.

pci_liveupdate_finish_path() has the same behavior BTW.

> 
> > +			return;
> > +	}
> > +}
> > +
> > +static void pci_liveupdate_unpreserve_path(struct pci_ser *ser,
> > +					   struct pci_dev *start)
> > +{
> > +	__pci_liveupdate_unpreserve_path(ser, start, /*end=*/NULL);
> > +}
> > +
> > +static int pci_liveupdate_preserve_path(struct pci_ser *ser,
> > +					struct pci_dev *start)
> > +{
> > +	struct pci_dev *dev;
> > +	int ret;
> > +
> > +	for_each_pci_dev_in_path(dev, start, NULL) {
> > +		ret = pci_liveupdate_preserve_device(ser, dev);
> > +		if (ret) {
> > +			__pci_liveupdate_unpreserve_path(ser, start, dev);
> > +			return ret;
> > +		}
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  /**
> >   * pci_liveupdate_preserve() - Preserve a PCI device across Live Update
> >   * @dev: The PCI device to preserve.
> > @@ -321,6 +403,9 @@ static int pci_liveupdate_preserve_device(struct pci_ser *ser, struct pci_dev *d
> >   * pci_liveupdate_preserve() from their struct liveupdate_file_handler
> >   * preserve() callback to ensure the outgoing struct pci_ser is already set up.
> >   *
> > + * pci_liveupdate_preserve() automatically preserves all bridges upstream of
> > + * @dev.
> > + *
> >   * Returns: 0 on success, <0 on failure.
> >   */
> >  int pci_liveupdate_preserve(struct pci_dev *dev)
> > @@ -336,7 +421,7 @@ int pci_liveupdate_preserve(struct pci_dev *dev)
> >  	if (IS_ERR(ser))
> >  		return PTR_ERR(ser);
> >  
> > -	return pci_liveupdate_preserve_device(ser, dev);
> > +	return pci_liveupdate_preserve_path(ser, dev);
> 
> Minor nit: I might be too nitpicky here (and it's NOT a strong opinion)
> but naming it pci_liveupdate_preserve_path_for_dev() reads better to me.

Noted :). I'll keep the current name for now since that is pretty long,
but if anyone else votes for it I'm happy to be overridden.

> >  }
> >  EXPORT_SYMBOL_GPL(pci_liveupdate_preserve);
> >  
> 
> [...]
> 
> Thanks,
> Praan