Re: [libvirt] [PATCHv2 2/7] qemu: add pcie-root controller

This controller is implicit on q35 machinetypes. It provides 31 PCIe (*not* PCI) slots as controller 0. Currently there are no devices that can connect to pcie-root, and no implicit pci controller on a q35 machine, so q35 is still unusable. For a usable q35 system, we need to add a "dmi-to-pci-bridge" pci controller, which can connect to pcie-root, and provides standard pci slots that can be used to connect other devices. --- docs/formatdomain.html.in | 27 ++++++++++++++++------ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 8 ++++--- src/conf/domain_conf.h | 1 + src/qemu/qemu_command.c | 23 ++++++++++++++---- src/qemu/qemu_command.h | 4 +++- src/qemu/qemu_domain.c | 19 +++++++++++---- tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 4 ++++ tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml | 21 +++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/qemuxml2xmltest.c | 1 + 11 files changed, 92 insertions(+), 19 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 49c7c8d..330dca2 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -2338,13 +2338,11 @@ <p> PCI controllers have an optional <code>model</code> attribute with - possible values <code>pci-root</code> or <code>pci-bridge</code>. - For machine types which provide an implicit pci bus, the pci-root + possible values <code>pci-root</code>, <code>pcie-root</code> + or <code>pci-bridge</code>. + For machine types which provide an implicit PCI bus, the pci-root controller with index=0 is auto-added and required to use PCI devices. - PCI root has no address. - PCI bridges are auto-added if there are too many devices to fit on - the one bus provided by pci-root, or a PCI bus number greater than zero - was specified. + pci-root has no address. PCI bridges can also be specified manually, but their addresses should only refer to PCI buses provided by already specified PCI controllers. Leaving gaps in the PCI controller indexes might lead to an invalid @@ -2356,11 +2354,26 @@ &lt;devices&gt; &lt;controller type='pci' index='0' model='pci-root'/&gt; &lt;controller type='pci' index='1' model='pci-bridge'&gt; - &lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction=off'/&gt; + &lt;address type='pci' domain='0' bus='0' slot='5' function='0' multifunction='off'/&gt; &lt;/controller&gt; &lt;/devices&gt; ...</pre>

+ <p> + For machine types which provide an implicit PCI Express (PCIe) + bus (for example, the machine types based on the Q35 chipset), + the pcie-root controller with index=0 is auto-added to the + domain's configuration. pcie-root has also no address, provides + 31 slots (numbered 1-31) and can only be used to attach PCIe + devices. (<span class="since">since 1.1.2</span>). + </p> +<pre> + ... + &lt;devices&gt; + &lt;controller type='pci' index='0' model='pcie-root'/&gt; + &lt;/devices&gt; + ...</pre> + <h4><a name="elementsLease">Device leases</a></h4> <p> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 745b959..e04be12 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1538,6 +1538,7 @@ <attribute name="model"> <choice> <value>pci-root</value> + <value>pcie-root</value> <value>pci-bridge</value> </choice> </attribute> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 63350b6..59a96f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -310,6 +310,7 @@ VIR_ENUM_IMPL(virDomainController, VIR_DOMAIN_CONTROLLER_TYPE_LAST, VIR_ENUM_IMPL(virDomainControllerModelPCI, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST, "pci-root", + "pcie-root", "pci-bridge") VIR_ENUM_IMPL(virDomainControllerModelSCSI, VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST, @@ -5714,16 +5715,17 @@ virDomainControllerDefParseXML(xmlNodePtr node, case VIR_DOMAIN_CONTROLLER_TYPE_PCI: switch (def->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: if (def->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("pci-root controller should not " + _("pci-root and pcie-root controllers should not " "have an address")); goto error; } if (def->idx != 0) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("pci-root controller should have " - "index 0")); + _("pci-root and pcie-root controllers " + "should have index 0")); goto error; } diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index de3b59c..63a1444 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -768,6 +768,7 @@ enum virDomainControllerType { typedef enum { VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT, + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE, VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cafc4bf..b6912ce 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -1453,6 +1453,12 @@ qemuDomainPCIAddressFlagsCompatible(virDevicePCIAddressPtr addr, "device. Device requires a standard PCI slot, " "which is not provided by this bus"), addr->domain, addr->bus); + } else if (devFlags & QEMU_PCI_CONNECT_TYPE_PCIE) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("PCI bus %.4x:%.2x is not compatible with the " + "device. Device requires a PCI Express slot, " + "which is not provided by this bus"), + addr->domain, addr->bus); } else { /* this should never happen. If it does, there is a * bug in the code that sets the flag bits for devices. @@ -1549,6 +1555,12 @@ qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus, bus->minSlot = 1; bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + /* slots 1 - 31, PCIe devices only, no hotplug */ + bus->flags = QEMU_PCI_CONNECT_TYPE_PCIE; + bus->minSlot = 1; + bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST; + break; default: virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid PCI controller model %d"), model); @@ -2347,7 +2359,8 @@ qemuAssignDevicePCISlots(virDomainDefPtr def, continue; switch (def->controllers[i]->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: - /* pci-root is implicit in the machine, + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + /* pci-root and pcie-root are implicit in the machine, * and needs no address */ continue; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: @@ -4336,8 +4349,9 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef, def->idx, def->idx); break; case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("wrong function called for pci-root")); + _("wrong function called for pci-root/pcie-root")); return NULL; } break; @@ -7615,9 +7629,10 @@ qemuBuildCommandLine(virConnectPtr conn, continue; } - /* Skip pci-root */ + /* Skip pci-root/pcie-root */ if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI && - cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { + (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT || + cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT)) { continue; } diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index bf4953a..e5111d2 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -233,13 +233,15 @@ typedef enum { QEMU_PCI_CONNECT_TYPE_PCI = 1 << 2, /* PCI devices can connect to this bus */ + QEMU_PCI_CONNECT_TYPE_PCIE = 1 << 3, + /* PCI Express devices can connect to this bus */ } qemuDomainPCIConnectFlags; /* a combination of all bit that describe the type of connections * allowed, e.g. PCI, PCIe, switch */ # define QEMU_PCI_CONNECT_TYPES_MASK \ - QEMU_PCI_CONNECT_TYPE_PCI + (QEMU_PCI_CONNECT_TYPE_PCI | QEMU_PCI_CONNECT_TYPE_PCIE) int qemuDomainAssignPCIAddresses(virDomainDefPtr def, diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index d3da666..4d04609 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -701,6 +701,7 @@ qemuDomainDefPostParse(virDomainDefPtr def, { bool addDefaultUSB = true; bool addPCIRoot = false; + bool addPCIeRoot = false; /* check for emulator and create a default one if needed */ if (!def->emulator && @@ -713,12 +714,16 @@ qemuDomainDefPostParse(virDomainDefPtr def, case VIR_ARCH_X86_64: if (!def->os.machine) break; - if (STRPREFIX(def->os.machine, "pc-q35") || - STREQ(def->os.machine, "q35") || - STREQ(def->os.machine, "isapc")) { + if (STREQ(def->os.machine, "isapc")) { addDefaultUSB = false; break; } + if (STRPREFIX(def->os.machine, "pc-q35") || + STREQ(def->os.machine, "q35")) { + addPCIeRoot = true; + addDefaultUSB = false; + break; + } if (!STRPREFIX(def->os.machine, "pc-0.") && !STRPREFIX(def->os.machine, "pc-1.") && !STRPREFIX(def->os.machine, "pc-i440") && @@ -755,6 +760,12 @@ qemuDomainDefPostParse(virDomainDefPtr def, VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0) return -1; + if (addPCIeRoot && + virDomainDefMaybeAddController( + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0, + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0) + return -1; + return 0; } @@ -1421,7 +1432,7 @@ qemuDomainDefFormatBuf(virQEMUDriverPtr driver, if (pci && pci->idx == 0 && pci->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) { - VIR_DEBUG("Removing default 'pci-root' from domain '%s'" + VIR_DEBUG("Removing default pci-root from domain '%s'" " for migration compatibility", def->name); toremove++; } else { diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args new file mode 100644 index 0000000..e937189 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args @@ -0,0 +1,4 @@ +LC_ALL=C PATH=/bin HOME=/home/test USER=test LOGNAME=test /usr/libexec/qemu-kvm \ +-S -M q35 -m 2048 -smp 2 -nographic -nodefaults \ +-monitor unix:/tmp/test-monitor,server,nowait -no-acpi -boot c \ +-device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.1,addr=0x1 -usb diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml new file mode 100644 index 0000000..1aa5455 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.xml @@ -0,0 +1,21 @@ +<domain type='qemu'> + <name>q35-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static' cpuset='0-1'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='usb' index='0'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b7485fc..57c6989 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -994,6 +994,8 @@ mymain(void) DO_TEST("pci-autoadd-idx", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST("pci-bridge-many-disks", QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE_PCI_BRIDGE); + DO_TEST("pcie-root", + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE, QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE, diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 66be40e..ea511b8 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -294,6 +294,7 @@ mymain(void) DO_TEST_DIFFERENT("pci-bridge-many-disks"); DO_TEST_DIFFERENT("pci-autoadd-addr"); DO_TEST_DIFFERENT("pci-autoadd-idx"); + DO_TEST("pcie-root"); DO_TEST("hostdev-scsi-lsi"); DO_TEST("hostdev-scsi-virtio-scsi"); -- 1.7.11.7 -- libvir-list mailing list libvir-list(a)redhat.com https://www.redhat.com/mailman/listinfo/libvir-list