Re: [Qemu-devel] [PATCH 1/2] hw/loader: Support ramdisk with u-boot header

[Peter Crosthwaite]

Hi Soren,

On Sat, Jun 15, 2013 at 3:14 AM, Sören Brinkmann
<address@hidden> wrote:
> Hi Peter,
>
> On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
>> On 14 June 2013 17:36, Soren Brinkmann <address@hidden> wrote:
>> > Introduce 'load_ramdisk()' which can load "normal" ramdisks and ramdisks
>> > with a u-boot header.
>> > To enable this and leverage synergies 'load_uimage()' is refactored to
>> > accomodate this additional use case.
[snip]
>> > +int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
>> > +                int *is_linux)
>> > +{
>> > +    return load_uboot_image(filename, ep, loadaddr, is_linux);
>> > +}
>> > +
>> > +/* Load a ramdisk.  */
>> > +int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
>> > +{
>> > +    int size = load_uboot_image(filename, NULL, &addr, NULL);
>> > +
>> > +    if (size < 0) {
>> > +        size = load_image_targphys(filename, addr, max_sz);
>> > +    }
>>
>> So I can sort of see why we end up this way, but it's a bit
>> asymmetric that we handle 'uimage or raw' ramdisk at this
>> level, but require the caller to do it for the kernel.
> I agree it's not symmetric. The question is: Leaving this as is and hope
> somebody changes the kernel loading. Or should this be changed with
> having the "normal" ramdisk fallback at caller level?
> I like this solution better, but well, that's probably just me.
>

I prefer the symmetric approach, The if-else logic on image types is a
board/arch specific policy (even though it arguably shouldn't be).
Fixing this I think is best left to when we get around to doing a
bootloader overhaul. Can we just drop this if and push the targphys
fallback to the arch bootloaders?

Regards,
Peter