[Qemu-devel] [PATCH 0/3] target-xtensa: fix loading uImage kernels on MMUv2 cores

[Qemu-devel] [PATCH 0/3] target-xtensa: fix loading uImage kernels on MMUv2 cores

[Max Filippov]

Hi,

this series fixes loading uImage kernels on MMUv2 xtensa cores.

U-boot for xtensa always treats uImage load address as virtual address.
This is important when booting uImage on xtensa core with MMUv2, because
MMUv2 has fixed non-identity virtual-to-physical mapping after reset.

I add two new functions: load_uboot_image_header that loads uImage header
and load_uimage_at that loads uImage at the specified address, and use them
to query uImage load address and load uImage at the correctly translated
address.

Max Filippov (3):
  hw/core/loader: implement load_uboot_image_header
  hw/core/loader: implement load_uimage_at
  target-xtensa: treat uImage load address as virtual

 hw/core/loader.c    | 62 +++++++++++++++++++++++++++++++++++++++++------------
 hw/xtensa/xtfpga.c  |  9 +++++++-
 include/hw/loader.h |  4 ++++
 3 files changed, 60 insertions(+), 15 deletions(-)

-- 
1.8.1.4

[Qemu-devel] [PATCH 1/3] hw/core/loader: implement load_uboot_image_header

[Max Filippov]

Extract uImage header loader and allow using it from the rest of the
code.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/core/loader.c    | 37 +++++++++++++++++++++++++++++--------
 include/hw/loader.h |  2 ++
 2 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 2bf6b8f..5bf7f9b 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -454,12 +454,25 @@ static ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
     return dstbytes;
 }
 
+static int load_uboot_image_header_fd(int fd, uboot_image_header_t *hdr)
+{
+    int size = read(fd, hdr, sizeof(uboot_image_header_t));
+
+    if (size < 0) {
+        return -1;
+    }
+    bswap_uboot_header(hdr);
+    if (hdr->ih_magic != IH_MAGIC) {
+        return -1;
+    }
+    return 0;
+}
+
 /* Load a U-Boot image.  */
 static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
                             int *is_linux, uint8_t image_type)
 {
     int fd;
-    int size;
     hwaddr address;
     uboot_image_header_t h;
     uboot_image_header_t *hdr = &h;
@@ -471,14 +484,9 @@ static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
     if (fd < 0)
         return -1;
 
-    size = read(fd, hdr, sizeof(uboot_image_header_t));
-    if (size < 0)
-        goto out;
-
-    bswap_uboot_header(hdr);
-
-    if (hdr->ih_magic != IH_MAGIC)
+    if (load_uboot_image_header_fd(fd, hdr) < 0) {
         goto out;
+    }
 
     if (hdr->ih_type != image_type) {
         fprintf(stderr, "Wrong image type %d, expected %d\n", hdr->ih_type,
@@ -565,6 +573,19 @@ out:
     return ret;
 }
 
+int load_uboot_image_header(const char *filename, uboot_image_header_t *hdr)
+{
+    int rc;
+    int fd = open(filename, O_RDONLY | O_BINARY);
+
+    if (fd < 0) {
+        return -1;
+    }
+    rc = load_uboot_image_header_fd(fd, hdr);
+    close(fd);
+    return rc;
+}
+
 int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
                 int *is_linux)
 {
diff --git a/include/hw/loader.h b/include/hw/loader.h
index 796cbf9..a8cd0cf 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -1,6 +1,7 @@
 #ifndef LOADER_H
 #define LOADER_H
 #include "qapi/qmp/qdict.h"
+#include "hw/core/uboot_image.h"
 #include "hw/nvram/fw_cfg.h"
 
 /* loader.c */
@@ -27,6 +28,7 @@ int load_elf(const char *filename, uint64_t (*translate_fn)(void *, uint64_t),
              int clear_lsb);
 int load_aout(const char *filename, hwaddr addr, int max_sz,
               int bswap_needed, hwaddr target_page_size);
+int load_uboot_image_header(const char *filename, uboot_image_header_t *hdr);
 int load_uimage(const char *filename, hwaddr *ep,
                 hwaddr *loadaddr, int *is_linux);
 
-- 
1.8.1.4

[Qemu-devel] [PATCH 2/3] hw/core/loader: implement load_uimage_at

[Max Filippov]

load_uimage_at loads kernel image at the specified address instead of
the address recorded in the uImage header.

Cc: qemu-stable@nongnu.org
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/core/loader.c    | 25 +++++++++++++++++++------
 include/hw/loader.h |  2 ++
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/hw/core/loader.c b/hw/core/loader.c
index 5bf7f9b..beb7c2b 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -470,7 +470,7 @@ static int load_uboot_image_header_fd(int fd, uboot_image_header_t *hdr)
 
 /* Load a U-Boot image.  */
 static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
-                            int *is_linux, uint8_t image_type)
+                            bool force_addr, int *is_linux, uint8_t image_type)
 {
     int fd;
     hwaddr address;
@@ -497,9 +497,13 @@ static int load_uboot_image(const char *filename, hwaddr *ep, hwaddr *loadaddr,
     /* TODO: Implement other image types.  */
     switch (hdr->ih_type) {
     case IH_TYPE_KERNEL:
-        address = hdr->ih_load;
-        if (loadaddr) {
-            *loadaddr = hdr->ih_load;
+        if (force_addr) {
+            address = *loadaddr;
+        } else {
+            address = hdr->ih_load;
+            if (loadaddr) {
+                *loadaddr = hdr->ih_load;
+            }
         }
 
         switch (hdr->ih_comp) {
@@ -589,13 +593,22 @@ int load_uboot_image_header(const char *filename, uboot_image_header_t *hdr)
 int load_uimage(const char *filename, hwaddr *ep, hwaddr *loadaddr,
                 int *is_linux)
 {
-    return load_uboot_image(filename, ep, loadaddr, is_linux, IH_TYPE_KERNEL);
+    return load_uboot_image(filename, ep, loadaddr, false, is_linux,
+                            IH_TYPE_KERNEL);
+}
+
+int load_uimage_at(const char *filename, hwaddr *ep, hwaddr loadaddr,
+                   int *is_linux)
+{
+    return load_uboot_image(filename, ep, &loadaddr, true, is_linux,
+                            IH_TYPE_KERNEL);
 }
 
 /* Load a ramdisk.  */
 int load_ramdisk(const char *filename, hwaddr addr, uint64_t max_sz)
 {
-    return load_uboot_image(filename, NULL, &addr, NULL, IH_TYPE_RAMDISK);
+    return load_uboot_image(filename, NULL, &addr, false, NULL,
+                            IH_TYPE_RAMDISK);
 }
 
 /*
diff --git a/include/hw/loader.h b/include/hw/loader.h
index a8cd0cf..76f068d 100644
--- a/include/hw/loader.h
+++ b/include/hw/loader.h
@@ -31,6 +31,8 @@ int load_aout(const char *filename, hwaddr addr, int max_sz,
 int load_uboot_image_header(const char *filename, uboot_image_header_t *hdr);
 int load_uimage(const char *filename, hwaddr *ep,
                 hwaddr *loadaddr, int *is_linux);
+int load_uimage_at(const char *filename, hwaddr *ep,
+                   hwaddr loadaddr, int *is_linux);
 
 /**
  * load_ramdisk:
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 2/3] hw/core/loader: implement load_uimage_at

[Alexander Graf]

On 12.08.14 06:22, Max Filippov wrote:
> load_uimage_at loads kernel image at the specified address instead of
> the address recorded in the uImage header.
> 
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>

Please check out load_elf() and friends. The way we usually handle these
things is by providing a special translate_fn function callback that
adds the offset.


Alex

Re: [Qemu-devel] [PATCH 2/3] hw/core/loader: implement load_uimage_at

[Max Filippov]

Hi Alex,

On Thu, Aug 21, 2014 at 1:16 PM, Alexander Graf <agraf@suse.de> wrote:
> On 12.08.14 06:22, Max Filippov wrote:
>> load_uimage_at loads kernel image at the specified address instead of
>> the address recorded in the uImage header.
>>
>> Cc: qemu-stable@nongnu.org
>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>
> Please check out load_elf() and friends. The way we usually handle these
> things is by providing a special translate_fn function callback that
> adds the offset.

Thanks for your comment. Nobody seems to need this functionality except
xtensa, so I tried to solve it the least intrusive way. But sure, I can add
generic translation support to uimage loader.

-- 
Thanks.
-- Max

Re: [Qemu-devel] [PATCH 2/3] hw/core/loader: implement load_uimage_at

[Alexander Graf]

> Am 21.08.2014 um 22:56 schrieb Max Filippov <jcmvbkbc@gmail.com>:
> 
> Hi Alex,
> 
>> On Thu, Aug 21, 2014 at 1:16 PM, Alexander Graf <agraf@suse.de> wrote:
>>> On 12.08.14 06:22, Max Filippov wrote:
>>> load_uimage_at loads kernel image at the specified address instead of
>>> the address recorded in the uImage header.
>>> 
>>> Cc: qemu-stable@nongnu.org
>>> Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
>> 
>> Please check out load_elf() and friends. The way we usually handle these
>> things is by providing a special translate_fn function callback that
>> adds the offset.
> 
> Thanks for your comment. Nobody seems to need this functionality except
> xtensa, so I tried to solve it the least intrusive way. But sure, I can add
> generic translation support to uimage loader.

I think I personally would prefer to have the interfaces be as consistent as possible, yeah :).

Alex

> 
> -- 
> Thanks.
> -- Max

[Qemu-devel] [PATCH 3/3] target-xtensa: treat uImage load address as virtual

[Max Filippov]

U-boot for xtensa always treats uImage load address as virtual address.
This is important when booting uImage on xtensa core with MMUv2, because
MMUv2 has fixed non-identity virtual-to-physical mapping after reset.

Always do virtual-to-physical translation of uImage load address and
load uImage at the translated address. This fixes booting uImage kernels
on dc232b and other MMUv2 cores.

Cc: qemu-stable@nongnu.org
Reported-by: Waldemar Brodkorb <mail@waldemar-brodkorb.de>
Signed-off-by: Max Filippov <jcmvbkbc@gmail.com>
---
 hw/xtensa/xtfpga.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/hw/xtensa/xtfpga.c b/hw/xtensa/xtfpga.c
index a2dff5a..71be863 100644
--- a/hw/xtensa/xtfpga.c
+++ b/hw/xtensa/xtfpga.c
@@ -323,9 +323,16 @@ static void lx_init(const LxBoardDesc *board, MachineState *machine)
         if (success > 0) {
             entry_point = elf_entry;
         } else {
+            uboot_image_header_t hdr;
             hwaddr ep;
             int is_linux;
-            success = load_uimage(kernel_filename, &ep, NULL, &is_linux);
+
+            success = load_uboot_image_header(kernel_filename, &hdr);
+            if (success == 0) {
+                success = load_uimage_at(kernel_filename, &ep,
+                                         translate_phys_addr(cpu, hdr.ih_load),
+                                         &is_linux);
+            }
             if (success > 0 && is_linux) {
                 entry_point = ep;
             } else {
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH 0/3] target-xtensa: fix loading uImage kernels on MMUv2 cores

[Max Filippov]

On Tue, Aug 12, 2014 at 8:22 AM, Max Filippov <jcmvbkbc@gmail.com> wrote:
> Hi,
>
> this series fixes loading uImage kernels on MMUv2 xtensa cores.
>
> U-boot for xtensa always treats uImage load address as virtual address.
> This is important when booting uImage on xtensa core with MMUv2, because
> MMUv2 has fixed non-identity virtual-to-physical mapping after reset.
>
> I add two new functions: load_uboot_image_header that loads uImage header
> and load_uimage_at that loads uImage at the specified address, and use them
> to query uImage load address and load uImage at the correctly translated
> address.
>
> Max Filippov (3):
>   hw/core/loader: implement load_uboot_image_header
>   hw/core/loader: implement load_uimage_at
>   target-xtensa: treat uImage load address as virtual
>
>  hw/core/loader.c    | 62 +++++++++++++++++++++++++++++++++++++++++------------
>  hw/xtensa/xtfpga.c  |  9 +++++++-
>  include/hw/loader.h |  4 ++++
>  3 files changed, 60 insertions(+), 15 deletions(-)

Michael, Alexander, Gerd, Igor, Peter or anyone interested,

could you please review the changes to the generic uimage loader code
made in this series?

-- 
Thanks.
-- Max