Re: [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support

From: Masami Hiramatsu <mhiramat@kernel.org>
To: Borislav Petkov <bp@alien8.de>
Cc: Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org, Ingo Molnar <mingo@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>
Subject: Re: [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support
Date: Tue, 11 Feb 2020 11:02:07 +0900	[thread overview]
Message-ID: <20200211110207.7e0f1b048cc207e1a31ddd31@kernel.org> (raw)
In-Reply-To: <20200210174053.GD29627@zn.tnic>

On Mon, 10 Feb 2020 18:40:53 +0100
Borislav Petkov <bp@alien8.de> wrote:

> On Tue, Feb 11, 2020 at 12:10:07AM +0900, Masami Hiramatsu wrote:
> > Sure, there are some examples under Documentation/trace/boottime-trace.rst.
> > Since the tracefs (ftrace's filesystem user interface) is extensible
> > by dynamic events and instanses, I need flexibility of option "keys"
> > not only values, also I need structured settings because some keys
> > will configure same events or trace instance.
> 
>  [ ... ]
> 
> Yap, gotcha. It makes a lot of sense for you guys because you don't want
> to be typing all those long ftrace command lines on every boot. And the
> command line in grub can become too long and hard to handle too.

Thanks :)

> 
> > 1) write a following bootconfig file
> > ----
> > kernel {
> > 	# root device and resume devices
> > 	root = /dev/disk/by-id/nvme-eui.0025385481b2fe2a-part2
> > 	resume = /dev/disk/by-id/nvme-eui.0025385481b2fe2a-part1
> > 	ro			# Mount root device readonly.
> > 	debug			# boot with debug log
> > 	ignore_loglevel	# this will print all messages
> > 	log_buf_len = 16M	# So expand log buffer to 16MB
> > 	no_console_suspend	# Debugging suspend process
> > 	mem_encrypt = off	# Set AMD SME to off
> > }
> > 
> > init {
> > 	net.ifnames = 0	# Use tradisional ifname
> > 	systemd.log_target = null	# Ignore systemd log?
> > }
> 
> Ok.
> 
> However, this has a downside. When you request dmesg from a user because
> you're debugging an issue - and we do that all the time - if the command
> line were in a config file, we would have to see that config file too.
> 
> VS now, it is simply in dmesg.

The above kernel.* and init.* are merged into the legacy command line
before printing it out to dmesg. For boot-time tracing, yes, we need
/proc/bootconfig too.

> > Boot time tracing is just a example. I think we can expand this for some
> > other subsystems too. And this might be also helpful for adding a bit more
> > complex syntax to those parameters without parser.
> 
> Yes, I think I see it now. And I still don't think you want this enabled
> by default on every box. It is expressed perfectly fine here:
> 
> config BOOTTIME_TRACING
>         bool "Boot-time Tracing support"
>         depends on BOOT_CONFIG && TRACING

Hm, at least "select BOOT_CONFIG" so that user can see this option is
there on menuconfig :)

> so if distros want to enable that, they will enable BOOT_CONFIG too,
> transitively. And other subsystems would simply depend on it the same if
> they wanna use bootconfig.
> 
> But the way we supply command line args now ain't broke. And they normal
> user doesn't care - grub simply pastes them everywhere.
> 
> Don't get me wrong - I don't mind using a bootconfig. I simply don't see
> a compelling argument to have it enabled everywhere and by default and
> think that other stuff selecting it is perfectly fine. IMO, of course.

Sorry, you might misunderstand that the bootconfig replaces legacy command
line. No, the legacy command line is still there and at least the part of
suppremental command line parts of bootconfig are merged into the command
line and shown in dmesg. So even if the user use bootconfig, their kernel
command line was just updated, they can grub it and simply paste on other
machine (if it accepts longer command line)

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>