On Tue, Sep 15, 2026 at 13:46:18 +0000, David Vrabel wrote:
Add speed/width target attributes for the generic PCIe root and switch ports to set the link speed/width capability.
This useful when used with passthrough devices to make the in-guest apparent bandwidth match the hardware bandwidth. This allows in-guest software (such as NCCL) to make better decisions about whether to use P2P transfers etc.
Signed-off-by: David Vrabel <david.vrabel@nutanix.com> --- docs/formatdomain.rst | 12 ++ src/conf/domain_conf.c | 22 ++++ src/conf/domain_conf.h | 3 + src/conf/schemas/domaincommon.rng | 10 ++ src/qemu/qemu_capabilities.c | 114 +++++++++--------- src/qemu/qemu_capabilities.h | 112 ++++++++--------- src/qemu/qemu_command.c | 46 +++++++ src/qemu/qemu_validate.c | 81 +++++++++++++ .../caps_11.1.0_x86_64.replies | 4 +- .../caps_11.1.0_x86_64.xml | 2 + ...e-root-port-speed-width.x86_64-latest.args | 37 ++++++ ...ie-root-port-speed-width.x86_64-latest.xml | 48 ++++++++ .../pcie-root-port-speed-width.xml | 23 ++++ ...ream-port-model-generic.x86_64-latest.args | 2 +- ...tream-port-model-generic.x86_64-latest.xml | 2 +- ...e-switch-downstream-port-model-generic.xml | 2 +- ...ream-port-model-generic.x86_64-latest.args | 2 +- ...tream-port-model-generic.x86_64-latest.xml | 1 + ...cie-switch-upstream-port-model-generic.xml | 1 + tests/qemuxmlconftest.c | 1 + 20 files changed, 411 insertions(+), 114 deletions(-) create mode 100644 tests/qemuxmlconfdata/pcie-root-port-speed-width.x86_64-latest.args create mode 100644 tests/qemuxmlconfdata/pcie-root-port-speed-width.x86_64-latest.xml create mode 100644 tests/qemuxmlconfdata/pcie-root-port-speed-width.xml
[...]
@@ -9233,6 +9237,20 @@ virDomainControllerDefParseXML(virDomainXMLOption *xmlopt, def->opts.pciopts.port) < 0) return NULL;
+ speed = virXMLPropString(targetNodes[0], "speed"); + if (speed && + (virStrToDouble(speed, NULL, &def->opts.pciopts.speed) < 0 ||
Double? That's a bad idea ...
+ def->opts.pciopts.speed <= 0)) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Invalid non-positive value for PCI controller link speed")); + return NULL; + }
[...]
static struct virQEMUCapsDevicePropsFlags virQEMUCapsDevicePropsPCIeRootPort[] = { { "hotplug", QEMU_CAPS_PCIE_ROOT_PORT_HOTPLUG, NULL }, + { "speed", QEMU_CAPS_PCIE_ROOT_PORT_SPEED, NULL }, + { "width", QEMU_CAPS_PCIE_ROOT_PORT_WIDTH, NULL }, };
[...]
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index bcddb9c922..e3e4fe2538 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -182,6 +182,26 @@ qemuOnOffAuto(virTristateSwitch s) }
+/* Map domain GT/s values to QEMU PCIELinkSpeed enum strings. */ +static const char * +qemuPCIELinkSpeedString(double speed) +{ + if (speed == 2.5) + return "2_5"; + if (speed == 5.0) + return "5"; + if (speed == 8.0) + return "8"; + if (speed == 16.0) + return "16"; + if (speed == 32.0) + return "32"; + if (speed == 64.0) + return "64";
Comparing floating point values like this is a bad idea. It may work here but can break if the mantissa doesn't round properly. This clearly shows that you need an enum here.
+ return NULL; +} + + static int qemuBuildObjectCommandlineFromJSON(virCommand *cmd, virJSONValue *props) @@ -2790,6 +2810,32 @@ qemuBuildControllerPCIDevProps(virDomainControllerDef *def, return -1; }
+ /* Options common to all PCIe ports. */ + if (def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT || + def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT || + def->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT) { + if (pciopts->speed > 0) { + const char *speedstr = qemuPCIELinkSpeedString(pciopts->speed); + + if (!speedstr) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unexpected PCI controller link speed %1$g"), + pciopts->speed); + return -1; + } + + if (virJSONValueObjectAdd(&props, "S:speed", speedstr, NULL) < 0) + return -1; + } + + if (pciopts->width > 0) { + g_autofree char *widthstr = g_strdup_printf("%d", pciopts->width); + + if (virJSONValueObjectAdd(&props, "s:width", widthstr, NULL) < 0)
virJSONValueObjectAdd has formatters for numbers. Do not convert it to string but use the appropriate converter. They can even omit the argument if it's 0.
+ return -1; + } + } + *devprops = g_steal_pointer(&props); return 0; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index 16beb3ca6a..562d6117af 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -4762,6 +4762,87 @@ qemuValidateDomainDeviceDefControllerPCI(const virDomainControllerDef *cont, return -1; }
+ /* speed and width */ + switch ((virDomainControllerModelPCI) cont->model) { + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_UPSTREAM_PORT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_SWITCH_DOWNSTREAM_PORT: + if (pciopts->speed != -1 && + (pciopts->speed != 2.5 && + pciopts->speed != 5.0 && + pciopts->speed != 8.0 && + pciopts->speed != 16.0 && + pciopts->speed != 32.0 && + pciopts->speed != 64.0)) { + virReportControllerInvalidValue(cont, model, modelName, "speed"); + return -1; + } + if (pciopts->speed != -1) { + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM || + pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("setting the '%1$s' property on a '%2$s' device is not supported"), + "speed", modelName);
This breaks translatability rules
+ return -1; + } + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIE_ROOT_PORT_SPEED)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("setting the '%1$s' property on a '%2$s' device is not supported by this QEMU binary"), + "speed", modelName); + return -1; + } + } + + if (pciopts->width != -1 && + pciopts->width != 1 && + pciopts->width != 2 && + pciopts->width != 4 && + pciopts->width != 8 && + pciopts->width != 16 && + pciopts->width != 32) { + virReportControllerInvalidValue(cont, model, modelName, "width"); + return -1; + } + if (pciopts->width != -1) { + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_X3130_UPSTREAM || + pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_XIO3130_DOWNSTREAM) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("setting the '%1$s' property on a '%2$s' device is not supported"), + "width", modelName); + return -1; + } + if (pciopts->modelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_PCIE_ROOT_PORT && + !virQEMUCapsGet(qemuCaps, QEMU_CAPS_PCIE_ROOT_PORT_WIDTH)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("setting the '%1$s' property on a '%2$s' device is not supported by this QEMU binary"), + "width", modelName); + return -1; + } + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_DMI_TO_PCI_BRIDGE: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_EXPANDER_BUS: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCIE_TO_PCI_BRIDGE: + if (pciopts->speed != -1) { + virReportControllerInvalidOption(cont, model, modelName, "speed"); + return -1; + } + if (pciopts->width != -1) { + virReportControllerInvalidOption(cont, model, modelName, "width"); + return -1; + } + break; + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_DEFAULT: + case VIR_DOMAIN_CONTROLLER_MODEL_PCI_LAST: + default: + virReportEnumRangeError(virDomainControllerModelPCI, cont->model); + } + /* hotplug */ if (pciopts->hotplug != VIR_TRISTATE_SWITCH_ABSENT) { switch ((virDomainControllerModelPCI) cont->model) { diff --git a/tests/qemucapabilitiesdata/caps_11.1.0_x86_64.replies b/tests/qemucapabilitiesdata/caps_11.1.0_x86_64.replies index bead7636b1..84ddf0e6dd 100644 --- a/tests/qemucapabilitiesdata/caps_11.1.0_x86_64.replies +++ b/tests/qemucapabilitiesdata/caps_11.1.0_x86_64.replies @@ -32371,7 +32371,7 @@ }, { "default-value": "32", - "name": "x-width", + "name": "width", "description": "1/2/4/8/12/16/32", "type": "PCIELinkWidth" }, @@ -32387,7 +32387,7 @@ }, { "default-value": "16", - "name": "x-speed", + "name": "speed",
NACK. This dump was obtained from a real qemu. You can't change this. Even in current upstream qemu this is still unstable (x-prefixed). Until that changes this patch can't be accepted.
"description": "2_5/5/8/16/32/64", "type": "PCIELinkSpeed" },