Benjamin Herrenschmidt <b...@kernel.crashing.org> wrote on 08/10/2009 23:04:03:
>
> On Thu, 2009-10-08 at 15:24 +0200, Joakim Tjernlund wrote:
>
> > +#define _PAGE_RW 0x0400 /* lsb PP bits, inverted in HW */
> > +#define _PAGE_USER 0x0800 /* msb PP bits */
> >
> > + /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> > + rlwimi. r10, r10, 27, 31, 31
> > + beq- cr0, 2f /* Can be removed, costs a ITLB Err */
>
> Did you mean
(counting bits) ... No, it should be >> 5
>
> + /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>4) */
> + rlwimi. r10, r10, 28, 30, 31
> + beq- cr0, 2f /* Can be removed, costs a ITLB Err */
>
> Which would still be incorrect. You want -both- to be set,
> and your code will move on if -any- is set. Might want to add
> a ~ of the whole thing maybe and invert the condition ?
I want:
if (!accessed)
present = 0;
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.
>
> I find it easier to just do li rX, requested_bits, and then, andc.
> rscratch, rX, rPTE
>
> > +#if 0 /* Dont' bother with PP lsb, bit 21 for now */
> > + /* r10 = (r10 & ~0x0400) | ((r10 & _PAGE_EXEC) << 7) */
> > + rlwimi r10, r10, 7, 21, 21 /* Set _PAGE_EXEC << 7 */
> > +#endif
>
> I don't get that one. Don't bother with _PAGE_EXEC, there's no
> control of execute permission that works on 8xx. It's #if 0 anyway.
What about the execute perms in Level 2 descriptor, page 247?
>
> 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.
>
> I would suggest you do the stuff I suggested initially with RW and USER
> being an "index" into a pre-cooked immediate value with all the encodings
> which also gives you the extended encoding for supervisor RO for free.
>
> > + /* Need to know if load/store -> force a TLB Error
> > + * by copying ACCESSED to PRESENT.
> > + */
> > + /* r10=(r10&~_PAGE_PRESENT)|((r10&_PAGE_ACCESSED)>>5) */
> > + rlwimi r10, r10, 27, 31, 31
>
> That doesn't sound right, just like ITLB... you need to AND those two
> bits in a way or another, or basically test that they are both set
> (or neither is set)
Same here as for ITLB.
>
> > +#if 0 /* Not yet */
> > + /* Honour kernel RO, User NA */
> > + andi. r11, r10, _PAGE_USER | _PAGE_RW
> > + bne- cr0, 5f
> > + ori r10,r10, 0x200 /* Extended encoding, bit 22 */
> > #endif
> > - mfspr r11, SPRN_MD_TWC /* get the pte address again */
> > - stw r10, 0(r11)
> > +5: xori r10, r10, _PAGE_RW /* invert RW bit */
>
> Well, you are missing that xori from the ITLB miss I think. Also, that
Nope, no xori needed for exec perms
> changes the table above to:
>
> PTE: Translates to PP bits:
> RW: 0 USER: 0 10 supervisor RW user RW (WRONG)
> RW: 0 USER: 1 11 supervisor RO user RO (ok)
> RW: 1 USER: 0 00 supervisor RW (ok)
> RW: 1 USER: 1 01 supervisor RW user RO (WRONG)
>
> So it's still not right :-)
User and RW swapped here too, as I read the table.
I don't think user space would boot if I got it wrong.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
