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

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

[Scott Parish]

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

How does the attached patch look?

sRp

-- 
Scott Parish
Signed-off-by: srparish@us.ibm.com

[-- Attachment #2: nx.diff --]
[-- Type: text/plain, Size: 5597 bytes --]

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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c	2005-05-24 17:34:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c	2005-06-08 03:17:21.000000000 +0000
@@ -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();
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c	2005-06-06 16:39:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c	2005-06-07 20:35:16.000000000 +0000
@@ -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;
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c	2005-06-02 23:09:31.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c	2005-06-07 15:39:50.000000000 +0000
@@ -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")))
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h	2005-05-28 09:20:36.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h	2005-06-08 02:48:53.000000000 +0000
@@ -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() */	\

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

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

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

[Scott Parish]

On Wed, Jun 08, 2005 at 08:56:32PM +0100, Keir Fraser wrote:

> 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.

> 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

-- 
Scott Parish
Signed-off-by: srparish@us.ibm.com

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

[Keir Fraser]

On 8 Jun 2005, at 19:06, Scott Parish wrote:

> 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

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

[Keir Fraser]

On 8 Jun 2005, at 20:47, Scott Parish wrote:

>> 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.)

>> 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

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

[Keir Fraser]

On 8 Jun 2005, at 22:30, Ian Pratt wrote:

>>> 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?
>
> I suspect normal hardware is prepared to put up with the bit being set
> even if its not supported...

I doubt that, actually. But the kernel ought to set NX in __PAGE_KERNEL 
only when the hardware supports it (__PAGE_KERNEL is a variable these 
days, not a macro).

  -- Keir

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

[Ian Pratt]

 
> >> 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?

I suspect normal hardware is prepared to put up with the bit being set
even if its not supported...

Ian

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

[Nakajima, Jun]

Keir Fraser wrote:
> On 8 Jun 2005, at 20:47, Scott Parish wrote:
> 
>>> 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.)
> 
>>> 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
Hold on.

Scott, in your patch:

-		pfn = pte->pte >> PAGE_SHIFT;
+		mfn = pte_mfn(*pte);

+#define pte_mfn(_pte) (((_pte).pte & PTE_MASK) >> PAGE_SHIFT)

These should not be necessary if pte is created correctly (w/ or w/o NX
bit depending on __supported_pte_mask) in the first place, as Keir
pointed out. That's what I meant by "We should fix the creator of the
pte (by __supported_pte_mask), not the consumer of it." And you always
cut off the NX bit in your patch.

So remove the changes to pci-dma.c and init.c. If that does not work,
move check_efer to right before pda_init(0) in x86_64_start_kernel() in
head64.c.

Jun
---
Intel Open Source Technology Center

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

[Keir Fraser]

On 8 Jun 2005, at 22:56, Nakajima, Jun wrote:

> These should not be necessary if pte is created correctly (w/ or w/o NX
> bit depending on __supported_pte_mask) in the first place, as Keir
> pointed out. That's what I meant by "We should fix the creator of the
> pte (by __supported_pte_mask), not the consumer of it." And you always
> cut off the NX bit in your patch.
>
> So remove the changes to pci-dma.c and init.c. If that does not work,
> move check_efer to right before pda_init(0) in x86_64_start_kernel() in
> head64.c.

I think the changes in pci-dma.c *are* required: that code currently 
just does pte->pte >> PAGE_SHIFT, which certainly isn't right. That's 
code that doesn't exist in native Linux so it probably does need fixing 
up for PAE/NX.

But 'fixes' to simple native functions like set_pud, set_pmd, etc. 
ought not to be necessary. We shouldn't have to fix ubiquitous 
functions like that to support nx bit on xenlinux. If we do, it's a 
sign that something is very wrong! :-)

  -- Keir

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

[Scott Parish]

On Wed, Jun 08, 2005 at 03:47:01PM -0700, Nakajima, Jun wrote:

> Agree on that part. Should read like:
> 
>   pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
> + pte->pte &= __supported_pte_mask;
>   pfn = pte->pte >> PAGE_SHIFT;

I dissent. Same reason as yesterday: the above might work right now, but
as soon as we enable NX the NX bit will end up in the pfn/mfn.

sRp

-- 
Scott Parish
Signed-off-by: srparish@us.ibm.com

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

[Scott Parish]

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

sRp

On Wed, Jun 08, 2005 at 10:31:36PM +0100, Keir Fraser wrote:

> 
> You sent this only to me (not Jun or the list). :-)
> 
> On 8 Jun 2005, at 21:47, Scott Parish wrote:
> 
> >On Wed, Jun 08, 2005 at 10:23:47PM +0100, Keir Fraser wrote:
> >
> >>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.
> >
> >>So the extra masking isn't required?
> >
> >Not for any reason i know of. Jun?
> >
> >sRp
> >
> >-- 
> >Scott Parish
> >Signed-off-by: srparish@us.ibm.com
> 
> 

-- 
Scott Parish
Signed-off-by: srparish@us.ibm.com

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

[Nakajima, Jun]

Keir Fraser wrote:
> On 8 Jun 2005, at 22:56, Nakajima, Jun wrote:
> 
>> These should not be necessary if pte is created correctly (w/ or w/o
>> NX bit depending on __supported_pte_mask) in the first place, as Keir
>> pointed out. That's what I meant by "We should fix the creator of the
>> pte (by __supported_pte_mask), not the consumer of it." And you
>> always cut off the NX bit in your patch.
>> 
>> So remove the changes to pci-dma.c and init.c. If that does not work,
>> move check_efer to right before pda_init(0) in x86_64_start_kernel()
>> in head64.c.
> 
> I think the changes in pci-dma.c *are* required: that code currently
> just does pte->pte >> PAGE_SHIFT, which certainly isn't right. That's
> code that doesn't exist in native Linux so it probably does need
> fixing up for PAE/NX.
Agree on that part. Should read like:

  pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
+ pte->pte &= __supported_pte_mask;
  pfn = pte->pte >> PAGE_SHIFT;

> 
> But 'fixes' to simple native functions like set_pud, set_pmd, etc.
> ought not to be necessary. We shouldn't have to fix ubiquitous
> functions like that to support nx bit on xenlinux. If we do, it's a
> sign that something is very wrong! :-)
> 
>   -- Keir
I found a bug in phys_pud_init(). I'll send a patch. 

Jun
---
Intel Open Source Technology Center

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

[Nakajima, Jun]

Scott Parish wrote:
> On Wed, Jun 08, 2005 at 03:47:01PM -0700, Nakajima, Jun wrote:
> 
>> Agree on that part. Should read like:
>> 
>>   pte = pte_offset_kernel(pmd, (vstart + (i*PAGE_SIZE)));
>> + pte->pte &= __supported_pte_mask;
>>   pfn = pte->pte >> PAGE_SHIFT;
> 
> I dissent. Same reason as yesterday: the above might work right now,
> but as soon as we enable NX the NX bit will end up in the pfn/mfn.
> 
> sRp

Your are correct. We always need to cut off the NX bit because that's
what HYPERVISOR_dom_mem_op expects. 

Jun
---
Intel Open Source Technology Center

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

[Scott Parish]

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

On Wed, Jun 08, 2005 at 10:23:47PM +0100, Keir Fraser wrote:

> 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

-- 
Scott Parish
Signed-off-by: srparish@us.ibm.com

[-- Attachment #2: unify-pci-dma.c.diff --]
[-- Type: text/plain, Size: 18892 bytes --]

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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/pci-dma.c	2005-05-24 17:34:53.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/kernel/pci-dma.c	2005-06-09 11:52:51.000000000 +0000
@@ -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 */
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/mm/hypervisor.c	2005-06-06 16:39:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/i386/mm/hypervisor.c	2005-06-09 12:38:43.000000000 +0000
@@ -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;
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/Makefile	2005-06-03 01:44:40.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/Makefile	2005-06-09 11:44:20.000000000 +0000
@@ -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
 
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c	2005-05-24 17:34:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/pci-dma.c	1970-01-01 00:00:00.000000000 +0000
@@ -1,256 +0,0 @@
-/*
- * Dynamic DMA mapping support.
- */
-
-#include <linux/types.h>
-#include <linux/mm.h>
-#include <linux/string.h>
-#include <linux/pci.h>
-#include <linux/module.h>
-#include <asm/io.h>
-#include <asm-xen/balloon.h>
-
-/* Map a set of buffers described by scatterlist in streaming
- * mode for DMA.  This is the scatter-gather version of the
- * above pci_map_single interface.  Here the scatter gather list
- * elements are each tagged with the appropriate dma address
- * and length.  They are obtained via sg_dma_{address,length}(SG).
- *
- * NOTE: An implementation may be able to use a smaller number of
- *       DMA address/length pairs than there are SG table elements.
- *       (for example via virtual mapping capabilities)
- *       The routine returns the number of addr/length pairs actually
- *       used, at most nents.
- *
- * Device ownership issues as mentioned above for pci_map_single are
- * the same here.
- */
-int dma_map_sg(struct device *hwdev, struct scatterlist *sg,
-	       int nents, int direction)
-{
-	int i;
-
-	BUG_ON(direction == DMA_NONE);
- 	for (i = 0; i < nents; i++ ) {
-		struct scatterlist *s = &sg[i];
-		BUG_ON(!s->page); 
-		s->dma_address = virt_to_bus(page_address(s->page) +s->offset);
-		s->dma_length = s->length;
-	}
-	return nents;
-}
-
-EXPORT_SYMBOL(dma_map_sg);
-
-/* Unmap a set of streaming mode DMA translations.
- * Again, cpu read rules concerning calls here are the same as for
- * pci_unmap_single() above.
- */
-void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
-		  int nents, int dir)
-{
-	int i;
-	for (i = 0; i < nents; i++) { 
-		struct scatterlist *s = &sg[i];
-		BUG_ON(s->page == NULL); 
-		BUG_ON(s->dma_address == 0); 
-		dma_unmap_single(dev, s->dma_address, s->dma_length, dir);
-	} 
-}
-
-struct dma_coherent_mem {
-	void		*virt_base;
-	u32		device_base;
-	int		size;
-	int		flags;
-	unsigned long	*bitmap;
-};
-
-static void
-xen_contig_memory(unsigned long vstart, unsigned int order)
-{
-	/*
-	 * Ensure multi-page extents are contiguous in machine memory.
-	 * This code could be cleaned up some, and the number of
-	 * hypercalls reduced.
-	 */
-	pgd_t         *pgd; 
-	pud_t         *pud; 
-	pmd_t         *pmd;
-	pte_t         *pte;
-	unsigned long  pfn, i, flags;
-
-	scrub_pages(vstart, 1 << order);
-
-        balloon_lock(flags);
-
-	/* 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->pte >> PAGE_SHIFT;
-		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();
-	}
-	/* 2. Get a new contiguous memory extent. */
-	if (HYPERVISOR_dom_mem_op(MEMOP_increase_reservation,
-				  &pfn, 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);
-		xen_machphys_update(
-			pfn+i, (__pa(vstart)>>PAGE_SHIFT)+i);
-		phys_to_machine_mapping[(__pa(vstart)>>PAGE_SHIFT)+i] =
-			pfn+i;
-	}
-	/* Flush updates through and flush the TLB. */
-	xen_tlb_flush();
-
-        balloon_unlock(flags);
-}
-
-void *dma_alloc_coherent(struct device *dev, size_t size,
-			   dma_addr_t *dma_handle, unsigned gfp)
-{
-	void *ret;
-	unsigned int order = get_order(size);
-	unsigned long vstart;
-
-	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
-
-	/* ignore region specifiers */
-	gfp &= ~(__GFP_DMA | __GFP_HIGHMEM);
-
-	if (mem) {
-		int page = bitmap_find_free_region(mem->bitmap, mem->size,
-						     order);
-		if (page >= 0) {
-			*dma_handle = mem->device_base + (page << PAGE_SHIFT);
-			ret = mem->virt_base + (page << PAGE_SHIFT);
-			memset(ret, 0, size);
-			return ret;
-		}
-		if (mem->flags & DMA_MEMORY_EXCLUSIVE)
-			return NULL;
-	}
-
-	if (dev == NULL || (dev->coherent_dma_mask < 0xffffffff))
-		gfp |= GFP_DMA;
-
-	vstart = __get_free_pages(gfp, order);
-	ret = (void *)vstart;
-	if (ret == NULL)
-		return ret;
-
-	xen_contig_memory(vstart, order);
-
-	memset(ret, 0, size);
-	*dma_handle = virt_to_bus(ret);
-
-	return ret;
-}
-EXPORT_SYMBOL(dma_alloc_coherent);
-
-void dma_free_coherent(struct device *dev, size_t size,
-			 void *vaddr, dma_addr_t dma_handle)
-{
-	struct dma_coherent_mem *mem = dev ? dev->dma_mem : NULL;
-	int order = get_order(size);
-	
-	if (mem && vaddr >= mem->virt_base && vaddr < (mem->virt_base + (mem->size << PAGE_SHIFT))) {
-		int page = (vaddr - mem->virt_base) >> PAGE_SHIFT;
-
-		bitmap_release_region(mem->bitmap, page, order);
-	} else
-		free_pages((unsigned long)vaddr, order);
-}
-EXPORT_SYMBOL(dma_free_coherent);
-
-#if 0
-int dma_declare_coherent_memory(struct device *dev, dma_addr_t bus_addr,
-				dma_addr_t device_addr, size_t size, int flags)
-{
-	void __iomem *mem_base;
-	int pages = size >> PAGE_SHIFT;
-	int bitmap_size = (pages + 31)/32;
-
-	if ((flags & (DMA_MEMORY_MAP | DMA_MEMORY_IO)) == 0)
-		goto out;
-	if (!size)
-		goto out;
-	if (dev->dma_mem)
-		goto out;
-
-	/* FIXME: this routine just ignores DMA_MEMORY_INCLUDES_CHILDREN */
-
-	mem_base = ioremap(bus_addr, size);
-	if (!mem_base)
-		goto out;
-
-	dev->dma_mem = kmalloc(sizeof(struct dma_coherent_mem), GFP_KERNEL);
-	if (!dev->dma_mem)
-		goto out;
-	memset(dev->dma_mem, 0, sizeof(struct dma_coherent_mem));
-	dev->dma_mem->bitmap = kmalloc(bitmap_size, GFP_KERNEL);
-	if (!dev->dma_mem->bitmap)
-		goto free1_out;
-	memset(dev->dma_mem->bitmap, 0, bitmap_size);
-
-	dev->dma_mem->virt_base = mem_base;
-	dev->dma_mem->device_base = device_addr;
-	dev->dma_mem->size = pages;
-	dev->dma_mem->flags = flags;
-
-	if (flags & DMA_MEMORY_MAP)
-		return DMA_MEMORY_MAP;
-
-	return DMA_MEMORY_IO;
-
- free1_out:
-	kfree(dev->dma_mem->bitmap);
- out:
-	return 0;
-}
-EXPORT_SYMBOL(dma_declare_coherent_memory);
-
-void dma_release_declared_memory(struct device *dev)
-{
-	struct dma_coherent_mem *mem = dev->dma_mem;
-	
-	if(!mem)
-		return;
-	dev->dma_mem = NULL;
-	iounmap(mem->virt_base);
-	kfree(mem->bitmap);
-	kfree(mem);
-}
-EXPORT_SYMBOL(dma_release_declared_memory);
-
-void *dma_mark_declared_memory_occupied(struct device *dev,
-					dma_addr_t device_addr, size_t size)
-{
-	struct dma_coherent_mem *mem = dev->dma_mem;
-	int pages = (size + (device_addr & ~PAGE_MASK) + PAGE_SIZE - 1) >> PAGE_SHIFT;
-	int pos, err;
-
-	if (!mem)
-		return ERR_PTR(-EINVAL);
-
-	pos = (device_addr - mem->device_base) >> PAGE_SHIFT;
-	err = bitmap_allocate_region(mem->bitmap, pos, get_order(pages));
-	if (err != 0)
-		return ERR_PTR(err);
-	return mem->virt_base + (pos << PAGE_SHIFT);
-}
-EXPORT_SYMBOL(dma_mark_declared_memory_occupied);
-#endif
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/x8664_ksyms.c	2005-05-24 17:34:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/kernel/x8664_ksyms.c	2005-06-09 11:24:34.000000000 +0000
@@ -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);
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c	2005-06-06 16:39:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/hypervisor.c	2005-06-07 20:35:16.000000000 +0000
@@ -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;
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c	2005-06-02 23:09:31.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/arch/xen/x86_64/mm/init.c	2005-06-07 15:39:50.000000000 +0000
@@ -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")))
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-i386/pgtable-2level.h	2005-05-24 17:34:54.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-i386/pgtable-2level.h	2005-06-09 12:11:58.000000000 +0000
@@ -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() */	\
diff -rN -u -p old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/dma-mapping.h new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/dma-mapping.h
--- old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/dma-mapping.h	2005-06-06 16:39:51.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/dma-mapping.h	2005-06-09 11:51:36.000000000 +0000
@@ -8,6 +8,7 @@
 
 #include <linux/config.h>
 
+#include <linux/mm.h>
 #include <asm/scatterlist.h>
 #include <asm/io.h>
 #include <asm/swiotlb.h>
@@ -17,7 +18,7 @@ extern dma_addr_t bad_dma_address;
 	(swiotlb ? swiotlb_dma_mapping_error(x) : ((x) == bad_dma_address))
 
 void *dma_alloc_coherent(struct device *dev, size_t size, dma_addr_t *dma_handle,
-			 unsigned gfp);
+			 int gfp);
 void dma_free_coherent(struct device *dev, size_t size, void *vaddr,
 			 dma_addr_t dma_handle);
 
@@ -53,6 +56,54 @@ static inline void dma_unmap_single(stru
 		out_of_line_bug();
 	/* Nothing to do */
 }
+
+/* Map a set of buffers described by scatterlist in streaming
+ * mode for DMA.  This is the scatter-gather version of the
+ * above pci_map_single interface.  Here the scatter gather list
+ * elements are each tagged with the appropriate dma address
+ * and length.  They are obtained via sg_dma_{address,length}(SG).
+ *
+ * NOTE: An implementation may be able to use a smaller number of
+ *       DMA address/length pairs than there are SG table elements.
+ *       (for example via virtual mapping capabilities)
+ *       The routine returns the number of addr/length pairs actually
+ *       used, at most nents.
+ *
+ * Device ownership issues as mentioned above for pci_map_single are
+ * the same here.
+ */
+static inline int dma_map_sg(struct device *hwdev, struct scatterlist *sg,
+	       int nents, int direction)
+{
+	int i;
+
+	BUG_ON(direction == DMA_NONE);
+ 	for (i = 0; i < nents; i++ ) {
+		struct scatterlist *s = &sg[i];
+		BUG_ON(!s->page); 
+		s->dma_address = virt_to_bus(page_address(s->page) +s->offset);
+		s->dma_length = s->length;
+	}
+	return nents;
+}
+
+EXPORT_SYMBOL(dma_map_sg);
+
+/* Unmap a set of streaming mode DMA translations.
+ * Again, cpu read rules concerning calls here are the same as for
+ * pci_unmap_single() above.
+ */
+static inline void dma_unmap_sg(struct device *dev, struct scatterlist *sg,
+		  int nents, int dir)
+{
+	int i;
+	for (i = 0; i < nents; i++) { 
+		struct scatterlist *s = &sg[i];
+		BUG_ON(s->page == NULL); 
+		BUG_ON(s->dma_address == 0); 
+		dma_unmap_single(dev, s->dma_address, s->dma_length, dir);
+	} 
+}
 #endif
 
 #define dma_map_page(dev,page,offset,size,dir) \
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
--- old-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h	2005-05-28 09:20:36.000000000 +0000
+++ new-xen-64-4/linux-2.6.11-xen-sparse/include/asm-xen/asm-x86_64/pgtable.h	2005-06-08 02:48:53.000000000 +0000
@@ -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() */	\

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

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

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

[Nakajima, Jun]

Scott Parish wrote:
> On Wed, Jun 08, 2005 at 10:23:47PM +0100, Keir Fraser wrote:
> 
>> 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

Looks good. That's the right thing.

BTW, I thought I found a bug in phys_pud_init(), but it's not.

Jun
---
Intel Open Source Technology Center