Hi Sughosh, On Sun, 6 Aug 2023 at 13:34, Sughosh Ganu <sughosh.g...@linaro.org> wrote: > > hi Simon, > > On Mon, 7 Aug 2023 at 00:16, Simon Glass <s...@chromium.org> wrote: > > > > Hi, > > > > On Sun, 6 Aug 2023 at 11:25, Tom Rini <tr...@konsulko.com> wrote: > > > > > > On Sun, Aug 06, 2023 at 04:48:11PM +0530, Sughosh Ganu wrote: > > > > 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. > > > > > > That logic is just in scripts/ somewhere and can be extended. We can > > > add flags to specific files when needed. > > > > > > > > > > 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. > > > > > > Sounds like this needs more re-working all around then, as we should > > > only have one of these fragments, and it probably shouldn't be included > > > when not needed. > > > > Another option is to include the actual file (or even the data!) in > > the capsule-key property for sandbox, rather than the CONFIG. > > Will this not mean that the dtsi file will have to be included > explicitly for all the files which want to include the public key? I > think it will be easier if the dtsi file can get included > automatically, similar to u-boot.dtsi. I will check if we can put a > dtsi file under lib/efi_loader/ and get that included automatically > with capsule auth enabled, similar to how u-boot.dtsi gets included.
Well I feel that the CONFIG is a bit silly in a way, at least for sandbox, since we know the file being used. It just makes it harder. Regards, Simon