Skip to contentSkip to conversation

KVM: IOMMU: hva align mapping page size

PATCH Superseded Greg Edwards 3 participants 5 messages Nov 1, 2013 – Nov 5, 2013 KVM

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 KVM: IOMMU: hva align mapping page size

When determining the page size we could use to map with the IOMMU, the page size should be aligned with the hva, not the gfn. The gfn may not reflect the real alignment within the hugetlbfs file.

Most of the time, this works fine. However, if the hugetlbfs file is backed by non-contiguous huge pages, a multi-huge page memslot starts at an unaligned offset within the hugetlbfs file, and the gfn is aligned with respect to the huge page size, kvm_host_page_size() will return the huge page size and we will use that to map with the IOMMU.

When we later unpin that same memslot, the IOMMU returns the unmap size as the huge page size, and we happily unpin that many pfns in monotonically increasing order, not realizing we are spanning non-contiguous huge pages and partially unpin the wrong huge page.

Instead, ensure the IOMMU mapping page size is aligned with the hva corresponding to the gfn, which does reflect the alignment within the hugetlbfs file.

---
This resolves the bug previously reported (and misdiagnosed) here:

http://www.spinics.net/lists/kvm/msg97599.html

 virt/kvm/iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
virt/kvm/iommu.c +2 -2
index 72a130b..0e2ff32 100644
@@ -99,8 +99,8 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		while ((gfn + (page_size >> PAGE_SHIFT)) > end_gfn)
 			page_size >>= 1;
 
-		/* Make sure gfn is aligned to the page size we want to map */
-		while ((gfn << PAGE_SHIFT) & (page_size - 1))
+		/* Make sure hva is aligned to the page size we want to map */
+		while (__gfn_to_hva_memslot(slot, gfn) & (page_size - 1))
 			page_size >>= 1;
 
 		/*
Signature · 1 line
1.8.3.2
#1 lore raw
Quoted 34 lines · Greg Edwards

When determining the page size we could use to map with the IOMMU, the page size should be aligned with the hva, not the gfn. The gfn may not reflect the real alignment within the hugetlbfs file.

Most of the time, this works fine. However, if the hugetlbfs file is backed by non-contiguous huge pages, a multi-huge page memslot starts at an unaligned offset within the hugetlbfs file, and the gfn is aligned with respect to the huge page size, kvm_host_page_size() will return the huge page size and we will use that to map with the IOMMU.

When we later unpin that same memslot, the IOMMU returns the unmap size as the huge page size, and we happily unpin that many pfns in monotonically increasing order, not realizing we are spanning non-contiguous huge pages and partially unpin the wrong huge page.

Instead, ensure the IOMMU mapping page size is aligned with the hva corresponding to the gfn, which does reflect the alignment within the hugetlbfs file.

---
This resolves the bug previously reported (and misdiagnosed) here:

http://www.spinics.net/lists/kvm/msg97599.html

 virt/kvm/iommu.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)
virt/kvm/iommu.c +0 -0
index 72a130b..0e2ff32 100644

virt/kvm/iommu.c int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot) · line 104

Quoted 8 lines
virt/kvm/iommu.c +2 -2
@@ -99,8 +99,8 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		while ((gfn + (page_size >> PAGE_SHIFT)) > end_gfn)
 			page_size >>= 1;
 
-		/* Make sure gfn is aligned to the page size we want to map */
-		while ((gfn << PAGE_SHIFT) & (page_size - 1))
+		/* Make sure hva is aligned to the page size we want to map */
+		while (__gfn_to_hva_memslot(slot, gfn) & (page_size - 1))
 			page_size >>= 1;

gfn should be aligned to page size as well (IOMMU requirement), so don't drop that check.

#2 lore raw

PATCH v2 1/1 KVM: IOMMU: hva align mapping page size

When determining the page size we could use to map with the IOMMU, the page size should also be aligned with the hva, not just the gfn. The gfn may not reflect the real alignment within the hugetlbfs file.

---
 virt/kvm/iommu.c | 4 ++++
 1 file changed, 4 insertions(+)
virt/kvm/iommu.c +4 -0
index 72a130b..c329c8f 100644
@@ -103,6 +103,10 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		while ((gfn << PAGE_SHIFT) & (page_size - 1))
 			page_size >>= 1;
 
+		/* Make sure hva is aligned to the page size we want to map */
+		while (__gfn_to_hva_memslot(slot, gfn) & (page_size - 1))
+			page_size >>= 1;
+
 		/*
 		 * Pin all pages we are about to map in memory. This is
 		 * important because we unmap and unpin in 4kb steps later.
Signature · 1 line
1.8.3.2
Reviewed-by on v2 1/1
#3 lore raw
Quoted 27 lines · Greg Edwards

When determining the page size we could use to map with the IOMMU, the page size should also be aligned with the hva, not just the gfn. The gfn may not reflect the real alignment within the hugetlbfs file.

---
 virt/kvm/iommu.c | 4 ++++
 1 file changed, 4 insertions(+)
virt/kvm/iommu.c +4 -0
index 72a130b..c329c8f 100644
@@ -103,6 +103,10 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		while ((gfn << PAGE_SHIFT) & (page_size - 1))
 			page_size >>= 1;
 
+		/* Make sure hva is aligned to the page size we want to map */
+		while (__gfn_to_hva_memslot(slot, gfn) & (page_size - 1))
+			page_size >>= 1;
+
 		/*
 		 * Pin all pages we are about to map in memory. This is
 		 * important because we unmap and unpin in 4kb steps later.
Signature · 1 line
1.8.3.2
#4 lore raw
In reply to Greg Edwards
Quoted 4 lines · Greg Edwards

When determining the page size we could use to map with the IOMMU, the page size should also be aligned with the hva, not just the gfn. The gfn may not reflect the real alignment within the hugetlbfs file.

For some reason you dropped very good commit message from v1. I applied v2 with v1 commit message.

Quoted 28 lines
---
 virt/kvm/iommu.c | 4 ++++
 1 file changed, 4 insertions(+)
virt/kvm/iommu.c +4 -0
index 72a130b..c329c8f 100644
@@ -103,6 +103,10 @@ int kvm_iommu_map_pages(struct kvm *kvm, struct kvm_memory_slot *slot)
 		while ((gfn << PAGE_SHIFT) & (page_size - 1))
 			page_size >>= 1;
 
+		/* Make sure hva is aligned to the page size we want to map */
+		while (__gfn_to_hva_memslot(slot, gfn) & (page_size - 1))
+			page_size >>= 1;
+
 		/*
 		 * Pin all pages we are about to map in memory. This is
 		 * important because we unmap and unpin in 4kb steps later.
Signature · 6 lines
1.8.3.2

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
Gleb.