Fixing kernel parameter issues with latest kernel and WM8650 laptop

[Brigham Campbell]

Alright, I now know that Linux 5.8.0 can be built to work on the WM8650
laptop (woohoo!). But there's a strange issue with the way boot
arguments are passed to the kernel.

At the moment, I can pass boot arguments into the kernel by compiling
them into the kernel via the kernel config. HOWEVER, this only works
when the WM8650 device tree also has "bootargs" defined. The arguments
defined in the device tree aren't the ones being used, though. Those in
the .config are.

Alexey has speculated that it's a bug with the code that compiles ATAGs
into the device tree node. Frankly, I have no clue. But I'm going to
compile earlier versions of the kernel source to see if I can find a
version that doesn't have the issue. Then we'll be able to track down
which commit introduced the problem.

Wish me luck!
Brigham

[Brigham Campbell]

I performed a git bisect on the kernel source. It looks like commit
abe60a3a7afb4058278864aa18c5faf62094c11a (also known as
v5.0-rc4-9-gabe60a3a7afb) from January 9, 2019 is the culprit.

It looks like this commit modified the relevant device tree and it's
paired with a lot changes to other devices. This was likely a change
which was applied to several devices en masse, so I doubt the changes
were tested on any WM8650-based devices.

I'll be testing some of my previous kernel configurations on this commit
and the commit immediately before it just to verify. If this commit
really did introduce the issue, I'll whip up a patch.

Cheers,
Brigham

[Alexey Charkov]

пт, 9 окт. 2020 г. в 15:23, Brigham Campbell <m...@brighamcampbell.com>:

>
> I performed a git bisect on the kernel source. It looks like commit
> abe60a3a7afb4058278864aa18c5faf62094c11a (also known as
> v5.0-rc4-9-gabe60a3a7afb) from January 9, 2019 is the culprit.

Does your problem disappear if you just add an empty 'chosen' node to
your device tree at the top level?

[Brigham Campbell]

> Does your problem disappear if you just add an empty 'chosen' node to
> your device tree at the top level?

Yes! It looks like inserting an empty "chosen" node fixes the issue in
both abe60a3a7 and v5.8.

What do you see?

[Alexey Charkov]

пт, 9 окт. 2020 г. в 16:11, Brigham Campbell <m...@brighamcampbell.com>:

That's awesome! Successful triaging of a regression in Linus' upstream
kernel - how does that feel? ;-)

> What do you see?

Sounds like it would be relevant for any board that relies on the
"appended DTB" boot method. Would you like to make a patch to add
those empty nodes into all of the WM device trees and submit it
upstream? Maybe also add a comment about its possible relevance for
other "appended DTB" poor souls, along with a reference to the commit
abe60a3a7 which introduced the issue.

Best,
Alexey

[Brigham Campbell]

> That's awesome! Successful triaging of a regression in Linus' upstream
> kernel - how does that feel? ;-)

Weird! I did't expect this project to lead to my first patch sent to the
LKML. Hahah! I sent the patch and I probably formatted it wrong. I guess
we'll see what the relevant maintainers have to say about it (I CC'ed
you, by the way).

> Sounds like it would be relevant for any board that relies on the
> "appended DTB" boot method. Would you like to make a patch to add
> those empty nodes into all of the WM device trees and submit it
> upstream? Maybe also add a comment about its possible relevance for
> other "appended DTB" poor souls, along with a reference to the commit
> abe60a3a7 which introduced the issue.

Done. Although, it's still a little unclear to me how we reached this
solution. I happened across half of the solution by mistake, it seems,
just by trying to add the boot parameters to the device tree.

Would you mind pointing me towards some documentation which would help
me figure out exactly _why_ this solution works? I imagine it has to do
with the device tree compiler and the code which gets kernel parameters
from ATAGs, but I wouldn't really know how to go about chasing down the
root cause.

Maybe replicating the issue on a similar ARM development board which I
have proper debugging access to would be a good start?

Thanks! This has been a super cool and crazy rabbit-hole...
Brigham

[Alexey Charkov]

On Sat, 10 Oct 2020 at 04:01, Brigham Campbell <m...@brighamcampbell.com> wrote:

> That's awesome! Successful triaging of a regression in Linus' upstream
> kernel - how does that feel? ;-)

Weird! I did't expect this project to lead to my first patch sent to the
LKML. Hahah! I sent the patch and I probably formatted it wrong. I guess
we'll see what the relevant maintainers have to say about it (I CC'ed
you, by the way).

Yep, I saw that one. The formatting looked good to me, but we’ll see what the gurus say :)

> Sounds like it would be relevant for any board that relies on the
> "appended DTB" boot method. Would you like to make a patch to add
> those empty nodes into all of the WM device trees and submit it
> upstream? Maybe also add a comment about its possible relevance for
> other "appended DTB" poor souls, along with a reference to the commit
> abe60a3a7 which introduced the issue.

Done. Although, it's still a little unclear to me how we reached this
solution. I happened across half of the solution by mistake, it seems,
just by trying to add the boot parameters to the device tree.

There is something wrong with allocating the string buffer that should carry your kernel command line when the DT parsing code doesn’t find the proper node apparently (bootargs under chosen). The commit you found as the culprit removed the skeleton.dtsi file, which contained some default memory nodes and an empty default ‘chosen’ node. It added back memory related stuff directly into board specific DTS files, but not the empty ‘chosen’.

Given that, the next step was to check if it’s indeed relevant to your observed problem - i.e. put back the possibly relevant bit which that commit removed. And it worked - QED :)

Would you mind pointing me towards some documentation which would help
me figure out exactly _why_ this solution works? I imagine it has to do
with the device tree compiler and the code which gets kernel parameters
from ATAGs, but I wouldn't really know how to go about chasing down the
root cause.

It’s well possible that we are not really addressing the true root cause by just adding that empty ‘chosen’ node, because even if DT fails to provide the command line I’d expect the built-in one to still show up. So going forward it would be helpful to trace through the early init code for whatever allocates the string buffer for the command line, and make sure it fails gracefully if the DT is messed up in any way. The other part of the problem is that the ATAGs parsing code apparently fails through the code that’s supposed to populate the command line into the in-memory DT when it doesn’t find a ‘chosen’ to put ‘bootargs’ under. A more graceful behavior for it would have been to create an empty ‘chosen’ automatically if one is absent from the bootloader provided DT.

Maybe replicating the issue on a similar ARM development board which I
have proper debugging access to would be a good start?

Possibly. The problem is complicated by the fact that we are struggling with early kernel init code - I’m not even sure earlyprintk is available at this stage, let alone KGDB. But it might be possible to step through the code in QEMU I guess, of you boot it with appended DTB and some ugly old bootloader (I suspect it doesn’t have to be WonderMedia hardware per se).

Thanks! This has been a super cool and crazy rabbit-hole...

You’re most welcome! Of course, it was purely selfish manipulation on my end to drag you into kernel hacking, but it’s awesome if you liked it :)

I for one really enjoyed the thoroughness and, I would say, academic rigor of your recap email by which you finished up the initial thread!

Best,

Alexey