| Message ID | 20230622214845.3980-8-joao.m.martins@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio: VFIO migration support with vIOMMU | expand |
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 >
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 >>
Hi Joao, On 6/22/23 23: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. VFIOAddressSpace > 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; > 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; nit can be set at init. > + 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"); Thanks Eric
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; 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");
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(-)