From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Byrne Subject: Re: x86-64 machine_to_phys vs NX bit Date: Mon, 13 Nov 2006 17:15:16 -0800 Message-ID: <455918A4.9070409@hp.com> References: <4558D979.9000902@hp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------020201030808030602060109" Return-path: In-Reply-To: <4558D979.9000902@hp.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xensource.com Errors-To: xen-devel-bounces@lists.xensource.com To: Keir Fraser Cc: xen-devel , Jan Beulich List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------020201030808030602060109 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit John Byrne wrote: > Keir Fraser wrote: >> On 13/11/06 8:07 am, "Jan Beulich" wrote: >> >>>> There was a bug in my previous patch. (There's nothing like trying to >>>> get to sleep and realizing you've screwed up.) The x86 pae >>>> PHYSICAL_PAGE_MASK I defined was incorrect because PAGE_MASK was only a >>>> long. I hope I haven't done anything else wrong. >>> I don't think this is correct - machine_to_phys() translates a >>> machine address >>> to a physical one, and in that translation the upper bits matter only >>> as much >>> as mfn_to_pfn() should return an invalid indicator if any of them is >>> set. In >>> turn, >>> it should be the caller's responsibility to make sure the NX bit (and >>> any >>> potential >>> other one being set beyond bit 52) gets masked off *before* calling this >>> function. (Specifically, the preserving of the lower bits is to properly >>> translate >>> a non-page aligned address, not to preserve attribute bits read from >>> a page >>> table entry). >> >> Yes, we should keep the old machine_to_phys() definition and rename >> John's >> new version as pte_machine_to_phys(). The latter should be used in all >> contexts where machine_to_phys() currently operates on a pte (that's >> most of >> its uses, actually). This is a worthwhile cleanup and clarification. >> Could >> you respin the patch, John? >> >> Thanks, >> Keir >> >> >> > > I've made the change. I'll send it out after I've built and tested it. > > John > > Here's the new patch. Signed-off-by: John Byrne --------------020201030808030602060109 Content-Type: text/x-patch; name="nxmask3.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="nxmask3.patch" diff -r d19deb173503 linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/maddr.h --- a/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/maddr.h Fri Nov 10 15:27:22 2006 +0000 +++ b/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/maddr.h Mon Nov 13 14:32:15 2006 -0600 @@ -127,10 +127,24 @@ static inline maddr_t phys_to_machine(pa machine = (machine << PAGE_SHIFT) | (phys & ~PAGE_MASK); return machine; } + static inline paddr_t machine_to_phys(maddr_t machine) { paddr_t phys = mfn_to_pfn(machine >> PAGE_SHIFT); phys = (phys << PAGE_SHIFT) | (machine & ~PAGE_MASK); + return phys; +} + +static inline paddr_t pte_machine_to_phys(maddr_t machine) +{ + /* + * In PAE mode, the NX bit needs to be dealt with in the value + * passed to mfn_to_pfn(). On x86_64, we need to mask it off, + * but for i386 the conversion to ulong for the argument will + * clip it off. + */ + paddr_t phys = mfn_to_pfn(machine >> PAGE_SHIFT); + phys = (phys << PAGE_SHIFT) | (machine & ~PHYSICAL_PAGE_MASK); return phys; } diff -r d19deb173503 linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/page.h --- a/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/page.h Fri Nov 10 15:27:22 2006 +0000 +++ b/linux-2.6-xen-sparse/include/asm-i386/mach-xen/asm/page.h Mon Nov 13 14:31:16 2006 -0600 @@ -5,6 +5,16 @@ #define PAGE_SHIFT 12 #define PAGE_SIZE (1UL << PAGE_SHIFT) #define PAGE_MASK (~(PAGE_SIZE-1)) + +#ifdef CONFIG_X86_PAE +#define __PHYSICAL_MASK_SHIFT 36 +#define __PHYSICAL_MASK ((1ULL << __PHYSICAL_MASK_SHIFT) - 1) +#define PHYSICAL_PAGE_MASK (~((1ULL << PAGE_SHIFT) - 1) & __PHYSICAL_MASK) +#else +#define __PHYSICAL_MASK_SHIFT 32 +#define __PHYSICAL_MASK (~0UL) +#define PHYSICAL_PAGE_MASK (PAGE_MASK & __PHYSICAL_MASK) +#endif #define LARGE_PAGE_MASK (~(LARGE_PAGE_SIZE-1)) #define LARGE_PAGE_SIZE (1UL << PMD_SHIFT) @@ -85,7 +95,7 @@ static inline unsigned long long pte_val if (x.pte_low) { ret = x.pte_low | (unsigned long long)x.pte_high << 32; - ret = machine_to_phys(ret) | 1; + ret = pte_machine_to_phys(ret) | 1; } else { ret = 0; } @@ -94,13 +104,13 @@ static inline unsigned long long pmd_val static inline unsigned long long pmd_val(pmd_t x) { unsigned long long ret = x.pmd; - if (ret) ret = machine_to_phys(ret) | 1; + if (ret) ret = pte_machine_to_phys(ret) | 1; return ret; } static inline unsigned long long pgd_val(pgd_t x) { unsigned long long ret = x.pgd; - if (ret) ret = machine_to_phys(ret) | 1; + if (ret) ret = pte_machine_to_phys(ret) | 1; return ret; } static inline unsigned long long pte_val_ma(pte_t x) @@ -115,7 +125,8 @@ typedef struct { unsigned long pgprot; } #define pgprot_val(x) ((x).pgprot) #include #define boot_pte_t pte_t /* or would you rather have a typedef */ -#define pte_val(x) (((x).pte_low & 1) ? machine_to_phys((x).pte_low) : \ +#define pte_val(x) (((x).pte_low & 1) ? \ + pte_machine_to_phys((x).pte_low) : \ (x).pte_low) #define pte_val_ma(x) ((x).pte_low) #define __pte(x) ({ unsigned long _x = (x); \ @@ -125,7 +136,7 @@ static inline unsigned long pgd_val(pgd_ static inline unsigned long pgd_val(pgd_t x) { unsigned long ret = x.pgd; - if (ret) ret = machine_to_phys(ret) | 1; + if (ret) ret = pte_machine_to_phys(ret) | 1; return ret; } #define HPAGE_SHIFT 22 diff -r d19deb173503 linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/maddr.h --- a/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/maddr.h Fri Nov 10 15:27:22 2006 +0000 +++ b/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/maddr.h Mon Nov 13 14:35:30 2006 -0600 @@ -127,6 +127,14 @@ static inline paddr_t machine_to_phys(ma return phys; } +static inline paddr_t pte_machine_to_phys(maddr_t machine) +{ + paddr_t phys; + phys = mfn_to_pfn((machine & PHYSICAL_PAGE_MASK) >> PAGE_SHIFT); + phys = (phys << PAGE_SHIFT) | (machine & ~PHYSICAL_PAGE_MASK); + return phys; +} + /* VIRT <-> MACHINE conversion */ #define virt_to_machine(v) (phys_to_machine(__pa(v))) #define virt_to_mfn(v) (pfn_to_mfn(__pa(v) >> PAGE_SHIFT)) diff -r d19deb173503 linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/page.h --- a/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/page.h Fri Nov 10 15:27:22 2006 +0000 +++ b/linux-2.6-xen-sparse/include/asm-x86_64/mach-xen/asm/page.h Mon Nov 13 14:33:02 2006 -0600 @@ -33,6 +33,13 @@ #define PAGE_SIZE (1UL << PAGE_SHIFT) #endif #define PAGE_MASK (~(PAGE_SIZE-1)) + +/* See Documentation/x86_64/mm.txt for a description of the memory map. */ +#define __PHYSICAL_MASK_SHIFT 46 +#define __PHYSICAL_MASK ((1UL << __PHYSICAL_MASK_SHIFT) - 1) +#define __VIRTUAL_MASK_SHIFT 48 +#define __VIRTUAL_MASK ((1UL << __VIRTUAL_MASK_SHIFT) - 1) + #define PHYSICAL_PAGE_MASK (~(PAGE_SIZE-1) & __PHYSICAL_MASK) #define THREAD_ORDER 1 @@ -90,28 +97,28 @@ typedef struct { unsigned long pgd; } pg typedef struct { unsigned long pgprot; } pgprot_t; -#define pte_val(x) (((x).pte & 1) ? machine_to_phys((x).pte) : \ +#define pte_val(x) (((x).pte & 1) ? pte_machine_to_phys((x).pte) : \ (x).pte) #define pte_val_ma(x) ((x).pte) static inline unsigned long pmd_val(pmd_t x) { unsigned long ret = x.pmd; - if (ret) ret = machine_to_phys(ret); + if (ret) ret = pte_machine_to_phys(ret); return ret; } static inline unsigned long pud_val(pud_t x) { unsigned long ret = x.pud; - if (ret) ret = machine_to_phys(ret); + if (ret) ret = pte_machine_to_phys(ret); return ret; } static inline unsigned long pgd_val(pgd_t x) { unsigned long ret = x.pgd; - if (ret) ret = machine_to_phys(ret); + if (ret) ret = pte_machine_to_phys(ret); return ret; } @@ -162,12 +169,6 @@ static inline pgd_t __pgd(unsigned long /* to align the pointer to the (next) page boundary */ #define PAGE_ALIGN(addr) (((addr)+PAGE_SIZE-1)&PAGE_MASK) - -/* See Documentation/x86_64/mm.txt for a description of the memory map. */ -#define __PHYSICAL_MASK_SHIFT 46 -#define __PHYSICAL_MASK ((1UL << __PHYSICAL_MASK_SHIFT) - 1) -#define __VIRTUAL_MASK_SHIFT 48 -#define __VIRTUAL_MASK ((1UL << __VIRTUAL_MASK_SHIFT) - 1) #define KERNEL_TEXT_SIZE (40UL*1024*1024) #define KERNEL_TEXT_START 0xffffffff80000000UL --------------020201030808030602060109 Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel --------------020201030808030602060109--