Hi Sughosh, On Thu, 15 Jun 2023 at 17:11, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Thu, 15 Jun 2023 at 14:44, Simon Glass <s...@chromium.org> wrote: > > > > Hi Sughosh, > > > > On Tue, 13 Jun 2023 at 11:41, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > > > > > The EFI capsule authentication logic in u-boot expects the public key > > > in the form of an EFI Signature List(ESL) to be provided as part of > > > the platform's dtb. Currently, the embedding of the ESL file into the > > > dtb needs to be done manually. > > > > > > Add a script for embedding the ESL used for capsule authentication in > > > the platform's dtb, and call this as part of building the dtb(s). This > > > brings the embedding of the ESL in the dtb into the u-boot build flow. > > > > > > The path to the ESL file is specified through the > > > CONFIG_EFI_CAPSULE_ESL_FILE symbol. > > > > > > Signed-off-by: Sughosh Ganu <sughosh.g...@linaro.org> > > > --- > > > lib/efi_loader/Kconfig | 11 +++++++++++ > > > scripts/Makefile.lib | 8 ++++++++ > > > scripts/embed_capsule_key.sh | 25 +++++++++++++++++++++++++ > > > 3 files changed, 44 insertions(+) > > > create mode 100755 scripts/embed_capsule_key.sh > > > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index c5835e6ef6..1326a1d109 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -234,6 +234,17 @@ config EFI_CAPSULE_MAX > > > Select the max capsule index value used for capsule report > > > variables. This value is used to create CapsuleMax variable. > > > > > > +config EFI_CAPSULE_ESL_FILE > > > + string "Path to the EFI Signature List File" > > > + default "" > > > + depends on EFI_CAPSULE_AUTHENTICATE > > > + help > > > + Provides the absolute path to the EFI Signature List > > > + file which will be embedded in the platform's device > > > + tree and used for capsule authentication at the time > > > + of capsule update. > > > + > > > + > > > config EFI_DEVICE_PATH_TO_TEXT > > > bool "Device path to text protocol" > > > default y > > > diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib > > > index 7b27224b5d..a4083d0a26 100644 > > > --- a/scripts/Makefile.lib > > > +++ b/scripts/Makefile.lib > > > @@ -192,6 +192,8 @@ dtc_cpp_flags = -Wp,-MD,$(depfile).pre.tmp -nostdinc > > > \ > > > -D__ASSEMBLY__ \ > > > -undef -D__DTS__ > > > > > > +export dtc_cpp_flags > > > + > > > # Finds the multi-part object the current object will be linked into > > > modname-multi = $(sort $(foreach m,$(multi-used),\ > > > $(if $(filter $(subst $(obj)/,,$*.o), $($(m:.o=-objs)) > > > $($(m:.o=-y))),$(m:.o=)))) > > > @@ -315,6 +317,9 @@ ifeq ($(CONFIG_OF_LIBFDT_OVERLAY),y) > > > DTC_FLAGS += -@ > > > endif > > > > > > +quiet_cmd_embedcapsulekey = EMBEDCAPSULEKEY $@ > > > +cmd_embedcapsulekey = $(srctree)/scripts/embed_capsule_key.sh $@ > > > + > > > quiet_cmd_dtc = DTC $@ > > > # Modified for U-Boot > > > # Bring in any U-Boot-specific include at the end of the file > > > @@ -333,6 +338,9 @@ cmd_dtc = mkdir -p $(dir ${dtc-tmp}) ; \ > > > > > > $(obj)/%.dtb: $(src)/%.dts FORCE > > > $(call if_changed_dep,dtc) > > > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y) > > > + $(call cmd,embedcapsulekey,$@) > > > +endif > > > > > > pre-tmp = $(subst $(comma),_,$(dot-target).pre.tmp) > > > dtc-tmp = $(subst $(comma),_,$(dot-target).dts.tmp) > > > diff --git a/scripts/embed_capsule_key.sh b/scripts/embed_capsule_key.sh > > > new file mode 100755 > > > index 0000000000..1c2e45f758 > > > --- /dev/null > > > +++ b/scripts/embed_capsule_key.sh > > > @@ -0,0 +1,25 @@ > > > +#! /bin/bash > > > +# SPDX-License-Identifier: GPL-2.0+ > > > +# > > > +# Copyright (C) 2023, Linaro Limited > > > +# > > > + > > > +gen_capsule_signature_file() { > > > +cat >> $1 << EOF > > > +/dts-v1/; > > > +/plugin/; > > > + > > > +&{/} { > > > + signature { > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > > > + }; > > > +}; > > > +EOF > > > +} > > > + > > > +gen_capsule_signature_file signature.$$.dts > /dev/null 2>&1 > > > +$CPP $dtc_cpp_flags -x assembler-with-cpp -o signature.$$.tmp > > > signature.$$.dts > /dev/null 2>&1 > > > +dtc -@ -O dtb -o signature.$$.dtbo signature.$$.tmp > /dev/null 2>&1 > > > +fdtoverlay -i $1 -o temp.$$.dtb -v signature.$$.dtbo > /dev/null 2>&1 > > > +mv temp.$$.dtb $1 > /dev/null 2>&1 > > > +rm -f signature.$$.* > /dev/null 2>&1 > > > -- > > > 2.34.1 > > > > > > > Can you please add this to binman instead? > > I had looked at using binman for this work earlier because I very much > expected this comment from you :). Having said that, I am very much > open to using binman instead if it turns out to be the better way of > achieving this. What this patch does is that, with capsule > authentication enabled, it embeds the public key esl file into the > dtb's as they get built. As per my understanding, binman gets called > at the end of the u-boot build, once the constituent images( e..g > u-boot.bin = u-boot-no-dtb.bin + dtb) have been generated. So, if we > call binman _after_ the requisite image(s) have been generated, we > would need to 1) identify the dtb's in which the esl needs to be > embedded, and then 2) generate the final image all over again. Don't > you think this is non optimal? Or is there a way of generating the > constituent images(including the dtb's) through binman instead?
The best way to do that IMO is to generate a second file, .e.g. u-boot-capsule.bin I don't think it is a good idea to add other junk to u-boot.bin. It should just be U-Boot + dtb. > > My understanding of binman is that it is a tool of packaging > constituent images together. But the constituent images are still > being built through make targets. In binman terminology, it collects a set of 'binaries' to produce firmware 'images'. Regards, Simon