[PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk ---
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough - the above points re modifying the .dtb after it is created but before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES + string "Extra .dtsi files to include when building DT control" + depends on OF_CONTROL + help + U-Boot's control .dtb is usually built from an in-tree .dts + file, plus (if available) an in-tree U-Boot-specific .dtsi + file. This option specifies a space-separated list of extra + .dtsi files that will also be used. + config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \ + $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \ + echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \
Hi, all I prepared 3 patches for fdt_add_pubkey adding. But in our company infrastructure I can't use git send-email. Our IT can't help me to resolve issue.
-----Original Message----- From: Rasmus Villemoes rasmus.villemoes@prevas.dk Sent: Tuesday, September 28, 2021 11:57 AM To: u-boot@lists.denx.de Cc: Alex Kiernan alex.kiernan@gmail.com; Simon Glass sjg@chromium.org; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org; Rasmus Villemoes rasmus.villemoes@prevas.dk Subject: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk ---
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough - the above points re modifying the .dtb after it is created but before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES + string "Extra .dtsi files to include when building DT control" + depends on OF_CONTROL + help + U-Boot's control .dtb is usually built from an in-tree .dts + file, plus (if available) an in-tree U-Boot-specific .dtsi + file. This option specifies a space-separated list of extra + .dtsi files that will also be used. + config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \ + $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \ + echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \ -- 2.31.1
Hi Roman,
On Tue, 28 Sept 2021 at 03:55, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, all I prepared 3 patches for fdt_add_pubkey adding. But in our company infrastructure I can't use git send-email. Our IT can't help me to resolve issue.
It might be easier to set up a new gmail account and send it with patman than to see a new job :-)
Regards, Simon
Please note that this is not a resubmission of fdt_add_pubkey, I merely mentioned that in passing (and cc'ed Roman because of his interest in that) as a previous attempt at solving the problem of getting the public key info baked into U-Boot's .dtb, an approach that I've since learnt is not without problems of its own. So please review this patch on its own merits.
Rasmus
Hi Roman,
On Tue, 28 Sept 2021 at 03:55, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, all I prepared 3 patches for fdt_add_pubkey adding. But in our company infrastructure I can't use git send-email. Our IT can't help me to resolve issue.
I suppose I should point out that the Internet does normally work, so it is most likely your IT that is stopping this from working. So you only need them to 'stop' helping. It is a pretty poor situation when you cannot use git send-email, a very common use case in software companies.
- Simon
-----Original Message----- From: Rasmus Villemoes rasmus.villemoes@prevas.dk Sent: Tuesday, September 28, 2021 11:57 AM To: u-boot@lists.denx.de Cc: Alex Kiernan alex.kiernan@gmail.com; Simon Glass sjg@chromium.org; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org; Rasmus Villemoes rasmus.villemoes@prevas.dk Subject: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
Hi Roman,
On 9/28/21 5:55 AM, Roman Kopytin wrote:
Hi, all I prepared 3 patches for fdt_add_pubkey adding. But in our company infrastructure I can't use git send-email. Our IT can't help me to resolve issue.
It appears that Linux offers email hosting [1] for this purpose. However, you need to already be a kernel developer. So you best choice may be GMail.
--Sean
On 28/09/2021 10.56, Rasmus Villemoes wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code.
I should add that it's one thing when dealing with U-Boot proper and that just needs to be generated by cat'ing u-boot-nodtb.bin and a modified .dtb. But when the final generation is more complicated than that, e.g. involving black magic binman (which doesn't care to write out .cmd files so one can figure out what exactly happened under the hood), it's really really cumbersome.
It's also somewhat annoying
and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
ping
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
Also, is there any interest in using binman? It is designed to do the 'packaging' step right at the end, when all the bits are available and just need to be put together.
I am trying to encourage people to move away from building from source always, to a two-step process:
- build all the bits - package them, update devicetree, etc.
https://u-boot.readthedocs.io/en/latest/develop/package/index.html
BTW if Roman can figure out how to send the patches I think that tool would be useful too.
Regards, Simon
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
On 26/10/2021 03.28, Simon Glass wrote:
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Will do.
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
I think I explained that in the commit message, but let me try with a more concrete example. I'm building, using Yocto as the umbrella build system, for a SOC that needs an SPL + U-Boot proper.
So I have a U-Boot recipe that handles the configuration and compilation. That's mostly a matter of "make foo_defconfig" followed by "make ${UBOOT_TARGETS}" where UBOOT_TARGETS is something set in the machine config. That results in two artifacts, say SPL and u-boot.itb (the names don't really matter) that are almost ready to be written to whatever storage is used for them. Most likely, the SPL binary is built from a u-boot-spl-nodtb.bin and a spl.dtb and perhaps there's some SOC-specific header in front of it all that the hardware/firmware needs, those details are hidden by U-Boot's build system invoking the right flavour of mkimage with the right parameters. If I really want to know, I can look in spl/.SPL.cmd to see just how it was made [unless it's binman creating a flash.bin, because then it's just black magic]. But I usually don't need to.
Enter verified boot. SPL needs to verify U-Boot, and U-Boot must in turn verify the kernel. I can easily, as a post-compile step, create a signed version of u-boot.itb. But the -K option is rather useless here, because SPL has already been built. So if I were to only add the corresponding public key to SPL's dtb after/while signing U-Boot proper, I'd have to manually repeat the step(s) needed to create SPL in the first place. Not to mention that the .dtb living inside u-boot.itb doesn't have the public key needed for verifying the kernel, so I'd _actually_ first have to repeat whatever steps were done to create u-boot.itb, after using mkimage to sign some dummy image just to use the -K option to modify u-boot.dtb. It's really cumbersome.
So part of the problem is this "you can only get the public keys in the form required inside the .dtb as a side-effect of signing an image". I believe that's a fundamental design mistake, hence my attempt at creating the fdt_add_pubkey tool. But even with that available, adding the pubkey info into an already-compiled .dtb isn't really enough, because the .dtb gets built as part of a normal "make". Hence the need for a way to ensure the pubkey info gets baked into that .dtb during the build.
Yes, I then need to prepare proper .dtsi files. But since key generation is a mostly one-time/manual thing, that easily goes along with the instructions (or script) that generates those, and for every foo.crt, I'll just have that directory also contain a foo.dtsi, which I can then point at during do_configure.
Also, is there any interest in using binman?
Not really. I mean, it's fine if U-Boot internally uses that, and I can just take the final output and use that. But as long as binman doesn't play nice with Kbuild and e.g. tells the commands that were used to create a given binary, and pollutes the build dir with stuff that's not removed by "make clean", I'm not very enthusiastic about adding more uses myself.
Also, this:
BINMAN all Image 'main-section' is missing external blobs and is non-functional: blob-ext@3
Some images are invalid $ echo $? 0
Really? When building manually, perhaps the developer sees that warning (unless there were other non-BINMAN targets that make decided to build later, making the above scroll away...), but it's not very useful in some CI scenario where artifacts get deployed automatically to a test system after a successful build. Recovering boards with a broken bootloader easily costs many man-hours, plus the time to figure out what's wrong.
Rasmus
Hi Rasmus,
On Tue, 26 Oct 2021 at 02:08, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 26/10/2021 03.28, Simon Glass wrote:
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Will do.
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
I think I explained that in the commit message, but let me try with a more concrete example. I'm building, using Yocto as the umbrella build system, for a SOC that needs an SPL + U-Boot proper.
So I have a U-Boot recipe that handles the configuration and compilation. That's mostly a matter of "make foo_defconfig" followed by "make ${UBOOT_TARGETS}" where UBOOT_TARGETS is something set in the machine config. That results in two artifacts, say SPL and u-boot.itb (the names don't really matter) that are almost ready to be written to whatever storage is used for them. Most likely, the SPL binary is built from a u-boot-spl-nodtb.bin and a spl.dtb and perhaps there's some SOC-specific header in front of it all that the hardware/firmware needs, those details are hidden by U-Boot's build system invoking the right flavour of mkimage with the right parameters. If I really want to know, I can look in spl/.SPL.cmd to see just how it was made [unless it's binman creating a flash.bin, because then it's just black magic]. But I usually don't need to.
Enter verified boot. SPL needs to verify U-Boot, and U-Boot must in turn verify the kernel. I can easily, as a post-compile step, create a signed version of u-boot.itb. But the -K option is rather useless here, because SPL has already been built. So if I were to only add the corresponding public key to SPL's dtb after/while signing U-Boot proper, I'd have to manually repeat the step(s) needed to create SPL in the first place. Not to mention that the .dtb living inside u-boot.itb doesn't have the public key needed for verifying the kernel, so I'd _actually_ first have to repeat whatever steps were done to create u-boot.itb, after using mkimage to sign some dummy image just to use the -K option to modify u-boot.dtb. It's really cumbersome.
So part of the problem is this "you can only get the public keys in the form required inside the .dtb as a side-effect of signing an image". I believe that's a fundamental design mistake, hence my attempt at creating the fdt_add_pubkey tool. But even with that available, adding the pubkey info into an already-compiled .dtb isn't really enough, because the .dtb gets built as part of a normal "make". Hence the need for a way to ensure the pubkey info gets baked into that .dtb during the build.
Yes, I then need to prepare proper .dtsi files. But since key generation is a mostly one-time/manual thing, that easily goes along with the instructions (or script) that generates those, and for every foo.crt, I'll just have that directory also contain a foo.dtsi, which I can then point at during do_configure.
Also, is there any interest in using binman?
Not really. I mean, it's fine if U-Boot internally uses that, and I can just take the final output and use that. But as long as binman doesn't play nice with Kbuild and e.g. tells the commands that were used to create a given binary, and pollutes the build dir with stuff that's not removed by "make clean", I'm not very enthusiastic about adding more uses myself.
Also, this:
BINMAN all Image 'main-section' is missing external blobs and is non-functional: blob-ext@3
Some images are invalid $ echo $? 0
Really? When building manually, perhaps the developer sees that warning (unless there were other non-BINMAN targets that make decided to build later, making the above scroll away...), but it's not very useful in some CI scenario where artifacts get deployed automatically to a test system after a successful build. Recovering boards with a broken bootloader easily costs many man-hours, plus the time to figure out what's wrong.
I still think binman is the best solution here. The points you are raising are annoyances that can be resolved.
We could return a non-zero value from binman when external blobs are missing; we'd just need to use a special value (we use 101 for buildman) and update the CI to detect and ignore that.
Normally people use a separate output directory from the source directory, which is why the 'pollution' is not normally a big problem. But it is possible to resolve that problem and it has been discussed on the mailing list. It's just that no one has done it yet.
In what way does binman play badly with Kbuild?
I'd really suggest changing the mindset to separating build from packaging, perhaps by having a separate yocto package/recipe that puts the firmware together, adds the signature, etc. Images are only getting more complicated. The idea that you'll be able to build everyone in the U-Boot tree and it won't need any post-processing is not going to survive very long IMO, so you may as well invest in it now :-)
Regards, Simon
On 14/01/2022 17.51, Simon Glass wrote:
Hi Rasmus,
On Tue, 26 Oct 2021 at 02:08, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 26/10/2021 03.28, Simon Glass wrote:
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Will do.
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
I think I explained that in the commit message, but let me try with a more concrete example. I'm building, using Yocto as the umbrella build system, for a SOC that needs an SPL + U-Boot proper.
So I have a U-Boot recipe that handles the configuration and compilation. That's mostly a matter of "make foo_defconfig" followed by "make ${UBOOT_TARGETS}" where UBOOT_TARGETS is something set in the machine config. That results in two artifacts, say SPL and u-boot.itb (the names don't really matter) that are almost ready to be written to whatever storage is used for them. Most likely, the SPL binary is built from a u-boot-spl-nodtb.bin and a spl.dtb and perhaps there's some SOC-specific header in front of it all that the hardware/firmware needs, those details are hidden by U-Boot's build system invoking the right flavour of mkimage with the right parameters. If I really want to know, I can look in spl/.SPL.cmd to see just how it was made [unless it's binman creating a flash.bin, because then it's just black magic]. But I usually don't need to.
Enter verified boot. SPL needs to verify U-Boot, and U-Boot must in turn verify the kernel. I can easily, as a post-compile step, create a signed version of u-boot.itb. But the -K option is rather useless here, because SPL has already been built. So if I were to only add the corresponding public key to SPL's dtb after/while signing U-Boot proper, I'd have to manually repeat the step(s) needed to create SPL in the first place. Not to mention that the .dtb living inside u-boot.itb doesn't have the public key needed for verifying the kernel, so I'd _actually_ first have to repeat whatever steps were done to create u-boot.itb, after using mkimage to sign some dummy image just to use the -K option to modify u-boot.dtb. It's really cumbersome.
So part of the problem is this "you can only get the public keys in the form required inside the .dtb as a side-effect of signing an image". I believe that's a fundamental design mistake, hence my attempt at creating the fdt_add_pubkey tool. But even with that available, adding the pubkey info into an already-compiled .dtb isn't really enough, because the .dtb gets built as part of a normal "make". Hence the need for a way to ensure the pubkey info gets baked into that .dtb during the build.
Yes, I then need to prepare proper .dtsi files. But since key generation is a mostly one-time/manual thing, that easily goes along with the instructions (or script) that generates those, and for every foo.crt, I'll just have that directory also contain a foo.dtsi, which I can then point at during do_configure.
Also, is there any interest in using binman?
Not really. I mean, it's fine if U-Boot internally uses that, and I can just take the final output and use that. But as long as binman doesn't play nice with Kbuild and e.g. tells the commands that were used to create a given binary, and pollutes the build dir with stuff that's not removed by "make clean", I'm not very enthusiastic about adding more uses myself.
Also, this:
BINMAN all Image 'main-section' is missing external blobs and is non-functional: blob-ext@3
Some images are invalid $ echo $? 0
Really? When building manually, perhaps the developer sees that warning (unless there were other non-BINMAN targets that make decided to build later, making the above scroll away...), but it's not very useful in some CI scenario where artifacts get deployed automatically to a test system after a successful build. Recovering boards with a broken bootloader easily costs many man-hours, plus the time to figure out what's wrong.
I still think binman is the best solution here. The points you are raising are annoyances that can be resolved.
We could return a non-zero value from binman when external blobs are missing; we'd just need to use a special value (we use 101 for buildman) and update the CI to detect and ignore that.
Eh, does make(1) exit with a code that depends on the exit code from a failing target? What if multiple targets failed to build? Unless one can quote chapter-and-verse from make documentation, I don't think that's a particularly robust solution.
Normally people use a separate output directory from the source directory, which is why the 'pollution' is not normally a big problem.
Well, build systems like Yocto do set up a separate build dir, but that's not really important, as I rarely look inside ${S} nor ${B} - but when doing my own tinkering, I almost always build in the source tree, simply because that's a lot faster for quick experiments. So I'd wish all binman's temporary files were stowed away in some .binman_tmp dir in the top build dir (whether that's the source dir or an out-of-tree dir). Commits like 824204e42 are a strong indication that the current state of things is broken and doesn't scale.
But it is possible to resolve that problem and it has been discussed on the mailing list. It's just that no one has done it yet.
In what way does binman play badly with Kbuild?
My main grievance is that when binman is merely a thin wrapper around some external tool (often mkimage or one of its derivates), there's no .<target>.cmd file generated allowing me to see just what command was run to generate <target>. Another thing is that I can't do "make <target>".
I'd really suggest changing the mindset to separating build from packaging, perhaps by having a separate yocto package/recipe that puts the firmware together, adds the signature, etc.
Oh, I couldn't agree more, and in fact I've already done that for all the kernels I build; Yocto's kernel-fitimage.bbclass is simply too inflexible to use directly from the kernel recipe, so I just build an Image (or Image.gz) in the kernel recipe, then have a separate kernel-fit.bb recipe for actually assembling the different FIT images I need (often one for normal use and another for bootstrapping, but possibly more).
But I really can't see how binman helps in any way or form - in fact, it obscures the process of creating such a separate recipe, exactly because I can't extract the necessary magic invocation of mkimage that is needed to wrap SPL in the header format expected by the hardware.
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
Rasmus
Hi Rasmus,
On Mon, 24 Jan 2022 at 03:42, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 14/01/2022 17.51, Simon Glass wrote:
Hi Rasmus,
On Tue, 26 Oct 2021 at 02:08, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 26/10/2021 03.28, Simon Glass wrote:
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes@preva... [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kaspersky.co...
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Will do.
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
I think I explained that in the commit message, but let me try with a more concrete example. I'm building, using Yocto as the umbrella build system, for a SOC that needs an SPL + U-Boot proper.
So I have a U-Boot recipe that handles the configuration and compilation. That's mostly a matter of "make foo_defconfig" followed by "make ${UBOOT_TARGETS}" where UBOOT_TARGETS is something set in the machine config. That results in two artifacts, say SPL and u-boot.itb (the names don't really matter) that are almost ready to be written to whatever storage is used for them. Most likely, the SPL binary is built from a u-boot-spl-nodtb.bin and a spl.dtb and perhaps there's some SOC-specific header in front of it all that the hardware/firmware needs, those details are hidden by U-Boot's build system invoking the right flavour of mkimage with the right parameters. If I really want to know, I can look in spl/.SPL.cmd to see just how it was made [unless it's binman creating a flash.bin, because then it's just black magic]. But I usually don't need to.
Enter verified boot. SPL needs to verify U-Boot, and U-Boot must in turn verify the kernel. I can easily, as a post-compile step, create a signed version of u-boot.itb. But the -K option is rather useless here, because SPL has already been built. So if I were to only add the corresponding public key to SPL's dtb after/while signing U-Boot proper, I'd have to manually repeat the step(s) needed to create SPL in the first place. Not to mention that the .dtb living inside u-boot.itb doesn't have the public key needed for verifying the kernel, so I'd _actually_ first have to repeat whatever steps were done to create u-boot.itb, after using mkimage to sign some dummy image just to use the -K option to modify u-boot.dtb. It's really cumbersome.
So part of the problem is this "you can only get the public keys in the form required inside the .dtb as a side-effect of signing an image". I believe that's a fundamental design mistake, hence my attempt at creating the fdt_add_pubkey tool. But even with that available, adding the pubkey info into an already-compiled .dtb isn't really enough, because the .dtb gets built as part of a normal "make". Hence the need for a way to ensure the pubkey info gets baked into that .dtb during the build.
Yes, I then need to prepare proper .dtsi files. But since key generation is a mostly one-time/manual thing, that easily goes along with the instructions (or script) that generates those, and for every foo.crt, I'll just have that directory also contain a foo.dtsi, which I can then point at during do_configure.
Also, is there any interest in using binman?
Not really. I mean, it's fine if U-Boot internally uses that, and I can just take the final output and use that. But as long as binman doesn't play nice with Kbuild and e.g. tells the commands that were used to create a given binary, and pollutes the build dir with stuff that's not removed by "make clean", I'm not very enthusiastic about adding more uses myself.
Also, this:
BINMAN all Image 'main-section' is missing external blobs and is non-functional: blob-ext@3
Some images are invalid $ echo $? 0
Really? When building manually, perhaps the developer sees that warning (unless there were other non-BINMAN targets that make decided to build later, making the above scroll away...), but it's not very useful in some CI scenario where artifacts get deployed automatically to a test system after a successful build. Recovering boards with a broken bootloader easily costs many man-hours, plus the time to figure out what's wrong.
I still think binman is the best solution here. The points you are raising are annoyances that can be resolved.
We could return a non-zero value from binman when external blobs are missing; we'd just need to use a special value (we use 101 for buildman) and update the CI to detect and ignore that.
Eh, does make(1) exit with a code that depends on the exit code from a failing target? What if multiple targets failed to build? Unless one can quote chapter-and-verse from make documentation, I don't think that's a particularly robust solution.
Make is not the issue here. The basic problem is that we want to have three results:
- build failed, e.g. a compiler error - build succeeded but some binary blobs or tools are missing so the image won't work - build succeeded, image is valid
Normally people use a separate output directory from the source directory, which is why the 'pollution' is not normally a big problem.
Well, build systems like Yocto do set up a separate build dir, but that's not really important, as I rarely look inside ${S} nor ${B} - but when doing my own tinkering, I almost always build in the source tree, simply because that's a lot faster for quick experiments. So I'd wish all binman's temporary files were stowed away in some .binman_tmp dir in the top build dir (whether that's the source dir or an out-of-tree dir). Commits like 824204e42 are a strong indication that the current state of things is broken and doesn't scale.
Well that commit was really just rearranging the deck chairs. I didn't even see it.
The topic has come up before and I have suggested how it could be done. There is even a long-standing TODO in the binman readme about it.
But it is possible to resolve that problem and it has been discussed on the mailing list. It's just that no one has done it yet.
In what way does binman play badly with Kbuild?
My main grievance is that when binman is merely a thin wrapper around some external tool (often mkimage or one of its derivates), there's no .<target>.cmd file generated allowing me to see just what command was run to generate <target>. Another thing is that I can't do "make <target>".
You can enable logging with BINMAN_VERBOSE. See also the bintool series (which I should apply soon) which provides a single place for running these tools. So we could perhaps improve things in this area.
I'd really suggest changing the mindset to separating build from packaging, perhaps by having a separate yocto package/recipe that puts the firmware together, adds the signature, etc.
Oh, I couldn't agree more, and in fact I've already done that for all the kernels I build; Yocto's kernel-fitimage.bbclass is simply too inflexible to use directly from the kernel recipe, so I just build an Image (or Image.gz) in the kernel recipe, then have a separate kernel-fit.bb recipe for actually assembling the different FIT images I need (often one for normal use and another for bootstrapping, but possibly more).
But I really can't see how binman helps in any way or form - in fact, it obscures the process of creating such a separate recipe, exactly because I can't extract the necessary magic invocation of mkimage that is needed to wrap SPL in the header format expected by the hardware.
Why are you wanting to do that? You can just run binman again, can't you?
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Regards, Simon
On 24/01/2022 18.57, Simon Glass wrote:
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Huh? Are we talking about the same thing? What you write makes no sense at all.
Rasmus
Hi Rasmus,
On Mon, 24 Jan 2022 at 15:15, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 24/01/2022 18.57, Simon Glass wrote:
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Huh? Are we talking about the same thing? What you write makes no sense at all.
Perhaps it is a terminology thing. For me:
image: the final firmware image with everything in it binary: a component of the image
So there is only one image.
Regards, Simon
On 25/01/2022 00.50, Simon Glass wrote:
Hi Rasmus,
On Mon, 24 Jan 2022 at 15:15, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 24/01/2022 18.57, Simon Glass wrote:
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Huh? Are we talking about the same thing? What you write makes no sense at all.
Perhaps it is a terminology thing. For me:
image: the final firmware image with everything in it binary: a component of the image
So there is only one image.
It still makes no sense. I'm talking about the -K option to mkimage, which modifies the .dtb-which-is-to-be-used-by-U-Boot while building a FIT image containing kernel, initramfs and whatnot. That's certainly two very different images (or, in your terminology, one image and an entirely distinct binary destined to be included in another image).
Or for that matter, if one wants to do the same dance during SPL -> U-Boot, this could be the FIT image containing U-Boot proper and its control dtb (which must, of course, already have had the public-key-for-verifying-the-kernel embedded) vs the SPL's control .dtb, again two entirely different beasts.
The point is, I should not _need_ to sign anything in order to get the public key info into the control .dtbs. That's why I call it a design bug.
I have explained all of this several times, including in the commit message.
And the more you push for everyone to be using binman to generate the final images, the more important it is that the U-Boot .dtb contains the public-key-info before it gets used to build the final U-Boot image (whether that's itself a FIT or just a concatenation of u-boot.bin and u-boot.dtb). If I was assembling the final U-Boot image myself, I would just need the u-boot recipe to produce u-boot.bin and u-boot.dtb, and then I could have a separate step that updated u-boot.dtb. But when binman is in charge of doing that assembly, there's no such place to hook in, so I just want a way to ensure that the u-boot.dtb that gets generated is already the final one - which is what this patch is supposed to help with.
Rasmus
On 25/01/2022 00.50, Simon Glass wrote:
Hi Rasmus,
On Mon, 24 Jan 2022 at 15:15, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
On 24/01/2022 18.57, Simon Glass wrote:
And the thing about "adding the signature" - yes, indeed, _signing_ can and should be done after building. But that is not at all what this started with, this is about embedding the metadata that U-Boot (or SPL) will need for _verifying_ during the build itself - when the private key may not even be available. Again, I think that it's a fundamental design bug that generating and adding that metadata in the form needed by U-Boot can only be done as a side effect of signing some unrelated image.
It is a side effect of signing *the same* image, i.e. the image that holds the signature and the public key. There is only one image, the firmware image produced by binman.
Huh? Are we talking about the same thing? What you write makes no sense at all.
Perhaps it is a terminology thing. For me:
image: the final firmware image with everything in it binary: a component of the image
So there is only one image.
It still makes no sense. I'm talking about the -K option to mkimage, which modifies the .dtb-which-is-to-be-used-by-U-Boot while building a FIT image containing kernel, initramfs and whatnot. That's certainly two very different images (or, in your terminology, one image and an entirely distinct binary destined to be included in another image).
Or for that matter, if one wants to do the same dance during SPL -> U-Boot, this could be the FIT image containing U-Boot proper and its control dtb (which must, of course, already have had the public-key-for-verifying-the-kernel embedded) vs the SPL's control .dtb, again two entirely different beasts.
The point is, I should not _need_ to sign anything in order to get the public key info into the control .dtbs. That's why I call it a design bug.
I have explained all of this several times, including in the commit message.
And the more you push for everyone to be using binman to generate the final images, the more important it is that the U-Boot .dtb contains the public-key-info before it gets used to build the final U-Boot image (whether that's itself a FIT or just a concatenation of u-boot.bin and u-boot.dtb). If I was assembling the final U-Boot image myself, I would just need the u-boot recipe to produce u-boot.bin and u-boot.dtb, and then I could have a separate step that updated u-boot.dtb. But when binman is in charge of doing that assembly, there's no such place to hook in, so I just want a way to ensure that the u-boot.dtb that gets generated is already the final one - which is what this patch is supposed to help with.
Rasmus
Applied to u-boot-dm, thanks!
Hi, all Currently I am waiting some help from our IT infrastructure department.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, October 26, 2021 4:28 AM To: Rasmus Villemoes rasmus.villemoes@prevas.dk Cc: U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes @prevas.dk/ [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kasper sky.com/
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
Also, is there any interest in using binman? It is designed to do the 'packaging' step right at the end, when all the bits are available and just need to be put together.
I am trying to encourage people to move away from building from source always, to a two-step process:
- build all the bits - package them, update devicetree, etc.
https://u-boot.readthedocs.io/en/latest/develop/package/index.html
BTW if Roman can figure out how to send the patches I think that tool would be useful too.
Regards, Simon
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
Hi Roman,
Good luck! I must get a copy of that BOFH book.
Regards, Simon
On Sun, 31 Oct 2021 at 22:30, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, all Currently I am waiting some help from our IT infrastructure department.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, October 26, 2021 4:28 AM To: Rasmus Villemoes rasmus.villemoes@prevas.dk Cc: U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemoes @prevas.dk/ [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kasper sky.com/
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
Also, is there any interest in using binman? It is designed to do the 'packaging' step right at the end, when all the bits are available and just need to be put together.
I am trying to encourage people to move away from building from source always, to a two-step process:
- build all the bits
- package them, update devicetree, etc.
https://u-boot.readthedocs.io/en/latest/develop/package/index.html
BTW if Roman can figure out how to send the patches I think that tool would be useful too.
Regards, Simon
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
Hi, I sent patches, please check. But before correct emails I sent several test emails.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 5, 2021 5:02 AM To: Roman Kopytin Roman.Kopytin@kaspersky.com Cc: Rasmus Villemoes rasmus.villemoes@prevas.dk; U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Roman,
Good luck! I must get a copy of that BOFH book.
Regards, Simon
On Sun, 31 Oct 2021 at 22:30, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, all Currently I am waiting some help from our IT infrastructure department.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, October 26, 2021 4:28 AM To: Rasmus Villemoes rasmus.villemoes@prevas.dk Cc: U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemo es @prevas.dk/ [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kasp er sky.com/
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
Also, is there any interest in using binman? It is designed to do the 'packaging' step right at the end, when all the bits are available and just need to be put together.
I am trying to encourage people to move away from building from source always, to a two-step process:
- build all the bits
- package them, update devicetree, etc.
https://u-boot.readthedocs.io/en/latest/develop/package/index.html
BTW if Roman can figure out how to send the patches I think that tool would be useful too.
Regards, Simon
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
Hi Roman,
On Mon, 8 Nov 2021 at 08:43, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, I sent patches, please check. But before correct emails I sent several test emails.
OK I see them, not copied to me, but I see them in the mailing list, thank you.
Regards, Simon
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Friday, November 5, 2021 5:02 AM To: Roman Kopytin Roman.Kopytin@kaspersky.com Cc: Rasmus Villemoes rasmus.villemoes@prevas.dk; U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Roman,
Good luck! I must get a copy of that BOFH book.
Regards, Simon
On Sun, 31 Oct 2021 at 22:30, Roman Kopytin Roman.Kopytin@kaspersky.com wrote:
Hi, all Currently I am waiting some help from our IT infrastructure department.
-----Original Message----- From: Simon Glass sjg@chromium.org Sent: Tuesday, October 26, 2021 4:28 AM To: Rasmus Villemoes rasmus.villemoes@prevas.dk Cc: U-Boot Mailing List u-boot@lists.denx.de; Alex Kiernan alex.kiernan@gmail.com; Roman Kopytin Roman.Kopytin@kaspersky.com; Masahiro Yamada masahiroy@kernel.org Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES
Hi Rasmus,
On Tue, 28 Sept 2021 at 02:57, Rasmus Villemoes rasmus.villemoes@prevas.dk wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
Getting the public key metadata into .dtsi form can be done with a little scripting (roughly 'mkimage -K' of a dummy image followed by 'dtc -I dtb -O dts'). I have a script that does that, along with options to set 'required' and 'required-mode' properties, include u-boot,dm-spl properties in case one wants to check U-Boot proper from SPL with the verified boot mechanism, etc. I'm happy to share that script if this gets accepted, but it's moot if this is rejected.
I have previously tried to get an fdt_add_pubkey tool accepted [1,2] to disentangle the kernel and u-boot builds (or u-boot and SPL builds for that matter!), but as I've since realized, that isn't quite enough
- the above points re modifying the .dtb after it is created but
before that is used to create further build artifacts still stand. However, such a tool could still be useful for creating the .dtsi info without the private keys being present, and my key2dtsi.sh script could easily be modified to use a tool like that should it ever appear.
[1] https://lore.kernel.org/u-boot/20200211094818.14219-3-rasmus.villemo es @prevas.dk/ [2] https://lore.kernel.org/u-boot/2184f1e6dd6247e48133c76205feeabe@kasp er sky.com/
dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 2 ++ 2 files changed, 11 insertions(+)
Reviewed-by: Simon Glass sjg@chromium.org
I suggest adding this to the documentation somewhere in doc/develop/devicetree/
Getting the key into the U-Boot .dtb is normally done with mkimage, as you say. I don't really understand why this approach is easier.
Also, is there any interest in using binman? It is designed to do the 'packaging' step right at the end, when all the bits are available and just need to be put together.
I am trying to encourage people to move away from building from source always, to a two-step process:
- build all the bits
- package them, update devicetree, etc.
https://u-boot.readthedocs.io/en/latest/develop/package/index.html
BTW if Roman can figure out how to send the patches I think that tool would be useful too.
Regards, Simon
diff --git a/dts/Kconfig b/dts/Kconfig index dabe0080c1..593dddbaf0 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -139,6 +139,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"depends on OF_CONTROLhelpU-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 78bbebe7e9..a2accba940 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -320,6 +320,8 @@ quiet_cmd_dtc = DTC $@ # Bring in any U-Boot-specific include at the end of the file cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
$(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(CPP) $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \-- 2.31.1
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk --- v2: rebase to current master, add paragraph to doc/develop/devicetree/control.rst as suggested by Simon. I've taken the liberty of keeping his R-b tag as this mostly just repeats what is in the Kconfig help text and commit message.
doc/develop/devicetree/control.rst | 18 ++++++++++++++++++ dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 3 +++ 3 files changed, 30 insertions(+)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index 0e6f85d5af..ff008ba943 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -182,6 +182,24 @@ main file, in this order:: Only one of these is selected but of course you can #include another one within that file, to create a hierarchy of shared files.
+ +External .dtsi fragments +------------------------ + +Apart from describing the hardware present, U-Boot also uses its +control dtb for various configuration purposes. For example, the +public key(s) used for Verified Boot are embedded in a specific format +in a /signature node. + +As mentioned above, the U-Boot build system automatically includes a +*-u-boot.dtsi file, if found, containing U-Boot specific +quirks. However, some data, such as the mentioned public keys, are not +appropriate for upstream U-Boot but are better kept and maintained +outside the U-Boot repository. You can use CONFIG_DEVICE_TREE_INCLUDES +to specify a list of .dtsi files that will also be included when +building .dtb files. + + Relocation, SPL and TPL -----------------------
diff --git a/dts/Kconfig b/dts/Kconfig index b7c4a2fec0..1f8debf1a8 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -131,6 +131,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES + string "Extra .dtsi files to include when building DT control" + depends on OF_CONTROL + help + U-Boot's control .dtb is usually built from an in-tree .dts + file, plus (if available) an in-tree U-Boot-specific .dtsi + file. This option specifies a space-separated list of extra + .dtsi files that will also be used. + config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 39f03398ed..4ab422c231 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -318,8 +318,11 @@ endif quiet_cmd_dtc = DTC $@ # Modified for U-Boot # Bring in any U-Boot-specific include at the end of the file +# And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \ + $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \ + echo '$(pound)include "$(f)"' >> $(pre-tmp);) \ $(HOSTCC) -E $(dtc_cpp_flags) -x assembler-with-cpp -o $(dtc-tmp) $(pre-tmp) ; \ $(DTC) -O dtb -o $@ -b 0 \ -i $(dir $<) $(DTC_FLAGS) \
Ping
On 21/11/2021 14.52, Rasmus Villemoes wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
v2: rebase to current master, add paragraph to doc/develop/devicetree/control.rst as suggested by Simon. I've taken the liberty of keeping his R-b tag as this mostly just repeats what is in the Kconfig help text and commit message.
doc/develop/devicetree/control.rst | 18 ++++++++++++++++++ dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 3 +++ 3 files changed, 30 insertions(+)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index 0e6f85d5af..ff008ba943 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -182,6 +182,24 @@ main file, in this order:: Only one of these is selected but of course you can #include another one within that file, to create a hierarchy of shared files.
+External .dtsi fragments +------------------------
+Apart from describing the hardware present, U-Boot also uses its +control dtb for various configuration purposes. For example, the +public key(s) used for Verified Boot are embedded in a specific format +in a /signature node.
+As mentioned above, the U-Boot build system automatically includes a +*-u-boot.dtsi file, if found, containing U-Boot specific +quirks. However, some data, such as the mentioned public keys, are not +appropriate for upstream U-Boot but are better kept and maintained +outside the U-Boot repository. You can use CONFIG_DEVICE_TREE_INCLUDES +to specify a list of .dtsi files that will also be included when +building .dtb files.
Relocation, SPL and TPL
diff --git a/dts/Kconfig b/dts/Kconfig index b7c4a2fec0..1f8debf1a8 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -131,6 +131,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"- depends on OF_CONTROL
- help
U-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 39f03398ed..4ab422c231 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -318,8 +318,11 @@ endif quiet_cmd_dtc = DTC $@ # Modified for U-Boot # Bring in any U-Boot-specific include at the end of the file +# And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
- $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \
echo '$(pound)include "$(f)"' >> $(pre-tmp);) \
On Fri, Jan 14, 2022 at 09:30:29AM +0100, Rasmus Villemoes wrote:
Ping
On 21/11/2021 14.52, Rasmus Villemoes wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
v2: rebase to current master, add paragraph to doc/develop/devicetree/control.rst as suggested by Simon. I've taken the liberty of keeping his R-b tag as this mostly just repeats what is in the Kconfig help text and commit message.
doc/develop/devicetree/control.rst | 18 ++++++++++++++++++ dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 3 +++ 3 files changed, 30 insertions(+)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index 0e6f85d5af..ff008ba943 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -182,6 +182,24 @@ main file, in this order:: Only one of these is selected but of course you can #include another one within that file, to create a hierarchy of shared files.
+External .dtsi fragments +------------------------
+Apart from describing the hardware present, U-Boot also uses its +control dtb for various configuration purposes. For example, the +public key(s) used for Verified Boot are embedded in a specific format +in a /signature node.
+As mentioned above, the U-Boot build system automatically includes a +*-u-boot.dtsi file, if found, containing U-Boot specific +quirks. However, some data, such as the mentioned public keys, are not +appropriate for upstream U-Boot but are better kept and maintained +outside the U-Boot repository. You can use CONFIG_DEVICE_TREE_INCLUDES +to specify a list of .dtsi files that will also be included when +building .dtb files.
Relocation, SPL and TPL
diff --git a/dts/Kconfig b/dts/Kconfig index b7c4a2fec0..1f8debf1a8 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -131,6 +131,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"- depends on OF_CONTROL
- help
U-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 39f03398ed..4ab422c231 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -318,8 +318,11 @@ endif quiet_cmd_dtc = DTC $@ # Modified for U-Boot # Bring in any U-Boot-specific include at the end of the file +# And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
- $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \
echo '$(pound)include "$(f)"' >> $(pre-tmp);) \
Simon?
Hi Tom,
On Fri, 14 Jan 2022 at 08:41, Tom Rini trini@konsulko.com wrote:
On Fri, Jan 14, 2022 at 09:30:29AM +0100, Rasmus Villemoes wrote:
Ping
On 21/11/2021 14.52, Rasmus Villemoes wrote:
The build system already automatically looks for and includes an in-tree *-u-boot.dtsi when building the control .dtb. However, there are some things that are awkward to maintain in such an in-tree file, most notably the metadata associated to public keys used for verified boot.
The only "official" API to get that metadata into the .dtb is via mkimage, as a side effect of building an actual signed image. But there are multiple problems with that. First of all, the final U-Boot (be it U-Boot proper or an SPL) image is built based on a binary image, the .dtb, and possibly some other binary artifacts. So modifying the .dtb after the build requires the meta-buildsystem (Yocto, buildroot, whatnot) to know about and repeat some of the steps that are already known to and handled by U-Boot's build system, resulting in needless duplication of code. It's also somewhat annoying and inconsistent to have a .dtb file in the build folder which is not generated by the command listed in the corresponding .cmd file (that of course applies to any generated file).
So the contents of the /signature node really needs to be baked into the .dtb file when it is first created, which means providing the relevant data in the form of a .dtsi file. One could in theory put that data into the *-u-boot.dtsi file, but it's more convenient to be able to provide it externally: For example, when developing for a customer, it's common to use a set of dummy keys for development, while the consultants do not (and should not) have access to the actual keys used in production. For such a setup, it's easier if the keys used are chosen via the meta-buildsystem and the path(s) patched in during the configure step. And of course, nothing prevents anybody from having DEVICE_TREE_INCLUDES point at files maintained in git, or for that matter from including the public key metadata in the *-u-boot.dtsi directly and ignore this feature.
There are other uses for this, e.g. in combination with ENV_IMPORT_FDT it can be used for providing the contents of the /config/environment node, so I don't want to tie this exclusively to use for verified boot.
Reviewed-by: Simon Glass sjg@chromium.org Signed-off-by: Rasmus Villemoes rasmus.villemoes@prevas.dk
v2: rebase to current master, add paragraph to doc/develop/devicetree/control.rst as suggested by Simon. I've taken the liberty of keeping his R-b tag as this mostly just repeats what is in the Kconfig help text and commit message.
doc/develop/devicetree/control.rst | 18 ++++++++++++++++++ dts/Kconfig | 9 +++++++++ scripts/Makefile.lib | 3 +++ 3 files changed, 30 insertions(+)
diff --git a/doc/develop/devicetree/control.rst b/doc/develop/devicetree/control.rst index 0e6f85d5af..ff008ba943 100644 --- a/doc/develop/devicetree/control.rst +++ b/doc/develop/devicetree/control.rst @@ -182,6 +182,24 @@ main file, in this order:: Only one of these is selected but of course you can #include another one within that file, to create a hierarchy of shared files.
+External .dtsi fragments +------------------------
+Apart from describing the hardware present, U-Boot also uses its +control dtb for various configuration purposes. For example, the +public key(s) used for Verified Boot are embedded in a specific format +in a /signature node.
+As mentioned above, the U-Boot build system automatically includes a +*-u-boot.dtsi file, if found, containing U-Boot specific +quirks. However, some data, such as the mentioned public keys, are not +appropriate for upstream U-Boot but are better kept and maintained +outside the U-Boot repository. You can use CONFIG_DEVICE_TREE_INCLUDES +to specify a list of .dtsi files that will also be included when +building .dtb files.
Relocation, SPL and TPL
diff --git a/dts/Kconfig b/dts/Kconfig index b7c4a2fec0..1f8debf1a8 100644 --- a/dts/Kconfig +++ b/dts/Kconfig @@ -131,6 +131,15 @@ config DEFAULT_DEVICE_TREE It can be overridden from the command line: $ make DEVICE_TREE=<device-tree-name>
+config DEVICE_TREE_INCLUDES
string "Extra .dtsi files to include when building DT control"- depends on OF_CONTROL
- help
U-Boot's control .dtb is usually built from an in-tree .dtsfile, plus (if available) an in-tree U-Boot-specific .dtsifile. This option specifies a space-separated list of extra.dtsi files that will also be used.config OF_LIST string "List of device tree files to include for DT control" depends on SPL_LOAD_FIT || MULTI_DTB_FIT diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib index 39f03398ed..4ab422c231 100644 --- a/scripts/Makefile.lib +++ b/scripts/Makefile.lib @@ -318,8 +318,11 @@ endif quiet_cmd_dtc = DTC $@ # Modified for U-Boot # Bring in any U-Boot-specific include at the end of the file +# And finally any custom .dtsi fragments specified with CONFIG_DEVICE_TREE_INCLUDES cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ (cat $<; $(if $(u_boot_dtsi),echo '$(pound)include "$(u_boot_dtsi)"')) > $(pre-tmp); \
- $(foreach f,$(subst $(quote),,$(CONFIG_DEVICE_TREE_INCLUDES)), \
echo '$(pound)include "$(f)"' >> $(pre-tmp);) \Simon?
I'm happy to apply it if you like, but it is not in my patchwork queue at present. I agree that keeping the review tag from the last version makes sense. I'll reply the other points separately, as I haven't got around to it.
Regards, Simon
participants (5)
-
Rasmus Villemoes -
Roman Kopytin -
Sean Anderson -
Simon Glass -
Tom Rini