[PATCH v2 0/3] qemu: Implement support for multiple SMMUv3s
Hi, This is a follow up to the first patchset [0] for supporting multiple vSMMU instances. This patchset implements support for specifying multiple <iommu> devices within the VM definition when smmuv3Dev IOMMU model is specified, and is tested with Shameer's recently accepted qemu series for user-creatable vSMMU devices [1] For instance, specifying the associated emulated device in a VM definition with multiple IOMMUs, configured to be routed to pcie-expander-bus or pcie-root controllers: <devices> <controller type='pci' index='0' model='pcie-root'/> <controller type='pci' index='1' model='pcie-expander-bus'> <model name='pxb-pcie'/> <target busNr='252'/> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> </controller> <controller type='pci' index='2' model='pcie-root-port'> <model name='pcie-root-port'/> <target chassis='2' port='0x0'/> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </controller> ... <rng model='virtio'> <backend model='random'>/dev/urandom</backend> <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> </rng> <rng model='virtio'> <backend model='random'>/dev/urandom</backend> <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/> </rng> <iommu model='smmuv3Dev'> <driver parentIdx='0'/> </iommu> <iommu model='smmuv3Dev'> <driver parentIdx='1'/> </iommu> </devices> This would get translated to a qemu command line with the arguments below. -device '{"driver":"pxb-pcie","bus_nr":252,"id":"pci.1","bus":"pcie.0","addr":"0x1"}' \ -device '{"driver":"pcie-root-port","port":0,"chassis":2,"id":"pci.2","bus":"pci.1","addr":"0x0"}' \ -device '{"driver":"arm-smmuv3","primary-bus":"pcie.0","id":"smmuv3.0"}' \ -device '{"driver":"arm-smmuv3","primary-bus":"pci.1","id":"smmuv3.1"}' \ ... -object '{"qom-type":"rng-random","id":"objrng0","filename":"/dev/urandom"}' \ -device '{"driver":"virtio-rng-pci","rng":"objrng0","id":"rng0","bus":"pci.2","addr":"0x0"}' \ -object '{"qom-type":"rng-random","id":"objrng1","filename":"/dev/urandom"}' \ -device '{"driver":"virtio-rng-pci","rng":"objrng1","id":"rng1","bus":"pcie.0","addr":"0x4"}' \ Changes from v1: - Moved parentIdx attribute under <driver> - Revised qemuxmlconfdata test cases to attach emulated devices This series is on Github: https://github.com/NathanChenNVIDIA/libvirt/tree/smmuv3Dev-no-accel-09-29-25 Thanks, Nathan [0] https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/3PYS3... [1] https://lore.kernel.org/qemu-devel/20250829082543.7680-1-skolothumtho@nvidia... Signed-off-by: Nathan Chen <nathanc@nvidia.com> Nathan Chen (4): qemu: add IOMMU model smmuv3Dev conf: Support multiple smmuv3Dev IOMMU devices tests: qemuxmlconfdata: provide smmuv3Dev sample XML and CLI args docs/formatdomain.rst | 9 +- src/conf/domain_conf.c | 101 +++++++-- src/conf/domain_conf.h | 11 +- src/conf/domain_validate.c | 58 +++-- src/conf/schemas/domaincommon.rng | 10 +- src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 15 +- src/qemu/qemu_command.c | 202 ++++++++++++------ src/qemu/qemu_domain_address.c | 33 +-- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_postparse.c | 11 +- src/qemu/qemu_validate.c | 18 +- .../iommu-smmuv3Dev.aarch64-latest.args | 41 ++++ .../iommu-smmuv3Dev.aarch64-latest.xml | 62 ++++++ tests/qemuxmlconfdata/iommu-smmuv3Dev.xml | 49 +++++ tests/qemuxmlconftest.c | 1 + 16 files changed, 499 insertions(+), 132 deletions(-) create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.xml -- 2.43.0
Introduce support for "smmuv3Dev" IOMMU model and its "parentIdx" driver attribute. The "parentIdx" attribute indicates the index of the controller that a smmuv3Dev IOMMU device is attached to. Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- docs/formatdomain.rst | 9 +++- src/conf/domain_conf.c | 17 ++++++++ src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 26 +++++++++-- src/conf/schemas/domaincommon.rng | 6 +++ src/qemu/qemu_command.c | 72 +++++++++++++++++++++++++++++-- src/qemu/qemu_domain_address.c | 2 + src/qemu/qemu_validate.c | 16 +++++++ 8 files changed, 141 insertions(+), 9 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index f50dce477f..6a62291600 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -9161,8 +9161,9 @@ Example: ``model`` Supported values are ``intel`` (for Q35 guests) ``smmuv3`` (:since:`since 5.5.0`, for ARM virt guests), ``virtio`` - (:since:`since 8.3.0`, for Q35 and ARM virt guests) and - ``amd`` (:since:`since 11.5.0`). + (:since:`since 8.3.0`, for Q35 and ARM virt guests), + ``amd`` (:since:`since 11.5.0`), and ``smmuv3Dev`` (for + ARM virt guests). ``driver`` The ``driver`` subelement can be used to configure additional options, some @@ -9212,6 +9213,10 @@ Example: Enable x2APIC mode. Useful for higher number of guest CPUs. :since:`Since 11.5.0` (QEMU/KVM and ``amd`` model only) + ``parentIdx`` + The ``parentIdx`` attribute notes the index of the controller that an + IOMMU device is attached to. (QEMU/KVM and ``smmuv3Dev`` model only) + The ``virtio`` IOMMU devices can further have ``address`` element as described in `Device addresses`_ (address has to by type of ``pci``). diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 281846dfbe..6d1adb831d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1353,6 +1353,7 @@ VIR_ENUM_IMPL(virDomainIOMMUModel, "smmuv3", "virtio", "amd", + "smmuv3Dev", ); VIR_ENUM_IMPL(virDomainVsockModel, @@ -2813,6 +2814,8 @@ virDomainIOMMUDefNew(void) iommu = g_new0(virDomainIOMMUDef, 1); + iommu->parent_idx = -1; + return g_steal_pointer(&iommu); } @@ -14439,6 +14442,10 @@ virDomainIOMMUDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropTristateSwitch(driver, "passthrough", VIR_XML_PROP_NONE, &iommu->pt) < 0) return NULL; + + if (virXMLPropInt(driver, "parentIdx", 10, VIR_XML_PROP_NONE, + &iommu->parent_idx, -1) < 0) + return NULL; } if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt, @@ -22092,6 +22099,12 @@ virDomainIOMMUDefCheckABIStability(virDomainIOMMUDef *src, dst->aw_bits, src->aw_bits); return false; } + if (src->parent_idx != dst->parent_idx) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain IOMMU device parent_idx value '%1$d' does not match source '%2$d'"), + dst->parent_idx, src->parent_idx); + return false; + } if (src->dma_translation != dst->dma_translation) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Target domain IOMMU device dma translation '%1$s' does not match source '%2$s'"), @@ -28409,6 +28422,10 @@ virDomainIOMMUDefFormat(virBuffer *buf, virBufferAsprintf(&driverAttrBuf, " xtsup='%s'", virTristateSwitchTypeToString(iommu->xtsup)); } + if (iommu->parent_idx >= 0) { + virBufferAsprintf(&driverAttrBuf, " parentIdx='%d'", + iommu->parent_idx); + } virXMLFormatElement(&childBuf, "driver", &driverAttrBuf, NULL); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 39807b5fe3..1d0c94a00a 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3039,6 +3039,7 @@ typedef enum { VIR_DOMAIN_IOMMU_MODEL_SMMUV3, VIR_DOMAIN_IOMMU_MODEL_VIRTIO, VIR_DOMAIN_IOMMU_MODEL_AMD, + VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV, VIR_DOMAIN_IOMMU_MODEL_LAST } virDomainIOMMUModel; @@ -3050,6 +3051,7 @@ struct _virDomainIOMMUDef { virTristateSwitch eim; virTristateSwitch iotlb; unsigned int aw_bits; + int parent_idx; virDomainDeviceInfo info; virTristateSwitch dma_translation; virTristateSwitch xtsup; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 93a2bc9b01..8a599fdbc6 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -3108,7 +3108,8 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu) iommu->eim != VIR_TRISTATE_SWITCH_ABSENT || iommu->iotlb != VIR_TRISTATE_SWITCH_ABSENT || iommu->aw_bits != 0 || - iommu->dma_translation != VIR_TRISTATE_SWITCH_ABSENT) { + iommu->dma_translation != VIR_TRISTATE_SWITCH_ABSENT || + iommu->parent_idx != -1) { virReportError(VIR_ERR_XML_ERROR, _("iommu model '%1$s' doesn't support additional attributes"), virDomainIOMMUModelTypeToString(iommu->model)); @@ -3120,7 +3121,8 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu) if (iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT || iommu->eim != VIR_TRISTATE_SWITCH_ABSENT || iommu->aw_bits != 0 || - iommu->dma_translation != VIR_TRISTATE_SWITCH_ABSENT) { + iommu->dma_translation != VIR_TRISTATE_SWITCH_ABSENT || + iommu->parent_idx != -1) { virReportError(VIR_ERR_XML_ERROR, _("iommu model '%1$s' doesn't support some additional attributes"), virDomainIOMMUModelTypeToString(iommu->model)); @@ -3130,7 +3132,24 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu) case VIR_DOMAIN_IOMMU_MODEL_INTEL: if (iommu->pt != VIR_TRISTATE_SWITCH_ABSENT || - iommu->xtsup != VIR_TRISTATE_SWITCH_ABSENT) { + iommu->xtsup != VIR_TRISTATE_SWITCH_ABSENT || + iommu->parent_idx != -1) { + virReportError(VIR_ERR_XML_ERROR, + _("iommu model '%1$s' doesn't support some additional attributes"), + virDomainIOMMUModelTypeToString(iommu->model)); + return -1; + } + break; + + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + if (iommu->intremap != VIR_TRISTATE_SWITCH_ABSENT || + iommu->caching_mode != VIR_TRISTATE_SWITCH_ABSENT || + iommu->eim != VIR_TRISTATE_SWITCH_ABSENT || + iommu->iotlb != VIR_TRISTATE_SWITCH_ABSENT || + iommu->aw_bits != 0 || + iommu->dma_translation != VIR_TRISTATE_SWITCH_ABSENT || + iommu->xtsup != VIR_TRISTATE_SWITCH_ABSENT || + iommu->pt != VIR_TRISTATE_SWITCH_ABSENT) { virReportError(VIR_ERR_XML_ERROR, _("iommu model '%1$s' doesn't support some additional attributes"), virDomainIOMMUModelTypeToString(iommu->model)); @@ -3155,6 +3174,7 @@ virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu) case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: case VIR_DOMAIN_IOMMU_MODEL_AMD: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: case VIR_DOMAIN_IOMMU_MODEL_LAST: break; } diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index b9230a35b4..26880b6db2 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6266,6 +6266,7 @@ <value>smmuv3</value> <value>virtio</value> <value>amd</value> + <value>smmuv3Dev</value> </choice> </attribute> <interleave> @@ -6311,6 +6312,11 @@ <ref name="virOnOff"/> </attribute> </optional> + <optional> + <attribute name="parentIdx"> + <data type="unsignedInt"/> + </attribute> + </optional> </element> </optional> <optional> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 031f09b7a5..e789e8cf2c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6294,6 +6294,62 @@ qemuBuildBootCommandLine(virCommand *cmd, } +static virJSONValue * +qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, + const virDomainIOMMUDef *iommu) +{ + g_autoptr(virJSONValue) props = NULL; + g_autofree char *bus = NULL; + size_t i; + bool contIsPHB = false; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDef *cont = def->controllers[i]; + if (cont->idx == iommu->parent_idx) { + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + const char *alias = cont->info.alias; + contIsPHB = virDomainControllerIsPSeriesPHB(cont); + + if (!alias) + return NULL; + + if (virDomainDeviceAliasIsUserAlias(alias)) { + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT && + iommu->parent_idx <= 0) { + if (qemuDomainSupportsPCIMultibus(def)) + bus = g_strdup("pci.0"); + else + bus = g_strdup("pci"); + } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + bus = g_strdup("pcie.0"); + } + } else { + bus = g_strdup(alias); + } + break; + } + } + } + + if (!bus) + return NULL; + + if (contIsPHB && iommu->parent_idx > 0) { + char *temp_bus = g_strdup_printf("%s.0", bus); + g_free(bus); + bus = temp_bus; + } + + if (virJSONValueObjectAdd(&props, + "s:driver", "arm-smmuv3", + "s:primary-bus", bus, + NULL) < 0) + return NULL; + + return g_steal_pointer(&props); +} + + static int qemuBuildIOMMUCommandLine(virCommand *cmd, const virDomainDef *def, @@ -6342,7 +6398,6 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, return 0; case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: - /* There is no -device for SMMUv3, so nothing to be done here */ return 0; case VIR_DOMAIN_IOMMU_MODEL_AMD: @@ -6373,6 +6428,14 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, return 0; + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + if (!(props = qemuBuildPCISmmuv3DevDevProps(def, iommu))) + return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; + + return 0; + case VIR_DOMAIN_IOMMU_MODEL_LAST: default: virReportEnumRangeError(virDomainIOMMUModel, iommu->model); @@ -7206,6 +7269,7 @@ qemuBuildMachineCommandLine(virCommand *cmd, case VIR_DOMAIN_IOMMU_MODEL_INTEL: case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: case VIR_DOMAIN_IOMMU_MODEL_AMD: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: /* These IOMMUs are formatted in qemuBuildIOMMUCommandLine */ break; @@ -10860,15 +10924,15 @@ qemuBuildCommandLine(virDomainObj *vm, if (qemuBuildBootCommandLine(cmd, def) < 0) return NULL; - if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0) - return NULL; - if (qemuBuildGlobalControllerCommandLine(cmd, def) < 0) return NULL; if (qemuBuildControllersCommandLine(cmd, def, qemuCaps) < 0) return NULL; + if (qemuBuildIOMMUCommandLine(cmd, def, qemuCaps) < 0) + return NULL; + if (qemuBuildMemoryDeviceCommandLine(cmd, cfg, def, priv) < 0) return NULL; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 96a9ca9b14..06bf4fab32 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -952,6 +952,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, case VIR_DOMAIN_IOMMU_MODEL_INTEL: case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: case VIR_DOMAIN_IOMMU_MODEL_LAST: /* These are not PCI devices */ return 0; @@ -2378,6 +2379,7 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, case VIR_DOMAIN_IOMMU_MODEL_INTEL: case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: case VIR_DOMAIN_IOMMU_MODEL_LAST: /* These are not PCI devices */ break; diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index c7ecb467a3..aac004c544 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -5414,6 +5414,22 @@ qemuValidateDomainDeviceDefIOMMU(const virDomainIOMMUDef *iommu, } break; + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + if (!qemuDomainIsARMVirt(def)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOMMU device: '%1$s' is only supported with ARM Virt machines"), + virDomainIOMMUModelTypeToString(iommu->model)); + return -1; + } + // TODO: Check for pluggable device SMMUv3 qemu capability + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_MACHINE_VIRT_IOMMU)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("IOMMU device: '%1$s' is not supported with this QEMU binary"), + virDomainIOMMUModelTypeToString(iommu->model)); + return -1; + } + break; + case VIR_DOMAIN_IOMMU_MODEL_LAST: default: virReportEnumRangeError(virDomainIOMMUModel, iommu->model); -- 2.43.0
On Wed, Oct 01, 2025 at 05:23:46PM -0700, Nathan Chen via Devel wrote:
Introduce support for "smmuv3Dev" IOMMU model and its "parentIdx" driver attribute. The "parentIdx" attribute indicates the index of the controller that a smmuv3Dev IOMMU device is attached to.
Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- docs/formatdomain.rst | 9 +++- src/conf/domain_conf.c | 17 ++++++++ src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 26 +++++++++-- src/conf/schemas/domaincommon.rng | 6 +++ src/qemu/qemu_command.c | 72 +++++++++++++++++++++++++++++-- src/qemu/qemu_domain_address.c | 2 + src/qemu/qemu_validate.c | 16 +++++++ 8 files changed, 141 insertions(+), 9 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 031f09b7a5..e789e8cf2c 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6294,6 +6294,62 @@ qemuBuildBootCommandLine(virCommand *cmd, }
+static virJSONValue * +qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, + const virDomainIOMMUDef *iommu) +{ + g_autoptr(virJSONValue) props = NULL; + g_autofree char *bus = NULL; + size_t i; + bool contIsPHB = false; + + for (i = 0; i < def->ncontrollers; i++) { + virDomainControllerDef *cont = def->controllers[i]; + if (cont->idx == iommu->parent_idx) { + if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + const char *alias = cont->info.alias; + contIsPHB = virDomainControllerIsPSeriesPHB(cont);
IIUC, PSeries PHB is an ppc64 device, while SMMUv3 is a aarch64 device. Given your validation check in qemu_validate.c, this combination of "IsPHB==true" and SMMUv3 seems like an impossible situation that doesn't need to be chedcked for and thus....
+ + if (!alias) + return NULL; + + if (virDomainDeviceAliasIsUserAlias(alias)) { + if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT && + iommu->parent_idx <= 0) { + if (qemuDomainSupportsPCIMultibus(def)) + bus = g_strdup("pci.0"); + else + bus = g_strdup("pci"); + } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) { + bus = g_strdup("pcie.0"); + } + } else { + bus = g_strdup(alias); + } + break; + } + } + } + + if (!bus) + return NULL;
This needs to be calling virReportError I think, since no earlier code path has already blocked this scenario ?
+ + if (contIsPHB && iommu->parent_idx > 0) { + char *temp_bus = g_strdup_printf("%s.0", bus); + g_free(bus); + bus = temp_bus; + }
...this if {} block should be dead code.
+ + if (virJSONValueObjectAdd(&props, + "s:driver", "arm-smmuv3", + "s:primary-bus", bus, + NULL) < 0) + return NULL; + + return g_steal_pointer(&props); +}
With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On Wed, Oct 01, 2025 at 05:23:46PM -0700, Nathan Chen via Devel wrote:
Introduce support for "smmuv3Dev" IOMMU model and its "parentIdx" driver attribute. The "parentIdx" attribute indicates the index of the controller that a smmuv3Dev IOMMU device is attached to.
Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- docs/formatdomain.rst | 9 +++- src/conf/domain_conf.c | 17 ++++++++ src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 26 +++++++++-- src/conf/schemas/domaincommon.rng | 6 +++ src/qemu/qemu_command.c | 72 +++++++++++++++++++++++++++++-- src/qemu/qemu_domain_address.c | 2 + src/qemu/qemu_validate.c | 16 +++++++ 8 files changed, 141 insertions(+), 9 deletions(-)
@@ -9212,6 +9213,10 @@ Example: Enable x2APIC mode. Useful for higher number of guest CPUs. :since:`Since 11.5.0` (QEMU/KVM and ``amd`` model only)
+ ``parentIdx`` + The ``parentIdx`` attribute notes the index of the controller that an + IOMMU device is attached to. (QEMU/KVM and ``smmuv3Dev`` model only) +
In the individual devices, for the <address> element we use 'bus' to refer to the index of the PCI controller. I wonder if we shouldn't keep that terminology similar, such that we get a 'pciBus' attribute instead of 'parentIdx'. eg <iommu .... pciBus='2'/> With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On a Wednesday in 2025, Nathan Chen via Devel wrote:
Introduce support for "smmuv3Dev" IOMMU model and its "parentIdx" driver attribute. The "parentIdx" attribute indicates the index of the controller that a smmuv3Dev IOMMU device is attached to.
Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- docs/formatdomain.rst | 9 +++- src/conf/domain_conf.c | 17 ++++++++ src/conf/domain_conf.h | 2 + src/conf/domain_validate.c | 26 +++++++++-- src/conf/schemas/domaincommon.rng | 6 +++ src/qemu/qemu_command.c | 72 +++++++++++++++++++++++++++++-- src/qemu/qemu_domain_address.c | 2 + src/qemu/qemu_validate.c | 16 +++++++ 8 files changed, 141 insertions(+), 9 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index f50dce477f..6a62291600 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -9161,8 +9161,9 @@ Example: ``model`` Supported values are ``intel`` (for Q35 guests) ``smmuv3`` (:since:`since 5.5.0`, for ARM virt guests), ``virtio`` - (:since:`since 8.3.0`, for Q35 and ARM virt guests) and - ``amd`` (:since:`since 11.5.0`). + (:since:`since 8.3.0`, for Q35 and ARM virt guests), + ``amd`` (:since:`since 11.5.0`), and ``smmuv3Dev`` (for + ARM virt guests).
``driver`` The ``driver`` subelement can be used to configure additional options, some @@ -9212,6 +9213,10 @@ Example: Enable x2APIC mode. Useful for higher number of guest CPUs. :since:`Since 11.5.0` (QEMU/KVM and ``amd`` model only)
+ ``parentIdx`` + The ``parentIdx`` attribute notes the index of the controller that an + IOMMU device is attached to. (QEMU/KVM and ``smmuv3Dev`` model only) + The ``virtio`` IOMMU devices can further have ``address`` element as described in `Device addresses`_ (address has to by type of ``pci``).
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 281846dfbe..6d1adb831d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1353,6 +1353,7 @@ VIR_ENUM_IMPL(virDomainIOMMUModel, "smmuv3", "virtio", "amd", + "smmuv3Dev",
Is a separate device model necessary here? The 'smmuv3' model is already there and the presence of the parentIdx/pciBus attribute specifies whether -machine or -device should be used.
);
VIR_ENUM_IMPL(virDomainVsockModel, @@ -2813,6 +2814,8 @@ virDomainIOMMUDefNew(void)
iommu = g_new0(virDomainIOMMUDef, 1);
+ iommu->parent_idx = -1; + return g_steal_pointer(&iommu); }
@@ -14439,6 +14442,10 @@ virDomainIOMMUDefParseXML(virDomainXMLOption *xmlopt, if (virXMLPropTristateSwitch(driver, "passthrough", VIR_XML_PROP_NONE, &iommu->pt) < 0) return NULL; + + if (virXMLPropInt(driver, "parentIdx", 10, VIR_XML_PROP_NONE,
To me, parentIdx somehow implies that this is also a PCI controller. Jano
+ &iommu->parent_idx, -1) < 0) + return NULL; }
if (virDomainDeviceInfoParseXML(xmlopt, node, ctxt,
Add support for parsing multiple IOMMU devices from the VM definition when "smmuv3Dev" is the IOMMU model. Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- src/conf/domain_conf.c | 84 +++++++++++++---- src/conf/domain_conf.h | 9 +- src/conf/domain_validate.c | 32 ++++--- src/conf/schemas/domaincommon.rng | 4 +- src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 15 ++- src/qemu/qemu_command.c | 146 ++++++++++++++++-------------- src/qemu/qemu_domain_address.c | 35 +++---- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_postparse.c | 11 ++- src/qemu/qemu_validate.c | 2 +- 11 files changed, 215 insertions(+), 133 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d1adb831d..1c2cf9a2d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4134,7 +4134,8 @@ void virDomainDefFree(virDomainDef *def) virDomainCryptoDefFree(def->cryptos[i]); g_free(def->cryptos); - virDomainIOMMUDefFree(def->iommu); + for (i = 0; i < def->niommus; i++) + virDomainIOMMUDefFree(def->iommu[i]); virDomainPstoreDefFree(def->pstore); @@ -5006,9 +5007,9 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def, } device.type = VIR_DOMAIN_DEVICE_IOMMU; - if (def->iommu) { - device.data.iommu = def->iommu; - if ((rc = cb(def, &device, &def->iommu->info, opaque)) != 0) + for (i = 0; i < def->niommus; i++) { + device.data.iommu = def->iommu[i]; + if ((rc = cb(def, &device, &def->iommu[i]->info, opaque)) != 0) return rc; } @@ -16487,6 +16488,43 @@ virDomainInputDefFind(const virDomainDef *def, } +bool +virDomainIOMMUDefEquals(const virDomainIOMMUDef *a, + const virDomainIOMMUDef *b) +{ + if (a->model != b->model || + a->intremap != b->intremap || + a->caching_mode != b->caching_mode || + a->eim != b->eim || + a->iotlb != b->iotlb || + a->aw_bits != b->aw_bits || + a->parent_idx != b->parent_idx || + a->dma_translation != b->dma_translation) + return false; + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + +ssize_t +virDomainIOMMUDefFind(const virDomainDef *def, + const virDomainIOMMUDef *iommu) +{ + size_t i; + + for (i = 0; i < def->niommus; i++) { + if (virDomainIOMMUDefEquals(iommu, def->iommu[i])) + return i; + } + + return -1; +} + + bool virDomainVsockDefEquals(const virDomainVsockDef *a, const virDomainVsockDef *b) @@ -20154,19 +20192,28 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, } VIR_FREE(nodes); + /* analysis of iommu devices */ if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0) return NULL; - if (n > 1) { + if (n > 1 && !virXPathBoolean("./devices/iommu/@model = 'smmuv3Dev'", ctxt)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single IOMMU device is supported")); + _("multiple IOMMU devices are only supported with model smmuv3Dev")); return NULL; } - if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(xmlopt, nodes[0], - ctxt, flags))) + if (n > 0) + def->iommu = g_new0(virDomainIOMMUDef *, n); + + for (i = 0; i < n; i++) { + virDomainIOMMUDef *iommu; + + iommu = virDomainIOMMUDefParseXML(xmlopt, nodes[i], ctxt, flags); + + if (!iommu) return NULL; + + def->iommu[def->niommus++] = iommu; } VIR_FREE(nodes); @@ -22619,15 +22666,17 @@ virDomainDefCheckABIStabilityFlags(virDomainDef *src, goto error; } - if (!!src->iommu != !!dst->iommu) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Target domain IOMMU device count does not match source")); + if (src->niommus != dst->niommus) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("Target domain IOMMU device count %1$zu does not match source %2$zu"), + dst->niommus, src->niommus); goto error; } - if (src->iommu && - !virDomainIOMMUDefCheckABIStability(src->iommu, dst->iommu)) - goto error; + for (i = 0; i < src->niommus; i++) { + if (!virDomainIOMMUDefCheckABIStability(src->iommu[i], dst->iommu[i])) + goto error; + } if (!!src->vsock != !!dst->vsock) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", @@ -29464,8 +29513,9 @@ virDomainDefFormatInternalSetRootName(virDomainDef *def, for (n = 0; n < def->ncryptos; n++) { virDomainCryptoDefFormat(buf, def->cryptos[n], flags); } - if (def->iommu) - virDomainIOMMUDefFormat(buf, def->iommu); + + for (n = 0; n < def->niommus; n++) + virDomainIOMMUDefFormat(buf, def->iommu[n]); if (def->vsock) virDomainVsockDefFormat(buf, def->vsock); diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1d0c94a00a..f830fe5226 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3297,6 +3297,9 @@ struct _virDomainDef { size_t nwatchdogs; virDomainWatchdogDef **watchdogs; + size_t niommus; + virDomainIOMMUDef **iommu; + /* At maximum 2 TPMs on the domain if a TPM Proxy is present. */ size_t ntpms; virDomainTPMDef **tpms; @@ -3306,7 +3309,6 @@ struct _virDomainDef { virDomainNVRAMDef *nvram; virCPUDef *cpu; virDomainRedirFilterDef *redirfilter; - virDomainIOMMUDef *iommu; virDomainVsockDef *vsock; virDomainPstoreDef *pstore; @@ -4311,6 +4313,11 @@ virDomainShmemDef *virDomainShmemDefRemove(virDomainDef *def, size_t idx) ssize_t virDomainInputDefFind(const virDomainDef *def, const virDomainInputDef *input) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; +bool virDomainIOMMUDefEquals(const virDomainIOMMUDef *a, + const virDomainIOMMUDef *b); +ssize_t virDomainIOMMUDefFind(const virDomainDef *def, + const virDomainIOMMUDef *iommu) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; bool virDomainVsockDefEquals(const virDomainVsockDef *a, const virDomainVsockDef *b) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 8a599fdbc6..588ecdaa20 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1851,21 +1851,31 @@ virDomainDefCputuneValidate(const virDomainDef *def) static int virDomainDefIOMMUValidate(const virDomainDef *def) { + size_t i; + if (!def->iommu) return 0; - if (def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && - def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOMMU interrupt remapping requires split I/O APIC (ioapic driver='qemu')")); - return -1; - } + for (i = 0; i < def->niommus; i++) { + virDomainIOMMUDef *iommu = def->iommu[i]; + if (def->niommus > 1 && iommu->model != VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU model smmuv3Dev must be specified for multiple IOMMU definitions")); + } - if (def->iommu->eim == VIR_TRISTATE_SWITCH_ON && - def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOMMU eim requires interrupt remapping to be enabled")); - return -1; + if (iommu->intremap == VIR_TRISTATE_SWITCH_ON && + def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOMMU interrupt remapping requires split I/O APIC (ioapic driver='qemu')")); + return -1; + } + + if (iommu->eim == VIR_TRISTATE_SWITCH_ON && + iommu->intremap != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOMMU eim requires interrupt remapping to be enabled")); + return -1; + } } return 0; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 26880b6db2..b9c00e98a5 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6959,9 +6959,9 @@ <zeroOrMore> <ref name="panic"/> </zeroOrMore> - <optional> + <zeroOrMore> <ref name="iommu"/> - </optional> + </zeroOrMore> <optional> <ref name="vsock"/> </optional> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fe72402527..34d9c263d4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -491,6 +491,8 @@ virDomainInputSourceGrabToggleTypeToString; virDomainInputSourceGrabTypeFromString; virDomainInputSourceGrabTypeToString; virDomainInputTypeToString; +virDomainIOMMUDefEquals; +virDomainIOMMUDefFind; virDomainIOMMUDefFree; virDomainIOMMUDefNew; virDomainIOMMUModelTypeFromString; diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index a27c688d79..5f2b11b9a6 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -647,10 +647,14 @@ qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock) static void -qemuAssignDeviceIOMMUAlias(virDomainIOMMUDef *iommu) +qemuAssignDeviceIOMMUAlias(virDomainDef *def, + virDomainIOMMUDef **iommu) { - if (!iommu->info.alias) - iommu->info.alias = g_strdup("iommu0"); + size_t i; + for (i = 0; i < def->niommus; i++) { + if (!iommu[i]->info.alias) + iommu[i]->info.alias = g_strdup_printf("iommu%zu", i); + } } @@ -766,8 +770,9 @@ qemuAssignDeviceAliases(virDomainDef *def) if (def->vsock) { qemuAssignDeviceVsockAlias(def->vsock); } - if (def->iommu) - qemuAssignDeviceIOMMUAlias(def->iommu); + if (def->iommu && def->niommus > 0) { + qemuAssignDeviceIOMMUAlias(def, def->iommu); + } for (i = 0; i < def->ncryptos; i++) { qemuAssignDeviceCryptoAlias(def, def->cryptos[i]); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e789e8cf2c..15ae919047 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6296,10 +6296,12 @@ qemuBuildBootCommandLine(virCommand *cmd, static virJSONValue * qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, - const virDomainIOMMUDef *iommu) + const virDomainIOMMUDef *iommu, + size_t id) { g_autoptr(virJSONValue) props = NULL; g_autofree char *bus = NULL; + g_autofree char *smmuv3_id = NULL; size_t i; bool contIsPHB = false; @@ -6340,9 +6342,12 @@ qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, bus = temp_bus; } + smmuv3_id = g_strdup_printf("smmuv3.%zu", id); + if (virJSONValueObjectAdd(&props, "s:driver", "arm-smmuv3", "s:primary-bus", bus, + "s:id", smmuv3_id, NULL) < 0) return NULL; @@ -6355,91 +6360,92 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, const virDomainDef *def, virQEMUCaps *qemuCaps) { + size_t i; g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) wrapperProps = NULL; - const virDomainIOMMUDef *iommu = def->iommu; - - if (!iommu) + if (!def->iommu || def->niommus <= 0) return 0; - switch (iommu->model) { - case VIR_DOMAIN_IOMMU_MODEL_INTEL: - if (virJSONValueObjectAdd(&props, - "s:driver", "intel-iommu", - "s:id", iommu->info.alias, - "S:intremap", qemuOnOffAuto(iommu->intremap), - "T:caching-mode", iommu->caching_mode, - "S:eim", qemuOnOffAuto(iommu->eim), - "T:device-iotlb", iommu->iotlb, - "z:aw-bits", iommu->aw_bits, - "T:dma-translation", iommu->dma_translation, - NULL) < 0) - return -1; + for (i = 0; i < def->niommus; i++) { + virDomainIOMMUDef *iommu = def->iommu[i]; + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + if (virJSONValueObjectAdd(&props, + "s:driver", "intel-iommu", + "s:id", iommu->info.alias, + "S:intremap", qemuOnOffAuto(iommu->intremap), + "T:caching-mode", iommu->caching_mode, + "S:eim", qemuOnOffAuto(iommu->eim), + "T:device-iotlb", iommu->iotlb, + "z:aw-bits", iommu->aw_bits, + "T:dma-translation", iommu->dma_translation, + NULL) < 0) + return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) - return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; - return 0; + return 0; + case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: + if (virJSONValueObjectAdd(&props, + "s:driver", "virtio-iommu", + "s:id", iommu->info.alias, + NULL) < 0) { + return -1; + } - case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: - if (virJSONValueObjectAdd(&props, - "s:driver", "virtio-iommu", - "s:id", iommu->info.alias, - NULL) < 0) { - return -1; - } + if (qemuBuildDeviceAddressProps(props, def, &iommu->info) < 0) + return -1; - if (qemuBuildDeviceAddressProps(props, def, &iommu->info) < 0) - return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) - return -1; + return 0; + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + /* There is no -device for SMMUv3, so nothing to be done here */ + return 0; - return 0; + case VIR_DOMAIN_IOMMU_MODEL_AMD: + if (virJSONValueObjectAdd(&wrapperProps, + "s:driver", "AMDVI-PCI", + "s:id", iommu->info.alias, + NULL) < 0) + return -1; - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: - return 0; + if (qemuBuildDeviceAddressProps(wrapperProps, def, &iommu->info) < 0) + return -1; - case VIR_DOMAIN_IOMMU_MODEL_AMD: - if (virJSONValueObjectAdd(&wrapperProps, - "s:driver", "AMDVI-PCI", - "s:id", iommu->info.alias, - NULL) < 0) - return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, wrapperProps, def, qemuCaps) < 0) + return -1; - if (qemuBuildDeviceAddressProps(wrapperProps, def, &iommu->info) < 0) - return -1; + if (virJSONValueObjectAdd(&props, + "s:driver", "amd-iommu", + "s:pci-id", iommu->info.alias, + "S:intremap", qemuOnOffAuto(iommu->intremap), + "T:pt", iommu->pt, + "T:xtsup", iommu->xtsup, + "T:device-iotlb", iommu->iotlb, + NULL) < 0) + return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, wrapperProps, def, qemuCaps) < 0) - return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; - if (virJSONValueObjectAdd(&props, - "s:driver", "amd-iommu", - "s:pci-id", iommu->info.alias, - "S:intremap", qemuOnOffAuto(iommu->intremap), - "T:pt", iommu->pt, - "T:xtsup", iommu->xtsup, - "T:device-iotlb", iommu->iotlb, - NULL) < 0) - return -1; + return 0; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) - return -1; + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + if (!(props = qemuBuildPCISmmuv3DevDevProps(def, iommu, i))) + return -1; + if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + return -1; + break; - return 0; - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: - if (!(props = qemuBuildPCISmmuv3DevDevProps(def, iommu))) - return -1; - if (qemuBuildDeviceCommandlineFromJSON(cmd, props, def, qemuCaps) < 0) + case VIR_DOMAIN_IOMMU_MODEL_LAST: + default: + virReportEnumRangeError(virDomainIOMMUModel, iommu->model); return -1; - - return 0; - - case VIR_DOMAIN_IOMMU_MODEL_LAST: - default: - virReportEnumRangeError(virDomainIOMMUModel, iommu->model); - return -1; + } } return 0; @@ -7260,8 +7266,8 @@ qemuBuildMachineCommandLine(virCommand *cmd, if (qemuAppendDomainFeaturesMachineParam(&buf, def, qemuCaps) < 0) return -1; - if (def->iommu) { - switch (def->iommu->model) { + if (def->iommu && def->niommus == 1) { + switch (def->iommu[0]->model) { case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: virBufferAddLit(&buf, ",iommu=smmuv3"); break; @@ -7275,7 +7281,7 @@ qemuBuildMachineCommandLine(virCommand *cmd, case VIR_DOMAIN_IOMMU_MODEL_LAST: default: - virReportEnumRangeError(virDomainIOMMUModel, def->iommu->model); + virReportEnumRangeError(virDomainIOMMUModel, def->iommu[0]->model); return -1; } } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 06bf4fab32..2ddc629304 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2365,24 +2365,25 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, /* Nada - none are PCI based (yet) */ } - if (def->iommu) { - virDomainIOMMUDef *iommu = def->iommu; - - switch (iommu->model) { - case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: - case VIR_DOMAIN_IOMMU_MODEL_AMD: - if (virDeviceInfoPCIAddressIsWanted(&iommu->info) && - qemuDomainPCIAddressReserveNextAddr(addrs, &iommu->info) < 0) { - return -1; - } - break; + if (def->iommu && def->niommus > 0) { + for (i = 0; i < def->niommus; i++) { + virDomainIOMMUDef *iommu = def->iommu[i]; + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: + case VIR_DOMAIN_IOMMU_MODEL_AMD: + if (virDeviceInfoPCIAddressIsWanted(&iommu->info) && + qemuDomainPCIAddressReserveNextAddr(addrs, &iommu->info) < 0) { + return -1; + } + break; - case VIR_DOMAIN_IOMMU_MODEL_INTEL: - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: - case VIR_DOMAIN_IOMMU_MODEL_LAST: - /* These are not PCI devices */ - break; + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + case VIR_DOMAIN_IOMMU_MODEL_LAST: + /* These are not PCI devices */ + break; + } } } diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac72ea5cb0..3d65f78c9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6894,12 +6894,12 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, break; case VIR_DOMAIN_DEVICE_IOMMU: - if (vmdef->iommu) { + if (vmdef->iommu && vmdef->niommus > 0) { virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain already has an iommu device")); return -1; } - vmdef->iommu = g_steal_pointer(&dev->data.iommu); + VIR_APPEND_ELEMENT(vmdef->iommu, vmdef->niommus, dev->data.iommu); break; case VIR_DOMAIN_DEVICE_VIDEO: @@ -7113,12 +7113,12 @@ qemuDomainDetachDeviceConfig(virDomainDef *vmdef, break; case VIR_DOMAIN_DEVICE_IOMMU: - if (!vmdef->iommu) { + if ((idx = virDomainIOMMUDefFind(vmdef, dev->data.iommu)) < 0) { virReportError(VIR_ERR_OPERATION_FAILED, "%s", _("matching iommu device not found")); return -1; } - g_clear_pointer(&vmdef->iommu, virDomainIOMMUDefFree); + VIR_DELETE_ELEMENT(vmdef->iommu, idx, vmdef->niommus); break; case VIR_DOMAIN_DEVICE_VIDEO: diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c index 9c2427970d..e2744a4a61 100644 --- a/src/qemu/qemu_postparse.c +++ b/src/qemu/qemu_postparse.c @@ -1503,7 +1503,7 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, } } - if (addIOMMU && !def->iommu && + if (addIOMMU && !def->iommu && def->niommus == 0 && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP) && virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_EIM)) { @@ -1515,7 +1515,8 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver, iommu->intremap = VIR_TRISTATE_SWITCH_ON; iommu->eim = VIR_TRISTATE_SWITCH_ON; - def->iommu = g_steal_pointer(&iommu); + def->iommu = g_new0(virDomainIOMMUDef *, 1); + def->iommu[def->niommus++] = g_steal_pointer(&iommu); } if (qemuDomainDefAddDefaultAudioBackend(driver, def) < 0) @@ -1591,9 +1592,9 @@ qemuDomainDefEnableDefaultFeatures(virDomainDef *def, * domain already has IOMMU without inremap. This will be fixed in * qemuDomainIOMMUDefPostParse() but there domain definition can't be * modified so change it now. */ - if (def->iommu && - (def->iommu->intremap == VIR_TRISTATE_SWITCH_ON || - qemuDomainNeedsIOMMUWithEIM(def)) && + if (def->iommu && def->niommus == 1 && + (def->iommu[0]->intremap == VIR_TRISTATE_SWITCH_ON || + qemuDomainNeedsIOMMUWithEIM(def)) && def->features[VIR_DOMAIN_FEATURE_IOAPIC] == VIR_DOMAIN_IOAPIC_NONE) { def->features[VIR_DOMAIN_FEATURE_IOAPIC] = VIR_DOMAIN_IOAPIC_QEMU; } diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c index aac004c544..bdaeefe976 100644 --- a/src/qemu/qemu_validate.c +++ b/src/qemu/qemu_validate.c @@ -851,7 +851,7 @@ qemuValidateDomainVCpuTopology(const virDomainDef *def, virQEMUCaps *qemuCaps) QEMU_MAX_VCPUS_WITHOUT_EIM); return -1; } - if (!def->iommu || def->iommu->eim != VIR_TRISTATE_SWITCH_ON) { + if (!def->iommu || def->iommu[0]->eim != VIR_TRISTATE_SWITCH_ON) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("more than %1$d vCPUs require extended interrupt mode enabled on the iommu device"), QEMU_MAX_VCPUS_WITHOUT_EIM); -- 2.43.0
On Wed, Oct 01, 2025 at 05:23:47PM -0700, Nathan Chen via Devel wrote:
Add support for parsing multiple IOMMU devices from the VM definition when "smmuv3Dev" is the IOMMU model.
Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- src/conf/domain_conf.c | 84 +++++++++++++---- src/conf/domain_conf.h | 9 +- src/conf/domain_validate.c | 32 ++++--- src/conf/schemas/domaincommon.rng | 4 +- src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 15 ++- src/qemu/qemu_command.c | 146 ++++++++++++++++-------------- src/qemu/qemu_domain_address.c | 35 +++---- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_postparse.c | 11 ++- src/qemu/qemu_validate.c | 2 +- 11 files changed, 215 insertions(+), 133 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d1adb831d..1c2cf9a2d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4134,7 +4134,8 @@ void virDomainDefFree(virDomainDef *def) virDomainCryptoDefFree(def->cryptos[i]); g_free(def->cryptos);
- virDomainIOMMUDefFree(def->iommu); + for (i = 0; i < def->niommus; i++) + virDomainIOMMUDefFree(def->iommu[i]);
Also need 'g_free(def->iommu)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1d0c94a00a..f830fe5226 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3297,6 +3297,9 @@ struct _virDomainDef { size_t nwatchdogs; virDomainWatchdogDef **watchdogs;
+ size_t niommus; + virDomainIOMMUDef **iommu;
Call that 'iommus' since we tend to use plurals for these fields. With regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Hi Daniel, Thank you for the detailed feedback on patches 1 and 2. I will address these review comments in the next revision. Thanks, Nathan
On a Wednesday in 2025, Nathan Chen via Devel wrote:
Add support for parsing multiple IOMMU devices from the VM definition when "smmuv3Dev" is the IOMMU model.
Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- src/conf/domain_conf.c | 84 +++++++++++++---- src/conf/domain_conf.h | 9 +- src/conf/domain_validate.c | 32 ++++--- src/conf/schemas/domaincommon.rng | 4 +- src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 15 ++- src/qemu/qemu_command.c | 146 ++++++++++++++++-------------- src/qemu/qemu_domain_address.c | 35 +++---- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_postparse.c | 11 ++- src/qemu/qemu_validate.c | 2 +- 11 files changed, 215 insertions(+), 133 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6d1adb831d..1c2cf9a2d9 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4134,7 +4134,8 @@ void virDomainDefFree(virDomainDef *def) virDomainCryptoDefFree(def->cryptos[i]); g_free(def->cryptos);
- virDomainIOMMUDefFree(def->iommu); + for (i = 0; i < def->niommus; i++) + virDomainIOMMUDefFree(def->iommu[i]);
virDomainPstoreDefFree(def->pstore);
@@ -5006,9 +5007,9 @@ virDomainDeviceInfoIterateFlags(virDomainDef *def, }
device.type = VIR_DOMAIN_DEVICE_IOMMU; - if (def->iommu) { - device.data.iommu = def->iommu; - if ((rc = cb(def, &device, &def->iommu->info, opaque)) != 0) + for (i = 0; i < def->niommus; i++) { + device.data.iommu = def->iommu[i]; + if ((rc = cb(def, &device, &def->iommu[i]->info, opaque)) != 0) return rc; }
@@ -16487,6 +16488,43 @@ virDomainInputDefFind(const virDomainDef *def, }
+bool +virDomainIOMMUDefEquals(const virDomainIOMMUDef *a, + const virDomainIOMMUDef *b) +{ + if (a->model != b->model || + a->intremap != b->intremap || + a->caching_mode != b->caching_mode || + a->eim != b->eim || + a->iotlb != b->iotlb || + a->aw_bits != b->aw_bits || + a->parent_idx != b->parent_idx || + a->dma_translation != b->dma_translation) + return false; + + if (a->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE && + !virDomainDeviceInfoAddressIsEqual(&a->info, &b->info)) + return false; + + return true; +} + + +ssize_t +virDomainIOMMUDefFind(const virDomainDef *def, + const virDomainIOMMUDef *iommu) +{ + size_t i; + + for (i = 0; i < def->niommus; i++) { + if (virDomainIOMMUDefEquals(iommu, def->iommu[i])) + return i; + } + + return -1; +} + + bool virDomainVsockDefEquals(const virDomainVsockDef *a, const virDomainVsockDef *b) @@ -20154,19 +20192,28 @@ virDomainDefParseXML(xmlXPathContextPtr ctxt, } VIR_FREE(nodes);
+ /* analysis of iommu devices */
Not sure if 'analysis' is the best word to use here. Or why it's already present in so many other cases.
if ((n = virXPathNodeSet("./devices/iommu", ctxt, &nodes)) < 0) return NULL;
- if (n > 1) { + if (n > 1 && !virXPathBoolean("./devices/iommu/@model = 'smmuv3Dev'", ctxt)) { virReportError(VIR_ERR_XML_ERROR, "%s", - _("only a single IOMMU device is supported")); + _("multiple IOMMU devices are only supported with model smmuv3Dev")); return NULL;
Before, having a single iommu device was a limitation of the parser because it was not stored in array. After the switch, the parser should just parse what it's given and the validator would reject incorrect combinations. Also, it would be nicer if the switch to an array was in a separate, preceding patch, that does not change the behavior at all.
}
- if (n > 0) { - if (!(def->iommu = virDomainIOMMUDefParseXML(xmlopt, nodes[0], - ctxt, flags))) + if (n > 0) + def->iommu = g_new0(virDomainIOMMUDef *, n); + + for (i = 0; i < n; i++) { + virDomainIOMMUDef *iommu; + + iommu = virDomainIOMMUDefParseXML(xmlopt, nodes[i], ctxt, flags); + + if (!iommu) return NULL; + + def->iommu[def->niommus++] = iommu; } VIR_FREE(nodes);
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 1d0c94a00a..f830fe5226 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3297,6 +3297,9 @@ struct _virDomainDef { size_t nwatchdogs; virDomainWatchdogDef **watchdogs;
+ size_t niommus; + virDomainIOMMUDef **iommu;
'iommus', to make it obvious that there are multiple. Also to let compiler throw an error or any places where the conversion omitted something.
+ /* At maximum 2 TPMs on the domain if a TPM Proxy is present. */ size_t ntpms; virDomainTPMDef **tpms; diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 8a599fdbc6..588ecdaa20 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -1851,21 +1851,31 @@ virDomainDefCputuneValidate(const virDomainDef *def) static int virDomainDefIOMMUValidate(const virDomainDef *def) { + size_t i; + if (!def->iommu) return 0;
This is no longer needed - if there were no iommus parsed, the loop below will be skipped.
- if (def->iommu->intremap == VIR_TRISTATE_SWITCH_ON && - def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOMMU interrupt remapping requires split I/O APIC (ioapic driver='qemu')")); - return -1; - } + for (i = 0; i < def->niommus; i++) { + virDomainIOMMUDef *iommu = def->iommu[i]; + if (def->niommus > 1 && iommu->model != VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("IOMMU model smmuv3Dev must be specified for multiple IOMMU definitions")); + }
- if (def->iommu->eim == VIR_TRISTATE_SWITCH_ON && - def->iommu->intremap != VIR_TRISTATE_SWITCH_ON) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("IOMMU eim requires interrupt remapping to be enabled")); - return -1; + if (iommu->intremap == VIR_TRISTATE_SWITCH_ON && + def->features[VIR_DOMAIN_FEATURE_IOAPIC] != VIR_DOMAIN_IOAPIC_QEMU) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOMMU interrupt remapping requires split I/O APIC (ioapic driver='qemu')")); + return -1; + } + + if (iommu->eim == VIR_TRISTATE_SWITCH_ON && + iommu->intremap != VIR_TRISTATE_SWITCH_ON) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("IOMMU eim requires interrupt remapping to be enabled")); + return -1; + } }
return 0; diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng index 26880b6db2..b9c00e98a5 100644 --- a/src/conf/schemas/domaincommon.rng +++ b/src/conf/schemas/domaincommon.rng @@ -6959,9 +6959,9 @@ <zeroOrMore> <ref name="panic"/> </zeroOrMore> - <optional> + <zeroOrMore> <ref name="iommu"/> - </optional> + </zeroOrMore> <optional> <ref name="vsock"/> </optional> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index fe72402527..34d9c263d4 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -491,6 +491,8 @@ virDomainInputSourceGrabToggleTypeToString; virDomainInputSourceGrabTypeFromString; virDomainInputSourceGrabTypeToString; virDomainInputTypeToString; +virDomainIOMMUDefEquals; +virDomainIOMMUDefFind; virDomainIOMMUDefFree; virDomainIOMMUDefNew; virDomainIOMMUModelTypeFromString; diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index a27c688d79..5f2b11b9a6 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -647,10 +647,14 @@ qemuAssignDeviceVsockAlias(virDomainVsockDef *vsock)
static void -qemuAssignDeviceIOMMUAlias(virDomainIOMMUDef *iommu) +qemuAssignDeviceIOMMUAlias(virDomainDef *def, + virDomainIOMMUDef **iommu) { - if (!iommu->info.alias) - iommu->info.alias = g_strdup("iommu0"); + size_t i; + for (i = 0; i < def->niommus; i++) { + if (!iommu[i]->info.alias) + iommu[i]->info.alias = g_strdup_printf("iommu%zu", i); + } }
@@ -766,8 +770,9 @@ qemuAssignDeviceAliases(virDomainDef *def) if (def->vsock) { qemuAssignDeviceVsockAlias(def->vsock); } - if (def->iommu) - qemuAssignDeviceIOMMUAlias(def->iommu); + if (def->iommu && def->niommus > 0) {
def->niommus > 0 is a sufficient condition.
+ qemuAssignDeviceIOMMUAlias(def, def->iommu); + } for (i = 0; i < def->ncryptos; i++) { qemuAssignDeviceCryptoAlias(def, def->cryptos[i]); } diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index e789e8cf2c..15ae919047 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6296,10 +6296,12 @@ qemuBuildBootCommandLine(virCommand *cmd,
static virJSONValue * qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, - const virDomainIOMMUDef *iommu) + const virDomainIOMMUDef *iommu, + size_t id) { g_autoptr(virJSONValue) props = NULL; g_autofree char *bus = NULL; + g_autofree char *smmuv3_id = NULL; size_t i; bool contIsPHB = false;
@@ -6340,9 +6342,12 @@ qemuBuildPCISmmuv3DevDevProps(const virDomainDef *def, bus = temp_bus; }
+ smmuv3_id = g_strdup_printf("smmuv3.%zu", id); + if (virJSONValueObjectAdd(&props, "s:driver", "arm-smmuv3", "s:primary-bus", bus, + "s:id", smmuv3_id,
This change belongs to the commit that added this function.
NULL) < 0) return NULL;
@@ -6355,91 +6360,92 @@ qemuBuildIOMMUCommandLine(virCommand *cmd, const virDomainDef *def, virQEMUCaps *qemuCaps) { + size_t i; g_autoptr(virJSONValue) props = NULL; g_autoptr(virJSONValue) wrapperProps = NULL; - const virDomainIOMMUDef *iommu = def->iommu; - - if (!iommu) + if (!def->iommu || def->niommus <= 0) return 0;
if (def->niommus == 0) return 0; niommus is size_t so it cannot be negative and we don't care whether def->iommu(s) is allocated or not if it does not contain any devices.
- switch (iommu->model) { - case VIR_DOMAIN_IOMMU_MODEL_INTEL: - if (virJSONValueObjectAdd(&props, - "s:driver", "intel-iommu", - "s:id", iommu->info.alias, - "S:intremap", qemuOnOffAuto(iommu->intremap), - "T:caching-mode", iommu->caching_mode, - "S:eim", qemuOnOffAuto(iommu->eim), - "T:device-iotlb", iommu->iotlb, - "z:aw-bits", iommu->aw_bits, - "T:dma-translation", iommu->dma_translation, - NULL) < 0) - return -1;
@@ -7260,8 +7266,8 @@ qemuBuildMachineCommandLine(virCommand *cmd, if (qemuAppendDomainFeaturesMachineParam(&buf, def, qemuCaps) < 0) return -1;
- if (def->iommu) { - switch (def->iommu->model) { + if (def->iommu && def->niommus == 1) {
Only 'niommus' is relevant here.
+ switch (def->iommu[0]->model) { case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: virBufferAddLit(&buf, ",iommu=smmuv3"); break; @@ -7275,7 +7281,7 @@ qemuBuildMachineCommandLine(virCommand *cmd,
case VIR_DOMAIN_IOMMU_MODEL_LAST: default: - virReportEnumRangeError(virDomainIOMMUModel, def->iommu->model); + virReportEnumRangeError(virDomainIOMMUModel, def->iommu[0]->model); return -1; } } diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 06bf4fab32..2ddc629304 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2365,24 +2365,25 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, /* Nada - none are PCI based (yet) */ }
- if (def->iommu) { - virDomainIOMMUDef *iommu = def->iommu; - - switch (iommu->model) { - case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: - case VIR_DOMAIN_IOMMU_MODEL_AMD: - if (virDeviceInfoPCIAddressIsWanted(&iommu->info) && - qemuDomainPCIAddressReserveNextAddr(addrs, &iommu->info) < 0) { - return -1; - } - break; + if (def->iommu && def->niommus > 0) {
The if is not necessary - if niommus is non-zero, def->iommu(s) will be allocated. And if it's zero the for loop is a no-op.
+ for (i = 0; i < def->niommus; i++) { + virDomainIOMMUDef *iommu = def->iommu[i]; + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: + case VIR_DOMAIN_IOMMU_MODEL_AMD: + if (virDeviceInfoPCIAddressIsWanted(&iommu->info) && + qemuDomainPCIAddressReserveNextAddr(addrs, &iommu->info) < 0) { + return -1; + } + break;
- case VIR_DOMAIN_IOMMU_MODEL_INTEL: - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: - case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: - case VIR_DOMAIN_IOMMU_MODEL_LAST: - /* These are not PCI devices */ - break; + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3_DEV: + case VIR_DOMAIN_IOMMU_MODEL_LAST: + /* These are not PCI devices */ + break; + } } }
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index ac72ea5cb0..3d65f78c9e 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6894,12 +6894,12 @@ qemuDomainAttachDeviceConfig(virDomainDef *vmdef, break;
case VIR_DOMAIN_DEVICE_IOMMU: - if (vmdef->iommu) { + if (vmdef->iommu && vmdef->niommus > 0) {
Same comment about checking niommus. Jano
virReportError(VIR_ERR_OPERATION_INVALID, "%s", _("domain already has an iommu device")); return -1; } - vmdef->iommu = g_steal_pointer(&dev->data.iommu); + VIR_APPEND_ELEMENT(vmdef->iommu, vmdef->niommus, dev->data.iommu); break;
case VIR_DOMAIN_DEVICE_VIDEO:
Hi Jano, Thank you for the detailed feedback on patches 1 and 2. I will address these review comments in the next revision. Thanks, Nathan
Provide sample XML and CLI args for the smmuv3Dev XML schema for virt machine type. Signed-off-by: Nathan Chen <nathanc@nvidia.com> --- .../iommu-smmuv3Dev.aarch64-latest.args | 41 ++++++++++++ .../iommu-smmuv3Dev.aarch64-latest.xml | 62 +++++++++++++++++++ tests/qemuxmlconfdata/iommu-smmuv3Dev.xml | 49 +++++++++++++++ tests/qemuxmlconftest.c | 1 + 4 files changed, 153 insertions(+) create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.args create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.xml create mode 100644 tests/qemuxmlconfdata/iommu-smmuv3Dev.xml diff --git a/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.args b/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.args new file mode 100644 index 0000000000..092fcf4623 --- /dev/null +++ b/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.args @@ -0,0 +1,41 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/var/lib/libvirt/qemu/domain--1-guest \ +USER=test \ +LOGNAME=test \ +XDG_DATA_HOME=/var/lib/libvirt/qemu/domain--1-guest/.local/share \ +XDG_CACHE_HOME=/var/lib/libvirt/qemu/domain--1-guest/.cache \ +XDG_CONFIG_HOME=/var/lib/libvirt/qemu/domain--1-guest/.config \ +/usr/bin/qemu-system-aarch64 \ +-name guest=guest,debug-threads=on \ +-S \ +-object '{"qom-type":"secret","id":"masterKey0","format":"raw","file":"/var/lib/libvirt/qemu/domain--1-guest/master-key.aes"}' \ +-machine virt,usb=off,gic-version=2,dump-guest-core=off,memory-backend=mach-virt.ram,acpi=off \ +-accel tcg \ +-cpu cortex-a15 \ +-m size=1048576k \ +-object '{"qom-type":"memory-backend-ram","id":"mach-virt.ram","size":1073741824}' \ +-overcommit mem-lock=off \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid 1ccfd97d-5eb4-478a-bbe6-88d254c16db7 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,fd=1729,server=on,wait=off \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot strict=on \ +-device '{"driver":"pxb-pcie","bus_nr":252,"id":"pci.1","bus":"pcie.0","addr":"0x1"}' \ +-device '{"driver":"pxb-pcie","bus_nr":248,"id":"pci.2","bus":"pcie.0","addr":"0x2"}' \ +-device '{"driver":"pcie-root-port","port":0,"chassis":21,"id":"pci.3","bus":"pci.1","addr":"0x0"}' \ +-device '{"driver":"pcie-root-port","port":168,"chassis":22,"id":"pci.4","bus":"pci.2","addr":"0x0"}' \ +-device '{"driver":"arm-smmuv3","primary-bus":"pci.1","id":"smmuv3.0"}' \ +-device '{"driver":"arm-smmuv3","primary-bus":"pci.2","id":"smmuv3.1"}' \ +-audiodev '{"id":"audio1","driver":"none"}' \ +-object '{"qom-type":"rng-random","id":"objrng0","filename":"/dev/urandom"}' \ +-device '{"driver":"virtio-rng-pci","rng":"objrng0","id":"rng0","bus":"pci.3","addr":"0x0"}' \ +-object '{"qom-type":"rng-random","id":"objrng1","filename":"/dev/urandom"}' \ +-device '{"driver":"virtio-rng-pci","rng":"objrng1","id":"rng1","bus":"pci.4","addr":"0x0"}' \ +-sandbox on,obsolete=deny,elevateprivileges=deny,spawn=deny,resourcecontrol=deny \ +-msg timestamp=on diff --git a/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.xml b/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.xml new file mode 100644 index 0000000000..dec29c245f --- /dev/null +++ b/tests/qemuxmlconfdata/iommu-smmuv3Dev.aarch64-latest.xml @@ -0,0 +1,62 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <currentMemory unit='KiB'>1048576</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + <boot dev='hd'/> + </os> + <features> + <gic version='2'/> + </features> + <cpu mode='custom' match='exact' check='none'> + <model fallback='forbid'>cortex-a15</model> + </cpu> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' index='0' model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-expander-bus'> + <model name='pxb-pcie'/> + <target busNr='252'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-expander-bus'> + <model name='pxb-pcie'/> + <target busNr='248'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='21' port='0x0'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='22' port='0xa8'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <audio id='1' type='none'/> + <memballoon model='none'/> + <rng model='virtio'> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </rng> + <rng model='virtio'> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </rng> + <iommu model='smmuv3Dev'> + <driver parentIdx='1'/> + </iommu> + <iommu model='smmuv3Dev'> + <driver parentIdx='2'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxmlconfdata/iommu-smmuv3Dev.xml b/tests/qemuxmlconfdata/iommu-smmuv3Dev.xml new file mode 100644 index 0000000000..6c6aea5952 --- /dev/null +++ b/tests/qemuxmlconfdata/iommu-smmuv3Dev.xml @@ -0,0 +1,49 @@ +<domain type='qemu'> + <name>guest</name> + <uuid>1ccfd97d-5eb4-478a-bbe6-88d254c16db7</uuid> + <memory unit='KiB'>1048576</memory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='aarch64' machine='virt'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-aarch64</emulator> + <controller type='usb' model='none'/> + <memballoon model='none'/> + <controller type='pci' index='0' model='pcie-root'/> + <controller type='pci' index='1' model='pcie-expander-bus'> + <model name='pxb-pcie'/> + <target busNr='252'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0'/> + </controller> + <controller type='pci' index='2' model='pcie-expander-bus'> + <model name='pxb-pcie'/> + <target busNr='248'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/> + </controller> + <controller type='pci' index='3' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='21' port='0x0'/> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> + </controller> + <controller type='pci' index='4' model='pcie-root-port'> + <model name='pcie-root-port'/> + <target chassis='22' port='0xa8'/> + <address type='pci' domain='0x0000' bus='0x02' slot='0x00' function='0x0'/> + </controller> + <rng model='virtio'> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x03' slot='0x00' function='0x0'/> + </rng> + <rng model='virtio'> + <backend model='random'>/dev/urandom</backend> + <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> + </rng> + <iommu model='smmuv3Dev'> + <driver parentIdx='1'/> + </iommu> + <iommu model='smmuv3Dev'> + <driver parentIdx='2'/> + </iommu> + </devices> +</domain> diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c index 99e2e1dc93..fe50df97af 100644 --- a/tests/qemuxmlconftest.c +++ b/tests/qemuxmlconftest.c @@ -2964,6 +2964,7 @@ mymain(void) DO_TEST_CAPS_LATEST_ABI_UPDATE("intel-iommu-eim-autoadd"); DO_TEST_CAPS_LATEST_ABI_UPDATE("intel-iommu-eim-autoadd-v2"); DO_TEST_CAPS_ARCH_LATEST("iommu-smmuv3", "aarch64"); + DO_TEST_CAPS_ARCH_LATEST("iommu-smmuv3Dev", "aarch64"); DO_TEST_CAPS_LATEST("virtio-iommu-x86_64"); DO_TEST_CAPS_ARCH_LATEST("virtio-iommu-aarch64", "aarch64"); DO_TEST_CAPS_LATEST_PARSE_ERROR("virtio-iommu-wrong-machine"); -- 2.43.0
participants (4)
-
Daniel P. Berrangé -
Ján Tomko -
Nathan Chen -
nathanc＠nvidia.com