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 <[email protected]>
> ---
> 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 :|
