[OpenBIOS] [PATCH v5] ppc: SDR1 fixes

[Alexander Graf]

On 22.11.2010, at 21:42, Andreas Färber wrote:

> Am 22.11.2010 um 21:31 schrieb Alexander Graf:
> 
>> On 22.11.2010, at 21:20, Andreas Färber wrote:
>> 
>>> Am 22.11.2010 um 00:46 schrieb Alexander Graf:
>>> 
>>>> On 22.11.2010, at 00:14, Andreas Färber <andreas.faerber at web.de> wrote:
>>>> 
>>>>> Clean up use of SDR1 fields.
>>>>> 
>>>>> IBM's "The Programming Environments for 32-Bit Microprocessors":
>>>>> The HTABMASK field in SDR1 contains a mask value that determines
>>>>> how many bits from the hash are used in the page table index. This mask
>>>>> must be of the form 0b00...011...1; that is, a string of 0 bits followed by
>>>>> a string of 1bits. The 1 bits determine how many additional bits (at least 10)
>>>>> from the hash are used in the index; HTABORG must have this same number of
>>>>> low-order bits equal to 0.
>>>>> 
>>>>> IBM's "The Programming Environments Manual for 64-bit Microprocessors" 3.0:
>>>>> The HTABSIZE field in SDR1 contains an integer value that determines
>>>>> how many bits from the hash are used in the page table index. This number
>>>>> must not exceed 28. HTABSIZE is used to generate a mask of the form
>>>>> 0b00...011...1; that is, a string of 0 bits followed by a string of 1-bits.
>>>>> The 1-bits determine how many additional bits (beyond the minimum of 11)
>>>>> from the hash are used in the index. The HTABORG must have this same number
>>>>> of low-order bits equal to 0.
>>>>> 
>>>>> Adjust alignment mask and hash size for ppc64.
>>>>> 
>>>>> Note that the page table does not yet scale with the memory size, and
>>>>> the secondary hash is not yet used.
>>>>> 
>>>>> v5:
>>>>> * Move HTABORG mask to header.
>>>>> * Keep is_ppc64() for now, suggested by Alex.
>>>>> Merge CONFIG_PPC_64BITSUPPORT handling here so that it behaves as intended,
>>>>> not recognizing a ppc64 CPU in pure 32-bit mode.
>>>>> * Stub out hash_page_32() for ppc64 to avoid having to care about it for ppc64.
>>>>> hash_page_64() is less critical for pure ppc build since the code needs to be
>>>>> 32-bit compatible anyway.
>>>>> 
>>>>> v4:
>>>>> * Don't redefine hash_page_{32,64} but add TODO comments for renaming.
>>>>> * Keep and simplify hash_page().
>>>>> 
>>>>> v3:
>>>>> * Fix shifts. Thanks to Segher and Alex.
>>>>> * Fix HTABSIZE for 32-bit ppc64: ((2 << 15) - 1) >> 16 happened to be 0,
>>>>> so that it worked as expected. Avoid underflow, spotted by Alex.
>>>>> Make this fix available for legacy 64-bit support as well.
>>>>> * Drop get_hash_size(). It was only used in hash_page_32() and caused
>>>>> unnecessary conversions between HTABMASK and size.
>>>>> * Introduce {mfsdr1,mtsdr1} as suggested by Alex.
>>>>> Note that this changes the volatility for hash_page_64().
>>>>> * Conditionalize 64-bit support, drop 32-bit support for ppc64.
>>>>> * Adjust hash size for ppc64.
>>>>> 
>>>>> v2:
>>>>> * Use HTABSIZE for ppc64, fix HTABMASK usage for ppc.
>>>>> Error spotted by Segher.
>>>>> 
>>>>> Cc: Alexander Graf <agraf at suse.de>
>>>>> Cc: Segher Boessenkool <segher at kernel.crashing.org>
>>>>> Signed-off-by: Andreas Färber <andreas.faerber at web.de>
>>>> 
>>>> This one looks a lot cleaner. I don't see any obvious glitches either, so:
>>>> 
>>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>> 
>>> Phew... thanks! Applied as r967.
>>> 
>>> A rather unproductive weekend unfortunately, no progress in gdb yet...
>> 
>> Sorry for making you go through so many iterations :).
> 
> I'll be stricter in the future, you too have commit rights, to create the inline functions you like. :)

True, just push me back when I get too annoying :).

> 
>> One thing came to mind still: Please make sure that you take into account that the page table size is different for ppc64-on-ppc32 than the define inclines. If anything needs to know the real htab size, you need to have that one call a function that find it out for real.
> 
> You mean the 15 that our MAX() turns into 18? Our code always reads HTABORG and HTABMASK/HTABSIZE from SDR1 so we should be just fine once it's correctly initialized.
> As noted, we should scale the page table with RAM size and use these defines as lower limit only.

I don't think it's necessary to scale with RAM size, as we shouldn't use more RAM with increasing RAM size. We're fortunately not a full blown OS :).


Alex