Re: x86-64 machine_to_phys vs NX bit

[John Byrne <john.l.byrne@hp.com>]

[-- Attachment #1: Type: text/plain, Size: 1574 bytes --]

John Byrne wrote:
> Keir Fraser wrote:
>> On 13/11/06 8:07 am, "Jan Beulich" <jbeulich@novell.com> 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@hp.com>



[-- Attachment #2: nxmask3.patch --]
[-- Type: text/x-patch, Size: 6170 bytes --]

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 

[-- Attachment #3: Type: text/plain, Size: 138 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel