Hi Simon,

On Mon, Oct 12, 2026 at 4:04 PM Simon Glass <[email protected]> wrote:
>
> Hi Igor,
>
> On 2026-10-11T13:10:26, Igor Opaniuk <[email protected]> wrote:
> > bootm: set os.arch for Android boot images
> >
> > bootm_find_os() leaves images.os.arch at IH_ARCH_INVALID for Android boot
> > images, so the arm64 Image relocation in bootm_load_os() (booti_setup()) is
> > skipped. An uncompressed kernel from a v3/v4 boot image is then executed in
> > place at boot.img + page size, which is not 2 MiB aligned. The kernel
> > reports "Kernel image misaligned at boot, please fix your bootloader!" and
> > dies early (observed as a stack overflow in start_kernel() with a GKI 6.12
> > kernel on QEMU virt).
> >
> > Mark the kernel with the default architecture, like the legacy and FIT
> > paths effectively do, so booti_setup() moves it to a properly aligned
> > address.
>
> The legacy and FIT paths read the arch from the image header rather
> than using a default. Please can you say that the Android header has
> no arch field, so IH_ARCH_DEFAULT is the best we can do? Also, the
> in-place case is not specific to v3/v4:
> android_image_get_kernel_addr() returns kernel_ptr for any
> uncompressed kernel with the default kernel address, or with
> ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR set.

the Android boot image header carries no explicit architecture field, so
IH_ARCH_DEFAULT is the only value avaliable, and the in-place execution
happens for any uncompressed kernel whose header holds the
default kernel address (or with ANDROID_BOOT_IMAGE_IGNORE_BLOB_ADDR),
whatever the header version. The v3/v4 mention came from my test setup, where
the kernel sits at boot.img + 4096 and nothing else can move it.
I can reword the commit message and add this information for v2.

>
> >
> > Signed-off-by: Igor Opaniuk <[email protected]>
> >
> > boot/bootm.c | 1 +
> >  1 file changed, 1 insertion(+)
>
> > diff --git a/boot/bootm.c b/boot/bootm.c
> > @@ -432,6 +432,7 @@ static int bootm_find_os(const char *cmd_name, const 
> > char *addr_fit)
> > +             images.os.arch = IH_ARCH_DEFAULT;
>
> Just to check, have you looked beyond arm64? Perhaps it would be safer
> to limit this to arm64 with IS_ENABLED(CONFIG_ARM64), or explain in
> the commit message why it is safe. What do you think?

I went through the readers of images.os.arch field after bootm_find_os():
* bootm_load_os(): the booti relocation, only for IH_ARCH_ARM64, and
  booti_setup() checks the Image magic first, so anything that is not an
  arm64 Image fails cleanly instead of being moved.
* arch/arm/lib/bootm.c: the AArch32-kernel-on-AArch64 switch only
  triggers for IH_ARCH_ARM, so an Android image keeps taking the
  AArch64 path exactly as with IH_ARCH_INVALID today. The other reader
  is update_os_arch_secondary_cores(): the Layerscape implementation
  marks the spin table entries OS_ARCH_SAME when os_arch equals
  IH_ARCH_DEFAULT and OS_ARCH_DIFF otherwise, so with
  IH_ARCH_INVALID an Android boot currently releases the secondary
  cores as if the kernel were a different architecture. That is a second thing
  this fixes.
* bootm.c: the setup.bin lookup for IH_ARCH_I386/X86_64, only reachable
  on an x86 U-Boot, where the Android format is not bootable through
  bootm anyway (the kernel is expected to be an Image/zImage and x86
  goes through zboot).
* ARM32 and RISC-V do not read os.arch after bootm_find_os().

So I would keep it unconditional: IH_ARCH_DEFAULT is by definition the
one architecture this U-Boot can boot, and the legacy path refuses
anything else through image_check_target_arch() anyway. I will put the
above in the commit message. If you still prefer the IS_ENABLED(ARM64)
guard I am fine with that too, it covers both the relocation and the
Layerscape case.

> BTW, for a v0-v2 image whose ramdisk is used in place, booti_setup()
> can now move the kernel forward by up to 2 MiB. The memmove() copies
> image_size bytes, which includes BSS, so it overwrites the start of
> the ramdisk that follows the kernel in boot.img. Executing in place
> already has a similar problem, since the kernel clears its BSS early,
> so this is not a regression, but it may be worth a note. The following
> LMB reservation also still covers the old load range rather than the
> relocated one, but that already happens with FIT.

Good point, I will add a note. The proper fix for both is to stop
executing the kernel in place, i.e. load it to kernel_addr_r like the
a compressed case does, which is a separate change.

The ramdisk is relocated by the ramdisk_high logic anyway when it is in
use.

>
> Reviewed-by: Simon Glass <[email protected]>

Thanks. v2 will only change the commit message (and the guard, if you
want it), so I will keep your tag unless you object.

>
> Regards,
> Simon

Reply via email to