Re: [PATCH v8 05/12] PCI: liveupdate: Preserve bus numbers during Live Update

[Bjorn Helgaas <helgaas@kernel.org>]

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.