From mboxrd@z Thu Jan 1 00:00:00 1970 From: John Byrne Subject: Re: x86-64 machine_to_phys vs NX bit Date: Sat, 11 Nov 2006 01:56:33 -0800 Message-ID: <45559E51.3010909@hp.com> References: <455551D7.7050409@hp.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------060706060801050408040304" Return-path: In-Reply-To: <455551D7.7050409@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 List-Id: xen-devel@lists.xenproject.org This is a multi-part message in MIME format. --------------060706060801050408040304 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit John Byrne wrote: > 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 > <...snipped...> 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. John Byrne Signed-off-by: John Byrne --------------060706060801050408040304 Content-Type: text/x-patch; name="nxmask2.patch" Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="nxmask2.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 Sat Nov 11 03:14:00 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) 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 --------------060706060801050408040304 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 --------------060706060801050408040304--