Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] PCI: number_initial clobbered on bus rescan in pci_scan_bridge_extend() --
commit 66abeabff58a043b28934ad23308d764456e9fa4 Author: Mario Limonciello <[email protected]> PCI: Preserve the initial bus number in pci_bus This commit saves the firmware-assigned secondary bus number before PCI bus renumbering and stores it in pci_bus->number_initial so drivers can match firmware records against the initial bus number. > diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c > index 27008e2ea5afc..9950a9dd1db1f 100644 > --- a/drivers/pci/probe.c > +++ b/drivers/pci/probe.c [ ... ] > @@ -1232,6 +1233,7 @@ static struct pci_bus *pci_alloc_child_bus(struct > pci_bus *parent, > > /* Set up the primary, secondary and subordinate bus numbers */ > child->number = child->busn_res.start = busnr; > + child->number_initial = busnr; > child->primary = parent->busn_res.start; > child->busn_res.end = 0xff; > > @@ -1470,6 +1472,7 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, > struct pci_dev *dev, > child = pci_add_new_bus(bus, dev, secondary); > if (!child) > goto out; > + child->number_initial = secondary; > child->primary = primary; > pci_bus_insert_busn_res(child, secondary, subordinate); > child->bridge_ctl = bctl; > @@ -1491,6 +1494,8 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, > struct pci_dev *dev, > * do in the second pass. > */ > if (!pass) { > + if (!dev->secondary_busnr) > + dev->secondary_busnr = secondary; > if (pcibios_assign_all_busses() || broken) > > /* > @@ -1529,6 +1534,10 @@ static int pci_scan_bridge_extend(struct pci_bus *bus, > struct pci_dev *dev, > pci_bus_insert_busn_res(child, next_busnr, > bus->busn_res.end); > } > + if (dev->secondary_busnr) { > + child->number_initial = dev->secondary_busnr; > + dev->secondary_busnr = 0; > + } [Severity: Medium] Can this overwrite child->number_initial with the reassigned runtime bus number during a subsequent PCI bus rescan when pcibios_assign_all_busses() is enabled? During the initial scan of a bridge taking the bus reassignment branch in pci_scan_bridge_extend(), pass 0 saves the firmware secondary bus number in dev->secondary_busnr. Pass 1 then transfers it to child->number_initial, resets dev->secondary_busnr to 0, and writes the newly assigned bus number (next_busnr) into the bridge's PCI_PRIMARY_BUS register: drivers/pci/probe.c:pci_scan_bridge_extend() { ... buses = (buses & PCI_SEC_LATENCY_TIMER_MASK) | FIELD_PREP(PCI_PRIMARY_BUS_MASK, child->primary) | FIELD_PREP(PCI_SECONDARY_BUS_MASK, child->busn_res.start) | FIELD_PREP(PCI_SUBORDINATE_BUS_MASK, child->busn_res.end); /* We need to blast all three values with a single write */ pci_write_config_dword(dev, PCI_PRIMARY_BUS, buses); ... } When a later bus rescan occurs (for example via pci_rescan_bus(), /sys/bus/pci/rescan, pci_rescan_bus_bridge_resize(), or hotplug), pass 0 reads PCI_PRIMARY_BUS again, where secondary is now the reassigned bus number. Because dev->secondary_busnr was reset to 0 in the first scan, !dev->secondary_busnr is true and dev->secondary_busnr is set to the reassigned bus number. In pass 1 of the rescan, pci_find_bus(pci_domain_nr(bus), next_busnr) finds the existing child bus, and the block after if (!child) overwrites child->number_initial with the reassigned bus number. A similar overwrite can occur if pci_find_bus() finds an already-existing bus during hotplug. Also, in the new-bus case, pci_add_new_bus() calls pci_alloc_child_bus(), which sets child->number_initial = busnr (next_busnr) before calling device_register(&child->dev), pcibios_add_bus(child), and child->ops->add_bus(child), so those callbacks see the reassigned bus number in child->number_initial before pci_scan_bridge_extend() updates it from dev->secondary_busnr. -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
