[LEDE-DEV] [PATCH] kernel: make fix extending dtb cmd with bootloader

[Hauke Mehrtens]

On 12/25/2016 07:12 PM, Martin Blumenstingl wrote:
> Hi John, Hi Hauke,
> 
> On Fri, Dec 9, 2016 at 8:08 AM, John Crispin <john at phrozen.org> wrote:
>>
>>
>> On 09/12/2016 00:21, Hauke Mehrtens wrote:
>>> This was introduced in kernel 4.4, but broken there and fixed in 4.5.
>>> I would like to activate it, but I am scared about the boot loader
>>> around giving us all sorts of command lines.
>>>
>>> Signed-off-by: Hauke Mehrtens <hauke at hauke-m.de>
>>
>> we had previously used a syntax of prepending the cmdline with +/- to
>> indicate whether this should replace/extend the bootloader cmdline. the
>> biggest problem is that the bootloaders will pass console= and root= and
>> thus breaking boot. merging/activating this as is will most likely lead
>> to 50% of lantiq boards not booting anymore. imho this feature should be
>> an opt-in based on the specific board
> Mathias just pointed me at a bug report [0] where I caused the exact
> problem you described (with [1]).
> When /chosen/bootargs is not defined the kernel falls back to using
> the cmdline passed from the bootloader - breaking at least the console
> in that specific case.
> 
> maybe we should consider the following use-cases:
> 1) bootloader passed bogus root=/console= parameters
> 2) Hauke wants to specify root= himself in the bootloader (where Hauke
> stands for "developer who wants to use NFS rootfs)

It would be sufficient if this would be possible for some boards where
I have also control over the boot loader. This way I can make sure they
will not add wrong data.

> 3) bootloader passes some valid parameters (for example ethaddr) -
> could be the same bootloader as #1

For Voice we also get some parameters to reserve memory for the
firmware, but we can also handle this reservation in a different way.
This would also not get upstream because upstream already have a better
mechanism to do this, but it needs a modified firmware which works on
virtual memory. This is also not board specific and can also be added
statically into the device tree.

> maybe we can brainstorm potential solutions which work in all cases:
> - if we simply revert my patch we're fixing #1 but that would still
> leave #2 and #3 unsolved

I like your patch in general. ;-)

> - checking with upstream why the bootloader cmdline is used
> (CONFIG_MIPS_CMDLINE_FROM_BOOTLOADER is not set in our kernel config),
> but that would only fix #1
> - not reverting my patch and enabling CONFIG_MIPS_CMDLINE_DTB_EXTEND
> would work for #2 and #3 but leave #1 broken

We could do CONFIG_MIPS_CMDLINE_DTB_EXTEND the other way around. let the
dtb overwrite everything is gets from the boot loader.

> - reverting my patch and enabling CONFIG_MIPS_CMDLINE_DTB_EXTEND would
> fix #2 and #3 while #1 is broken again
> - not reverting my patch, enabling CONFIG_MIPS_CMDLINE_DTB_EXTEND and
> adding a "black- or white-listed bootargs from bootloader" per device
> might work for all 3 (John, I guess this is what you meant with this
> opt-in specific behavior?)

I would like this approach, would it be possible to get this into mainline?

> I'm open for more ideas. I can also have a look at the implementation
> of a patch once we decided on a solution.
> 
> 
> Regards,
> Martin
> 
> 
> [0] https://bugs.lede-project.org/index.php?do=details&task_id=350
> [1] https://git.lede-project.org/?p=source.git;a=commitdiff;h=6b94234a6598b855573a6516494de8e7d755e944
>