| Message ID | 20240712114704.8708-9-joao.m.martins@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | hw/iommufd: IOMMUFD Dirty Tracking | expand |
On 7/12/24 13:47, Joao Martins wrote: > Probe hardware dirty tracking support by querying device hw capabilities via > IOMMUFD_GET_HW_INFO. > > In preparation to using the dirty tracking UAPI, request dirty tracking in the > HWPT flags when the IOMMU supports dirty tracking. > > The auto domain logic allows different IOMMU domains to be created when DMA > dirty tracking is not desired (and VF can provide it) while others doesn't have > it and want the IOMMU capability. This is not used in this way here given how > VFIODevice migration capability checking takes place *after* the device > attachment. > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > include/hw/vfio/vfio-common.h | 1 + > hw/vfio/iommufd.c | 12 ++++++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 2dd468ce3c02..760f31d84ac8 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend; > > typedef struct VFIOIOASHwpt { > uint32_t hwpt_id; > + uint32_t hwpt_flags; > QLIST_HEAD(, VFIODevice) device_list; > QLIST_ENTRY(VFIOIOASHwpt) next; > } VFIOIOASHwpt; > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index d34dc88231ec..edc8f97d8f3d 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > } > } > > + /* > + * This is quite early and VFIODevice isn't yet fully initialized, > + * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty > + * tracking is going to be needed. > + */ > + if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) { > + flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; > + } > + > if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid, > container->ioas_id, flags, > IOMMU_HWPT_DATA_NONE, 0, NULL, > @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > > hwpt = g_malloc0(sizeof(*hwpt)); > hwpt->hwpt_id = hwpt_id; > + hwpt->hwpt_flags = flags; > QLIST_INIT(&hwpt->device_list); > > ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp); > @@ -267,6 +277,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > vbasedev->hwpt = hwpt; > QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); > QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); > + container->bcontainer.dirty_pages_supported |= > + (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING); > return true; > } > Could you please introduce in this patch helper : static bool iommufd_hwpt_dirty_tracking(VFIOIOASHwpt *hwpt) { return hwpt->hwpt_flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING; } Thanks, C.
Hi Joao, On 7/12/24 13:47, Joao Martins wrote: > Probe hardware dirty tracking support by querying device hw capabilities via > IOMMUFD_GET_HW_INFO. this is not what the patch brings. GET_HW_INFO is always in place. > > In preparation to using the dirty tracking UAPI, request dirty tracking in the > HWPT flags when the IOMMU supports dirty tracking. this is what the patch brings. > > The auto domain logic allows different IOMMU domains to be created when DMA > dirty tracking is not desired (and VF can provide it) while others doesn't have don't > it and want the IOMMU capability. This is not used in this way here given how > VFIODevice migration capability checking takes place *after* the device > attachment. Id on't understand the above sentence Eric > > Signed-off-by: Joao Martins <joao.m.martins@oracle.com> > --- > include/hw/vfio/vfio-common.h | 1 + > hw/vfio/iommufd.c | 12 ++++++++++++ > 2 files changed, 13 insertions(+) > > diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h > index 2dd468ce3c02..760f31d84ac8 100644 > --- a/include/hw/vfio/vfio-common.h > +++ b/include/hw/vfio/vfio-common.h > @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend; > > typedef struct VFIOIOASHwpt { > uint32_t hwpt_id; > + uint32_t hwpt_flags; > QLIST_HEAD(, VFIODevice) device_list; > QLIST_ENTRY(VFIOIOASHwpt) next; > } VFIOIOASHwpt; > diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c > index d34dc88231ec..edc8f97d8f3d 100644 > --- a/hw/vfio/iommufd.c > +++ b/hw/vfio/iommufd.c > @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > } > } > > + /* > + * This is quite early and VFIODevice isn't yet fully initialized, so what's the problem exactly with the above? > + * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty > + * tracking is going to be needed. > + */ > + if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) { > + flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; > + } > + > if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid, > container->ioas_id, flags, > IOMMU_HWPT_DATA_NONE, 0, NULL, > @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > > hwpt = g_malloc0(sizeof(*hwpt)); > hwpt->hwpt_id = hwpt_id; > + hwpt->hwpt_flags = flags; > QLIST_INIT(&hwpt->device_list); > > ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp); > @@ -267,6 +277,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, > vbasedev->hwpt = hwpt; > QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); > QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); > + container->bcontainer.dirty_pages_supported |= > + (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING); > return true; > } >
On 17/07/2024 13:27, Eric Auger wrote: > Hi Joao, > > On 7/12/24 13:47, Joao Martins wrote: >> Probe hardware dirty tracking support by querying device hw capabilities via >> IOMMUFD_GET_HW_INFO. > this is not what the patch brings. GET_HW_INFO is always in place. Yes. This is my mistake in squashing things as there was some shuffling going around on how we do GET_HW_INFO. and didn't adjust the right hand of this sentence. I'll rephrase it. >> >> In preparation to using the dirty tracking UAPI, request dirty tracking in the >> HWPT flags when the IOMMU supports dirty tracking. > this is what the patch brings. Right. >> >> The auto domain logic allows different IOMMU domains to be created when DMA >> dirty tracking is not desired (and VF can provide it) while others doesn't have > don't Right >> it and want the IOMMU capability. This is not used in this way here given how >> VFIODevice migration capability checking takes place *after* the device >> attachment. > Id on't understand the above sentence > The whole paragraph is meant to emphasize that we don't know if VF dirty tracking is supported because VFIODevice migration state hasn't been probed *yet*. And so we can't pick VF dirty tracking vs IOMMU dirty tracking at this stage when using IOMMU_HWPT_ALLOC_DIRTY_TRACKING flag and hence we always use it if IOMMU hw supports it even if later on VFIOMigration decides to use VF dirty tracking always instead. > Eric >> >> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >> --- >> include/hw/vfio/vfio-common.h | 1 + >> hw/vfio/iommufd.c | 12 ++++++++++++ >> 2 files changed, 13 insertions(+) >> >> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >> index 2dd468ce3c02..760f31d84ac8 100644 >> --- a/include/hw/vfio/vfio-common.h >> +++ b/include/hw/vfio/vfio-common.h >> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend; >> >> typedef struct VFIOIOASHwpt { >> uint32_t hwpt_id; >> + uint32_t hwpt_flags; >> QLIST_HEAD(, VFIODevice) device_list; >> QLIST_ENTRY(VFIOIOASHwpt) next; >> } VFIOIOASHwpt; >> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >> index d34dc88231ec..edc8f97d8f3d 100644 >> --- a/hw/vfio/iommufd.c >> +++ b/hw/vfio/iommufd.c >> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >> } >> } >> >> + /* >> + * This is quite early and VFIODevice isn't yet fully initialized, > so what's the problem exactly with the above? I should really say 'VFIO Migration state' here (see previous comment) >> + * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty >> + * tracking is going to be needed. >> + */ >> + if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) { >> + flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; >> + } >> + >> if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid, >> container->ioas_id, flags, >> IOMMU_HWPT_DATA_NONE, 0, NULL, >> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >> >> hwpt = g_malloc0(sizeof(*hwpt)); >> hwpt->hwpt_id = hwpt_id; >> + hwpt->hwpt_flags = flags; >> QLIST_INIT(&hwpt->device_list); >> >> ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp); >> @@ -267,6 +277,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >> vbasedev->hwpt = hwpt; >> QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); >> QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); >> + container->bcontainer.dirty_pages_supported |= >> + (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING); >> return true; >> } >> >
On 7/17/24 14:38, Joao Martins wrote: > On 17/07/2024 13:27, Eric Auger wrote: >> Hi Joao, >> >> On 7/12/24 13:47, Joao Martins wrote: >>> Probe hardware dirty tracking support by querying device hw capabilities via >>> IOMMUFD_GET_HW_INFO. >> this is not what the patch brings. GET_HW_INFO is always in place. > Yes. This is my mistake in squashing things as there was some shuffling going > around on how we do GET_HW_INFO. and didn't adjust the right hand of this sentence. > > I'll rephrase it. > >>> In preparation to using the dirty tracking UAPI, request dirty tracking in the >>> HWPT flags when the IOMMU supports dirty tracking. >> this is what the patch brings. > Right. > >>> The auto domain logic allows different IOMMU domains to be created when DMA >>> dirty tracking is not desired (and VF can provide it) while others doesn't have >> don't > Right > >>> it and want the IOMMU capability. This is not used in this way here given how >>> VFIODevice migration capability checking takes place *after* the device >>> attachment. >> Id on't understand the above sentence >> > The whole paragraph is meant to emphasize that we don't know if VF dirty > tracking is supported because VFIODevice migration state hasn't been probed > *yet*. And so we can't pick VF dirty tracking vs IOMMU dirty tracking at this > stage when using IOMMU_HWPT_ALLOC_DIRTY_TRACKING flag and hence we always use it > if IOMMU hw supports it even if later on VFIOMigration decides to use VF dirty > tracking always instead. that sounds a clearer explanation to me Eric > >> Eric >>> Signed-off-by: Joao Martins <joao.m.martins@oracle.com> >>> --- >>> include/hw/vfio/vfio-common.h | 1 + >>> hw/vfio/iommufd.c | 12 ++++++++++++ >>> 2 files changed, 13 insertions(+) >>> >>> diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h >>> index 2dd468ce3c02..760f31d84ac8 100644 >>> --- a/include/hw/vfio/vfio-common.h >>> +++ b/include/hw/vfio/vfio-common.h >>> @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend; >>> >>> typedef struct VFIOIOASHwpt { >>> uint32_t hwpt_id; >>> + uint32_t hwpt_flags; >>> QLIST_HEAD(, VFIODevice) device_list; >>> QLIST_ENTRY(VFIOIOASHwpt) next; >>> } VFIOIOASHwpt; >>> diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c >>> index d34dc88231ec..edc8f97d8f3d 100644 >>> --- a/hw/vfio/iommufd.c >>> +++ b/hw/vfio/iommufd.c >>> @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >>> } >>> } >>> >>> + /* >>> + * This is quite early and VFIODevice isn't yet fully initialized, >> so what's the problem exactly with the above? > I should really say 'VFIO Migration state' here (see previous comment) > >>> + * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty >>> + * tracking is going to be needed. >>> + */ >>> + if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) { >>> + flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; >>> + } >>> + >>> if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid, >>> container->ioas_id, flags, >>> IOMMU_HWPT_DATA_NONE, 0, NULL, >>> @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >>> >>> hwpt = g_malloc0(sizeof(*hwpt)); >>> hwpt->hwpt_id = hwpt_id; >>> + hwpt->hwpt_flags = flags; >>> QLIST_INIT(&hwpt->device_list); >>> >>> ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp); >>> @@ -267,6 +277,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, >>> vbasedev->hwpt = hwpt; >>> QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); >>> QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); >>> + container->bcontainer.dirty_pages_supported |= >>> + (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING); >>> return true; >>> } >>>
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h index 2dd468ce3c02..760f31d84ac8 100644 --- a/include/hw/vfio/vfio-common.h +++ b/include/hw/vfio/vfio-common.h @@ -97,6 +97,7 @@ typedef struct IOMMUFDBackend IOMMUFDBackend; typedef struct VFIOIOASHwpt { uint32_t hwpt_id; + uint32_t hwpt_flags; QLIST_HEAD(, VFIODevice) device_list; QLIST_ENTRY(VFIOIOASHwpt) next; } VFIOIOASHwpt; diff --git a/hw/vfio/iommufd.c b/hw/vfio/iommufd.c index d34dc88231ec..edc8f97d8f3d 100644 --- a/hw/vfio/iommufd.c +++ b/hw/vfio/iommufd.c @@ -246,6 +246,15 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, } } + /* + * This is quite early and VFIODevice isn't yet fully initialized, + * thus rely on IOMMU hardware capabilities as to whether IOMMU dirty + * tracking is going to be needed. + */ + if (vbasedev->hiod->caps.hw_caps & IOMMU_HW_CAP_DIRTY_TRACKING) { + flags = IOMMU_HWPT_ALLOC_DIRTY_TRACKING; + } + if (!iommufd_backend_alloc_hwpt(iommufd, vbasedev->devid, container->ioas_id, flags, IOMMU_HWPT_DATA_NONE, 0, NULL, @@ -255,6 +264,7 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, hwpt = g_malloc0(sizeof(*hwpt)); hwpt->hwpt_id = hwpt_id; + hwpt->hwpt_flags = flags; QLIST_INIT(&hwpt->device_list); ret = iommufd_cdev_attach_ioas_hwpt(vbasedev, hwpt->hwpt_id, errp); @@ -267,6 +277,8 @@ static bool iommufd_cdev_autodomains_get(VFIODevice *vbasedev, vbasedev->hwpt = hwpt; QLIST_INSERT_HEAD(&hwpt->device_list, vbasedev, hwpt_next); QLIST_INSERT_HEAD(&container->hwpt_list, hwpt, next); + container->bcontainer.dirty_pages_supported |= + (flags & IOMMU_HWPT_ALLOC_DIRTY_TRACKING); return true; }
Probe hardware dirty tracking support by querying device hw capabilities via IOMMUFD_GET_HW_INFO. In preparation to using the dirty tracking UAPI, request dirty tracking in the HWPT flags when the IOMMU supports dirty tracking. The auto domain logic allows different IOMMU domains to be created when DMA dirty tracking is not desired (and VF can provide it) while others doesn't have it and want the IOMMU capability. This is not used in this way here given how VFIODevice migration capability checking takes place *after* the device attachment. Signed-off-by: Joao Martins <joao.m.martins@oracle.com> --- include/hw/vfio/vfio-common.h | 1 + hw/vfio/iommufd.c | 12 ++++++++++++ 2 files changed, 13 insertions(+)