[libvirt] [PATCH 0/4] qemu: Add support for generic PCIe Root Ports
Andrea Bolognani (4): qemu: Add support for generic PCIe Root Ports qemu: Use generic PCIe Root Ports by default for aarch64/virt tests: Test generic PCIe Root Ports news: Document support for generic PCIe Root Ports docs/news.xml | 9 +++++ docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 4 ++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 ++++++++-- src/qemu/qemu_domain_address.c | 15 +++++++-- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + ...uxml2argv-pcie-root-port-mach-virt-generic.args | 22 +++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-generic.xml | 22 +++++++++++++ ...uxml2argv-pcie-root-port-mach-virt-ioh3420.args | 21 ++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-ioh3420.xml | 19 +++++++++++ ...t.args => qemuxml2argv-pcie-root-port-q35.args} | 7 ++-- .../qemuxml2argv-pcie-root-port-q35.xml | 23 +++++++++++++ .../qemuxml2argv-pcie-root-port.xml | 36 -------------------- tests/qemuxml2argvtest.c | 27 ++++++++++++--- ...xml2xmlout-pcie-root-port-mach-virt-generic.xml | 38 ++++++++++++++++++++++ ...xml2xmlout-pcie-root-port-mach-virt-ioh3420.xml | 33 +++++++++++++++++++ ...t.xml => qemuxml2xmlout-pcie-root-port-q35.xml} | 19 +++++------ tests/qemuxml2xmltest.c | 22 ++++++++++--- 21 files changed, 272 insertions(+), 69 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2argvdata/{qemuxml2argv-pcie-root-port.args => qemuxml2argv-pcie-root-port-q35.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-pcie-root-port.xml => qemuxml2xmlout-pcie-root-port-q35.xml} (71%) -- 2.7.4
QEMU 2.9 introduces the pcie-root-port device, which is a generic version of the existing ioh3420 device. Make the new device available to libvirt users. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1408808 --- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 4 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 +++++++++++++++--- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 + 7 files changed, 24 insertions(+), 4 deletions(-) diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5e59328..e4cf990 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1904,6 +1904,7 @@ <value>i82801b11-bridge</value> <!-- implementations of 'pcie-root-port' --> <value>ioh3420</value> + <value>pcie-root-port</value> <!-- implementations of 'pcie-switch-upstream-port' --> <value>x3130-upstream</value> <!-- implementations of 'pcie-switch-downstream-port' --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88d419e..f110c31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "x3130-upstream", "xio3130-downstream", "pxb", - "pxb-pcie") + "pxb-pcie", + "pcie-root-port", +); 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 b788a82..72901ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -694,6 +694,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 70f9ed7..453bc8a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -359,6 +359,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-model-expansion", /* 245 */ "virtio-net.host_mtu", "spice-rendernode", + "pcie-root-port", ); @@ -1619,6 +1620,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN }, { "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL }, { "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI }, + { "pcie-root-port", QEMU_CAPS_DEVICE_PCIE_ROOT_PORT }, }; static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index cc9f46e..79de977 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -394,6 +394,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION, /* qmp query-cpu-model-expansion */ QEMU_CAPS_VIRTIO_NET_HOST_MTU, /* virtio-net-*.host_mtu */ QEMU_CAPS_SPICE_RENDERNODE, /* -spice rendernode */ + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, /* -device pcie-root-port */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b484b7b..7e5f727 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2874,20 +2874,32 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->opts.pciopts.modelName); goto error; } - if (def->opts.pciopts.modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) { + if ((def->opts.pciopts.modelName != + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + (def->opts.pciopts.modelName != + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " "is not valid for a pcie-root-port"), modelName); goto error; } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { + if ((def->opts.pciopts.modelName == + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pcie-root-port (ioh3420) " "controller is not supported in this QEMU binary")); goto error; } + if ((def->opts.pciopts.modelName == + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-root-port (pcie-root-port) " + "controller is not supported in this QEMU binary")); + goto error; + } virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", modelName, def->opts.pciopts.port, diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 334f8e7..a397615 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='drive-iotune-group'/> <flag name='query-cpu-model-expansion'/> <flag name='virtio-net.host_mtu'/> + <flag name='pcie-root-port'/> <version>2008050</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0-1961-g5b10b94bd5)</package> -- 2.7.4
On 03/14/2017 12:58 PM, Andrea Bolognani wrote:
QEMU 2.9 introduces the pcie-root-port device, which is a generic version of the existing ioh3420 device.
Make the new device available to libvirt users.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1408808
Yes, but kind of no. It makes it available, but still difficult and cumbersome to use. I would say it *partially* resolves that BZ, with full resolution coming from the followup patches that make it the default. (I'm only pointing this out because we wouldn't want some distro maintainer to be looking through patches for backport and erroneously believe, based on the commit log, that this was the only patch they needed).
--- docs/schemas/domaincommon.rng | 1 + src/conf/domain_conf.c | 4 +++- src/conf/domain_conf.h | 1 + src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 18 +++++++++++++++--- tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml | 1 +
Lately everyone has gotten in the habit of making a separate patch for 1) caps, 2) config+docs, and 3) functionality in hypervisor, but I'm okay with the old school style of combining them all (I think that the documentation change should be included in this same patch though, or if you split them then the convention seems to be that the docs change is included in the patch that updates the XML). BTW, although you added an entry to your new-fangled "news" file in patch 4, you never added new info to formatdomain.html.in - at least that should be included in this patch (with a small addition to its text when you change the default for aarch64). Other than the missing documentation, ACK.
7 files changed, 24 insertions(+), 4 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index 5e59328..e4cf990 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -1904,6 +1904,7 @@ <value>i82801b11-bridge</value> <!-- implementations of 'pcie-root-port' --> <value>ioh3420</value> + <value>pcie-root-port</value> <!-- implementations of 'pcie-switch-upstream-port' --> <value>x3130-upstream</value> <!-- implementations of 'pcie-switch-downstream-port' --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 88d419e..f110c31 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "x3130-upstream", "xio3130-downstream", "pxb", - "pxb-pcie") + "pxb-pcie", + "pcie-root-port",
Sigh. As this becomes the norm, it's going to make libvirt config look redundant (and is also likely to confuse people about which attribute to change if they want to use iohh3420 instead of the generic one), but there's nothing that can be done about it. (I agree that this was the best choice for device name in qemu btw).
+);
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 b788a82..72901ca 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -694,6 +694,7 @@ typedef enum { VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB, VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PXB_PCIE, + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT,
VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_LAST } virDomainControllerPCIModelName; diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 70f9ed7..453bc8a 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -359,6 +359,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "query-cpu-model-expansion", /* 245 */ "virtio-net.host_mtu", "spice-rendernode", + "pcie-root-port", );
@@ -1619,6 +1620,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "ivshmem-plain", QEMU_CAPS_DEVICE_IVSHMEM_PLAIN }, { "ivshmem-doorbell", QEMU_CAPS_DEVICE_IVSHMEM_DOORBELL }, { "vhost-scsi", QEMU_CAPS_DEVICE_VHOST_SCSI }, + { "pcie-root-port", QEMU_CAPS_DEVICE_PCIE_ROOT_PORT }, };
static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index cc9f46e..79de977 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -394,6 +394,7 @@ typedef enum { QEMU_CAPS_QUERY_CPU_MODEL_EXPANSION, /* qmp query-cpu-model-expansion */ QEMU_CAPS_VIRTIO_NET_HOST_MTU, /* virtio-net-*.host_mtu */ QEMU_CAPS_SPICE_RENDERNODE, /* -spice rendernode */ + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, /* -device pcie-root-port */
QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index b484b7b..7e5f727 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2874,20 +2874,32 @@ qemuBuildControllerDevStr(const virDomainDef *domainDef, def->opts.pciopts.modelName); goto error; } - if (def->opts.pciopts.modelName - != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) { + if ((def->opts.pciopts.modelName != + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + (def->opts.pciopts.modelName != + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("PCI controller model name '%s' " "is not valid for a pcie-root-port"), modelName); goto error; } - if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { + if ((def->opts.pciopts.modelName == + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_IOH3420)) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("the pcie-root-port (ioh3420) " "controller is not supported in this QEMU binary")); goto error; } + if ((def->opts.pciopts.modelName == + VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT) && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("the pcie-root-port (pcie-root-port) " + "controller is not supported in this QEMU binary")); + goto error; + }
virBufferAsprintf(&buf, "%s,port=0x%x,chassis=%d,id=%s", modelName, def->opts.pciopts.port, diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml index 334f8e7..a397615 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.x86_64.xml @@ -202,6 +202,7 @@ <flag name='drive-iotune-group'/> <flag name='query-cpu-model-expansion'/> <flag name='virtio-net.host_mtu'/> + <flag name='pcie-root-port'/> <version>2008050</version> <kvmVersion>0</kvmVersion> <package> (v2.8.0-1961-g5b10b94bd5)</package>
On Wed, 2017-03-15 at 15:18 -0400, Laine Stump wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1408808 Yes, but kind of no. It makes it available, but still difficult and cumbersome to use. I would say it *partially* resolves that BZ, with full resolution coming from the followup patches that make it the default. (I'm only pointing this out because we wouldn't want some distro maintainer to be looking through patches for backport and erroneously believe, based on the commit log, that this was the only patch they needed).
I was debating about this myself. Strictly speaking, the bug report is about adding support for generic PCIe Root Ports, which this patch does. That said, I see your point, and since I don't have any specific objection to moving the Resolves: to the next patch I'll do just that. [...]
BTW, although you added an entry to your new-fangled "news" file in patch 4, you never added new info to formatdomain.html.in - at least that should be included in this patch (with a small addition to its text when you change the default for aarch64).
The existing documentation is fairly vague about PCI controller models: PCI controllers also have an optional subelement <model> with an attribute name. The name attribute holds the name of the specific device that qemu is emulating (e.g. "i82801b11-bridge") rather than simply the class of device ("dmi-to-pci-bridge", "pci-bridge"), which is set in the controller element's model attribute. In almost all cases, you should not manually add a <model> subelement to a controller, nor should you modify one that is automatically generated by libvirt. Since 1.2.19 (QEMU only). Nowhere are the valid models for each PCI controllers listed, and that's fine in my book: the documentation explicitly tells the user that they should let libvirt do its thing in basically all cases. Do you really think we should make that more explicit? [...]
@@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "x3130-upstream", "xio3130-downstream", "pxb", - "pxb-pcie") + "pxb-pcie", + "pcie-root-port", Sigh. As this becomes the norm, it's going to make libvirt config look redundant (and is also likely to confuse people about which attribute to change if they want to use iohh3420 instead of the generic one), but there's nothing that can be done about it. (I agree that this was the best choice for device name in qemu btw).
You just have to change the model, as opposed to, you know... The model. What's so confusing about that? :D -- Andrea Bolognani / Red Hat / Virtualization
On 03/16/2017 10:18 AM, Andrea Bolognani wrote:
On Wed, 2017-03-15 at 15:18 -0400, Laine Stump wrote:
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1408808
Yes, but kind of no. It makes it available, but still difficult and cumbersome to use. I would say it *partially* resolves that BZ, with full resolution coming from the followup patches that make it the default. (I'm only pointing this out because we wouldn't want some distro maintainer to be looking through patches for backport and erroneously believe, based on the commit log, that this was the only patch they needed).
I was debating about this myself.
Strictly speaking, the bug report is about adding support for generic PCIe Root Ports, which this patch does.
That said, I see your point, and since I don't have any specific objection to moving the Resolves: to the next patch I'll do just that.
[...]
BTW, although you added an entry to your new-fangled "news" file in patch 4, you never added new info to formatdomain.html.in - at least that should be included in this patch (with a small addition to its text when you change the default for aarch64).
The existing documentation is fairly vague about PCI controller models:
PCI controllers also have an optional subelement <model> with an attribute name. The name attribute holds the name of the specific device that qemu is emulating (e.g. "i82801b11-bridge") rather than simply the class of device ("dmi-to-pci-bridge", "pci-bridge"), which is set in the controller element's model attribute. In almost all cases, you should not manually add a <model> subelement to a controller, nor should you modify one that is automatically generated by libvirt. Since 1.2.19 (QEMU only).
Nowhere are the valid models for each PCI controllers listed, and that's fine in my book: the documentation explicitly tells the user that they should let libvirt do its thing in basically all cases.
Do you really think we should make that more explicit?
Well, "yes" because everything should be documented, but "no" because once it's documented then people will start playing with it and whine when something breaks because they played with it "in the wrong way". I'm undecided about this, since there are some other cases where we describe some things that users should never have to set and say "if you don't know what this is for, you shouldn't change it" which seems like an adequate warning, but then we get bug reports due to people changing it anyway. On the other hand, when we *don't* document things that show up in the XML, then people start crafting cargo-cult solutions that use those undocumented things in maybe not the right way. (for example, we added in a specifier to allow choosing between vfio and legacy-kvm device assignment when we first added vfio support because someone was concerned that there might be some bug in vfio and they would need a way to fall back to legacy kvm assignment. Very soon after that legacy kvm assignment was completely disabled in the kernel, but as least a couple of times I've seen people trying to explicitly set that when assigning devices, and wondering why their setup doesn't work) So I guess you can push this without the extra documentation, and we can discuss it to death in the meantime :-)
[...]
@@ -338,7 +338,9 @@ VIR_ENUM_IMPL(virDomainControllerPCIModelName, "x3130-upstream", "xio3130-downstream", "pxb", - "pxb-pcie") + "pxb-pcie", + "pcie-root-port",
Sigh. As this becomes the norm, it's going to make libvirt config look redundant (and is also likely to confuse people about which attribute to change if they want to use iohh3420 instead of the generic one), but there's nothing that can be done about it. (I agree that this was the best choice for device name in qemu btw).
You just have to change the model, as opposed to, you know... The model. What's so confusing about that? :D
-- Andrea Bolognani / Red Hat / Virtualization
ioh3420 is emulated Intel hardware, so it always looked quite out of place in aarch64/virt guests. If pcie-root-port is available in QEMU, use that device instead. --- It was mentioned somewhere, at some point, that we might want to switch to generic PCIe Root Ports for x86/q35 guests as well. It sounds like a good idea to me, and in fact QEMU's sample configuration files for x86/q35 (docs/q35-virtio-*.cfg) already push in that direction; however, I didn't want to risk holding up the aarch64/virt change, which we unquestionably want, because of that, so I will propose it as a separate patch further down the line. src/qemu/qemu_domain_address.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 64aa4ef..bd2f718 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1846,13 +1846,16 @@ qemuDomainSupportsPCI(virDomainDefPtr def, static void -qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) +qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { int *modelName = &cont->opts.pciopts.modelName; /* make sure it's not already set */ if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) return; + switch ((virDomainControllerModelPCI)cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; @@ -1861,7 +1864,13 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; + /* Use generic PCIe Root Ports for mach-virt guests, if available */ + if (qemuDomainMachineIsVirt(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT; + } else { + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; + } break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; @@ -2143,7 +2152,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * device in qemu) for any controller that doesn't yet * have it set. */ - qemuDomainPCIControllerSetDefaultModelName(cont); + qemuDomainPCIControllerSetDefaultModelName(cont, def, qemuCaps); /* set defaults for any other auto-generated config * options for this controller that haven't been -- 2.7.4
On 03/14/2017 12:58 PM, Andrea Bolognani wrote:
ioh3420 is emulated Intel hardware, so it always looked quite out of place in aarch64/virt guests.
If pcie-root-port is available in QEMU, use that device instead. --- It was mentioned somewhere, at some point, that we might want to switch to generic PCIe Root Ports for x86/q35 guests as well.
It sounds like a good idea to me, and in fact QEMU's sample configuration files for x86/q35 (docs/q35-virtio-*.cfg) already push in that direction; however, I didn't want to risk holding up the aarch64/virt change, which we unquestionably want, because of that, so I will propose it as a separate patch further down the line.
Personally I'm fine with making it the default for new root ports on any machinetype that has it in the capabilities - if anything doesn't work, it's a bug and needs to be fixed, and we can't find the things that don't work if people aren't using it. In other words "the sooner the better". (Aside from that is the fact that the generic root port is going to have IO space turned off by default, and we should all be switching to that as soon as possible). ACK to this patch as it is, and also ACK to this patch with the "MachineIsVirt()" qualifier removed (or to a separate patch that removes it).
src/qemu/qemu_domain_address.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 64aa4ef..bd2f718 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1846,13 +1846,16 @@ qemuDomainSupportsPCI(virDomainDefPtr def,
static void -qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) +qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont, + virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) { int *modelName = &cont->opts.pciopts.modelName;
/* make sure it's not already set */ if (*modelName != VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_NONE) return; + switch ((virDomainControllerModelPCI)cont->model) { case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCI_BRIDGE; @@ -1861,7 +1864,13 @@ qemuDomainPCIControllerSetDefaultModelName(virDomainControllerDefPtr cont) *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_I82801B11_BRIDGE; break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: - *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; + /* Use generic PCIe Root Ports for mach-virt guests, if available */ + if (qemuDomainMachineIsVirt(def) && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCIE_ROOT_PORT)) { + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT; + } else { + *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_IOH3420; + } break; case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: *modelName = VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM; @@ -2143,7 +2152,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, * device in qemu) for any controller that doesn't yet * have it set. */ - qemuDomainPCIControllerSetDefaultModelName(cont); + qemuDomainPCIControllerSetDefaultModelName(cont, def, qemuCaps);
/* set defaults for any other auto-generated config * options for this controller that haven't been
On Wed, 2017-03-15 at 15:22 -0400, Laine Stump wrote:
It was mentioned somewhere, at some point, that we might want to switch to generic PCIe Root Ports for x86/q35 guests as well. It sounds like a good idea to me, and in fact QEMU's sample configuration files for x86/q35 (docs/q35-virtio-*.cfg) already push in that direction; however, I didn't want to risk holding up the aarch64/virt change, which we unquestionably want, because of that, so I will propose it as a separate patch further down the line. Personally I'm fine with making it the default for new root ports on any machinetype that has it in the capabilities - if anything doesn't work, it's a bug and needs to be fixed, and we can't find the things that don't work if people aren't using it. In other words "the sooner the better". (Aside from that is the fact that the generic root port is going to have IO space turned off by default, and we should all be switching to that as soon as possible).
I agree with you, let's just change the default for all architectures and machine types in one fell swoop. -- Andrea Bolognani / Red Hat / Virtualization
We want pcie-root-ports to be used for aarch64/virt guests when available in QEMU, but at the same time we need to ensure that other machine type and hosts where QEMU releases lacking the new device type are not affected. --- ...uxml2argv-pcie-root-port-mach-virt-generic.args | 22 +++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-generic.xml | 22 +++++++++++++ ...uxml2argv-pcie-root-port-mach-virt-ioh3420.args | 21 ++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-ioh3420.xml | 19 +++++++++++ ...t.args => qemuxml2argv-pcie-root-port-q35.args} | 7 ++-- .../qemuxml2argv-pcie-root-port-q35.xml | 23 +++++++++++++ .../qemuxml2argv-pcie-root-port.xml | 36 -------------------- tests/qemuxml2argvtest.c | 27 ++++++++++++--- ...xml2xmlout-pcie-root-port-mach-virt-generic.xml | 38 ++++++++++++++++++++++ ...xml2xmlout-pcie-root-port-mach-virt-ioh3420.xml | 33 +++++++++++++++++++ ...t.xml => qemuxml2xmlout-pcie-root-port-q35.xml} | 19 +++++------ tests/qemuxml2xmltest.c | 22 ++++++++++--- 12 files changed, 227 insertions(+), 62 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2argvdata/{qemuxml2argv-pcie-root-port.args => qemuxml2argv-pcie-root-port-q35.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-pcie-root-port.xml => qemuxml2xmlout-pcie-root-port-q35.xml} (71%) diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args new file mode 100644 index 0000000..b0ae8b2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name mach-virt-test \ +-S \ +-M virt \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-mach-virt-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ +-device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml new file mode 100644 index 0000000..950397a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args new file mode 100644 index 0000000..a57cdfd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name mach-virt-test \ +-S \ +-M virt \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-mach-virt-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml new file mode 100644 index 0000000..897547b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args similarity index 59% rename from tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args rename to tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args index 4e852ff..2e9d8da 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args @@ -18,8 +18,5 @@ QEMU_AUDIO_DRV=none \ -boot c \ -device ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ addr=0x2 \ --device ioh3420,port=0x1a,chassis=40,id=pci.2,bus=pcie.0,addr=0x2.0x1 \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-sata0-0-0 \ --device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ --device qxl-vga,id=video0,ram_size=67108864,vram_size=33554432,bus=pcie.0,\ -addr=0x1 +-device ioh3420,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1 \ +-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml new file mode 100644 index 0000000..1102919 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.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'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + </controller> + <controller type='usb' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml deleted file mode 100644 index 7ecc4a6..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml +++ /dev/null @@ -1,36 +0,0 @@ -<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> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='sata'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='pcie-root-port'/> - <controller type='pci' index='2' model='pcie-root-port'> - <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> - </controller> - <controller type='sata' index='0'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <video> - <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> - </video> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6bd7465..db4da95 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2011,12 +2011,15 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_USB_REDIR); - DO_TEST("pcie-root-port", + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */ + DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420, - QEMU_CAPS_ICH9_AHCI, - QEMU_CAPS_PCI_MULTIFUNCTION, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY, - QEMU_CAPS_DEVICE_QXL); + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("autoindex", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, @@ -2294,6 +2297,20 @@ mymain(void) DO_TEST_FAILURE("aarch64-kvm-32-on-64", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_KVM); + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */ + DO_TEST("pcie-root-port-mach-virt-generic", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-ioh3420", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_PCI_MULTIFUNCTION); + qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE); DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml new file mode 100644 index 0000000..0eabafd --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <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='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml new file mode 100644 index 0000000..6c80d2a --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <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='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml similarity index 71% rename from tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml rename to tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml index 5775eb9..0e6bb98 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml @@ -3,7 +3,7 @@ <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> + <vcpu placement='static'>2</vcpu> <os> <type arch='x86_64' machine='q35'>hvm</type> <boot dev='hd'/> @@ -14,11 +14,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='sata'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='pcie-root-port'> <model name='ioh3420'/> @@ -27,18 +22,20 @@ </controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> + <target chassis='2' port='0x11'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/> </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0x12'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/> + </controller> + <controller type='usb' index='0' model='none'/> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <video> - <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1' primary='yes'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> - </video> <memballoon model='none'/> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4353ad2..950d1c5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -884,12 +884,24 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); - DO_TEST("pcie-root-port", + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */ + DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420, - QEMU_CAPS_ICH9_AHCI, - QEMU_CAPS_PCI_MULTIFUNCTION, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY, - QEMU_CAPS_DEVICE_QXL); + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-generic", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-ioh3420", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-switch-upstream-port", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_X3130_UPSTREAM, -- 2.7.4
On 03/14/2017 12:58 PM, Andrea Bolognani wrote:
We want pcie-root-ports to be used for aarch64/virt guests when available in QEMU, but at the same time we need to ensure that other machine type and hosts where QEMU releases lacking the new device type are not affected. --- ...uxml2argv-pcie-root-port-mach-virt-generic.args | 22 +++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-generic.xml | 22 +++++++++++++ ...uxml2argv-pcie-root-port-mach-virt-ioh3420.args | 21 ++++++++++++ ...muxml2argv-pcie-root-port-mach-virt-ioh3420.xml | 19 +++++++++++ ...t.args => qemuxml2argv-pcie-root-port-q35.args} | 7 ++-- .../qemuxml2argv-pcie-root-port-q35.xml | 23 +++++++++++++ .../qemuxml2argv-pcie-root-port.xml | 36 -------------------- tests/qemuxml2argvtest.c | 27 ++++++++++++--- ...xml2xmlout-pcie-root-port-mach-virt-generic.xml | 38 ++++++++++++++++++++++ ...xml2xmlout-pcie-root-port-mach-virt-ioh3420.xml | 33 +++++++++++++++++++ ...t.xml => qemuxml2xmlout-pcie-root-port-q35.xml} | 19 +++++------ tests/qemuxml2xmltest.c | 22 ++++++++++--- 12 files changed, 227 insertions(+), 62 deletions(-) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2argvdata/{qemuxml2argv-pcie-root-port.args => qemuxml2argv-pcie-root-port-q35.args} (59%) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml delete mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml rename tests/qemuxml2xmloutdata/{qemuxml2xmlout-pcie-root-port.xml => qemuxml2xmlout-pcie-root-port-q35.xml} (71%)
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args new file mode 100644 index 0000000..b0ae8b2 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.args @@ -0,0 +1,22 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name mach-virt-test \ +-S \ +-M virt \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-mach-virt-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device pcie-root-port,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 \ +-device pcie-root-port,port=0xa,chassis=3,id=pci.3,bus=pcie.0,addr=0x1.0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml new file mode 100644 index 0000000..950397a --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-generic.xml @@ -0,0 +1,22 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args new file mode 100644 index 0000000..a57cdfd --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.args @@ -0,0 +1,21 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/libexec/qemu-kvm \ +-name mach-virt-test \ +-S \ +-M virt \ +-m 2048 \ +-smp 2,sockets=2,cores=1,threads=1 \ +-uuid 11dbdcdd-4c3b-482b-8903-9bdb8c0a2774 \ +-nographic \ +-nodefaults \ +-monitor unix:/tmp/lib/domain--1-mach-virt-test/monitor.sock,server,nowait \ +-no-acpi \ +-boot c \ +-device ioh3420,port=0x8,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ +addr=0x1 \ +-device ioh3420,port=0x9,chassis=2,id=pci.2,bus=pcie.0,addr=0x1.0x1 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml new file mode 100644 index 0000000..897547b --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-mach-virt-ioh3420.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args similarity index 59% rename from tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args rename to tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args index 4e852ff..2e9d8da 100644 --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.args +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.args @@ -18,8 +18,5 @@ QEMU_AUDIO_DRV=none \ -boot c \ -device ioh3420,port=0x10,chassis=1,id=pci.1,bus=pcie.0,multifunction=on,\ addr=0x2 \ --device ioh3420,port=0x1a,chassis=40,id=pci.2,bus=pcie.0,addr=0x2.0x1 \ --drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-sata0-0-0 \ --device ide-drive,bus=ide.0,drive=drive-sata0-0-0,id=sata0-0-0 \ --device qxl-vga,id=video0,ram_size=67108864,vram_size=33554432,bus=pcie.0,\ -addr=0x1 +-device ioh3420,port=0x11,chassis=2,id=pci.2,bus=pcie.0,addr=0x2.0x1 \ +-device pcie-root-port,port=0x12,chassis=3,id=pci.3,bus=pcie.0,addr=0x2.0x2 diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.xml new file mode 100644 index 0000000..1102919 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port-q35.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'>2</vcpu> + <os> + <type arch='x86_64' machine='q35'>hvm</type> + </os> + <devices> + <emulator>/usr/libexec/qemu-kvm</emulator> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-root-port'/> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + </controller> + <controller type='usb' model='none'/> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml b/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml deleted file mode 100644 index 7ecc4a6..0000000 --- a/tests/qemuxml2argvdata/qemuxml2argv-pcie-root-port.xml +++ /dev/null @@ -1,36 +0,0 @@ -<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> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='sata'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> - <controller type='pci' index='0' model='pcie-root'/> - <controller type='pci' index='1' model='pcie-root-port'/> - <controller type='pci' index='2' model='pcie-root-port'> - <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> - </controller> - <controller type='sata' index='0'/> - <input type='mouse' bus='ps2'/> - <input type='keyboard' bus='ps2'/> - <video> - <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1'/> - </video> - <memballoon model='none'/> - </devices> -</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 6bd7465..db4da95 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -2011,12 +2011,15 @@ mymain(void) QEMU_CAPS_DEVICE_QXL, QEMU_CAPS_HDA_DUPLEX, QEMU_CAPS_USB_REDIR); - DO_TEST("pcie-root-port", + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */ + DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420, - QEMU_CAPS_ICH9_AHCI, - QEMU_CAPS_PCI_MULTIFUNCTION, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY, - QEMU_CAPS_DEVICE_QXL); + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("autoindex", QEMU_CAPS_DEVICE_PCI_BRIDGE, QEMU_CAPS_DEVICE_DMI_TO_PCI_BRIDGE, @@ -2294,6 +2297,20 @@ mymain(void) DO_TEST_FAILURE("aarch64-kvm-32-on-64", QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DEVICE_VIRTIO_MMIO, QEMU_CAPS_KVM); + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */ + DO_TEST("pcie-root-port-mach-virt-generic", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-ioh3420", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_PCI_MULTIFUNCTION); + qemuTestSetHostArch(driver.caps, VIR_ARCH_NONE);
DO_TEST("kvm-pit-delay", QEMU_CAPS_KVM_PIT_TICK_POLICY); diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml new file mode 100644 index 0000000..0eabafd --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-generic.xml @@ -0,0 +1,38 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <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='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0xa'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml new file mode 100644 index 0000000..6c80d2a --- /dev/null +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-mach-virt-ioh3420.xml @@ -0,0 +1,33 @@ +<domain type='qemu'> + <name>mach-virt-test</name> + <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>2097152</currentMemory> + <vcpu placement='static'>2</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <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='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='1' port='0x8'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/> + </controller> + <controller type='pci' index='2' model='pcie-root-port'> + <model name='ioh3420'/> + <target chassis='2' port='0x9'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/> + </controller> + <memballoon model='none'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml similarity index 71% rename from tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml rename to tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml index 5775eb9..0e6bb98 100644 --- a/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port.xml +++ b/tests/qemuxml2xmloutdata/qemuxml2xmlout-pcie-root-port-q35.xml @@ -3,7 +3,7 @@ <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> + <vcpu placement='static'>2</vcpu>
You made some other changes to the input XML beyond just the differences in root ports. Mostly they're innocuous and easy to verify, but...
<os> <type arch='x86_64' machine='q35'>hvm</type> <boot dev='hd'/> @@ -14,11 +14,6 @@ <on_crash>destroy</on_crash> <devices> <emulator>/usr/libexec/qemu-kvm</emulator> - <disk type='block' device='disk'> - <source dev='/dev/HostVG/QEMUGuest1'/> - <target dev='sda' bus='sata'/> - <address type='drive' controller='0' bus='0' target='0' unit='0'/> - </disk> <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='pcie-root-port'> <model name='ioh3420'/> @@ -27,18 +22,20 @@ </controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> + <target chassis='2' port='0x11'/>
...you removed the <target chassis='40' port='0x1a'/> from the input file, but that was there for a reason - it was in the test to assure that non-default values specified for chassis and port would be honored. Please put that back in.
<address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x1'/> </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='3' port='0x12'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x2'/> + </controller> + <controller type='usb' index='0' model='none'/> <controller type='sata' index='0'> <address type='pci' domain='0x0000' bus='0x00' slot='0x1f' function='0x2'/> </controller> <input type='mouse' bus='ps2'/> <input type='keyboard' bus='ps2'/> - <video> - <model type='qxl' ram='65536' vram='32768' vgamem='8192' heads='1' primary='yes'/> - <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> - </video> <memballoon model='none'/> </devices> </domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index 4353ad2..950d1c5 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -884,12 +884,24 @@ mymain(void) QEMU_CAPS_ICH9_AHCI, QEMU_CAPS_DEVICE_VIDEO_PRIMARY, QEMU_CAPS_DEVICE_QXL); - DO_TEST("pcie-root-port", + + /* Make sure the default model for PCIe Root Ports is picked correctly + * according to architecture, machine type and binary capabilities; also + * make sure that the user can override the default */
+1 for putting comments here to give future me (and you) a clue about what these tests are testing for, so that maybe they won't be unintentionally invalidated.
+ DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420,
If we were going to continue using ioh3420 for Q35, I would suggest that you should add QEMU_CAPS_DEVICE_PCIE_ROOT_PORT here to verify that the output still uses ioh3420. But as I said earlier I think we should switch Q35 to using the generic root port too, so.... you *still* should add that CAP, and change the expected output (and add a separate "...-q35-old" test that doesn't have the cap for the generic root port)
- QEMU_CAPS_ICH9_AHCI, - QEMU_CAPS_PCI_MULTIFUNCTION, - QEMU_CAPS_DEVICE_VIDEO_PRIMARY, - QEMU_CAPS_DEVICE_QXL); + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-generic", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_DEVICE_PCIE_ROOT_PORT, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-root-port-mach-virt-ioh3420", + QEMU_CAPS_OBJECT_GPEX, + QEMU_CAPS_DEVICE_IOH3420, + QEMU_CAPS_PCI_MULTIFUNCTION); + DO_TEST("pcie-switch-upstream-port", QEMU_CAPS_DEVICE_IOH3420, QEMU_CAPS_DEVICE_X3130_UPSTREAM,
ACK with the <target ...> you removed from the -q35 test added back in.
On Wed, 2017-03-15 at 17:49 -0400, Laine Stump wrote: [...]
@@ -3,7 +3,7 @@ <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> + <vcpu placement='static'>2</vcpu> You made some other changes to the input XML beyond just the differences in root ports. Mostly they're innocuous and easy to verify, but...
[...]
</controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> + <target chassis='2' port='0x11'/> ...you removed the <target chassis='40' port='0x1a'/> from the input file, but that was there for a reason - it was in the test to assure that non-default values specified for chassis and port would be honored. Please put that back in.
I'll just leave it alone and add a comment about its purpose instead. Possibly change the name so it reflects the idea behind the test a little bit better.
+ DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420, If we were going to continue using ioh3420 for Q35, I would suggest that you should add QEMU_CAPS_DEVICE_PCIE_ROOT_PORT here to verify that the output still uses ioh3420.
It's there, exactly for that purpose ;)
But as I said earlier I think we should switch Q35 to using the generic root port too, so.... you *still* should add that CAP, and change the expected output (and add a separate "...-q35-old" test that doesn't have the cap for the generic root port)
Yeah, I'll have just two new tests, one where the capability for the new controller is present and one where it's not. -- Andrea Bolognani / Red Hat / Virtualization
On 03/16/2017 10:37 AM, Andrea Bolognani wrote:
On Wed, 2017-03-15 at 17:49 -0400, Laine Stump wrote: [...]
@@ -3,7 +3,7 @@ <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> + <vcpu placement='static'>2</vcpu>
You made some other changes to the input XML beyond just the differences in root ports. Mostly they're innocuous and easy to verify, but...
[...]
</controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='ioh3420'/> - <target chassis='40' port='0x1a'/> + <target chassis='2' port='0x11'/>
...you removed the <target chassis='40' port='0x1a'/> from the input file, but that was there for a reason - it was in the test to assure that non-default values specified for chassis and port would be honored. Please put that back in.
I'll just leave it alone and add a comment about its purpose instead. Possibly change the name so it reflects the idea behind the test a little bit better.
It was the original test added when we added support for pcie-root-port, and the idea behind the test was to test automatic and manual setting of every associated attribute (i.e. it's not a special purpose test intended just to test manual setting of the attributes in <target>, if that's what you're implying.
+ DO_TEST("pcie-root-port-q35", QEMU_CAPS_DEVICE_IOH3420,
If we were going to continue using ioh3420 for Q35, I would suggest that you should add QEMU_CAPS_DEVICE_PCIE_ROOT_PORT here to verify that the output still uses ioh3420.
It's there, exactly for that purpose ;)
Yeah, I don't know how I missed that.
But as I said earlier I think we should switch Q35 to using the generic root port too, so.... you *still* should add that CAP, and change the expected output (and add a separate "...-q35-old" test that doesn't have the cap for the generic root port)
Yeah, I'll have just two new tests, one where the capability for the new controller is present and one where it's not.
-- Andrea Bolognani / Red Hat / Virtualization
--- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 04783aa..434a9f7 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -17,6 +17,15 @@ The virt-host-validate tool now supports bhyve hypervisor </summary> </change> + <change> + <summary> + qemu: Introduce support for generic PCIe Root Ports + </summary> + <description> + This new PCI controller model will be used by default for + aarch64/virt guests if the QEMU binary supports it. + </description> + </change> </section> <section title="Improvements"> <change> -- 2.7.4
On 03/14/2017 12:58 PM, Andrea Bolognani wrote:
--- docs/news.xml | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/docs/news.xml b/docs/news.xml index 04783aa..434a9f7 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -17,6 +17,15 @@ The virt-host-validate tool now supports bhyve hypervisor </summary> </change> + <change> + <summary> + qemu: Introduce support for generic PCIe Root Ports + </summary> + <description> + This new PCI controller model will be used by default for + aarch64/virt guests if the QEMU binary supports it.
Maybe name it explicitly: qemu: Introduce support for the non-vendor-specific device "pcie-root-port" This new PCI controller model will be used by default for new pcie-root-port controllers if the QEMU binary supports it. But whatever - ACK.
+ </description> + </change> </section> <section title="Improvements"> <change>
On Wed, 2017-03-15 at 17:57 -0400, Laine Stump wrote: [...]
@@ -17,6 +17,15 @@ The virt-host-validate tool now supports bhyve hypervisor </summary> </change> + <change> + <summary> + qemu: Introduce support for generic PCIe Root Ports + </summary> + <description> + This new PCI controller model will be used by default for + aarch64/virt guests if the QEMU binary supports it. Maybe name it explicitly: qemu: Introduce support for the non-vendor-specific device "pcie-root-port"
As noted when commenting on patch 1, I think it's okay to leave this vague since users are generally not supposed to have to think about this at all. -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani -
Laine Stump