[OpenBIOS] [PATCH v5] ppc: SDR1 fixes

Andreas Färber andreas.faerber at web.de
Mon Nov 22 21:42:50 CET 2010


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. :)

> 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.

Andreas


More information about the OpenBIOS mailing list