Hi Mattijs, CC: Casey, Thx for your reply.
在 2025年4月29日週二 16:30,Mattijs Korpershoek <mkorpersh...@kernel.org> 寫道: > Hi George, > > Thank you for contributing. > > On lun., avril 28, 2025 at 15:53, Casey Connolly < > casey.conno...@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-13e8e44af...@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? > > > > 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 <gchan9...@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) >