Re: [PATCH v2] POWERPC: Allow 32-bit pgtable code to support 36-bit physical
From: Becky Bruce <hidden>
Date: 2008-08-28 21:13:25
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:quoted
On Aug 28, 2008, at 11:07 AM, Scott Wood wrote:quoted
Becky Bruce wrote:quoted
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.
quoted
quoted
quoted
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.
quoted
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.
quoted
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 :)
quoted
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 platformNo, unless the code for all platforms makes it safe to do so.
Thanks! -Becky