public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [kvm-unit-tests 0/5] Split large EPT mappings properly
@ 2016-03-01 19:30 Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes Peter Feiner
                   ` (7 more replies)
  0 siblings, 8 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

install_ept_entry would overwrite arbitrary memory when it encountered
a large page.

I discovered the bug while working on a simple multi-page allocator,
which I'm using for huge page testing, that sorts the free list in
ascending order of HPA.

Peter Feiner (5):
  x86: vmx.h: trivial whitespace fixes
  x86: vmx: Named constant: EPT_ADDR_MASK
  x86: vmx: Named constant: EPT_LEVEL_SHIFT
  x86: vmx: split large EPTEs in install_ept_entry
  x86: vmx: don't explicitly split identity EPT map

 x86/vmx.c       | 65 +++++++++++++++++++++++++++++++++++++++++++--------------
 x86/vmx.h       |  9 +++++---
 x86/vmx_tests.c |  7 -------
 3 files changed, 55 insertions(+), 26 deletions(-)

-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
@ 2016-03-01 19:30 ` Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index f868e5d..cae9274 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -463,9 +463,9 @@ enum Ctrl1 {
 
 #define PAGE_SIZE_2M		(512 * PAGE_SIZE)
 #define PAGE_SIZE_1G		(512 * PAGE_SIZE_2M)
-#define	EPT_PAGE_LEVEL	4
-#define	EPT_PGDIR_WIDTH	9
-#define	EPT_PGDIR_MASK	511
+#define EPT_PAGE_LEVEL		4
+#define EPT_PGDIR_WIDTH		9
+#define EPT_PGDIR_MASK		511
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes Peter Feiner
@ 2016-03-01 19:30 ` Peter Feiner
  2016-03-01 21:12   ` Paolo Bonzini
  2016-03-01 19:30 ` [kvm-unit-tests 3/5] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
                   ` (5 subsequent siblings)
  7 siblings, 1 reply; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 6 +++---
 x86/vmx.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 3fa1a73..9811a28 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
 					| EPT_RA | EPT_WA | EPT_EA;
 		} else
 			pt[offset] &= ~EPT_LARGE_PAGE;
-		pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
+		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = ((unsigned long)guest_addr >> ((level-1) *
 			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
@@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			break;
 		if (l < 4 && (pte & EPT_LARGE_PAGE))
 			return pte;
-		pt = (unsigned long *)(pte & 0xffffffffff000ull);
+		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
@@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
-		pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
+		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index cae9274..c2a2121 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -466,6 +466,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_ADDR_MASK		0xffffffffff000ul
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests 3/5] x86: vmx: Named constant: EPT_LEVEL_SHIFT
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
@ 2016-03-01 19:30 ` Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 4/5] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 17 ++++++-----------
 x86/vmx.h |  1 +
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 9811a28..140ad86 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -233,7 +233,7 @@ void install_ept_entry(unsigned long *pml4,
 	unsigned offset;
 
 	for (level = EPT_PAGE_LEVEL; level > pte_level; --level) {
-		offset = (guest_addr >> ((level-1) * EPT_PGDIR_WIDTH + 12))
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(level))
 				& EPT_PGDIR_MASK;
 		if (!(pt[offset] & (EPT_PRESENT))) {
 			unsigned long *new_pt = pt_page;
@@ -248,8 +248,7 @@ void install_ept_entry(unsigned long *pml4,
 			pt[offset] &= ~EPT_LARGE_PAGE;
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = ((unsigned long)guest_addr >> ((level-1) *
-			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
 	pt[offset] = pte;
 }
 
@@ -325,8 +324,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		pte = pt[offset];
 		if (!(pte & (EPT_PRESENT)))
 			return 0;
@@ -336,8 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			return pte;
 		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pte = pt[offset];
 	return pte;
 }
@@ -372,16 +369,14 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		if (l == level)
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
 		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pt[offset] = pte_val;
 	return 0;
 }
diff --git a/x86/vmx.h b/x86/vmx.h
index c2a2121..0b646fa 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -466,6 +466,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		0xffffffffff000ul
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests 4/5] x86: vmx: split large EPTEs in install_ept_entry
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
                   ` (2 preceding siblings ...)
  2016-03-01 19:30 ` [kvm-unit-tests 3/5] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
@ 2016-03-01 19:30 ` Peter Feiner
  2016-03-01 19:30 ` [kvm-unit-tests 5/5] x86: vmx: don't explicitly split identity EPT map Peter Feiner
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

When install_ept_entry encountered a leaf entry above pte_level, it
just cleared the EPT_LARGE_PAGE bit and continued the traversal _using
the first 4K page from the large page as the next level of the page
table_! This is broken because (1) the data in the large page would be
overwritten, and (2) all of other entires in the new level of the page
table would contain garbage.

Now, install_ept_entry splits the large mapping by allocating a new
page and filling it in with 512 PTEs that point to the large page's
constituent 2M or 4K pages.

This path is exercised in the VMX EPT test when 2m EPT pages are
enabled. The bug wasn't obvious because the free list is sorted in
descending order of HPA, thus the large page being overwritten with an
EPTE happened to only contain 0s.

Fixes: 04b0e0f342978f08b8b0b068c08c9d45ee80e3f7 ("nEPT: Fix test cases for 2M huge pages").
Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 x86/vmx.h |  1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 140ad86..d3fdc71 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -215,6 +215,44 @@ asm(
 );
 
 /* EPT paging structure related functions */
+/* split_large_ept_entry: Split a 2M/1G large page into 512 smaller PTEs.
+		@ptep : large page table entry to split
+		@level : level of ptep (2 or 3)
+ */
+static void split_large_ept_entry(unsigned long *ptep, int level)
+{
+	unsigned long *new_pt;
+	unsigned long gpa;
+	unsigned long pte;
+	unsigned long prototype;
+	int i;
+
+	pte = *ptep;
+	assert(pte & EPT_PRESENT);
+	assert(pte & EPT_LARGE_PAGE);
+	assert(level == 2 || level == 3);
+
+	new_pt = alloc_page();
+	assert(new_pt);
+	memset(new_pt, 0, PAGE_SIZE);
+
+	prototype = pte & ~EPT_ADDR_MASK;
+	if (level == 2)
+		prototype &= ~EPT_LARGE_PAGE;
+
+	gpa = pte & EPT_ADDR_MASK;
+	for (i = 0; i < EPT_PGDIR_ENTRIES; i++) {
+		new_pt[i] = prototype | gpa;
+		gpa += 1ul << EPT_LEVEL_SHIFT(level - 1);
+	}
+
+	pte &= ~EPT_LARGE_PAGE;
+	pte &= ~EPT_ADDR_MASK;
+	pte |= virt_to_phys(new_pt);
+
+	*ptep = pte;
+}
+
 /* install_ept_entry : Install a page to a given level in EPT
 		@pml4 : addr of pml4 table
 		@pte_level : level of PTE to set
@@ -244,8 +282,8 @@ void install_ept_entry(unsigned long *pml4,
 			memset(new_pt, 0, PAGE_SIZE);
 			pt[offset] = virt_to_phys(new_pt)
 					| EPT_RA | EPT_WA | EPT_EA;
-		} else
-			pt[offset] &= ~EPT_LARGE_PAGE;
+		} else if (pt[offset] & EPT_LARGE_PAGE)
+			split_large_ept_entry(&pt[offset], level);
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index 0b646fa..11ece90 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -466,6 +466,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_PGDIR_ENTRIES	(1 << EPT_PGDIR_WIDTH)
 #define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		0xffffffffff000ul
 #define PAGE_MASK		(~(PAGE_SIZE-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests 5/5] x86: vmx: don't explicitly split identity EPT map
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
                   ` (3 preceding siblings ...)
  2016-03-01 19:30 ` [kvm-unit-tests 4/5] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
@ 2016-03-01 19:30 ` Peter Feiner
  2016-03-01 21:13 ` [kvm-unit-tests 0/5] Split large EPT mappings properly Paolo Bonzini
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 19:30 UTC (permalink / raw)
  To: kvm, jan.kiszka, pbonzini; +Cc: pfeiner

Since install_ept_entry automatically splits large EPTEs, it's no
longer necessary to explicitly pave the EPT identity map with 4K
entries mapping [base_addr1, base_addr1 + PAGE_SIZE_2M). I.e.,
install_ept(data_page1, data_page2) does the paving before installing
the 4k data_page1 -> data_page2 mapping.

The other setup_ept_range call was unnecessary.

Note that there was benign bug in the deleted code. The third argument
of setup_ept_range is length not end! Thus the range of GPA mappings
being split was [base_addr1, base_addr1 * 2 + PAGE_SIZE_2M).

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx_tests.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bb65c06..4ef2247 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -954,7 +954,6 @@ static int apic_version;
 
 static int ept_init()
 {
-	unsigned long base_addr1, base_addr2;
 	u32 ctrl_cpu[2];
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
@@ -979,12 +978,6 @@ static int ept_init()
 	memset(data_page2, 0x0, PAGE_SIZE);
 	*((u32 *)data_page1) = MAGIC_VAL_1;
 	*((u32 *)data_page2) = MAGIC_VAL_2;
-	base_addr1 = (unsigned long)data_page1 & PAGE_MASK_2M;
-	base_addr2 = (unsigned long)data_page2 & PAGE_MASK_2M;
-	setup_ept_range(pml4, base_addr1, base_addr1 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
-	setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 19:30 ` [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
@ 2016-03-01 21:12   ` Paolo Bonzini
  2016-03-01 21:16     ` Peter Feiner
  0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2016-03-01 21:12 UTC (permalink / raw)
  To: Peter Feiner, kvm, jan.kiszka



On 01/03/2016 20:30, Peter Feiner wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  x86/vmx.c | 6 +++---
>  x86/vmx.h | 1 +
>  2 files changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 3fa1a73..9811a28 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
>  					| EPT_RA | EPT_WA | EPT_EA;
>  		} else
>  			pt[offset] &= ~EPT_LARGE_PAGE;
> -		pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
> +		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
>  	}
>  	offset = ((unsigned long)guest_addr >> ((level-1) *
>  			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
> @@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
>  			break;
>  		if (l < 4 && (pte & EPT_LARGE_PAGE))
>  			return pte;
> -		pt = (unsigned long *)(pte & 0xffffffffff000ull);
> +		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
>  	}
>  	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>  			& EPT_PGDIR_MASK;
> @@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
>  			break;
>  		if (!(pt[offset] & (EPT_PRESENT)))
>  			return -1;
> -		pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
> +		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
>  	}
>  	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>  			& EPT_PGDIR_MASK;
> diff --git a/x86/vmx.h b/x86/vmx.h
> index cae9274..c2a2121 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -466,6 +466,7 @@ enum Ctrl1 {
>  #define EPT_PAGE_LEVEL		4
>  #define EPT_PGDIR_WIDTH		9
>  #define EPT_PGDIR_MASK		511
> +#define EPT_ADDR_MASK		0xffffffffff000ul

This is just PAGE_MASK, defined one line below. :)

Paolo

>  #define PAGE_MASK		(~(PAGE_SIZE-1))
>  #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
>  
> 

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests 0/5] Split large EPT mappings properly
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
                   ` (4 preceding siblings ...)
  2016-03-01 19:30 ` [kvm-unit-tests 5/5] x86: vmx: don't explicitly split identity EPT map Peter Feiner
@ 2016-03-01 21:13 ` Paolo Bonzini
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
  7 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-03-01 21:13 UTC (permalink / raw)
  To: Peter Feiner, kvm, jan.kiszka



On 01/03/2016 20:30, Peter Feiner wrote:
> install_ept_entry would overwrite arbitrary memory when it encountered
> a large page.
> 
> I discovered the bug while working on a simple multi-page allocator,
> which I'm using for huge page testing, that sorts the free list in
> ascending order of HPA.
> 
> Peter Feiner (5):
>   x86: vmx.h: trivial whitespace fixes
>   x86: vmx: Named constant: EPT_ADDR_MASK
>   x86: vmx: Named constant: EPT_LEVEL_SHIFT
>   x86: vmx: split large EPTEs in install_ept_entry
>   x86: vmx: don't explicitly split identity EPT map
> 
>  x86/vmx.c       | 65 +++++++++++++++++++++++++++++++++++++++++++--------------
>  x86/vmx.h       |  9 +++++---
>  x86/vmx_tests.c |  7 -------
>  3 files changed, 55 insertions(+), 26 deletions(-)

Looks good apart for not reusing PAGE_MASK in patch 2.  Thanks!

Paolo

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 21:12   ` Paolo Bonzini
@ 2016-03-01 21:16     ` Peter Feiner
  2016-03-01 21:27       ` Jan Kiszka
  0 siblings, 1 reply; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 21:16 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: kvm, J. Kiszka

On Tue, Mar 1, 2016 at 1:12 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
>
> On 01/03/2016 20:30, Peter Feiner wrote:
>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>> ---
>>  x86/vmx.c | 6 +++---
>>  x86/vmx.h | 1 +
>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/x86/vmx.c b/x86/vmx.c
>> index 3fa1a73..9811a28 100644
>> --- a/x86/vmx.c
>> +++ b/x86/vmx.c
>> @@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
>>                                       | EPT_RA | EPT_WA | EPT_EA;
>>               } else
>>                       pt[offset] &= ~EPT_LARGE_PAGE;
>> -             pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
>> +             pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
>>       }
>>       offset = ((unsigned long)guest_addr >> ((level-1) *
>>                       EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
>> @@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
>>                       break;
>>               if (l < 4 && (pte & EPT_LARGE_PAGE))
>>                       return pte;
>> -             pt = (unsigned long *)(pte & 0xffffffffff000ull);
>> +             pt = (unsigned long *)(pte & EPT_ADDR_MASK);
>>       }
>>       offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>>                       & EPT_PGDIR_MASK;
>> @@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
>>                       break;
>>               if (!(pt[offset] & (EPT_PRESENT)))
>>                       return -1;
>> -             pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
>> +             pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
>>       }
>>       offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>>                       & EPT_PGDIR_MASK;
>> diff --git a/x86/vmx.h b/x86/vmx.h
>> index cae9274..c2a2121 100644
>> --- a/x86/vmx.h
>> +++ b/x86/vmx.h
>> @@ -466,6 +466,7 @@ enum Ctrl1 {
>>  #define EPT_PAGE_LEVEL               4
>>  #define EPT_PGDIR_WIDTH              9
>>  #define EPT_PGDIR_MASK               511
>> +#define EPT_ADDR_MASK                0xffffffffff000ul
>
> This is just PAGE_MASK, defined one line below. :)
>
> Paolo
>
>>  #define PAGE_MASK            (~(PAGE_SIZE-1))
>>  #define PAGE_MASK_2M         (~(PAGE_SIZE_2M-1))
>>
>>

It's not precisely the same thing since PAGE_MASK sets the upper
reserved bits :-)

I'll send v2.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 21:16     ` Peter Feiner
@ 2016-03-01 21:27       ` Jan Kiszka
  2016-03-01 21:36         ` Paolo Bonzini
  0 siblings, 1 reply; 29+ messages in thread
From: Jan Kiszka @ 2016-03-01 21:27 UTC (permalink / raw)
  To: Peter Feiner, Paolo Bonzini; +Cc: kvm

On 2016-03-01 22:16, Peter Feiner wrote:
> On Tue, Mar 1, 2016 at 1:12 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
>>
>>
>> On 01/03/2016 20:30, Peter Feiner wrote:
>>> Signed-off-by: Peter Feiner <pfeiner@google.com>
>>> ---
>>>  x86/vmx.c | 6 +++---
>>>  x86/vmx.h | 1 +
>>>  2 files changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/x86/vmx.c b/x86/vmx.c
>>> index 3fa1a73..9811a28 100644
>>> --- a/x86/vmx.c
>>> +++ b/x86/vmx.c
>>> @@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
>>>                                       | EPT_RA | EPT_WA | EPT_EA;
>>>               } else
>>>                       pt[offset] &= ~EPT_LARGE_PAGE;
>>> -             pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
>>> +             pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
>>>       }
>>>       offset = ((unsigned long)guest_addr >> ((level-1) *
>>>                       EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
>>> @@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
>>>                       break;
>>>               if (l < 4 && (pte & EPT_LARGE_PAGE))
>>>                       return pte;
>>> -             pt = (unsigned long *)(pte & 0xffffffffff000ull);
>>> +             pt = (unsigned long *)(pte & EPT_ADDR_MASK);
>>>       }
>>>       offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>>>                       & EPT_PGDIR_MASK;
>>> @@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
>>>                       break;
>>>               if (!(pt[offset] & (EPT_PRESENT)))
>>>                       return -1;
>>> -             pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
>>> +             pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
>>>       }
>>>       offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>>>                       & EPT_PGDIR_MASK;
>>> diff --git a/x86/vmx.h b/x86/vmx.h
>>> index cae9274..c2a2121 100644
>>> --- a/x86/vmx.h
>>> +++ b/x86/vmx.h
>>> @@ -466,6 +466,7 @@ enum Ctrl1 {
>>>  #define EPT_PAGE_LEVEL               4
>>>  #define EPT_PGDIR_WIDTH              9
>>>  #define EPT_PGDIR_MASK               511
>>> +#define EPT_ADDR_MASK                0xffffffffff000ul
>>
>> This is just PAGE_MASK, defined one line below. :)
>>
>> Paolo
>>
>>>  #define PAGE_MASK            (~(PAGE_SIZE-1))
>>>  #define PAGE_MASK_2M         (~(PAGE_SIZE_2M-1))
>>>
>>>
> 
> It's not precisely the same thing since PAGE_MASK sets the upper
> reserved bits :-)
> 
> I'll send v2.

Consider defining the constant as GENMASK(51, 12) (from linux/bitops.h).

A while back, I re-invented this macro for Jailhouse, and I just
realized that the kernel has the very same thing. It makes such bitmasks
much more readable (and verifiable when looking at specs).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 21:27       ` Jan Kiszka
@ 2016-03-01 21:36         ` Paolo Bonzini
  0 siblings, 0 replies; 29+ messages in thread
From: Paolo Bonzini @ 2016-03-01 21:36 UTC (permalink / raw)
  To: Jan Kiszka, Peter Feiner; +Cc: kvm



On 01/03/2016 22:27, Jan Kiszka wrote:
> > It's not precisely the same thing since PAGE_MASK sets the upper
> > reserved bits :-)

Aha! :)  Jan's suggestion is obviously a good one, please adopt it for v2.

Paolo

> > I'll send v2.
> 
> Consider defining the constant as GENMASK(51, 12) (from linux/bitops.h).
> 
> A while back, I re-invented this macro for Jailhouse, and I just
> realized that the kernel has the very same thing. It makes such bitmasks
> much more readable (and verifiable when looking at specs).

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 0/6] Split large EPT mappings properly
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
                   ` (5 preceding siblings ...)
  2016-03-01 21:13 ` [kvm-unit-tests 0/5] Split large EPT mappings properly Paolo Bonzini
@ 2016-03-01 22:34 ` Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
                     ` (5 more replies)
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
  7 siblings, 6 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

install_ept_entry would overwrite arbitrary memory when it encountered
a large page.

I discovered the bug while working on a simple multi-page allocator,
which I'm using for huge page testing, that sorts the free list in
ascending order of HPA.

v2:
	* Added lib/bitops.h for GENMASK.
	* Factored common stuff out of lib/*/asm/bitops.h
	* Fixed all of the whitespace in vmx.h.

Peter Feiner (6):
  x86: vmx.h: trivial whitespace fixes
  lib: generic bitops.h
  x86: vmx: Named constant: EPT_ADDR_MASK
  x86: vmx: Named constant: EPT_LEVEL_SHIFT
  x86: vmx: split large EPTEs in install_ept_entry
  x86: vmx: don't explicitly split identity EPT map

 lib/arm/asm/bitops.h   |   8 ++--
 lib/arm/asm/cpumask.h  |   2 +-
 lib/arm/bitops.c       |   2 +-
 lib/arm64/asm/bitops.h |   8 ++--
 lib/bitops.h           |  35 ++++++++++++++++
 lib/ppc64/asm/bitops.h |  10 +++++
 lib/x86/asm/bitops.h   |  14 +++++++
 x86/vmx.c              |  65 ++++++++++++++++++++++-------
 x86/vmx.h              | 111 +++++++++++++++++++++++++------------------------
 x86/vmx_tests.c        |   7 ----
 10 files changed, 175 insertions(+), 87 deletions(-)
-- 
2.7.0.rc3.207.g0ac5344

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 2/6] lib: generic bitops.h Peter Feiner
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.h | 107 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 53 insertions(+), 54 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index f868e5d..8b79191 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -196,8 +196,8 @@ enum Encoding {
 	/* Natural-Width Control Fields */
 	CR0_MASK		= 0x6000ul,
 	CR4_MASK		= 0x6002ul,
-	CR0_READ_SHADOW	= 0x6004ul,
-	CR4_READ_SHADOW	= 0x6006ul,
+	CR0_READ_SHADOW		= 0x6004ul,
+	CR4_READ_SHADOW		= 0x6006ul,
 	CR3_TARGET_0		= 0x6008ul,
 	CR3_TARGET_1		= 0x600aul,
 	CR3_TARGET_2		= 0x600cul,
@@ -304,27 +304,27 @@ enum Reason {
 
 enum Ctrl_exi {
 	EXI_SAVE_DBGCTLS	= 1UL << 2,
-	EXI_HOST_64             = 1UL << 9,
+	EXI_HOST_64		= 1UL << 9,
 	EXI_LOAD_PERF		= 1UL << 12,
-	EXI_INTA                = 1UL << 15,
+	EXI_INTA		= 1UL << 15,
 	EXI_SAVE_PAT		= 1UL << 18,
 	EXI_LOAD_PAT		= 1UL << 19,
 	EXI_SAVE_EFER		= 1UL << 20,
-	EXI_LOAD_EFER           = 1UL << 21,
+	EXI_LOAD_EFER		= 1UL << 21,
 	EXI_SAVE_PREEMPT	= 1UL << 22,
 };
 
 enum Ctrl_ent {
 	ENT_LOAD_DBGCTLS	= 1UL << 2,
-	ENT_GUEST_64            = 1UL << 9,
+	ENT_GUEST_64		= 1UL << 9,
 	ENT_LOAD_PAT		= 1UL << 14,
-	ENT_LOAD_EFER           = 1UL << 15,
+	ENT_LOAD_EFER		= 1UL << 15,
 };
 
 enum Ctrl_pin {
-	PIN_EXTINT              = 1ul << 0,
-	PIN_NMI                 = 1ul << 3,
-	PIN_VIRT_NMI            = 1ul << 5,
+	PIN_EXTINT		= 1ul << 0,
+	PIN_NMI			= 1ul << 3,
+	PIN_VIRT_NMI		= 1ul << 5,
 	PIN_PREEMPT		= 1ul << 6,
 };
 
@@ -396,85 +396,85 @@ enum Ctrl1 {
 #define LOAD_GPR_C	SAVE_GPR_C
 
 #define SAVE_RFLAGS		\
-	"pushf\n\t"			\
+	"pushf\n\t"		\
 	"pop host_rflags\n\t"
 
 #define LOAD_RFLAGS		\
 	"push host_rflags\n\t"	\
 	"popf\n\t"
 
-#define VMX_IO_SIZE_MASK		0x7
-#define _VMX_IO_BYTE			0
-#define _VMX_IO_WORD			1
-#define _VMX_IO_LONG			3
-#define VMX_IO_DIRECTION_MASK		(1ul << 3)
-#define VMX_IO_IN			(1ul << 3)
-#define VMX_IO_OUT			0
-#define VMX_IO_STRING			(1ul << 4)
-#define VMX_IO_REP			(1ul << 5)
-#define VMX_IO_OPRAND_IMM		(1ul << 6)
-#define VMX_IO_PORT_MASK		0xFFFF0000
-#define VMX_IO_PORT_SHIFT		16
-
-#define VMX_TEST_START			0
-#define VMX_TEST_VMEXIT			1
-#define VMX_TEST_EXIT			2
-#define VMX_TEST_RESUME			3
-#define VMX_TEST_LAUNCH_ERR		4
-#define VMX_TEST_RESUME_ERR		5
+#define VMX_IO_SIZE_MASK	0x7
+#define _VMX_IO_BYTE		0
+#define _VMX_IO_WORD		1
+#define _VMX_IO_LONG		3
+#define VMX_IO_DIRECTION_MASK	(1ul << 3)
+#define VMX_IO_IN		(1ul << 3)
+#define VMX_IO_OUT		0
+#define VMX_IO_STRING		(1ul << 4)
+#define VMX_IO_REP		(1ul << 5)
+#define VMX_IO_OPRAND_IMM	(1ul << 6)
+#define VMX_IO_PORT_MASK	0xFFFF0000
+#define VMX_IO_PORT_SHIFT	16
+
+#define VMX_TEST_START		0
+#define VMX_TEST_VMEXIT		1
+#define VMX_TEST_EXIT		2
+#define VMX_TEST_RESUME		3
+#define VMX_TEST_LAUNCH_ERR	4
+#define VMX_TEST_RESUME_ERR	5
 
 #define HYPERCALL_BIT		(1ul << 12)
 #define HYPERCALL_MASK		0xFFF
 #define HYPERCALL_VMEXIT	0x1
 
 #define EPTP_PG_WALK_LEN_SHIFT	3ul
-#define EPTP_AD_FLAG			(1ul << 6)
+#define EPTP_AD_FLAG		(1ul << 6)
 
-#define EPT_MEM_TYPE_UC	0ul
-#define EPT_MEM_TYPE_WC	1ul
-#define EPT_MEM_TYPE_WT	4ul
-#define EPT_MEM_TYPE_WP	5ul
-#define EPT_MEM_TYPE_WB	6ul
+#define EPT_MEM_TYPE_UC		0ul
+#define EPT_MEM_TYPE_WC		1ul
+#define EPT_MEM_TYPE_WT		4ul
+#define EPT_MEM_TYPE_WP		5ul
+#define EPT_MEM_TYPE_WB		6ul
 
 #define EPT_RA			1ul
 #define EPT_WA			2ul
 #define EPT_EA			4ul
-#define EPT_PRESENT		(EPT_RA | EPT_WA | EPT_EA)	
-#define EPT_ACCESS_FLAG	(1ul << 8)
+#define EPT_PRESENT		(EPT_RA | EPT_WA | EPT_EA)
+#define EPT_ACCESS_FLAG		(1ul << 8)
 #define EPT_DIRTY_FLAG		(1ul << 9)
 #define EPT_LARGE_PAGE		(1ul << 7)
 #define EPT_MEM_TYPE_SHIFT	3ul
 #define EPT_IGNORE_PAT		(1ul << 6)
-#define EPT_SUPPRESS_VE	(1ull << 63)
+#define EPT_SUPPRESS_VE		(1ull << 63)
 
 #define EPT_CAP_WT		1ull
 #define EPT_CAP_PWL4		(1ull << 6)
 #define EPT_CAP_UC		(1ull << 8)
 #define EPT_CAP_WB		(1ull << 14)
-#define EPT_CAP_2M_PAGE	(1ull << 16)
-#define EPT_CAP_1G_PAGE	(1ull << 17)
+#define EPT_CAP_2M_PAGE		(1ull << 16)
+#define EPT_CAP_1G_PAGE		(1ull << 17)
 #define EPT_CAP_INVEPT		(1ull << 20)
 #define EPT_CAP_INVEPT_SINGLE	(1ull << 25)
 #define EPT_CAP_INVEPT_ALL	(1ull << 26)
-#define EPT_CAP_AD_FLAG	(1ull << 21)
-#define VPID_CAP_INVVPID (1ull << 32)
-#define VPID_CAP_INVVPID_SINGLE  (1ull << 41)
-#define VPID_CAP_INVVPID_ALL  (1ull << 42)
+#define EPT_CAP_AD_FLAG		(1ull << 21)
+#define VPID_CAP_INVVPID	(1ull << 32)
+#define VPID_CAP_INVVPID_SINGLE	(1ull << 41)
+#define VPID_CAP_INVVPID_ALL	(1ull << 42)
 
 #define PAGE_SIZE_2M		(512 * PAGE_SIZE)
 #define PAGE_SIZE_1G		(512 * PAGE_SIZE_2M)
-#define	EPT_PAGE_LEVEL	4
-#define	EPT_PGDIR_WIDTH	9
-#define	EPT_PGDIR_MASK	511
+#define EPT_PAGE_LEVEL		4
+#define EPT_PGDIR_WIDTH		9
+#define EPT_PGDIR_MASK		511
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
 #define EPT_VLT_RD		1
 #define EPT_VLT_WR		(1 << 1)
 #define EPT_VLT_FETCH		(1 << 2)
-#define EPT_VLT_PERM_RD	(1 << 3)
-#define EPT_VLT_PERM_WR	(1 << 4)
-#define EPT_VLT_PERM_EX	(1 << 5)
+#define EPT_VLT_PERM_RD		(1 << 3)
+#define EPT_VLT_PERM_WR		(1 << 4)
+#define EPT_VLT_PERM_EX		(1 << 5)
 #define EPT_VLT_LADDR_VLD	(1 << 7)
 #define EPT_VLT_PADDR		(1 << 8)
 
@@ -485,8 +485,8 @@ enum Ctrl1 {
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
 
-#define INVVPID_SINGLE      1
-#define INVVPID_ALL         2
+#define INVVPID_SINGLE		1
+#define INVVPID_ALL		2
 
 #define ACTV_ACTIVE		0
 #define ACTV_HLT		1
@@ -577,4 +577,3 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 		int level, u64 pte_val);
 
 #endif
-
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 2/6] lib: generic bitops.h
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
                     ` (3 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Factored out common bitops stuff, just like Linux's
include/linux/bitops.h.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/arm/asm/bitops.h   |  8 ++++----
 lib/arm/asm/cpumask.h  |  2 +-
 lib/arm/bitops.c       |  2 +-
 lib/arm64/asm/bitops.h |  8 ++++----
 lib/bitops.h           | 35 +++++++++++++++++++++++++++++++++++
 lib/ppc64/asm/bitops.h | 10 ++++++++++
 lib/x86/asm/bitops.h   | 14 ++++++++++++++
 7 files changed, 69 insertions(+), 10 deletions(-)
 create mode 100644 lib/bitops.h
 create mode 100644 lib/ppc64/asm/bitops.h
 create mode 100644 lib/x86/asm/bitops.h

diff --git a/lib/arm/asm/bitops.h b/lib/arm/asm/bitops.h
index 8049634..d46cc5d 100644
--- a/lib/arm/asm/bitops.h
+++ b/lib/arm/asm/bitops.h
@@ -2,7 +2,6 @@
 #define _ASMARM_BITOPS_H_
 /*
  * Adapated from
- *   include/linux/bitops.h
  *   arch/arm/lib/bitops.h
  *
  * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
@@ -10,10 +9,11 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
 #define BITS_PER_LONG	32
-#define BIT(nr)		(1UL << (nr))
-#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
 
 #define ATOMIC_BITOP(insn, mask, word)				\
 ({								\
diff --git a/lib/arm/asm/cpumask.h b/lib/arm/asm/cpumask.h
index 85b8e4b..6683bb6 100644
--- a/lib/arm/asm/cpumask.h
+++ b/lib/arm/asm/cpumask.h
@@ -8,7 +8,7 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <asm/setup.h>
-#include <asm/bitops.h>
+#include <bitops.h>
 
 #define CPUMASK_NR_LONGS ((NR_CPUS + BITS_PER_LONG - 1) / BITS_PER_LONG)
 
diff --git a/lib/arm/bitops.c b/lib/arm/bitops.c
index 9ad1121..1f1db93 100644
--- a/lib/arm/bitops.c
+++ b/lib/arm/bitops.c
@@ -6,7 +6,7 @@
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
-#include <asm/bitops.h>
+#include <bitops.h>
 #include <asm/barrier.h>
 #include <asm/mmu.h>
 
diff --git a/lib/arm64/asm/bitops.h b/lib/arm64/asm/bitops.h
index 3371c60..618468c 100644
--- a/lib/arm64/asm/bitops.h
+++ b/lib/arm64/asm/bitops.h
@@ -2,7 +2,6 @@
 #define _ASMARM64_BITOPS_H_
 /*
  * Adapated from
- *   include/linux/bitops.h
  *   arch/arm64/lib/bitops.S
  *
  * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
@@ -10,10 +9,11 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
 #define BITS_PER_LONG	64
-#define BIT(nr)		(1UL << (nr))
-#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
 
 #define ATOMIC_BITOP(insn, mask, word)				\
 ({								\
diff --git a/lib/bitops.h b/lib/bitops.h
new file mode 100644
index 0000000..c550eba
--- /dev/null
+++ b/lib/bitops.h
@@ -0,0 +1,35 @@
+#ifndef _BITOPS_H_
+#define _BITOPS_H_
+
+/*
+ * Adapated from
+ *   include/linux/bitops.h
+ *
+ * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#define BIT(nr)			(1UL << (nr))
+#define BIT_ULL(nr)		(1ULL << (nr))
+#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
+#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
+#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
+#define BITS_PER_BYTE		8
+#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+
+#include <asm/bitops.h>
+
+/*
+ * Create a contiguous bitmask starting at bit position @l and ending at
+ * position @h. For example
+ * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
+ */
+#define GENMASK(h, l) \
+	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+
+#define GENMASK_ULL(h, l) \
+	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+
+#endif
diff --git a/lib/ppc64/asm/bitops.h b/lib/ppc64/asm/bitops.h
new file mode 100644
index 0000000..34624b4
--- /dev/null
+++ b/lib/ppc64/asm/bitops.h
@@ -0,0 +1,10 @@
+#ifndef _ASMPPC64_BITOPS_H_
+#define _ASMPPC64_BITOPS_H_
+
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
+#define BITS_PER_LONG	64
+
+#endif
diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
new file mode 100644
index 0000000..eb4aaa9
--- /dev/null
+++ b/lib/x86/asm/bitops.h
@@ -0,0 +1,14 @@
+#ifndef _ASMX86_BITOPS_H_
+#define _ASMX86_BITOPS_H_
+
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
+#ifdef __x86_64__
+#define BITS_PER_LONG	64
+#else
+#define BITS_PER_LONG	32
+#endif
+
+#endif
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 2/6] lib: generic bitops.h Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  2016-03-02  6:24     ` Jan Kiszka
  2016-03-01 22:34   ` [kvm-unit-tests v2 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
                     ` (2 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 6 +++---
 x86/vmx.h | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 3fa1a73..9811a28 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
 					| EPT_RA | EPT_WA | EPT_EA;
 		} else
 			pt[offset] &= ~EPT_LARGE_PAGE;
-		pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
+		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = ((unsigned long)guest_addr >> ((level-1) *
 			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
@@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			break;
 		if (l < 4 && (pte & EPT_LARGE_PAGE))
 			return pte;
-		pt = (unsigned long *)(pte & 0xffffffffff000ull);
+		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
@@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
-		pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
+		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index 8b79191..616ffc0 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -3,6 +3,7 @@
 
 #include "libcflat.h"
 #include "processor.h"
+#include "bitops.h"
 
 struct vmcs {
 	u32 revision_id; /* vmcs revision identifier */
@@ -466,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_ADDR_MASK		GENMASK(52, 11)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
                     ` (2 preceding siblings ...)
  2016-03-01 22:34   ` [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 17 ++++++-----------
 x86/vmx.h |  1 +
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 9811a28..140ad86 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -233,7 +233,7 @@ void install_ept_entry(unsigned long *pml4,
 	unsigned offset;
 
 	for (level = EPT_PAGE_LEVEL; level > pte_level; --level) {
-		offset = (guest_addr >> ((level-1) * EPT_PGDIR_WIDTH + 12))
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(level))
 				& EPT_PGDIR_MASK;
 		if (!(pt[offset] & (EPT_PRESENT))) {
 			unsigned long *new_pt = pt_page;
@@ -248,8 +248,7 @@ void install_ept_entry(unsigned long *pml4,
 			pt[offset] &= ~EPT_LARGE_PAGE;
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = ((unsigned long)guest_addr >> ((level-1) *
-			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
 	pt[offset] = pte;
 }
 
@@ -325,8 +324,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		pte = pt[offset];
 		if (!(pte & (EPT_PRESENT)))
 			return 0;
@@ -336,8 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			return pte;
 		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pte = pt[offset];
 	return pte;
 }
@@ -372,16 +369,14 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		if (l == level)
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
 		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pt[offset] = pte_val;
 	return 0;
 }
diff --git a/x86/vmx.h b/x86/vmx.h
index 616ffc0..018051a 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -467,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		GENMASK(52, 11)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 5/6] x86: vmx: split large EPTEs in install_ept_entry
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
                     ` (3 preceding siblings ...)
  2016-03-01 22:34   ` [kvm-unit-tests v2 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  2016-03-01 22:34   ` [kvm-unit-tests v2 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

When install_ept_entry encountered a leaf entry above pte_level, it
just cleared the EPT_LARGE_PAGE bit and continued the traversal _using
the first 4K page from the large page as the next level of the page
table_! This is broken because (1) the data in the large page would be
overwritten, and (2) all of other entires in the new level of the page
table would contain garbage.

Now, install_ept_entry splits the large mapping by allocating a new
page and filling it in with 512 PTEs that point to the large page's
constituent 2M or 4K pages.

This path is exercised in the VMX EPT test when 2m EPT pages are
enabled. The bug wasn't obvious because the free list is sorted in
descending order of HPA, thus the large page being overwritten with an
EPTE happened to only contain 0s.

Fixes: 04b0e0f342978f08b8b0b068c08c9d45ee80e3f7 ("nEPT: Fix test cases for 2M huge pages").
Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 x86/vmx.h |  1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 140ad86..d3fdc71 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -215,6 +215,44 @@ asm(
 );
 
 /* EPT paging structure related functions */
+/* split_large_ept_entry: Split a 2M/1G large page into 512 smaller PTEs.
+		@ptep : large page table entry to split
+		@level : level of ptep (2 or 3)
+ */
+static void split_large_ept_entry(unsigned long *ptep, int level)
+{
+	unsigned long *new_pt;
+	unsigned long gpa;
+	unsigned long pte;
+	unsigned long prototype;
+	int i;
+
+	pte = *ptep;
+	assert(pte & EPT_PRESENT);
+	assert(pte & EPT_LARGE_PAGE);
+	assert(level == 2 || level == 3);
+
+	new_pt = alloc_page();
+	assert(new_pt);
+	memset(new_pt, 0, PAGE_SIZE);
+
+	prototype = pte & ~EPT_ADDR_MASK;
+	if (level == 2)
+		prototype &= ~EPT_LARGE_PAGE;
+
+	gpa = pte & EPT_ADDR_MASK;
+	for (i = 0; i < EPT_PGDIR_ENTRIES; i++) {
+		new_pt[i] = prototype | gpa;
+		gpa += 1ul << EPT_LEVEL_SHIFT(level - 1);
+	}
+
+	pte &= ~EPT_LARGE_PAGE;
+	pte &= ~EPT_ADDR_MASK;
+	pte |= virt_to_phys(new_pt);
+
+	*ptep = pte;
+}
+
 /* install_ept_entry : Install a page to a given level in EPT
 		@pml4 : addr of pml4 table
 		@pte_level : level of PTE to set
@@ -244,8 +282,8 @@ void install_ept_entry(unsigned long *pml4,
 			memset(new_pt, 0, PAGE_SIZE);
 			pt[offset] = virt_to_phys(new_pt)
 					| EPT_RA | EPT_WA | EPT_EA;
-		} else
-			pt[offset] &= ~EPT_LARGE_PAGE;
+		} else if (pt[offset] & EPT_LARGE_PAGE)
+			split_large_ept_entry(&pt[offset], level);
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index 018051a..35b5431 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -467,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_PGDIR_ENTRIES	(1 << EPT_PGDIR_WIDTH)
 #define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		GENMASK(52, 11)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v2 6/6] x86: vmx: don't explicitly split identity EPT map
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
                     ` (4 preceding siblings ...)
  2016-03-01 22:34   ` [kvm-unit-tests v2 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
@ 2016-03-01 22:34   ` Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-01 22:34 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Since install_ept_entry automatically splits large EPTEs, it's no
longer necessary to explicitly pave the EPT identity map with 4K
entries mapping [base_addr1, base_addr1 + PAGE_SIZE_2M). I.e.,
install_ept(data_page1, data_page2) does the paving before installing
the 4k data_page1 -> data_page2 mapping.

The other setup_ept_range call was unnecessary.

Note that there was benign bug in the deleted code. The third argument
of setup_ept_range is length not end! Thus the range of GPA mappings
being split was [base_addr1, base_addr1 * 2 + PAGE_SIZE_2M).

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx_tests.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bb65c06..4ef2247 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -954,7 +954,6 @@ static int apic_version;
 
 static int ept_init()
 {
-	unsigned long base_addr1, base_addr2;
 	u32 ctrl_cpu[2];
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
@@ -979,12 +978,6 @@ static int ept_init()
 	memset(data_page2, 0x0, PAGE_SIZE);
 	*((u32 *)data_page1) = MAGIC_VAL_1;
 	*((u32 *)data_page2) = MAGIC_VAL_2;
-	base_addr1 = (unsigned long)data_page1 & PAGE_MASK_2M;
-	base_addr2 = (unsigned long)data_page2 & PAGE_MASK_2M;
-	setup_ept_range(pml4, base_addr1, base_addr1 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
-	setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-01 22:34   ` [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
@ 2016-03-02  6:24     ` Jan Kiszka
  2016-03-02  8:47       ` Paolo Bonzini
  0 siblings, 1 reply; 29+ messages in thread
From: Jan Kiszka @ 2016-03-02  6:24 UTC (permalink / raw)
  To: Peter Feiner, kvm, drjones, pbonzini

On 2016-03-01 23:34, Peter Feiner wrote:
> Signed-off-by: Peter Feiner <pfeiner@google.com>
> ---
>  x86/vmx.c | 6 +++---
>  x86/vmx.h | 2 ++
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/x86/vmx.c b/x86/vmx.c
> index 3fa1a73..9811a28 100644
> --- a/x86/vmx.c
> +++ b/x86/vmx.c
> @@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
>  					| EPT_RA | EPT_WA | EPT_EA;
>  		} else
>  			pt[offset] &= ~EPT_LARGE_PAGE;
> -		pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
> +		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
>  	}
>  	offset = ((unsigned long)guest_addr >> ((level-1) *
>  			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
> @@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
>  			break;
>  		if (l < 4 && (pte & EPT_LARGE_PAGE))
>  			return pte;
> -		pt = (unsigned long *)(pte & 0xffffffffff000ull);
> +		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
>  	}
>  	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>  			& EPT_PGDIR_MASK;
> @@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
>  			break;
>  		if (!(pt[offset] & (EPT_PRESENT)))
>  			return -1;
> -		pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
> +		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
>  	}
>  	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
>  			& EPT_PGDIR_MASK;
> diff --git a/x86/vmx.h b/x86/vmx.h
> index 8b79191..616ffc0 100644
> --- a/x86/vmx.h
> +++ b/x86/vmx.h
> @@ -3,6 +3,7 @@
>  
>  #include "libcflat.h"
>  #include "processor.h"
> +#include "bitops.h"
>  
>  struct vmcs {
>  	u32 revision_id; /* vmcs revision identifier */
> @@ -466,6 +467,7 @@ enum Ctrl1 {
>  #define EPT_PAGE_LEVEL		4
>  #define EPT_PGDIR_WIDTH		9
>  #define EPT_PGDIR_MASK		511
> +#define EPT_ADDR_MASK		GENMASK(52, 11)

The kernel macro looks different from mine, but I think it gives the
same result... yes: this must be really GENMASK(51, 12) (bits 51..12).

Jan

-- 
Siemens AG, Corporate Technology, CT RDA ITP SES-DE
Corporate Competence Center Embedded Linux

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-02  6:24     ` Jan Kiszka
@ 2016-03-02  8:47       ` Paolo Bonzini
  2016-03-02 16:50         ` Peter Feiner
  0 siblings, 1 reply; 29+ messages in thread
From: Paolo Bonzini @ 2016-03-02  8:47 UTC (permalink / raw)
  To: Jan Kiszka, Peter Feiner, kvm, drjones



On 02/03/2016 07:24, Jan Kiszka wrote:
> The kernel macro looks different from mine, but I think it gives the
> same result... yes: this must be really GENMASK(51, 12) (bits 51..12).

While at it, it should probably be GENMASK_ULL(51, 12), even though 
right now the test case is only 64-bit.  Which in turn would show that 
BITS_PER_LONG_LONG is not defined anywhere:

$ git grep PER_LONG_LONG
lib/bitops.h:#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
lib/bitops.h:#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
lib/bitops.h:	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

Paolo

^ permalink raw reply	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-02  8:47       ` Paolo Bonzini
@ 2016-03-02 16:50         ` Peter Feiner
  0 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 16:50 UTC (permalink / raw)
  To: Paolo Bonzini; +Cc: Jan Kiszka, kvm, Andrew Jones

On Wed, Mar 2, 2016 at 12:47 AM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> On 02/03/2016 07:24, Jan Kiszka wrote:
>> The kernel macro looks different from mine, but I think it gives the
>> same result... yes: this must be really GENMASK(51, 12) (bits 51..12).
>
> While at it, it should probably be GENMASK_ULL(51, 12), even though
> right now the test case is only 64-bit.  Which in turn would show that
> BITS_PER_LONG_LONG is not defined anywhere:
>
> $ git grep PER_LONG_LONG
> lib/bitops.h:#define BIT_ULL_MASK(nr)   (1ULL << ((nr) % BITS_PER_LONG_LONG))
> lib/bitops.h:#define BIT_ULL_WORD(nr)   ((nr) / BITS_PER_LONG_LONG)
> lib/bitops.h:   (((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))

Obviously I didn't actually run the test! It's fixed now. I've also
defined BITS_PER_LONG_LONG in lib/bitops.h. Will send v3.

^ permalink raw reply	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 0/6] Split large EPT mappings properly
  2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
                   ` (6 preceding siblings ...)
  2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
@ 2016-03-02 17:10 ` Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
                     ` (5 more replies)
  7 siblings, 6 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

install_ept_entry would overwrite arbitrary memory when it encountered
a large page.

I discovered the bug while working on a simple multi-page allocator,
which I'm using for huge page testing, that sorts the free list in
ascending order of HPA.

v3:
	* Added #define BITS_PER_LONG_LONG 64
	* Fixed EPT_ADDR_MASK bit range
	* Made EPT_ADDR_MASK unsigned long long

v2:
	* Added lib/bitops.h for GENMASK.
	* Factored common stuff out of lib/*/asm/bitops.h
	* Fixed all of the whitespace in vmx.h.

Peter Feiner (6):
  x86: vmx.h: trivial whitespace fixes
  lib: generic bitops.h
  x86: vmx: Named constant: EPT_ADDR_MASK
  x86: vmx: Named constant: EPT_LEVEL_SHIFT
  x86: vmx: split large EPTEs in install_ept_entry
  x86: vmx: don't explicitly split identity EPT map

 lib/arm/asm/bitops.h   |   8 ++--
 lib/arm/asm/cpumask.h  |   2 +-
 lib/arm/bitops.c       |   2 +-
 lib/arm64/asm/bitops.h |   8 ++--
 lib/bitops.h           |  36 ++++++++++++++++
 lib/ppc64/asm/bitops.h |  10 +++++
 lib/x86/asm/bitops.h   |  14 +++++++
 x86/vmx.c              |  65 ++++++++++++++++++++++-------
 x86/vmx.h              | 111 +++++++++++++++++++++++++------------------------
 x86/vmx_tests.c        |   7 ----
 10 files changed, 176 insertions(+), 87 deletions(-)
 create mode 100644 lib/bitops.h
 create mode 100644 lib/ppc64/asm/bitops.h
 create mode 100644 lib/x86/asm/bitops.h

-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 2/6] lib: generic bitops.h Peter Feiner
                     ` (4 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.h | 107 +++++++++++++++++++++++++++++++-------------------------------
 1 file changed, 53 insertions(+), 54 deletions(-)

diff --git a/x86/vmx.h b/x86/vmx.h
index f868e5d..8b79191 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -196,8 +196,8 @@ enum Encoding {
 	/* Natural-Width Control Fields */
 	CR0_MASK		= 0x6000ul,
 	CR4_MASK		= 0x6002ul,
-	CR0_READ_SHADOW	= 0x6004ul,
-	CR4_READ_SHADOW	= 0x6006ul,
+	CR0_READ_SHADOW		= 0x6004ul,
+	CR4_READ_SHADOW		= 0x6006ul,
 	CR3_TARGET_0		= 0x6008ul,
 	CR3_TARGET_1		= 0x600aul,
 	CR3_TARGET_2		= 0x600cul,
@@ -304,27 +304,27 @@ enum Reason {
 
 enum Ctrl_exi {
 	EXI_SAVE_DBGCTLS	= 1UL << 2,
-	EXI_HOST_64             = 1UL << 9,
+	EXI_HOST_64		= 1UL << 9,
 	EXI_LOAD_PERF		= 1UL << 12,
-	EXI_INTA                = 1UL << 15,
+	EXI_INTA		= 1UL << 15,
 	EXI_SAVE_PAT		= 1UL << 18,
 	EXI_LOAD_PAT		= 1UL << 19,
 	EXI_SAVE_EFER		= 1UL << 20,
-	EXI_LOAD_EFER           = 1UL << 21,
+	EXI_LOAD_EFER		= 1UL << 21,
 	EXI_SAVE_PREEMPT	= 1UL << 22,
 };
 
 enum Ctrl_ent {
 	ENT_LOAD_DBGCTLS	= 1UL << 2,
-	ENT_GUEST_64            = 1UL << 9,
+	ENT_GUEST_64		= 1UL << 9,
 	ENT_LOAD_PAT		= 1UL << 14,
-	ENT_LOAD_EFER           = 1UL << 15,
+	ENT_LOAD_EFER		= 1UL << 15,
 };
 
 enum Ctrl_pin {
-	PIN_EXTINT              = 1ul << 0,
-	PIN_NMI                 = 1ul << 3,
-	PIN_VIRT_NMI            = 1ul << 5,
+	PIN_EXTINT		= 1ul << 0,
+	PIN_NMI			= 1ul << 3,
+	PIN_VIRT_NMI		= 1ul << 5,
 	PIN_PREEMPT		= 1ul << 6,
 };
 
@@ -396,85 +396,85 @@ enum Ctrl1 {
 #define LOAD_GPR_C	SAVE_GPR_C
 
 #define SAVE_RFLAGS		\
-	"pushf\n\t"			\
+	"pushf\n\t"		\
 	"pop host_rflags\n\t"
 
 #define LOAD_RFLAGS		\
 	"push host_rflags\n\t"	\
 	"popf\n\t"
 
-#define VMX_IO_SIZE_MASK		0x7
-#define _VMX_IO_BYTE			0
-#define _VMX_IO_WORD			1
-#define _VMX_IO_LONG			3
-#define VMX_IO_DIRECTION_MASK		(1ul << 3)
-#define VMX_IO_IN			(1ul << 3)
-#define VMX_IO_OUT			0
-#define VMX_IO_STRING			(1ul << 4)
-#define VMX_IO_REP			(1ul << 5)
-#define VMX_IO_OPRAND_IMM		(1ul << 6)
-#define VMX_IO_PORT_MASK		0xFFFF0000
-#define VMX_IO_PORT_SHIFT		16
-
-#define VMX_TEST_START			0
-#define VMX_TEST_VMEXIT			1
-#define VMX_TEST_EXIT			2
-#define VMX_TEST_RESUME			3
-#define VMX_TEST_LAUNCH_ERR		4
-#define VMX_TEST_RESUME_ERR		5
+#define VMX_IO_SIZE_MASK	0x7
+#define _VMX_IO_BYTE		0
+#define _VMX_IO_WORD		1
+#define _VMX_IO_LONG		3
+#define VMX_IO_DIRECTION_MASK	(1ul << 3)
+#define VMX_IO_IN		(1ul << 3)
+#define VMX_IO_OUT		0
+#define VMX_IO_STRING		(1ul << 4)
+#define VMX_IO_REP		(1ul << 5)
+#define VMX_IO_OPRAND_IMM	(1ul << 6)
+#define VMX_IO_PORT_MASK	0xFFFF0000
+#define VMX_IO_PORT_SHIFT	16
+
+#define VMX_TEST_START		0
+#define VMX_TEST_VMEXIT		1
+#define VMX_TEST_EXIT		2
+#define VMX_TEST_RESUME		3
+#define VMX_TEST_LAUNCH_ERR	4
+#define VMX_TEST_RESUME_ERR	5
 
 #define HYPERCALL_BIT		(1ul << 12)
 #define HYPERCALL_MASK		0xFFF
 #define HYPERCALL_VMEXIT	0x1
 
 #define EPTP_PG_WALK_LEN_SHIFT	3ul
-#define EPTP_AD_FLAG			(1ul << 6)
+#define EPTP_AD_FLAG		(1ul << 6)
 
-#define EPT_MEM_TYPE_UC	0ul
-#define EPT_MEM_TYPE_WC	1ul
-#define EPT_MEM_TYPE_WT	4ul
-#define EPT_MEM_TYPE_WP	5ul
-#define EPT_MEM_TYPE_WB	6ul
+#define EPT_MEM_TYPE_UC		0ul
+#define EPT_MEM_TYPE_WC		1ul
+#define EPT_MEM_TYPE_WT		4ul
+#define EPT_MEM_TYPE_WP		5ul
+#define EPT_MEM_TYPE_WB		6ul
 
 #define EPT_RA			1ul
 #define EPT_WA			2ul
 #define EPT_EA			4ul
-#define EPT_PRESENT		(EPT_RA | EPT_WA | EPT_EA)	
-#define EPT_ACCESS_FLAG	(1ul << 8)
+#define EPT_PRESENT		(EPT_RA | EPT_WA | EPT_EA)
+#define EPT_ACCESS_FLAG		(1ul << 8)
 #define EPT_DIRTY_FLAG		(1ul << 9)
 #define EPT_LARGE_PAGE		(1ul << 7)
 #define EPT_MEM_TYPE_SHIFT	3ul
 #define EPT_IGNORE_PAT		(1ul << 6)
-#define EPT_SUPPRESS_VE	(1ull << 63)
+#define EPT_SUPPRESS_VE		(1ull << 63)
 
 #define EPT_CAP_WT		1ull
 #define EPT_CAP_PWL4		(1ull << 6)
 #define EPT_CAP_UC		(1ull << 8)
 #define EPT_CAP_WB		(1ull << 14)
-#define EPT_CAP_2M_PAGE	(1ull << 16)
-#define EPT_CAP_1G_PAGE	(1ull << 17)
+#define EPT_CAP_2M_PAGE		(1ull << 16)
+#define EPT_CAP_1G_PAGE		(1ull << 17)
 #define EPT_CAP_INVEPT		(1ull << 20)
 #define EPT_CAP_INVEPT_SINGLE	(1ull << 25)
 #define EPT_CAP_INVEPT_ALL	(1ull << 26)
-#define EPT_CAP_AD_FLAG	(1ull << 21)
-#define VPID_CAP_INVVPID (1ull << 32)
-#define VPID_CAP_INVVPID_SINGLE  (1ull << 41)
-#define VPID_CAP_INVVPID_ALL  (1ull << 42)
+#define EPT_CAP_AD_FLAG		(1ull << 21)
+#define VPID_CAP_INVVPID	(1ull << 32)
+#define VPID_CAP_INVVPID_SINGLE	(1ull << 41)
+#define VPID_CAP_INVVPID_ALL	(1ull << 42)
 
 #define PAGE_SIZE_2M		(512 * PAGE_SIZE)
 #define PAGE_SIZE_1G		(512 * PAGE_SIZE_2M)
-#define	EPT_PAGE_LEVEL	4
-#define	EPT_PGDIR_WIDTH	9
-#define	EPT_PGDIR_MASK	511
+#define EPT_PAGE_LEVEL		4
+#define EPT_PGDIR_WIDTH		9
+#define EPT_PGDIR_MASK		511
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
 #define EPT_VLT_RD		1
 #define EPT_VLT_WR		(1 << 1)
 #define EPT_VLT_FETCH		(1 << 2)
-#define EPT_VLT_PERM_RD	(1 << 3)
-#define EPT_VLT_PERM_WR	(1 << 4)
-#define EPT_VLT_PERM_EX	(1 << 5)
+#define EPT_VLT_PERM_RD		(1 << 3)
+#define EPT_VLT_PERM_WR		(1 << 4)
+#define EPT_VLT_PERM_EX		(1 << 5)
 #define EPT_VLT_LADDR_VLD	(1 << 7)
 #define EPT_VLT_PADDR		(1 << 8)
 
@@ -485,8 +485,8 @@ enum Ctrl1 {
 #define INVEPT_SINGLE		1
 #define INVEPT_GLOBAL		2
 
-#define INVVPID_SINGLE      1
-#define INVVPID_ALL         2
+#define INVVPID_SINGLE		1
+#define INVVPID_ALL		2
 
 #define ACTV_ACTIVE		0
 #define ACTV_HLT		1
@@ -577,4 +577,3 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 		int level, u64 pte_val);
 
 #endif
-
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 2/6] lib: generic bitops.h
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  2016-03-02 18:13     ` Andrew Jones
  2016-03-02 17:10   ` [kvm-unit-tests v3 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Factored out common bitops stuff, just like Linux's
include/linux/bitops.h.

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 lib/arm/asm/bitops.h   |  8 ++++----
 lib/arm/asm/cpumask.h  |  2 +-
 lib/arm/bitops.c       |  2 +-
 lib/arm64/asm/bitops.h |  8 ++++----
 lib/bitops.h           | 36 ++++++++++++++++++++++++++++++++++++
 lib/ppc64/asm/bitops.h | 10 ++++++++++
 lib/x86/asm/bitops.h   | 14 ++++++++++++++
 7 files changed, 70 insertions(+), 10 deletions(-)
 create mode 100644 lib/bitops.h
 create mode 100644 lib/ppc64/asm/bitops.h
 create mode 100644 lib/x86/asm/bitops.h

diff --git a/lib/arm/asm/bitops.h b/lib/arm/asm/bitops.h
index 8049634..d46cc5d 100644
--- a/lib/arm/asm/bitops.h
+++ b/lib/arm/asm/bitops.h
@@ -2,7 +2,6 @@
 #define _ASMARM_BITOPS_H_
 /*
  * Adapated from
- *   include/linux/bitops.h
  *   arch/arm/lib/bitops.h
  *
  * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
@@ -10,10 +9,11 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
 #define BITS_PER_LONG	32
-#define BIT(nr)		(1UL << (nr))
-#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
 
 #define ATOMIC_BITOP(insn, mask, word)				\
 ({								\
diff --git a/lib/arm/asm/cpumask.h b/lib/arm/asm/cpumask.h
index 85b8e4b..6683bb6 100644
--- a/lib/arm/asm/cpumask.h
+++ b/lib/arm/asm/cpumask.h
@@ -8,7 +8,7 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <asm/setup.h>
-#include <asm/bitops.h>
+#include <bitops.h>
 
 #define CPUMASK_NR_LONGS ((NR_CPUS + BITS_PER_LONG - 1) / BITS_PER_LONG)
 
diff --git a/lib/arm/bitops.c b/lib/arm/bitops.c
index 9ad1121..1f1db93 100644
--- a/lib/arm/bitops.c
+++ b/lib/arm/bitops.c
@@ -6,7 +6,7 @@
  *
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
-#include <asm/bitops.h>
+#include <bitops.h>
 #include <asm/barrier.h>
 #include <asm/mmu.h>
 
diff --git a/lib/arm64/asm/bitops.h b/lib/arm64/asm/bitops.h
index 3371c60..618468c 100644
--- a/lib/arm64/asm/bitops.h
+++ b/lib/arm64/asm/bitops.h
@@ -2,7 +2,6 @@
 #define _ASMARM64_BITOPS_H_
 /*
  * Adapated from
- *   include/linux/bitops.h
  *   arch/arm64/lib/bitops.S
  *
  * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
@@ -10,10 +9,11 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
 #define BITS_PER_LONG	64
-#define BIT(nr)		(1UL << (nr))
-#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
-#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
 
 #define ATOMIC_BITOP(insn, mask, word)				\
 ({								\
diff --git a/lib/bitops.h b/lib/bitops.h
new file mode 100644
index 0000000..9aa847e
--- /dev/null
+++ b/lib/bitops.h
@@ -0,0 +1,36 @@
+#ifndef _BITOPS_H_
+#define _BITOPS_H_
+
+/*
+ * Adapated from
+ *   include/linux/bitops.h
+ *
+ * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+
+#define BITS_PER_LONG_LONG	64
+#define BIT(nr)			(1UL << (nr))
+#define BIT_ULL(nr)		(1ULL << (nr))
+#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
+#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
+#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
+#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
+#define BITS_PER_BYTE		8
+#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
+
+#include <asm/bitops.h>
+
+/*
+ * Create a contiguous bitmask starting at bit position @l and ending at
+ * position @h. For example
+ * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
+ */
+#define GENMASK(h, l) \
+	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
+
+#define GENMASK_ULL(h, l) \
+	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
+
+#endif
diff --git a/lib/ppc64/asm/bitops.h b/lib/ppc64/asm/bitops.h
new file mode 100644
index 0000000..34624b4
--- /dev/null
+++ b/lib/ppc64/asm/bitops.h
@@ -0,0 +1,10 @@
+#ifndef _ASMPPC64_BITOPS_H_
+#define _ASMPPC64_BITOPS_H_
+
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
+#define BITS_PER_LONG	64
+
+#endif
diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
new file mode 100644
index 0000000..eb4aaa9
--- /dev/null
+++ b/lib/x86/asm/bitops.h
@@ -0,0 +1,14 @@
+#ifndef _ASMX86_BITOPS_H_
+#define _ASMX86_BITOPS_H_
+
+#ifndef _BITOPS_H_
+#error only <bitops.h> can be included directly
+#endif
+
+#ifdef __x86_64__
+#define BITS_PER_LONG	64
+#else
+#define BITS_PER_LONG	32
+#endif
+
+#endif
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 3/6] x86: vmx: Named constant: EPT_ADDR_MASK
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 2/6] lib: generic bitops.h Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 6 +++---
 x86/vmx.h | 2 ++
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 3fa1a73..9811a28 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -246,7 +246,7 @@ void install_ept_entry(unsigned long *pml4,
 					| EPT_RA | EPT_WA | EPT_EA;
 		} else
 			pt[offset] &= ~EPT_LARGE_PAGE;
-		pt = phys_to_virt(pt[offset] & 0xffffffffff000ull);
+		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = ((unsigned long)guest_addr >> ((level-1) *
 			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
@@ -334,7 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			break;
 		if (l < 4 && (pte & EPT_LARGE_PAGE))
 			return pte;
-		pt = (unsigned long *)(pte & 0xffffffffff000ull);
+		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
@@ -378,7 +378,7 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
-		pt = (unsigned long *)(pt[offset] & 0xffffffffff000ull);
+		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
 			& EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index 8b79191..459dd13 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -3,6 +3,7 @@
 
 #include "libcflat.h"
 #include "processor.h"
+#include "bitops.h"
 
 struct vmcs {
 	u32 revision_id; /* vmcs revision identifier */
@@ -466,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_ADDR_MASK		GENMASK_ULL(51, 12)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
                     ` (2 preceding siblings ...)
  2016-03-02 17:10   ` [kvm-unit-tests v3 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 17 ++++++-----------
 x86/vmx.h |  1 +
 2 files changed, 7 insertions(+), 11 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 9811a28..140ad86 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -233,7 +233,7 @@ void install_ept_entry(unsigned long *pml4,
 	unsigned offset;
 
 	for (level = EPT_PAGE_LEVEL; level > pte_level; --level) {
-		offset = (guest_addr >> ((level-1) * EPT_PGDIR_WIDTH + 12))
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(level))
 				& EPT_PGDIR_MASK;
 		if (!(pt[offset] & (EPT_PRESENT))) {
 			unsigned long *new_pt = pt_page;
@@ -248,8 +248,7 @@ void install_ept_entry(unsigned long *pml4,
 			pt[offset] &= ~EPT_LARGE_PAGE;
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = ((unsigned long)guest_addr >> ((level-1) *
-			EPT_PGDIR_WIDTH + 12)) & EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
 	pt[offset] = pte;
 }
 
@@ -325,8 +324,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		pte = pt[offset];
 		if (!(pte & (EPT_PRESENT)))
 			return 0;
@@ -336,8 +334,7 @@ unsigned long get_ept_pte(unsigned long *pml4,
 			return pte;
 		pt = (unsigned long *)(pte & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pte = pt[offset];
 	return pte;
 }
@@ -372,16 +369,14 @@ int set_ept_pte(unsigned long *pml4, unsigned long guest_addr,
 	if (level < 1 || level > 3)
 		return -1;
 	for (l = EPT_PAGE_LEVEL; ; --l) {
-		offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-				& EPT_PGDIR_MASK;
+		offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 		if (l == level)
 			break;
 		if (!(pt[offset] & (EPT_PRESENT)))
 			return -1;
 		pt = (unsigned long *)(pt[offset] & EPT_ADDR_MASK);
 	}
-	offset = (guest_addr >> (((l-1) * EPT_PGDIR_WIDTH) + 12))
-			& EPT_PGDIR_MASK;
+	offset = (guest_addr >> EPT_LEVEL_SHIFT(l)) & EPT_PGDIR_MASK;
 	pt[offset] = pte_val;
 	return 0;
 }
diff --git a/x86/vmx.h b/x86/vmx.h
index 459dd13..d3a852e 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -467,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		GENMASK_ULL(51, 12)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
 #define PAGE_MASK_2M		(~(PAGE_SIZE_2M-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 5/6] x86: vmx: split large EPTEs in install_ept_entry
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
                     ` (3 preceding siblings ...)
  2016-03-02 17:10   ` [kvm-unit-tests v3 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  2016-03-02 17:10   ` [kvm-unit-tests v3 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

When install_ept_entry encountered a leaf entry above pte_level, it
just cleared the EPT_LARGE_PAGE bit and continued the traversal _using
the first 4K page from the large page as the next level of the page
table_! This is broken because (1) the data in the large page would be
overwritten, and (2) all of other entires in the new level of the page
table would contain garbage.

Now, install_ept_entry splits the large mapping by allocating a new
page and filling it in with 512 PTEs that point to the large page's
constituent 2M or 4K pages.

This path is exercised in the VMX EPT test when 2m EPT pages are
enabled. The bug wasn't obvious because the free list is sorted in
descending order of HPA, thus the large page being overwritten with an
EPTE happened to only contain 0s.

Fixes: 04b0e0f342978f08b8b0b068c08c9d45ee80e3f7 ("nEPT: Fix test cases for 2M huge pages").
Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx.c | 42 ++++++++++++++++++++++++++++++++++++++++--
 x86/vmx.h |  1 +
 2 files changed, 41 insertions(+), 2 deletions(-)

diff --git a/x86/vmx.c b/x86/vmx.c
index 140ad86..d3fdc71 100644
--- a/x86/vmx.c
+++ b/x86/vmx.c
@@ -215,6 +215,44 @@ asm(
 );
 
 /* EPT paging structure related functions */
+/* split_large_ept_entry: Split a 2M/1G large page into 512 smaller PTEs.
+		@ptep : large page table entry to split
+		@level : level of ptep (2 or 3)
+ */
+static void split_large_ept_entry(unsigned long *ptep, int level)
+{
+	unsigned long *new_pt;
+	unsigned long gpa;
+	unsigned long pte;
+	unsigned long prototype;
+	int i;
+
+	pte = *ptep;
+	assert(pte & EPT_PRESENT);
+	assert(pte & EPT_LARGE_PAGE);
+	assert(level == 2 || level == 3);
+
+	new_pt = alloc_page();
+	assert(new_pt);
+	memset(new_pt, 0, PAGE_SIZE);
+
+	prototype = pte & ~EPT_ADDR_MASK;
+	if (level == 2)
+		prototype &= ~EPT_LARGE_PAGE;
+
+	gpa = pte & EPT_ADDR_MASK;
+	for (i = 0; i < EPT_PGDIR_ENTRIES; i++) {
+		new_pt[i] = prototype | gpa;
+		gpa += 1ul << EPT_LEVEL_SHIFT(level - 1);
+	}
+
+	pte &= ~EPT_LARGE_PAGE;
+	pte &= ~EPT_ADDR_MASK;
+	pte |= virt_to_phys(new_pt);
+
+	*ptep = pte;
+}
+
 /* install_ept_entry : Install a page to a given level in EPT
 		@pml4 : addr of pml4 table
 		@pte_level : level of PTE to set
@@ -244,8 +282,8 @@ void install_ept_entry(unsigned long *pml4,
 			memset(new_pt, 0, PAGE_SIZE);
 			pt[offset] = virt_to_phys(new_pt)
 					| EPT_RA | EPT_WA | EPT_EA;
-		} else
-			pt[offset] &= ~EPT_LARGE_PAGE;
+		} else if (pt[offset] & EPT_LARGE_PAGE)
+			split_large_ept_entry(&pt[offset], level);
 		pt = phys_to_virt(pt[offset] & EPT_ADDR_MASK);
 	}
 	offset = (guest_addr >> EPT_LEVEL_SHIFT(level)) & EPT_PGDIR_MASK;
diff --git a/x86/vmx.h b/x86/vmx.h
index d3a852e..34e9be4 100644
--- a/x86/vmx.h
+++ b/x86/vmx.h
@@ -467,6 +467,7 @@ enum Ctrl1 {
 #define EPT_PAGE_LEVEL		4
 #define EPT_PGDIR_WIDTH		9
 #define EPT_PGDIR_MASK		511
+#define EPT_PGDIR_ENTRIES	(1 << EPT_PGDIR_WIDTH)
 #define EPT_LEVEL_SHIFT(level)	(((level)-1) * EPT_PGDIR_WIDTH + 12)
 #define EPT_ADDR_MASK		GENMASK_ULL(51, 12)
 #define PAGE_MASK		(~(PAGE_SIZE-1))
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* [kvm-unit-tests v3 6/6] x86: vmx: don't explicitly split identity EPT map
  2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
                     ` (4 preceding siblings ...)
  2016-03-02 17:10   ` [kvm-unit-tests v3 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
@ 2016-03-02 17:10   ` Peter Feiner
  5 siblings, 0 replies; 29+ messages in thread
From: Peter Feiner @ 2016-03-02 17:10 UTC (permalink / raw)
  To: kvm, jan.kiszka, drjones, pbonzini; +Cc: pfeiner

Since install_ept_entry automatically splits large EPTEs, it's no
longer necessary to explicitly pave the EPT identity map with 4K
entries mapping [base_addr1, base_addr1 + PAGE_SIZE_2M). I.e.,
install_ept(data_page1, data_page2) does the paving before installing
the 4k data_page1 -> data_page2 mapping.

The other setup_ept_range call was unnecessary.

Note that there was benign bug in the deleted code. The third argument
of setup_ept_range is length not end! Thus the range of GPA mappings
being split was [base_addr1, base_addr1 * 2 + PAGE_SIZE_2M).

Signed-off-by: Peter Feiner <pfeiner@google.com>
---
 x86/vmx_tests.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/x86/vmx_tests.c b/x86/vmx_tests.c
index bb65c06..4ef2247 100644
--- a/x86/vmx_tests.c
+++ b/x86/vmx_tests.c
@@ -954,7 +954,6 @@ static int apic_version;
 
 static int ept_init()
 {
-	unsigned long base_addr1, base_addr2;
 	u32 ctrl_cpu[2];
 
 	if (!(ctrl_cpu_rev[0].clr & CPU_SECONDARY) ||
@@ -979,12 +978,6 @@ static int ept_init()
 	memset(data_page2, 0x0, PAGE_SIZE);
 	*((u32 *)data_page1) = MAGIC_VAL_1;
 	*((u32 *)data_page2) = MAGIC_VAL_2;
-	base_addr1 = (unsigned long)data_page1 & PAGE_MASK_2M;
-	base_addr2 = (unsigned long)data_page2 & PAGE_MASK_2M;
-	setup_ept_range(pml4, base_addr1, base_addr1 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
-	setup_ept_range(pml4, base_addr2, base_addr2 + PAGE_SIZE_2M, 0, 0,
-			EPT_WA | EPT_RA | EPT_EA);
 	install_ept(pml4, (unsigned long)data_page1, (unsigned long)data_page2,
 			EPT_RA | EPT_WA | EPT_EA);
 
-- 
2.7.0.rc3.207.g0ac5344


^ permalink raw reply related	[flat|nested] 29+ messages in thread

* Re: [kvm-unit-tests v3 2/6] lib: generic bitops.h
  2016-03-02 17:10   ` [kvm-unit-tests v3 2/6] lib: generic bitops.h Peter Feiner
@ 2016-03-02 18:13     ` Andrew Jones
  0 siblings, 0 replies; 29+ messages in thread
From: Andrew Jones @ 2016-03-02 18:13 UTC (permalink / raw)
  To: Peter Feiner; +Cc: kvm, jan.kiszka, pbonzini

On Wed, Mar 02, 2016 at 09:10:53AM -0800, Peter Feiner wrote:
> Factored out common bitops stuff, just like Linux's
> include/linux/bitops.h.
> 
> Signed-off-by: Peter Feiner <pfeiner@google.com>

Reviewed-by: Andrew Jones <drjones@redhat.com>

> ---
>  lib/arm/asm/bitops.h   |  8 ++++----
>  lib/arm/asm/cpumask.h  |  2 +-
>  lib/arm/bitops.c       |  2 +-
>  lib/arm64/asm/bitops.h |  8 ++++----
>  lib/bitops.h           | 36 ++++++++++++++++++++++++++++++++++++
>  lib/ppc64/asm/bitops.h | 10 ++++++++++
>  lib/x86/asm/bitops.h   | 14 ++++++++++++++
>  7 files changed, 70 insertions(+), 10 deletions(-)
>  create mode 100644 lib/bitops.h
>  create mode 100644 lib/ppc64/asm/bitops.h
>  create mode 100644 lib/x86/asm/bitops.h
> 
> diff --git a/lib/arm/asm/bitops.h b/lib/arm/asm/bitops.h
> index 8049634..d46cc5d 100644
> --- a/lib/arm/asm/bitops.h
> +++ b/lib/arm/asm/bitops.h
> @@ -2,7 +2,6 @@
>  #define _ASMARM_BITOPS_H_
>  /*
>   * Adapated from
> - *   include/linux/bitops.h
>   *   arch/arm/lib/bitops.h
>   *
>   * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> @@ -10,10 +9,11 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  
> +#ifndef _BITOPS_H_
> +#error only <bitops.h> can be included directly
> +#endif
> +
>  #define BITS_PER_LONG	32
> -#define BIT(nr)		(1UL << (nr))
> -#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
> -#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
>  
>  #define ATOMIC_BITOP(insn, mask, word)				\
>  ({								\
> diff --git a/lib/arm/asm/cpumask.h b/lib/arm/asm/cpumask.h
> index 85b8e4b..6683bb6 100644
> --- a/lib/arm/asm/cpumask.h
> +++ b/lib/arm/asm/cpumask.h
> @@ -8,7 +8,7 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  #include <asm/setup.h>
> -#include <asm/bitops.h>
> +#include <bitops.h>
>  
>  #define CPUMASK_NR_LONGS ((NR_CPUS + BITS_PER_LONG - 1) / BITS_PER_LONG)
>  
> diff --git a/lib/arm/bitops.c b/lib/arm/bitops.c
> index 9ad1121..1f1db93 100644
> --- a/lib/arm/bitops.c
> +++ b/lib/arm/bitops.c
> @@ -6,7 +6,7 @@
>   *
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
> -#include <asm/bitops.h>
> +#include <bitops.h>
>  #include <asm/barrier.h>
>  #include <asm/mmu.h>
>  
> diff --git a/lib/arm64/asm/bitops.h b/lib/arm64/asm/bitops.h
> index 3371c60..618468c 100644
> --- a/lib/arm64/asm/bitops.h
> +++ b/lib/arm64/asm/bitops.h
> @@ -2,7 +2,6 @@
>  #define _ASMARM64_BITOPS_H_
>  /*
>   * Adapated from
> - *   include/linux/bitops.h
>   *   arch/arm64/lib/bitops.S
>   *
>   * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> @@ -10,10 +9,11 @@
>   * This work is licensed under the terms of the GNU LGPL, version 2.
>   */
>  
> +#ifndef _BITOPS_H_
> +#error only <bitops.h> can be included directly
> +#endif
> +
>  #define BITS_PER_LONG	64
> -#define BIT(nr)		(1UL << (nr))
> -#define BIT_MASK(nr)	(1UL << ((nr) % BITS_PER_LONG))
> -#define BIT_WORD(nr)	((nr) / BITS_PER_LONG)
>  
>  #define ATOMIC_BITOP(insn, mask, word)				\
>  ({								\
> diff --git a/lib/bitops.h b/lib/bitops.h
> new file mode 100644
> index 0000000..9aa847e
> --- /dev/null
> +++ b/lib/bitops.h
> @@ -0,0 +1,36 @@
> +#ifndef _BITOPS_H_
> +#define _BITOPS_H_
> +
> +/*
> + * Adapated from
> + *   include/linux/bitops.h
> + *
> + * Copyright (C) 2015, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +
> +#define BITS_PER_LONG_LONG	64
> +#define BIT(nr)			(1UL << (nr))
> +#define BIT_ULL(nr)		(1ULL << (nr))
> +#define BIT_MASK(nr)		(1UL << ((nr) % BITS_PER_LONG))
> +#define BIT_WORD(nr)		((nr) / BITS_PER_LONG)
> +#define BIT_ULL_MASK(nr)	(1ULL << ((nr) % BITS_PER_LONG_LONG))
> +#define BIT_ULL_WORD(nr)	((nr) / BITS_PER_LONG_LONG)
> +#define BITS_PER_BYTE		8
> +#define BITS_TO_LONGS(nr)	DIV_ROUND_UP(nr, BITS_PER_BYTE * sizeof(long))
> +
> +#include <asm/bitops.h>
> +
> +/*
> + * Create a contiguous bitmask starting at bit position @l and ending at
> + * position @h. For example
> + * GENMASK_ULL(39, 21) gives us the 64bit vector 0x000000ffffe00000.
> + */
> +#define GENMASK(h, l) \
> +	(((~0UL) << (l)) & (~0UL >> (BITS_PER_LONG - 1 - (h))))
> +
> +#define GENMASK_ULL(h, l) \
> +	(((~0ULL) << (l)) & (~0ULL >> (BITS_PER_LONG_LONG - 1 - (h))))
> +
> +#endif
> diff --git a/lib/ppc64/asm/bitops.h b/lib/ppc64/asm/bitops.h
> new file mode 100644
> index 0000000..34624b4
> --- /dev/null
> +++ b/lib/ppc64/asm/bitops.h
> @@ -0,0 +1,10 @@
> +#ifndef _ASMPPC64_BITOPS_H_
> +#define _ASMPPC64_BITOPS_H_
> +
> +#ifndef _BITOPS_H_
> +#error only <bitops.h> can be included directly
> +#endif
> +
> +#define BITS_PER_LONG	64
> +
> +#endif
> diff --git a/lib/x86/asm/bitops.h b/lib/x86/asm/bitops.h
> new file mode 100644
> index 0000000..eb4aaa9
> --- /dev/null
> +++ b/lib/x86/asm/bitops.h
> @@ -0,0 +1,14 @@
> +#ifndef _ASMX86_BITOPS_H_
> +#define _ASMX86_BITOPS_H_
> +
> +#ifndef _BITOPS_H_
> +#error only <bitops.h> can be included directly
> +#endif
> +
> +#ifdef __x86_64__
> +#define BITS_PER_LONG	64
> +#else
> +#define BITS_PER_LONG	32
> +#endif
> +
> +#endif
> -- 
> 2.7.0.rc3.207.g0ac5344
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 29+ messages in thread

end of thread, other threads:[~2016-03-02 18:13 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-03-01 19:30 [kvm-unit-tests 0/5] Split large EPT mappings properly Peter Feiner
2016-03-01 19:30 ` [kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes Peter Feiner
2016-03-01 19:30 ` [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
2016-03-01 21:12   ` Paolo Bonzini
2016-03-01 21:16     ` Peter Feiner
2016-03-01 21:27       ` Jan Kiszka
2016-03-01 21:36         ` Paolo Bonzini
2016-03-01 19:30 ` [kvm-unit-tests 3/5] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
2016-03-01 19:30 ` [kvm-unit-tests 4/5] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
2016-03-01 19:30 ` [kvm-unit-tests 5/5] x86: vmx: don't explicitly split identity EPT map Peter Feiner
2016-03-01 21:13 ` [kvm-unit-tests 0/5] Split large EPT mappings properly Paolo Bonzini
2016-03-01 22:34 ` [kvm-unit-tests v2 0/6] " Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 2/6] lib: generic bitops.h Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
2016-03-02  6:24     ` Jan Kiszka
2016-03-02  8:47       ` Paolo Bonzini
2016-03-02 16:50         ` Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
2016-03-01 22:34   ` [kvm-unit-tests v2 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner
2016-03-02 17:10 ` [kvm-unit-tests v3 0/6] Split large EPT mappings properly Peter Feiner
2016-03-02 17:10   ` [kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes Peter Feiner
2016-03-02 17:10   ` [kvm-unit-tests v3 2/6] lib: generic bitops.h Peter Feiner
2016-03-02 18:13     ` Andrew Jones
2016-03-02 17:10   ` [kvm-unit-tests v3 3/6] x86: vmx: Named constant: EPT_ADDR_MASK Peter Feiner
2016-03-02 17:10   ` [kvm-unit-tests v3 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT Peter Feiner
2016-03-02 17:10   ` [kvm-unit-tests v3 5/6] x86: vmx: split large EPTEs in install_ept_entry Peter Feiner
2016-03-02 17:10   ` [kvm-unit-tests v3 6/6] x86: vmx: don't explicitly split identity EPT map Peter Feiner

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox