From 2f57db8a2781787a0a31d3a9ce4a286fc45a42b3 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:37 +1000 Subject: pcie: Simplify pci_adjust_config_limit() Since c2077e2c "pci: Adjust PCI config limit based on bus topology", pci_adjust_config_limit() has been used in the config space read and write paths to only permit access to extended config space on buses which permit it. Specifically it prevents access on devices below a vanilla-PCI bus via some combination of bridges, even if both the host bridge and the device itself are PCI-E. It accomplishes this with a somewhat complex call up the chain of bridges to see if any of them prohibit extended config space access. This is overly complex, since we can always know if the bus will support such access at the point it is constructed. This patch simplifies the test by using a flag in the PCIBus instance indicating whether extended configuration space is accessible. It is false for vanilla PCI buses. For PCI-E buses, it is true for root buses and equal to the parent bus's's capability otherwise. For the special case of sPAPR's paravirtualized PCI root bus, which acts mostly like vanilla PCI, but does allow extended config space access, we override the default value of the flag from the host bridge code. This should cause no behavioural change. Signed-off-by: David Gibson Reviewed-by: Greg Kurz Message-Id: <20190513061939.3464-4-david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin --- hw/pci/pci.c | 41 +++++++++++++++++++++++------------------ 1 file changed, 23 insertions(+), 18 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index b386777045..7e5f8d001b 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -120,6 +120,27 @@ static void pci_bus_realize(BusState *qbus, Error **errp) vmstate_register(NULL, -1, &vmstate_pcibus, bus); } +static void pcie_bus_realize(BusState *qbus, Error **errp) +{ + PCIBus *bus = PCI_BUS(qbus); + + pci_bus_realize(qbus, errp); + + /* + * A PCI-E bus can support extended config space if it's the root + * bus, or if the bus/bridge above it does as well + */ + if (pci_bus_is_root(bus)) { + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; + } else { + PCIBus *parent_bus = pci_get_bus(bus->parent_dev); + + if (pci_bus_allows_extended_config_space(parent_bus)) { + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; + } + } +} + static void pci_bus_unrealize(BusState *qbus, Error **errp) { PCIBus *bus = PCI_BUS(qbus); @@ -142,11 +163,6 @@ static uint16_t pcibus_numa_node(PCIBus *bus) return NUMA_NODE_UNASSIGNED; } -static bool pcibus_allows_extended_config_space(PCIBus *bus) -{ - return false; -} - static void pci_bus_class_init(ObjectClass *klass, void *data) { BusClass *k = BUS_CLASS(klass); @@ -161,7 +177,6 @@ static void pci_bus_class_init(ObjectClass *klass, void *data) pbc->bus_num = pcibus_num; pbc->numa_node = pcibus_numa_node; - pbc->allows_extended_config_space = pcibus_allows_extended_config_space; } static const TypeInfo pci_bus_info = { @@ -182,16 +197,11 @@ static const TypeInfo conventional_pci_interface_info = { .parent = TYPE_INTERFACE, }; -static bool pciebus_allows_extended_config_space(PCIBus *bus) -{ - return true; -} - static void pcie_bus_class_init(ObjectClass *klass, void *data) { - PCIBusClass *pbc = PCI_BUS_CLASS(klass); + BusClass *k = BUS_CLASS(klass); - pbc->allows_extended_config_space = pciebus_allows_extended_config_space; + k->realize = pcie_bus_realize; } static const TypeInfo pcie_bus_info = { @@ -410,11 +420,6 @@ bool pci_bus_is_express(PCIBus *bus) return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS); } -bool pci_bus_allows_extended_config_space(PCIBus *bus) -{ - return PCI_BUS_GET_CLASS(bus)->allows_extended_config_space(bus); -} - void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, const char *name, MemoryRegion *address_space_mem, -- cgit v1.2.3 From 2ad778b8c2175cb16f731d1e789e151a691172ce Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 13 May 2019 16:19:39 +1000 Subject: pci: Fold pci_get_bus_devfn() into its sole caller The only remaining caller of pci_get_bus_devfn() is pci_nic_init_nofail(), itself an old compatibility function. Fold the two together to avoid re-using the stale interface. While we're there replace the explicit fprintf()s with error_report(). Signed-off-by: David Gibson Message-Id: <20190513061939.3464-6-david@gibson.dropbear.id.au> Reviewed-by: Michael S. Tsirkin Signed-off-by: Michael S. Tsirkin Reviewed-by: Greg Kurz --- hw/pci/pci.c | 60 ++++++++++++++++++++++++++++-------------------------------- 1 file changed, 28 insertions(+), 32 deletions(-) (limited to 'hw/pci/pci.c') diff --git a/hw/pci/pci.c b/hw/pci/pci.c index 7e5f8d001b..d3893bdfe1 100644 --- a/hw/pci/pci.c +++ b/hw/pci/pci.c @@ -723,37 +723,6 @@ static int pci_parse_devaddr(const char *addr, int *domp, int *busp, return 0; } -static PCIBus *pci_get_bus_devfn(int *devfnp, PCIBus *root, - const char *devaddr) -{ - int dom, bus; - unsigned slot; - - if (!root) { - fprintf(stderr, "No primary PCI bus\n"); - return NULL; - } - - assert(!root->parent_dev); - - if (!devaddr) { - *devfnp = -1; - return pci_find_bus_nr(root, 0); - } - - if (pci_parse_devaddr(devaddr, &dom, &bus, &slot, NULL) < 0) { - return NULL; - } - - if (dom != 0) { - fprintf(stderr, "No support for non-zero PCI domains\n"); - return NULL; - } - - *devfnp = PCI_DEVFN(slot, 0); - return pci_find_bus_nr(root, bus); -} - static void pci_init_cmask(PCIDevice *dev) { pci_set_word(dev->cmask + PCI_VENDOR_ID, 0xffff); @@ -1895,6 +1864,8 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, DeviceState *dev; int devfn; int i; + int dom, busnr; + unsigned slot; if (nd->model && !strcmp(nd->model, "virtio")) { g_free(nd->model); @@ -1928,7 +1899,32 @@ PCIDevice *pci_nic_init_nofail(NICInfo *nd, PCIBus *rootbus, exit(1); } - bus = pci_get_bus_devfn(&devfn, rootbus, devaddr); + if (!rootbus) { + error_report("No primary PCI bus"); + exit(1); + } + + assert(!rootbus->parent_dev); + + if (!devaddr) { + devfn = -1; + busnr = 0; + } else { + if (pci_parse_devaddr(devaddr, &dom, &busnr, &slot, NULL) < 0) { + error_report("Invalid PCI device address %s for device %s", + devaddr, nd->model); + exit(1); + } + + if (dom != 0) { + error_report("No support for non-zero PCI domains"); + exit(1); + } + + devfn = PCI_DEVFN(slot, 0); + } + + bus = pci_find_bus_nr(rootbus, busnr); if (!bus) { error_report("Invalid PCI device address %s for device %s", devaddr, nd->model); -- cgit v1.2.3