WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

Re: [Xen-devel] x86-64 machine_to_phys vs NX bit

To: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
Subject: Re: [Xen-devel] x86-64 machine_to_phys vs NX bit
From: John Byrne <john.l.byrne@xxxxxx>
Date: Fri, 10 Nov 2006 20:30:15 -0800
Cc: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Fri, 10 Nov 2006 20:30:49 -0800
Envelope-to: www-data@xxxxxxxxxxxxxxxxxx
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/cgi-bin/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Thunderbird 1.5.0.8 (X11/20061025)
Keir Fraser wrote:
On 24/8/06 8:25 pm, "Rik van Riel" <riel@xxxxxxxxxx> 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 <john.l.byrne@xxxxxx>







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 
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel