Re: [PATCH 2/6] 8xx: Update TLB asm so it behaves as linux mm expects.

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Fri, 2009-10-09 at 00:44 +0200, Joakim Tjernlund wrote:

> accessed == 1 and present = 0 is impossible, right?
> So basically just copy over accessed to present and
> linux mm set both when trapping to C.

No, when present = 0, then the rest of the PTE can contain unrelated
things, you can't trust ACCESSED.

> What about the execute perms in Level 2 descriptor, page 247?

Not useful, not fine grained enough.

> > You still need to massage the PP bits into place. I don't see that
> > happening.
> 
> Not at the moment, later.
> 
> >
> > As it is, your PTE contains for bit 20 and 21, which translates to:
> >
> >    PTE:                 Translates to PP bits:
> > RW: 0   USER: 0          00  supervisor RW (ok)
> > RW: 0   USER: 1          01  supervisor RW user RO (WRONG)
> > RW: 1   USER: 0          10  supervisor RW user RW (WRONG)
> > RW: 1   USER: 1          11  supervisor RO user RO (WRONG)
> 
> You got USER and RW swapped and the table is different
> for exec.

Hrm, let me see... yes. You are right, I mixed RW and USER. However,
I don't think the PP bits change do they ? IE. Basically, Read == Exec
at the page level. So the table isn't really different between I and D.

However, indeed, since you don't have a unified TLB, the case can be
made that we can ignore R vs. W in the iTLB case. In which case, you get
for iTLB:


    PTE:                 Translates to PP bits:
 RW: 0   USER: 0          00  supervisor X only (ok)
 RW: 0   USER: 1          10  supervisor X user X (ok)
 RW: 1   USER: 0          01  supervisor X user X (WRONG)
 RW: 1   USER: 1          11  supervisor X user X (ok)

So a page with _PAGE_RW and not _PAGE_USER would still be executable
from user... oops :-)

I think what you want for iTLB is just basically have a base of 00
and or-in _PAGE_USER only (ie, keep _PAGE_RW clear with a rlwinm)
so that you basically get supervisor X only if _PAGE_USER is 0 and
both X if _PAGE_USER is 1

For the dTLB, the table becomes (including your inversion of _PAGE_RW)

    PTE:                 Translates to PP bits: 
 RW: 0   USER: 0          01  supervisor RW user RO (WRONG)
 RW: 0   USER: 1          11  supervisor RO user RO (ok)
 RW: 1   USER: 0          00  supervisor RW only (ok)
 RW: 1   USER: 1          10  supervisor RW user RW (ok)

So it's -almost- right :-) You still got the RW:0 USER:0 case wrong,
ie a read-only kernel page would be user readable.

You can work around that by never setting kernel pages read-only (which
we do mostly), but in the grand scheme of things, my trick I proposed
initially would sort it out all including support for kernel RO :-)

In any case, the above, while wrong, wouldn't cause crashes or issues
for well behaved userspace so it's a step forward.

> Same here as for ITLB.

And still not right :-) ie. you cannot rely on the value of
_PAGE_ACCESSED if _PAGE_PRESENT is not set.

> Nope, no xori needed for exec perms

Right, thanks to having a split TLB, but you still need to mask
out one of the bits afaik.

> I don't think user space would boot if I got it wrong.

True. I think it's more correct than I initially thought but still
subtely wrong :-)

Cheers,
Ben.