Skip to contentSkip to conversation

target-xtensa: fix loading uImage kernels on MMUv2 cores

PATCH Superseded Max Filippov 2 participants 8 messages Aug 12, 2014 – Aug 21, 2014

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

Max Filippov

author
#0 lore raw

cover target-xtensa: fix loading uImage kernels on MMUv2 cores

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(-)
Signature · 1 line
1.8.1.4
author
#1 lore raw

PATCH 1/3 hw/core/loader: implement load_uboot_image_header

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

Cc [email protected]
Signed-off-by Max Filippov <[email protected]>
---
 hw/core/loader.c    | 37 +++++++++++++++++++++++++++++--------
 include/hw/loader.h |  2 ++
 2 files changed, 31 insertions(+), 8 deletions(-)
hw/core/loader.c +29 -8
index 2bf6b8f..5bf7f9b 100644
@@ -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)
 {
include/hw/loader.h +2 -0
index 796cbf9..a8cd0cf 100644
@@ -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);
Signature · 1 line
1.8.1.4
author
#2 lore raw

PATCH 2/3 hw/core/loader: implement load_uimage_at

In reply to Max Filippov

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

Cc [email protected]
Signed-off-by Max Filippov <[email protected]>
---
 hw/core/loader.c    | 25 +++++++++++++++++++------
 include/hw/loader.h |  2 ++
 2 files changed, 21 insertions(+), 6 deletions(-)
hw/core/loader.c +19 -6
index 5bf7f9b..beb7c2b 100644
@@ -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);
 }
 
 /*
include/hw/loader.h +2 -0
index a8cd0cf..76f068d 100644
@@ -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:
Signature · 1 line
1.8.1.4
#3 lore raw

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

Quoted 5 lines · Max Filippov

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

Cc [email protected]
Signed-off-by Max Filippov <[email protected]>

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

author
#4 lore raw

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

Hi Alex,

Quoted 10 lines · Alexander Graf
Quoted 5 lines · Max Filippov

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

Cc [email protected]
Signed-off-by Max Filippov <[email protected]>

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.

Signature · 2 lines
Thanks.
-- Max
#5 lore raw

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

Quoted 14 lines

Am 21.08.2014 um 22:56 schrieb Max Filippov <[email protected]>:

Hi Alex,

Quoted 8 lines · Alexander Graf
Quoted 6 lines · Max Filippov

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

Cc [email protected]
Signed-off-by Max Filippov <[email protected]>
Quoted 7 lines
Quoted 3 lines

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

Quoted 4 lines
Signature · 3 lines
-- 
Thanks.
-- Max
author
#6 lore raw

PATCH 3/3 target-xtensa: treat uImage load address as virtual

In reply to 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 [email protected]
Reported-by Waldemar Brodkorb <[email protected]>
Signed-off-by Max Filippov <[email protected]>
---
 hw/xtensa/xtfpga.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
hw/xtensa/xtfpga.c +8 -1
index a2dff5a..71be863 100644
@@ -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 {
Signature · 1 line
1.8.1.4
author
#7 lore raw
In reply to Max Filippov
Quoted 18 lines · 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

Quoted 4 lines
 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?

Signature · 2 lines
Thanks.
-- Max