On Tue, 12 Nov 2019 22:33:40 +0530
Kirti Wankhede <kwankhede@xxxxxxxxxx> wrote:
> If pages are pinned by external interface for requested IO virtual address
> range, bitmap of such pages is created and then that range is unmapped.
> To get bitmap during unmap, user should set flag
> VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP, bitmap memory should be allocated and
> bitmap_size should be set. If flag is not set, then it behaves same as
> VFIO_IOMMU_UNMAP_DMA ioctl.
>
> Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
> Reviewed-by: Neo Jia <cjia@xxxxxxxxxx>
> ---
> drivers/vfio/vfio_iommu_type1.c | 71 +++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 69 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index ac176e672857..d6b988452ba6 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -926,7 +926,8 @@ static int vfio_iova_get_dirty_bitmap(struct vfio_iommu *iommu,
> }
>
> static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
> - struct vfio_iommu_type1_dma_unmap *unmap)
> + struct vfio_iommu_type1_dma_unmap *unmap,
> + unsigned long *bitmap)
> {
> uint64_t mask;
> struct vfio_dma *dma, *dma_last = NULL;
> @@ -1026,6 +1027,12 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
> &nb_unmap);
> goto again;
> }
> +
> + if (bitmap) {
> + vfio_iova_dirty_bitmap(iommu, dma->iova, dma->size,
> + unmap->iova, bitmap);
> + }
> +
> unmapped += dma->size;
> vfio_remove_dma(iommu, dma);
> }
> @@ -1039,6 +1046,43 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
> return ret;
> }
>
> +static int vfio_dma_do_unmap_bitmap(struct vfio_iommu *iommu,
> + struct vfio_iommu_type1_dma_unmap_bitmap *unmap_bitmap)
> +{
> + struct vfio_iommu_type1_dma_unmap unmap;
> + unsigned long *bitmap = NULL;
> + int ret;
> +
> + /* check bitmap size */
> + if ((unmap_bitmap->flags | VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP)) {
It's required to enforce other flag bits are zero or else we can never
guarantee we can use them in the future without breaking existing
userspace, but I'd really rather extend the existing ioctl.
Should we provide any optimization to indicate to the user that dirty
bits were set? Thanks,
Alex
> + ret = verify_bitmap_size(unmap_bitmap->size >> PAGE_SHIFT,
> + unmap_bitmap->bitmap_size);
> + if (ret)
> + return ret;
> +
> + /* one bit per page */
> + bitmap = bitmap_zalloc(unmap_bitmap->size >> PAGE_SHIFT,
> + GFP_KERNEL);
> + if (!bitmap)
> + return -ENOMEM;
> + }
> +
> + unmap.iova = unmap_bitmap->iova;
> + unmap.size = unmap_bitmap->size;
> + ret = vfio_dma_do_unmap(iommu, &unmap, bitmap);
> + if (!ret)
> + unmap_bitmap->size = unmap.size;
> +
> + if (bitmap) {
> + if (!ret && copy_to_user(unmap_bitmap->bitmap, bitmap,
> + unmap_bitmap->bitmap_size))
> + ret = -EFAULT;
> + bitmap_free(bitmap);
> + }
> +
> + return ret;
> +}
> +
> static int vfio_iommu_map(struct vfio_iommu *iommu, dma_addr_t iova,
> unsigned long pfn, long npage, int prot)
> {
> @@ -2366,7 +2410,7 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> if (unmap.argsz < minsz || unmap.flags)
> return -EINVAL;
>
> - ret = vfio_dma_do_unmap(iommu, &unmap);
> + ret = vfio_dma_do_unmap(iommu, &unmap, NULL);
> if (ret)
> return ret;
>
> @@ -2389,6 +2433,29 @@ static long vfio_iommu_type1_ioctl(void *iommu_data,
> return -EINVAL;
>
> return vfio_iova_get_dirty_bitmap(iommu, &range);
> + } else if (cmd == VFIO_IOMMU_UNMAP_DMA_GET_BITMAP) {
> + struct vfio_iommu_type1_dma_unmap_bitmap unmap_bitmap;
> + long ret;
> +
> + /* Supported for v2 version only */
> + if (!iommu->v2)
> + return -EACCES;
> +
> + minsz = offsetofend(struct vfio_iommu_type1_dma_unmap_bitmap,
> + bitmap);
> +
> + if (copy_from_user(&unmap_bitmap, (void __user *)arg, minsz))
> + return -EFAULT;
> +
> + if (unmap_bitmap.argsz < minsz)
> + return -EINVAL;
> +
> + ret = vfio_dma_do_unmap_bitmap(iommu, &unmap_bitmap);
> + if (ret)
> + return ret;
> +
> + return copy_to_user((void __user *)arg, &unmap_bitmap, minsz) ?
> + -EFAULT : 0;
> }
>
> return -ENOTTY;
