Re: [PATCH v2] introduce CONFIG_DEVICE_TREE_INCLUDES

[Tom Rini <trini@konsulko.com>]

[-- Attachment #1: Type: text/plain, Size: 6055 bytes --]

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 .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) \

Simon?

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]