From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Byrne Subject: Re: x86-64 machine_to_phys vs NX bit Date: Fri, 10 Nov 2006 20:30:15 -0800 Message-ID: <455551D7.7050409@hp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060104030602080803040906" Return-path: 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 List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------060104030602080803040906 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Keir Fraser wrote: > On 24/8/06 8:25 pm, "Rik van Riel" wrote: > >> Say, something like the following? >> >> - paddr_t phys = mfn_to_pfn(machine >> PAGE_SHIFT); >> + paddr_t phys = mfn_to_pfn((machine >> PAGE_SHIFT) & PHYSICAL_MASK); >> >> I'm still thinking I may have missed something in the code >> somewhere, but I've been looking at this for over an hour now >> and can't seem to find it... >> >> Any ideas? > > Your suggested patch looks reasonable but it'd be good to find out why this > hasn't caused us problems. For example, perhaps supported_pte_mask doesn't > include PAGE_NX, so we're never setting the NX bit on 64-bit PTEs? That must > be worth checking out, possibly also tracing machine_to_phys to find out > where that bit 63 goes -- I agree that it looks like mfn_to_pfn() shouldn#t > work if bit63 is set in the 'maddr' argument. > > -- Keir > > While trying to debug a migration problem in Xen 3.0.3 I have noticed this issue. I don't see a fix in xen-unstable. Has this gotten dropped on the floor? The suggested patch above is not quite correct or complete. My proposed patch aqainst xen-unstable changeset 12364:d19deb173503 is attached. Note that there is also an issue in x86 PAE: machine_to_phys() currently will strip the NX bit. Signed-off-by: John Byrne --------------060104030602080803040906 Content-Type: text/x-patch; name="nxmask.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="nxmask.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 Fri Nov 10 21:37:45 2006 -0600 @@ -127,10 +127,17 @@ 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) { + /* + * 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 & ~PAGE_MASK); + 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 Fri Nov 10 21:37:45 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) +#else +#define __PHYSICAL_MASK_SHIFT 32 +#define __PHYSICAL_MASK (~0UL) +#endif + +#define PHYSICAL_PAGE_MASK (PAGE_MASK & __PHYSICAL_MASK) #define LARGE_PAGE_MASK (~(LARGE_PAGE_SIZE-1)) #define LARGE_PAGE_SIZE (1UL << PMD_SHIFT) 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 Fri Nov 10 21:36:35 2006 -0600 @@ -122,8 +122,8 @@ static inline maddr_t phys_to_machine(pa 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); + paddr_t phys = mfn_to_pfn((machine & PHYSICAL_PAGE_MASK) >> PAGE_SHIFT); + phys = (phys << PAGE_SHIFT) | (machine & ~PHYSICAL_PAGE_MASK); return phys; } 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 Fri Nov 10 21:36:35 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 @@ -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 --------------060104030602080803040906 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 --------------060104030602080803040906--