Hack around IOMMU changes
PATCH 4/4 Hack around IOMMU changes
Not for the faint of heart, this patch subverts the code by reassigning a local variable from a macro.
Quoted 1 line · Zachary Amsden
Not for the faint of heart, this patch subverts the code by reassigning a local variable from a macro.
This time, with patch.
Quoted 1 line
From
97b9230f699aba1c5f47972032b2d4d935a83054Mon Sep 17 00:00:00 2001
From: Zachary Amsden <[email protected]>
Date: Tue, 25 May 2010 17:17:32 -1000
Subject: [PATCH 4/5] IOMMU API changed
Ugly, dirty, disease ridden fix for IOMMU changes; the module is now trying to use larger IOMMU intervals; deny it this, and stick to page size. This requires forcibly setting page_size variable through knowledge of the code. Yuck. If you have a better solution, implement it.
--- external-module-compat-comm.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)
external-module-compat-comm.h +12 -0
@@ -1128,3 +1128,15 @@ perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,32 )#define lockdep_is_held (m) (1)#endif + +#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,34 ) +/* This is a dirty, nasty trick */ +#define iommu_map (domain, iova, paddr, gfp_order, prot) \ +({ \ + int _r = iommu_map_range(domain, iova, paddr, PAGE_SIZE, prot); \ + page_size = PAGE_SIZE; \ + _r; \ +}) +#define iommu_unmap (domain, iova, gfp_order) \ + (iommu_unmap_range(domain, iova, PAGE_SIZE),1) +#endif
Signature · 1 line
1.7.0.1
Quoted 27 lines · Zachary Amsden
Quoted 2 lines · Zachary Amsden
Not for the faint of heart, this patch subverts the code by reassigning a local variable from a macro.
This time, with patch.
From
97b9230f699aba1c5f47972032b2d4d935a83054Mon Sep 17 00:00:00 2001
From: Zachary Amsden <[email protected]>
Date: Tue, 25 May 2010 17:17:32 -1000
Subject: [PATCH 4/5] IOMMU API changedUgly, dirty, disease ridden fix for IOMMU changes; the module is now trying to use larger IOMMU intervals; deny it this, and stick to page size. This requires forcibly setting
page_sizevariable through knowledge of the code. Yuck. If you have a better solution, implement it.Signed-off-by Zachary Amsden <[email protected]>--- external-module-compat-comm.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)external-module-compat-comm.h +0
-0
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) · line 1136
Quoted 9 lines
external-module-compat-comm.h +6
-0@@ -1128,3 +1128,15 @@ perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,32 )#define lockdep_is_held (m) (1)#endif + +#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,34 ) +/* This is a dirty, nasty trick */ +#define iommu_map (domain, iova, paddr, gfp_order, prot) \ +({ \ + int _r = iommu_map_range(domain, iova, paddr, PAGE_SIZE, prot); \
This should be (PAGE_SIZE << gfp_order) according to my current understanding.
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) · line 1137
Quoted 1 line
external-module-compat-comm.h +1
-0+ page_size =PAGE_SIZE ; \
And what is this for?
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
Quoted 6 lines
external-module-compat-comm.h +5
-1+ _r; \ +}) +#define iommu_unmap (domain, iova, gfp_order) \ + (iommu_unmap_range(domain, iova, PAGE_SIZE),1) +#endif -- 1.7.0.1
Besides that, putting this under CONFIG_IOMMU_API and making the functions static inline would be preferred.
Jan
Quoted 35 lines · Jan Kiszka
Quoted 33 lines · Zachary Amsden
Quoted 3 lines · Zachary Amsden
Not for the faint of heart, this patch subverts the code by reassigning a local variable from a macro.
This time, with patch.
From
97b9230f699aba1c5f47972032b2d4d935a83054Mon Sep 17 00:00:00 2001
From: Zachary Amsden<[email protected]>
Date: Tue, 25 May 2010 17:17:32 -1000
Subject: [PATCH 4/5] IOMMU API changedUgly, dirty, disease ridden fix for IOMMU changes; the module is now trying to use larger IOMMU intervals; deny it this, and stick to page size. This requires forcibly setting
page_sizevariable through knowledge of the code. Yuck. If you have a better solution, implement it.Signed-off-by Zachary Amsden<[email protected]>--- external-module-compat-comm.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)external-module-compat-comm.h +1
-0@@ -1128,3 +1128,15 @@ perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,32 )#define lockdep_is_held (m) (1)#endif +
Quoted 9 lines
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) · line 1136
Quoted 6 lines
external-module-compat-comm.h +5
-0+#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,34 ) +/* This is a dirty, nasty trick */ +#define iommu_map (domain, iova, paddr, gfp_order, prot) \ +({ \ + int _r = iommu_map_range(domain, iova, paddr, PAGE_SIZE, prot); \This should be (PAGE_SIZE<<
gfp_order) according to my current understanding.
It should, but can't be.
Quoted 5 lines
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) · line 1137
Quoted 3 lines
external-module-compat-comm.h +1
-0+ page_size =PAGE_SIZE ; \And what is this for?
iommu_unmap is now passed an unreasonable value for order: PAGE_SIZE. The way the code work, that ends up becoming zero, then -1, for an infinite sized unmap (if I'm reading correctly). Then iommu_unmap_range returns the gfp_order of the unmap which it actually did.
Since we have no way to know the size of the original map, I deliberately force all mappings to be in units of PAGE_SIZE. This requires changing the local variable page_size in the following code. (Note the pin is unchanged, so we are doubling pinning... maybe this is causing my bug).
/*
* Pin all pages we are about to map in memory. This is
* important because we unmap and unpin in 4kb steps later.
*/
pfn = kvm_pin_pages(kvm, slot, gfn, page_size);
if (is_error_pfn(pfn)) {
gfn += 1;
continue;
}
/* Map into IO address space */
r = iommu_map(domain, gfn_to_gpa(gfn), pfn_to_hpa(pfn),
get_order(page_size), flags);
if (r) {
printk(KERN_ERR "kvm_iommu_map_address:"
"iommu failed to map pfn=%lx\n", pfn);
goto unmap_pages;
}
gfn += page_size >> PAGE_SHIFT;In any case, fixing the gfn computation is needed so the unmap can work:
while (gfn < end_gfn) {
unsigned long unmap_pages;
int order;
/* Get physical address */
phys = iommu_iova_to_phys(domain, gfn_to_gpa(gfn));
pfn = phys >> PAGE_SHIFT;
/* Unmap address from IO address space */
order = iommu_unmap(domain, gfn_to_gpa(gfn), PAGE_SIZE);
unmap_pages = 1ULL << order;
/* Unpin all pages we just unmapped to not leak any memory */
kvm_unpin_pages(kvm, pfn, unmap_pages);
gfn += unmap_pages;
}Note iommu_unmap(,,PAGE_SIZE) is a bogus argument.
Quoted 14 lines
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)
Quoted 8 lines
external-module-compat-comm.h +5
-1+ _r; \ +}) +#define iommu_unmap (domain, iova, gfp_order) \ + (iommu_unmap_range(domain, iova, PAGE_SIZE),1) +#endif -- 1.7.0.1Besides that, putting this under
CONFIG_IOMMU_APIand making the functions static inline would be preferred.Jan
Quoted 48 lines · Zachary Amsden
Quoted 37 lines · Jan Kiszka
Quoted 35 lines · Zachary Amsden
Quoted 3 lines · Zachary Amsden
Not for the faint of heart, this patch subverts the code by reassigning a local variable from a macro.
This time, with patch.
From
97b9230f699aba1c5f47972032b2d4d935a83054Mon Sep 17 00:00:00 2001
From: Zachary Amsden<[email protected]>
Date: Tue, 25 May 2010 17:17:32 -1000
Subject: [PATCH 4/5] IOMMU API changedUgly, dirty, disease ridden fix for IOMMU changes; the module is now trying to use larger IOMMU intervals; deny it this, and stick to page size. This requires forcibly setting
page_sizevariable through knowledge of the code. Yuck. If you have a better solution, implement it.Signed-off-by Zachary Amsden<[email protected]>--- external-module-compat-comm.h | 12 ++++++++++++ 1 files changed, 12 insertions(+), 0 deletions(-)external-module-compat-comm.h +1
-0@@ -1128,3 +1128,15 @@ perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs)#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,32 )#define lockdep_is_held (m) (1)#endif +Quoted 9 lines
external-module-compat-comm.h perf_unregister_guest_info_callbacks(struct perf_guest_info_callbacks *cbs) · line 1136
Quoted 6 lines
external-module-compat-comm.h +5
-0+#if LINUX_VERSION_CODE <KERNEL_VERSION (2 ,6 ,34 ) +/* This is a dirty, nasty trick */ +#define iommu_map (domain, iova, paddr, gfp_order, prot) \ +({ \ + int _r = iommu_map_range(domain, iova, paddr, PAGE_SIZE, prot); \This should be (PAGE_SIZE<<
gfp_order) according to my current understanding.
Quoted 8 lines
It should, but can't be.
Quoted 5 lines
Quoted 3 lines
+
page_size=PAGE_SIZE; \And what is this for?
iommu_unmapis now passed an unreasonable value for order:PAGE_SIZE.
Indeed. This is an upstream bug and has to be fixed instead of worked around. I'll send a patch.
Jan
PS: If there were already a KVM release with this included, the more appropriate way for kvm-kmod to deal with it would be patching the particular code line during sync.