On Sat, Aug 3, 2013 at 9:01 PM, Laine Stump <[email protected]> wrote:
> This PCI controller, named "dmi-to-pci-bridge" in the libvirt config,
> and implemented with qemu's "i82801b11-bridge" device, connects to a
> PCI Express slot (e.g. one of the slots provided by the pcie-root
> controller, aka "pcie.0" on the qemu commandline), and provides 31
> *non-hot-pluggable* PCI (*not* PCIe) slots, numbered 1-31.
>
> Any time a machine is defined which has a pcie-root controller
> (i.e. any q35-based machinetype), libvirt will automatically add a
> dmi-to-pci-bridge controller if one doesn't exist, and also add a
> pci-bridge controller. The reasoning here is that any useful domain
> will have either an immediate (startup time) or eventual (subsequent
> hot-plug) need for a standard PCI slot; since the pcie-root controller
> only provides PCIe slots, we need to connect a dmi-to-pci-bridge
> controller to it in order to get a non-hot-plug PCI slot that we can
> then use to connect a pci-bridge - the slots provided by the
> pci-bridge will be both standard PCI and hot-pluggable.
>
> Since pci-bridge devices themselves are not hot-pluggable, any new
> pci-bridge controller that is added can (and will) be plugged into the
> dmi-to-pci-bridge as long as it has empty slots available.
Worth noting DO_TEST_DIFFERENT to pcie-root change here since you
mention changes like that in later commits.
> ---
> docs/formatdomain.html.in | 28 +++++++++++++++---
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c | 3 +-
> src/conf/domain_conf.h | 1 +
> src/qemu/qemu_capabilities.c | 2 ++
> src/qemu/qemu_capabilities.h | 1 +
> src/qemu/qemu_command.c | 33
> ++++++++++++++++++++++
> src/qemu/qemu_domain.c | 14 +++++++--
> tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args | 3 +-
> tests/qemuxml2argvdata/qemuxml2argv-q35.args | 7 +++++
> tests/qemuxml2argvdata/qemuxml2argv-q35.xml | 25 ++++++++++++++++
> tests/qemuxml2argvtest.c | 8 +++++-
> .../qemuxml2xmlout-pcie-root.xml | 23 +++++++++++++++
> tests/qemuxml2xmltest.c | 3 +-
> 14 files changed, 142 insertions(+), 10 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.args
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-q35.xml
> create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
>
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index 330dca2..8fa4c0e 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -2338,16 +2338,19 @@
>
> <p>
> PCI controllers have an optional <code>model</code> attribute with
> - possible values <code>pci-root</code>, <code>pcie-root</code>
> - or <code>pci-bridge</code>.
> + possible values <code>pci-root</code>, <code>pcie-root</code>,
> + <code>pci-bridge</code>, or <code>dmi-to-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 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
> configuration.
> - (<span class="since">since 1.0.5</span>)
> + (pci-root and pci-bridge <span class="since">since 1.0.5</span>).
Probably belonged as part of the last patch technically, but they'll
be part of the series so it should be ok.
> </p>
> <pre>
> ...
> @@ -2365,12 +2368,29 @@
> 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>).
> + devices. In order to connect standard PCI devices on a system
> + which has a pcie-root controller, a pci controller
> + with <code>model='dmi-to-pci-bridge'</code> is automatically
> + added. A dmi-to-pci-bridge controller plugs into a PCIe slot (as
> + provided by pcie-root), and itself provides 31 standard PCI
> + slots (which are not hot-pluggable). In order to have
> + hot-pluggable PCI slots in the guest system, a pci-bridge
> + controller will also be automatically created and connected to
> + one of the slots of the auto-created dmi-to-pci-bridge
> + controller; all guest devices with PCI addresses that are
> + auto-determined by libvirt will be placed on this pci-bridge
> + device. (<span class="since">since 1.1.2</span>).
> </p>
> <pre>
> ...
> <devices>
> <controller type='pci' index='0' model='pcie-root'/>
> + <controller type='pci' index='1' model='dmi-to-pci-bridge'>
> + <address type='pci' domain='0' bus='0' slot='0xe' function='0'/>
> + </controller>
> + <controller type='pci' index='2' model='pci-bridge'>
> + <address type='pci' domain='0' bus='1' slot='1' function='0'/>
> + </controller>
> </devices>
> ...</pre>
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index e04be12..173359c 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -1540,6 +1540,7 @@
> <value>pci-root</value>
> <value>pcie-root</value>
> <value>pci-bridge</value>
> + <value>dmi-to-pci-bridge</value>
> </choice>
> </attribute>
> </group>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 59a96f2..d17008f 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -311,7 +311,8 @@ 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")
> + "pci-bridge",
> + "dmi-to-pci-bridge")
>
> VIR_ENUM_IMPL(virDomainControllerModelSCSI,
> VIR_DOMAIN_CONTROLLER_MODEL_SCSI_LAST,
> "auto",
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 63a1444..3e118d6 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -770,6 +770,7 @@ typedef enum {
> VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT,
> VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT,
> VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE,
> + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE,
>
> VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST
> } virDomainControllerModelPCI;
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index 08406b8..47cc07a 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -234,6 +234,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>
> "vnc-share-policy", /* 150 */
> "device-del-event",
> + "dmi-to-pci-bridge",
> );
>
> struct _virQEMUCaps {
> @@ -1381,6 +1382,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[]
> = {
> { "pci-bridge", QEMU_CAPS_DEVICE_PCI_BRIDGE },
> { "vfio-pci", QEMU_CAPS_DEVICE_VFIO_PCI },
> { "scsi-generic", QEMU_CAPS_DEVICE_SCSI_GENERIC },
> + { "i82801b11-bridge", QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE },
> };
>
> static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBlk[] = {
> diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
> index f5f685d..074e55d 100644
> --- a/src/qemu/qemu_capabilities.h
> +++ b/src/qemu/qemu_capabilities.h
> @@ -190,6 +190,7 @@ enum virQEMUCapsFlags {
> QEMU_CAPS_MLOCK = 149, /* -realtime mlock=on|off */
> QEMU_CAPS_VNC_SHARE_POLICY = 150, /* set display sharing policy */
> QEMU_CAPS_DEVICE_DEL_EVENT = 151, /* DEVICE_DELETED event */
> + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE = 152, /* -device i82801b11-bridge */
>
> QEMU_CAPS_LAST, /* this must always be the last item */
> };
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index b6912ce..13a68a5 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -1561,6 +1561,13 @@
> qemuDomainPCIAddressBusSetModel(qemuDomainPCIAddressBusPtr bus,
> bus->minSlot = 1;
> bus->maxSlot = QEMU_PCI_ADDRESS_SLOT_LAST;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> + /* slots 1 - 31, standard PCI slots,
> + * but *not* hot-pluggable */
> + bus->flags = 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);
> @@ -1669,6 +1676,12 @@ qemuCollectPCIAddress(virDomainDefPtr def
> ATTRIBUTE_UNUSED,
> */
> flags = QEMU_PCI_CONNECT_TYPE_PCI;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> + /* pci-bridge needs a PCIe slot, but it isn't
> + * hot-pluggable, so it doesn't need a hot-pluggable slot.
> + */
> + flags = QEMU_PCI_CONNECT_TYPE_PCIE;
> + break;
> default:
> break;
> }
> @@ -2369,6 +2382,12 @@ qemuAssignDevicePCISlots(virDomainDefPtr def,
> */
> flags = QEMU_PCI_CONNECT_TYPE_PCI;
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> + /* dmi-to-pci-bridge requires a non-hotplug PCIe
> + * slot
> + */
> + flags = QEMU_PCI_CONNECT_TYPE_PCIE;
> + break;
> default:
> flags = QEMU_PCI_CONNECT_HOTPLUGGABLE |
> QEMU_PCI_CONNECT_TYPE_PCI;
> break;
> @@ -4348,6 +4367,20 @@ qemuBuildControllerDevStr(virDomainDefPtr domainDef,
> virBufferAsprintf(&buf, "pci-bridge,chassis_nr=%d,id=pci.%d",
> def->idx, def->idx);
> break;
> + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE:
> + if (!virQEMUCapsGet(qemuCaps,
> QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE)) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("The dmi-to-pci-bridge (i82801b11-bridge) "
> + "controller is not supported in this QEMU
> binary"));
Do we ever print out the path to the QEMU binary used in these kinds
of errors? Might be nice.
> + goto error;
> + }
> + if (def->idx == 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("dmi-to-pci-bridge index should be > 0"));
> + goto error;
> + }
> + virBufferAsprintf(&buf, "i82801b11-bridge,id=pci.%d", def->idx);
> + break;
> case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT:
> case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT:
> virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 4d04609..f5030cd 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -760,10 +760,20 @@ qemuDomainDefPostParse(virDomainDefPtr def,
> VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
> return -1;
>
> + /* When a machine has a pcie-root, make sure that there is always
> + * a dmi-to-pci-bridge controller added as bus 1, and a pci-bridge
> + * as bus 2, so that standard PCI devices can be connected
> + */
> if (addPCIeRoot &&
> - virDomainDefMaybeAddController(
> + (virDomainDefMaybeAddController(
> def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 0,
> - VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0)
> + VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) < 0 ||
> + virDomainDefMaybeAddController(
> + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 1,
> + VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE) < 0 ||
> + virDomainDefMaybeAddController(
> + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, 2,
> + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE) < 0))
> return -1;
Not going to lie, that's an if check of hell. Had to really stare for
a second to make sure everything was lined up correctly.
>
> return 0;
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> index e937189..23db85c 100644
> --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root.args
> @@ -1,4 +1,5 @@
> 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
> +-device i82801b11-bridge,id=pci.1,bus=pci.0,addr=0x1 \
> +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 -usb
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> new file mode 100644
> index 0000000..ddff6f0
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.args
> @@ -0,0 +1,7 @@
> +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 i82801b11-bridge,id=pci.1,bus=pci.0,addr=0x1 \
> +-device pci-bridge,chassis_nr=2,id=pci.2,bus=pci.1,addr=0x1 \
> +-usb \
> +-vga qxl -global qxl.ram_size=67108864 -global qxl.vram_size=18874368
> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
> b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
> new file mode 100644
> index 0000000..3541b14
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-q35.xml
> @@ -0,0 +1,25 @@
> +<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='pci' index='1' model='dmi-to-pci-bridge'/>
> + <controller type='pci' index='2' model='pci-bridge'/>
> + <video>
> + <model type='qxl' ram='65536' vram='18432' heads='1'/>
> + </video>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
> index 57c6989..aba0f88 100644
> --- a/tests/qemuxml2argvtest.c
> +++ b/tests/qemuxml2argvtest.c
> @@ -995,7 +995,13 @@ mymain(void)
> 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);
> + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
> + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE);
> + DO_TEST("q35",
> + QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE,
> + QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE,
> + QEMU_CAPS_VGA, QEMU_CAPS_DEVICE_VIDEO_PRIMARY,
> + QEMU_CAPS_VGA, QEMU_CAPS_VGA_QXL, QEMU_CAPS_DEVICE_QXL);
>
> DO_TEST("hostdev-scsi-lsi", QEMU_CAPS_DRIVE,
> QEMU_CAPS_DEVICE, QEMU_CAPS_DRIVE,
> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
> b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
> new file mode 100644
> index 0000000..25c77f1
> --- /dev/null
> +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root.xml
> @@ -0,0 +1,23 @@
> +<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'/>
> + <controller type='pci' index='1' model='dmi-to-pci-bridge'/>
> + <controller type='pci' index='2' model='pci-bridge'/>
> + <memballoon model='none'/>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index ea511b8..8b4590a 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -294,7 +294,8 @@ 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_DIFFERENT("pcie-root");
> + DO_TEST("q35");
>
> DO_TEST("hostdev-scsi-lsi");
> DO_TEST("hostdev-scsi-virtio-scsi");
> --
> 1.7.11.7
>
> --
> libvir-list mailing list
> [email protected]
> https://www.redhat.com/mailman/listinfo/libvir-list
Overall an ACK, just fix up the commit message and move the since
documentation fixup to 2/7, which I think you can do without
reposting.
--
Doug Goldstein
--
libvir-list mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/libvir-list
