On Tue, 19 May 2020 09:53:56 -0600 Alex Williamson <alex.williamson@xxxxxxxxxx> wrote: > On Tue, 19 May 2020 17:35:07 +0200 > Cornelia Huck <cohuck@xxxxxxxxxx> wrote: > > > On Fri, 15 May 2020 23:05:24 +0530 > > Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > > > > > On 5/15/2020 4:29 PM, Cornelia Huck wrote: > > > > On Fri, 15 May 2020 02:07:43 +0530 > > > > Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote: > > > > > > > >> IOMMU container maintains a list of all pages pinned by vfio_pin_pages API. > > > >> All pages pinned by vendor driver through this API should be considered as > > > >> dirty during migration. When container consists of IOMMU capable device and > > > >> all pages are pinned and mapped, then all pages are marked dirty. > > > >> Added support to start/stop dirtied pages tracking and to get bitmap of all > > > >> dirtied pages for requested IO virtual address range. > > > >> > > > >> Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx> > > > >> Reviewed-by: Neo Jia <cjia@xxxxxxxxxx> > > > >> --- > > > >> include/uapi/linux/vfio.h | 55 +++++++++++++++++++++++++++++++++++++++++++++++ > > > >> 1 file changed, 55 insertions(+) > > > > (...) > > > > > >> + * When IOCTL is called with VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP flag set, > > > >> + * IOCTL returns dirty pages bitmap for IOMMU container during migration for > > > >> + * given IOVA range. > > > > > > > > "Calling the IOCTL with VFIO_IOMMU_DIRTY_PAGES_GET_BITMAP returns the > > > > dirty pages bitmap for the IOMMU container for a given IOVA range." ? > > > > > > > > Q: How does this interact with the two other operations? I imagine > > > > getting an empty bitmap before _START > > > > > > No, if dirty page tracking is not started, get_bitmap IOCTL will fail > > > with -EINVAL. > > > > > > > and a bitmap-in-progress between > > > > _START and _STOP. > After _STOP, will subsequent calls always give the > > > > same bitmap? > > > > > > > > > > No, return -EINVAL. > > > > Maybe add > > > > "If the IOCTL has not yet been called with > > VFIO_IOMMU_DIRTY_PAGES_FLAG_START, or if it has been called with > > VFIO_IOMMU_DIRTY_PAGES_FLAG_STOP, calling it with > > VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP will return -EINVAL." ? > > Let's not specify ourselves into a corner, I think we can simply say > that the dirty bitmap is only available while dirty logging is enabled. > We certainly don't need to specify specific errno values that'll trip > us up later. "If dirty logging is not enabled, an error will be returned." ? (...) > > > >> Caller must set argsz including size of structure > > > >> + * vfio_iommu_type1_dirty_bitmap_get. > > > > > > > > s/Caller/The caller/ > > > > > > > > Does argz also include the size of the bitmap? > > > > > > No. > > > > "The caller must set argsz to a value including the size of stuct > > vfio_io_type1_dirty_bitmap_get, but excluding the size of the actual > > bitmap." ? > > Yes, it wouldn't make sense for argsz to include the size of the bitmap > itself, that's accessed independently via a user provided pointer and > we have a separate size field for that. Thanks, > > Alex Yes, I just wanted to make it as obvious as possible to make it easier for folks trying to interact with this interface.