[U-Boot] [PATCH 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 --- arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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, + int ignore_ep) { 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 (!ignore_ep && (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..6d449b1995 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, 0); if (ret != 0) return 1;
diff --git a/include/image.h b/include/image.h index 95d5934344..0f70f2a9d2 100644 --- a/include/image.h +++ b/include/image.h @@ -881,9 +881,11 @@ int bootz_setup(ulong image, ulong *start, ulong *end); * @image: Address of image * @start: Returns start address of image * @size : Returns size image + * @ignore_ep: 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, + int ignore_ep);
/*******************************************************************/ /* 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 --- common/bootm.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index a0ffc1cd67..5b0e755ded 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -198,8 +198,22 @@ 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; + ulong image_addr; + ulong image_size; + + if (images.os.arch == IH_ARCH_ARM64) { + ret = booti_setup(images.os.image_start, &image_addr, + &image_size, 1); + 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 Sat, Jun 02, 2018 at 11:36:01PM +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
common/bootm.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-)
diff --git a/common/bootm.c b/common/bootm.c index a0ffc1cd67..5b0e755ded 100644 --- a/common/bootm.c +++ b/common/bootm.c @@ -198,8 +198,22 @@ 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;
ulong image_addr;
ulong image_size;
if (images.os.arch == IH_ARCH_ARM64) {
ret = booti_setup(images.os.image_start, &image_addr,
&image_size, 1);
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;
}
We need to (and using if (IS_ENABLED(...)) would be good for readability) make sure this change doesn't cause size growth on all targets, only when CONFIG_CMD_BOOTI is setup, as it also won't link otherwise. Thanks!

On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
{ 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 (!ignore_ep && (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..6d449b1995 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, 0); if (ret != 0) return 1;
diff --git a/include/image.h b/include/image.h index 95d5934344..0f70f2a9d2 100644 --- a/include/image.h +++ b/include/image.h @@ -881,9 +881,11 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
- @image: Address of image
- @start: Returns start address of image
- @size : Returns size image
- @ignore_ep: Ignore image->ep field, always place image to RAM start
First, to quote the kernel doc here: " Bit 3: Kernel physical placement 0 - 2MB aligned base should be as close as possible to the base of DRAM, since memory below it is not accessible via the linear mapping 1 - 2MB aligned base may be anywhere in physical memory"
maybe call it "force_reloc_to_base" ? Or at least something about "force"

On 06/04/2018 02:59 AM, Tom Rini wrote:
On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
Which blows up a couple of things, so to keep this patch small, it's int.
{ 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 (!ignore_ep && (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..6d449b1995 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, 0); if (ret != 0) return 1;
diff --git a/include/image.h b/include/image.h index 95d5934344..0f70f2a9d2 100644 --- a/include/image.h +++ b/include/image.h @@ -881,9 +881,11 @@ int bootz_setup(ulong image, ulong *start, ulong *end);
- @image: Address of image
- @start: Returns start address of image
- @size : Returns size image
- @ignore_ep: Ignore image->ep field, always place image to RAM start
First, to quote the kernel doc here: " Bit 3: Kernel physical placement 0 - 2MB aligned base should be as close as possible to the base of DRAM, since memory below it is not accessible via the linear mapping 1 - 2MB aligned base may be anywhere in physical memory"
maybe call it "force_reloc_to_base" ? Or at least something about "force"
Hummm.

On Mon, Jun 04, 2018 at 06:27:32PM +0200, Marek Vasut wrote:
On 06/04/2018 02:59 AM, Tom Rini wrote:
On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
Which blows up a couple of things, so to keep this patch small, it's int.
Can you elaborate please? Thanks!

On 06/04/2018 08:01 PM, Tom Rini wrote:
On Mon, Jun 04, 2018 at 06:27:32PM +0200, Marek Vasut wrote:
On 06/04/2018 02:59 AM, Tom Rini wrote:
On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
Which blows up a couple of things, so to keep this patch small, it's int.
Can you elaborate please? Thanks!
Well, just try compiling it with bool, it'll choke on bool being undeclared on some boards which include the header.

On Mon, Jun 04, 2018 at 08:05:55PM +0200, Marek Vasut wrote:
On 06/04/2018 08:01 PM, Tom Rini wrote:
On Mon, Jun 04, 2018 at 06:27:32PM +0200, Marek Vasut wrote:
On 06/04/2018 02:59 AM, Tom Rini wrote:
On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
Which blows up a couple of things, so to keep this patch small, it's int.
Can you elaborate please? Thanks!
Well, just try compiling it with bool, it'll choke on bool being undeclared on some boards which include the header.
Which boards? Or rather, I bet a file or two in tools/ needs <stdbool.h> ? That said, I'm not even 100% sure on that as tools/kwbimage.c uses 'bool' and doesn't include stdbool.h.
Regardless, please fix whatever needs fixing so that we can continue to use 'bool' for true/false and not int. Thanks!

On 06/04/2018 08:16 PM, Tom Rini wrote:
On Mon, Jun 04, 2018 at 08:05:55PM +0200, Marek Vasut wrote:
On 06/04/2018 08:01 PM, Tom Rini wrote:
On Mon, Jun 04, 2018 at 06:27:32PM +0200, Marek Vasut wrote:
On 06/04/2018 02:59 AM, Tom Rini wrote:
On Sat, Jun 02, 2018 at 11:36:00PM +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
arch/arm/lib/image.c | 5 +++-- cmd/booti.c | 2 +- include/image.h | 4 +++- 3 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/arch/arm/lib/image.c b/arch/arm/lib/image.c index 1a04e2b875..08c7b8a54f 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,
int ignore_ep)
This should be 'bool' not 'int'.
Which blows up a couple of things, so to keep this patch small, it's int.
Can you elaborate please? Thanks!
Well, just try compiling it with bool, it'll choke on bool being undeclared on some boards which include the header.
Which boards? Or rather, I bet a file or two in tools/ needs <stdbool.h> ? That said, I'm not even 100% sure on that as tools/kwbimage.c uses 'bool' and doesn't include stdbool.h.
Regardless, please fix whatever needs fixing so that we can continue to use 'bool' for true/false and not int. Thanks!
This feels like an overkill to me ? But I dont think the header should include stdbool itself either.
participants (2)
-
Marek Vasut
-
Tom Rini