[RFC] move booti_setup to arch/arm/lig/image.c

Message ID 1501229051-31491-1-git-send-email-bin.chen@linaro.org
State New
Headers show

Commit Message

Bin Chen July 28, 2017, 8:04 a.m.
Follow bootz's pattern by moving the booti_setup to arch/arm/lib.
This allows to use that function in other path, e.g booting
an android image contains Image format.

Note that kernel relocation is move out of booti_setup and it is the
caller's responsibility to do it and allows them do it differently. say,
cmd/booti.c just do a manually, while in the bootm path, we can use
bootm_load_os(with some changes).

Signed-off-by: Bin Chen <bin.chen@linaro.org>
---
 arch/arm/lib/Makefile |  2 +-
 arch/arm/lib/image.c  | 76 ++++++++++++++++++++++++++++++++++++++++++
 cmd/booti.c           | 92 ++++++++-------------------------------------------
 include/image.h       | 10 ++++++
 4 files changed, 100 insertions(+), 80 deletions(-)
 create mode 100644 arch/arm/lib/image.c

Comments

Tom Rini Aug. 7, 2017, 4:13 p.m. | #1
On Fri, Jul 28, 2017 at 06:04:11PM +1000, Bin Chen wrote:

> Follow bootz's pattern by moving the booti_setup to arch/arm/lib.

> This allows to use that function in other path, e.g booting

> an android image contains Image format.

> 

> Note that kernel relocation is move out of booti_setup and it is the

> caller's responsibility to do it and allows them do it differently. say,

> cmd/booti.c just do a manually, while in the bootm path, we can use

> bootm_load_os(with some changes).


Just to be clear, did you boot test this path on hardware?

And, a minor comment:
> -	ih = (struct Image_header *)map_sysmem(images->ep, 0);

> -

> -	lmb_reserve(&images->lmb, images->ep, le32_to_cpu(ih->image_size));

> +	/* Handle BOOTM_STATE_LOADOS */

> +	if (relocated_addr != ld) {

> +        debug("Moving Image from 0x%lx to 0x%lx\n", ld, relocated_addr);

> +        memmove((void *)relocated_addr, (void *)ld, image_size);

> +    }


Please correct the indentation here.

Otherwise, and assuming you've booted the kernel with this patch, fix
the above, post as v2 instead and:

Reviewed-by: Tom Rini <trini@konsulko.com>


Thanks!

-- 
Tom
Bin Chen Aug. 8, 2017, 2:29 a.m. | #2
On 8 August 2017 at 02:13, Tom Rini <trini@konsulko.com> wrote:

> On Fri, Jul 28, 2017 at 06:04:11PM +1000, Bin Chen wrote:
>
> > Follow bootz's pattern by moving the booti_setup to arch/arm/lib.
> > This allows to use that function in other path, e.g booting
> > an android image contains Image format.
> >
> > Note that kernel relocation is move out of booti_setup and it is the
> > caller's responsibility to do it and allows them do it differently. say,
> > cmd/booti.c just do a manually, while in the bootm path, we can use
> > bootm_load_os(with some changes).
>
> Just to be clear, did you boot test this path on hardware?
>

yes, I tested the booti path with this patch applied.

>
> And, a minor comment:
> > -     ih = (struct Image_header *)map_sysmem(images->ep, 0);
> > -
> > -     lmb_reserve(&images->lmb, images->ep, le32_to_cpu(ih->image_size));
> > +     /* Handle BOOTM_STATE_LOADOS */
> > +     if (relocated_addr != ld) {
> > +        debug("Moving Image from 0x%lx to 0x%lx\n", ld, relocated_addr);
> > +        memmove((void *)relocated_addr, (void *)ld, image_size);
> > +    }
>
> Please correct the indentation here.
>
OK. Thanks for the review!

>
> Otherwise, and assuming you've booted the kernel with this patch, fix
> the above, post as v2 instead and:
>
> Reviewed-by: Tom Rini <trini@konsulko.com>
>
> Thanks!
>
> --
> Tom
>

Patch

diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile
index 6e96cfb..8122788 100644
--- a/arch/arm/lib/Makefile
+++ b/arch/arm/lib/Makefile
@@ -26,7 +26,7 @@  endif
 
 obj-$(CONFIG_CPU_V7M) += cmd_boot.o
 obj-$(CONFIG_OF_LIBFDT) += bootm-fdt.o
-obj-$(CONFIG_CMD_BOOTI) += bootm.o
+obj-$(CONFIG_CMD_BOOTI) += bootm.o image.o
 obj-$(CONFIG_CMD_BOOTM) += bootm.o
 obj-$(CONFIG_CMD_BOOTZ) += bootm.o zimage.o
 obj-$(CONFIG_SYS_L2_PL310) += cache-pl310.o
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c
new file mode 100644
index 0000000..460f8eb
--- /dev/null
+++ b/arch/arm/lib/image.c
@@ -0,0 +1,76 @@ 
+/*
+ * (C) Copyright 2000-2009
+ * Wolfgang Denk, DENX Software Engineering, wd@denx.de.
+ *
+ * SPDX-License-Identifier: GPL-2.0+
+ */
+
+#include <common.h>
+#include <mapmem.h>
+
+DECLARE_GLOBAL_DATA_PTR;
+
+#define LINUX_ARM64_IMAGE_MAGIC 0x644d5241
+
+/* See Documentation/arm64/booting.txt in the Linux kernel */
+struct Image_header {
+    uint32_t    code0;      /* Executable code */
+    uint32_t    code1;      /* Executable code */
+    uint64_t    text_offset;    /* Image load offset, LE */
+    uint64_t    image_size; /* Effective Image size, LE */
+    uint64_t    flags;      /* Kernel flags, LE */
+    uint64_t    res2;       /* reserved */
+    uint64_t    res3;       /* reserved */
+    uint64_t    res4;       /* reserved */
+    uint32_t    magic;      /* Magic number */
+    uint32_t    res5;
+};
+
+int booti_setup(ulong image, ulong *relocated_addr, ulong *size)
+{
+    struct Image_header *ih;
+    uint64_t dst;
+    uint64_t image_size, text_offset;
+
+    *relocated_addr = image;
+
+    ih = (struct Image_header *)map_sysmem(image, 0);
+
+    if (ih->magic != le32_to_cpu(LINUX_ARM64_IMAGE_MAGIC)) {
+        puts("Bad Linux ARM64 Image magic!\n");
+        return 1;
+    }
+
+    /*
+     * Prior to Linux commit a2c1d73b94ed, the text_offset field
+     * is of unknown endianness.  In these cases, the image_size
+     * field is zero, and we can assume a fixed value of 0x80000.
+     */
+    if (ih->image_size == 0) {
+        puts("Image lacks image_size field, assuming 16MiB\n");
+        image_size = 16 << 20;
+        text_offset = 0x80000;
+    } else {
+        image_size = le64_to_cpu(ih->image_size);
+        text_offset = le64_to_cpu(ih->text_offset);
+    }
+
+    *size = image_size;
+
+    /*
+     * If bit 3 of the flags field is set, the 2MB aligned base of the
+     * kernel image can be anywhere in physical memory, so respect
+     * images->ep.  Otherwise, relocate the image to the base of RAM
+     * since memory below it is not accessible via the linear mapping.
+     */
+    if (le64_to_cpu(ih->flags) & BIT(3))
+        dst = image - text_offset;
+    else
+        dst = gd->bd->bi_dram[0].start;
+
+    *relocated_addr = ALIGN(dst, SZ_2M) + text_offset;
+
+    unmap_sysmem(ih);
+
+    return 0;
+}
diff --git a/cmd/booti.c b/cmd/booti.c
index da6fb01..91e61da 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -16,77 +16,6 @@ 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-/* See Documentation/arm64/booting.txt in the Linux kernel */
-struct Image_header {
-	uint32_t	code0;		/* Executable code */
-	uint32_t	code1;		/* Executable code */
-	uint64_t	text_offset;	/* Image load offset, LE */
-	uint64_t	image_size;	/* Effective Image size, LE */
-	uint64_t	flags;		/* Kernel flags, LE */
-	uint64_t	res2;		/* reserved */
-	uint64_t	res3;		/* reserved */
-	uint64_t	res4;		/* reserved */
-	uint32_t	magic;		/* Magic number */
-	uint32_t	res5;
-};
-
-#define LINUX_ARM64_IMAGE_MAGIC	0x644d5241
-
-static int booti_setup(bootm_headers_t *images)
-{
-	struct Image_header *ih;
-	uint64_t dst;
-	uint64_t image_size, text_offset;
-
-	ih = (struct Image_header *)map_sysmem(images->ep, 0);
-
-	if (ih->magic != le32_to_cpu(LINUX_ARM64_IMAGE_MAGIC)) {
-		puts("Bad Linux ARM64 Image magic!\n");
-		return 1;
-	}
-
-	/*
-	 * Prior to Linux commit a2c1d73b94ed, the text_offset field
-	 * is of unknown endianness.  In these cases, the image_size
-	 * field is zero, and we can assume a fixed value of 0x80000.
-	 */
-	if (ih->image_size == 0) {
-		puts("Image lacks image_size field, assuming 16MiB\n");
-		image_size = 16 << 20;
-		text_offset = 0x80000;
-	} else {
-		image_size = le64_to_cpu(ih->image_size);
-		text_offset = le64_to_cpu(ih->text_offset);
-	}
-
-	/*
-	 * If bit 3 of the flags field is set, the 2MB aligned base of the
-	 * kernel image can be anywhere in physical memory, so respect
-	 * images->ep.  Otherwise, relocate the image to the base of RAM
-	 * since memory below it is not accessible via the linear mapping.
-	 */
-	if (le64_to_cpu(ih->flags) & BIT(3))
-		dst = images->ep - text_offset;
-	else
-		dst = gd->bd->bi_dram[0].start;
-
-	dst = ALIGN(dst, SZ_2M) + text_offset;
-
-	unmap_sysmem(ih);
-
-	if (images->ep != dst) {
-		void *src;
-
-		debug("Moving Image from 0x%lx to 0x%llx\n", images->ep, dst);
-
-		src = (void *)images->ep;
-		images->ep = dst;
-		memmove((void *)dst, src, image_size);
-	}
-
-	return 0;
-}
-
 /*
  * Image booting support
  */
@@ -94,31 +23,36 @@  static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc,
 			char * const argv[], bootm_headers_t *images)
 {
 	int ret;
-	struct Image_header *ih;
+	ulong ld;
+	ulong relocated_addr;
+	ulong image_size;
 
 	ret = do_bootm_states(cmdtp, flag, argc, argv, BOOTM_STATE_START,
 			      images, 1);
 
 	/* Setup Linux kernel Image entry point */
 	if (!argc) {
-		images->ep = load_addr;
+		ld = load_addr;
 		debug("*  kernel: default image load address = 0x%08lx\n",
 				load_addr);
 	} else {
-		images->ep = simple_strtoul(argv[0], NULL, 16);
+		ld = simple_strtoul(argv[0], NULL, 16);
 		debug("*  kernel: cmdline image address = 0x%08lx\n",
 			images->ep);
 	}
 
-	ret = booti_setup(images);
+	ret = booti_setup(ld, &relocated_addr, &image_size);
 	if (ret != 0)
 		return 1;
 
-	ih = (struct Image_header *)map_sysmem(images->ep, 0);
-
-	lmb_reserve(&images->lmb, images->ep, le32_to_cpu(ih->image_size));
+	/* Handle BOOTM_STATE_LOADOS */
+	if (relocated_addr != ld) {
+        debug("Moving Image from 0x%lx to 0x%lx\n", ld, relocated_addr);
+        memmove((void *)relocated_addr, (void *)ld, image_size);
+    }
 
-	unmap_sysmem(ih);
+	images->ep = relocated_addr;
+	lmb_reserve(&images->lmb, images->ep, le32_to_cpu(image_size));
 
 	/*
 	 * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not
diff --git a/include/image.h b/include/image.h
index 8264c52..ade948e 100644
--- a/include/image.h
+++ b/include/image.h
@@ -848,6 +848,16 @@  int image_setup_linux(bootm_headers_t *images);
  */
 int bootz_setup(ulong image, ulong *start, ulong *end);
 
+/**
+ * Return the correct start address and size of a Linux aarch64 Image,
+ * also kernel relocation inside, if needed.
+ *
+ * @image: Address of image
+ * @start: Returns start address of image
+ * @size : Returns size image
+ * @return 0 if OK, 1 if the image was not recognised
+ */
+int booti_setup(ulong image, ulong *relocated_addr, ulong *size);
 
 /*******************************************************************/
 /* New uImage format specific code (prefixed with fit_) */