[PATCH 0/3] IOMMU improvements
These are inspired by: https://bugzilla.redhat.com/show_bug.cgi?id=2101633 While there are some missing info to fix the actual but, I've noticed couple of areas for improvement while investigating the bug. Michal Prívozník (3): qemu_domain_address: Drop needless virDomainIOMMUModel typecast docs: Document <address/> for IOMMU device domain_validate: Disallow non-virtio IOMMU with an <address/> docs/formatdomain.rst | 3 +++ src/conf/domain_validate.c | 25 ++++++++++++++++++++++++- src/qemu/qemu_domain_address.c | 4 ++-- 3 files changed, 29 insertions(+), 3 deletions(-) -- 2.35.1
There are two places where the @model member of _virDomainIOMMUDef struct is typecasted to virDomainIOMMUModel which is completely unnecessary because the struct already defines the member of that type. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain_address.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 753733d1b9..026be99ba9 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1003,7 +1003,7 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDef *dev, break; case VIR_DOMAIN_DEVICE_IOMMU: - switch ((virDomainIOMMUModel) dev->data.iommu->model) { + switch (dev->data.iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: return virtioFlags | VIR_PCI_CONNECT_INTEGRATED; @@ -2384,7 +2384,7 @@ qemuDomainAssignDevicePCISlots(virDomainDef *def, if (def->iommu) { virDomainIOMMUDef *iommu = def->iommu; - switch ((virDomainIOMMUModel) iommu->model) { + switch (iommu->model) { case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: if (virDeviceInfoPCIAddressIsWanted(&iommu->info) && qemuDomainPCIAddressReserveNextAddr(addrs, &iommu->info) < 0) { -- 2.35.1
The commit v8.3.0-rc1~199 introduced <address/> to <iommu/> device. And while it updated the RNG it forgot to update the docs. Fix that. Fixes: b0eb1e193f5db033d0fbbf91ff71a121066ad77c Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- docs/formatdomain.rst | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 62a94890f0..a4e3c39928 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -8083,6 +8083,9 @@ Example: mapping larger iova addresses in the guest. :since:`Since 6.5.0` (QEMU/KVM only) +The ``virtio`` IOMMU devices can further have ``address`` element as described +in `Device addresses`_ (address has to by type of ``pci``). + Vsock ~~~~~ -- 2.35.1
Per v8.3.0-rc1~199 it's only a virtio IOMMU that can have <address/>. The rest (Intel and SMMUv3) are system devices and thus have no address associated with them. However, this assumption is never checked for. Fixes: b0eb1e193f5db033d0fbbf91ff71a121066ad77c Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/conf/domain_validate.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index c977c39144..d35451c26a 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2582,6 +2582,27 @@ virDomainGraphicsDefValidate(const virDomainDef *def, return 0; } +static int +virDomainIOMMUDefValidate(const virDomainIOMMUDef *iommu) +{ + switch (iommu->model) { + case VIR_DOMAIN_IOMMU_MODEL_INTEL: + case VIR_DOMAIN_IOMMU_MODEL_SMMUV3: + if (iommu->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { + virReportError(VIR_ERR_XML_ERROR, + _("iommu model '%s' can't have address"), + virDomainIOMMUModelTypeToString(iommu->model)); + return -1; + } + break; + + case VIR_DOMAIN_IOMMU_MODEL_VIRTIO: + case VIR_DOMAIN_IOMMU_MODEL_LAST: + } + + return 0; +} + static int virDomainDeviceInfoValidate(const virDomainDeviceDef *dev) { @@ -2683,6 +2704,9 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_GRAPHICS: return virDomainGraphicsDefValidate(def, dev->data.graphics); + case VIR_DOMAIN_DEVICE_IOMMU: + return virDomainIOMMUDefValidate(dev->data.iommu); + case VIR_DOMAIN_DEVICE_LEASE: case VIR_DOMAIN_DEVICE_WATCHDOG: case VIR_DOMAIN_DEVICE_HUB: @@ -2690,7 +2714,6 @@ virDomainDeviceDefValidateInternal(const virDomainDeviceDef *dev, case VIR_DOMAIN_DEVICE_NVRAM: case VIR_DOMAIN_DEVICE_TPM: case VIR_DOMAIN_DEVICE_PANIC: - case VIR_DOMAIN_DEVICE_IOMMU: case VIR_DOMAIN_DEVICE_NONE: case VIR_DOMAIN_DEVICE_LAST: break; -- 2.35.1
On a Monday in 2022, Michal Privoznik wrote:
These are inspired by:
https://bugzilla.redhat.com/show_bug.cgi?id=2101633
While there are some missing info to fix the actual but, I've noticed couple of areas for improvement while investigating the bug.
Michal Prívozník (3): qemu_domain_address: Drop needless virDomainIOMMUModel typecast docs: Document <address/> for IOMMU device domain_validate: Disallow non-virtio IOMMU with an <address/>
docs/formatdomain.rst | 3 +++ src/conf/domain_validate.c | 25 ++++++++++++++++++++++++- src/qemu/qemu_domain_address.c | 4 ++-- 3 files changed, 29 insertions(+), 3 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik