[U-Boot-Users] [PATCH v2] [new uImage] ppc: Re-order ramdisk/fdt handling sequence

[Marian Balakowicz <m8@semihalf.com>]

Jerry Van Baren wrote:
> Marian Balakowicz wrote:
>> Kumar Gala wrote:
>>> On Feb 18, 2008, at 1:46 PM, Jerry Van Baren wrote:
>>>
>>>> Kumar Gala wrote:
>>>>> On Feb 18, 2008, at 1:15 PM, Jerry Van Baren wrote:
>>>>>> Kumar Gala wrote:
>>>>>>> On Feb 18, 2008, at 11:51 AM, Jerry Van Baren wrote:
>>>>>>>> Kumar Gala wrote:
>>>> [[[[snip]]]]
>>>>
>>>>>> The patch is creating dummy initrd entries in the reserved map
>>>>>> and  in /chosen, only to work hard to delete and re-create the
>>>>>> reserved  map entries and rewrite the /chosen entries.
>>>>>>
>>>>>> My counter-proposal is to not bother with dummy values.  Simply 
>>>>>> pass in 0,0 which will prevent the creation of the initrd entries 
>>>>>> by fdt_chosen().  By not creating dummy entries, you can simply 
>>>>>> create the proper entries once you know what the the correct 
>>>>>> values are, rather than the more complicated rsvmap search & 
>>>>>> delete + rsvmap creation + /chosen modifications.
>>>>> Ahh, the reason I wanted them created was to ensure we have enough 
>>>>> size for them up front rather than figuring that out later.  By 
>>>>> creating them and replacing them I will not being changing the
>>>>> size  at all.
>>>>> - k
>>>> OK, I see.
>>>>
>>>> Currently this isn't an issue because our blob has a fixed size
>>>> that  has free space inside it, so creating the rsvmap and /chosen
>>>> entries  eat at the internal free space and don't change the total
>>>> blob size.
>>>>
>>>> People are advocating dynamically increasing the blob size, which 
>>>> simplifies things for blob generation (don't have to guess how big 
>>>> to make the blob when running the dtc to create it), but that would 
>>>> cause problems with my counter-proposal.
>>> And the whole point of my patch was to enable the ability to 
>>> dynamically grow the blob before we do anything w/the ramdisk.
>>
>> But we don't really grow the blob, we are just allocating the space for
>> the initrd properties - *if* the blob already has enough free space. If
>> the blob does not have enough free space we'll hit the bottom anyway,
>> whether in fdt_chosen() or ft_board_setup(), so it seem that it doesn't
>> matter whether we pre-allocate space for initrd or not. Or am I missing
>> something?
>>
>> I was rather thinking of increasing the total blob size when relocating
>> it. Currently relocation happens only when the blob is not within
>> BOOTMAPSZ region, so we would need to always relocate the blob and
>> figure out the size delta: (1) get it from env variable, if set (2) or
>> use some default delta. What do you think?
>>

> The missing part is libfdt doesn't exactly support dynamic resizing and
> our current code doesn't do in-place resizing (which it could do by
> doing a move to the same location, but with a larger/smaller length).
> 
> Kumar is lining up the pieces to get there, but we aren't there yet...

I see, but how about resizing to a new location:

-   err = fdt_open_into (fdt_blob, (void *)of_start, of_len);
+   err = fdt_open_into (fdt_blob, (void *)of_start, of_len + delta);

Should that work?

If we add LMB and rework bootm memory allocation, putting things
(kernel, cmdline, kdb, initrd (optionally), fdt) in sequence starting
from bootm_low then we may want to always relocate fdt to avoid
overlapping. And, in case of new uImage FDT blob will be embedded in a
new uImage shell which is a blob itself. So, in this case in-place
resizing is not really a clean option, we would need to resize the
embedding new uImage blob first, and this one may have significant size,
so I suspect it may impact performance.

m.