Re: early fixmap causes kmap breakage

[Ian Campbell <ijc@hellion.org.uk>]

Adding Jeremy to CC

On Wed, 2008-12-31 at 10:33 +0100, Nick Piggin wrote:
> 
> > >> Ditching the swapper_pg_fixmap has some problems.
> > >> 
> > >> This appears to break early_printk to a usb debug port, which 
> > >> calls set_fixmap_nocache and expects the mapping to last.
> > >> 
> > >> This looks like it will have problems with Xen and other 
> > >> environments where we come in with a pre-populated page table,
> > >> possibly unmapping something important.
> > >
> > > My patch copies the early fixmap mappings to the new page table.

> > > Isn't this enough?
> > 
> > I'm not certain.  Xen can get really finicky if you mess with it's
> > pre-established mappings.  Which is one of the reasons why we 
> > preserve the current mappings.  It was a real struggle to find a 
> > solution that worked for everything last time we had a PAE problem,
> > and apparently we failed to test with large numbers of cpus in the
> > config.
> > 
> > So please excuse me if I'm leery of a quick fix, to code that is 
> > obviously insufficiently clean to do what needs to be done 
> > intentionally.
> 
> Well we should just ask the Xen people whether it will work or not.
> Why wouldn't it? Xen has to be able to cope with kernel mappings
> coming and going.

I think it should be fine from Xen's PoV since one_page_table_init()
correctly uses paravirt_alloc_pte() and set_pte() does the right thing
under Xen.

BTW, I needed this fixlet since set_pte takes a pte_t not a pteval_t,
otherwise:
  CC      arch/x86/mm/init_32.o
[...]arch/x86/mm/init_32.c: In function 'page_table_range_init':
[...]arch/x86/mm/init_32.c:188: error: incompatible type for argument 2 of 'set_pte'

diff -r cf59aba1694f arch/x86/mm/init_32.c
--- a/arch/x86/mm/init_32.c	Fri Jan 09 10:13:31 2009 +0000
+++ b/arch/x86/mm/init_32.c	Fri Jan 09 10:16:38 2009 +0000
@@ -185,7 +185,7 @@
 				newpte = one_page_table_init(pmd);
 				BUG_ON(lastpte + PTRS_PER_PTE != newpte);
 				for (i = 0; i < PTRS_PER_PTE; i++) {
-					set_pte(newpte + i, pte_val(*(pte + i)));
+					set_pte(newpte + i, *(pte + i));
 				}
 				pte = lastpte;
 			}

-- 
Ian Campbell
Current Noise: Firebird - Needle In The Groove

"Now we'll have to kill you."
		-- Linus Torvalds