On 2020-08-28, Andre Heider wrote:
> On 27/08/2020 22:43, Vagrant Cascadian wrote:
>> On 2020-08-27, Andre Heider wrote:
>>> Since [0] flash-kernel does:
>>>
>>> if test -n "${console}"; then
>>> setenv bootargs "${bootargs} console=${console}"
>>> fi
>>>
>>> The common $console format as set by u-boot includes the leading "console=":
>>> include/configs/arndale.h:#define CONFIG_DEFAULT_CONSOLE
>>> "console=ttySAC2,115200n8\0"
...
>>> include/configs/trats.h: "console=" CONFIG_DEFAULT_CONSOLE \
>>> include/configs/trats2.h:#define CONFIG_DEFAULT_CONSOLE
>>> "console=ttySAC2,115200n8\0"
>>> include/configs/trats2.h: "console=" CONFIG_DEFAULT_CONSOLE \
>>>
>>> So on some boards we end up with bootargs containing
>>> "console=console=...", which, combined with a systemd bug [1] (present
>>> in buster), makes the system unbootable:
>>> [ 4.632197] systemd-udevd[90]: Starting version 241
>>> [ 4.639324] systemd-udevd[91]: Failed to create udev control event
>>> source: Operation not permitted
>>> <full hang here>
>>
>> In debian's u-boot package, this has been patched out for some targets
>> (and upstream originally included the patches, but eventually reverted).
>>
>> The problem stems from an inconsistancy in u-boot, as some platforms use
>> this argument differently(especially when you get into vendor forks of
>> u-boot), and I don't believe u-boot has pattern matching to be able to
>> handle this properly (e.g. behave differently when console=* is set).
>>
>>
>>
>>> Doing a "env delete console" and the system boots up properly. That
>>> includes the kernel output over serial, since the kernel dts files have
>>> contain "chosen { stdout-path = &uart0; };".
>>
>> Not all systems have stdout-path defined in the device-tree.
>> Maybe now those are the exceptions...
>>
>>> So I'd say appending $console to $bootargs is some historical leftover,
>>> and it can just be removed from the generic scripts.
>>
>> The problem is that *not* doing this with console can also result in an
>> unbootable system.
>
> Huh, how does that break?
Ok, strictly speaking, it may result in a system without a useable
console... not exactly broken, per se, but might result in an
inaccessible system if console access is required?
>> Reverting this would be breaking one thing to fix another. There's no
>> "correct" here, only different. :/
>
> flash-kernel supports assigning different boot scripts per boards. I
> mean, we could get rid of $console from the generic scripts, and then
> use a different one with it for boards requiring it.
True, we can find all the boards that do not set console appropriately
from device-tree and set them up with a different boot script...
> Stats:
> Going by u-boot upstream, only 17 of +700 boards set $console (grep for
> CONFIG_DEFAULT_CONSOLE, which too is deprecated).
>
> Going by linux upstream, 112 of 125 arm64 boards used "stdout-path":
> $ find /usr/lib/linux-image-4.19.0-10-arm64 -name \*.dtb|xargs strings
> -f|grep stdout-path|wc -l
> 112
>
> $ find /usr/lib/linux-image-4.19.0-10-arm64 -name \*.dtb|wc -l
> 125
Those numbers sound pretty convincing, certainly for arm64...
> I didn't check 32bit arm, but going by those numbers we could at least
> drop it for bootscript/arm64/bootscr.uboot-generic (I ran into this
> issue on arm64).
Likely uglier on armhf, but still might be worth doing.
So you've convinced me that it's probably time to consider removing this
workaround, and have the defaults cater to the majority of systems that
"do the right thing". Systems that need it can use a different boot
script as you suggested, or use ubootenv.d to set the console as needed
on an ad-hoc basis.
We also can also possibly leverage the ability to specify multiple
concatenated boot scripts on a per-platform basis to reduce duplicated
scripts.
It will definitely take some work to do well without breaking existing
systems; I'm not sure I have the time and/or energy for it personally,
but can help reviewing patches.
On a related note, the debian-installer case is a bit trickier, since
we're trying to produce a single image that boots on multiple platforms,
and we don't know in advance which defaults to use...
live well,
vagrant
Attachment:
signature.asc
Description: PGP signature