[PATCH v7 08/11] binman: capsule: Add support for generating EFI capsules
Simon Glass
sjg at chromium.org
Mon Aug 7 17:11:30 CEST 2023
Hi Sughosh,
On Mon, 7 Aug 2023 at 00:40, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
>
> hi Simon,
>
> On Mon, 7 Aug 2023 at 07:04, Simon Glass <sjg at chromium.org> wrote:
> >
> > Hi Sughosh,
> >
> > On Sun, 6 Aug 2023 at 13:27, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > >
> > > hi Simon,
> > >
> > > On Mon, 7 Aug 2023 at 00:16, Simon Glass <sjg at chromium.org> wrote:
> > > >
> > > > Hi Sughosh,
> > > >
> > > > On Sun, 6 Aug 2023 at 09:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > >
> > > > > hi Simon,
> > > > >
> > > > > On Sat, 5 Aug 2023 at 22:50, Simon Glass <sjg at chromium.org> wrote:
> > > > > >
> > > > > > Hi Sughosh,
> > > > > >
> > > > > > On Sat, 5 Aug 2023 at 09:03, Simon Glass <sjg at chromium.org> wrote:
> > > > > > >
> > > > > > > Hi Sughosh,
> > > > > > >
> > > > > > > On Sat, 5 Aug 2023 at 05:35, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> > > > > > > >
> > > > > > > > Add support in binman for generating EFI capsules. The capsule
> > > > > > > > parameters can be specified through the capsule binman entry. Also add
> > > > > > > > test cases in binman for testing capsule generation.
> > > > > > > >
> > > > > > > > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > > > > > > > ---
> > > > > > > > Changes since V6:
> > > > > > > > * Add macros for the GUID strings in sandbox_efi_capsule.h
> > > > > > > > * Highlight that the private and public keys are mandatory for capsule
> > > > > > > > signing.
> > > > > > > > * Add a URL link to the UEFI spec, as used in the rst files.
> > > > > > > > * Use local vars for private and public keys in BuildSectionData()
> > > > > > > > * Use local vars for input payload and capsule filenames in
> > > > > > > > BuildSectionData().
> > > > > > > > * Drop the ProcessContents() and SetImagePos() as the superclass
> > > > > > > > functions suffice.
> > > > > > > > * Use GUID macro names in the capsule test dts files.
> > > > > > > > * Rename efi_capsule_payload.bin to capsule_input.bin.
> > > > > > > >
> > > > > > > >
> > > > > > > > include/sandbox_efi_capsule.h | 14 ++
> > > > > > >
> > > > > > > Please move this file to a later patch - see below.
> > > > > > >
> > > > > > > Could we have a single header file with all the GUIDs, i.e. sandbox, ARM, etc.
> > > > > > >
> > > > > > > > tools/binman/entries.rst | 62 ++++++++
> > > > > > > > tools/binman/etype/efi_capsule.py | 143 ++++++++++++++++++
> > > > > > > > tools/binman/ftest.py | 121 +++++++++++++++
> > > > > > > > tools/binman/test/307_capsule.dts | 23 +++
> > > > > > > > tools/binman/test/308_capsule_signed.dts | 25 +++
> > > > > > > > tools/binman/test/309_capsule_version.dts | 24 +++
> > > > > > > > tools/binman/test/310_capsule_signed_ver.dts | 26 ++++
> > > > > > > > tools/binman/test/311_capsule_oemflags.dts | 24 +++
> > > > > > > > tools/binman/test/312_capsule_missing_key.dts | 24 +++
> > > > > > > > .../binman/test/313_capsule_missing_index.dts | 22 +++
> > > > > > > > .../binman/test/314_capsule_missing_guid.dts | 19 +++
> > > > > > > > .../test/315_capsule_missing_payload.dts | 19 +++
> > > > > > > > 13 files changed, 546 insertions(+)
> > > > > > > > create mode 100644 include/sandbox_efi_capsule.h
> > > > > > > > create mode 100644 tools/binman/etype/efi_capsule.py
> > > > > > > > create mode 100644 tools/binman/test/307_capsule.dts
> > > > > > > > create mode 100644 tools/binman/test/308_capsule_signed.dts
> > > > > > > > create mode 100644 tools/binman/test/309_capsule_version.dts
> > > > > > > > create mode 100644 tools/binman/test/310_capsule_signed_ver.dts
> > > > > > > > create mode 100644 tools/binman/test/311_capsule_oemflags.dts
> > > > > > > > create mode 100644 tools/binman/test/312_capsule_missing_key.dts
> > > > > > > > create mode 100644 tools/binman/test/313_capsule_missing_index.dts
> > > > > > > > create mode 100644 tools/binman/test/314_capsule_missing_guid.dts
> > > > > > > > create mode 100644 tools/binman/test/315_capsule_missing_payload.dts
> > > > > > > >
> > > > > >
> > > > > > [..]
> > > > > >
> > > > > > > > diff --git a/tools/binman/test/307_capsule.dts b/tools/binman/test/307_capsule.dts
> > > > > > > > new file mode 100644
> > > > > > > > index 0000000000..86552ff83f
> > > > > > > > --- /dev/null
> > > > > > > > +++ b/tools/binman/test/307_capsule.dts
> > > > > > > > @@ -0,0 +1,23 @@
> > > > > > > > +// SPDX-License-Identifier: GPL-2.0+
> > > > > > > > +
> > > > > > > > +/dts-v1/;
> > > > > > > > +
> > > > > > > > +#include <sandbox_efi_capsule.h>
> > > > > > >
> > > > > > > We can't include sandbox files in binman! I Does it actually matter
> > > > > > > what the GUID value is? Can they all be the same? For now I think it
> > > > > > > is best to have
> > > > > > >
> > > > > > > #define BINMAN_TEST_GUID "xxx"
> > > > > > >
> > > > > > > repeated at the top of each .dts file. Hopefully at some point we can
> > > > > > > figure out a sensible way to provide a decoder ring for this mess, so
> > > > > > > we can use actually names in the binman definition instead of GUIDs.
> > > > > >
> > > > > > Oh, having thought about this a bit more, there is a much better way.
> > > > > >
> > > > > > In the entry type, allow the image_guid to be a string, like
> > > > > > 'binman-test' or 'sandbox-test'. Then binman can have a conversion
> > > > > > function to figure out the GUID to pass to the tool, e.g. in
> > > > > > efi_capsule.py:
> > > > > >
> > > > > > TYPE_TO_GUID = {
> > > > > > 'binman-test': '09123987987f98712',
> > > > > > 'sandbox-test':, '98123987123123987123987',
> > > > > > }
> > > > > >
> > > > > > def get_guid(type_str):
> > > > > > 'Get the GUID to use for a particular purpose"""
> > > > > > return TYPE_TO_GUID.get(type_str, type_str)
> > > > > >
> > > > > > Then you can use these same strings in the sandbox tests, without
> > > > > > needing to include anything.
> > > > >
> > > > > While we can use this method for both sandbox and binman tests, I
> > > > > would prefer using the actual GUID strings for the sandbox tests. This
> > > > > will serve as an illustrative example to the user for how to pass the
> > > > > image GUID value for generating the capsule. For the binman tests, I
> > > > > can use this logic to avoid including the header file.
> > > >
> > > > So long as it is easy, that's OK. But how does the user know which GUID to use?
> > >
> > > I think you got me wrong here. What I meant was that passing the GUID
> > > value in the capsule entry like how it is currently done in the
> > > u-boot.dtsi file would give the user an idea of how to specify the
> > > GUID value for an image. If we use the string that you suggest above
> > > for both binman tests as well as sandbox, the user might get confused
> > > as to how the GUID value is to be passed in the normal scenario. I
> > > want the sandbox capsule entries to also be kind of illustrative of
> > > how a capsule entry would look like.
> >
> > OK I see.
> >
> > Of course, I would like to see a decoder ring for GUIDs somewhere.
> > Perhaps Binman is a good place for that, or a file it can read?
>
> I'd suggest we start with your above solution for the binman tests. If
> we get consensus amongst the folks who work on EFI to have such a GUID
> decoder file, it can be worked upon later. I think these are things
> which are part of a larger point that you have about specifying GUIDs
> in a particular way, and that should be tackled separately, and not
> through this series, especially at this stage in the series. My 2
> cents.
That sounds OK to me. When we get to it, I think we should create a
database of GUIDs, with a name for each, and perhaps an ID number, so
we can allocate them.
Regards,
Simon
More information about the U-Boot
mailing list