[PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical

[Becky Bruce]

Great, so *you* got my email, and I did not.  I love our mailserver!

On Aug 28, 2008, at 3:28 PM, Scott Wood wrote:

> Becky Bruce wrote:
>> On Aug 28, 2008, at 11:07 AM, Scott Wood wrote:
>>> Becky Bruce wrote:
>>>> I'm pretty sure I went through this in great detail at one point
>>>> and concluded that I did in fact need the lwarx/stwcx.  IIRC, it
>>>> has to do with other non-set_pte_at writers not necessarily
>>>> holding the page table lock. FYI, the existing 32-bit PTE code is
>>>> doing atomic updates as well.
>>> But will those updates happen if there isn't already a valid PTE?
>> I understand what you're saying, I've been here before :)  However, I
>> was never able to convince myself that it's safe without the lwarx/ 
>> stwcx.  There's hashing code that wanks around with the HASHPTE
>> bit doing a RMW without holding any lock (other than lwarx/stwcx-ing
>> the PTE itself).
>
> OK.  I was concerned not just about efficiency, but of the safety of  
> the  "stw" write if there were other modifications going on (even if  
> the set_pte_at stwcx fails, the other updater could have lwarxed an  
> succesfully stwcxed after the stw and ended up with a mixed PTE),  
> but it may not be an issue depending on the nature of the updates.

It shouldn't be an issue - set_pte_at() is the only writer of the high  
bits of the PTE, the pte is invalid upon entry to set_pte_at(), and  
none of the potential interfering writers should be turning on the  
valid bit.

>
>
>>>> About PTE_ATOMIC_UPDATES, I didn't add that in because hashed
>>>> page table implementations require atomic updates.
>>> Right, I misread it and thought it was being used for non-hashed  
>>> implementations as well.
>>> What happens if you enable 64-bit PTEs on a 603-ish CPU?  The
>>> kconfig seems to allow it.
>> Don't do that :)  That's why the help is there in the Kconfig.
>
> People will do it anyway -- and there's multiplatform to consider.

>
>
>> Otherwise, I have to list out every 74xx part that supports 36-bit  
>> physical addressing.  In any event, nothing interesting will happen  
>> other than that you'll waste some space.  The kernel boots fine with
>> a non-36b physical u-boot and small amounts of RAM.
>
> My concern was the generic code trying to use 64-bit PTEs, and the  
> 603 TLB miss handlers continuing to assume that the PTEs are 32-bit,  
> and loading the wrong thing.
>
> Wasted space alone is an acceptable consequence of turning on things  
> you don't need. :-)

Actually, I'm lying to you - I forgot about the old parts with DTLB/ 
ITLB handlers.  That code will actually break, and I'd rather not hack  
it up to pointlessly accomodate large PTEs.  I do need to fix this the  
Kconfig, even though it's going to be gross. Thanks for pointing this  
out.

>
>
>> I'm still not sure where you're going with this - I can remove 44x
>> from the conditional part, but we still have to deal with e500 and
>> 6xx.
>
> You still need it in "depends" (in the absence of a  
> "PHYS_64BIT_CAPABLE" or some such), but not "bool '...' if".  It's  
> not a big deal, just a pet peeve.

I'll look at making it less peevy :)

>
>
>> In which case you're now setting this in different places for difft
>> plats, making it potentially harder to read.  Unless you're
>> suggesting allowing the selection of PHYS_64BIT on any platform
>
> No, unless the code for all platforms makes it safe to do so.
>

Thanks!
-Becky