Re: [PATCH 2/2] bootconfig: Apply early options from embedded config

[Masami Hiramatsu (Google) <mhiramat@kernel.org>]

On Mon, 27 Nov 2023 16:02:22 +0100
Petr Malat <oss@malat.biz> wrote:

> Hi Masami,
> On Mon, Nov 27, 2023 at 07:46:30AM +0900, Masami Hiramatsu wrote:
> 
> Shortened the mail as this seems to be the last open point
> 
> > > > And as I pointed, we can remove CONFIG_BOOT_CONFIG_EMBED_APPEND_INITRD so this case
> > > > should be removed.
> > > 
> > > I have added BOOT_CONFIG_EMBED_APPEND_INITRD, because it's not backward
> > > compatible change and I didn't want to risk breaking current use cases.
> > > My change tries to get early options working without affecting how
> > > other options are handled, but I think appending the config is more
> > > reasonable behavior and if you do not see it as a problem to not be
> > > backward compatible here, I will delete the "replace" behavior.
> > 
> > That's a good point. OK if disabling CONFIG_BOOT_CONFIG_EMBED_APPEND_INITRD,
> > it must skip setting early_params to avoid "hidden setting" from the
> > embedded bootconfig.
>  
> That's not a good idea because then disabling BOOT_CONFIG_EMBED_APPEND_INITRD
> would disable early options handling even if the user doesn't use initrd at
> all, which we do not want.

Yes, so the config name and comment needs to be updated. The problematic point
is that we can not know where there is an initrd (and bootconfig on initrd)
until parsing/applying the early params. And we have to avoid setting "hidden"
parameter.

> 
> I suggest logging a KERN_NOTICE message if any early option was applied and
> at the same time embedded bootconfig was replaced.

No, that's no good because kernel log can be cleared.
Usually user will check /proc/cmdline to check what parameters are set, so at
least it needs to be updated, but also, we need to keep the original cmdline
for make sure the user can reuse it for kexec. To solve this issue without
contradiction, we need to ask user to set BOOT_CONFIG_EMBED_APPEND_INITRD=y
to support early params in bootconfig. (Thus it will be something like
"BOOT_CONFIG_EMBED_EARLY_PARAM")

Thank you,

>   Petr


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>