[PATCH] blspec: add checking of optional key machine-id

[PATCH] blspec: add checking of optional key machine-id

[Andreas Schmidt]

For filtering of Bootloader Spec entries, Bootloader Spec
specify an optional key machine-id. By set the
global.boot.machine-id variable the checking of machine-id
key in Bootloader Spec entries will be activate. If the
variable and key match, appropriate boot entry will be
booting. If it not match boot entry will be ignore and
barebox check the next boot entry.

Signed-off-by: Andreas Schmidt <mail@schmidt-andreas.de>
---
Hi,
we use the same barebox for all our devices. The devices have
same CPU-module but different interface boards. So the bootloader
has to choose the right boot entry. To determine the right boot entry we
decide to use Bootloader Spec. But we have a issue: Currently, only oftree 
compatibility is checking by barebox for Bootloader Spec entries. But all 
our ofstrees are compatible with our barebox, because we use only one. So
we need a additional possibility to check witch Bootloader Spec entry
is the right one for appropriate device. I guess, for such an use case
Bootloader Spec specify "machine-id" key. This patch implement the support
for machine-id key.
The machine-id key is optional. I decide to activate the checking of this
key only if "global.boot.machine_id" variable is set to non-empty value.
If "global.boot.machine_id" not set machine-id key will be ignored, 
independent if it exists in Bootloader Spec entry or not.

Regards,
Andreas

---
 Documentation/user/booting-linux.rst |  6 ++++++
 common/blspec.c                      | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
index b5e4320ef..408f87d8e 100644
--- a/Documentation/user/booting-linux.rst
+++ b/Documentation/user/booting-linux.rst
@@ -212,6 +212,12 @@ The entry can be listed with the -l option:
 When on barebox the SD card shows up as ``mmc1`` then this entry can be booted with
 ``boot mmc1`` or with setting ``global.boot.default`` to ``mmc1``.
 
+``machine-id`` is an optional key. If ``global.boot.machine_id`` variable is set to
+non-empty value, then barebox accepts only Bootloader Spec entries with ``machine-id``
+key. In case if value of global variable and Bootloader Spec key match each other,
+barebox will choose the boot entry for booting. All other Bootloader Spec entries will
+be ignored.
+
 A bootloader spec entry can also reside on an NFS server in which case a RFC2224
 compatible NFS URI string must be passed to the boot command:
 
diff --git a/common/blspec.c b/common/blspec.c
index 6171461a7..4ffb25dd6 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -419,6 +419,30 @@ out:
 	return ret;
 }
 
+/*
+ * entry_is_match_machine_id - check if a bootspec entry is match with
+ *                            the machine id given by global variable.
+ *
+ * returns true if the entry is match, false otherwise
+ */
+
+static bool entry_is_match_machine_id(struct blspec_entry *entry)
+{
+	int ret = true;
+	const char *env_machineid = getenv_nonempty("global.boot.machine_id");
+
+	if (env_machineid) {
+		const char *machineid = blspec_entry_var_get(entry, "machine-id");
+		if (!machineid || strcmp(machineid, env_machineid)) {
+			pr_info("ignoring entry with missmatched machine-id \"%s\"\n",
+				env_machineid);
+			ret = false;
+		}
+	}
+
+	return ret;
+}
+
 /*
  * blspec_scan_directory - scan over a directory
  *
@@ -504,6 +528,11 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 			continue;
 		}
 
+		if (!entry_is_match_machine_id(entry)) {
+			blspec_entry_free(&entry->entry);
+			continue;
+		}
+
 		found++;
 
 		if (entry->cdev && entry->cdev->dev) {
@@ -756,4 +785,4 @@ static int blspec_init(void)
 {
 	return bootentry_register_provider(blspec_bootentry_provider);
 }
-device_initcall(blspec_init);
\ No newline at end of file
+device_initcall(blspec_init);
-- 
2.14.1


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

Re: [PATCH] blspec: add checking of optional key machine-id

[Sascha Hauer]

Hi Andreas,

On Sun, Apr 29, 2018 at 06:01:25PM +0200, Andreas Schmidt wrote:
> For filtering of Bootloader Spec entries, Bootloader Spec
> specify an optional key machine-id. By set the
> global.boot.machine-id variable the checking of machine-id
> key in Bootloader Spec entries will be activate. If the
> variable and key match, appropriate boot entry will be
> booting. If it not match boot entry will be ignore and
> barebox check the next boot entry.
> 
> Signed-off-by: Andreas Schmidt <mail@schmidt-andreas.de>
> ---
> Hi,
> we use the same barebox for all our devices. The devices have
> same CPU-module but different interface boards. So the bootloader
> has to choose the right boot entry. To determine the right boot entry we
> decide to use Bootloader Spec. But we have a issue: Currently, only oftree 
> compatibility is checking by barebox for Bootloader Spec entries. But all 
> our ofstrees are compatible with our barebox, because we use only one. So
> we need a additional possibility to check witch Bootloader Spec entry
> is the right one for appropriate device. I guess, for such an use case
> Bootloader Spec specify "machine-id" key. This patch implement the support
> for machine-id key.
> The machine-id key is optional. I decide to activate the checking of this
> key only if "global.boot.machine_id" variable is set to non-empty value.
> If "global.boot.machine_id" not set machine-id key will be ignored, 
> independent if it exists in Bootloader Spec entry or not.

I like the idea and the patch looks mosty fine.

> +static bool entry_is_match_machine_id(struct blspec_entry *entry)
> +{
> +	int ret = true;
> +	const char *env_machineid = getenv_nonempty("global.boot.machine_id");
> +
> +	if (env_machineid) {
> +		const char *machineid = blspec_entry_var_get(entry, "machine-id");
> +		if (!machineid || strcmp(machineid, env_machineid)) {
> +			pr_info("ignoring entry with missmatched machine-id \"%s\"\n",
> +				env_machineid);

Don't you want to print the machine id of the current entry rather than
the desired machine id?

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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

[PATCH v2] blspec: add checking of optional key machine-id

[Andreas Schmidt]

For filtering of Bootloader Spec entries, Bootloader Spec
specify an optional key machine-id. By set the
global.boot.machine-id variable the checking of machine-id
key in Bootloader Spec entries will be activate. If the
variable and key match, appropriate boot entry will be
booting. If it not match boot entry will be ignore and
barebox check the next boot entry.

Signed-off-by: Andreas Schmidt <mail@schmidt-andreas.de>
---

Changes since v1:
  - change pr_info to pr_debug in case of missmatching this machine-id
  - print both machine-ids in debug message

Hi Sascha,
yes you are right. The message has to be improve for better understanding.
I oriented me on device-tree compatibility and used the equal message.

While improving I realised, that my decision to use pr_info was not very well.
Because, in case of device-tree compatibility the usual case is matching. 
But in case of Bootloader Spec entries with machine-id only one of X entries
will/shuld match usaully. Bootloader would print some unnecessary info messages
each boot.

I changed the output to pr_debug, to avoid unnecessary outputs while normal
booting. And I add both machine ids to the message to make debuging easier.

Regards,
Andreas
---
 Documentation/user/booting-linux.rst |  6 ++++++
 common/blspec.c                      | 31 ++++++++++++++++++++++++++++++-
 2 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/Documentation/user/booting-linux.rst b/Documentation/user/booting-linux.rst
index b5e4320ef..408f87d8e 100644
--- a/Documentation/user/booting-linux.rst
+++ b/Documentation/user/booting-linux.rst
@@ -212,6 +212,12 @@ The entry can be listed with the -l option:
 When on barebox the SD card shows up as ``mmc1`` then this entry can be booted with
 ``boot mmc1`` or with setting ``global.boot.default`` to ``mmc1``.
 
+``machine-id`` is an optional key. If ``global.boot.machine_id`` variable is set to
+non-empty value, then barebox accepts only Bootloader Spec entries with ``machine-id``
+key. In case if value of global variable and Bootloader Spec key match each other,
+barebox will choose the boot entry for booting. All other Bootloader Spec entries will
+be ignored.
+
 A bootloader spec entry can also reside on an NFS server in which case a RFC2224
 compatible NFS URI string must be passed to the boot command:
 
diff --git a/common/blspec.c b/common/blspec.c
index 6171461a7..2c682e199 100644
--- a/common/blspec.c
+++ b/common/blspec.c
@@ -419,6 +419,30 @@ out:
 	return ret;
 }
 
+/*
+ * entry_is_match_machine_id - check if a bootspec entry is match with
+ *                            the machine id given by global variable.
+ *
+ * returns true if the entry is match, false otherwise
+ */
+
+static bool entry_is_match_machine_id(struct blspec_entry *entry)
+{
+	int ret = true;
+	const char *env_machineid = getenv_nonempty("global.boot.machine_id");
+
+	if (env_machineid) {
+		const char *machineid = blspec_entry_var_get(entry, "machine-id");
+		if (!machineid || strcmp(machineid, env_machineid)) {
+			pr_debug("ignoring entry with missmatched machine-id " \
+				"\"%s\" != \"%s\"\n", env_machineid, machineid);
+			ret = false;
+		}
+	}
+
+	return ret;
+}
+
 /*
  * blspec_scan_directory - scan over a directory
  *
@@ -504,6 +528,11 @@ int blspec_scan_directory(struct bootentries *bootentries, const char *root)
 			continue;
 		}
 
+		if (!entry_is_match_machine_id(entry)) {
+			blspec_entry_free(&entry->entry);
+			continue;
+		}
+
 		found++;
 
 		if (entry->cdev && entry->cdev->dev) {
@@ -756,4 +785,4 @@ static int blspec_init(void)
 {
 	return bootentry_register_provider(blspec_bootentry_provider);
 }
-device_initcall(blspec_init);
\ No newline at end of file
+device_initcall(blspec_init);
-- 
2.14.1


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

Re: [PATCH] blspec: add checking of optional key machine-id

[Jan Lübbe]

On Sun, 2018-04-29 at 18:01 +0200, Andreas Schmidt wrote:
> I guess, for such an use case Bootloader Spec specify "machine-id"
> key. This patch implement the support for machine-id key.

The way I read https://www.freedesktop.org/wiki/Specifications/BootLoad
erSpec/ is that the "machine-id" field should contain the ID from
/etc/machine-id, which is basically a UUID:
"machine-id shall contain the machine ID of the OS /etc/machine-id.
This is useful for boot loaders and applications to filter out boot
entries, for example to show only a single newest kernel per OS, or to
group items by OS, or to maybe filter out the currently booted OS in
UIs that want to show only other installed operating systems."

Your use-case sound like you'd need a way to add a more specific DT
board compatible at runtime. Then the existing selection logic would
handle your case as well.

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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] blspec: add checking of optional key machine-id

[Andreas Schmidt]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1909 bytes --]

Hallo Jan,


On 02.05.2018 16:32, Jan Lübbe wrote:
> On Sun, 2018-04-29 at 18:01 +0200, Andreas Schmidt wrote:
>> I guess, for such an use case Bootloader Spec specify "machine-id"
>> key. This patch implement the support for machine-id key.
> The way I read https://www.freedesktop.org/wiki/Specifications/BootLoad
> erSpec/ is that the "machine-id" field should contain the ID from
> /etc/machine-id, which is basically a UUID:
> "machine-id shall contain the machine ID of the OS /etc/machine-id.
> This is useful for boot loaders and applications to filter out boot
> entries, for example to show only a single newest kernel per OS, or to
> group items by OS, or to maybe filter out the currently booted OS in
> UIs that want to show only other installed operating systems."

I read this part in same way. I think, the patch isn't in conflict with
that. And, yes, you're right we would use it not exact in this way.

Out /etc/machine-id is building dynamically and would be the same as barebox
choose one. (Same way to set global.boot.machine_id and content of
/etc/machine-id)
But, all our FW would have different machine-id in dependent of devices
and not on
OS.

The spec says: "for example to show only ...". So I guess, it is not
hard requirement.
Was I'm wrong?

> Your use-case sound like you'd need a way to add a more specific DT
> board compatible at runtime. 

We need a decision witch DT has to be load at runtime. But they are all
compatible with the barebox DT. We have only one barebox DT for all devices.

> Then the existing selection logic would
> handle your case as well.

Ohh ... ok. And could you explain please, who we could do that? Because,
all our DTs are compatible with barebox DT and barebox would choose simple
the first one and boot it. Or is using of Bootloader Spec isn't right
way, to solve
this issue?

Regards,
Andreas


[-- Attachment #1.1.1.2: 0xFEE0A611BEA6DEA0.asc --]
[-- Type: application/pgp-keys, Size: 3133 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

Re: [PATCH] blspec: add checking of optional key machine-id

[Jan Lübbe]

On Thu, 2018-05-03 at 10:42 +0200, Andreas Schmidt wrote:
> > Your use-case sound like you'd need a way to add a more specific DT
> > board compatible at runtime. 
> 
> We need a decision witch DT has to be load at runtime. But they are all
> compatible with the barebox DT. We have only one barebox DT for all devices.

Yes. So you have additional device declarations in the kernel device
trees, which are not relevant to barebox.

> > Then the existing selection logic would
> > handle your case as well.
> 
> Ohh ... ok. And could you explain please, who we could do that? Because,
> all our DTs are compatible with barebox DT and barebox would choose simple
> the first one and boot it. Or is using of Bootloader Spec isn't right
> way, to solve this issue?

So in the barebox DT you'd have:
/ {
        model = "Foo Common i.MX6 Board";
        compatible = "foo,mx6-common", "fsl,imx6q";
};
[...]

Then for the kernel DTs you can use different compatible lists, which
each use a different and more specific entry.

mx6-variant1.dts:
/ {
        model = "Foo Variant 1 i.MX6 Board";
        compatible =
"foo,mx6-variant1", "foo,mx6-common", "fsl,imx6q";
};
[...]

mx6-variant2.dts:
/ {
        model = "Foo Variant 2 i.MX6 Board";
        compatible = "foo,mx6-variant2", "foo,mx6-common", "fsl,imx6q";
};
[...]

Then, in barebox, you need to have a way to override which compatible
string the bootspec code is looking for.

I'd prefer this to the machine-id approach, as the DT compatible list
mechanism is designed exactly for handing these different levels of
compatibility declarations.

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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] blspec: add checking of optional key machine-id

[Andreas Schmidt]

[-- Attachment #1.1.1.1: Type: text/plain, Size: 1680 bytes --]


On 03.05.2018 11:34, Jan Lübbe wrote:
[...]
>>> Then the existing selection logic would
>>> handle your case as well.
>> Ohh ... ok. And could you explain please, who we could do that? Because,
>> all our DTs are compatible with barebox DT and barebox would choose simple
>> the first one and boot it. Or is using of Bootloader Spec isn't right
>> way, to solve this issue?
> So in the barebox DT you'd have:
> / {
>         model = "Foo Common i.MX6 Board";
>         compatible = "foo,mx6-common", "fsl,imx6q";
> };
> [...]
>
> Then for the kernel DTs you can use different compatible lists, which
> each use a different and more specific entry.
>
> mx6-variant1.dts:
> / {
>         model = "Foo Variant 1 i.MX6 Board";
>         compatible =
> "foo,mx6-variant1", "foo,mx6-common", "fsl,imx6q";
> };
> [...]
>
> mx6-variant2.dts:
> / {
>         model = "Foo Variant 2 i.MX6 Board";
>         compatible = "foo,mx6-variant2", "foo,mx6-common", "fsl,imx6q";
> };
> [...]
>
> Then, in barebox, you need to have a way to override which compatible
> string the bootspec code is looking for.
Ok, the way is to override/extend barebox DT compatible string while
initialisation,
so barebox would be match only one boot entry.

> I'd prefer this to the machine-id approach, as the DT compatible list
> mechanism is designed exactly for handing these different levels of
> compatibility declarations.
Yes, I agree with you. That could be better for us to solve our issue.
Thank you for explanation!

Do your mean, for using the machine-id in right way, the patch could be
apply?
If Sascha still agree, of course.

Regards,
Andreas

[-- Attachment #1.1.1.2: 0xFEE0A611BEA6DEA0.asc --]
[-- Type: application/pgp-keys, Size: 3133 bytes --]

[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 801 bytes --]

[-- Attachment #2: Type: text/plain, Size: 149 bytes --]

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

Re: [PATCH v2] blspec: add checking of optional key machine-id

[Sascha Hauer]

On Wed, May 02, 2018 at 03:23:56PM +0200, Andreas Schmidt wrote:
> For filtering of Bootloader Spec entries, Bootloader Spec
> specify an optional key machine-id. By set the
> global.boot.machine-id variable the checking of machine-id
> key in Bootloader Spec entries will be activate. If the
> variable and key match, appropriate boot entry will be
> booting. If it not match boot entry will be ignore and
> barebox check the next boot entry.
> 
> Signed-off-by: Andreas Schmidt <mail@schmidt-andreas.de>
> ---

As mentioned, machine-id is not for filtering out different bootloader
spec entries for the same OS, but for selecting a particular OS if
different ones are installed. So this patch has a valid usecase, it's
just not the one Andreas made this patch for.

Because it's a valid patch: Applied, thanks

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 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