Skip to contentSkip to conversation

predicate NX flag

PATCH In review Nakajima, Jun 3 participants 7 messages Jun 7, 2005 – Jun 9, 2005 2 messages not in the archive

lore
Share & tools
lore
#0 lore raw

BTW, did this solve the driver problem on your machine?

Jun
---
Intel Open Source Technology Center

-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Nakajima, Jun
Sent: Tuesday, June 07, 2005 1:46 PM
To: Scott Parish
Cc: [email protected]
Subject: RE: [Xen-devel] [patch] predicate NX flag

Quoted 3 lines · Scott Parish

the NX flag should only be set when its use is enabled.

sRp

Rather than changing __PAGE_KERNEL, I think we should change set_p?d (? = g, u, m, e) like

#define set_pmd(pmdptr, pmdval) xen_l2_entry_update(pmdptr, (pmdval)& __supported_pte_mask)

Jun
---
Intel Open Source Technology Center

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#1 lore raw

PATCH 1/1 nx bit shouldn't get set when disabled

How does the attached patch look?

sRp

Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c +7 -8
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c
@@ -78,7 +78,7 @@ xen_contig_memory(unsigned long vstart, 
 	pud_t         *pud; 
 	pmd_t         *pmd;
 	pte_t         *pte;
-	unsigned long  pfn, i, flags;
+	unsigned long  mfn, i, flags;
 
 	scrub_pages(vstart, 1 << order);
 
@@ -90,28 +90,27 @@ xen_contig_memory(unsigned long vstart, 
 		pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
 		pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
 		pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
-		pfn = pte->pte >> PAGE_SHIFT;
+		mfn = pte_mfn(*pte);
 		xen_l1_entry_update(pte, 0);
 		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
 			(u32)INVALID_P2M_ENTRY;
 		if (HYPERVISOR_dom_mem_op(MEMOP_decrease_reservation, 
-					  &pfn, 1, 0) != 1) BUG();
+					  &mfn, 1, 0) != 1) BUG();
 	}
 	/* 2. Get a new contiguous memory extent. */
 	if (HYPERVISOR_dom_mem_op(MEMOP_increase_reservation,
-				  &pfn, 1, order) != 1) BUG();
+				  &mfn, 1, order) != 1) BUG();
 	/* 3. Map the new extent in place of old pages. */
 	for (i = 0; i < (1<<order); i++) {
 		pgd = pgd_offset_k(   (vstart + (i*PAGE_SIZE)));
 		pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
 		pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
 		pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
-		xen_l1_entry_update(
-			pte, ((pfn+i)<<PAGE_SHIFT)|__PAGE_KERNEL);
+		set_pte(pte, __pte_ma(((mfn+i)<<PAGE_SHIFT)|__PAGE_KERNEL));
 		xen_machphys_update(
-			pfn+i, (__pa(vstart)>>PAGE_SHIFT)+i);
+			mfn+i, (__pa(vstart)>>PAGE_SHIFT)+i);
 		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
-			pfn+i;
+			mfn+i;
 	}
 	/* Flush updates through and flush the TLB. */
 	xen_tlb_flush();
linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c +1 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c
@@ -256,7 +276,7 @@ unsigned long allocate_empty_lowmem_regi
         pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
         pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
         pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE))); 
-        pfn_array[i] = pte->pte >> PAGE_SHIFT;
+        pfn_array[i] = pte_mfn(*pte);
         xen_l1_entry_update(pte, 0);
         phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
             (u32)INVALID_P2M_ENTRY;
linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c +1 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c
@@ -395,7 +395,7 @@ unsigned long get_machine_pfn(unsigned l
         pmd_t* pmd = pmd_offset(pud, addr);
         pte_t *pte = pte_offset_kernel(pmd, addr);
         
-        return (pte->pte >> PAGE_SHIFT);
+        return pte_mfn(*pte);
 } 
 
 #define ALIGN_TO_4K __attribute__((section(".data.page_aligned")))
linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h +21 -5
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h
@@ -81,7 +81,8 @@ extern inline int pud_present(pud_t pud)
 #define set_pte(pteptr, pteval) xen_l1_entry_update(pteptr, (pteval).pte)
 
 #else
-#define set_pte(pteptr, pteval) xen_l1_entry_update(pteptr, (pteval.pte))
+#define set_pte(pteptr, pteval) xen_l1_entry_update(pteptr, \
+					(pteval).pte & __supported_pte_mask)
 #if 0
 static inline void set_pte(pte_t *dst, pte_t val)
 {
@@ -90,9 +91,23 @@ static inline void set_pte(pte_t *dst, p
 #endif
 #endif
 
-#define set_pmd(pmdptr, pmdval) xen_l2_entry_update(pmdptr, (pmdval))
-#define set_pud(pudptr, pudval) xen_l3_entry_update(pudptr, (pudval))
-#define set_pgd(pgdptr, pgdval) xen_l4_entry_update(pgdptr, (pgdval))
+static inline void set_pmd(pmd_t *ptr, pmd_t val)
+{
+    val.pmd &= __supported_pte_mask;
+    xen_l2_entry_update(ptr, val);
+}
+
+static inline void set_pud(pud_t *ptr, pud_t val)
+{
+    val.pud &= __supported_pte_mask;
+    xen_l3_entry_update(ptr, val);
+}
+
+static inline void set_pgd(pgd_t *ptr, pgd_t val)
+{
+    val.pgd &= __supported_pte_mask;
+    xen_l4_entry_update(ptr, val);
+}
 
 extern inline void pud_clear (pud_t * pud)
 {
@@ -277,9 +292,10 @@ static inline unsigned long pud_bad(pud_
  */
 #define INVALID_P2M_ENTRY (~0UL)
 #define FOREIGN_FRAME(_m) ((_m) | (1UL<<((sizeof(unsigned long)*8)-1)))
+#define pte_mfn(_pte) (((_pte).pte & PTE_MASK) >> PAGE_SHIFT)
 #define pte_pfn(_pte)							\
 ({									\
-	unsigned long mfn = (_pte).pte >> PAGE_SHIFT;                   \
+	unsigned long mfn = pte_mfn(_pte);				\
 	unsigned pfn = mfn_to_pfn(mfn);                                 \
 	if ((pfn >= max_mapnr) || (pfn_to_mfn(pfn) != mfn))		\
 		pfn = max_mapnr; /* special: force !pfn_valid() */	\

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

#2 lore raw

Re: [patch] nx bit shouldn't get set when disabled

Quoted 3 lines · Scott Parish

How does the attached patch look?

sRp

Why does x86_64 get pte_mfn, but not pae i386? I think pci-dma.c should probably be shared between i386 and x86/64. Why the extra mask ops with __supported_pte_mask? Native x86/64 builds obviously don't need them...

-- Keir

#3 lore raw

Re: [patch] nx bit shouldn't get set when disabled

Quoted 2 lines · Keir Fraser

Why does x86_64 get pte_mfn, but not pae i386? I think pci-dma.c should probably be shared between i386 and x86/64.

Last time i checked, the linux side of i386 pae wasn't merged into bk, so i have nothing to test such a patch against. I'll plan on getting a pae setup going again and sending a patch to gerd.

Quoted 2 lines

Why the extra mask ops with __supported_pte_mask? Native x86/64 builds obviously don't need them...

Definitions such as __PAGE_KERNEL set NX, but as Jun pointed out, those should only be set when NX mode is enabled.

sRp

Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
#4 lore raw

Re: [patch] nx bit shouldn't get set when disabled

Quoted 6 lines · Scott Parish
Quoted 2 lines

Why does x86_64 get pte_mfn, but not pae i386? I think pci-dma.c should probably be shared between i386 and x86/64.

Last time i checked, the linux side of i386 pae wasn't merged into bk, so i have nothing to test such a patch against. I'll plan on getting a pae setup going again and sending a patch to gerd.

The functions that are changed aren't pae-specific, and they are already in the xen/i386 tree. They can be patched in anticipation of pae, even though they can only be properly tested non-pae for the time being. I'm not inclined to take patches for xen/x86_64/pci-dma.c anyway: I think we can patch the xen/i386 one and share it with xen/x86_64. Otherwise we're going to get unnecessary divergence between what really ought to be two identical files. (I already did this for arch/xen/i386/kernel/time.c, for example.)

Quoted 5 lines
Quoted 2 lines

Why the extra mask ops with __supported_pte_mask? Native x86/64 builds obviously don't need them...

Definitions such as __PAGE_KERNEL set NX, but as Jun pointed out, those should only be set when NX mode is enabled.

So the extra masking isn't required?

-- Keir

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): [email protected]

#7 lore raw

Re: [patch] nx bit shouldn't get set when disabled

Sorry, i'll be more careful in the future.

sRp

Quoted 24 lines · Keir Fraser

You sent this only to me (not Jun or the list). :-)

Quoted 17 lines · Scott Parish
Quoted 3 lines · Keir Fraser

I'm not inclined to take patches for xen/x86_64/pci-dma.c
anyway: I think we can patch the xen/i386 one and share it with xen/x86_64.

I'll start looking into this.

Quoted 1 line

So the extra masking isn't required?

Not for any reason i know of. Jun?

sRp

Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
#8 lore raw

Re: [patch] nx bit shouldn't get set when disabled

In reply to Keir Fraser
Quoted 5 lines · Keir Fraser

I'm not inclined to take patches for xen/x86_64/pci-dma.c anyway: I think we can patch the xen/i386 one and share it with xen/x86_64. Otherwise we're going to get unnecessary divergence between what really ought to be two identical files. (I already did this for arch/xen/i386/kernel/time.c, for example.)

The attached patch unifies pci-dma.c and adds the pte_mfn() macro.

The one thing that might need an explanation, there's 4 lines of changes (walking the page table) that add parentheses. The x86_64 compiler doesn't seem to like doing unparenthesized math for function arguments.

Boot tested dom0 on x86_64 and x86_32 (non-pae)

sRp

Signature · 2 lines
Scott Parish
Signed-off-by: [email protected]
linux-2.6.11-xen-sparse/arch/xen/i386/kernel/pci-dma.c +14 -12
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/pci-dma.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/pci-dma.c
@@ -36,7 +36,7 @@ xen_contig_memory(unsigned long vstart, 
 	pud_t         *pud; 
 	pmd_t         *pmd;
 	pte_t         *pte;
-	unsigned long  pfn, i, flags;
+	unsigned long  mfn, i, flags;
 
 	scrub_pages(vstart, 1 << order);
 
@@ -45,26 +45,26 @@ xen_contig_memory(unsigned long vstart, 
 	/* 1. Zap current PTEs, giving away the underlying pages. */
 	for (i = 0; i < (1<<order); i++) {
 		pgd = pgd_offset_k(vstart + (i*PAGE_SIZE));
-		pud = pud_offset(pgd, vstart + (i*PAGE_SIZE));
-		pmd = pmd_offset(pud, vstart + (i*PAGE_SIZE));
-		pte = pte_offset_kernel(pmd, vstart + (i*PAGE_SIZE));
-		pfn = pte_val_ma(*pte) >> PAGE_SHIFT;
-		HYPERVISOR_update_va_mapping(vstart + (i*PAGE_SIZE),
+		pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
+		pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
+		pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
+		mfn = pte_mfn(*pte);
+		HYPERVISOR_update_va_mapping((vstart + (i*PAGE_SIZE)),
 					     __pte_ma(0), 0);
 		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
-			INVALID_P2M_ENTRY;
+			(u32)INVALID_P2M_ENTRY;
 		if (HYPERVISOR_dom_mem_op(MEMOP_decrease_reservation, 
-					  &pfn, 1, 0) != 1) BUG();
+					  &mfn, 1, 0) != 1) BUG();
 	}
 	/* 2. Get a new contiguous memory extent. */
 	if (HYPERVISOR_dom_mem_op(MEMOP_increase_reservation,
-				  &pfn, 1, order) != 1) BUG();
+				  &mfn, 1, order) != 1) BUG();
 	/* 3. Map the new extent in place of old pages. */
 	for (i = 0; i < (1<<order); i++) {
 		HYPERVISOR_update_va_mapping(vstart + (i*PAGE_SIZE),
-			__pte_ma(((pfn+i)<<PAGE_SHIFT)|__PAGE_KERNEL), 0);
-		xen_machphys_update(pfn+i, (__pa(vstart)>>PAGE_SHIFT)+i);
-		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] = pfn+i;
+			__pte_ma(((mfn+i)<<PAGE_SHIFT)|__PAGE_KERNEL), 0);
+		xen_machphys_update(mfn+i, (__pa(vstart)>>PAGE_SHIFT)+i);
+		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] = mfn+i;
 	}
 	flush_tlb_all();
 
@@ -123,6 +123,7 @@ void dma_free_coherent(struct device *de
 		free_pages((unsigned long)vaddr, order);
 }
 
+#ifdef ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY
 int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
 				dma_addr_t device_addr, size_t size, int flags)
 {
@@ -199,3 +200,4 @@ void *dma_mark_declared_memory_occupied(
 	return mem->virt_base + (pos << PAGE_SHIFT);
 }
 EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
+#endif /* ARCH_HAS_DMA_DECLARE_COHERENT_MEMORY */
linux-2.6.11-xen-sparse/arch/xen/i386/mm/hypervisor.c +1 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/mm/hypervisor.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/mm/hypervisor.c
@@ -209,7 +209,7 @@ unsigned long allocate_empty_lowmem_regi
         pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
         pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
         pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE))); 
-        pfn_array[i] = pte->pte_low >> PAGE_SHIFT;
+        pfn_array[i] = pte_mfn(*pte);
         HYPERVISOR_update_va_mapping(vstart + (i*PAGE_SIZE), __pte_ma(0), 0);
         phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
             INVALID_P2M_ENTRY;
linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/Makefile +2 -2
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/Makefile new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/Makefile
@@ -14,7 +14,7 @@ obj-y	:= process.o signal.o entry.o trap
 c-obj-y	:= semaphore.o i387.o sys_x86_64.o \
 		ptrace.o quirks.o syscall.o bootflag.o
 
-i386-obj-y			:= time.o
+i386-obj-y			:= time.o pci-dma.o
 obj-y				+= ../../i386/kernel/timers/
 
 s-obj-y	:=
@@ -35,7 +35,7 @@ c-obj-$(CONFIG_X86_IO_APIC)	+= genapic.o
 #obj-$(CONFIG_CPU_FREQ)		+= cpufreq/
 #obj-$(CONFIG_EARLY_PRINTK)	+= early_printk.o
 #obj-$(CONFIG_GART_IOMMU)	+= pci-gart.o aperture.o
-c-obj-$(CONFIG_DUMMY_IOMMU)	+= pci-nommu.o pci-dma.o
+c-obj-$(CONFIG_DUMMY_IOMMU)	+= pci-nommu.o
 #obj-$(CONFIG_SWIOTLB)		+= swiotlb.o
 obj-$(CONFIG_KPROBES)		+= kprobes.o
 
linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c +0 -256

256 lines not on this page · raw message

linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/x8664_ksyms.c +3 -0
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/x8664_ksyms.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/x8664_ksyms.c
@@ -97,6 +97,9 @@ EXPORT_SYMBOL(copy_to_user);
 EXPORT_SYMBOL(copy_in_user);
 EXPORT_SYMBOL(strnlen_user);
 
+EXPORT_SYMBOL(dma_alloc_coherent);
+EXPORT_SYMBOL(dma_free_coherent);
+
 #ifdef CONFIG_PCI
 EXPORT_SYMBOL(pci_alloc_consistent);
 EXPORT_SYMBOL(pci_free_consistent);
linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c +1 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c
@@ -256,7 +276,7 @@ unsigned long allocate_empty_lowmem_regi
         pud = pud_offset(pgd, (vstart + (i*PAGE_SIZE)));
         pmd = pmd_offset(pud, (vstart + (i*PAGE_SIZE)));
         pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE))); 
-        pfn_array[i] = pte->pte >> PAGE_SHIFT;
+        pfn_array[i] = pte_mfn(*pte);
         xen_l1_entry_update(pte, 0);
         phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
             (u32)INVALID_P2M_ENTRY;
linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c +1 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c
@@ -395,7 +395,7 @@ unsigned long get_machine_pfn(unsigned l
         pmd_t* pmd = pmd_offset(pud, addr);
         pte_t *pte = pte_offset_kernel(pmd, addr);
         
-        return (pte->pte >> PAGE_SHIFT);
+        return pte_mfn(*pte);
 } 
 
 #define ALIGN_TO_4K __attribute__((section(".data.page_aligned")))
linux-2.6.11-xen-sparse/include/asm-xen/asm-i386/pgtable-2level.h +2 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-i386/pgtable-2level.h new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-i386/pgtable-2level.h
@@ -46,9 +46,10 @@
  */
 #define INVALID_P2M_ENTRY (~0U)
 #define FOREIGN_FRAME(_m) ((_m) | (1UL<<((sizeof(unsigned long)*8)-1)))
+#define pte_mfn(_pte) ((_pte).pte_low >> PAGE_SHIFT)
 #define pte_pfn(_pte)							\
 ({									\
-	unsigned long mfn = (_pte).pte_low >> PAGE_SHIFT;		\
+	unsigned long mfn = pte_mfn(_pte);				\
 	unsigned long pfn = mfn_to_pfn(mfn);				\
 	if ((pfn >= max_mapnr) || (pfn_to_mfn(pfn) != mfn))		\
 		pfn = max_mapnr; /* special: force !pfn_valid() */	\
linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/dma-mapping.h +50 -1

69 lines not on this page · raw message

linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h +2 -1
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h
@@ -277,9 +292,10 @@ static inline unsigned long pud_bad(pud_
  */
 #define INVALID_P2M_ENTRY (~0UL)
 #define FOREIGN_FRAME(_m) ((_m) | (1UL<<((sizeof(unsigned long)*8)-1)))
+#define pte_mfn(_pte) (((_pte).pte & PTE_MASK) >> PAGE_SHIFT)
 #define pte_pfn(_pte)							\
 ({									\
-	unsigned long mfn = (_pte).pte >> PAGE_SHIFT;                   \
+	unsigned long mfn = pte_mfn(_pte);				\
 	unsigned pfn = mfn_to_pfn(mfn);                                 \
 	if ((pfn >= max_mapnr) || (pfn_to_mfn(pfn) != mfn))		\
 		pfn = max_mapnr; /* special: force !pfn_valid() */	\

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel