On Sun, 6 Aug 2023 at 03:42, Tom Rini <tr...@konsulko.com> wrote: > > On Sat, Aug 05, 2023 at 09:03:53AM -0600, Simon Glass wrote: > > Hi Sughosh, > > > > On Sat, 5 Aug 2023 at 05:35, 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 signature node in the u-boot dtsi file and include the public > > > key through the capsule-key property. This file is per architecture, > > > and is currently being added for sandbox and arm architectures. It > > > will have to be added for other architectures which need to enable > > > capsule authentication support. > > > > > > 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> > > > --- > > > Changes since V6: > > > * Populate the CONFIG_EFI_CAPSULE_ESL_FILE symbol for sandbox and > > > sandbox_flattree which enable capsule authentication. > > > > > > Note: > > > Simon Glass had asked me to rid of the CONFIG_EFI_HAVE_CAPSULE_SUPPORT > > > ifdef used in the sandbox' u-boot.dtsi file. However, that results in > > > the sandbox_vpl test failing in CI. Hence that check has been kept. > > > > > > > > > arch/arm/dts/u-boot.dtsi | 14 ++++++++++++++ > > > arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++ > > We've already had some go-round as to why this basically identical file > can't be in like lib/efi_loader/ or something, yes?
Yes, these need to be under the arch/$(ARCH)/dts/ directory for the dtc to include them as part of the platform's dts. > > > > configs/sandbox_defconfig | 1 + > > > configs/sandbox_flattree_defconfig | 1 + > > > lib/efi_loader/Kconfig | 9 +++++++++ > > > 5 files changed, 42 insertions(+) > > > create mode 100644 arch/arm/dts/u-boot.dtsi > > > create mode 100644 arch/sandbox/dts/u-boot.dtsi > > > > > > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi > > > new file mode 100644 > > > index 0000000000..4f31da4521 > > > --- /dev/null > > > +++ b/arch/arm/dts/u-boot.dtsi > > > @@ -0,0 +1,14 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/** > > > + * Devicetree file with miscellaneous nodes that will be included > > > + * at build time into the DTB. Currently being used for including > > > + * capsule related information. > > > + */ > > > + > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > > +/ { > > > + signature { > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > > > + }; > > > +}; > > > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */ > > > diff --git a/arch/sandbox/dts/u-boot.dtsi b/arch/sandbox/dts/u-boot.dtsi > > > new file mode 100644 > > > index 0000000000..60bd004937 > > > --- /dev/null > > > +++ b/arch/sandbox/dts/u-boot.dtsi > > > @@ -0,0 +1,17 @@ > > > +// SPDX-License-Identifier: GPL-2.0+ > > > +/* > > > + * Devicetree file with miscellaneous nodes that will be included > > > + * at build time into the DTB. Currently being used for including > > > + * capsule related information. > > > + * > > > + */ > > > + > > > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT > > > +/ { > > > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE > > > + signature { > > > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE); > > > + }; > > > +#endif > > > +}; > > > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ > > And why are these two different? For the sandbox's u-boot.dtsi, we need CONFIG_EFI_CAPSULE_AUTHENTICATE so that the ESL file is looked for only when capsule authentication is enabled. The outer CONFIG_EFI_HAVE_CAPSULE_SUPPORT is to not include stuff from this file unless capsule support is enabled. Simon had asked me to rid of the outer ifdef, but the sandbox_vpl test fails without it. > > > > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig > > > index a22e47616f..0d559ff3a1 100644 > > > --- a/lib/efi_loader/Kconfig > > > +++ b/lib/efi_loader/Kconfig > > > @@ -235,6 +235,15 @@ 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 "" > > No default. Okay > > > > + depends on EFI_CAPSULE_AUTHENTICATE > > > + help > > > + Provides the absolute path to the EFI Signature List file which > > > > It isn't really an absolute path as it doesn't start with / > > > > You really can't/shouldn't use absolute paths in a U-Boot build. > > You can't as it will break reproducible builds (or make them harder than > required). Okay -sughosh