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

[Eric Auger <eric.auger@redhat.com>]

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
>