jcs.org / qemu
qemu with hax to log dma reads & writes jcs.org/2018/11/12/vfio
fork atom

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 <david@gibson.dropbear.id.au>
Reviewed-by: Greg Kurz <groug@kaod.org>
Message-Id: <20190513061939.3464-4-david@gibson.dropbear.id.au>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>

authored by

David Gibson and committed by
Michael S. Tsirkin 2f57db8a e4610781

+43 -54
+23 -18
hw/pci/pci.c
··· 120 120 vmstate_register(NULL, -1, &vmstate_pcibus, bus); 121 121 } 122 122 123 + static void pcie_bus_realize(BusState *qbus, Error **errp) 124 + { 125 + PCIBus *bus = PCI_BUS(qbus); 126 + 127 + pci_bus_realize(qbus, errp); 128 + 129 + /* 130 + * A PCI-E bus can support extended config space if it's the root 131 + * bus, or if the bus/bridge above it does as well 132 + */ 133 + if (pci_bus_is_root(bus)) { 134 + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; 135 + } else { 136 + PCIBus *parent_bus = pci_get_bus(bus->parent_dev); 137 + 138 + if (pci_bus_allows_extended_config_space(parent_bus)) { 139 + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; 140 + } 141 + } 142 + } 143 + 123 144 static void pci_bus_unrealize(BusState *qbus, Error **errp) 124 145 { 125 146 PCIBus *bus = PCI_BUS(qbus); ··· 142 163 return NUMA_NODE_UNASSIGNED; 143 164 } 144 165 145 - static bool pcibus_allows_extended_config_space(PCIBus *bus) 146 - { 147 - return false; 148 - } 149 - 150 166 static void pci_bus_class_init(ObjectClass *klass, void *data) 151 167 { 152 168 BusClass *k = BUS_CLASS(klass); ··· 161 177 162 178 pbc->bus_num = pcibus_num; 163 179 pbc->numa_node = pcibus_numa_node; 164 - pbc->allows_extended_config_space = pcibus_allows_extended_config_space; 165 180 } 166 181 167 182 static const TypeInfo pci_bus_info = { ··· 182 197 .parent = TYPE_INTERFACE, 183 198 }; 184 199 185 - static bool pciebus_allows_extended_config_space(PCIBus *bus) 186 - { 187 - return true; 188 - } 189 - 190 200 static void pcie_bus_class_init(ObjectClass *klass, void *data) 191 201 { 192 - PCIBusClass *pbc = PCI_BUS_CLASS(klass); 202 + BusClass *k = BUS_CLASS(klass); 193 203 194 - pbc->allows_extended_config_space = pciebus_allows_extended_config_space; 204 + k->realize = pcie_bus_realize; 195 205 } 196 206 197 207 static const TypeInfo pcie_bus_info = { ··· 408 418 bool pci_bus_is_express(PCIBus *bus) 409 419 { 410 420 return object_dynamic_cast(OBJECT(bus), TYPE_PCIE_BUS); 411 - } 412 - 413 - bool pci_bus_allows_extended_config_space(PCIBus *bus) 414 - { 415 - return PCI_BUS_GET_CLASS(bus)->allows_extended_config_space(bus); 416 421 } 417 422 418 423 void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent,
+3 -10
hw/pci/pci_host.c
··· 53 53 54 54 static void pci_adjust_config_limit(PCIBus *bus, uint32_t *limit) 55 55 { 56 - if (*limit > PCI_CONFIG_SPACE_SIZE) { 57 - if (!pci_bus_allows_extended_config_space(bus)) { 58 - *limit = PCI_CONFIG_SPACE_SIZE; 59 - return; 60 - } 61 - 62 - if (!pci_bus_is_root(bus)) { 63 - PCIDevice *bridge = pci_bridge_get_device(bus); 64 - pci_adjust_config_limit(pci_get_bus(bridge), limit); 65 - } 56 + if ((*limit > PCI_CONFIG_SPACE_SIZE) && 57 + !pci_bus_allows_extended_config_space(bus)) { 58 + *limit = PCI_CONFIG_SPACE_SIZE; 66 59 } 67 60 } 68 61
+10 -24
hw/ppc/spapr_pci.c
··· 1626 1626 memory_region_del_subregion(get_system_memory(), &sphb->mem32window); 1627 1627 } 1628 1628 1629 - static bool spapr_phb_allows_extended_config_space(PCIBus *bus) 1630 - { 1631 - SpaprPhbState *sphb = SPAPR_PCI_HOST_BRIDGE(BUS(bus)->parent); 1632 - 1633 - return sphb->pcie_ecs; 1634 - } 1635 - 1636 - static void spapr_phb_root_bus_class_init(ObjectClass *klass, void *data) 1637 - { 1638 - PCIBusClass *pbc = PCI_BUS_CLASS(klass); 1639 - 1640 - pbc->allows_extended_config_space = spapr_phb_allows_extended_config_space; 1641 - } 1642 - 1643 - #define TYPE_SPAPR_PHB_ROOT_BUS "pci" 1644 - 1645 - static const TypeInfo spapr_phb_root_bus_info = { 1646 - .name = TYPE_SPAPR_PHB_ROOT_BUS, 1647 - .parent = TYPE_PCI_BUS, 1648 - .class_init = spapr_phb_root_bus_class_init, 1649 - }; 1650 - 1651 1629 static void spapr_phb_realize(DeviceState *dev, Error **errp) 1652 1630 { 1653 1631 /* We don't use SPAPR_MACHINE() in order to exit gracefully if the user ··· 1753 1731 pci_spapr_set_irq, pci_swizzle_map_irq_fn, sphb, 1754 1732 &sphb->memspace, &sphb->iospace, 1755 1733 PCI_DEVFN(0, 0), PCI_NUM_PINS, 1756 - TYPE_SPAPR_PHB_ROOT_BUS); 1734 + TYPE_PCI_BUS); 1735 + 1736 + /* 1737 + * Despite resembling a vanilla PCI bus in most ways, the PAPR 1738 + * para-virtualized PCI bus *does* permit PCI-E extended config 1739 + * space access 1740 + */ 1741 + if (sphb->pcie_ecs) { 1742 + bus->flags |= PCI_BUS_EXTENDED_CONFIG_SPACE; 1743 + } 1757 1744 phb->bus = bus; 1758 1745 qbus_set_hotplug_handler(BUS(phb->bus), OBJECT(sphb), NULL); 1759 1746 ··· 2348 2335 static void spapr_pci_register_types(void) 2349 2336 { 2350 2337 type_register_static(&spapr_phb_info); 2351 - type_register_static(&spapr_phb_root_bus_info); 2352 2338 } 2353 2339 2354 2340 type_init(spapr_pci_register_types)
-1
include/hw/pci/pci.h
··· 395 395 #define TYPE_PCIE_BUS "PCIE" 396 396 397 397 bool pci_bus_is_express(PCIBus *bus); 398 - bool pci_bus_allows_extended_config_space(PCIBus *bus); 399 398 400 399 void pci_root_bus_new_inplace(PCIBus *bus, size_t bus_size, DeviceState *parent, 401 400 const char *name,
+7 -1
include/hw/pci/pci_bus.h
··· 17 17 18 18 int (*bus_num)(PCIBus *bus); 19 19 uint16_t (*numa_node)(PCIBus *bus); 20 - bool (*allows_extended_config_space)(PCIBus *bus); 21 20 } PCIBusClass; 22 21 23 22 enum PCIBusFlags { 24 23 /* This bus is the root of a PCI domain */ 25 24 PCI_BUS_IS_ROOT = 0x0001, 25 + /* PCIe extended configuration space is accessible on this bus */ 26 + PCI_BUS_EXTENDED_CONFIG_SPACE = 0x0002, 26 27 }; 27 28 28 29 struct PCIBus { ··· 55 56 static inline bool pci_bus_is_root(PCIBus *bus) 56 57 { 57 58 return !!(bus->flags & PCI_BUS_IS_ROOT); 59 + } 60 + 61 + static inline bool pci_bus_allows_extended_config_space(PCIBus *bus) 62 + { 63 + return !!(bus->flags & PCI_BUS_EXTENDED_CONFIG_SPACE); 58 64 } 59 65 60 66 #endif /* QEMU_PCI_BUS_H */