linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: Masami Hiramatsu <mhiramat@kernel.org>
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, 18 Feb 2020 14:27:24 +0100	[thread overview]
Message-ID: <20200218132724.GC14449@zn.tnic> (raw)
In-Reply-To: <20200211110207.7e0f1b048cc207e1a31ddd31@kernel.org>

On Tue, Feb 11, 2020 at 11:02:07AM +0900, Masami Hiramatsu wrote:
> 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)

Ok, I did just play with the thing. I used this boot config for my guest:

---
kernel {
	root=/dev/sda1
	resume=/dev/sda2
	debug
	ignore_loglevel
	log_buf_len=16M
	earlyprintk=ttyS0,115200
	console=ttyS0,115200
	console=tty0
	no_console_suspend
}

init {
	net.ifnames=0
}
---

and added it to an initrd which I created especially for this. Which
brings me to the next issue I see: AFAIU, bootconfig requires people
to have initrd. And the majority of deployments do have initrd enabled
but there are use cases where people don't need it. So if bootconfig is
going to be default Y, it better support that case.

For example and for a similar reason I support builtin microcode in the
microcode loader - when you don't want to enable initrd.

Then, I did add the above bootconfig to the initrd:

./tools/bootconfig/bootconfig -a bootconfig /tmp/initrd.img
Apply bootconfig to /tmp/initrd.img
        Number of nodes: 21
        Size: 194 bytes
        Checksum: 16268

and booted my guest.

The thing I noticed is that the guest doesn't show any output in its
window anymore but only shows the login window when it is done booting.

And that is probably because of the console= thing. /proc/bootconfig has

$ cat /proc/bootconfig
kernel.root = "/dev/sda1"
kernel.resume = "/dev/sda2"
kernel.debug = ""
kernel.ignore_loglevel = ""
kernel.log_buf_len = "16M"
kernel.earlyprintk = "ttyS0", "115200"
kernel.console = "ttyS0", "115200", "tty0"
kernel.no_console_suspend = ""
init.net.ifnames = "0"

and bootconfig parsing has merged the console= parameters. And the same
thing happens if I boot the guest with "console=ttyS0,115200,tty0" on
the normal cmdline. But

console=ttyS0,115200 console=tty0

is not the same as

console=ttyS0,115200,tty0

Former has been this way since forever if you wanna have serial output
to ttyS* *and* tty output at the same time.

Also, I wouldn't want to rule out any other changes in behavior in
other command line options due to this change in parsing and merging of
options.

So again, I'm still not convinced bootconfig should be used everywhere
and by default.

In addition to the initrd requirement, it needs a special tool to glue
it to the end of the initrd. Yeah yeah, it is part of the kernel and the
glueing can be made part of the initrd creation tool but still, it is
one more and new step.

Now, if bootconfig can *supplement* the normal command line so that you
can add more complex configuration directives with it, in *addition* to
the normal command line, then sure, by all means.

And it seems that it does that: I did

---
kernel {
        initcall_debug
}

init {
}
---

and added it to the cmdline:

[    0.047358] Kernel command line: initcall_debug root=/dev/sda1 resume=/dev/sda2 debug ignore_loglevel log_buf_len=16M earlyprintk=ttyS0,115200 console=ttyS0,115200 console=tty0 no_console_suspend net.ifnames=0  bootconfig

and you can see "initcall_debug" there and it did enable the initcall
debugging output so yeah, that seems to work.

Btw, if you remove the boot config with the tool (-d) and still boot
with "bootconfig" it says:

[    0.043958] bootconfig size -23483140 greater than max size 32767

so you need to check presence of bootconfig blob and limits in the
parsing code too or so.

In any case, this is only my opinion, of course, and I might very well
be missing something.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2020-02-18 13:27 UTC|newest]

Thread overview: 65+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-14 21:03 [for-next][PATCH 00/26] tracing: Updates for 5.6 Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 01/26] perf: Make struct ring_buffer less ambiguous Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 02/26] tracing: Rename trace_buffer to array_buffer Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 03/26] tracing: Make struct ring_buffer less ambiguous Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support Steven Rostedt
2020-02-06 11:54   ` Borislav Petkov
2020-02-06 14:41     ` Masami Hiramatsu
2020-02-06 17:20       ` Geert Uytterhoeven
2020-02-07  0:30         ` Masami Hiramatsu
2020-02-07  8:49           ` Geert Uytterhoeven
2020-02-07 13:17             ` Masami Hiramatsu
2020-02-07 14:28               ` [PATCH] bootconfig: Allocate xbc_nodes array dynamically Masami Hiramatsu
2020-02-06 17:58       ` [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support Borislav Petkov
2020-02-06 18:10         ` Randy Dunlap
2020-02-06 22:39           ` Steven Rostedt
2020-02-06 22:51             ` Borislav Petkov
2020-02-07  8:59             ` Geert Uytterhoeven
2020-02-07  2:46         ` Masami Hiramatsu
2020-02-07 11:41           ` Borislav Petkov
2020-02-07 15:06             ` Masami Hiramatsu
2020-02-10 11:25               ` Borislav Petkov
2020-02-10 15:10                 ` Masami Hiramatsu
2020-02-10 17:40                   ` Borislav Petkov
2020-02-11  2:02                     ` Masami Hiramatsu
2020-02-18 13:27                       ` Borislav Petkov [this message]
2020-02-18 17:57                         ` Steven Rostedt
2020-02-18 19:38                           ` Borislav Petkov
2020-02-19  2:26                             ` Masami Hiramatsu
2020-02-19  2:26                           ` Masami Hiramatsu
2020-02-19  2:26                         ` Masami Hiramatsu
2020-02-11  9:25                     ` Geert Uytterhoeven
2020-01-14 21:03 ` [for-next][PATCH 05/26] bootconfig: Load boot config from the tail of initrd Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 06/26] tools: bootconfig: Add bootconfig command Steven Rostedt
2020-02-07 13:02   ` Michael Ellerman
2020-02-07 13:39     ` Masami Hiramatsu
2020-02-07 13:55     ` [PATCH] tools/bootconfig: Fix wrong __VA_ARGS__ usage Masami Hiramatsu
2020-02-08 11:10       ` Michael Ellerman
2020-02-09  4:09         ` Masami Hiramatsu
2020-02-09 13:05         ` [PATCH] tools/bootconfig: Suppress non-error messages Masami Hiramatsu
2020-02-10  2:06           ` Michael Ellerman
2020-02-10  7:18             ` Masami Hiramatsu
2020-02-10  9:50               ` Michael Ellerman
2020-02-10  9:50       ` [PATCH] tools/bootconfig: Fix wrong __VA_ARGS__ usage Michael Ellerman
2020-02-10 17:07         ` Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 07/26] tools: bootconfig: Add bootconfig test script Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 08/26] proc: bootconfig: Add /proc/bootconfig to show boot config list Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 09/26] init/main.c: Alloc initcall_command_line in do_initcall() and free it Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 10/26] bootconfig: init: Allow admin to use bootconfig for kernel command line Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 11/26] bootconfig: init: Allow admin to use bootconfig for init " Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 12/26] Documentation: bootconfig: Add a doc for extended boot config Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 13/26] tracing: Apply soft-disabled and filter to tracepoints printk Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 14/26] tracing: kprobes: Output kprobe event to printk buffer Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 15/26] tracing: kprobes: Register to dynevent earlier stage Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 16/26] tracing: Accept different type for synthetic event fields Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 17/26] tracing: Add NULL trace-array check in print_synth_event() Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 18/26] tracing/boot: Add boot-time tracing Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 19/26] tracing/boot: Add per-event settings Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 20/26] tracing/boot Add kprobe event support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 21/26] tracing/boot: Add synthetic " Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 22/26] tracing/boot: Add instance node support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 23/26] tracing/boot: Add cpu_mask option support Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 24/26] tracing/boot: Add function tracer filter options Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 25/26] Documentation: tracing: Add boot-time tracing document Steven Rostedt
2020-01-14 21:03 ` [for-next][PATCH 26/26] tracing: trigger: Replace unneeded RCU-list traversals Steven Rostedt
2020-01-14 22:11   ` Steven Rostedt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200218132724.GC14449@zn.tnic \
    --to=bp@alien8.de \
    --cc=akpm@linux-foundation.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mhiramat@kernel.org \
    --cc=mingo@kernel.org \
    --cc=rostedt@goodmis.org \
    --subject='Re: [for-next][PATCH 04/26] bootconfig: Add Extra Boot Config support' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox