diff mbox

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

Message ID 1456860622-31251-3-git-send-email-pfeiner@google.com (mailing list archive)
State New, archived
Headers show

Commit Message

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

Comments

Paolo Bonzini March 1, 2016, 9:12 p.m. UTC | #1 
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))
>  
> 
--
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 1, 2016, 9:16 p.m. UTC | #2 
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.
--
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
Jan Kiszka March 1, 2016, 9:27 p.m. UTC | #3 
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
Paolo Bonzini March 1, 2016, 9:36 p.m. UTC | #4 
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).
--
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 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))