map_page() and pinned TLB entries

map_page() and pinned TLB entries

[Marcelo Tosatti]

Hi Dan,

So we've got rid of direct pte manipulators similar to what was done in v2.4.

Such patch for v2.6 is in the 8xx tree I posted previously, you've also seen 
it here in the list. 

Remaining problem is that map_page() is going to flush the TLB for the 
addresses being created. Obviously there is a collision with the 4kb 
PTE's, which thrases the 8Mbyte entry.

So what I have done internally is to simply comment out the 
flush_HPTE() call in map_page(), which does the trick.

What would be an elegant way of dealing with this? We can insert
a conditional there such that only addresses not covered 
by other mappings (in this case the 8Mbyte entry) get flushed.

Any better ideas?


diff -Nur --exclude-from=/home/marcelo/excl /mnt/test2/tslinux_mv21/linux-2.6/arch/ppc/mm/pgtable.c linux-2.6/arch/ppc/mm/pgtable.c
--- /mnt/test2/tslinux_mv21/linux-2.6/arch/ppc/mm/pgtable.c     2005-05-16 13:19:47.000000000 -0300
+++ linux-2.6/arch/ppc/mm/pgtable.c     2005-06-16 15:52:32.000000000 -0300
@@ -299,8 +299,8 @@
        if (pg != 0) {
                err = 0;
                set_pte(pg, pfn_pte(pa >> PAGE_SHIFT, __pgprot(flags)));
-               if (mem_init_done)
-                       flush_HPTE(0, va, pmd_val(*pd));
+       //      if (mem_init_done)
+//                     flush_HPTE(0, va, pmd_val(*pd));
        }
        spin_unlock(&init_mm.page_table_lock);
        return err;

Re: map_page() and pinned TLB entries

[Marcelo Tosatti]

On Thu, Jun 16, 2005 at 05:13:19PM -0400, Dan Malek wrote:
> 
> On Jun 16, 2005, at 10:47 AM, Marcelo Tosatti wrote:
> 
> >What would be an elegant way of dealing with this? We can insert
> >a conditional there such that only addresses not covered
> >by other mappings (in this case the 8Mbyte entry) get flushed.
> 
> I'd like to make _tlbie() a macro on 8xx that simply tests the VA
> and doesn't invalidate anything in the pinned space.  Of course,
> the code shouldn't be doing this anyway, and I think this
> should be treated as a bug if it happens. 

Well it is expected that mapin_ram() flushes the va for the 
PTE's being created.

That is the only well known occurence of _tlbie() on 
a possibly pinned region. 

I agree that adding a debugging check on _tlbie() to 
catch invalidations in pinned space is a good thing.

> If the Linux VM thinks it needs to be doing this, pinned space 
> isn't going to work. 

Well the VM does that now, but it can be changed to skip the
pinned region. 

>The _tlbie() is an exported symbol, I suspect for MOL, so
> we have to accommodate that.

What does MOL stand for? 

> Thanks.

There are two separate approaches:

a) Fix mapin_ram() to skip pinned regions
b) Fix _tlbie() to skip pinned regions and warn about such
misusage.

If b) is chosen alone, which is what you seem to suggest, we
will get tons of warnings (or traps) anyway from mapin_ram(). 

So having that said, isnt the correct way out of this to change
both a) and b)?

Re: map_page() and pinned TLB entries

[Dan Malek]

On Jun 16, 2005, at 10:47 AM, Marcelo Tosatti wrote:

> What would be an elegant way of dealing with this? We can insert
> a conditional there such that only addresses not covered
> by other mappings (in this case the 8Mbyte entry) get flushed.

I'd like to make _tlbie() a macro on 8xx that simply tests the VA
and doesn't invalidate anything in the pinned space.  Of course,
the code shouldn't be doing this anyway, and I think this
should be treated as a bug if it happens.  If the Linux VM
thinks it needs to be doing this, pinned space isn't going to work.
The _tlbie() is an exported symbol, I suspect for MOL, so
we have to accommodate that.

Thanks.


	-- Dan

Re: map_page() and pinned TLB entries

[Dan Malek]

On Jun 16, 2005, at 12:47 PM, Marcelo Tosatti wrote:

> Well it is expected that mapin_ram() flushes the va for the
> PTE's being created.
>
> That is the only well known occurence of _tlbie() on
> a possibly pinned region.

Crap ... no, it's not.  It's map_page() that does the flush,
which happens on ioremap() as well.  When we pin
entries, we also pin some IO space as well.  We need
to update ioremap() to be smart about pinned entries
like it does BATs and CAMs.  I wonder if we can just
make this a generic test in ioremap().  It doesn't have
to know how the spaces are "pinned", just that they are
and how they are mapped.

> What does MOL stand for?

MacOS on Linux.  I don't think 8xx will run this any
time soon :-)

> If b) is chosen alone, which is what you seem to suggest, we
> will get tons of warnings (or traps) anyway from mapin_ram().

It's from more than just mapin_ram().  As I said, we also
use some of the entries to pin IO space.  We used to pin only
8M of instructions, which is more than sufficient, then various
8M windows of data space.  I think we used to pin 24M of
data and 8M of IMMR (plus anything above that, which could
be flash or other IO).

> So having that said, isnt the correct way out of this to change
> both a) and b)?

We certainly have to implement b) to catch all pinned spaces,
then update accordingly to ensure none of the wired entries
get evicted.

Thanks.


	-- Dan