Hi,
On 11/25/25 11:22 AM, Fabian Pflug wrote:
> Hey :)
>
> On Mon, 2025-11-24 at 13:00 +0100, Ahmad Fatoum wrote:
>> Hi Fabian,
>>
>> On 11/24/25 11:16 AM, Fabian Pflug wrote:
>>> In verified boot settings, it might not be wanted to have a root=
>>> command line parameter for the kernel, because if the initramfs is not
>>> configured, or barebox boots the kernel directly due to
>>> misconfiguration, the device might boot correctly, but without verified
>>> boot, because the kernel will parse root=
>>>
>>> Allowing to change the parameter name will help circumvent such errors,
>>> as there now needs to be multiple misconfigurations for the verified
>>> boot to go wrong and the initramfs can use a the different name from the
>>> commandline to get the rootfs.
>>
>> Thanks for your patch!
>>
>> This is a useful thing to have, but I think deciding replacement of
>> root= across all functionality at compile-time is too invasive:
>>
>> Imagine you use the same barebox both for development and in release
>> mode (or you support runtime unlocking with a token).
>> When booting securely, you want to use verity_root= for example, but
>> when you do boot /mnt/nfs, you'll want barebox to fix up root= as before.
>>
>> I thus think that this should be a $global.bootm.root_arg evaluated at
>> runtime instead.
>>
>> By adding it to bootm_data_restore_defaults(), its value will then be
>> restored after the boot script has done executing.
>>
>> What do you think?
>
>
> Do you want to have string-replacement in bootm, or should all of them query
> the value for root_arg?
>
> Are you thinking something along the lines of:
>
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -868,6 +868,11 @@ int bootm_boot(struct bootm_data *bootm_data)
> if (IS_ERR(rootarg)) {
> pr_err("Failed to append kernel cmdline parameter
> 'root='\n");
> } else {
> + if (bootm_data->prefix_root) {
> + char* rootarg_old = rootarg;
> + rootarg =
> xasprintf("%s%s",bootm_data->prefix_root, rootarg+5);
> + free(rootarg_old);
> + }
> pr_info("Adding \"%s\" to Kernel commandline\n",
> rootarg);
>
> globalvar_add_simple("linux.bootargs.bootm.appendroot",
> rootarg);
>
>
> This could work as a hack, but it does seem error-prone.
I'd rather not rely on root= being at the beginning. Otherwise, there is
little robustness benefit over:
$ devlookup -k -v ROOT /dev/mmc0.root-A
$ global linux.bootargs.root=verity_${ROOT}
As discussed off-list, given that there is no simple way to retrofit it,
the API should be adapted, so file systems and cdevs report the root=
and the opts separately and it's then up to the consumer how to
concatenate them.
Cheers,
Ahmad
>
> Kind regards
> Fabian
>
>>
>> Cheers,
>> Ahmad
>>
>>>
>>> Signed-off-by: Fabian Pflug <[email protected]>
>>> ---
>>> common/Kconfig | 13 +++++++++++++
>>> common/block.c | 3 +--
>>> common/bootm.c | 8 ++++----
>>> drivers/mci/mci-core.c | 2 +-
>>> fs/9p/vfs_super.c | 2 +-
>>> fs/nfs.c | 2 +-
>>> fs/squashfs/squashfs.c | 2 +-
>>> fs/ubifs/ubifs.c | 2 +-
>>> 8 files changed, 23 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/common/Kconfig b/common/Kconfig
>>> index d923d4c4b6..62797a9e69 100644
>>> --- a/common/Kconfig
>>> +++ b/common/Kconfig
>>> @@ -1252,6 +1252,19 @@ config SERIAL_NUMBER_FIXUP_SYSTEMD_HOSTNAME
>>> This option without effect if global.bootm.provide_hostname
>>> is unset.
>>>
>>> +config ROOT_COMMANDLINE_PARAMETER
>>> + string "parameter name for root partition"
>>> + default "root"
>>> + help
>>> + When setting the root partition on the commandline string to the os,
>>> use
>>> + this name as an indicator of the root partition.
>>> +
>>> + Changing this could be useful in verified boot contexts to not give
>>> the
>>> + kernel the chance to boot a non-secure image, but still use the power
>>> of
>>> + barebox to get the right disk for the rootfs.
>>> +
>>> + If unsure, leave as default (root)
>>> +
>>> config SYSTEMD_OF_WATCHDOG
>>> bool "inform devicetree-enabled kernel of used watchdog"
>>> depends on WATCHDOG && OFTREE && FLEXIBLE_BOOTARGS
>>> diff --git a/common/block.c b/common/block.c
>>> index ca2ed37dbd..bb8b01c0a0 100644
>>> --- a/common/block.c
>>> +++ b/common/block.c
>>> @@ -601,11 +601,10 @@ char *cdev_get_linux_rootarg(const struct cdev
>>> *partcdev)
>>> if (blk->ops->get_rootarg)
>>> rootarg = blk->ops->get_rootarg(blk, partcdev);
>>> if (!rootarg && partcdev->partuuid[0] != 0)
>>> - rootarg = basprintf("root=PARTUUID=%s", partcdev->partuuid);
>>> + rootarg = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER
>>> "=PARTUUID=%s", partcdev->partuuid);
>>>
>>> if (IS_ENABLED(CONFIG_ROOTWAIT_BOOTARG) && blk->rootwait)
>>> rootarg = linux_bootargs_append_rootwait(rootarg);
>>>
>>> return rootarg;
>>> }
>>> -
>>> diff --git a/common/bootm.c b/common/bootm.c
>>> index 17792b2a1d..4549bfeb8b 100644
>>> --- a/common/bootm.c
>>> +++ b/common/bootm.c
>>> @@ -852,10 +852,10 @@ int bootm_boot(struct bootm_data *bootm_data)
>>> rootarg = ERR_PTR(-EINVAL);
>>>
>>> if (!root_cdev)
>>> - pr_err("no cdev found for %s, cannot
>>> set root= option\n",
>>> + pr_err("no cdev found for %s, cannot
>>> set "
>>> CONFIG_ROOT_COMMANDLINE_PARAMETER "= option\n",
>>> root_dev_name);
>>> else if (!root_cdev->partuuid[0])
>>> - pr_err("%s doesn't have a PARTUUID,
>>> cannot set root= option\n",
>>> + pr_err("%s doesn't have a PARTUUID,
>>> cannot set "
>>> CONFIG_ROOT_COMMANDLINE_PARAMETER "= option\n",
>>> root_dev_name);
>>> }
>>>
>>> @@ -866,7 +866,7 @@ int bootm_boot(struct bootm_data *bootm_data)
>>> }
>>>
>>> if (IS_ERR(rootarg)) {
>>> - pr_err("Failed to append kernel cmdline parameter
>>> 'root='\n");
>>> + pr_err("Failed to append kernel cmdline parameter '"
>>> CONFIG_ROOT_COMMANDLINE_PARAMETER
>>> "='\n");
>>> } else {
>>> pr_info("Adding \"%s\" to Kernel commandline\n",
>>> rootarg);
>>> globalvar_add_simple("linux.bootargs.bootm.appendroot",
>>> @@ -1165,7 +1165,7 @@ BAREBOX_MAGICVAR(global.bootm.dryrun, "bootm default
>>> dryrun level");
>>> BAREBOX_MAGICVAR(global.bootm.verify, "bootm default verify level");
>>> BAREBOX_MAGICVAR(global.bootm.verbose, "bootm default verbosity level
>>> (0=quiet)");
>>> BAREBOX_MAGICVAR(global.bootm.earlycon, "Add earlycon option to Kernel for
>>> early log output");
>>> -BAREBOX_MAGICVAR(global.bootm.appendroot, "Add root= option to Kernel to
>>> mount rootfs from the device the Kernel
>>> comes from (default, device can be overridden via global.bootm.root_dev)");
>>> +BAREBOX_MAGICVAR(global.bootm.appendroot, "Add "
>>> CONFIG_ROOT_COMMANDLINE_PARAMETER "= option to Kernel to mount
>>> rootfs from the device the Kernel comes from (default, device can be
>>> overridden via global.bootm.root_dev)");
>>> BAREBOX_MAGICVAR(global.bootm.root_dev, "bootm default root device
>>> (overrides default device in
>>> global.bootm.appendroot)");
>>> BAREBOX_MAGICVAR(global.bootm.provide_machine_id, "If true, append
>>> systemd.machine_id=$global.machine_id to Kernel
>>> command line");
>>> BAREBOX_MAGICVAR(global.bootm.provide_hostname, "If true, append
>>> systemd.hostname=$global.hostname to Kernel
>>> command line");
>>> diff --git a/drivers/mci/mci-core.c b/drivers/mci/mci-core.c
>>> index 57825bc849..79cebe19f3 100644
>>> --- a/drivers/mci/mci-core.c
>>> +++ b/drivers/mci/mci-core.c
>>> @@ -2691,7 +2691,7 @@ static char *mci_get_linux_mmcblkdev(struct
>>> block_device *blk,
>>> * skipping it.
>>> */
>>> if (cdev_partname_equal(partcdev, cdev))
>>> - return basprintf("root=/dev/mmcblk%dp%d", id, partnum);
>>> + return basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER
>>> "=/dev/mmcblk%dp%d", id, partnum);
>>> if (cdev->flags & DEVFS_PARTITION_FROM_TABLE)
>>> partnum++;
>>> }
>>> diff --git a/fs/9p/vfs_super.c b/fs/9p/vfs_super.c
>>> index 6a451d9ef5..39a8529a4e 100644
>>> --- a/fs/9p/vfs_super.c
>>> +++ b/fs/9p/vfs_super.c
>>> @@ -70,7 +70,7 @@ static void v9fs_set_rootarg(struct v9fs_session_info
>>> *v9ses,
>>> trans = v9ses->clnt->trans_mod->name;
>>> path = v9ses->aname;
>>>
>>> - str = basprintf("root=%s rootfstype=9p rootflags=trans=%s,msize=%d,"
>>> + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=%s rootfstype=9p
>>> rootflags=trans=%s,msize=%d,"
>>> "cache=loose,uname=%s,dfltuid=0,dfltgid=0,aname=%s",
>>> tag, trans, v9ses->clnt->msize, tag, path);
>>>
>>> diff --git a/fs/nfs.c b/fs/nfs.c
>>> index 5c2476cd88..9db23c5d18 100644
>>> --- a/fs/nfs.c
>>> +++ b/fs/nfs.c
>>> @@ -1558,7 +1558,7 @@ static void nfs_set_rootarg(struct nfs_priv *npriv,
>>> struct fs_device *fsdev)
>>> char *str, *tmp;
>>> const char *bootargs;
>>>
>>> - str = basprintf("root=/dev/nfs nfsroot=%pI4:%s%s%s", &npriv->server,
>>> npriv->path,
>>> + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=/dev/nfs
>>> nfsroot=%pI4:%s%s%s", &npriv->server, npriv-
>>>> path,
>>> rootnfsopts[0] ? "," : "", rootnfsopts);
>>>
>>> /* forward specific mount options on demand */
>>> diff --git a/fs/squashfs/squashfs.c b/fs/squashfs/squashfs.c
>>> index e30372627a..8267e3cba0 100644
>>> --- a/fs/squashfs/squashfs.c
>>> +++ b/fs/squashfs/squashfs.c
>>> @@ -60,7 +60,7 @@ static void squashfs_set_rootarg(struct fs_device *fsdev)
>>> ubi_get_device_info(vi.ubi_num, &di);
>>> mtd = di.mtd;
>>>
>>> - str = basprintf("root=/dev/ubiblock%d_%d ubi.mtd=%s ubi.block=%d,%d
>>> rootfstype=squashfs",
>>> + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=/dev/ubiblock%d_%d
>>> ubi.mtd=%s ubi.block=%d,%d
>>> rootfstype=squashfs",
>>> vi.ubi_num, vi.vol_id, mtd->cdev.partname, vi.ubi_num,
>>> vi.vol_id);
>>>
>>> fsdev_set_linux_rootarg(fsdev, str);
>>> diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c
>>> index 37986acbb2..34151a0c9f 100644
>>> --- a/fs/ubifs/ubifs.c
>>> +++ b/fs/ubifs/ubifs.c
>>> @@ -442,7 +442,7 @@ static void ubifs_set_rootarg(struct ubifs_priv *priv,
>>>
>>> mtd = di.mtd;
>>>
>>> - str = basprintf("root=ubi0:%s ubi.mtd=%s rootfstype=ubifs",
>>> + str = basprintf(CONFIG_ROOT_COMMANDLINE_PARAMETER "=ubi0:%s ubi.mtd=%s
>>> rootfstype=ubifs",
>>> vi.name, mtd->cdev.partname);
>>>
>>> fsdev_set_linux_rootarg(fsdev, str);
>
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
