> -----Original Message-----
> From: Cédric Le Goater <[email protected]>
> Sent: 11 December 2025 14:04
> To: Shameer Kolothum <[email protected]>; qemu-
> [email protected]; [email protected]
> Cc: [email protected]; [email protected]; Jason Gunthorpe
> <[email protected]>; Nicolin Chen <[email protected]>; [email protected];
> [email protected]; Nathan Chen <[email protected]>; Matt Ochs
> <[email protected]>; [email protected]; [email protected];
> [email protected]; [email protected];
> [email protected]; [email protected]; [email protected];
> Krishnakant Jaju <[email protected]>
> Subject: Re: [PATCH v6 17/33] hw/arm/smmuv3: Add support for providing a
> direct MSI doorbell GPA
> 
> External email: Use caution opening links or attachments
> 
> 
> 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.

Ok.

> >
> > Signed-off-by: Shameer Kolothum <[email protected]>
> > ---
> >   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 ?

Could do I guess. I placed it here thinking pre_plug is before realize(),
allowing an early exit. But probably realize is better as that will cover
all future machine types as well(if any).
 
> 
> > +            }
> > +        }
> >       }
> >   }
> >
> > @@ -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.

Looking at this again, I think this can be moved to pre_plug handler.

Thanks,
Shameer

Reply via email to