Re: [PATCH v4 07/15] vfio/common: Track whether DMA Translation is enabled on the vIOMMU

[Joao Martins <joao.m.martins@oracle.com>]

On 09/07/2023 16:10, Avihai Horon wrote:
> On 23/06/2023 0:48, Joao Martins wrote:
>> vfio_get_group() allocates and fills the group/container/space on
>> success which will store the AddressSpace inside the VFIOSpace struct.
>> Use the newly added pci_device_iommu_get_attr() to see if DMA
>> translation is enabled or not. Assume that by default it is enabled.
>>
>> Today, this means only intel-iommu supports it.
>>
>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com>
>> ---
>>   include/hw/vfio/vfio-common.h |  1 +
>>   hw/vfio/pci.c                 | 15 ++++++++++++++-
>>   2 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
>> index eed244f25f34..f41860988d6b 100644
>> --- a/include/hw/vfio/vfio-common.h
>> +++ b/include/hw/vfio/vfio-common.h
>> @@ -70,6 +70,7 @@ typedef struct VFIOMigration {
>>
>>   typedef struct VFIOAddressSpace {
>>       AddressSpace *as;
>> +    bool no_dma_translation;
> 
> I find this negation a bit confusing, especially when below local variable is
> "dma_translation" (there is also double negation in next patch).
> Maybe rename to "dma_translation" or "have_dma_translation"?
> 

Good idea -- I can switch to that.

> Thanks.
> 
>>       QLIST_HEAD(, VFIOContainer) containers;
>>       QLIST_ENTRY(VFIOAddressSpace) list;
>>   } VFIOAddressSpace;
>> diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
>> index 73874a94de12..8a98e6ffc480 100644
>> --- a/hw/vfio/pci.c
>> +++ b/hw/vfio/pci.c
>> @@ -2900,6 +2900,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>       VFIOPCIDevice *vdev = VFIO_PCI(pdev);
>>       VFIODevice *vbasedev = &vdev->vbasedev;
>>       VFIODevice *vbasedev_iter;
>> +    VFIOAddressSpace *space;
>>       VFIOGroup *group;
>>       char *tmp, *subsys, group_path[PATH_MAX], *group_name;
>>       Error *err = NULL;
>> @@ -2907,7 +2908,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>       struct stat st;
>>       int groupid;
>>       int i, ret;
>> -    bool is_mdev;
>> +    bool is_mdev, dma_translation;
>>       char uuid[UUID_FMT_LEN];
>>       char *name;
>>
>> @@ -2961,6 +2962,18 @@ static void vfio_realize(PCIDevice *pdev, Error **errp)
>>           goto error;
>>       }
>>
>> +    space = group->container->space;
>> +
>> +    /*
>> +     * Support for toggling DMA translation is optional.
>> +     * By default, DMA translation is assumed to be enabled i.e.
>> +     * space::no_dma_translation is 0.
>> +     */
>> +    dma_translation = true;
>> +    pci_device_iommu_get_attr(pdev, IOMMU_ATTR_DMA_TRANSLATION,
>> +                              &dma_translation);
>> +    space->no_dma_translation = !dma_translation;
>> +
>>       QLIST_FOREACH(vbasedev_iter, &group->device_list, next) {
>>           if (strcmp(vbasedev_iter->name, vbasedev->name) == 0) {
>>               error_setg(errp, "device is already attached");
>> -- 
>> 2.17.2
>>