RE: [patch] predicate NX flag

RE: [patch] predicate NX flag

[Nakajima, Jun]

Scott Parish wrote:
> 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

Re: [patch] predicate NX flag

[Scott Parish]

On Tue, Jun 07, 2005 at 01:45:45PM -0700, Nakajima, Jun wrote:

> Scott Parish wrote:
> > 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)

Working on a patch.. there are places that xen_l1_entry_update() is
called directly with __PAGE_KERNEL, should those be converted to a
set_pte, or should xen_l1_entry_update be updated instead of the macros,
or should '& __supported_pte_mask' be hardcoded into these places. I'd
prefer to just modify the xen_l?_entry_update()s

btw, __supported_PTE_mask seems an unfortunate name for a mask that gets
used for other pagetable levels.

sRp

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

Re: [patch] predicate NX flag

[Scott Parish]

On Tue, Jun 07, 2005 at 08:47:16PM +0000, Scott Parish wrote:

> ... or should xen_l1_entry_update be updated instead of the macros ...

i guess this option is ruled out in light of future writable pagetable
support.

sRp

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

RE: [patch] predicate NX flag

[Nakajima, Jun]

Ian Pratt wrote:
>>> ... or should xen_l1_entry_update be updated instead of the macros
>>> ... 
>> 
>> i guess this option is ruled out in light of future writable
>> pagetable support.
> 
> Yes. It's critical that everything uses writeable pagetables, as its
> essential for SMP guest support. In general, the x86_64 linux port
> would really benefit from some more unification with i386. It's
> currently missing all sorts of fixes and improvements that have been
> implemented on i386.
> 
> Ian

BTW, with PAE support, is NX working on x86 side?

Jun
---
Intel Open Source Technology Center

RE: [patch] predicate NX flag

[Ian Pratt]

> > ... or should xen_l1_entry_update be updated instead of the 
> macros ...
> 
> i guess this option is ruled out in light of future writable 
> pagetable support.

Yes. It's critical that everything uses writeable pagetables, as its
essential for SMP guest support. In general, the x86_64 linux port would
really benefit from some more unification with i386. It's currently
missing all sorts of fixes and improvements that have been implemented
on i386.

Ian

RE: [patch] predicate NX flag

[Nakajima, Jun]

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

Jun
---
Intel Open Source Technology Center 

-----Original Message-----
From: xen-devel-bounces@lists.xensource.com
[mailto:xen-devel-bounces@lists.xensource.com] On Behalf Of Nakajima,
Jun
Sent: Tuesday, June 07, 2005 1:46 PM
To: Scott Parish
Cc: xen-devel@lists.xensource.com
Subject: RE: [Xen-devel] [patch] predicate NX flag

Scott Parish wrote:
> 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
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

RE: [patch] (resend) mask out nx bits when calculatingpfn/mfn

[Nakajima, Jun]

Scott Parish wrote:
> I'll poke around and try to find who's setting the NX bit. Its still
> not clear to me though why my patch isn't valid. Maybe the traceback
> will help:
> 
> (XEN) (file=dom_mem_ops.c, line=101) Domain 0 page number out of
> range (80000000016b0 >= 180000) ----------- [cut here ] ---------

Right, it's on.

> [please bite here ] --------- 
> Kernel BUG at pci_dma:98
> invalid operand: 0000 [1]
> CPU 0
> Modules linked in:
> Pid: 10266, comm: ifconfig Not tainted 2.6.11.11-xen0
> RIP: e030:[<ffffffff80113bdc>]
> <ffffffff80113bdc>{xen_contig_memory+460} 
> RSP: e02b:ffff880008881ca8  EFLAGS: 00010297
> RAX: 00000000ffffffff RBX: ffff88000e840000 RCX: ffffffff80113bd8
> RDX: 0000000000000001 RSI: ffff880008881cc8 RDI: 0000000000000001
> RBP: 0000000000000000 R08: 0000000000007ff0 R09: ffff880008881cc8
> R10: 0000000000000000 R11: 0000000000000293 R12: 003fffe20023a100
> R13: 000000000003a100 R14: ffff88000e840000 R15: 0000000000000001
> FS:  00002aaaaaac1250(0000) GS:ffffffff804f2400(0000)
> knlGS:0000000000000000 
> CS:  e033 DS: 0000 ES: 0000
> Process ifconfig (pid: 10266, threadinfo ffff880008880000, task
> ffff880000f3c230) 
> Stack: ffff880008881cc8 0000002000000000 0000000000000000
>        0000000400000000 00080000000016b0 ffff88000e840000
>        0000000000000004 0000000000000020 ffff880000b02870
> 0000000000009000 
> Call Trace:<ffffffff80113f97>{dma_alloc_coherent+343}
>        <ffffffff80274d31>{e100_alloc_cbs+113}
>        <ffffffff80275c00>{e100_up+48}
>        <ffffffff80276db8>{e100_open+56}
>        <ffffffff80330f23>{dev_open+67}
>        <ffffffff8033257a>{dev_change_flags+90}
>        <ffffffff80367ee9>{devinet_ioctl+697}
> <ffffffff80369fe7>{inet_ioctl+87} <ffffffff8032894c>{sock_ioctl+588}
> <ffffffff80168c21>{do_ioctl+33} <ffffffff80168f83>{vfs_ioctl+419}
> <ffffffff80168fed>{sys_ioctl+77} <ffffffff8010d421>{system_call+125}
> <ffffffff8010d3a4>{system_call+0}    
> 
> 
> If you look at free_dom_mem(), where the DPRINTK is called, the mfn is
> being copied directly from the user. The hypervisor then checks it to
> make sure that its not out of bounds with max_page, which is where we
> fail. This is why i'm still skeptical that we want the hypervisor to
> have to mask off bits in this exact code.
> 
> sRp

I'm not sure why free_dom_mem() is called (is it the result of the
problem?), but I agree. The caller should mask off the bits in this case
because they are not ptes (but a list of page frame numbers).

       if ( unlikely(__get_user(mpfn, &extent_list[i]) != 0) )
            return i;

        for ( j = 0; j < (1 << extent_order); j++ )
        {
            if ( unlikely((mpfn + j) >= max_page) )
            {
                DPRINTK("Domain %u page number out of range (%lx >=
%lx)\n",
                        d->domain_id, mpfn + j, max_page);
                return i;
            }


Jun
---
Intel Open Source Technology Center

[patch] predicate NX flag

[Scott Parish]

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

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

sRp

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

[-- Attachment #2: predicate-nx.diff --]
[-- Type: text/plain, Size: 1141 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
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-07 18:55:36.000000000 +0000
@@ -186,12 +186,17 @@ static inline pte_t ptep_get_and_clear(p
 #define PAGE_READONLY	__pgprot(_PAGE_PRESENT | _PAGE_USER | _PAGE_ACCESSED | _PAGE_NX)
 #define PAGE_READONLY_EXEC __pgprot(_PAGE_PRESENT | _PAGE_USER | _PAGE_ACCESSED)
 #define __PAGE_KERNEL \
-	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_NX | _PAGE_USER )
+	((_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | \
+	 _PAGE_NX | _PAGE_USER ) & __supported_pte_mask)
 #define __PAGE_KERNEL_EXEC \
 	(_PAGE_PRESENT | _PAGE_RW | _PAGE_DIRTY | _PAGE_ACCESSED | _PAGE_USER )
 #define __PAGE_KERNEL_NOCACHE \

[-- 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] predicate NX flag

[David F Barrera]

Scott,

This patch allowed me to finally be able to boot Dom0 on the HS20 Blade
(x86_64).

On Tue, 2005-06-07 at 19:12 +0000, Scott Parish wrote:
> the NX flag should only be set when its use is enabled.
> 
> sRp
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel
-- 
Regards,

David F Barrera
Linux Technology Center
Systems and Technology Group, IBM

"The wisest men follow their own direction. "
                                                        Euripides