[Qemu-devel] [PATCH for-4.0] spapr_pci: Fix broken naming of PCI bus

Greg Kurz posted 1 patch 5 years ago
Test docker-mingw@fedora passed
Test docker-clang@ubuntu passed
Test checkpatch passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/155500034416.646888.1307366522340665522.stgit@bahia.lab.toulouse-stg.fr.ibm.com
Maintainers: David Gibson <david@gibson.dropbear.id.au>
hw/ppc/spapr_pci.c |    4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
[Qemu-devel] [PATCH for-4.0] spapr_pci: Fix broken naming of PCI bus
Posted by Greg Kurz 5 years ago
Recent commit 5cf0d326a0fe fixed a regression which was preventing the
guest to access the extended config space of a PCIe device. This was
done by introducing a new PCI bus subtype for PAPR. The original fix
was causing PCI busses to be named "spapr-pci-host-bridge-root-bus.N"
instead of "pci.N", which was making upper layers unhappy of course.
This got worked around by hardcoding the PCI bus name to "pci.0", but
this only works for the default PHB. And we're now hitting:

# qemu-system-ppc64 \
             -device spapr-pci-host-bridge,index=1 \
             -device e1000e,bus=pci.0 \
             -device e1000e,bus=pci.1
qemu-system-ppc64: -device e1000e,bus=pci.1: Bus 'pci.1' not found

David already posted some patches [1] to control PCI extended config
space accesses with a new flag in the base PCI bus class instead of
subtyping. These patches are a bit more intrusive though, and
are targetted for 4.1.

When no name is passed to pci_register_bus(), the core device code
generates a lowercase name based on the QOM typename. The typename
for the base PCI bus class is "PCI", hence the "pci.0", "pci.1"
bus names. Rename the type of the PAPR PCI bus to "pci", so that
the QOM code can generate proper names. This is a hack but it is
enough to fix the regression. And all this will be reworked properly
in 4.1.

[1] https://patchwork.ozlabs.org/project/qemu-devel/list/?series=100486

Fixes: 5cf0d326a0fe
Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr_pci.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
index f0b6b23afc9b..f62e6833b8c9 100644
--- a/hw/ppc/spapr_pci.c
+++ b/hw/ppc/spapr_pci.c
@@ -1652,7 +1652,7 @@ static void spapr_phb_root_bus_class_init(ObjectClass *klass, void *data)
     pbc->allows_extended_config_space = spapr_phb_allows_extended_config_space;
 }
 
-#define TYPE_SPAPR_PHB_ROOT_BUS "spapr-pci-host-bridge-root-bus"
+#define TYPE_SPAPR_PHB_ROOT_BUS "pci"
 
 static const TypeInfo spapr_phb_root_bus_info = {
     .name = TYPE_SPAPR_PHB_ROOT_BUS,
@@ -1761,7 +1761,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
     memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
                                 &sphb->iowindow);
 
-    bus = pci_register_root_bus(dev, "pci.0",
+    bus = pci_register_root_bus(dev, NULL,
                                 pci_spapr_set_irq, pci_spapr_map_irq, sphb,
                                 &sphb->memspace, &sphb->iospace,
                                 PCI_DEVFN(0, 0), PCI_NUM_PINS,


Re: [Qemu-devel] [PATCH for-4.0] spapr_pci: Fix broken naming of PCI bus
Posted by David Gibson 5 years ago
On Thu, Apr 11, 2019 at 06:32:24PM +0200, Greg Kurz wrote:
> Recent commit 5cf0d326a0fe fixed a regression which was preventing the
> guest to access the extended config space of a PCIe device. This was
> done by introducing a new PCI bus subtype for PAPR. The original fix
> was causing PCI busses to be named "spapr-pci-host-bridge-root-bus.N"
> instead of "pci.N", which was making upper layers unhappy of course.
> This got worked around by hardcoding the PCI bus name to "pci.0", but
> this only works for the default PHB. And we're now hitting:
> 
> # qemu-system-ppc64 \
>              -device spapr-pci-host-bridge,index=1 \
>              -device e1000e,bus=pci.0 \
>              -device e1000e,bus=pci.1
> qemu-system-ppc64: -device e1000e,bus=pci.1: Bus 'pci.1' not found
> 
> David already posted some patches [1] to control PCI extended config
> space accesses with a new flag in the base PCI bus class instead of
> subtyping. These patches are a bit more intrusive though, and
> are targetted for 4.1.
> 
> When no name is passed to pci_register_bus(), the core device code
> generates a lowercase name based on the QOM typename. The typename
> for the base PCI bus class is "PCI", hence the "pci.0", "pci.1"
> bus names. Rename the type of the PAPR PCI bus to "pci", so that
> the QOM code can generate proper names. This is a hack but it is
> enough to fix the regression. And all this will be reworked properly
> in 4.1.
> 
> [1] https://patchwork.ozlabs.org/project/qemu-devel/list/?series=100486
> 
> Fixes: 5cf0d326a0fe
> Signed-off-by: Greg Kurz <groug@kaod.org>

Oof.  That's a hack-and-a-half.  But, I don't see a better way of
fixing this quickly, and as you say, I already have clean ups for this
ready to go for 4.1.  Applied to ppc-for-4.0, and I'll try to rush out
a pull request today.

> ---
>  hw/ppc/spapr_pci.c |    4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index f0b6b23afc9b..f62e6833b8c9 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1652,7 +1652,7 @@ static void spapr_phb_root_bus_class_init(ObjectClass *klass, void *data)
>      pbc->allows_extended_config_space = spapr_phb_allows_extended_config_space;
>  }
>  
> -#define TYPE_SPAPR_PHB_ROOT_BUS "spapr-pci-host-bridge-root-bus"
> +#define TYPE_SPAPR_PHB_ROOT_BUS "pci"
>  
>  static const TypeInfo spapr_phb_root_bus_info = {
>      .name = TYPE_SPAPR_PHB_ROOT_BUS,
> @@ -1761,7 +1761,7 @@ static void spapr_phb_realize(DeviceState *dev, Error **errp)
>      memory_region_add_subregion(get_system_memory(), sphb->io_win_addr,
>                                  &sphb->iowindow);
>  
> -    bus = pci_register_root_bus(dev, "pci.0",
> +    bus = pci_register_root_bus(dev, NULL,
>                                  pci_spapr_set_irq, pci_spapr_map_irq, sphb,
>                                  &sphb->memspace, &sphb->iospace,
>                                  PCI_DEVFN(0, 0), PCI_NUM_PINS,
> 

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson