On Sat, Sep 12, 2026 at 05:31:58PM +0000, David Matlack wrote:
> On 2026-09-11 06:30 PM, David Matlack wrote:
> > On 2026-09-10 06:51 PM, Bjorn Helgaas wrote:
> > > On Tue, Jul 28, 2026 at 10:09:59PM +0000, David Matlack wrote:
>
> > > > +bool pci_liveupdate_preserve_bus_numbers(struct pci_bus *bus, struct
> > > > pci_dev *dev)
> > > > +{
> > > > + struct pci_dev *parent = bus->self;
> > > > +
> > > > + if (dev->liveupdate.preserve_bus_numbers)
> > > > + return true;
> > > > +
> > > > + if (parent && parent->liveupdate.preserve_bus_numbers) {
> > > > + /*
> > > > + * Preserve bus numbers if the parent bridge is
> > > > required to
> > > > + * preserve bus numbers. Otherwise the PCI core could
> > > > expand
> > > > + * this bridge's reservation beyond its parent (which
> > > > cannot
> > > > + * expand).
> > > > + */
> > > > + dev->liveupdate.preserve_bus_numbers = true;
> > > > + } else {
> > > > + /*
> > > > + * Otherwise preserve bus numbers if there are any
> > > > incoming
> > > > + * preserved devices. This ensures that the PCI core
> > > > does not
> > > > + * allocate a bus number to a non-preserved device that
> > > > + * conflicts with the bus number already assigned to a
> > > > preserved
> > > > + * device.
> > > > + *
> > > > + * This is slightly more restrictive than it needs to
> > > > be. For
> > > > + * example, each host bridges have their own range of
> > > > bus
> > > > + * numbers that won't conflict with other host bridges.
> > > > But the
> > > > + * previous kernel should have assigned a sane bus
> > > > topology and
> > > > + * it is simpler to just adopt that entire topology.
> > > > + */
> > > > + dev->liveupdate.preserve_bus_numbers =
> > > > + pci_has_incoming_preserved_devices();
> > > > + }
> > > > +
> > > > + return dev->liveupdate.preserve_bus_numbers;
> > >
> > > I'm not sure why you don't just return
> > > pci_has_incoming_preserved_devices() in all cases, which is what the
> > > commit log suggests this patch does. What's gained by all the logic
> > > here? It's not like devices will be hot-added during the kexec.
> >
> > To protect against pci_has_incoming_preserved_devices() flipping from
> > true to false while the PCI core is in the middle of a scan. It is not
> > likely to ever happen given most host bridge scanning should happen
> > during early boot, but theoretically possible with the way the PCI core
> > code is structured. I did not see way to structurally ensure these 2
> > things cannot race. A lot of the host bridge scanning happens without
> > taking the rescan lock, for example.
>
> After working on this more, I do see a way to simplify the logic in
> pci_liveupdate_preserve_bus_numbers().
>
> pci_liveupdate_preserve_bus_numbers() is used in 2 places during
> scanning. First to decide if the PCI core should preserve bus numbers or
> is free to allocate new ones, and second to decide if the PCI core is
> allowed to assign bus numbers to bridges that are missing bus numbers.
>
> The latter case should never happen during initial scanning unless a
> bridge was somehow reset during the kexec, but could legitimately happen
> if a bridge is later hot-plugged and I did not want Live Update to
> unnecessarily break that scenario. But then that creates this problem
> where pci_has_incoming_preserved_devices() can suddenly flip from true
> to false at any time and I needed all the complex logic to keep it
> consistent for a given scan.
>
> Instead we can split the handling of these cases:
>
> 1. When the PCI core needs to decide if it should preserve bus numbers
> due to Live Update, pci_liveupdate_preserve_bus_numbers() can return
> true forever if any device was preserved by the previous kernel,
> which simplifies the logic.
>
> 2. Then to handle the case of a bridge is enumerated that does not have
> bus numbers assigned, we can handle that separately. If we reorder this
> with the next commit so the PCI core knows exactly which bridges have
> preserved downstream endpoints, then it is possible to determine if it
> is safe for the PCI core to allow bus numbers to be assigned to an
> unconfigured bridge.
>
> After re-ordering, we can end up with something like this:
>
> bool pci_liveupdate_preserve_bus_numbers(void)
> {
> return pci_liveupdate.had_incoming;
> }
>
> bool pci_liveupdate_refuse_bus_numbers(struct pci_bus *bus, struct pci_dev
> *dev)
> {
> struct pci_dev *bridge;
>
> for_each_pci_bridge(bridge, bus) {
> if (!bridge->liveupdate.was_incoming || bridge->subordinate)
> continue;
>
> pci_err(dev, "Not assigning bus numbers, preserved bridge %s
> lost its bus number configuration\n",
> pci_name(bridge));
> return true;
> }
>
> return false;
> }
>
> The net effect on pci_scan_bridge_extend() is:
>
> bool preserve_bus_numbers = !pcibios_assign_all_busses() ||
> pci_liveupdate_preserve_bus_numbers();
> ...
> if (pci_liveupdate_refuse_bus_numbers(bus, dev))
> goto out;
>
> We could further scope pci_liveupdate_preserve_bus_numbers() to only
> return true for host bridges with preserved endpoints downstream, but
> that doesn't seem worth the extra complexity. It also seems nice to keep
> the pci_liveupdate_preserve_bus_numbers() policy global to match how the
> existing pcibios_assign_all_busses() policy is global.
>
> Does that look reasonable?
Yep, sounds good to me.
