Re: [libvirt] [PATCH 4/5] qemu: set/validate slot/connection type when assigning slots for PCI devices

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 059aa6a..64787b6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1412,7 +1412,15 @@ cleanup: #define QEMU_PCI_ADDRESS_FUNCTION_LAST 7 typedef struct { - /* Each bit in a slot represents one function on that slot */ + virDomainControllerModelPCI model; + /* flags an min/max can be computed from model, but + * having them ready makes life easier. + */ + qemuDomainPCIConnectFlags flags; + size_t minSlot, maxSlot; /* usually 0,0 or 1,31 */ + /* Each bit in a slot represents one function on that slot. If the + * bit is set, that function is in use by a device. + */ uint8_t slots[QEMU_PCI_ADDRESS_SLOT_LAST + 1]; } qemuDomainPCIAddressBus; typedef qemuDomainPCIAddressBus *qemuDomainPCIAddressBusPtr; @@ -1430,9 +1438,13 @@ struct _qemuDomainPCIAddressSet { * Check that the PCI address is valid for use * with the specified PCI address set. */ -static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, - virDevicePCIAddressPtr addr) +static bool +qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UNUSED, + virDevicePCIAddressPtr addr, + qemuDomainPCIConnectFlags flags) { + qemuDomainPCIAddressBusPtr bus; + if (addrs->nbuses == 0) { virReportError(VIR_ERR_XML_ERROR, "%s", _("No PCI buses available")); return false; @@ -1448,32 +1460,81 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN addrs->nbuses - 1); return false; } - if (addr->function > QEMU_PCI_ADDRESS_FUNCTION_LAST) { + + bus = &addrs->buses[addr->bus]; + + /* assure that at least one of the requested connection types is + * provided by this bus + */ + if (!(flags & bus->flags & QEMU_PCI_CONNECT_TYPES_MASK)) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid PCI address: function must be <= %u"), - QEMU_PCI_ADDRESS_FUNCTION_LAST); + _("Invalid PCI address: The PCI controller " + "providing bus %04x doesn't support " + "connections appropriate for the device " + "(%x required vs. %x provided by bus)"), + addr->bus, flags & QEMU_PCI_CONNECT_TYPES_MASK, + bus->flags & QEMU_PCI_CONNECT_TYPES_MASK); return false; } - if (addr->slot > QEMU_PCI_ADDRESS_SLOT_LAST) { + /* make sure this bus allows hot-plug if the caller demands it */ + if ((flags & QEMU_PCI_CONNECT_HOTPLUGGABLE) && + !(bus->flags & QEMU_PCI_CONNECT_HOTPLUGGABLE)) { virReportError(VIR_ERR_XML_ERROR, - _("Invalid PCI address: slot must be <= %u"), - QEMU_PCI_ADDRESS_SLOT_LAST); + _("Invalid PCI address: hot-pluggable slot requested, " + "but bus %04x doesn't support hot-plug"), addr->bus); return false; } - if (addr->slot == 0) { - if (addr->bus) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Slot 0 is unusable on PCI bridges")); - } else { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("Slot 0 on bus 0 is reserved for the host bridge")); - } + /* some "buses" are really just a single port */ + if (bus->minSlot && addr->slot < bus->minSlot) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid PCI address: slot must be >= %zu"), + bus->minSlot); + return false; + } + if (addr->slot > bus->maxSlot) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid PCI address: slot must be <= %zu"), + bus->maxSlot); + return false; + } + if (addr->function > QEMU_PCI_ADDRESS_FUNCTION_LAST) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid PCI address: function must be <= %u"), + QEMU_PCI_ADDRESS_FUNCTION_LAST);

+ +static int +qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus, + virDomainControllerModelPCI model) +{ + switch (model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + bus->flags = (QEMU_PCI_CONNECT_HOTPLUGGABLE | + QEMU_PCI_CONNECT_TYPE_PCI); + bus->minSlot = 1; + bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; + break; + default: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid PCI controller model %d"), model); + return -1; + } + + bus->model = model; + return 0; +} + + /* Ensure addr fits in the address set, by expanding it if needed. + * This will only grow if the flags say that we need a normal + * hot-pluggable PCI slot. If we need a different type of slot, it + * will fail. + * * Return value: * -1 = OOM * 0 = no action performed @@ -1481,7 +1542,8 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN */ static int qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, - virDevicePCIAddressPtr addr) + virDevicePCIAddressPtr addr, + qemuDomainPCIConnectFlags flags) { int add; size_t i; @@ -1490,16 +1552,33 @@ qemuDomainPCIAddressSetGrow(qemuDomainPCIAddressSetPtr addrs, i = addrs->nbuses; if (add <= 0) return 0; + + /* auto-grow only works when we're adding plain PCI devices */ + if (!(flags & QEMU_PCI_CONNECT_TYPE_PCI)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot automatically add a new PCI bus for a " + "device requiring a slot other than standard PCI.")); + return -1; + } + if (VIR_EXPAND_N(addrs->buses, addrs->nbuses, add) < 0) return -1; - /* reserve slot 0 on the new buses */ - for (; i < addrs->nbuses; i++) - addrs->buses[i].slots[0] = 0xFF; + + for (; i < addrs->nbuses; i++) { + /* Any time we auto-add a bus, we will want a multi-slot + * bus. Currently the only type of bus we will auto-add is a + * pci-bridge, which is hot-pluggable and provides standard + * PCI slots. + */ + qemuDomainPCIAddressBusSetModel(&addrs->buses[i], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + } return add;