Re: [PATCH v7 09/12] PCI: liveupdate: Inherit ARI Forwarding Enable on preserved bridges

On Mon, Jul 27, 2026 at 4:07 PM Bjorn Helgaas <[email protected]> wrote:
>
> On Fri, Jul 10, 2026 at 09:26:12PM +0000, David Matlack wrote:
> > Inherit the ARI Forwarding Enable on preserved bridges and update
> > pci_dev->ari_enabled accordingly during a Live Update. This ensures that
> > the preserved devices on the bridge's secondary bus can be identified
> > with the same expanded 8-bit function number after a Live Update.
> >
> > Signed-off-by: David Matlack <[email protected]>
> > ---
> >  drivers/pci/liveupdate.c | 18 ++++++++++++++++++
> >  drivers/pci/liveupdate.h |  6 ++++++
> >  drivers/pci/pci.c        |  8 +++++++-
> >  3 files changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/pci/liveupdate.c b/drivers/pci/liveupdate.c
> > index a95bfe5eff77..74a11e520f0d 100644
> > --- a/drivers/pci/liveupdate.c
> > +++ b/drivers/pci/liveupdate.c
> > @@ -128,6 +128,10 @@
> >   *    way after Live Update and ensures that IOMMU groups do not change. Note
> >   *    that a device will use its inherited ACS flags for the lifetime of its
> >   *    struct pci_dev (i.e. even after pci_liveupdate_finish()).
> > + *
> > + *  * The PCI core inherits ARI Forwarding Enable on all bridges with downstream
> > + *    preserved devices to ensure that all preserved devices on the bridge's
> > + *    secondary bus are addressable after the Live Update.
> >   */
> >
> >  #define pr_fmt(fmt) "PCI: " KBUILD_BASENAME ": " fmt
> > @@ -816,6 +820,20 @@ int pci_liveupdate_enable_acs(struct pci_dev *dev)
> >       return 0;
> >  }
> >
> > +int pci_liveupdate_configure_ari(struct pci_dev *dev)
> > +{
> > +     u16 val;
> > +
> > +     guard(rwsem_read)(&pci_liveupdate.rwsem);
> > +
> > +     if (!dev->liveupdate.incoming)
> > +             return -EINVAL;
> > +
> > +     pcie_capability_read_word(dev, PCI_EXP_DEVCTL2, &val);
> > +     dev->ari_enabled = !!(val & PCI_EXP_DEVCTL2_ARI);
> > +     return 0;
> > +}
>
> I'm guessing we're going to see a lot of this pattern, so it will
> eventually become familiar, but it's not familiar yet ;)  This doesn't
> "configure" anything (I do understand that avoiding configuration in
> the new kernel is really the main point of liveupdate).
>
> Maybe a comment is the solution.  Or maybe a rename to
> "pci_liveupdate_ari_preserved" or something?  That would have to
> reverse the sense of return values, but I think something like this in
> the callers would read better:
>
>   if (pci_liveupdate_ari_preserved(dev))
>     return;
>
> or maybe:
>
>   if (pci_liveupdate_preserve_ari(dev))
>     return;

I do think a better naming and return value convention is needed.

Perhaps "adopt" or "inherit" as the verb? i.e. "adopt/inherit the
state of the device established by the previous kernel"

if (pci_liveupdate_adopt_ari(dev))
  return;

or

if (pci_liveupdate_inherit_ari(dev))
  return?

>
> >  /**
> >   * pci_liveupdate_is_incoming() - Check if a device is incoming-preserved
> >   * @dev: The PCI device to check
> > diff --git a/drivers/pci/liveupdate.h b/drivers/pci/liveupdate.h
> > index 4e8a01bcb4bb..6f21ec50927b 100644
> > --- a/drivers/pci/liveupdate.h
> > +++ b/drivers/pci/liveupdate.h
> > @@ -18,6 +18,7 @@ bool pci_liveupdate_scan_bridge_begin(struct pci_bus *bus, struct pci_dev *dev,
> >  void pci_liveupdate_scan_bridge_end(struct pci_dev *dev, int pass);
> >  void pci_liveupdate_init_acs(struct pci_dev *dev);
> >  int pci_liveupdate_enable_acs(struct pci_dev *dev);
> > +int pci_liveupdate_configure_ari(struct pci_dev *dev);
> >  #else
> >  static inline void pci_liveupdate_setup_device(struct pci_dev *dev)
> >  {
> > @@ -46,6 +47,11 @@ static inline int pci_liveupdate_enable_acs(struct pci_dev *dev)
> >  {
> >       return -EINVAL;
> >  }
> > +
> > +static inline int pci_liveupdate_configure_ari(struct pci_dev *dev)
> > +{
> > +     return -EINVAL;
> > +}
> >  #endif
> >
> >  #endif /* DRIVERS_PCI_LIVEUPDATE_H */
> > diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> > index 739ecaab2e76..e0c133b66a35 100644
> > --- a/drivers/pci/pci.c
> > +++ b/drivers/pci/pci.c
> > @@ -3528,7 +3528,7 @@ void pci_configure_ari(struct pci_dev *dev)
> >       u32 cap;
> >       struct pci_dev *bridge;
> >
> > -     if (pcie_ari_disabled || !pci_is_pcie(dev) || dev->devfn)
> > +     if (!pci_is_pcie(dev) || dev->devfn)
> >               return;
> >
> >       bridge = dev->bus->self;
> > @@ -3539,6 +3539,12 @@ void pci_configure_ari(struct pci_dev *dev)
> >       if (!(cap & PCI_EXP_DEVCAP2_ARI))
> >               return;
> >
> > +     if (!pci_liveupdate_configure_ari(bridge))
> > +             return;
> > +
> > +     if (pcie_ari_disabled)
> > +             return;
> > +
> >       if (pci_find_ext_capability(dev, PCI_EXT_CAP_ID_ARI)) {
> >               pcie_capability_set_word(bridge, PCI_EXP_DEVCTL2,
> >                                        PCI_EXP_DEVCTL2_ARI);
> > --
> > 2.55.0.795.g602f6c329a-goog
> >