[OpenBIOS] [PATCH] ppc: fix stack usage in mmu_claim, mem_claim

[Mark Cave-Ayland]

On 18/06/15 00:37, Segher Boessenkool wrote:

> On Wed, Jun 17, 2015 at 11:30:37PM +0100, Mark Cave-Ayland wrote:
>>>> The 'phys' argument to mem_claim() and 'virt' argument to mmu_claim() are now
>>>> only popped from the stack if the 'align' argument is provided.
>>>
>>> You probably also want to mention you no longer THROW when the
>>> return value is -1 (and explain why).
>>
>> I'm not too worried about missing the throw here right now.
> 
> Neither am I, but it's a silent change; the commit message should say.
> I think the original code is simply wrong fwiw, the client interface
> shouldn't throw (but return an error).

Okay. How about we add a note mentioning that the throw is removed due
to problems across multiple C/Forth boundaries and to keep everything
consistent across all archs? If we do come up with a way to get this to
work, we should do it consistently across all archs.

>> One of the more recent changes I managed to do was to unmap page 0 on
>> all 3 of SPARC32, SPARC64 and PPC and still have everything boot.
> 
> :-)
> 
>>>> +	phys_addr_t phys = -1UL;
>>>
>>> Is this the common style in this code?  Will "long int" always fit
>>> phys_addr_t?  Seems shaky.
>>
>> In the original OFMEM implementation sizeof(phys_addr_t) == sizeof(virt)
>> == word size of machine. The only reason phys_addr_t is different here
>> was since SPARC32 was switched over to OFMEM as SPARC32 has a 36-bit
>> physical address space vs. a 32-bit virtual address space just to make
>> things more interesting :)  I haven't compile-tested this yet but if you
>> can suggest a better alternative, please do.
> 
> -1ULL should work.  Plain -1 works fine, too.  -1UL doesn't work correctly
> (if phys_addr_t is bigger than long int).  So just -1 is my preference.

Plain -1 it is then :)


ATB,

Mark.