[Qemu-devel] [PATCH 0/2] Support ramdisks with U-Boot header

[Qemu-devel] [PATCH 0/2] Support ramdisks with U-Boot header

[Soren Brinkmann]

For loading Linux kernels, QEMU already is able to recognize and load
them when they feature a U-Boot header. This patch series targets to
extend this support to ramdisks.
Furthermore the ARM support code is changed to use the new functionality.

	Regards,
	Sören

Soren Brinkmann (2):
  hw/loader: Support ramdisk with u-boot header
  hw/arm: Use 'load_ramdisk()' for loading ramdisk

 hw/arm/boot.c       |  8 ++---
 hw/core/loader.c    | 86 +++++++++++++++++++++++++++++++++++++----------------
 include/hw/loader.h |  1 +
 3 files changed, 65 insertions(+), 30 deletions(-)

-- 
1.8.3.1

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

[Soren Brinkmann]

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.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 hw/core/loader.c    | 86 +++++++++++++++++++++++++++++++++++++----------------
 include/hw/loader.h |  1 +
 2 files changed, 61 insertions(+), 26 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 7507914..e72d682 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
 }
 
 /* Load a U-Boot image.  */
-int load_uimage(const char *filename, hwaddr *ep,
-                hwaddr *loadaddr, int *is_linux)
+static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
+                            int *is_linux)
 {
     int fd;
     int size;
+    hwaddr address;
     uboot_image_header_t h;
     uboot_image_header_t *hdr = &h;
     uint8_t *data = NULL;
     int ret = -1;
+    int do_uncompress = 0;
 
     fd = open(filename, O_RDONLY | O_BINARY);
     if (fd < 0)
@@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
         goto out;
 
     /* TODO: Implement other image types.  */
-    if (hdr->ih_type != IH_TYPE_KERNEL) {
-        fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
-        goto out;
-    }
+    switch (hdr->ih_type) {
+    case IH_TYPE_KERNEL:
+        address = hdr->ih_load;
+        if (loadaddr) {
+            *loadaddr = hdr->ih_load;
+        }
+
+        switch (hdr->ih_comp) {
+        case IH_COMP_NONE:
+            break;
+        case IH_COMP_GZIP:
+            do_uncompress = 1;
+            break;
+        default:
+            fprintf(stderr,
+                    "Unable to load u-boot images with compression type %d\n",
+                    hdr->ih_comp);
+            goto out;
+        }
+
+        if (ep) {
+            *ep = hdr->ih_ep;
+        }
 
-    switch (hdr->ih_comp) {
-    case IH_COMP_NONE:
-    case IH_COMP_GZIP:
+        /* TODO: Check CPU type.  */
+        if (is_linux) {
+            if (hdr->ih_os == IH_OS_LINUX) {
+                *is_linux = 1;
+            } else {
+                *is_linux = 0;
+            }
+        }
+
+        break;
+    case IH_TYPE_RAMDISK:
+        address = *loadaddr;
         break;
     default:
-        fprintf(stderr,
-                "Unable to load u-boot images with compression type %d\n",
-                hdr->ih_comp);
+        fprintf(stderr, "Unsupported u-boot image type\n");
         goto out;
     }
 
-    /* TODO: Check CPU type.  */
-    if (is_linux) {
-        if (hdr->ih_os == IH_OS_LINUX)
-            *is_linux = 1;
-        else
-            *is_linux = 0;
-    }
-
-    *ep = hdr->ih_ep;
     data = g_malloc(hdr->ih_size);
 
     if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
@@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
         goto out;
     }
 
-    if (hdr->ih_comp == IH_COMP_GZIP) {
+    if (do_uncompress) {
         uint8_t *compressed_data;
         size_t max_bytes;
         ssize_t bytes;
@@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
         hdr->ih_size = bytes;
     }
 
-    rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
-
-    if (loadaddr)
-        *loadaddr = hdr->ih_load;
+    rom_add_blob_fixed(filename, data, hdr->ih_size, address);
 
     ret = hdr->ih_size;
 
@@ -522,6 +538,24 @@ out:
     return ret;
 }
 
+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);
+    }
+
+    return size;
+}
+
 /*
  * Functions for reboot-persistent memory regions.
  *  - used for vga bios and option roms.
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 0958f06..8ef403e 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -15,6 +15,7 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
               int bswap_needed, hwaddr target_page_size);
 int load_uimage(const char *filename, hwaddr *ep,
                 hwaddr *loadaddr, int *is_linux);
+int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);
 
 ssize_t read_targphys(const char *name,
                       int fd, hwaddr dst_addr, size_t nbytes);
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/2] hw/arm: Use 'load_ramdisk()' for loading ramdisk

[Soren Brinkmann]

The load_ramdisk function takes over loading traditional ramdisks images
and does also load ramdisks with u-boot header.

Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
---
 hw/arm/boot.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index f310f73..ab2b234 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -436,10 +436,10 @@ void arm_load_kernel(ARMCPU *cpu, struct arm_boot_info *info)
     info->entry = entry;
     if (is_linux) {
         if (info->initrd_filename) {
-            initrd_size = load_image_targphys(info->initrd_filename,
-                                              info->initrd_start,
-                                              info->ram_size -
-                                              info->initrd_start);
+            initrd_size = load_ramdisk(info->initrd_filename,
+                                       info->initrd_start,
+                                       info->ram_size -
+                                       info->initrd_start);
             if (initrd_size < 0) {
                 fprintf(stderr, "qemu: could not load initrd '%s'\n",
                         info->initrd_filename);
-- 
1.8.3.1

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

[Peter Maydell]

On 14 June 2013 17:36, Soren Brinkmann <soren.brinkmann@xilinx.com> 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.

Hi; thanks for this patch; some review comments below.

>
> Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> ---
>  hw/core/loader.c    | 86 +++++++++++++++++++++++++++++++++++++----------------
>  include/hw/loader.h |  1 +
>  2 files changed, 61 insertions(+), 26 deletions(-)
>
> diff --git a/hw/core/loader.c b/hw/core/loader.c
> index 7507914..e72d682 100644
> --- a/hw/core/loader.c
> +++ b/hw/core/loader.c
> @@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
>  }
>
>  /* Load a U-Boot image.  */
> -int load_uimage(const char *filename, hwaddr *ep,
> -                hwaddr *loadaddr, int *is_linux)
> +static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> +                            int *is_linux)
>  {
>      int fd;
>      int size;
> +    hwaddr address;
>      uboot_image_header_t h;
>      uboot_image_header_t *hdr = &h;
>      uint8_t *data = NULL;
>      int ret = -1;
> +    int do_uncompress = 0;
>
>      fd = open(filename, O_RDONLY | O_BINARY);
>      if (fd < 0)
> @@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
>          goto out;
>
>      /* TODO: Implement other image types.  */

Doesn't this patch fix this TODO item?

> -    if (hdr->ih_type != IH_TYPE_KERNEL) {
> -        fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
> -        goto out;
> -    }
> +    switch (hdr->ih_type) {
> +    case IH_TYPE_KERNEL:
> +        address = hdr->ih_load;
> +        if (loadaddr) {
> +            *loadaddr = hdr->ih_load;
> +        }
> +
> +        switch (hdr->ih_comp) {
> +        case IH_COMP_NONE:
> +            break;
> +        case IH_COMP_GZIP:
> +            do_uncompress = 1;
> +            break;
> +        default:
> +            fprintf(stderr,
> +                    "Unable to load u-boot images with compression type %d\n",
> +                    hdr->ih_comp);
> +            goto out;
> +        }
> +
> +        if (ep) {
> +            *ep = hdr->ih_ep;
> +        }
>
> -    switch (hdr->ih_comp) {
> -    case IH_COMP_NONE:
> -    case IH_COMP_GZIP:
> +        /* TODO: Check CPU type.  */
> +        if (is_linux) {
> +            if (hdr->ih_os == IH_OS_LINUX) {
> +                *is_linux = 1;
> +            } else {
> +                *is_linux = 0;
> +            }
> +        }
> +
> +        break;
> +    case IH_TYPE_RAMDISK:
> +        address = *loadaddr;
>          break;
>      default:
> -        fprintf(stderr,
> -                "Unable to load u-boot images with compression type %d\n",
> -                hdr->ih_comp);
> +        fprintf(stderr, "Unsupported u-boot image type\n");

You could include the type byte here, the way we do for
unknown compression types.

>          goto out;
>      }
>
> -    /* TODO: Check CPU type.  */
> -    if (is_linux) {
> -        if (hdr->ih_os == IH_OS_LINUX)
> -            *is_linux = 1;
> -        else
> -            *is_linux = 0;
> -    }
> -
> -    *ep = hdr->ih_ep;
>      data = g_malloc(hdr->ih_size);
>
>      if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> @@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
>          goto out;
>      }
>
> -    if (hdr->ih_comp == IH_COMP_GZIP) {
> +    if (do_uncompress) {
>          uint8_t *compressed_data;
>          size_t max_bytes;
>          ssize_t bytes;
> @@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
>          hdr->ih_size = bytes;
>      }
>
> -    rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> -
> -    if (loadaddr)
> -        *loadaddr = hdr->ih_load;
> +    rom_add_blob_fixed(filename, data, hdr->ih_size, address);
>
>      ret = hdr->ih_size;
>
> @@ -522,6 +538,24 @@ out:
>      return ret;
>  }
>
> +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.

> +
> +    return size;
> +}

If we're providing separate functions for kernel and
ramdisk loading shouldn't they check that the uimage
has the appropriate type?

> +
>  /*
>   * Functions for reboot-persistent memory regions.
>   *  - used for vga bios and option roms.
> diff --git a/include/hw/loader.h b/include/hw/loader.h
> index 0958f06..8ef403e 100644
> --- a/include/hw/loader.h
> +++ b/include/hw/loader.h
> @@ -15,6 +15,7 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
>                int bswap_needed, hwaddr target_page_size);
>  int load_uimage(const char *filename, hwaddr *ep,
>                  hwaddr *loadaddr, int *is_linux);
> +int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);

Since this is a new function, it should have a suitably
formatted documentation comment. (the extract and deposit
functions at the bottom of include/qemu/bitops.h are the
examples of the format that I usually crib from.)

thanks
-- PMM

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

[Sören Brinkmann]

Hi Peter,

On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
> On 14 June 2013 17:36, Soren Brinkmann <soren.brinkmann@xilinx.com> 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.
> 
> Hi; thanks for this patch; some review comments below.
> 
> >
> > Signed-off-by: Soren Brinkmann <soren.brinkmann@xilinx.com>
> > ---
> >  hw/core/loader.c    | 86 +++++++++++++++++++++++++++++++++++++----------------
> >  include/hw/loader.h |  1 +
> >  2 files changed, 61 insertions(+), 26 deletions(-)
> >
> > diff --git a/hw/core/loader.c b/hw/core/loader.c
> > index 7507914..e72d682 100644
> > --- a/hw/core/loader.c
> > +++ b/hw/core/loader.c
> > @@ -434,15 +434,17 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
> >  }
> >
> >  /* Load a U-Boot image.  */
> > -int load_uimage(const char *filename, hwaddr *ep,
> > -                hwaddr *loadaddr, int *is_linux)
> > +static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
> > +                            int *is_linux)
> >  {
> >      int fd;
> >      int size;
> > +    hwaddr address;
> >      uboot_image_header_t h;
> >      uboot_image_header_t *hdr = &h;
> >      uint8_t *data = NULL;
> >      int ret = -1;
> > +    int do_uncompress = 0;
> >
> >      fd = open(filename, O_RDONLY | O_BINARY);
> >      if (fd < 0)
> > @@ -458,31 +460,48 @@ int load_uimage(const char *filename, hwaddr *ep,
> >          goto out;
> >
> >      /* TODO: Implement other image types.  */
> 
> Doesn't this patch fix this TODO item?
Well, partly. There are a couple of more image types which are still
not supported. So, I didnt' bother touching this comment just because
this adds support for a second out of 10 or something types.

> 
> > -    if (hdr->ih_type != IH_TYPE_KERNEL) {
> > -        fprintf(stderr, "Can only load u-boot image type \"kernel\"\n");
> > -        goto out;
> > -    }
> > +    switch (hdr->ih_type) {
> > +    case IH_TYPE_KERNEL:
> > +        address = hdr->ih_load;
> > +        if (loadaddr) {
> > +            *loadaddr = hdr->ih_load;
> > +        }
> > +
> > +        switch (hdr->ih_comp) {
> > +        case IH_COMP_NONE:
> > +            break;
> > +        case IH_COMP_GZIP:
> > +            do_uncompress = 1;
> > +            break;
> > +        default:
> > +            fprintf(stderr,
> > +                    "Unable to load u-boot images with compression type %d\n",
> > +                    hdr->ih_comp);
> > +            goto out;
> > +        }
> > +
> > +        if (ep) {
> > +            *ep = hdr->ih_ep;
> > +        }
> >
> > -    switch (hdr->ih_comp) {
> > -    case IH_COMP_NONE:
> > -    case IH_COMP_GZIP:
> > +        /* TODO: Check CPU type.  */
> > +        if (is_linux) {
> > +            if (hdr->ih_os == IH_OS_LINUX) {
> > +                *is_linux = 1;
> > +            } else {
> > +                *is_linux = 0;
> > +            }
> > +        }
> > +
> > +        break;
> > +    case IH_TYPE_RAMDISK:
> > +        address = *loadaddr;
> >          break;
> >      default:
> > -        fprintf(stderr,
> > -                "Unable to load u-boot images with compression type %d\n",
> > -                hdr->ih_comp);
> > +        fprintf(stderr, "Unsupported u-boot image type\n");
> 
> You could include the type byte here, the way we do for
> unknown compression types.
good call. I'll add that.

> 
> >          goto out;
> >      }
> >
> > -    /* TODO: Check CPU type.  */
> > -    if (is_linux) {
> > -        if (hdr->ih_os == IH_OS_LINUX)
> > -            *is_linux = 1;
> > -        else
> > -            *is_linux = 0;
> > -    }
> > -
> > -    *ep = hdr->ih_ep;
> >      data = g_malloc(hdr->ih_size);
> >
> >      if (read(fd, data, hdr->ih_size) != hdr->ih_size) {
> > @@ -490,7 +509,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> >          goto out;
> >      }
> >
> > -    if (hdr->ih_comp == IH_COMP_GZIP) {
> > +    if (do_uncompress) {
> >          uint8_t *compressed_data;
> >          size_t max_bytes;
> >          ssize_t bytes;
> > @@ -508,10 +527,7 @@ int load_uimage(const char *filename, hwaddr *ep,
> >          hdr->ih_size = bytes;
> >      }
> >
> > -    rom_add_blob_fixed(filename, data, hdr->ih_size, hdr->ih_load);
> > -
> > -    if (loadaddr)
> > -        *loadaddr = hdr->ih_load;
> > +    rom_add_blob_fixed(filename, data, hdr->ih_size, address);
> >
> >      ret = hdr->ih_size;
> >
> > @@ -522,6 +538,24 @@ out:
> >      return ret;
> >  }
> >
> > +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.

> 
> > +
> > +    return size;
> > +}
> 
> If we're providing separate functions for kernel and
> ramdisk loading shouldn't they check that the uimage
> has the appropriate type?
I'm not sure I understand what you mean here. Moving the check for the
appropriate u-boot header type down here? I tried to find some
reasonable way to further split the load_uboot_image() routine to move
some of it's functionality into the two functions down here. But it all
seemed pretty messy and I left pretty much all functionality in
load_uboot_image() and just pass errors through.

> 
> > +
> >  /*
> >   * Functions for reboot-persistent memory regions.
> >   *  - used for vga bios and option roms.
> > diff --git a/include/hw/loader.h b/include/hw/loader.h
> > index 0958f06..8ef403e 100644
> > --- a/include/hw/loader.h
> > +++ b/include/hw/loader.h
> > @@ -15,6 +15,7 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
> >                int bswap_needed, hwaddr target_page_size);
> >  int load_uimage(const char *filename, hwaddr *ep,
> >                  hwaddr *loadaddr, int *is_linux);
> > +int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz);
> 
> Since this is a new function, it should have a suitably
> formatted documentation comment. (the extract and deposit
> functions at the bottom of include/qemu/bitops.h are the
> examples of the format that I usually crib from.)
Okay, I'll have a look at those.

	Sören

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

[Peter Maydell]

On 14 June 2013 18:14, Sören Brinkmann <soren.brinkmann@xilinx.com> wrote:
> On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
>> If we're providing separate functions for kernel and
>> ramdisk loading shouldn't they check that the uimage
>> has the appropriate type?
> I'm not sure I understand what you mean here. Moving the check for the
> appropriate u-boot header type down here? I tried to find some
> reasonable way to further split the load_uboot_image() routine to move
> some of it's functionality into the two functions down here. But it all
> seemed pretty messy and I left pretty much all functionality in
> load_uboot_image() and just pass errors through.

I had in mind that these functions should pass the "expected"
type byte in to the load_uboot_image() function so it can
fail if the blob it is passed isn't actually the right type
(eg you pass the kernel to -initrd and vice-versa).

thanks
-- PMM

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

[Sören Brinkmann]

On Fri, Jun 14, 2013 at 06:36:06PM +0100, Peter Maydell wrote:
> On 14 June 2013 18:14, Sören Brinkmann <soren.brinkmann@xilinx.com> wrote:
> > On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
> >> If we're providing separate functions for kernel and
> >> ramdisk loading shouldn't they check that the uimage
> >> has the appropriate type?
> > I'm not sure I understand what you mean here. Moving the check for the
> > appropriate u-boot header type down here? I tried to find some
> > reasonable way to further split the load_uboot_image() routine to move
> > some of it's functionality into the two functions down here. But it all
> > seemed pretty messy and I left pretty much all functionality in
> > load_uboot_image() and just pass errors through.
> 
> I had in mind that these functions should pass the "expected"
> type byte in to the load_uboot_image() function so it can
> fail if the blob it is passed isn't actually the right type
> (eg you pass the kernel to -initrd and vice-versa).
Thanks for the clarification. I agree. I'll add such a check.

	Sören

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
<soren.brinkmann@xilinx.com> wrote:
> Hi Peter,
>
> On Fri, Jun 14, 2013 at 05:56:31PM +0100, Peter Maydell wrote:
>> On 14 June 2013 17:36, Soren Brinkmann <soren.brinkmann@xilinx.com> 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