From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id DA938C433EF for ; Tue, 26 Oct 2021 01:28:59 +0000 (UTC) Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 14EB060F46 for ; Tue, 26 Oct 2021 01:28:59 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 14EB060F46 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=chromium.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=lists.denx.de Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 24372807CD; Tue, 26 Oct 2021 03:28:57 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="m3C/I65v"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id 0F31F834F4; Tue, 26 Oct 2021 03:28:46 +0200 (CEST) Received: from mail-ua1-x935.google.com (mail-ua1-x935.google.com [IPv6:2607:f8b0:4864:20::935]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id 0B30380607 for ; Tue, 26 Oct 2021 03:28:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=sjg@google.com Received: by mail-ua1-x935.google.com with SMTP id f4so25984244uad.4 for ; Mon, 25 Oct 2021 18:28:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=YO8JNq2A1eOmabhZyRQi8n+mIvHG5o0b1PlhTJNS9DI=; b=m3C/I65vgFqnidBKYO1yXBxwHQbMr9r1niFVUCpSjMIxolSpaOAUFPhoAONQiIQel2 5YARlKxdzz9jCgbU25f1IDvD4OCZy17aSmBWdhNqsny4/r4KE2Jcv3TH37UmrOiqSd3q ZPbrtGuPPncQ1ccz13t7egV79vmsTAXaaqt70= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=YO8JNq2A1eOmabhZyRQi8n+mIvHG5o0b1PlhTJNS9DI=; b=Qy2FBn6TgCUAxv4/sNisUX91fa0rt5gMGk8q8ZU6fQnfX1EURvjuNenjI+6lTKeX/9 a7ekVq5AgJzrTOQ7rJ0Ub56Y7LHhVEkuNMydfqnNVmkro8Ea89cQbB/jAFOBF0U7Gwp/ KZxwRtP9X0kfDbRiUk2idhSmHXMJ4N8/HIpIMJrf+N6HiiWnqsrMxi7BrcxTigdNrYhc U4/z4eDmhS+S4TA7MmYDto+Y9dyrRzzECQUVcJJ8umbSj+i8vmQMDOhi2obpW6EUYFin imSJbs9+udufovGIoa4pO4QeRI3LJRgXb7a8axnaQzdMzRFCBiDpgbQc02dGYhK4V1g2 /LZg== X-Gm-Message-State: AOAM530Pswe/fMK0dXToOoZ+DiFomj0iqWoNwg5WViRwk5jkUXSqKOOs np4MJMYp0vSyuJEjWUUSisKi5nJQutothswSaqFbzw== X-Google-Smtp-Source: ABdhPJwOt+U0OorPCi9tjcpsTCycmrrjI6i67jG5ZFF1SV037vOJE9LDCcLT1WCUaDvV1huOaBu6JIwkVSb5xtlkzmw= X-Received: by 2002:a67:d51c:: with SMTP id l28mr8050412vsj.58.1635211718513; Mon, 25 Oct 2021 18:28:38 -0700 (PDT) MIME-Version: 1.0 References: <20210928085651.619892-1-rasmus.villemoes@prevas.dk> In-Reply-To: <20210928085651.619892-1-rasmus.villemoes@prevas.dk> From: Simon Glass Date: Mon, 25 Oct 2021 19:28:27 -0600 Message-ID: Subject: Re: [PATCH] introduce CONFIG_DEVICE_TREE_INCLUDES To: Rasmus Villemoes Cc: U-Boot Mailing List , Alex Kiernan , Roman Kopytin , Masahiro Yamada Content-Type: text/plain; charset="UTF-8" X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.34 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.2 at phobos.denx.de X-Virus-Status: Clean Hi Rasmus, On Tue, 28 Sept 2021 at 02:57, 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. > > Signed-off-by: Rasmus Villemoes > --- > > 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@kaspersky.com/ > > dts/Kconfig | 9 +++++++++ > scripts/Makefile.lib | 2 ++ > 2 files changed, 11 insertions(+) Reviewed-by: Simon Glass 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= > > +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 >