[U-Boot] [PATCH 0/2] Android and arm64 improvement
I rebased the two patches submitted (quite )a while ago on top of current U-boot master fb4413295c765aa8c013650984dc2d908964c81d (and there were no conflicts).
The first patch added the support of parsing the second area of Android image so that it can be used for good. On Poplar board, we are using that area for dtb. The first patch shouldn't impact any functionality since it is new.
The second patch moves the booti_setup out of booti.c. It was the result of a discusion[1] how to better support the arm64 Android image.
I have boot tested it on Poplar board[2] with additional patch[3] that is built on top of the booti. I have hacked version of using bootm but it seems quit mess up - need lots of changes in different places ("Shotgun Surgery") to handle Android specific and/or arm64 specific code. Now, I feel a separate bootandroid command might be a cleaner solution.
That being said, the second patch doesn't depend on or impact any change we might have in the future for better Android support.
[1] https://lists.denx.de/pipermail/u-boot/2017-July/297933.html [2] https://github.com/96boards-poplar/Documentation [3] https://github.com/pierrchen/u-boot/commit/8463e6177654026746161487d0f0bd899...
Bin Chen (2): parse the second area of android image move booti_setup to arch/arm/lig/image.c
arch/arm/lib/Makefile | 2 +- arch/arm/lib/image.c | 76 +++++++++++++++++++++++++++++++++++++++++ cmd/booti.c | 92 +++++++------------------------------------------- common/image-android.c | 19 +++++++++++ include/image.h | 11 ++++++ 5 files changed, 120 insertions(+), 80 deletions(-) create mode 100644 arch/arm/lib/image.c
The second area of android image was intended to put a 2nd stage bootloader but in practice were rarely used (in my knowledge).
An proposal was made to the AOSP to (re)use the second area as the dtb[1], This patch itself doesn't depend on that proposal being accepted but it won't be that helpful as well if that proposal won't be accepted. But don't do any harm as well.
[1] https://android-review.googlesource.com/#/c/417447/ Signed-off-by: Bin Chen bin.chen@linaro.org Reviewed-by: Tom Rini trini@konsulko.com --- common/image-android.c | 19 +++++++++++++++++++ include/image.h | 2 ++ 2 files changed, 21 insertions(+)
diff --git a/common/image-android.c b/common/image-android.c index e74d0aafca..5ad3a1fa38 100644 --- a/common/image-android.c +++ b/common/image-android.c @@ -146,6 +146,25 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr, return 0; }
+int android_image_get_second(const struct andr_img_hdr *hdr, + ulong *second_data, ulong *second_len) +{ + if (!hdr->second_size) { + *second_data = *second_len = 0; + return -1; + } + + *second_data = (unsigned long)hdr; + *second_data += hdr->page_size; + *second_data += ALIGN(hdr->kernel_size, hdr->page_size); + *second_data += ALIGN(hdr->ramdisk_size, hdr->page_size); + + printf("second address is 0x%lx\n",*second_data); + + *second_len = hdr->second_size; + return 0; +} + #if !defined(CONFIG_SPL_BUILD) /** * android_print_contents - prints out the contents of the Android format image diff --git a/include/image.h b/include/image.h index b2b23a96f1..c8ce4da901 100644 --- a/include/image.h +++ b/include/image.h @@ -1263,6 +1263,8 @@ int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify, ulong *os_data, ulong *os_len); int android_image_get_ramdisk(const struct andr_img_hdr *hdr, ulong *rd_data, ulong *rd_len); +int android_image_get_second(const struct andr_img_hdr *hdr, + ulong *second_data, ulong *second_len); ulong android_image_get_end(const struct andr_img_hdr *hdr); ulong android_image_get_kload(const struct andr_img_hdr *hdr); void android_print_contents(const struct andr_img_hdr *hdr);
On 01/27/2018 01:59 PM, Bin Chen wrote:
The second area of android image was intended to put a 2nd stage bootloader but in practice were rarely used (in my knowledge).
An proposal was made to the AOSP to (re)use the second area as the dtb[1], This patch itself doesn't depend on that proposal being accepted but it won't be that helpful as well if that proposal won't be accepted. But don't do any harm as well.
[1] https://android-review.googlesource.com/#/c/417447/ Signed-off-by: Bin Chen bin.chen@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
Looks good to me,
Reviewed-by: Kever Yang kever.yang@rock-chips.com
Thanks, - Kever
common/image-android.c | 19 +++++++++++++++++++ include/image.h | 2 ++ 2 files changed, 21 insertions(+)
diff --git a/common/image-android.c b/common/image-android.c index e74d0aafca..5ad3a1fa38 100644 --- a/common/image-android.c +++ b/common/image-android.c @@ -146,6 +146,25 @@ int android_image_get_ramdisk(const struct andr_img_hdr *hdr, return 0; }
+int android_image_get_second(const struct andr_img_hdr *hdr,
ulong *second_data, ulong *second_len)+{
- if (!hdr->second_size) {
*second_data = *second_len = 0;return -1;- }
- *second_data = (unsigned long)hdr;
- *second_data += hdr->page_size;
- *second_data += ALIGN(hdr->kernel_size, hdr->page_size);
- *second_data += ALIGN(hdr->ramdisk_size, hdr->page_size);
- printf("second address is 0x%lx\n",*second_data);
- *second_len = hdr->second_size;
- return 0;
+}
#if !defined(CONFIG_SPL_BUILD) /**
- android_print_contents - prints out the contents of the Android format image
diff --git a/include/image.h b/include/image.h index b2b23a96f1..c8ce4da901 100644 --- a/include/image.h +++ b/include/image.h @@ -1263,6 +1263,8 @@ int android_image_get_kernel(const struct andr_img_hdr *hdr, int verify, ulong *os_data, ulong *os_len); int android_image_get_ramdisk(const struct andr_img_hdr *hdr, ulong *rd_data, ulong *rd_len); +int android_image_get_second(const struct andr_img_hdr *hdr,
ulong *second_data, ulong *second_len);ulong android_image_get_end(const struct andr_img_hdr *hdr); ulong android_image_get_kload(const struct andr_img_hdr *hdr); void android_print_contents(const struct andr_img_hdr *hdr);
On Sat, Jan 27, 2018 at 04:59:08PM +1100, Bin Chen wrote:
The second area of android image was intended to put a 2nd stage bootloader but in practice were rarely used (in my knowledge).
An proposal was made to the AOSP to (re)use the second area as the dtb[1], This patch itself doesn't depend on that proposal being accepted but it won't be that helpful as well if that proposal won't be accepted. But don't do any harm as well.
[1] https://android-review.googlesource.com/#/c/417447/ Signed-off-by: Bin Chen bin.chen@linaro.org Reviewed-by: Tom Rini trini@konsulko.com Reviewed-by: Kever Yang kever.yang@rock-chips.com
Applied to u-boot/master, thanks!
Follow bootz's pattern by moving the booti_setup to arch/arm/lib. This allows to use booti_setup in other paths, e.g booting an Android image containing 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).
v2: - fix review comments for indentation - fix the comment for booti_setup - rebase to lastest u-boot - improve the commit comment
Signed-off-by: Bin Chen bin.chen@linaro.org Reviewed-by: Tom Rini trini@konsulko.com --- arch/arm/lib/Makefile | 2 +- arch/arm/lib/image.c | 76 ++++++++++++++++++++++++++++++++++++++++++ cmd/booti.c | 92 ++++++++------------------------------------------- include/image.h | 9 +++++ 4 files changed, 99 insertions(+), 80 deletions(-) create mode 100644 arch/arm/lib/image.c
diff --git a/arch/arm/lib/Makefile b/arch/arm/lib/Makefile index 876024fc15..b5ffffd4e3 100644 --- a/arch/arm/lib/Makefile +++ b/arch/arm/lib/Makefile @@ -32,7 +32,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 0000000000..460f8eb0e9 --- /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 da6fb01c11..0be786cb6b 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 c8ce4da901..325b014754 100644 --- a/include/image.h +++ b/include/image.h @@ -871,6 +871,15 @@ 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. + * + * @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_) */
On Sat, Jan 27, 2018 at 04:59:09PM +1100, Bin Chen wrote:
Follow bootz's pattern by moving the booti_setup to arch/arm/lib. This allows to use booti_setup in other paths, e.g booting an Android image containing 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).
v2:
- fix review comments for indentation
- fix the comment for booti_setup
- rebase to lastest u-boot
- improve the commit comment
Signed-off-by: Bin Chen bin.chen@linaro.org Reviewed-by: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
Hello Tom,
Any feedback on this?
On 27 January 2018 at 16:59, Bin Chen bin.chen@linaro.org wrote:
I rebased the two patches submitted (quite )a while ago on top of current U-boot master fb4413295c765aa8c013650984dc2d908964c81d (and there were no conflicts).
The first patch added the support of parsing the second area of Android image so that it can be used for good. On Poplar board, we are using that area for dtb. The first patch shouldn't impact any functionality since it is new.
The second patch moves the booti_setup out of booti.c. It was the result of a discusion[1] how to better support the arm64 Android image.
I have boot tested it on Poplar board[2] with additional patch[3] that is built on top of the booti. I have hacked version of using bootm but it seems quit mess up - need lots of changes in different places ("Shotgun Surgery") to handle Android specific and/or arm64 specific code. Now, I feel a separate bootandroid command might be a cleaner solution.
That being said, the second patch doesn't depend on or impact any change we might have in the future for better Android support.
[1] https://lists.denx.de/pipermail/u-boot/2017-July/297933.html [2] https://github.com/96boards-poplar/Documentation [3] https://github.com/pierrchen/u-boot/commit/ 8463e6177654026746161487d0f0bd8998bb7a5b
Bin Chen (2): parse the second area of android image move booti_setup to arch/arm/lig/image.c
arch/arm/lib/Makefile | 2 +- arch/arm/lib/image.c | 76 +++++++++++++++++++++++++++++++++++++++++ cmd/booti.c | 92 +++++++-----------------------
common/image-android.c | 19 +++++++++++ include/image.h | 11 ++++++ 5 files changed, 120 insertions(+), 80 deletions(-) create mode 100644 arch/arm/lib/image.c
-- 2.15.0
On Tue, Feb 06, 2018 at 10:31:10AM +1100, Bin Chen wrote:
Hello Tom,
Any feedback on this?
It's on my list to grab soon, thanks!
On 27 January 2018 at 16:59, Bin Chen bin.chen@linaro.org wrote:
I rebased the two patches submitted (quite )a while ago on top of current U-boot master fb4413295c765aa8c013650984dc2d908964c81d (and there were no conflicts).
The first patch added the support of parsing the second area of Android image so that it can be used for good. On Poplar board, we are using that area for dtb. The first patch shouldn't impact any functionality since it is new.
The second patch moves the booti_setup out of booti.c. It was the result of a discusion[1] how to better support the arm64 Android image.
I have boot tested it on Poplar board[2] with additional patch[3] that is built on top of the booti. I have hacked version of using bootm but it seems quit mess up - need lots of changes in different places ("Shotgun Surgery") to handle Android specific and/or arm64 specific code. Now, I feel a separate bootandroid command might be a cleaner solution.
That being said, the second patch doesn't depend on or impact any change we might have in the future for better Android support.
[1] https://lists.denx.de/pipermail/u-boot/2017-July/297933.html [2] https://github.com/96boards-poplar/Documentation [3] https://github.com/pierrchen/u-boot/commit/ 8463e6177654026746161487d0f0bd8998bb7a5b
Bin Chen (2): parse the second area of android image move booti_setup to arch/arm/lig/image.c
arch/arm/lib/Makefile | 2 +- arch/arm/lib/image.c | 76 +++++++++++++++++++++++++++++++++++++++++ cmd/booti.c | 92 +++++++-----------------------
common/image-android.c | 19 +++++++++++ include/image.h | 11 ++++++ 5 files changed, 120 insertions(+), 80 deletions(-) create mode 100644 arch/arm/lib/image.c
-- 2.15.0
-- Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
On 6 February 2018 at 13:12, Tom Rini trini@konsulko.com wrote:
On Tue, Feb 06, 2018 at 10:31:10AM +1100, Bin Chen wrote:
Hello Tom,
Any feedback on this?
It's on my list to grab soon, thanks!
Excellent! Thanks!
On 27 January 2018 at 16:59, Bin Chen bin.chen@linaro.org wrote:
I rebased the two patches submitted (quite )a while ago on top of current U-boot master fb4413295c765aa8c013650984dc2d908964c81d (and there were no conflicts).
The first patch added the support of parsing the second area of Android image so that it can be used for good. On Poplar board, we are using that area
for
dtb. The first patch shouldn't impact any functionality since it is new.
The second patch moves the booti_setup out of booti.c. It was the
result
of a discusion[1] how to better support the arm64 Android image.
I have boot tested it on Poplar board[2] with additional patch[3] that
is
built on top of the booti. I have hacked version of using bootm but it seems quit mess up - need lots of changes in different places ("Shotgun Surgery")
to
handle Android specific and/or arm64 specific code. Now, I feel a separate bootandroid command might be a cleaner solution.
That being said, the second patch doesn't depend on or impact any
change we
might have in the future for better Android support.
[1] https://lists.denx.de/pipermail/u-boot/2017-July/297933.html [2] https://github.com/96boards-poplar/Documentation [3] https://github.com/pierrchen/u-boot/commit/ 8463e6177654026746161487d0f0bd8998bb7a5b
Bin Chen (2): parse the second area of android image move booti_setup to arch/arm/lig/image.c
arch/arm/lib/Makefile | 2 +- arch/arm/lib/image.c | 76 +++++++++++++++++++++++++++++++++++++++++ cmd/booti.c | 92 +++++++-----------------------
common/image-android.c | 19 +++++++++++ include/image.h | 11 ++++++ 5 files changed, 120 insertions(+), 80 deletions(-) create mode 100644 arch/arm/lib/image.c
-- 2.15.0
-- Regards, Bin _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
-- Tom
participants (3)
-
Bin Chen -
Kever Yang -
Tom Rini