Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a direct MSI doorbell GPA

["Cédric Le Goater" <clg@redhat.com>]

On 11/20/25 14:21, Shameer Kolothum wrote:
> Accelerated SMMUv3 instances rely on the physical SMMUv3 for nested
> translation (Guest Stage-1, Host Stage-2). In this mode the guest’s
> Stage-1 tables are programmed directly into hardware, and QEMU should
> not attempt to walk them for translation since doing so is not reliably
> safe. For vfio-pci endpoints behind such a vSMMU, the only translation
> QEMU is responsible for is the MSI doorbell used during KVM MSI setup.
> 
> Add a device property to carry the MSI doorbell GPA from the virt
> machine, and expose it through a new get_msi_direct_gpa PCIIOMMUOp.
> kvm_arch_fixup_msi_route() can then use this GPA directly instead of
> attempting a software walk of guest translation tables.
> 
> This enables correct MSI routing with accelerated SMMUv3 while avoiding
> unsafe accesses to page tables.
> 
> For meaningful use of vfio-pci devices with accelerated SMMUv3, both KVM
> and a kernel irqchip are required. Enforce this requirement when accel=on
> is selected.

There are multiple changes in one patch. I suggest splitting.

> 
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>   hw/arm/smmuv3-accel.c   | 10 ++++++++++
>   hw/arm/smmuv3.c         |  2 ++
>   hw/arm/virt.c           | 22 ++++++++++++++++++++++
>   include/hw/arm/smmuv3.h |  1 +
>   4 files changed, 35 insertions(+)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 65b577f49a..8f7c0cda05 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -392,6 +392,15 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
>       }
>   }
>   
> +static uint64_t smmuv3_accel_get_msi_gpa(PCIBus *bus, void *opaque, int devfn)
> +{
> +    SMMUState *bs = opaque;
> +    SMMUv3State *s = ARM_SMMUV3(bs);
> +
> +    g_assert(s->msi_gpa);
> +    return s->msi_gpa;
> +}
> +
>   /*
>    * Only allow PCIe bridges, pxb-pcie roots, and GPEX roots so vfio-pci
>    * endpoints can sit downstream. Accelerated SMMUv3 requires a vfio-pci
> @@ -496,6 +505,7 @@ static const PCIIOMMUOps smmuv3_accel_ops = {
>       .get_viommu_flags = smmuv3_accel_get_viommu_flags,
>       .set_iommu_device = smmuv3_accel_set_iommu_device,
>       .unset_iommu_device = smmuv3_accel_unset_iommu_device,
> +    .get_msi_direct_gpa = smmuv3_accel_get_msi_gpa,
>   };
>   
>   /* Based on SMUUv3 GPBA.ABORT configuration, attach a corresponding HWPT */
> diff --git a/hw/arm/smmuv3.c b/hw/arm/smmuv3.c
> index 42c60b1ec8..f02e3ee46c 100644
> --- a/hw/arm/smmuv3.c
> +++ b/hw/arm/smmuv3.c
> @@ -1998,6 +1998,8 @@ static const Property smmuv3_properties[] = {
>        * Defaults to stage 1
>        */
>       DEFINE_PROP_STRING("stage", SMMUv3State, stage),
> +    /* GPA of MSI doorbell, for SMMUv3 accel use. */
> +    DEFINE_PROP_UINT64("msi-gpa", SMMUv3State, msi_gpa, 0),
>   };
>   
>   static void smmuv3_instance_init(Object *obj)
> diff --git a/hw/arm/virt.c b/hw/arm/virt.c
> index 25fb2bab56..ea3231543a 100644
> --- a/hw/arm/virt.c
> +++ b/hw/arm/virt.c
> @@ -3052,6 +3052,14 @@ static void virt_machine_device_pre_plug_cb(HotplugHandler *hotplug_dev,
>               /* The new SMMUv3 device is specific to the PCI bus */
>               object_property_set_bool(OBJECT(dev), "smmu_per_bus", true, NULL);
>           }
> +        if (object_property_find(OBJECT(dev), "accel") &&
> +            object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +            if (!kvm_enabled() || !kvm_irqchip_in_kernel()) {
> +                error_setg(errp, "SMMUv3 accel=on requires KVM with "
> +                           "kernel-irqchip=on support");
> +                    return;

Couldn't these checks be done in the "smmuv3-accel" model realize
handler instead ?

  
> +            }
> +        }
>       }
>   }
>   
> @@ -3088,6 +3096,20 @@ static void virt_machine_device_plug_cb(HotplugHandler *hotplug_dev,
>               }
>   
>               create_smmuv3_dev_dtb(vms, dev, bus);
> +            if (object_property_find(OBJECT(dev), "accel") &&
> +                object_property_get_bool(OBJECT(dev), "accel", &error_abort)) {
> +                hwaddr db_start;
> +
> +                if (vms->msi_controller == VIRT_MSI_CTRL_ITS) {
> +                    /* GITS_TRANSLATER page + offset */
> +                    db_start = base_memmap[VIRT_GIC_ITS].base + 0x10000 + 0x40;
> +                } else {
> +                    /* MSI_SETSPI_NS page + offset */
> +                    db_start = base_memmap[VIRT_GIC_V2M].base + 0x40;
> +                }
> +                object_property_set_uint(OBJECT(dev), "msi-gpa", db_start,
> +                                         &error_abort);

IIRC, this plug handler is called after the device realize handler
and setting properties after realize is strongly discouraged.

C.

> +            }
>           }
>       }
>   
> diff --git a/include/hw/arm/smmuv3.h b/include/hw/arm/smmuv3.h
> index e54ece2d38..5616a8a2be 100644
> --- a/include/hw/arm/smmuv3.h
> +++ b/include/hw/arm/smmuv3.h
> @@ -67,6 +67,7 @@ struct SMMUv3State {
>       /* SMMU has HW accelerator support for nested S1 + s2 */
>       bool accel;
>       struct SMMUv3AccelState *s_accel;
> +    uint64_t msi_gpa;
>   };
>   
>   typedef enum {