[kvm-unit-tests 0/5] Split large EPT mappings properly

[kvm-unit-tests 0/5] Split large EPT mappings properly

[Peter Feiner]

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

[kvm-unit-tests 1/5] x86: vmx.h: trivial whitespace fixes

[Peter Feiner]

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

[kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK

[Peter Feiner]

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

Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK

[Paolo Bonzini]

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))
>  
> 

Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK

[Peter Feiner]

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.

Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK

[Jan Kiszka]

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

Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK

[Paolo Bonzini]

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).

[kvm-unit-tests 3/5] x86: vmx: Named constant: EPT_LEVEL_SHIFT

[Peter Feiner]

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

[kvm-unit-tests 4/5] x86: vmx: split large EPTEs in install_ept_entry

[Peter Feiner]

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

[kvm-unit-tests 5/5] x86: vmx: don't explicitly split identity EPT map

[Peter Feiner]

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

Re: [kvm-unit-tests 0/5] Split large EPT mappings properly

[Paolo Bonzini]

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

[kvm-unit-tests v2 0/6] Split large EPT mappings properly

[Peter Feiner]

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

[kvm-unit-tests v2 1/6] x86: vmx.h: trivial whitespace fixes

[Peter Feiner]

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

[kvm-unit-tests v2 2/6] lib: generic bitops.h

[Peter Feiner]

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

[kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK

[Peter Feiner]

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

Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK

[Jan Kiszka]

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

Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK

[Paolo Bonzini]

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

Re: [kvm-unit-tests v2 3/6] x86: vmx: Named constant: EPT_ADDR_MASK

[Peter Feiner]

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.

[kvm-unit-tests v2 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT

[Peter Feiner]

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

[kvm-unit-tests v2 5/6] x86: vmx: split large EPTEs in install_ept_entry

[Peter Feiner]

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

[kvm-unit-tests v2 6/6] x86: vmx: don't explicitly split identity EPT map

[Peter Feiner]

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

[kvm-unit-tests v3 0/6] Split large EPT mappings properly

[Peter Feiner]

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

[kvm-unit-tests v3 1/6] x86: vmx.h: trivial whitespace fixes

[Peter Feiner]

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

[kvm-unit-tests v3 2/6] lib: generic bitops.h

[Peter Feiner]

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

Re: [kvm-unit-tests v3 2/6] lib: generic bitops.h

[Andrew Jones]

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

[kvm-unit-tests v3 3/6] x86: vmx: Named constant: EPT_ADDR_MASK

[Peter Feiner]

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

[kvm-unit-tests v3 4/6] x86: vmx: Named constant: EPT_LEVEL_SHIFT

[Peter Feiner]

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

[kvm-unit-tests v3 5/6] x86: vmx: split large EPTEs in install_ept_entry

[Peter Feiner]

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

[kvm-unit-tests v3 6/6] x86: vmx: don't explicitly split identity EPT map

[Peter Feiner]

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