[PATCH 1/2] blspec: allow board files to overwrite config file settings

[PATCH 1/2] blspec: allow board files to overwrite config file settings

[Marco Felsch]

This commit adds the support to overwrite and/or adapt blspec config
files by the following global variables:
  - global.blspec.fixup.devicetree
  - global.blspec.fixup.initrd
  - global.blspec.fixup.options
  - global.blspec.fixup.linux
  - global.blspec.fixup.devicetree-overlay.

Overwriting blspec-config's shouldn't be the normal case but there are
cases where this support is needed. One use-case can be a special
handling during update. E.g. the normal boot-flow don't need the initrd
but the update-flow uses it because the update system is on the initrd.
Another use-case could be a dynamic devicetree-overlay handling. E.g.
the board code dynamically checks which periphery boards are connected
and adds the required overlays dynamically.

A warning is printed if a already set option is overwritten to identify
unintended wrong boot behaviour early.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/blspec.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/common/blspec.c b/common/blspec.c
index a07343f427..b527731a27 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -210,12 +210,21 @@ err_out:
  */
 const char *blspec_entry_var_get(struct blspec_entry *entry, const char *name)
 {
+	const char *match, *fixup;
 	const char *str;
 	int ret;
 
 	ret = of_property_read_string(entry->node, name, &str);
 
-	return ret ? NULL : str;
+	/* Check user specific overrides */
+	match = basprintf("global.blspec.fixup.%s", name);
+	fixup = getenv_nonempty(match);
+	free((void *)match);
+
+	if (!ret && fixup)
+		pr_warn("Overwrite %s blspec entry by fixup\n", name);
+
+	return fixup ? fixup : (ret ? NULL : str);
 }
 
 static void blspec_entry_free(struct bootentry *be)
-- 
2.20.1


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

[PATCH 2/2] Documentation: blspec: Add blspec fixup documentation

[Marco Felsch]

Document the fixup mechanism.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 Documentation/user/booting-linux.rst | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
index 60babb513c..fd7575bbe9 100644
--- a/Documentation/user/booting-linux.rst
+++ b/Documentation/user/booting-linux.rst
@@ -232,6 +232,28 @@ device where the entry is found on. This makes it possible to use the same rootf
 image on different devices without having to specify a different root= option each
 time.
 
+The following environment variables can be used to update the bootloader spec entry
+values on the fly.
+
+.. warning::
+
+  In most use cases this is not necessary. Modifying the bootloader spec values on
+  the fly can trigger unwanted boot behaviours.
+
++-----------------------------------------+
+| name                                    |
++=========================================+
+| global.blspec.fixup.devicetree          |
++-----------------------------------------+
+| global.blspec.fixup.initrd              |
++-----------------------------------------+
+| global.blspec.fixup.options             |
++-----------------------------------------+
+| global.blspec.fixup.linux               |
++-----------------------------------------+
+| global.blspec.fixup.devicetree-overlay  |
++-----------------------------------------+
+
 Network boot
 ------------
 
-- 
2.20.1


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

Re: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Sascha Hauer]

Hi Marco,
On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> This commit adds the support to overwrite and/or adapt blspec config
> files by the following global variables:
>   - global.blspec.fixup.devicetree
>   - global.blspec.fixup.initrd
>   - global.blspec.fixup.options
>   - global.blspec.fixup.linux
>   - global.blspec.fixup.devicetree-overlay.
> 
> Overwriting blspec-config's shouldn't be the normal case but there are
> cases where this support is needed. One use-case can be a special
> handling during update. E.g. the normal boot-flow don't need the initrd
> but the update-flow uses it because the update system is on the initrd.

When you have an update system and a regular system on the same
filesystem then I would expect an additional bootspec entry for the
update system.

> Another use-case could be a dynamic devicetree-overlay handling. E.g.
> the board code dynamically checks which periphery boards are connected
> and adds the required overlays dynamically.

Board code can register overlays already, there's no need for bootspec
to do that.

Overall overwriting bootspec variables somewhat contradicts the whole
purpose of bootspec. I don't like that very much, so you'll need very
good reasons to get this through ;)

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: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Marco Felsch]

Hi Sascha,

On 21-01-19 12:01, Sascha Hauer wrote:
> Hi Marco,
> On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> > This commit adds the support to overwrite and/or adapt blspec config
> > files by the following global variables:
> >   - global.blspec.fixup.devicetree
> >   - global.blspec.fixup.initrd
> >   - global.blspec.fixup.options
> >   - global.blspec.fixup.linux
> >   - global.blspec.fixup.devicetree-overlay.
> > 
> > Overwriting blspec-config's shouldn't be the normal case but there are
> > cases where this support is needed. One use-case can be a special
> > handling during update. E.g. the normal boot-flow don't need the initrd
> > but the update-flow uses it because the update system is on the initrd.
> 
> When you have an update system and a regular system on the same
> filesystem then I would expect an additional bootspec entry for the
> update system.

AFAIK, barebox tries to find the correct entry by checking all config
files and loads the first matching config file. Is there a way to
specify an explicite config file?

> > Another use-case could be a dynamic devicetree-overlay handling. E.g.
> > the board code dynamically checks which periphery boards are connected
> > and adds the required overlays dynamically.
> 
> Board code can register overlays already, there's no need for bootspec
> to do that.

But this needs a lot more handling if we boot from nfs or from emmc.
Therefore I utilized the already existing code :)

> Overall overwriting bootspec variables somewhat contradicts the whole
> purpose of bootspec. I don't like that very much, so you'll need very
> good reasons to get this through ;)

Yep, I thought so ;) Therefore I wrote a big warning. It gives you a lot
opportunities to shot yourself in the foot. Therefore I print a warning
if you overwrite a already exisiting config.

Regards,
  Marco

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

Re: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Sascha Hauer]

On Tue, Jan 19, 2021 at 12:42:08PM +0100, Marco Felsch wrote:
> Hi Sascha,
> 
> On 21-01-19 12:01, Sascha Hauer wrote:
> > Hi Marco,
> > On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> > > This commit adds the support to overwrite and/or adapt blspec config
> > > files by the following global variables:
> > >   - global.blspec.fixup.devicetree
> > >   - global.blspec.fixup.initrd
> > >   - global.blspec.fixup.options
> > >   - global.blspec.fixup.linux
> > >   - global.blspec.fixup.devicetree-overlay.
> > > 
> > > Overwriting blspec-config's shouldn't be the normal case but there are
> > > cases where this support is needed. One use-case can be a special
> > > handling during update. E.g. the normal boot-flow don't need the initrd
> > > but the update-flow uses it because the update system is on the initrd.
> > 
> > When you have an update system and a regular system on the same
> > filesystem then I would expect an additional bootspec entry for the
> > update system.
> 
> AFAIK, barebox tries to find the correct entry by checking all config
> files and loads the first matching config file. Is there a way to
> specify an explicite config file?

Not yet, but maybe we are at a point where a way should be added.

> 
> > > Another use-case could be a dynamic devicetree-overlay handling. E.g.
> > > the board code dynamically checks which periphery boards are connected
> > > and adds the required overlays dynamically.
> > 
> > Board code can register overlays already, there's no need for bootspec
> > to do that.
> 
> But this needs a lot more handling if we boot from nfs or from emmc.
> Therefore I utilized the already existing code :)

I'm not sure I understand you correctly. You probably mean that you have
to decide which overlay to use when the path to the rootfs is already
known, right?

> 
> > Overall overwriting bootspec variables somewhat contradicts the whole
> > purpose of bootspec. I don't like that very much, so you'll need very
> > good reasons to get this through ;)
> 
> Yep, I thought so ;) Therefore I wrote a big warning. It gives you a lot
> opportunities to shot yourself in the foot. Therefore I print a warning
> if you overwrite a already exisiting config.

Your code shouldn't issue a warning when it's exactly used like
intended. A warning tells people they are doing something wrong, but in
this case they are doing everything correctly, so they might be tempted
to just patch the warning away.

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: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Marco Felsch]

Hi Sascha,

On 21-01-20 10:03, Sascha Hauer wrote:
> On Tue, Jan 19, 2021 at 12:42:08PM +0100, Marco Felsch wrote:
> > Hi Sascha,
> > 
> > On 21-01-19 12:01, Sascha Hauer wrote:
> > > Hi Marco,
> > > On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> > > > This commit adds the support to overwrite and/or adapt blspec config
> > > > files by the following global variables:
> > > >   - global.blspec.fixup.devicetree
> > > >   - global.blspec.fixup.initrd
> > > >   - global.blspec.fixup.options
> > > >   - global.blspec.fixup.linux
> > > >   - global.blspec.fixup.devicetree-overlay.
> > > > 
> > > > Overwriting blspec-config's shouldn't be the normal case but there are
> > > > cases where this support is needed. One use-case can be a special
> > > > handling during update. E.g. the normal boot-flow don't need the initrd
> > > > but the update-flow uses it because the update system is on the initrd.
> > > 
> > > When you have an update system and a regular system on the same
> > > filesystem then I would expect an additional bootspec entry for the
> > > update system.
> > 
> > AFAIK, barebox tries to find the correct entry by checking all config
> > files and loads the first matching config file. Is there a way to
> > specify an explicite config file?
> 
> Not yet, but maybe we are at a point where a way should be added.

Maybe but how do you decide it? Also IMHO this wouldn't be that flexible
as this solution.

> > 
> > > > Another use-case could be a dynamic devicetree-overlay handling. E.g.
> > > > the board code dynamically checks which periphery boards are connected
> > > > and adds the required overlays dynamically.
> > > 
> > > Board code can register overlays already, there's no need for bootspec
> > > to do that.
> > 
> > But this needs a lot more handling if we boot from nfs or from emmc.
> > Therefore I utilized the already existing code :)
> 
> I'm not sure I understand you correctly. You probably mean that you have
> to decide which overlay to use when the path to the rootfs is already
> known, right?

Hopefully I get it right. What I mean is that the blspec code already
determines the correct boot medium and all blspec entries are relative
to this boot medium. Therefore I love it since booting a device behaves
the same for nfs or emmc :)

So yes, you're right :)

> > > Overall overwriting bootspec variables somewhat contradicts the whole
> > > purpose of bootspec. I don't like that very much, so you'll need very
> > > good reasons to get this through ;)
> > 
> > Yep, I thought so ;) Therefore I wrote a big warning. It gives you a lot
> > opportunities to shot yourself in the foot. Therefore I print a warning
> > if you overwrite a already exisiting config.
> 
> Your code shouldn't issue a warning when it's exactly used like
> intended. A warning tells people they are doing something wrong, but in
> this case they are doing everything correctly, so they might be tempted
> to just patch the warning away.

Mh.. The blspec config shouldn't contain any dynamic entry, therefore I
added the warning. E.g. on my system we check for the connected daughter
boards and add the devicetree-overlay entry. The original blspec-config
file don't have a entry for it.

Regards,
  Marco

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

Re: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Michael Olbrich]

On Wed, Jan 20, 2021 at 11:54:22AM +0100, Marco Felsch wrote:
> On 21-01-20 10:03, Sascha Hauer wrote:
> > On Tue, Jan 19, 2021 at 12:42:08PM +0100, Marco Felsch wrote:
> > > Hi Sascha,
> > > 
> > > On 21-01-19 12:01, Sascha Hauer wrote:
> > > > Hi Marco,
> > > > On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> > > > > This commit adds the support to overwrite and/or adapt blspec config
> > > > > files by the following global variables:
> > > > >   - global.blspec.fixup.devicetree
> > > > >   - global.blspec.fixup.initrd
> > > > >   - global.blspec.fixup.options
> > > > >   - global.blspec.fixup.linux
> > > > >   - global.blspec.fixup.devicetree-overlay.
> > > > > 
> > > > > Overwriting blspec-config's shouldn't be the normal case but there are
> > > > > cases where this support is needed. One use-case can be a special
> > > > > handling during update. E.g. the normal boot-flow don't need the initrd
> > > > > but the update-flow uses it because the update system is on the initrd.
> > > > 
> > > > When you have an update system and a regular system on the same
> > > > filesystem then I would expect an additional bootspec entry for the
> > > > update system.
> > > 
> > > AFAIK, barebox tries to find the correct entry by checking all config
> > > files and loads the first matching config file. Is there a way to
> > > specify an explicite config file?
> > 
> > Not yet, but maybe we are at a point where a way should be added.
> 
> Maybe but how do you decide it? Also IMHO this wouldn't be that flexible
> as this solution.

Maybe allow globing?

So I can say I want to boot 'default.conf' and create that as a symlink in
the filesystem. That would be for the desktop use-case with multiple kernel
versions.

Or I could switch between '*-regular.conf' and '*-update.conf'. And then
provide two entries for each device-tree.

Michael


-- 
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: [PATCH 1/2] blspec: allow board files to overwrite config file settings

[Marco Felsch]

On 21-01-20 13:31, Michael Olbrich wrote:
> On Wed, Jan 20, 2021 at 11:54:22AM +0100, Marco Felsch wrote:
> > On 21-01-20 10:03, Sascha Hauer wrote:
> > > On Tue, Jan 19, 2021 at 12:42:08PM +0100, Marco Felsch wrote:
> > > > Hi Sascha,
> > > > 
> > > > On 21-01-19 12:01, Sascha Hauer wrote:
> > > > > Hi Marco,
> > > > > On Mon, Jan 18, 2021 at 09:48:49PM +0100, Marco Felsch wrote:
> > > > > > This commit adds the support to overwrite and/or adapt blspec config
> > > > > > files by the following global variables:
> > > > > >   - global.blspec.fixup.devicetree
> > > > > >   - global.blspec.fixup.initrd
> > > > > >   - global.blspec.fixup.options
> > > > > >   - global.blspec.fixup.linux
> > > > > >   - global.blspec.fixup.devicetree-overlay.
> > > > > > 
> > > > > > Overwriting blspec-config's shouldn't be the normal case but there are
> > > > > > cases where this support is needed. One use-case can be a special
> > > > > > handling during update. E.g. the normal boot-flow don't need the initrd
> > > > > > but the update-flow uses it because the update system is on the initrd.
> > > > > 
> > > > > When you have an update system and a regular system on the same
> > > > > filesystem then I would expect an additional bootspec entry for the
> > > > > update system.
> > > > 
> > > > AFAIK, barebox tries to find the correct entry by checking all config
> > > > files and loads the first matching config file. Is there a way to
> > > > specify an explicite config file?
> > > 
> > > Not yet, but maybe we are at a point where a way should be added.
> > 
> > Maybe but how do you decide it? Also IMHO this wouldn't be that flexible
> > as this solution.
> 
> Maybe allow globing?
> 
> So I can say I want to boot 'default.conf' and create that as a symlink in
> the filesystem. That would be for the desktop use-case with multiple kernel
> versions.

That would be a solution. But...

> Or I could switch between '*-regular.conf' and '*-update.conf'. And then
> provide two entries for each device-tree.

.. if I wanna cover all platforms I have at least 2 platform confs for
each platform. The reason for that is because different platforms using
different overlays. By platform I mean a different variant of the same
platform family. Therefore I use my abbroach to patch the needed
overlays live and the platform is always the same.

Regards,
  Marco

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