diff mbox

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

Message ID 1456871694-23042-4-git-send-email-pfeiner@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

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

Comments

Jan Kiszka March 2, 2016, 6:24 a.m. UTC | #1 
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
Paolo Bonzini March 2, 2016, 8:47 a.m. UTC | #2 
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
--
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
Peter Feiner March 2, 2016, 4:50 p.m. UTC | #3 
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.
--
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
diff mbox

Patch

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