On Fri, 28 Oct 2016 18:56:40 +1100
Alexey Kardashevskiy <a...@ozlabs.ru> wrote:
> At the moment sPAPR PHB creates a root buf of TYPE_PCI_BUS type.
> This means that vfio-pci devices attached to it (and this is
> a default behaviour) hide PCIe extended capabilities as
> the bus does not pass a pci_bus_is_express(pdev->bus) check.
>
> This changes adds a default PCI bus type property to sPAPR PHB
> and uses TYPE_PCIE_BUS if none passed; older machines get TYPE_PCI_BUS
> for backward compatibility as a bus type is used in the bus name
> so the root bus name becomes "pcie.0" instead of "pci.0".
>
> Signed-off-by: Alexey Kardashevskiy <a...@ozlabs.ru>
> ---
>
> What can possibly go wrong with such change of a name?
> From devices prospective, I cannot see any.
>
> libvirt might get upset as "pci.0" will not be available,
> will it make sense to create pcie.0 as a root bus and always
> add a PCIe->PCI bridge and name its bus "pci.0"?
>
> Or create root bus from TYPE_PCIE_BUS and force name to "pci.0"?
> pci_register_bus() can do this.
>
>
> ---
> hw/ppc/spapr.c | 5 +++++
> hw/ppc/spapr_pci.c | 5 ++++-
> include/hw/pci-host/spapr.h | 1 +
> 3 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 0b3820b..a268511 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -2541,6 +2541,11 @@ DEFINE_SPAPR_MACHINE(2_8, "2.8", true);
> .driver = TYPE_SPAPR_PCI_HOST_BRIDGE, \
> .property = "mem64_win_size", \
> .value = "0", \
> + }, \
> + { \
> + .driver = TYPE_SPAPR_PCI_HOST_BRIDGE, \
> + .property = "root_bus_type", \
> + .value = TYPE_PCI_BUS, \
> },
>
> static void phb_placement_2_7(sPAPRMachineState *spapr, uint32_t index,
> diff --git a/hw/ppc/spapr_pci.c b/hw/ppc/spapr_pci.c
> index 7cde30e..2fa1f22 100644
> --- a/hw/ppc/spapr_pci.c
> +++ b/hw/ppc/spapr_pci.c
> @@ -1434,7 +1434,9 @@ static void spapr_phb_realize(DeviceState *dev, Error
> **errp)
> bus = pci_register_bus(dev, NULL,
> pci_spapr_set_irq, pci_spapr_map_irq, sphb,
> &sphb->memspace, &sphb->iospace,
> - PCI_DEVFN(0, 0), PCI_NUM_PINS, TYPE_PCI_BUS);
> + PCI_DEVFN(0, 0), PCI_NUM_PINS,
> + sphb->root_bus_type ? sphb->root_bus_type :
> + TYPE_PCIE_BUS);
Shouldn't we ensure that sphb->root_bus_type is either TYPE_PCIE_BUS or
TYPE_PCI_BUS ?
Otherwise, if we pass something like:
-global spapr-pci-host-bridge.root_bus_type=pcie
we get the following not really explicit error:
ERROR:/home/greg/Work/qemu/qemu-spapr/qom/object.c:474:object_new_with_type:
assertion failed: (type != NULL)
> phb->bus = bus;
> qbus_set_hotplug_handler(BUS(phb->bus), DEVICE(sphb), NULL);
>
> @@ -1590,6 +1592,7 @@ static Property spapr_phb_properties[] = {
> DEFINE_PROP_UINT64("pgsz", sPAPRPHBState, page_size_mask,
> (1ULL << 12) | (1ULL << 16)),
> DEFINE_PROP_UINT32("numa_node", sPAPRPHBState, numa_node, -1),
> + DEFINE_PROP_STRING("root_bus_type", sPAPRPHBState, root_bus_type),
> DEFINE_PROP_END_OF_LIST(),
> };
>
> diff --git a/include/hw/pci-host/spapr.h b/include/hw/pci-host/spapr.h
> index b92c1b5..a4ee873 100644
> --- a/include/hw/pci-host/spapr.h
> +++ b/include/hw/pci-host/spapr.h
> @@ -79,6 +79,7 @@ struct sPAPRPHBState {
> uint64_t dma64_win_addr;
>
> uint32_t numa_node;
> + char *root_bus_type;
> };
>
> #define SPAPR_PCI_MEM_WIN_BUS_OFFSET 0x80000000ULL
