[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



John Byrne wrote:
Keir Fraser wrote:
On 13/11/06 8:07 am, "Jan Beulich" <jbeulich@xxxxxxxxxx> 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 <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        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 <asm/maddr.h>
 #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 
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.