Nakajima, Jun
2005-Jun-08 21:56 UTC
RE: [Xen-devel] [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? > > -- KeirHold 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
Keir Fraser
2005-Jun-08 22:00 UTC
Re: [Xen-devel] [patch] nx bit shouldn''t get set when disabled
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