On 23.01.19 09:24, David Hildenbrand wrote:
> While looking at the s390x implementation, looks like spapr has a
> similar BUG when building the topology.
>
> The primary bus number corresponds always to the bus number of the
> bus the bridge is attached to.
>
> Right now, if we have two bridges attached to the same bus (e.g. root
> bus) this is however not the case. The first bridge will have primary
> bus 0, the second bridge primary bus 1, which is wrong. Fix the assignment.
>
> While at it, drop setting the PCI_SUBORDINATE_BUS temporarily to 0xff.
> Setting it temporarily to that value (as discussed e.g. in [1]), is
> only relevant for a running system that probes the buses. The value is
> effectively unused for us just doing a DFS.
>
> [1] http://www.science.unitn.it/~fiorella/guidelinux/tlk/node76.html
>
> Note: Is hotplug of bridges supported? I can't find where the topology
> is fixed up when hotplugging a PCI bridge. (e.g. bus numbers assigned
> and PCI_SUBORDINATE_BUS of path to the root updated). But maybe we are
> excluding bridges or this is not necessary for some reason.
This note can be dropped. I learned that this is under guest control.
(which has to reserve bus numbers and rebuild the topology)
>
> Signed-off-by: David Hildenbrand <da...@redhat.com>
> ---
> hw/ppc/spapr_pci.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index b74f2632ec..5cdc98513d 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -2030,8 +2030,6 @@ static void spapr_phb_pci_enumerate_bridge(PCIBus *bus,
> PCIDevice *pdev,
> void *opaque)
> {
> unsigned int *bus_no = opaque;
> - unsigned int primary = *bus_no;
> - unsigned int subordinate = 0xff;
> PCIBus *sec_bus = NULL;
>
> if ((pci_default_read_config(pdev, PCI_HEADER_TYPE, 1) !=
> @@ -2040,7 +2038,7 @@ static void spapr_phb_pci_enumerate_bridge(PCIBus *bus,
> PCIDevice *pdev,
> }
>
> (*bus_no)++;
> - pci_default_write_config(pdev, PCI_PRIMARY_BUS, primary, 1);
> + pci_default_write_config(pdev, PCI_PRIMARY_BUS, pci_dev_bus_num(pdev),
> 1);
> pci_default_write_config(pdev, PCI_SECONDARY_BUS, *bus_no, 1);
> pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
>
> @@ -2049,7 +2047,6 @@ static void spapr_phb_pci_enumerate_bridge(PCIBus *bus,
> PCIDevice *pdev,
> return;
> }
>
> - pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, subordinate, 1);
> pci_for_each_device(sec_bus, pci_bus_num(sec_bus),
> spapr_phb_pci_enumerate_bridge, bus_no);
> pci_default_write_config(pdev, PCI_SUBORDINATE_BUS, *bus_no, 1);
>
--
Thanks,
David / dhildenb
