hi Simon,
On Sun, 16 Jul 2023 at 05:12, Simon Glass <s...@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Sat, 15 Jul 2023 at 07:46, 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 V3:
> > * Put the two ifdef statements together in arm architecture's
> > u-boot.dtsi file.
> > * Remove the extra blank line in the Kconfig.
> >
> > arch/arm/dts/u-boot.dtsi | 17 +++++++++++++++++
> > arch/sandbox/dts/u-boot.dtsi | 17 +++++++++++++++++
> > lib/efi_loader/Kconfig | 10 ++++++++++
> > lib/efi_loader/Makefile | 7 +++++++
> > 4 files changed, 51 insertions(+)
> > create mode 100644 arch/arm/dts/u-boot.dtsi
> > create mode 100644 arch/sandbox/dts/u-boot.dtsi
>
> This approach seems OK to me for now. It is a bit strange to specify a
> CONFIG option to add something to the DT, but we can always adjust it
> later if needed.
>
> >
> > diff --git a/arch/arm/dts/u-boot.dtsi b/arch/arm/dts/u-boot.dtsi
> > new file mode 100644
> > index 0000000000..2a9359c43c
> > --- /dev/null
> > +++ b/arch/arm/dts/u-boot.dtsi
> > @@ -0,0 +1,17 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
>
> /**
>
> for multi-line comments
Okay
>
> > + * Devicetree file with miscellaneous nodes that will be included
> > + * at build time into the DTB. Currently being used for including
> > + * capsule related information.
> > + *
>
> drop blank line
Okay
>
> > + */
> > +
> > +#ifdef CONFIG_EFI_HAVE_CAPSULE_SUPPORT
> > +#ifdef CONFIG_EFI_CAPSULE_AUTHENTICATE
>
> Can you combine these, or can you omit the first one?
I will drop the first line. Should build for all platforms I believe.
>
> > +/ {
> > + signature {
> > + capsule-key = /incbin/(CONFIG_EFI_CAPSULE_ESL_FILE);
> > + };
> > +};
> > +#endif /* CONFIG_EFI_CAPSULE_AUTHENTICATE */
> > +#endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */
> > 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 */
> > diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
> > index a22e47616f..9abb9a4db3 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -235,6 +235,16 @@ 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.
>
> Can you wrap to 72 chars or so?
Okay
-sughosh
>
> > +
> > config EFI_DEVICE_PATH_TO_TEXT
> > bool "Device path to text protocol"
> > default y
> > diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> > index 1a8c8d7cab..c52c9d27bd 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -89,3 +89,10 @@ obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> >
> > EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> > $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> > +
> > +ifeq ($(CONFIG_EFI_CAPSULE_AUTHENTICATE),y)
> > +EFI_CAPSULE_KEY_PATH := $(subst $\",,$(CONFIG_EFI_CAPSULE_ESL_FILE))
> > +ifeq ("$(wildcard $(EFI_CAPSULE_KEY_PATH))","")
> > +$(error .esl cerificate not found. Configure your
> > CONFIG_EFI_CAPSULE_ESL_FILE)
> > +endif
> > +endif
> > --
> > 2.34.1
> >
>
> REgards,
> Simon
