Re: [PATCH v10 Kernel 4/5] vfio iommu: Implementation of ioctl to for dirty pages tracking.

Kirti Wankhede
2019-12-19T18:42:50Z

On 12/19/2019 3:09 AM, Alex Williamson wrote:
> On Tue, 17 Dec 2019 14:54:14 +0530
> Kirti Wankhede <kwankhede@nvidia.com> wrote:
>
>> On 12/17/2019 10:45 AM, Yan Zhao wrote:
>>> On Tue, Dec 17, 2019 at 04:21:39AM +0800, Kirti Wankhede wrote:
>>>> + } else if (range.flags &
>>>> + VFIO_IOMMU_DIRTY_PAGES_FLAG_GET_BITMAP) {
>>>> + uint64_t iommu_pgmask;
>>>> + unsigned long pgshift = __ffs(range.pgsize);
>>>> + unsigned long *bitmap;
>>>> + long bsize;
>>>> +
>>>> + iommu_pgmask =
>>>> + ((uint64_t)1 << __ffs(vfio_pgsize_bitmap(iommu))) - 1;
>>>> +
>>>> + if (((range.pgsize - 1) & iommu_pgmask) !=
>>>> + (range.pgsize - 1))
>>>> + return -EINVAL;
>>>> +
>>>> + if (range.iova & iommu_pgmask)
>>>> + return -EINVAL;
>>>> + if (!range.size || range.size > SIZE_MAX)
>>>> + return -EINVAL;
>>>> + if (range.iova + range.size < range.iova)
>>>> + return -EINVAL;
>>>> +
>>>> + bsize = verify_bitmap_size(range.size >> pgshift,
>>>> + range.bitmap_size);
>>>> + if (bsize)
>>>> + return ret;
>>>> +
>>>> + bitmap = kmalloc(bsize, GFP_KERNEL);
>>>> + if (!bitmap)
>>>> + return -ENOMEM;
>>>> +
>>>> + ret = copy_from_user(bitmap,
>>>> + (void __user *)range.bitmap, bsize) ? -EFAULT : 0;
>>>> + if (ret)
>>>> + goto bitmap_exit;
>>>> +
>>>> + iommu->dirty_page_tracking = false;
>>> why iommu->dirty_page_tracking is false here?
>>> suppose this ioctl can be called several times.
>>>
>>
>> This ioctl can be called several times, but once this ioctl is called
>> that means vCPUs are stopped and VFIO devices are stopped (i.e. in
>> stop-and-copy phase) and dirty pages bitmap are being queried by user.
>
> Do not assume how userspace works or its intent. If dirty tracking is
> on, it should remain on until the user turns it off. We cannot assume
> userspace uses a one-shot approach. Thanks,
>

Dirty tracking should be on until user turns it off or user reads
bitmap, right? This ioctl is used to read bitmap.

Thanks,
Kirti