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

Mark Cave-Ayland mark.cave-ayland at ilande.co.uk
Thu Jun 18 08:20:49 CEST 2015

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



More information about the OpenBIOS mailing list