hi Simon,
On Sun, 6 Aug 2023 at 00:35, Simon Glass <s...@chromium.org> wrote:
>
> Hi Sughosh,
>
> On Sat, 5 Aug 2023 at 12:47, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> >
> > hi Simon,
> >
> > On Sun, 6 Aug 2023 at 00:06, Simon Glass <s...@chromium.org> wrote:
> > >
> > > Hi Sughosh,
> > >
> > > On Sat, 5 Aug 2023 at 11:54, Sughosh Ganu <sughosh.g...@linaro.org> wrote:
> > > >
> > > > hi Simon,
> > > >
> > > > On Sat, 5 Aug 2023 at 20:34, Simon Glass <s...@chromium.org> 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 +++++++++++++++++
> > > > > > 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 */
> > > > > > diff --git a/configs/sandbox_defconfig b/configs/sandbox_defconfig
> > > > > > index b6c4f735f2..779af4abc8 100644
> > > > > > --- a/configs/sandbox_defconfig
> > > > > > +++ b/configs/sandbox_defconfig
> > > > > > @@ -341,6 +341,7 @@ CONFIG_EFI_RUNTIME_UPDATE_CAPSULE=y
> > > > > > CONFIG_EFI_CAPSULE_ON_DISK=y
> > > > > > CONFIG_EFI_CAPSULE_FIRMWARE_RAW=y
> > > > > > CONFIG_EFI_CAPSULE_AUTHENTICATE=y
> > > > > > +CONFIG_EFI_CAPSULE_ESL_FILE="../../../board/sandbox/SIGNER.esl"
> > > > >
> > > > > Can we avoid the path here, and just use e.g. good.esl ?
> > > >
> > > > Unfortunately no. I believe the way incbin works in dts is that it
> > > > looks for the binary to be included in the same directory as the dts.
> > > > Which is why I have to point it to the location of the esl relative to
> > > > the dts.
> > > >
> > > > >
> > > > > Perhaps this could be fixed up later, e.g. by adding the board
> > > > > directory as an include dir when building the DT?
> > > >
> > > > Again, this is not how incbin seems to work in dts. I tried putting
> > > > the esl in one of the existing include directory locations, but it
> > > > does not pick the file from those dirs. It works with the assumption
> > > > that the bin file is to be in the same dir as the dts.
> > >
> > > Yes but you can change that. Try adding to the cmd_dtc rule in
> > > Makefile.lib:
> > >
> > > -i $(srctree)/board/$(BOARDDIR) \
> >
> > We already have the
> >
> > -I$(srctree)/include
> >
> > and I had tried putting the esl under the include directory, but it
> > was not found.
>
> I think the board directory is better, though. It isn't really an include.
What I was trying to say is that putting the esl file under an include
directory does not work, in that the dtc is not able to locate the esl
file from the include directories. The incbin file has to be in a
directory relative to that of the dts file.
I just put the SIGNER.esl under the include/ directory to check if dtc
is able to locate the file from an include directory, and it does not.
-sughosh
>
> Regards,
> Simon
