From: Laine Stump <laine@redhat.com> A guest was created with PCIe bus and 2 PCIe devices attached to two separate pcie-root-ports that were themselves attached to a pcie-expander-bus. This guest was unable to boot from the 2nd device on the pcie-expander-bus. It turns out this failure was because not enough "bus numbers" were reserved for the pcie-expander-bus in the configuration, and this led to a failure of the system firmware to initialize those devices properly. Background: Each pci[e]-expander-bus has a busNr attribute that can be optionally specified in the <target> subelement; it defines the bus number of the expander bus itself. Any pci controller attached to that expander bus is then assigned a bus number between busNr+1 and the busNr of the next higher expander bus (or 255, if there are no expander buses with a higher busNr. The pci-root (or pcie-root) bus is bus number 0, and any pci controller attached to that bus (or its subordinate buses *except for pci[e]-expander-buses*) will be automatically numbered starting at 1, and continuing up until the lowest numbered expander bus. When libvirt originally added support for the pci-expander-bus, each pci-expander-bus' busNr was initialized by default to be the busNr of the current lowest expander bus - 2 (reserving one for the pci-expander-bus itself, and one for the implicit pci-bridge that is always added (by qemu) along with the pci-expander-bus). When pcie-expander-bus support was added, this logic was left untouched, but that is incorrect - a hotplug-supporting PCIe topology requires that each device have its own pcie-root-port, and so adding multiple hotpluggable devices to a pcie-expander-bus uses up one bus number for each device. So, the solution to the original problem is to just lower the busNr of each pcie-expander-bus by 16 (rather than 2) from the previous. This allows connection of up to 15 pcie-root-port controllers (and thus 15 hotpluggable endpoint devices) to each pcie-expander bus with the default setting of busNr. Resolves: https://redhat.atlassian.net/browse/RHEL-144343 Signed-off-by: Laine Stump <laine@redhat.com> -- Note that I could have chosen 32 or 8 or any other number, and no matter which I chose it would be likely to not work for *somebody*. Anyway, if this scheme ever doesn't work out, manual intervention is always possible, either by manually setting busNr, or by putting multiple endpoint devices on different functions of the same pcie-root-port (which is completely unproblematic if the device will never be unplugged at runtime). Signed-off-by: Laine Stump <laine@redhat.com> --- src/qemu/qemu_domain_address.c | 85 ++++++++++++------- ...e-expander-bus-aarch64.aarch64-latest.args | 2 +- ...ie-expander-bus-aarch64.aarch64-latest.xml | 2 +- 3 files changed, 57 insertions(+), 32 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index a8292a9782..81cf34fe86 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2567,43 +2567,63 @@ qemuDomainAddressFindNewTargetIndex(virDomainDef *def) static int -qemuDomainAddressFindNewBusNr(virDomainDef *def) +qemuDomainAddressFindNewBusNr(virDomainDef *def, + virDomainControllerModelPCI model) { /* Try to find a nice default for busNr for a new pci-expander-bus. * This is a bit tricky, since you need to satisfy the following: * - * 1) There need to be enough unused bus numbers between busNr of this - * bus and busNr of the next highest bus for the guest to assign a - * unique bus number to each PCI bus that is a child of this - * bus. Each PCI controller. On top of this, the pxb device (which - * implements the pci-expander-bus) includes a pci-bridge within - * it, and that bridge also uses one bus number (so each pxb device - * requires at least 2 bus numbers). + * 1) There need to be enough unused bus numbers between busNr of + * this bus and busNr of the next highest bus for the guest to + * assign a unique bus number to each PCI bus that is a child + * of this bus. * * 2) There need to be enough bus numbers *below* this for all the - * child controllers of the pci-expander-bus with the next lower + * child controllers of the pci[e]-expander-bus with the next lower * busNr (or the pci-root bus if there are no lower - * pci-expander-buses). + * pci[e]-expander-buses). * * 3) If at all possible, we want to avoid needing to change the busNr * of a bus in the future, as that changes the guest's device ABI, * which could potentially lead to issues with a guest OS that is * picky about such things. * - * Due to the impossibility of predicting what might be added to the - * config in the future, we can't make a foolproof choice, but since - * a pci-expander-bus (pxb) has slots for 32 devices, and the only - * practical use for it is to assign real devices on a particular - * NUMA node in the host, it's reasonably safe to assume it should - * never need any additional child buses (probably only a few of the - * 32 will ever be used). So for pci-expander-bus we find the lowest - * existing busNr, and set this one to the current lowest - 2 (one - * for the pxb, one for the integrated pci-bridge), thus leaving the - * maximum possible bus numbers available for other buses plugged - * into pci-root (i.e. pci-bridges and other - * pci-expander-buses). Anyone who needs more than 32 devices - * descended from one pci-expander-bus should set the busNr manually - * in the config. + * Due to the impossibility of predicting what might be added to + * the config in the future, we can't make a foolproof choice, + * and the parameters are different for a pci-expander-bus and a + * pcie-expander-bus. + * + * pci-expander-bus: the pxb device (which implements the + * pci-expander-bus) includes a pci-bridge within it, and that + * bridge uuses one bus number, so each pxb device requires at + * least 2 bus numbers. That bridge has slots for 32 devices, and + * its only practical use is to assign real physical devices on a + * particular NUMA node in the host. It's reasonably safe to + * assume no guest should never have more than 32 physical + * devices on a single NUMA node that need to be assigned to a + * guest (probably only a few of the 32 will ever be used), and + * so we shouldn't need more than 2 bus numbers for each + * pci-expander-bus. So we find the lowest existing busNr, and + * set busNr of the new pci-expander-bus to that value - + * 2. Anyone who needs more than 32 devices descended from one + * pci-expander-bus should set the busNr manually in the config. + * + * pcie-expander-bus: this is (as you'd expect) a more + * complicated story - if you want to support hotplugging on a + * pcie based machinetype, then each endpoint device needs its + * own pcie-root-port, and each root port uses up one bus number + * in the range available to its parent pcie-expander-bus. Taking + * a very generous example of a guest that has 8 GPUs, 8 NICs, + * and 8 NVME drives split between two NUMA nodes, giving 16 bus + * numbers to each pcie-expander-bus gives plenty of bus number + * space for all the controllers needed for each device in this + * extreme config, while still leaving plenty of overhead for 1) + * lots of devices on NUMA node 0 and 2) more than 2 NUMA nodes + * in the unlikely case that someone was forced to create a guest + * with more than 2 NUMA nodes. Again, if someone has a config + * more extreme than this, they should just set busNr manually + * (after all, the pci addresses of all these devices are already + * all being set manually, so that shouldn't be a huge burden). * * There is room for more error checking here - in particular we * can/should determine the ultimate parent (root-bus) of each PCI @@ -2614,6 +2634,10 @@ qemuDomainAddressFindNewBusNr(virDomainDef *def) size_t i; int lowestBusNr = 256; + int maxChildControllers = 1; /* default for pci-expander-bus */ + + if (model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS) + maxChildControllers = 15; for (i = 0; i < def->ncontrollers; i++) { virDomainControllerDef *cont = def->controllers[i]; @@ -2626,14 +2650,15 @@ qemuDomainAddressFindNewBusNr(virDomainDef *def) } } - /* If we already have a busNR = 1, then we can't auto-assign (0 is - * the pci[e]-root, and the others may have been assigned - * purposefully). + /* If we already have a controller with busNR <= + * maxChildControllers+1, then we can't auto-assign busNr to this + * new expander bus (0 is the pci[e]-root, and the others may have + * been assigned purposefully). */ - if (lowestBusNr <= 2) + if (lowestBusNr <= (maxChildControllers + 1)) return -1; - return lowestBusNr - 2; + return lowestBusNr - (maxChildControllers + 1); } @@ -2877,7 +2902,7 @@ qemuDomainAssignPCIAddresses(virDomainDef *def, case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: if (options->busNr == -1) - options->busNr = qemuDomainAddressFindNewBusNr(def); + options->busNr = qemuDomainAddressFindNewBusNr(def, cont->model); if (options->busNr == -1) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("No free busNr lower than current lowest busNr is available to auto-assign to bus %1$d. Must be manually assigned"), diff --git a/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.args b/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.args index e76510dbbb..52011865a0 100644 --- a/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.args +++ b/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.args @@ -25,7 +25,7 @@ XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-pcie-expander-bus-te/.config \ -rtc base=utc \ -no-shutdown \ -boot strict=on \ --device '{"driver":"pxb-pcie","bus_nr":254,"id":"pci.1","bus":"pcie.0","addr":"0x1"}' \ +-device '{"driver":"pxb-pcie","bus_nr":240,"id":"pci.1","bus":"pcie.0","addr":"0x1"}' \ -audiodev '{"id":"audio1","driver":"none"}' \ -sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ -msg timestamp=on diff --git a/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.xml b/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.xml index ce88da574f..0d0cac7653 100644 --- a/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.xml +++ b/tests/qemuxmlconfdata/pcie-expander-bus-aarch64.aarch64-latest.xml @@ -20,7 +20,7 @@ <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='pcie-expander-bus'> <model name='pxb-pcie'/> - <target busNr='254'/> + <target busNr='240'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> <audio id='1' type='none'/> -- 2.55.0