In kvm_iommu_put_pages() we use this bizarre and horrid API for
iommu_unmap() where we call it to unmap PAGE_SIZE at a time, but if it
actually finds a larger page than that in its page tables then it's
allowed to unmap and return the larger size.

For some IOMMUs including the Intel one, breaking unmaps into tiny
chunks like this is *really* pessimal.

It looks like the actual reason it's done this way is because we need to
unpin the underlying physical pages when we're done with them, and the
only way we can do that is by doing it a page at a time calling
iommu_iova_to_phys() on each page. 

I wonder if this could be better handled by passing a callback function
in to the iommu_unmap() function, which can be called for each physical
address range that's unmapped.

That way, we wouldn't need all the cache and IOTLB flushing between the
individual page unmaps; we could still just do it the quick way.

It would look something like this...

diff --git a/virt/kvm/iommu.c b/virt/kvm/iommu.c
index 0df7d4b..bf3f4f9 100644
--- a/virt/kvm/iommu.c
+++ b/virt/kvm/iommu.c
@@ -278,41 +278,14 @@ static void kvm_iommu_put_pages(struct kvm *kvm,
                                gfn_t base_gfn, unsigned long npages)
 {
        struct iommu_domain *domain;
-       gfn_t end_gfn, gfn;
-       pfn_t pfn;
-       u64 phys;
 
        domain  = kvm->arch.iommu_domain;
-       end_gfn = base_gfn + npages;
-       gfn     = base_gfn;
 
        /* check if iommu exists and in use */
        if (!domain)
                return;
 
-       while (gfn < end_gfn) {
-               unsigned long unmap_pages;
-               size_t size;
-
-               /* Get physical address */
-               phys = iommu_iova_to_phys(domain, gfn_to_gpa(gfn));
-
-               if (!phys) {
-                       gfn++;
-                       continue;
-               }
-
-               pfn  = phys >> PAGE_SHIFT;
-
-               /* Unmap address from IO address space */
-               size       = iommu_unmap(domain, gfn_to_gpa(gfn), PAGE_SIZE);
-               unmap_pages = 1ULL << get_order(size);
-
-               /* Unpin all pages we just unmapped to not leak any memory */
-               kvm_unpin_pages(kvm, pfn, unmap_pages);
-
-               gfn += unmap_pages;
-       }
+       iommu_unmap(domain, gfn_to_gpa(base_gfn), npages << PAGE_SHIFT, 
kvm_unpin_pages, kvm);
 }
 
 void kvm_iommu_unmap_pages(struct kvm *kvm, struct kvm_memory_slot *slot)

-- 
David Woodhouse                            Open Source Technology Centre
david.woodho...@intel.com                              Intel Corporation

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Reply via email to