[RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Robert Karszniewicz]

This patch introduces a new env var which specifies which device
is the rootfs device to be used in Linux, passed to Linux via bootargs,
identified by the rootfs partition's PARTUUID.

global.bootm.root supplements global.bootm.appendroot, in that it overrides
appendroot's naïve default, which picks the partition that the kernel resides
on (global.bootm.image).

I don't know if it is the right way, or a good way, but this is the shortest
and simplest way that I've found.

What do you think of this? And is it generally something that would be
accepted, or is this out of scope for barebox?

Example:
detect mmc2
global.bootm.image='/mnt/mmc2.0/zImage'
global.bootm.appendroot=1
global.bootm.root='/mnt/mmc2.1/'
boot mmc

Note that the trailing slash is important.
See comment in follow_automount() in fs/fs.c

Robert Karszniewicz (4):
  bootm: add env var root_dev
  globalvar: add bootm.root
  bootm: handle global.bootm.root
  bootm: mount root device before accessing linux_rootarg

 common/bootm.c  | 13 +++++++++++--
 include/bootm.h |  2 ++
 2 files changed, 13 insertions(+), 2 deletions(-)

-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 1/4] bootm: add env var root_dev

[Robert Karszniewicz]

---
 common/bootm.c  | 4 +++-
 include/bootm.h | 2 ++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index 8fec1ee34dd8..af9f9b8f447d 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -64,6 +64,7 @@ void bootm_data_init_defaults(struct bootm_data *data)
 	getenv_ul("global.bootm.image.loadaddr", &data->os_address);
 	getenv_ul("global.bootm.initrd.loadaddr", &data->initrd_address);
 	data->initrd_file = getenv_nonempty("global.bootm.initrd");
+	data->root_dev = getenv_nonempty("global.bootm.root");
 	data->verify = bootm_get_verify_mode();
 	data->appendroot = bootm_appendroot;
 	data->provide_machine_id = bootm_provide_machine_id;
@@ -771,5 +772,6 @@ BAREBOX_MAGICVAR_NAMED(global_bootm_oftree, global.bootm.oftree, "bootm default
 BAREBOX_MAGICVAR_NAMED(global_bootm_tee, global.bootm.tee, "bootm default tee image");
 BAREBOX_MAGICVAR_NAMED(global_bootm_verify, global.bootm.verify, "bootm default verify level");
 BAREBOX_MAGICVAR_NAMED(global_bootm_verbose, global.bootm.verbose, "bootm default verbosity level (0=quiet)");
-BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, global.bootm.appendroot, "Add root= option to Kernel to mount rootfs from the device the Kernel comes from");
+BAREBOX_MAGICVAR_NAMED(global_bootm_appendroot, 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)");
+BAREBOX_MAGICVAR_NAMED(global_bootm_root, global.bootm.root, "bootm default root device (overrides default device in global.bootm.appendroot)");
 BAREBOX_MAGICVAR_NAMED(global_bootm_provide_machine_id, global.bootm.provide_machine_id, "If true, add systemd.machine_id= with value of global.machine_id to Kernel");
diff --git a/include/bootm.h b/include/bootm.h
index 7782de7a476d..3628a7eea5b9 100644
--- a/include/bootm.h
+++ b/include/bootm.h
@@ -18,6 +18,7 @@ struct bootm_data {
 	const char *initrd_file;
 	const char *oftree_file;
 	const char *tee_file;
+	const char *root_dev;
 	int verbose;
 	enum bootm_verify verify;
 	bool force;
@@ -25,6 +26,7 @@ struct bootm_data {
 	/*
 	 * appendroot - if true, try to add a suitable root= Kernel option to
 	 * mount the rootfs from the same device as the Kernel comes from.
+	 * The default rootfs device can be overridden with root_dev.
 	 */
 	bool appendroot;
 	/*
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 2/4] globalvar: add bootm.root

[Robert Karszniewicz]

---
 common/bootm.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/bootm.c b/common/bootm.c
index af9f9b8f447d..3772fa4c2c51 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -743,6 +743,7 @@ static int bootm_init(void)
 	globalvar_add_simple("bootm.image", NULL);
 	globalvar_add_simple("bootm.image.loadaddr", NULL);
 	globalvar_add_simple("bootm.oftree", NULL);
+	globalvar_add_simple("bootm.root", NULL);
 	globalvar_add_simple("bootm.tee", NULL);
 	globalvar_add_simple_bool("bootm.appendroot", &bootm_appendroot);
 	globalvar_add_simple_bool("bootm.provide_machine_id", &bootm_provide_machine_id);
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 3/4] bootm: handle global.bootm.root

[Robert Karszniewicz]

---
 common/bootm.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index 3772fa4c2c51..1a9ef0673e55 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -656,7 +656,11 @@ int bootm_boot(struct bootm_data *bootm_data)
 	if (bootm_data->appendroot) {
 		char *rootarg;
 
-		rootarg = path_get_linux_rootarg(data->os_file);
+		if (bootm_data->root_dev) {
+			rootarg = path_get_linux_rootarg(bootm_data->root_dev);
+		} else {
+			rootarg = path_get_linux_rootarg(data->os_file);
+		}
 		if (!IS_ERR(rootarg)) {
 			printf("Adding \"%s\" to Kernel commandline\n", rootarg);
 			globalvar_add_simple("linux.bootargs.bootm.appendroot",
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[RFC PATCH 4/4] bootm: mount root device before accessing linux_rootarg

[Robert Karszniewicz]

---
 common/bootm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/common/bootm.c b/common/bootm.c
index 1a9ef0673e55..b7c15cac0ba4 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -657,6 +657,8 @@ int bootm_boot(struct bootm_data *bootm_data)
 		char *rootarg;
 
 		if (bootm_data->root_dev) {
+			struct stat s;
+			stat(bootm_data->root_dev, &s); /* mount root device */
 			rootarg = path_get_linux_rootarg(bootm_data->root_dev);
 		} else {
 			rootarg = path_get_linux_rootarg(data->os_file);
-- 
2.7.4


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Robert Karszniewicz]

The problem is that we want to be able to have the rootfs and kernel on 
separate partitions. We've looked into the Boot Loader Specification, 
but from what we saw, it makes A-B systems difficult (according to the 
spec, there can only be one "$BOOT" filesystem on a device).

For reference: https://systemd.io/BOOT_LOADER_SPECIFICATION/

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Sascha Hauer]

Hi Robert,

On Tue, Jun 30, 2020 at 04:20:17PM +0200, Robert Karszniewicz wrote:
> The problem is that we want to be able to have the rootfs and kernel on
> separate partitions.

Why do you want to have that? It's kind of traditional to have the
kernel separated from the rootfs in some extra "kernel" partition, but
are there good reasons for it? There's an overhead in that the
bootloader has to read from a filesystem instead of only a raw
partition. Is that the reason?

Here at Pengutronix we are happy that we only have one partition image
that has everything needed to boot, including a description how to boot
it and including the kernel. No extra items that the bootloader has to
take care of, just put one thing somewhere and be done with it.

> We've looked into the Boot Loader Specification, but
> from what we saw, it makes A-B systems difficult (according to the spec,
> there can only be one "$BOOT" filesystem on a device).

barebox is more relaxed here. What we do here is to put two full root
filesystems into two different partitions on a SD/MMC. Each of the
partitions has one or more /loader/entries/*.conf file(s) and kernels.
You can then boot with "boot mmc0.0" into the first rootfs or with "boot
mmc0.1" into the second.
This may not be really conform to the specification, but works in
barebox and is a supported usecase. We do this for A/B Boot scenarios in
many projects.

Regards,
  Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Stefan Riedmüller]

Hi Sascha,

let me answer on behalf of Robert.

On 01.07.20 07:58, Sascha Hauer wrote:
> Hi Robert,
> 
> On Tue, Jun 30, 2020 at 04:20:17PM +0200, Robert Karszniewicz wrote:
>> The problem is that we want to be able to have the rootfs and kernel on
>> separate partitions.
> 
> Why do you want to have that? It's kind of traditional to have the
> kernel separated from the rootfs in some extra "kernel" partition, but
> are there good reasons for it? There's an overhead in that the
> bootloader has to read from a filesystem instead of only a raw
> partition. Is that the reason?

Actually the reason is that we have this separate setup for quite some time 
now in the field and changing it would have a whole string of consequences 
attached to it. So we try not to change this setup, at least for existing 
systems.

That is why we would like to be able to get the PARTUUID from another 
Partition than the one containing the kernel image.

What do you think of Roberts proposal?

> 
> Here at Pengutronix we are happy that we only have one partition image
> that has everything needed to boot, including a description how to boot
> it and including the kernel. No extra items that the bootloader has to
> take care of, just put one thing somewhere and be done with it.

This sounds like a good thing to have and I think we should consider this 
for new platforms.

Regards,
Stefan

> 
>> We've looked into the Boot Loader Specification, but
>> from what we saw, it makes A-B systems difficult (according to the spec,
>> there can only be one "$BOOT" filesystem on a device).
> 
> barebox is more relaxed here. What we do here is to put two full root
> filesystems into two different partitions on a SD/MMC. Each of the
> partitions has one or more /loader/entries/*.conf file(s) and kernels.
> You can then boot with "boot mmc0.0" into the first rootfs or with "boot
> mmc0.1" into the second.
> This may not be really conform to the specification, but works in
> barebox and is a supported usecase. We do this for A/B Boot scenarios in
> many projects.
> 
> Regards,
>    Sascha
> 

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Sascha Hauer]

On Tue, Jun 30, 2020 at 03:48:30PM +0200, Robert Karszniewicz wrote:
> This patch introduces a new env var which specifies which device
> is the rootfs device to be used in Linux, passed to Linux via bootargs,
> identified by the rootfs partition's PARTUUID.
> 
> global.bootm.root supplements global.bootm.appendroot, in that it overrides
> appendroot's naïve default, which picks the partition that the kernel resides
> on (global.bootm.image).
> 
> I don't know if it is the right way, or a good way, but this is the shortest
> and simplest way that I've found.
> 
> What do you think of this? And is it generally something that would be
> accepted, or is this out of scope for barebox?
> 
> Example:
> detect mmc2
> global.bootm.image='/mnt/mmc2.0/zImage'
> global.bootm.appendroot=1
> global.bootm.root='/mnt/mmc2.1/'

Why do you pass the standard mount path here? I would expect /dev/mmc2.1.

In 4/4 you mount the root device. I think this should be avoided as it
only works when barebox has support for the rootfs, i.e. it doesn't work
for XFS or the like.

Ok, fsdev_set_linux_rootarg() is tied to a filesystem, so maybe we need
something similar for a cdev.

Generally I think barebox should support this usecase, but I am not
convinced the approach you took is the right API.

Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Robert Karszniewicz]

On 7/9/20 4:14 PM, Sascha Hauer wrote:
> On Tue, Jun 30, 2020 at 03:48:30PM +0200, Robert Karszniewicz wrote:
>> This patch introduces a new env var which specifies which device
>> is the rootfs device to be used in Linux, passed to Linux via bootargs,
>> identified by the rootfs partition's PARTUUID.
>>
>> global.bootm.root supplements global.bootm.appendroot, in that it overrides
>> appendroot's naïve default, which picks the partition that the kernel resides
>> on (global.bootm.image).
>>
>> I don't know if it is the right way, or a good way, but this is the shortest
>> and simplest way that I've found.
>>
>> What do you think of this? And is it generally something that would be
>> accepted, or is this out of scope for barebox?
>>
>> Example:
>> detect mmc2
>> global.bootm.image='/mnt/mmc2.0/zImage'
>> global.bootm.appendroot=1
>> global.bootm.root='/mnt/mmc2.1/'
> 

Generally, this was just the shortest possible way to make it basically 
work, before I knew if such a feature would be accepted at all.

> Why do you pass the standard mount path here? I would expect /dev/mmc2.1.
> 

Agree.

> In 4/4 you mount the root device. I think this should be avoided as it

Agree.

> only works when barebox has support for the rootfs, i.e. it doesn't work
> for XFS or the like.
> 

I see.

> Ok, fsdev_set_linux_rootarg() is tied to a filesystem, so maybe we need
> something similar for a cdev.
> 
> Generally I think barebox should support this usecase, but I am not
> convinced the approach you took is the right API.
> 

So do I understand it correctly, that doing it via a new 'root_dev' 
variable is fine, just that the implementation needs to be better?

> Sascha
> 

Robert

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [RFC PATCH 0/4] Introduce global.bootm.root env var for booting via PARTUUID

[Sascha Hauer]

On Mon, Jul 13, 2020 at 01:18:04PM +0200, Robert Karszniewicz wrote:
> On 7/9/20 4:14 PM, Sascha Hauer wrote:
> > On Tue, Jun 30, 2020 at 03:48:30PM +0200, Robert Karszniewicz wrote:
> > > This patch introduces a new env var which specifies which device
> > > is the rootfs device to be used in Linux, passed to Linux via bootargs,
> > > identified by the rootfs partition's PARTUUID.
> > > 
> > > global.bootm.root supplements global.bootm.appendroot, in that it overrides
> > > appendroot's naïve default, which picks the partition that the kernel resides
> > > on (global.bootm.image).
> > > 
> > > I don't know if it is the right way, or a good way, but this is the shortest
> > > and simplest way that I've found.
> > > 
> > > What do you think of this? And is it generally something that would be
> > > accepted, or is this out of scope for barebox?
> > > 
> > > Example:
> > > detect mmc2
> > > global.bootm.image='/mnt/mmc2.0/zImage'
> > > global.bootm.appendroot=1
> > > global.bootm.root='/mnt/mmc2.1/'
> > 
> 
> Generally, this was just the shortest possible way to make it basically
> work, before I knew if such a feature would be accepted at all.
> 
> > Why do you pass the standard mount path here? I would expect /dev/mmc2.1.
> > 
> 
> Agree.
> 
> > In 4/4 you mount the root device. I think this should be avoided as it
> 
> Agree.
> 
> > only works when barebox has support for the rootfs, i.e. it doesn't work
> > for XFS or the like.
> > 
> 
> I see.
> 
> > Ok, fsdev_set_linux_rootarg() is tied to a filesystem, so maybe we need
> > something similar for a cdev.
> > 
> > Generally I think barebox should support this usecase, but I am not
> > convinced the approach you took is the right API.
> > 
> 
> So do I understand it correctly, that doing it via a new 'root_dev' variable
> is fine, just that the implementation needs to be better?

Yes.

Sascha

-- 
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 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox