Xen devel - Jun 2005 - RE: [patch] nx bit shouldn''t get set when disabled

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

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

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


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