Re: [PATCH 0/3] u-boot chain-loading LineageOS bootimg

[Mattijs Korpershoek <mkorpershoek@redhat.com>]

Hi George,

On mer., avril 30, 2025 at 11:58, george chan <gchan9527@gmail.com> wrote:

> Hi Mattijs,
> CC: Casey,
>
> Thx for your reply.
>
>
> 在 2025年4月29日週二 16:30，Mattijs Korpershoek <mkorpershoek@kernel.org> 寫道：
>
>> Hi George,
>>
>> Thank you for contributing.
>>
>> On lun., avril 28, 2025 at 15:53, Casey Connolly <
>> casey.connolly@linaro.org> wrote:
>>
>> > Hi George,
>> >
>> > Thanks a lot for the series, it's super exciting to see support for
>> > booting Android on top of U-Boot :D
>> >
>> > On 4/27/25 13:25, George Chan via B4 Relay wrote:
>> >> This is a series of patches to enable chainloading LineageOS on qcom
>> SOC.
>> >>
>> >> First patch is to workaround kernel/ramdisk invalid addr by identify
>> >> its physical memory address out-of-range. Since qcom SOC usually have
>> >> 0x80000000 as start/base/real memory address but androidboot img
>> >> specified to around 0x0. If other vendor bootloader behave similar then
>> >> this patch can also workaround it as well.
>> >
>> > I'm curious to know what values are there, tbh I think U-Boot should
>> > entirely ignore the values in the boot image but I guess that could
>> > break some other platforms.
>>
>> Yes, some platforms actually use the values from the boot.img.
>> We had some breakage on Khadas VIM3 in this area. See:
>>
>> https://lore.kernel.org/all/20241003-android-fix-boot-v2-v1-1-13e8e44af4b7@baylibre.com/
>>
>> The above thread also mentions how to repack a boot.img, where we can
>> specify a different ramdisk address.
>>
>> It's a bit unclear to me why it's impractical to repack the boot.img and
>> specify the appropriate address. Could you elaborate ?
>
>
> It is hard to ask less experience people to modify the boot img....I do
> prefer no mod is the best. Why get life so complicated, my friend?

Sorry, In my opinion this is not a valid argument. Why is patching the
bootloader more complicated than repacking the boot.img ?

Can you maybe illustrate with a detailed example?

The offsets can also be configured via the Android build system directly
by modifying BOARD_MKBOOTIMG_ARGS, see:
https://cs.android.com/android/platform/superproject/main/+/main:device/amlogic/yukawa/BoardConfig.mk;l=114?q=BOARD_MKBOOTIMG_ARGS&start=1
https://cs.android.com/android/platform/superproject/main/+/main:device/linaro/dragonboard/linaro_swr/BoardConfig.mk;l=17?q=BOARD_MKBOOTIMG_ARGS&start=21

>
>
>> >
>> > How about adding a config option CONFIG_ANDROID_BOOTIMG_IGNORE_ADDRS and
>> > just ORing it in all the places that have checks like
>> > "img_data->kernel_addr == 0". Then you can enable this for
>> > mach-snapdragon by selecting it in the ARCH_SNAPDRAGON config definition.
>>
>> If we need to add this in U-Boot, I'd prefer this option as suggested by
>> Casey.
>>
>
> No problem. If this approach have wider audience and also benefit other
> soc/device, I am glad to know. Lets wait for user of below code and see if
> any willingness to migrate.
>
> https://github.com/u-boot/u-boot/blob/d75998b476de439a05b2f7ec95d426410bcaae18/boot/image-android.c#L271
>
> And
>
> https://github.com/u-boot/u-boot/blob/d75998b476de439a05b2f7ec95d426410bcaae18/boot/image-android.c#L281
>
>
>> >
>> >>
>> >> Second patch is enable bootmeth-android to have chance for extra mem
>> block.
>> >> Usually the fastboot mem block, to hold androidboot img, and loadaddr
>> for
>> >> unzipped kernel. If other SOC have extra mem block available but
>> >> fastboot block, we can add extra logic to take care of it.
>> >
>> > There is a small hack in mach-snapdragon that we put kernel_addr_r and
>> > loadaddr at the same address to avoid using up too much memory on
>> > devices that only have 1GB which explains this crash because we try to
>> > decompress the kernel to the same address it got loaded too.
>> >
>> > I think we can get away with giving loadaddr its own allocation of 64M
>> > or so, maybe 96. This would make your second patch redundant (and feels
>> > like the right fix to me)
>>
>> I was not aware of this hack, but I agree with Casey.
>>
>
> Hmm... I have no idea/experience about it. Casey will you follow this issue
> with a proper patch?
>
>
>> >
>> >>
>> >> Third patch is optional to enable snapdragon board to have extra cmdline
>> >> found from original bootloader that is required for LineageOS boot init.
>> >> An alternate method is to put the append string into a dummy device-tree
>> >> file, as /chosen/bootarg ofnode porperty, that also contain msm-id. Then
>> >> encapsulate as androidboot img and let u-boot as kernel binary. Below is
>> >> an example for Xiaomi Miatoll device.
>> >
>> > I assume you're chainloading U-Boot on this device? If so, isn't it
>> > enough to just copy the boot args that ABL gives us and maybe tweak a
>> few?
>> >
>> > I'd also rather keep this constrained, maybe an android specific board
>> > callback to set boot args (if there isn't one already) and have the FDT
>> > fixup code be alongside some other generic FDT fixup stuff. I'd rather
>> > not have this stuff in board specific code.
>>
>> I also agree with Casey here. Looking at patch: "[PATCH 3/3]
>> mach-snapdragon: Add support to append string to kernel cmdline", the
>> android bootmethod already takes care of some androidboot.* arguments.
>> See:
>>
>>
>> https://source.denx.de/u-boot/u-boot/-/blob/master/boot/bootmeth_android.c?ref_type=heads#L504
>>
>> For androidboot.verifiedbootstate=orange, this is done via
>> avb_append_commandline_arg() when AVB is enabled in U-Boot.
>>
>
> Reasonable. My plan is remove all default params. I still not decided to
> let kconfig do it or leave it as a new env var. And put that post-pending
> logic into fdt-support.c
>
> Thx again,
> George
>
>
>> >
>> > I hope this makes sense, feel free to ask for clarifications on anything.
>> >
>> > Thanks and kind regards,
>> >
>> >>
>> >> /dts-v1/;
>> >>
>> >> / {
>> >>          qcom,msm-id = <443 0x0>;
>> >>          qcom,board-id = <0 0>,
>> >>                      <0x10022 1>,
>> >>                      <0x20022 1>,
>> >>                      <0x30022 1>,
>> >>                      <0x40022 1>,
>> >>                      <0x50022 1>;
>> >>
>> >>          #address-cells = <2>;
>> >>          #size-cells = <2>;
>> >>
>> >>          memory {
>> >>                  ddr_device_type = <0x07>;
>> >>                  /* We expect the bootloader to fill in the size */
>> >>                  reg = <0 0 0 0>;
>> >>          };
>> >>
>> >>      chosen {
>> >>              bootargs = "";
>> >>      };
>> >> };
>> >>
>> >> Signed-off-by: George Chan <gchan9527@gmail.com>
>> >> ---
>> >> George Chan (3):
>> >>        boot/image-android.c: Workaround androidboot kernel/ramdisk addr
>> >>        boot/bootmeth-android.c: Reuse fastboot memory block for unzip
>> kernel
>> >>        mach-snapdragon: Add support to append string to kernel cmdline
>> >>
>> >>   arch/arm/mach-snapdragon/Kconfig | 11 +++++
>> >>   arch/arm/mach-snapdragon/board.c | 97
>> ++++++++++++++++++++++++++++++++++++++++
>> >>   boot/bootmeth_android.c          | 12 +++++
>> >>   boot/image-android.c             | 10 +++++
>> >>   4 files changed, 130 insertions(+)
>> >> ---
>> >> base-commit: 5a0a93a768487e55ebe50a34cc90d751bf99cc56
>> >> change-id: 20250427-android-boot-ecbb768cda72
>> >>
>> >> Best regards,
>> > --
>> > Casey (she/they)
>>