[PATCH v2 3/3] ipq40xx: pakedge_wr-1: workaround for kernel bigger than 4MiB

[Tomasz Maciej Nowak]

W dniu 10.07.2023 o 00:16, Felix Baumann pisze:
> Am 9. Juli 2023 23:26:13 MESZ schrieb Tomasz Maciej Nowak <tmn505 at gmail.com>:
>> W dniu 9.07.2023 o 12:43, Felix Baumann pisze:
>>> Am 7. Juli 2023 17:11:43 MESZ schrieb Tomasz Maciej Nowak <tmn505 at terefe.re>:
>>>> From: Tomasz Maciej Nowak <tmn505 at gmail.com>
>>>>
>>>> Alter command boot sequence in U-Boot on first boot after upgrade to
>>>> workaround the kernel size limitation. The kernel size read from flash
>>>> will be 8MiB, which should suffice for forseable future. As precaution a
>>>> warning interupting upgrade is emited, explaining what to do.
>>>>
>>>> Signed-off-by: Tomasz Maciej Nowak <tmn505 at gmail.com>
>>>> ---
>>>> .../etc/uci-defaults/05_fix-compat-version         |  3 ++-
>>>> .../base-files/lib/preinit/81_patch_uboot_env.sh   | 14 ++++++++++++++
>>>> target/linux/ipq40xx/image/generic.mk              | 10 +++++++++-
>>>> 3 files changed, 25 insertions(+), 2 deletions(-)
>>>> create mode 100644 target/linux/ipq40xx/base-files/lib/preinit/81_patch_uboot_env.sh
>>>>
>>>> diff --git a/target/linux/ipq40xx/base-files/etc/uci-defaults/05_fix-compat-version b/target/linux/ipq40xx/base-files/etc/uci-defaults/05_fix-compat-version
>>>> index c01192089712..a3f90125c6e5 100644
>>>> --- a/target/linux/ipq40xx/base-files/etc/uci-defaults/05_fix-compat-version
>>>> +++ b/target/linux/ipq40xx/base-files/etc/uci-defaults/05_fix-compat-version
>>>> @@ -4,7 +4,8 @@ case "$(board_name)" in
>>>> linksys,ea6350v3|\
>>>> linksys,ea8300|\
>>>> linksys,mr8300|\
>>>> -ezviz,cs-w3-wd1200g-eup)
>>>> +ezviz,cs-w3-wd1200g-eup|\
>>>> +pakedge,wr-1)
>>>> 	uci set system. at system[0].compat_version="2.0"
>>>> 	uci commit system
>>>> 	;;
>>>> diff --git a/target/linux/ipq40xx/base-files/lib/preinit/81_patch_uboot_env.sh b/target/linux/ipq40xx/base-files/lib/preinit/81_patch_uboot_env.sh
>>>> new file mode 100644
>>>> index 000000000000..0cc0ab0fca6f
>>>> --- /dev/null
>>>> +++ b/target/linux/ipq40xx/base-files/lib/preinit/81_patch_uboot_env.sh
>>>> @@ -0,0 +1,14 @@
>>>> +. /lib/functions.sh
>>>> +
>>>> +preinit_patch_uboot_env() {
>>>> +	case $(board_name) in
>>>> +	pakedge,wr-1)
>>>> +		if ! fw_printenv -n OpenWrt &>/dev/null; then
>>>> +			fw_setenv OpenWrt 'sf probe; sf read 0x84000000 0x180000 0x800000; bootm 0x84000000'
>>>> +			fw_setenv bootcmd "$(fw_printenv -n bootcmd | sed -e 's,bootipq,run OpenWrt,')"
>>>> +		fi
>>>> +		;;
>>>> +	esac
>>>> +}
>>>> +
>>>> +boot_hook_add preinit_main preinit_patch_uboot_env
>>>> diff --git a/target/linux/ipq40xx/image/generic.mk b/target/linux/ipq40xx/image/generic.mk
>>>> index d9d60a25ff0f..0dadae122a87 100644
>>>> --- a/target/linux/ipq40xx/image/generic.mk
>>>> +++ b/target/linux/ipq40xx/image/generic.mk
>>>> @@ -958,8 +958,16 @@ define Device/pakedge_wr-1
>>>> 	SOC := qcom-ipq4018
>>>> 	BLOCKSIZE := 64k
>>>> 	IMAGE_SIZE := 31232k
>>>> -	KERNEL_SIZE := 4096k
>>>> +	KERNEL_SIZE := 8192k
>>>> 	IMAGE/sysupgrade.bin := append-kernel | pad-to $$$$(BLOCKSIZE) | append-rootfs | pad-rootfs | append-metadata
>>>> +	DEVICE_COMPAT_VERSION := 2.0
>>>> +	DEVICE_COMPAT_MESSAGE := \n$\
>>>> +		Booting command in U-Boot needs to be changed because of kernel \n$\
>>>> +		growing beyond 4MiB. If OpenWrt 23.05.0 or later is running on \n$\
>>>> +		on the device, it has already been done and You can safely force \n$\
>>>> +		upgrade. If that's not the case, first upgrade to 23.05.0 and it \n$\
>>>> +		will automatically amend booting command in U-Boot on first boot. \n$\
>>>> +		Then proceed with the upgrade to desired version.
>>>> endef
>>>> TARGET_DEVICES += pakedge_wr-1
>>>>
>>>
>>> This is one way of fixing this. But we could also switch all of these from FitImage to FitzImage. (self-decompressing xz) 
>>> That should suffice for quite a while (kernel was 1/4 smaller for my device afterwards). 3MiB in use leaves us quite some headroom.
>>
>> This device uses FitImageLZMA, so it's not affected, yet.
>>
>>> Headroom to implement lzma-loader at some point.
>>
>> That's one way to remedy the problem. The second option would be writing extension
>> for mtd to update Qualcomm partition table in MIBIB partition with new partition
>> sizes, similar to devices with RedBoot bootloader. Unfortunately all of devices
>> have MIBIB partition marked as read-only so that would be a hassle to introduce.
>> BTW The Qualcomm SMEM flash partition parser is already in kernel tree[1].
>> Both of options are outside of my capabilities and I'm not inclined to wait for
>> someone implementing those. Waiting brings only bigger pain later, since usually
>> it gets forgotten and it is reminded when device breaks down. 
>>
>>> What for? To prevent compat increases. Asking people to modify their bootloader environment can work but it's a hassle. Also you have to modify lots of wiki entries so the installation instructions all mention this.
>>
>> User does not have to modify the bootloader environment, since that is done by
>> first boot automatically after flashing new firmware. Everything needed is
>> explained in the message when one wants to perform sysupgrade. The only thing user
>> needs to do, is flash device with firmware version mentioned in the message first.
>> Nothing too complicated I would say.
>>
>> 1. https://www.kernel.org/doc/Documentation/devicetree/bindings/mtd/partitions/qcom%2Csmem-part.yaml
>>
>>> Regards
>>> Felix Baumann / Djfe
>>
>> Regards
>>
> 
> Sounds to me like a chicken and egg problem.
> If it doesn't boot, then it won't be able to change the bootloader variable (bootcmd) after a factory install.
> You would have to do factory installs with older openwrt releases.

That's not a problem here, there's no factory image for this device. The initial
installation should be done with a help of initramfs. Even single boot of
initramfs will change the environment, since it's not inhibited there. Even if
someone would use an older initramfs image, the compat message will forbid
accidental usage of improper image version for sysupgrade.

In that regard, maybe I should inhibit the environment change when simply booting
initramfs image and add it to sysupgrade platform code, so booting it won't change
environment without user action.

> smem looks interesting, thx for mentioning it.
> 
> Since you are setting a KERNEL_SIZE, we won't have anyone bricking their device anymore. Your workaround could still be implemented when builds error. Devices would literally stop building when a new kernel is reintroduced after branching so we probably have half a year to find a fix before the next release then.

That's fine, but I don't want to wait for that. Problem is that if the issue
appears between stable version (not the RC phase), there's difficulty pointing
to the right one, and that will really need manual user intervention, which could
go awry. Also I would rather support device continuously, instead of introducing
holes in between (an example being Linksys WRT devices), explaining when it works
and when it doesn't.
If the fix for the problem lands in tree, I'll happily convert it.

> But in the end maintainers will have to decide here :)

That's what I'm expecting :)

> I just wanted to represent the opposite opinion in this discussion.
> 
> We have some of these devices out in the field (Freifunk, OpenWrt fork called Gluon). Since people install and deploy these routers (just) once we kind of rely on a working sysupgrade which a major compat increase breaks.
> Else we end up with certain devices being stuck on OpenWrt 2022.

I would assume there's some kind of newsletter with important announcements, which
users are subscribed to, which explains some issues with major version bump.
Is that not a thing? Anyway, this commit is a workaround for rather unpopular
device, which I don't think has any users in Germany.

> It's nothing dramatic but an argument why I want to avoid/delay compat increases :)
> In the end we can still patch this out for ourselves if necessary.
> 
> 
> Regards
> Felix Baumann

Regards

-- 
TMN