[PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback

Shameer Kolothum posted 27 patches 1 month, 2 weeks ago
There is a newer version of this series
[PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Shameer Kolothum 1 month, 2 weeks ago
Here we return the IOMMU address space if the device has S1 translation
enabled by Guest. Otherwise return system address space.

Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
---
 hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
index 790887ac31..f4e01fba6d 100644
--- a/hw/arm/smmuv3-accel.c
+++ b/hw/arm/smmuv3-accel.c
@@ -387,6 +387,26 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
     }
 }
 
+static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void *opaque,
+                                              int devfn)
+{
+    SMMUState *bs = opaque;
+    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
+    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
+    SMMUDevice *sdev = &accel_dev->sdev;
+
+    /*
+     * If the assigned vfio-pci dev has S1 translation enabled by
+     * Guest, return IOMMU address space for MSI translation.
+     * Otherwise, return system address space.
+     */
+    if (accel_dev->s1_hwpt) {
+        return &sdev->as;
+    } else {
+        return &address_space_memory;
+    }
+}
+
 static bool smmuv3_accel_pdev_allowed(PCIDevice *pdev, bool *vfio_pci)
 {
 
@@ -475,6 +495,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_address_space = smmuv3_accel_find_msi_as,
 };
 
 void smmuv3_accel_init(SMMUv3State *s)
-- 
2.43.0
Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Eric Auger 3 weeks, 4 days ago
Hi Shameer,

On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> Here we return the IOMMU address space if the device has S1 translation
> enabled by Guest. Otherwise return system address space.
>
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> ---
>  hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 790887ac31..f4e01fba6d 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -387,6 +387,26 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
>      }
>  }
>  
> +static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void *opaque,
> +                                              int devfn)
> +{
> +    SMMUState *bs = opaque;
> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
> +    SMMUDevice *sdev = &accel_dev->sdev;
> +
> +    /*
> +     * If the assigned vfio-pci dev has S1 translation enabled by
> +     * Guest, return IOMMU address space for MSI translation.
> +     * Otherwise, return system address space.
> +     */
> +    if (accel_dev->s1_hwpt) {
> +        return &sdev->as;
> +    } else {
> +        return &address_space_memory;
> +    }
At the moment I don't understand this code either. In case of emulated
device it then returns address_space_memory whereas I would have
expected the opposite. I definitively need to trace things ;-)

Thanks

Eric
> +}
> +
>  static bool smmuv3_accel_pdev_allowed(PCIDevice *pdev, bool *vfio_pci)
>  {
>  
> @@ -475,6 +495,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_address_space = smmuv3_accel_find_msi_as,
>  };
>  
>  void smmuv3_accel_init(SMMUv3State *s)
RE: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Shameer Kolothum 3 weeks, 3 days ago
Hi Eric,

> -----Original Message-----
> From: Eric Auger <eric.auger@redhat.com>
> Sent: 20 October 2025 17:44
> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
> arm@nongnu.org; qemu-devel@nongnu.org
> Cc: peter.maydell@linaro.org; Jason Gunthorpe <jgg@nvidia.com>; Nicolin
> Chen <nicolinc@nvidia.com>; ddutile@redhat.com; berrange@redhat.com;
> Nathan Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
> smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> shameerkolothum@gmail.com
> Subject: Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of
> get_msi_address_space() callback
> 
> External email: Use caution opening links or attachments
> 
> 
> Hi Shameer,
> 
> On 9/29/25 3:36 PM, Shameer Kolothum wrote:
> > Here we return the IOMMU address space if the device has S1 translation
> > enabled by Guest. Otherwise return system address space.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> > ---
> >  hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > index 790887ac31..f4e01fba6d 100644
> > --- a/hw/arm/smmuv3-accel.c
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -387,6 +387,26 @@ static void
> smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
> >      }
> >  }
> >
> > +static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void
> *opaque,
> > +                                              int devfn)
> > +{
> > +    SMMUState *bs = opaque;
> > +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> > +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
> bus, devfn);
> > +    SMMUDevice *sdev = &accel_dev->sdev;
> > +
> > +    /*
> > +     * If the assigned vfio-pci dev has S1 translation enabled by
> > +     * Guest, return IOMMU address space for MSI translation.
> > +     * Otherwise, return system address space.
> > +     */
> > +    if (accel_dev->s1_hwpt) {
> > +        return &sdev->as;
> > +    } else {
> > +        return &address_space_memory;
> > +    }
> At the moment I don't understand this code either. In case of emulated
> device it then returns address_space_memory whereas I would have
> expected the opposite. I definitively need to trace things ;-)

We have,
[VIRT_GIC_ITS] =            { 0x08080000, 0x00020000 },

I added a few prints in kvm_arch_fixup_msi_route() so that it may help
to understand how the translation of MSI doorbell is performed here.

If we return IOMMU addr space(&sdev->as) here,

kvm_arch_fixup_msi_route: MSI IOVA=0xffbf0040 msi_addr_lo=0xffbf0040 msi_addr_hi=0x0
kvm_arch_fixup_msi_route: Translated doorbell_gpa= 0x8090040
kvm_arch_fixup_msi_route: ret:MSI IOVA=0xffbf0040 translated: msi_addr_lo=0x8090040 msi_addr_hi=0x0

It gets the correct vITS gpA address after the translation through address_space_translate().

Since host uses the (MSI_IOVA_BASE, MSI_IOVA_LENGTH) for ITS doorbell mapping 
and using IORT RMR we make sure there is an identity mapping for that range, it all
works fine.

Now, suppose if we return system addr space(&address_space_memory):

kvm_arch_fixup_msi_route: MSI IOVA=0xffbf0040 msi_addr_lo 0xffbf0040 msi_addr_hi 0x0
kvm_arch_fixup_msi_route:  address_space_memory, nothing to do, return

And the device doorbell gets configured with gIOVA 0xffbf0040 instead of the vITS gPA
as Nicolin explained in the other thread.

Hope this helps.

Thanks,
Shameer

Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Eric Auger 3 weeks, 3 days ago
Hi Shameer,

On 10/21/25 10:15 AM, Shameer Kolothum wrote:
> Hi Eric,
>
>> -----Original Message-----
>> From: Eric Auger <eric.auger@redhat.com>
>> Sent: 20 October 2025 17:44
>> To: Shameer Kolothum <skolothumtho@nvidia.com>; qemu-
>> arm@nongnu.org; qemu-devel@nongnu.org
>> Cc: peter.maydell@linaro.org; Jason Gunthorpe <jgg@nvidia.com>; Nicolin
>> Chen <nicolinc@nvidia.com>; ddutile@redhat.com; berrange@redhat.com;
>> Nathan Chen <nathanc@nvidia.com>; Matt Ochs <mochs@nvidia.com>;
>> smostafa@google.com; wangzhou1@hisilicon.com;
>> jiangkunkun@huawei.com; jonathan.cameron@huawei.com;
>> zhangfei.gao@linaro.org; zhenzhong.duan@intel.com; yi.l.liu@intel.com;
>> shameerkolothum@gmail.com
>> Subject: Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of
>> get_msi_address_space() callback
>>
>> External email: Use caution opening links or attachments
>>
>>
>> Hi Shameer,
>>
>> On 9/29/25 3:36 PM, Shameer Kolothum wrote:
>>> Here we return the IOMMU address space if the device has S1 translation
>>> enabled by Guest. Otherwise return system address space.
>>>
>>> Signed-off-by: Shameer Kolothum
>> <shameerali.kolothum.thodi@huawei.com>
>>> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
>>> ---
>>>  hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
>>>  1 file changed, 21 insertions(+)
>>>
>>> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
>>> index 790887ac31..f4e01fba6d 100644
>>> --- a/hw/arm/smmuv3-accel.c
>>> +++ b/hw/arm/smmuv3-accel.c
>>> @@ -387,6 +387,26 @@ static void
>> smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
>>>      }
>>>  }
>>>
>>> +static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void
>> *opaque,
>>> +                                              int devfn)
>>> +{
>>> +    SMMUState *bs = opaque;
>>> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
>>> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus,
>> bus, devfn);
>>> +    SMMUDevice *sdev = &accel_dev->sdev;
>>> +
>>> +    /*
>>> +     * If the assigned vfio-pci dev has S1 translation enabled by
>>> +     * Guest, return IOMMU address space for MSI translation.
>>> +     * Otherwise, return system address space.
>>> +     */
>>> +    if (accel_dev->s1_hwpt) {
>>> +        return &sdev->as;
>>> +    } else {
>>> +        return &address_space_memory;
>>> +    }
>> At the moment I don't understand this code either. In case of emulated
>> device it then returns address_space_memory whereas I would have
>> expected the opposite. I definitively need to trace things ;-)
Thank you for the traces!
> We have,
> [VIRT_GIC_ITS] =            { 0x08080000, 0x00020000 },
>
> I added a few prints in kvm_arch_fixup_msi_route() so that it may help
> to understand how the translation of MSI doorbell is performed here.
>
> If we return IOMMU addr space(&sdev->as) here,
>
> kvm_arch_fixup_msi_route: MSI IOVA=0xffbf0040 msi_addr_lo=0xffbf0040 msi_addr_hi=0x0
so this gIOVA
> kvm_arch_fixup_msi_route: Translated doorbell_gpa= 0x8090040
> kvm_arch_fixup_msi_route: ret:MSI IOVA=0xffbf0040 translated: msi_addr_lo=0x8090040 msi_addr_hi=0x0
>
> It gets the correct vITS gpA address after the translation through address_space_translate().
I agree it needs to be translated into the vITS doorbell reg.
>
> Since host uses the (MSI_IOVA_BASE, MSI_IOVA_LENGTH) for ITS doorbell mapping 
> and using IORT RMR we make sure there is an identity mapping for that range, it all
> works fine.
>
> Now, suppose if we return system addr space(&address_space_memory):
>
> kvm_arch_fixup_msi_route: MSI IOVA=0xffbf0040 msi_addr_lo 0xffbf0040 msi_addr_hi 0x0
> kvm_arch_fixup_msi_route:  address_space_memory, nothing to do, return
>
> And the device doorbell gets configured with gIOVA 0xffbf0040 instead of the vITS gPA
> as Nicolin explained in the other thread.
I agree that for MSI support you must remy on the IOMMU MR translate
function, even for VFIO devices.

Thanks

Eric
>
> Hope this helps.
>
> Thanks,
> Shameer
>
Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Nicolin Chen 4 weeks, 1 day ago
On Mon, Sep 29, 2025 at 02:36:28PM +0100, Shameer Kolothum wrote:
> Here we return the IOMMU address space if the device has S1 translation
> enabled by Guest. Otherwise return system address space.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>

Apart from the naming that Jonathan pointed out,
 
Reviewed-by: Nicolin Chen <nicolinc@nvidia.com>
Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Jonathan Cameron via 1 month, 2 weeks ago
On Mon, 29 Sep 2025 14:36:28 +0100
Shameer Kolothum <skolothumtho@nvidia.com> wrote:

> Here we return the IOMMU address space if the device has S1 translation
> enabled by Guest. Otherwise return system address space.
> 
> Signed-off-by: Shameer Kolothum <shameerali.kolothum.thodi@huawei.com>
> Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
Naming question inline.

> ---
>  hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> index 790887ac31..f4e01fba6d 100644
> --- a/hw/arm/smmuv3-accel.c
> +++ b/hw/arm/smmuv3-accel.c
> @@ -387,6 +387,26 @@ static void smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
>      }
>  }
>  
> +static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void *opaque,

Why find rather than get for naming?

> +                                              int devfn)
> +{
> +    SMMUState *bs = opaque;
> +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus, devfn);
> +    SMMUDevice *sdev = &accel_dev->sdev;
> +
> +    /*
> +     * If the assigned vfio-pci dev has S1 translation enabled by
> +     * Guest, return IOMMU address space for MSI translation.
> +     * Otherwise, return system address space.
> +     */
> +    if (accel_dev->s1_hwpt) {
> +        return &sdev->as;
> +    } else {
> +        return &address_space_memory;
> +    }
> +}
> +
>  static bool smmuv3_accel_pdev_allowed(PCIDevice *pdev, bool *vfio_pci)
>  {
>  
> @@ -475,6 +495,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_address_space = smmuv3_accel_find_msi_as,
>  };
>  
>  void smmuv3_accel_init(SMMUv3State *s)
RE: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of get_msi_address_space() callback
Posted by Shameer Kolothum 1 month, 1 week ago

> -----Original Message-----
> From: Jonathan Cameron <jonathan.cameron@huawei.com>
> Sent: 29 September 2025 17:51
> To: Shameer Kolothum <skolothumtho@nvidia.com>
> Cc: qemu-arm@nongnu.org; qemu-devel@nongnu.org;
> eric.auger@redhat.com; peter.maydell@linaro.org; Jason Gunthorpe
> <jgg@nvidia.com>; Nicolin Chen <nicolinc@nvidia.com>; ddutile@redhat.com;
> berrange@redhat.com; Nathan Chen <nathanc@nvidia.com>; Matt Ochs
> <mochs@nvidia.com>; smostafa@google.com; wangzhou1@hisilicon.com;
> jiangkunkun@huawei.com; zhangfei.gao@linaro.org;
> zhenzhong.duan@intel.com; yi.l.liu@intel.com;
> shameerkolothum@gmail.com
> Subject: Re: [PATCH v4 12/27] hw/arm/smmuv3-accel: Make use of
> get_msi_address_space() callback
> 
> External email: Use caution opening links or attachments
> 
> 
> On Mon, 29 Sep 2025 14:36:28 +0100
> Shameer Kolothum <skolothumtho@nvidia.com> wrote:
> 
> > Here we return the IOMMU address space if the device has S1 translation
> > enabled by Guest. Otherwise return system address space.
> >
> > Signed-off-by: Shameer Kolothum
> <shameerali.kolothum.thodi@huawei.com>
> > Signed-off-by: Shameer Kolothum <skolothumtho@nvidia.com>
> Naming question inline.
> 
> > ---
> >  hw/arm/smmuv3-accel.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/hw/arm/smmuv3-accel.c b/hw/arm/smmuv3-accel.c
> > index 790887ac31..f4e01fba6d 100644
> > --- a/hw/arm/smmuv3-accel.c
> > +++ b/hw/arm/smmuv3-accel.c
> > @@ -387,6 +387,26 @@ static void
> smmuv3_accel_unset_iommu_device(PCIBus *bus, void *opaque,
> >      }
> >  }
> >
> > +static AddressSpace *smmuv3_accel_find_msi_as(PCIBus *bus, void
> *opaque,
> 
> Why find rather than get for naming?

I just followed the get_address_space() convention. Will revisit.

Thanks,
Shameer
> 
> > +                                              int devfn)
> > +{
> > +    SMMUState *bs = opaque;
> > +    SMMUPciBus *sbus = smmu_get_sbus(bs, bus);
> > +    SMMUv3AccelDevice *accel_dev = smmuv3_accel_get_dev(bs, sbus, bus,
> devfn);
> > +    SMMUDevice *sdev = &accel_dev->sdev;
> > +
> > +    /*
> > +     * If the assigned vfio-pci dev has S1 translation enabled by
> > +     * Guest, return IOMMU address space for MSI translation.
> > +     * Otherwise, return system address space.
> > +     */
> > +    if (accel_dev->s1_hwpt) {
> > +        return &sdev->as;
> > +    } else {
> > +        return &address_space_memory;
> > +    }
> > +}
> > +
> >  static bool smmuv3_accel_pdev_allowed(PCIDevice *pdev, bool *vfio_pci)
> >  {
> >
> > @@ -475,6 +495,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_address_space = smmuv3_accel_find_msi_as,
> >  };
> >
> >  void smmuv3_accel_init(SMMUv3State *s)