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

[Kumar Gala]

On Feb 22, 2008, at 11:08 AM, Marian Balakowicz wrote:

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

I felt the sequence (on PPC) is either:
kernel, cmdline, kdb, initrd

or

kernel, fdt, initrd

The reason being is that initrd doesn't need to be constrained to  
BOOTMAPSZ but cmdline, kdb, and fdt would/should be.

- k