From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jan Kiszka Subject: Re: [kvm-unit-tests 2/5] x86: vmx: Named constant: EPT_ADDR_MASK Date: Tue, 1 Mar 2016 22:27:34 +0100 Message-ID: <56D60946.9000504@siemens.com> References: <1456860622-31251-1-git-send-email-pfeiner@google.com> <1456860622-31251-3-git-send-email-pfeiner@google.com> <56D605CB.3050408@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Cc: kvm@vger.kernel.org To: Peter Feiner , Paolo Bonzini Return-path: Received: from thoth.sbs.de ([192.35.17.2]:35613 "EHLO thoth.sbs.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751501AbcCAV1m (ORCPT ); Tue, 1 Mar 2016 16:27:42 -0500 In-Reply-To: Sender: kvm-owner@vger.kernel.org List-ID: On 2016-03-01 22:16, Peter Feiner wrote: > On Tue, Mar 1, 2016 at 1:12 PM, Paolo Bonzini wrote: >> >> >> On 01/03/2016 20:30, Peter Feiner wrote: >>> Signed-off-by: Peter Feiner >>> --- >>> 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