Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 13 Dec 2022 09:29:20 -0600
From:   Rob Herring <robh@kernel.org>
To:     Linus Walleij <linus.walleij@linaro.org>,
        Alexander Sverdlin <alexander.sverdlin@gmail.com>
Cc:     devicetree@vger.kernel.org, Frank Rowand <frowand.list@gmail.com>,
        linux-kernel@vger.kernel.org, Arnd Bergmann <arnd@kernel.org>,
        Nikita Shubin <nikita.shubin@maquefel.me>,
        Hartley Sweeten <hsweeten@visionengravers.com>,
        Lukasz Majewski <lukma@denx.de>,
        Linus Walleij <linusw@kernel.org>
Subject: Re: [PATCH] of: fdt: Honor CONFIG_CMDLINE* even without /chosen node
Message-ID: <20221213152920.GA1025576-robh@kernel.org>
References: <11af73e05bad75e4ef49067515e3214f6d944b3d.camel@gmail.com>
 <CACRpkdaXxPE3XhPnprhJ7GGjn9X_GMsCJ3mn26P90GcHzhsQDg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CACRpkdaXxPE3XhPnprhJ7GGjn9X_GMsCJ3mn26P90GcHzhsQDg@mail.gmail.com>
Precedence: bulk

On Tue, Dec 13, 2022 at 09:51:33AM +0100, Linus Walleij wrote:
> On Mon, Dec 12, 2022 at 7:01 AM Alexander Sverdlin
> <alexander.sverdlin@gmail.com> wrote:
> 
> > I do not read a strict requirement on /chosen node in either ePAPR or in
> > Documentation/devicetree. Help text for CONFIG_CMDLINE and
> > CONFIG_CMDLINE_EXTEND doesn't make their behavior explicitly dependent on
> > the presence of /chosen or the presense of /chosen/bootargs.
> >
> > However the early check for /chosen and bailing out in
> > early_init_dt_scan_chosen() skips CONFIG_CMDLINE handling which is not
> > really related to /chosen node or the particular method of passing cmdline
> > from bootloader.
> >
> > This leads to counterintuitive combinations (assuming
> > CONFIG_CMDLINE_EXTEND=y):
> >
> > a) bootargs="foo", CONFIG_CMDLINE="bar" => cmdline=="foo bar"
> > b) /chosen missing, CONFIG_CMDLINE="bar" => cmdline==""
> > c) bootargs="", CONFIG_CMDLINE="bar" => cmdline==" bar"
> >
> > Move CONFIG_CMDLINE handling outside of early_init_dt_scan_chosen() so that
> > cases b and c above result in the same cmdline.
> >
> > Signed-off-by: Alexander Sverdlin <alexander.sverdlin@gmail.com>
> 
> Excellent debugging Alexander!
> Reviewed-by: Linus Walleij <linus.walleij@linaro.org>
> 
> I also think this should go to stable.

We have to be careful there. This could change behavior on a working 
system. A system taking the cmdline entirely from a built kernel and 
no initrd is going to be pretty customized already, I think they can 
carry a patch. What platform is this anyways?

This has actually been known for some time[1][2]. My concern in the past 
(besides wanting all the cmdline manipulation being common) was MIPS. 
MIPS in particular has lots of sources for cmdline and ways to combine 
it. However, MIPS has since stopped using this code and does their own 
parsing (not great either IMO).

Rob

[1] https://lore.kernel.org/all/CAL_Jsq+CgxWbCMz_qwLbMJS3fYwKyCMBGVS501-5ShXywyDAXA@mail.gmail.com/
[2] https://lore.kernel.org/all/CAL_Jsq+n4e5_ptuh89CJibViGS_bgHz0A+Ki-uwtcGU38+mHXQ@mail.gmail.com/