On a Wednesday in 2025, Nathan Chen via Devel wrote:
Add support for parsing multiple IOMMU devices from the VM definition when "smmuv3" is the IOMMU model.Signed-off-by: Nathan Chen <[email protected]> --- src/conf/domain_conf.c | 84 +++++++++++++++----- src/conf/domain_conf.h | 9 ++- src/conf/domain_validate.c | 33 +++++--- src/conf/schemas/domaincommon.rng | 4 +- src/libvirt_private.syms | 2 + src/qemu/qemu_alias.c | 15 ++-- src/qemu/qemu_command.c | 127 +++++++++++++++--------------- src/qemu/qemu_domain.c | 2 +- src/qemu/qemu_domain_address.c | 5 +- src/qemu/qemu_driver.c | 8 +- src/qemu/qemu_postparse.c | 11 +-- src/qemu/qemu_validate.c | 2 +- 12 files changed, 184 insertions(+), 118 deletions(-) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index c56c321a6e..97fe4267ec 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6244,84 +6244,85 @@ 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->niommus == 0)
return 0;
This condition is no longer needed - the for cycle already checks def->niommus.
- 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->iommus[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;
break; instead of return; - even though there can practially be only one device, the function iterates through all of them and should not exit early
+ 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;
Same here
+ case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + /* There is no -device for SMMUv3, so nothing to be done here */ + return 0;
And here.
- 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: - /* There is no -device for SMMUv3, so nothing to be done here */ - 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;
Here too.
- 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;
diff --git a/src/qemu/qemu_postparse.c b/src/qemu/qemu_postparse.c
index fd27f8be27..3b417ea5d0 100644
--- a/src/qemu/qemu_postparse.c
+++ b/src/qemu/qemu_postparse.c
@@ -1470,7 +1470,7 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
}
}
- if (addIOMMU && !def->iommu &&
+ if (addIOMMU && !def->iommus && def->niommus == 0 &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_INTEL_IOMMU) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_INTREMAP) &&
virQEMUCapsGet(qemuCaps, QEMU_CAPS_INTEL_IOMMU_EIM)) {
@@ -1482,7 +1482,8 @@ qemuDomainDefAddDefaultDevices(virQEMUDriver *driver,
iommu->intremap = VIR_TRISTATE_SWITCH_ON;
iommu->eim = VIR_TRISTATE_SWITCH_ON;
- def->iommu = g_steal_pointer(&iommu);
+ def->iommus = g_new0(virDomainIOMMUDef *, 1);
+ def->iommus[def->niommus++] = g_steal_pointer(&iommu);
}
if (qemuDomainDefAddDefaultAudioBackend(driver, def) < 0)
@@ -1558,9 +1559,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->iommus && def->niommus == 1 &&
+ (def->iommus[0]->intremap == VIR_TRISTATE_SWITCH_ON ||
+ qemuDomainNeedsIOMMUWithEIM(def)) &&
qemuDomainNeedsIOMMUWithEIM was aligned properly originally.
def->features[VIR_DOMAIN_FEATURE_IOAPIC] == VIR_DOMAIN_IOAPIC_NONE) {
def->features[VIR_DOMAIN_FEATURE_IOAPIC] = VIR_DOMAIN_IOAPIC_QEMU;
}
Reviewed-by: Ján Tomko <[email protected]> Jano
signature.asc
Description: PGP signature
