[U-Boot] [PATCH V4 1/2] ARM: image: Add option for ignoring ep bit 3
Add option to the booti_setup() which indicates to it that the caller requires the image to be relocated to the beginning of the RAM and that the information whether the image can be located anywhere in RAM at 2 MiB aligned boundary or not is to be ignored. This is useful ie. in case the Image is wrapped in another envelope, ie. fitImage and not relocating it but moving it would corrupt the envelope.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com --- V2: Rename ignore_ep to force_reloc V3: No change V4: - Add stdbool.h include - Switch force_reloc to bool --- arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 5 ++++- 3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..699bf44e70 100644 --- a/arch/arm/lib/image.c +++ b/arch/arm/lib/image.c @@ -26,7 +26,8 @@ struct Image_header { uint32_t res5; };
-int booti_setup(ulong image, ulong *relocated_addr, ulong *size) +int booti_setup(ulong image, ulong *relocated_addr, ulong *size, + bool force_reloc) { struct Image_header *ih; uint64_t dst; @@ -63,7 +64,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size) * 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)) + if (!force_reloc && (le64_to_cpu(ih->flags) & BIT(3))) dst = image - text_offset; else dst = gd->bd->bi_dram[0].start; diff --git a/cmd/booti.c b/cmd/booti.c index 45fbb99b68..04353b68ec 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -37,7 +37,7 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc, debug("* kernel: cmdline image address = 0x%08lx\n", ld); }
- ret = booti_setup(ld, &relocated_addr, &image_size); + ret = booti_setup(ld, &relocated_addr, &image_size, false); if (ret != 0) return 1;
diff --git a/include/image.h b/include/image.h index 95d5934344..420b8ff576 100644 --- a/include/image.h +++ b/include/image.h @@ -17,6 +17,7 @@
#include "compiler.h" #include <asm/byteorder.h> +#include <stdbool.h>
/* Define this to avoid #ifdefs later on */ struct lmb; @@ -881,9 +882,11 @@ int bootz_setup(ulong image, ulong *start, ulong *end); * @image: Address of image * @start: Returns start address of image * @size : Returns size image + * @force_reloc: Ignore image->ep field, always place image to RAM start * @return 0 if OK, 1 if the image was not recognised */ -int booti_setup(ulong image, ulong *relocated_addr, ulong *size); +int booti_setup(ulong image, ulong *relocated_addr, ulong *size, + bool force_reloc);
/*******************************************************************/ /* New uImage format specific code (prefixed with fit_) */
The ARM64 has 2 MiB alignment requirement for the kernel. When using fitImage, this requirement may by violated, the kernel will thus be executed from unaligned address and fail to boot. Do what booti does and run booti_setup() for kernel_noload images on arm64 to obtain a suitable aligned address to which the image shall be relocated.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com --- V2: Protect the ARM64 booti bit with if IS_ENABLED(CMD_BOOTI) V3: Use if() instead of #ifdef V4: Switch force_reloc to bool --- common/bootm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index e789f6818a..e517d9f118 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -202,8 +202,23 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, }
if (images.os.type == IH_TYPE_KERNEL_NOLOAD) { - images.os.load = images.os.image_start; - images.ep += images.os.load; + if (CONFIG_IS_ENABLED(CMD_BOOTI) && + images.os.arch == IH_ARCH_ARM64) { + ulong image_addr; + ulong image_size; + + ret = booti_setup(images.os.image_start, &image_addr, + &image_size, true); + if (ret != 0) + return 1; + + images.os.type = IH_TYPE_KERNEL; + images.os.load = image_addr; + images.ep = image_addr; + } else { + images.os.load = images.os.image_start; + images.ep += images.os.image_start; + } }
images.os.start = map_to_sysmem(os_hdr);
On 13 June 2018 at 14:13, Marek Vasut marek.vasut@gmail.com wrote:
The ARM64 has 2 MiB alignment requirement for the kernel. When using fitImage, this requirement may by violated, the kernel will thus be executed from unaligned address and fail to boot. Do what booti does and run booti_setup() for kernel_noload images on arm64 to obtain a suitable aligned address to which the image shall be relocated.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com
V2: Protect the ARM64 booti bit with if IS_ENABLED(CMD_BOOTI) V3: Use if() instead of #ifdef V4: Switch force_reloc to bool
common/bootm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index e789f6818a..e517d9f118 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -202,8 +202,23 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, }
if (images.os.type == IH_TYPE_KERNEL_NOLOAD) {
images.os.load = images.os.image_start;images.ep += images.os.load;
if (CONFIG_IS_ENABLED(CMD_BOOTI) &&images.os.arch == IH_ARCH_ARM64) {ulong image_addr;ulong image_size;ret = booti_setup(images.os.image_start,&image_addr,
&image_size, true);
Is it guaranteed to be conflict free (with other images) by always moving the kernel to the start of the RAM?
if (ret != 0)return 1;
Do you think it helps to add some debug/error message here as in other path that returns 1?
images.os.type = IH_TYPE_KERNEL;images.os.load = image_addr;images.ep = image_addr;} else {images.os.load = images.os.image_start;images.ep += images.os.image_start;
I know this is same the orinigal code. I'm not famaliar the history/purpse of type IH_TYPE_KERNEL_NOLOAD (compared with IH_TYPE_KERNEL), so just for my understanding, why in this case we have to increment the images.ep by images.os.load, not just set images.ep to images.os.load.
+ }
} images.os.start = map_to_sysmem(os_hdr);-- 2.17.1
On 06/13/2018 09:51 AM, Bin Chen wrote:
On 13 June 2018 at 14:13, Marek Vasut <marek.vasut@gmail.com mailto:marek.vasut@gmail.com> wrote:
The ARM64 has 2 MiB alignment requirement for the kernel. When using fitImage, this requirement may by violated, the kernel will thus be executed from unaligned address and fail to boot. Do what booti does and run booti_setup() for kernel_noload images on arm64 to obtain a suitable aligned address to which the image shall be relocated. Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com <mailto:marek.vasut%2Brenesas@gmail.com>> Cc: Bin Chen <bin.chen@linaro.org <mailto:bin.chen@linaro.org>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com <mailto:yamada.masahiro@socionext.com>> Cc: Tom Rini <trini@konsulko.com <mailto:trini@konsulko.com>> --- V2: Protect the ARM64 booti bit with if IS_ENABLED(CMD_BOOTI) V3: Use if() instead of #ifdef V4: Switch force_reloc to bool --- common/bootm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/common/bootm.c b/common/bootm.c index e789f6818a..e517d9f118 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -202,8 +202,23 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, } if (images.os.type == IH_TYPE_KERNEL_NOLOAD) { - images.os.load = images.os.image_start; - images.ep += images.os.load; + if (CONFIG_IS_ENABLED(CMD_BOOTI) && + images.os.arch == IH_ARCH_ARM64) { + ulong image_addr; + ulong image_size; + + ret = booti_setup(images.os.image_start, &image_addr, + &image_size, true);Is it guaranteed to be conflict free (with other images) by always moving the kernel to the start of the RAM?
Yes
+ if (ret != 0) + return 1;Do you think it helps to add some debug/error message here as in other path that returns 1?
No, what for ?
+ + images.os.type = IH_TYPE_KERNEL; + images.os.load = image_addr; + images.ep = image_addr; + } else { + images.os.load = images.os.image_start; + images.ep += images.os.image_start;I know this is same the orinigal code. I'm not famaliar the history/purpse of type IH_TYPE_KERNEL_NOLOAD (compared with IH_TYPE_KERNEL), so just for my understanding, why in this case we have to increment the images.ep by images.os.load, not just set images.ep to images.os.load.
The ep is incremented by the offset from the start of the image. That's also the purpose of kernel noload -- don't copy the kernel to the load address and just execute it in place. Problem with arm64 is that it does not work so because the kernel needs to be at 2 MiB offset, which is hard to achieve with fitImage. Thus we work around that by placing the kernel at the start of RAM. It's overloading the kernel noload, but at least it works.
[...]
On Wed, Jun 13, 2018 at 12:39:25PM +0200, Marek Vasut wrote:
On 06/13/2018 09:51 AM, Bin Chen wrote:
On 13 June 2018 at 14:13, Marek Vasut <marek.vasut@gmail.com mailto:marek.vasut@gmail.com> wrote:
The ARM64 has 2 MiB alignment requirement for the kernel. When using fitImage, this requirement may by violated, the kernel will thus be executed from unaligned address and fail to boot. Do what booti does and run booti_setup() for kernel_noload images on arm64 to obtain a suitable aligned address to which the image shall be relocated. Signed-off-by: Marek Vasut <marek.vasut+renesas@gmail.com <mailto:marek.vasut%2Brenesas@gmail.com>> Cc: Bin Chen <bin.chen@linaro.org <mailto:bin.chen@linaro.org>> Cc: Masahiro Yamada <yamada.masahiro@socionext.com <mailto:yamada.masahiro@socionext.com>> Cc: Tom Rini <trini@konsulko.com <mailto:trini@konsulko.com>> --- V2: Protect the ARM64 booti bit with if IS_ENABLED(CMD_BOOTI) V3: Use if() instead of #ifdef V4: Switch force_reloc to bool --- common/bootm.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/common/bootm.c b/common/bootm.c index e789f6818a..e517d9f118 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -202,8 +202,23 @@ static int bootm_find_os(cmd_tbl_t *cmdtp, int flag, int argc, } if (images.os.type == IH_TYPE_KERNEL_NOLOAD) { - images.os.load = images.os.image_start; - images.ep += images.os.load; + if (CONFIG_IS_ENABLED(CMD_BOOTI) && + images.os.arch == IH_ARCH_ARM64) { + ulong image_addr; + ulong image_size; + + ret = booti_setup(images.os.image_start, &image_addr, + &image_size, true);Is it guaranteed to be conflict free (with other images) by always moving the kernel to the start of the RAM?
Yes
+ if (ret != 0) + return 1;Do you think it helps to add some debug/error message here as in other path that returns 1?
No, what for ?
+ + images.os.type = IH_TYPE_KERNEL; + images.os.load = image_addr; + images.ep = image_addr; + } else { + images.os.load = images.os.image_start; + images.ep += images.os.image_start;I know this is same the orinigal code. I'm not famaliar the history/purpse of type IH_TYPE_KERNEL_NOLOAD (compared with IH_TYPE_KERNEL), so just for my understanding, why in this case we have to increment the images.ep by images.os.load, not just set images.ep to images.os.load.
The ep is incremented by the offset from the start of the image. That's also the purpose of kernel noload -- don't copy the kernel to the load address and just execute it in place. Problem with arm64 is that it does not work so because the kernel needs to be at 2 MiB offset, which is hard to achieve with fitImage. Thus we work around that by placing the kernel at the start of RAM. It's overloading the kernel noload, but at least it works.
And, we have also overloaded the term "noload", historically. It does mean "execute the contents where they are". But it also means (to humans) "You don't need to specify a load address, it will just work". So it is important to fix this so that a fitImage for aarch64 can work on multiple SoCs so long as of course it has the dtbs for each of those SoCs.
On Wed, Jun 13, 2018 at 06:13:33AM +0200, Marek Vasut wrote:
The ARM64 has 2 MiB alignment requirement for the kernel. When using fitImage, this requirement may by violated, the kernel will thus be executed from unaligned address and fail to boot. Do what booti does and run booti_setup() for kernel_noload images on arm64 to obtain a suitable aligned address to which the image shall be relocated.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com
Applied to u-boot/master, thanks!
On 13 June 2018 at 14:13, Marek Vasut marek.vasut@gmail.com wrote:
Add option to the booti_setup() which indicates to it that the caller requires the image to be relocated to the beginning of the RAM and that the information whether the image can be located anywhere in RAM at 2 MiB aligned boundary or not is to be ignored. This is useful ie. in case the Image is wrapped in another envelope, ie. fitImage and not relocating it but moving it would corrupt the envelope.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com
V2: Rename ignore_ep to force_reloc V3: No change V4: - Add stdbool.h include - Switch force_reloc to bool
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 5 ++++- 3 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..699bf44e70 100644 --- a/arch/arm/lib/image.c +++ b/arch/arm/lib/image.c @@ -26,7 +26,8 @@ struct Image_header { uint32_t res5; };
-int booti_setup(ulong image, ulong *relocated_addr, ulong *size) +int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
bool force_reloc){ struct Image_header *ih; uint64_t dst; @@ -63,7 +64,7 @@ int booti_setup(ulong image, ulong *relocated_addr, ulong *size) * 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))
if (!force_reloc && (le64_to_cpu(ih->flags) & BIT(3))) dst = image - text_offset; else dst = gd->bd->bi_dram[0].start;diff --git a/cmd/booti.c b/cmd/booti.c index 45fbb99b68..04353b68ec 100644 --- a/cmd/booti.c +++ b/cmd/booti.c @@ -37,7 +37,7 @@ static int booti_start(cmd_tbl_t *cmdtp, int flag, int argc, debug("* kernel: cmdline image address = 0x%08lx\n", ld); }
ret = booti_setup(ld, &relocated_addr, &image_size);
ret = booti_setup(ld, &relocated_addr, &image_size, false); if (ret != 0) return 1;diff --git a/include/image.h b/include/image.h index 95d5934344..420b8ff576 100644 --- a/include/image.h +++ b/include/image.h @@ -17,6 +17,7 @@
#include "compiler.h" #include <asm/byteorder.h> +#include <stdbool.h>
/* Define this to avoid #ifdefs later on */ struct lmb; @@ -881,9 +882,11 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
- @image: Address of image
- @start: Returns start address of image
- @size : Returns size image
*/
- @force_reloc: Ignore image->ep field, always place image to RAM start
- @return 0 if OK, 1 if the image was not recognised
-int booti_setup(ulong image, ulong *relocated_addr, ulong *size); +int booti_setup(ulong image, ulong *relocated_addr, ulong *size,
bool force_reloc);/*******************************************************************/ /* New uImage format specific code (prefixed with fit_) */
Reviewed-By: Bin Chen bin.chen@linaro.org
-- 2.17.1
On Wed, Jun 13, 2018 at 06:13:32AM +0200, Marek Vasut wrote:
Add option to the booti_setup() which indicates to it that the caller requires the image to be relocated to the beginning of the RAM and that the information whether the image can be located anywhere in RAM at 2 MiB aligned boundary or not is to be ignored. This is useful ie. in case the Image is wrapped in another envelope, ie. fitImage and not relocating it but moving it would corrupt the envelope.
Signed-off-by: Marek Vasut marek.vasut+renesas@gmail.com Cc: Bin Chen bin.chen@linaro.org Cc: Masahiro Yamada yamada.masahiro@socionext.com Cc: Tom Rini trini@konsulko.com Reviewed-By: Bin Chen bin.chen@linaro.org
Applied to u-boot/master, thanks!
participants (3)
-
Bin Chen -
Marek Vasut -
Tom Rini