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

[Avihai Horon]

On 23/06/2023 0:48, Joao Martins wrote:
> External email: Use caution opening links or attachments
>
>
> 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"?

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