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

[John Byrne <john.l.byrne@xxxxxx>]

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