On Thu, May 15, 2025 at 01:36:41PM -0700, Nathan Chen via Devel wrote:
> Implement "iommufdId" and "iommufdFd" attributes for "hostdev" devices that
> can be used to specify associated iommufd object and fd for externally opening
> the /dev/iommu and VFIO cdev, when launching a qemu VM.
What does it mean if we enable iommufd on the 'hostdev' without enabling
iommufd on the 'iommu' device, or vica-verca ?
IMHO it seems like we should be enabling iommufd once only.
>
> Signed-off-by: Nathan Chen <nath...@nvidia.com>
> ---
> src/conf/domain_conf.c | 31 +++++++++++++++++++++++++++++++
> src/conf/domain_conf.h | 2 ++
> src/conf/domain_validate.c | 14 ++++++++++++++
> src/conf/schemas/domaincommon.rng | 17 +++++++++++++++++
> src/qemu/qemu_command.c | 2 ++
> 5 files changed, 66 insertions(+)
>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 9330c47b7a..47b7e9acb1 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -13582,6 +13582,19 @@ virDomainVideoDefParseXML(virDomainXMLOption *xmlopt,
> return g_steal_pointer(&def);
> }
>
> +static void
> +virDomainHostdevDefIommufdParseXML(xmlXPathContextPtr ctxt,
> + char** iommufdId,
> + char** iommufdFd)
> +{
> + g_autofree char *iommufdIdtmp = virXPathString("string(./iommufdId)",
> ctxt);
> + g_autofree char *iommufdFdtmp = virXPathString("string(./iommufdFd)",
> ctxt);
> + if (iommufdIdtmp)
> + *iommufdId = g_steal_pointer(&iommufdIdtmp);
> + if (iommufdFdtmp)
> + *iommufdFd = g_steal_pointer(&iommufdFdtmp);
> +}
> +
> static virDomainHostdevDef *
> virDomainHostdevDefParseXML(virDomainXMLOption *xmlopt,
> xmlNodePtr node,
> @@ -13656,6 +13669,8 @@ virDomainHostdevDefParseXML(virDomainXMLOption
> *xmlopt,
> if (virDomainNetTeamingInfoParseXML(ctxt, &def->teaming) < 0)
> goto error;
>
> + virDomainHostdevDefIommufdParseXML(ctxt, &def->iommufdId,
> &def->iommufdFd);
> +
> return def;
>
> error:
> @@ -21193,6 +21208,16 @@
> virDomainHostdevDefCheckABIStability(virDomainHostdevDef *src,
> }
> }
>
> + if (src->iommufdId && dst->iommufdId) {
> + if (STRNEQ(src->iommufdId, dst->iommufdId))
> + return false;
> + }
> +
> + if (src->iommufdFd && dst->iommufdFd) {
> + if (STRNEQ(src->iommufdFd, dst->iommufdFd))
> + return false;
> + }
> +
> if (!virDomainDeviceInfoCheckABIStability(src->info, dst->info))
> return false;
>
> @@ -27511,6 +27536,12 @@ virDomainHostdevDefFormat(virBuffer *buf,
> if (def->shareable)
> virBufferAddLit(buf, "<shareable/>\n");
>
> + if (def->iommufdId) {
> + virBufferAsprintf(buf, "<iommufdId>%s</iommufdId>\n",
> def->iommufdId);
> + if (def->iommufdFd)
> + virBufferAsprintf(buf, "<iommufdFd>%s</iommufdFd>\n",
> def->iommufdFd);
> + }
> +
> virDomainDeviceInfoFormat(buf, def->info, flags |
> VIR_DOMAIN_DEF_FORMAT_ALLOW_BOOT
> |
> VIR_DOMAIN_DEF_FORMAT_ALLOW_ROM);
>
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index ce447e9823..78507d78b7 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -375,6 +375,8 @@ struct _virDomainHostdevDef {
> virDomainHostdevCaps caps;
> } source;
> virDomainNetTeamingInfo *teaming;
> + char *iommufdId;
> + char *iommufdFd;
> virDomainDeviceInfo *info; /* Guest address */
> };
>
> diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c
> index 88649ba0b9..292398b115 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -1401,6 +1401,20 @@ virDomainDefHostdevValidate(const virDomainDef *def)
> ramfbEnabled = true;
> }
> }
> +
> + /* Check for unsupported conditions:
> + * - A hostdev's iommufd fd number is defined but its iommufd id
> + * number is not
> + * - An iommu device is defined, iommufd is not defined,
> + * but a hostdev's iommufd id or fd are defined */
> + if ((!dev->iommufdId && dev->iommufdFd) ||
> + (def->iommu && def->niommus > 0 &&
> + !def->iommu[0]->iommufd &&
> + (dev->iommufdId || dev->iommufdFd))) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> + _("Unsupported hostdev iommufd configuration"));
> + return -1;
> + }
> }
>
> return 0;
> diff --git a/src/conf/schemas/domaincommon.rng
> b/src/conf/schemas/domaincommon.rng
> index 41db338c71..6c1f901eb4 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -6442,6 +6442,12 @@
> <optional>
> <ref name="address"/>
> </optional>
> + <optional>
> + <ref name="iommufdId"/>
> + </optional>
> + <optional>
> + <ref name="iommufdFd"/>
> + </optional>
> <optional>
> <element name="readonly">
> <empty/>
> @@ -7809,6 +7815,17 @@
> </element>
> </define>
>
> + <define name="iommufdId">
> + <element name="iommufdId">
> + <text/>
> + </element>
> + </define>
> + <define name="iommufdFd">
> + <element name="iommufdFd">
> + <text/>
> + </element>
> + </define>
> +
> <define name="deviceBoot">
> <element name="boot">
> <attribute name="order">
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index d683d0eef7..3a8b71a00a 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4724,6 +4724,8 @@ qemuBuildPCIHostdevDevProps(const virDomainDef *def,
> "S:failover_pair_id", failover_pair_id,
> "S:display", qemuOnOffAuto(pcisrc->display),
> "B:ramfb", ramfb,
> + "S:iommufd", dev->iommufdId,
> + "S:fd", dev->iommufdFd,
> NULL) < 0)
> return NULL;
>
> --
> 2.43.0
>
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 :|
