public inbox for linux-doc@vger.kernel.org
 help / color / mirror / Atom feed
From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Breno Leitao <leitao@debian.org>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Shuah Khan <skhan@linuxfoundation.org>,
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, oss@malat.biz, paulmck@kernel.org,
	rostedt@goodmis.org, kernel-team@meta.com
Subject: Re: [PATCH v2] bootconfig: Apply early options from embedded config
Date: Wed, 25 Mar 2026 23:22:04 +0900	[thread overview]
Message-ID: <20260325232204.05edbb21c7602b6408ca007b@kernel.org> (raw)
In-Reply-To: <20260325-early_bootconfig-v2-1-6b05a36fbfb5@debian.org>

Hi Breno,

On Wed, 25 Mar 2026 03:05:38 -0700
Breno Leitao <leitao@debian.org> wrote:

> Bootconfig currently cannot be used to configure early kernel
> parameters. For example, the "mitigations=" parameter must be passed
> through traditional boot methods because bootconfig parsing happens
> after these early parameters need to be processed.
> 
> This patch allows early options such as:
> 
>   kernel.mitigations = off
> 
> to be placed in the embedded bootconfig and take effect, without
> requiring them to be on the kernel command line.
> 
> Add bootconfig_apply_early_params() which walks all kernel.* keys in the
> parsed XBC tree and calls do_early_param() for each one. It is called
> from setup_boot_config() immediately after a successful xbc_init() on
> the embedded data, which happens before parse_early_param() runs in
> start_kernel().
> 
> Early options in initrd bootconfig are still silently ignored, as the
> initrd is only available after the early param window has closed.
> 
> Document this behaviour in both Kconfig and the admin guide.

AI review made some comments. Some of the review comments seem
reasonable.

https://sashiko.dev/#/patchset/20260325-early_bootconfig-v2-1-6b05a36fbfb5%40debian.org



[..]
> 
> diff --git a/init/main.c b/init/main.c
> index 453ac9dff2da0..14a04c283fa48 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -416,9 +416,64 @@ static int __init warn_bootconfig(char *str)
>  	return 0;
>  }
>  
> +/*
> + * do_early_param() is defined later in this file but called from
> + * bootconfig_apply_early_params() below, so we need a forward declaration.
> + */
> +static int __init do_early_param(char *param, char *val,
> +				 const char *unused, void *arg);
> +
> +/*
> + * bootconfig_apply_early_params - dispatch kernel.* keys from the embedded
> + * bootconfig as early_param() calls.
> + *
> + * early_param() handlers must run before most of the kernel initialises
> + * (e.g. before the GIC driver reads irqchip.gicv3_pseudo_nmi).  A bootconfig
> + * attached to the initrd arrives too late for this because the initrd is not
> + * mapped yet when early params are processed.  The embedded bootconfig lives
> + * in the kernel image itself (.init.data), so it is always reachable.
> + *
> + * This function is called from setup_boot_config() which runs in
> + * start_kernel() before parse_early_param(), making the timing correct.
> + */
> +static void __init bootconfig_apply_early_params(void)

[sashiko comment]
| Does this run early enough for architectural parameters?
| While setup_boot_config() runs before parse_early_param() in start_kernel(),
| it runs after setup_arch(). setup_boot_config() relies on xbc_init() which
| uses the memblock allocator, requiring setup_arch() to have already
| initialized it.
| However, the kernel expects many early parameters (like mem=, earlycon,
| noapic, and iommu) to be parsed during setup_arch() via the architecture's
| call to parse_early_param(). Since setup_arch() completes before
| setup_boot_config() runs, will these architectural early parameters be
| silently ignored because the decisions they influence were already
| finalized?

This is the major reason that I did not support early parameter
in bootconfig. Some archs initialize kernel_cmdline in setup_arch()
and setup early parameters in it.
To fix this, we need to change setup_arch() for each architecture so
that it calls this bootconfig_apply_early_params().

> +{
> +	static char val_buf[COMMAND_LINE_SIZE] __initdata;

[sashiko comment]
| Can using a single shared static buffer cause data corruption for handlers
| that save the argument pointer?
| Several early_param handlers assume the passed string pointer is persistent
| (like the boot_command_line) and retain it internally. For example,
| setup_earlycon() calls register_earlycon(), which sets
| early_console_dev.con->options = options, where options is a pointer
| directly into the passed buffer.
| Because val_buf is overwritten on every loop iteration, the stored pointer
| will point to the value of the last bootconfig key processed.

Ah, good catch. Since we don't have any standard way to handle the
parameters, some of them does not copy the value but try to keep
reference to the given string. 

> +	struct xbc_node *knode, *root;
> +	const char *val;
> +	ssize_t ret;
> +
> +	root = xbc_find_node("kernel");
> +	if (!root)
> +		return;
> +
> +	/*
> +	 * Keys that do not match any early_param() handler are silently
> +	 * ignored — do_early_param() always returns 0.
> +	 */
> +	xbc_node_for_each_key_value(root, knode, val) {

[sashiko comment]
| Does this loop handle array values correctly?
| xbc_node_for_each_key_value() only assigns the first value of an array to
| the val pointer before advancing to the next key. It does not iterate over
| the child nodes of the array.
| If the bootconfig contains a multi-value key like
| kernel.console = "ttyS0", "tty0", will the subsequent values in the array
| be silently dropped instead of passed to the early_param handlers?

Also, good catch :) we need to use xbc_node_for_each_array_value()
for inner loop.

> +		if (xbc_node_compose_key_after(root, knode, xbc_namebuf, XBC_KEYLEN_MAX) < 0)
> +			continue;
> +
> +		/*
> +		 * We need to copy const char *val to a char pointer,
> +		 * which is what do_early_param() need, given it might
> +		 * call strsep(), strtok() later.
> +		 */
> +		ret = strscpy(val_buf, val, sizeof(val_buf));
> +		if (ret < 0) {
> +			pr_warn("ignoring bootconfig value '%s', too long\n",
> +				xbc_namebuf);
> +			continue;
> +		}
> +		do_early_param(xbc_namebuf, val_buf, NULL, NULL);

[sashiko comment]
| How does this handle valueless parameters (boolean flags)?
| When parsing the standard kernel command line, parse_args() passes a NULL
| value to the setup function for flags that lack an = sign (e.g., ro or
| earlycon).
| However, the bootconfig parser returns a zero-length string for valueless
| keys, which gets copied into val_buf as "" and passed to do_early_param().
| This semantic deviation breaks handlers that explicitly check if (!val).
| For instance, param_setup_earlycon() and parse_lapic() check for a NULL
| argument to enable features. Will passing "" instead of NULL prevent these
| handlers from working correctly?

See fs/proc/bootconfig.c. You can check whether the key has a value or
not by checking xbc_node_get_child(knode) != NULL.

Thank you,

> +	}
> +}
> +
>  static void __init setup_boot_config(void)
>  {
>  	static char tmp_cmdline[COMMAND_LINE_SIZE] __initdata;
> +	bool using_embedded = false;
>  	const char *msg, *data;
>  	int pos, ret;
>  	size_t size;
> @@ -427,8 +482,17 @@ static void __init setup_boot_config(void)
>  	/* Cut out the bootconfig data even if we have no bootconfig option */
>  	data = get_boot_config_from_initrd(&size);
>  	/* If there is no bootconfig in initrd, try embedded one. */
> -	if (!data)
> +	if (!data) {
>  		data = xbc_get_embedded_bootconfig(&size);
> +		/*
> +		 * Record that we are using the embedded config so that
> +		 * bootconfig_apply_early_params() is called below.
> +		 * When CONFIG_BOOT_CONFIG_EMBED is not set,
> +		 * xbc_get_embedded_bootconfig() is a stub returning NULL, so
> +		 * data is always NULL here and using_embedded stays false.
> +		 */
> +		using_embedded = data;
> +	}
>  
>  	strscpy(tmp_cmdline, boot_command_line, COMMAND_LINE_SIZE);
>  	err = parse_args("bootconfig", tmp_cmdline, NULL, 0, 0, 0, NULL,
> @@ -466,6 +530,8 @@ static void __init setup_boot_config(void)
>  	} else {
>  		xbc_get_info(&ret, NULL);
>  		pr_info("Load bootconfig: %ld bytes %d nodes\n", (long)size, ret);
> +		if (using_embedded)
> +			bootconfig_apply_early_params();
>  		/* keys starting with "kernel." are passed via cmdline */
>  		extra_command_line = xbc_make_cmdline("kernel");
>  		/* Also, "init." keys are init arguments */
> 
> ---
> base-commit: 785f0eb2f85decbe7c1ef9ae922931f0194ffc2e
> change-id: 20260323-early_bootconfig-2efc4509af3d
> 
> Best regards,
> --  
> Breno Leitao <leitao@debian.org>
> 


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

  reply	other threads:[~2026-03-25 14:22 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-25 10:05 [PATCH v2] bootconfig: Apply early options from embedded config Breno Leitao
2026-03-25 14:22 ` Masami Hiramatsu [this message]
2026-03-26 14:30   ` Masami Hiramatsu
2026-03-27 10:18     ` Breno Leitao
2026-03-27 14:16       ` Masami Hiramatsu
2026-03-27 16:11         ` Breno Leitao
2026-03-27 10:06   ` Breno Leitao
2026-03-27 13:37     ` Masami Hiramatsu

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=20260325232204.05edbb21c7602b6408ca007b@kernel.org \
    --to=mhiramat@kernel.org \
    --cc=corbet@lwn.net \
    --cc=kernel-team@meta.com \
    --cc=leitao@debian.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=oss@malat.biz \
    --cc=paulmck@kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=skhan@linuxfoundation.org \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox