Re: [PATCH v16 Kernel 5/7] vfio iommu: Update UNMAP_DMA ioctl to get dirty bitmap before unmap

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/27/2020 5:34 AM, Yan Zhao wrote:
On Fri, Mar 27, 2020 at 05:39:44AM +0800, Kirti Wankhede wrote:


On 3/25/2020 7:48 AM, Yan Zhao wrote:
On Wed, Mar 25, 2020 at 03:32:37AM +0800, Kirti Wankhede wrote:
DMA mapped pages, including those pinned by mdev vendor drivers, might
get unpinned and unmapped while migration is active and device is still
running. For example, in pre-copy phase while guest driver could access
those pages, host device or vendor driver can dirty these mapped pages.
Such pages should be marked dirty so as to maintain memory consistency
for a user making use of dirty page tracking.

To get bitmap during unmap, user should allocate memory for bitmap, set
size of allocated memory, set page size to be considered for bitmap and
set flag VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP.

Signed-off-by: Kirti Wankhede <kwankhede@xxxxxxxxxx>
Reviewed-by: Neo Jia <cjia@xxxxxxxxxx>
---
   drivers/vfio/vfio_iommu_type1.c | 54 ++++++++++++++++++++++++++++++++++++++---
   include/uapi/linux/vfio.h       | 10 ++++++++
   2 files changed, 60 insertions(+), 4 deletions(-)

diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
index 27ed069c5053..b98a8d79e13a 100644
--- a/drivers/vfio/vfio_iommu_type1.c
+++ b/drivers/vfio/vfio_iommu_type1.c
@@ -982,7 +982,8 @@ static int verify_bitmap_size(uint64_t npages, uint64_t bitmap_size)
   }
static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
-			     struct vfio_iommu_type1_dma_unmap *unmap)
+			     struct vfio_iommu_type1_dma_unmap *unmap,
+			     struct vfio_bitmap *bitmap)
   {
   	uint64_t mask;
   	struct vfio_dma *dma, *dma_last = NULL;
@@ -1033,6 +1034,10 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
   	 * will be returned if these conditions are not met.  The v2 interface
   	 * will only return success and a size of zero if there were no
   	 * mappings within the range.
+	 *
+	 * When VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP flag is set, unmap request
+	 * must be for single mapping. Multiple mappings with this flag set is
+	 * not supported.
   	 */
   	if (iommu->v2) {
   		dma = vfio_find_dma(iommu, unmap->iova, 1);
@@ -1040,6 +1045,13 @@ static int vfio_dma_do_unmap(struct vfio_iommu *iommu,
   			ret = -EINVAL;
   			goto unlock;
   		}
+
+		if ((unmap->flags & VFIO_DMA_UNMAP_FLAG_GET_DIRTY_BITMAP) &&
+		    (dma->iova != unmap->iova || dma->size != unmap->size)) {
potential NULL pointer!

And could you address the comments in v14?
How to handle DSI unmaps in vIOMMU
(https://lore.kernel.org/kvm/20200323011041.GB5456@joy-OptiPlex-7040/)


Sorry, I drafted reply to it, but I missed to send, it remained in my drafts

  >
  > it happens in vIOMMU Domain level invalidation of IOTLB
  > (domain-selective invalidation, see vtd_iotlb_domain_invalidate() in
qemu).
  > common in VTD lazy mode, and NOT just happening once at boot time.
  > rather than invalidate page by page, it batches the page invalidation.
  > so, when this invalidation takes place, even higher level page tables
  > have been invalid and therefore it has to invalidate a bigger
combined range.
  > That's why we see IOVAs are mapped in 4k pages, but are unmapped in 2M
  > pages.
  >
  > I think those UNMAPs should also have GET_DIRTY_BIMTAP flag on, right?


vtd_iotlb_domain_invalidate()
    vtd_sync_shadow_page_table()
      vtd_sync_shadow_page_table_range(vtd_as, &ce, 0, UINT64_MAX)
        vtd_page_walk()
          vtd_page_walk_level() - walk over specific level for IOVA range
            vtd_page_walk_one()
              memory_region_notify_iommu()
              ...
                vfio_iommu_map_notify()

In the above trace, isn't page walk will take care of creating proper
IOTLB entry which should be same as created during mapping for that
IOTLB entry?

No. It does walk the page table, but as it's dsi (delay & batched unmap),
pages table entry for a whole 2M (the higher level, not last level for 4K)
range is invalid, so the iotlb->addr_mask what vfio_iommu_map_notify()
receives is (2M - 1), not the same as the size for map.


When do this happen? during my testing I never hit this case. How can I hit this case?

In this case, will adjacent whole vfio_dmas will be clubbed together or will there be any intersection of vfio_dmas?

Thanks,
Kirti



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux