[PATCH v6 6/9] binman: capsule: Add support for generating EFI capsules

Sughosh Ganu sughosh.ganu at linaro.org
Thu Aug 3 13:08:21 CEST 2023


hi Simon,

On Wed, 2 Aug 2023 at 18:23, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Tue, 1 Aug 2023 at 11:41, 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 V5:
> > * Add support for the oemflag parameter used in FWU A/B updates. This
> >   was missed in the earlier version.
> > * Use a single function, generate_capsule in the mkeficapsule bintool,
> >   instead of the multiple functions in earlier version.
> > * Remove the logic for generating capsules from config file as
> >   suggested by Simon.
> > * Use required_props for image index and GUID parameters.
> > * Use a subnode for the capsule payload instead of using a filename
> >   for the payload, as suggested by Simon.
> > * Add a capsule generation test with oemflag parameter being passed.
> >
> >
> >  configs/sandbox_spl_defconfig                 |   1 +
>
> move to a later commit

Okay. I think it'd be better to have this change before adding support
for efi_capsule entry in binman.


>
> >  tools/binman/btool/mkeficapsule.py            | 101 +++++++++++
>
> nit:  Please split this into its own commit, with the etype in the
> second commit.

Okay

>
> >  tools/binman/entries.rst                      |  64 +++++++
> >  tools/binman/etype/efi_capsule.py             | 160 ++++++++++++++++++
> >  tools/binman/ftest.py                         | 122 +++++++++++++
> >  tools/binman/test/307_capsule.dts             |  21 +++
> >  tools/binman/test/308_capsule_signed.dts      |  23 +++
> >  tools/binman/test/309_capsule_version.dts     |  22 +++
> >  tools/binman/test/310_capsule_signed_ver.dts  |  24 +++
> >  tools/binman/test/311_capsule_oemflags.dts    |  22 +++
> >  tools/binman/test/312_capsule_missing_key.dts |  22 +++
> >  .../binman/test/313_capsule_missing_index.dts |  20 +++
> >  .../binman/test/314_capsule_missing_guid.dts  |  19 +++
> >  .../test/315_capsule_missing_payload.dts      |  17 ++
> >  14 files changed, 638 insertions(+)
> >  create mode 100644 tools/binman/btool/mkeficapsule.py
> >  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/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
> > index 8d50162b27..65223475ab 100644
> > --- a/configs/sandbox_spl_defconfig
> > +++ b/configs/sandbox_spl_defconfig
> > @@ -249,3 +249,4 @@ CONFIG_UNIT_TEST=y
> >  CONFIG_SPL_UNIT_TEST=y
> >  CONFIG_UT_TIME=y
> >  CONFIG_UT_DM=y
> > +CONFIG_TOOLS_MKEFICAPSULE=y
> > diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py
> > new file mode 100644
> > index 0000000000..da1d5de873
> > --- /dev/null
> > +++ b/tools/binman/btool/mkeficapsule.py
> > @@ -0,0 +1,101 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright 2023 Linaro Limited
> > +#
> > +"""Bintool implementation for mkeficapsule tool
> > +
> > +mkeficapsule is a tool used for generating EFI capsules.
> > +
> > +The following are the command-line options to be provided
> > +to the tool
> > +Usage: mkeficapsule [options] <image blob> <output file>
> > +Options:
> > +       -g, --guid <guid string>    guid for image blob type
> > +       -i, --index <index>         update image index
> > +       -I, --instance <instance>   update hardware instance
> > +       -v, --fw-version <version>  firmware version
> > +       -p, --private-key <privkey file>  private key file
> > +       -c, --certificate <cert file>     signer's certificate file
> > +       -m, --monotonic-count <count>     monotonic count
> > +       -d, --dump_sig              dump signature (*.p7)
> > +       -A, --fw-accept  firmware accept capsule, requires GUID, no image blob
> > +       -R, --fw-revert  firmware revert capsule, takes no GUID, no image blob
> > +       -o, --capoemflag Capsule OEM Flag, an integer between 0x0000 and 0xffff
> > +       -h, --help                  print a help message
> > +"""
> > +
> > +from binman import bintool
> > +
> > +class Bintoolmkeficapsule(bintool.Bintool):
> > +    """Handles the 'mkeficapsule' tool
> > +
> > +    This bintool is used for generating the EFI capsules. The
> > +    capsule generation parameters can either be specified through
> > +    command-line, or through a config file.
>
> commandline (or command line, but please be consistent

Okay

> )
> > +    """
> > +    def __init__(self, name):
> > +        super().__init__(name, 'mkeficapsule tool for generating capsules')
> > +
> > +    def generate_capsule(self, image_index, image_guid, hardware_instance,
> > +                         payload, output_fname, priv_key, pub_key,
> > +                         monotonic_count=0, version=0, oemflags=0):
> > +        """Generate a capsule through commandline provided parameters
>
> commandline-provided

Okay

>
> > +
> > +        Args:
> > +            image_index (int): Unique number for identifying payload image
> > +            image_guid (str): GUID used for identifying the image
> > +            hardware_instance (int): Optional unique hardware instance of
> > +            a device in the system. 0 if not being used
> > +            payload (str): Path to the input payload image
> > +            output_fname (str): Path to the output capsule file
> > +            priv_key (str): Path to the private key
> > +            pub_key(str): Path to the public key
> > +            monotonic_count (int): Count used when signing an image
> > +            version (int): Image version (Optional)
> > +            oemflags (int): Optional 16 bit OEM flags
> > +
> > +        Returns:
> > +            str: Tool output
> > +        """
> > +        args = [
> > +            f'--index={image_index}',
> > +            f'--guid={image_guid}',
> > +            f'--instance={hardware_instance}'
> > +        ]
> > +
> > +        if version:
> > +            args += [f'--fw-version={version}']
> > +        if oemflags:
> > +            args += [f'--capoemflag={oemflags}']
> > +        if priv_key and pub_key:
> > +            args += [
> > +                f'--monotonic-count={monotonic_count}',
> > +                f'--private-key={priv_key}',
> > +                f'--certificate={pub_key}'
> > +            ]
> > +
> > +        args += [
> > +            payload,
> > +            output_fname
> > +        ]
> > +
> > +        return self.run_cmd(*args)
> > +
> > +    def fetch(self, method):
> > +        """Fetch handler for mkeficapsule
> > +
> > +        This builds the tool from source
> > +
> > +        Returns:
> > +            tuple:
> > +                str: Filename of fetched file to copy to a suitable directory
> > +                str: Name of temp directory to remove, or None
> > +        """
> > +        if method != bintool.FETCH_BUILD:
> > +            return None
> > +
> > +        cmd = ['tools-only_defconfig', 'tools']
> > +        result = self.build_from_git(
> > +            'https://source.denx.de/u-boot/u-boot.git',
> > +            cmd,
> > +            'tools/mkeficapsule')
> > +        return result
> > diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> > index f2376932be..cfe699361c 100644
> > --- a/tools/binman/entries.rst
> > +++ b/tools/binman/entries.rst
> > @@ -468,6 +468,70 @@ updating the EC on startup via software sync.
> >
> >
>
> The btool looks fine to me
>
> [..]
>
> > diff --git a/tools/binman/etype/efi_capsule.py b/tools/binman/etype/efi_capsule.py
> > new file mode 100644
> > index 0000000000..b8a269e247
> > --- /dev/null
> > +++ b/tools/binman/etype/efi_capsule.py
> > @@ -0,0 +1,160 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2023 Linaro Limited
> > +#
> > +# Entry-type module for producing a EFI capsule
> > +#
> > +
> > +from collections import OrderedDict
> > +import os
> > +
> > +from binman.entry import Entry
> > +from binman.etype.section import Entry_section
> > +from dtoc import fdt_util
> > +from u_boot_pylib import tools
> > +
> > +class Entry_efi_capsule(Entry_section):
> > +    """Entry for generating EFI capsules
>
> """Generate EPI capsules
>
> (as we know it is an entry type)

Okay

>
> > +
> > +    This is an entry for generating EFI capsules.
>
> Drop that as it repeats the above line

Okay

>
> > +
> > +    The parameters needed for generation of the capsules can
> > +    either be provided as properties in the entry.
>
> or...?

Vestige from the earlier version where capsule generation was
supported through a config file as well.

>
> > +
> > +    Properties / Entry arguments:
> > +    - image-index: Unique number for identifying corresponding
> > +      payload image. Number between 1 and descriptor count, i.e.
> > +      the total number of firmware images that can be updated.
> > +    - image-type-id: Image GUID which will be used for identifying the
> > +      updatable image on the board.
> > +    - hardware-instance: Optional number for identifying unique
> > +      hardware instance of a device in the system. Default value of 0
> > +      for images where value is not to be used.
> > +    - fw-version: Optional value of image version that can be put on
> > +      the capsule through the Firmware Management Protocol(FMP) header.
> > +    - monotonic-count: Count used when signing an image.
> > +    - private-key: Path to PEM formatted .key private key file.
>
> Can you perhaps put these in a different order do you can say that if
> private-key is provided, you must also provide public-key-cert ?

For both the private and public key properties, I have added the following line.

 "This property is mandatory for generating signed capsules."

>
> > +    - pub-key-cert: Path to PEM formatted .crt public key certificate
>
> public, I think, since you write out private in full

Okay

>
> > +      file.
> > +    - capoemflags - Optional OEM flags to be passed through capsule header.
>
> oem-flags ?

Okay

>
> If 'cap' refers to capsule, we already know it is a capsule
>
> > +    - filename:
>
> Technically this is not true...it is the section output. I think it is
> better to mention that it inherits the section property, which allows
> an output filename.
>
> Optional path to the output capsule file. A capsule is a
> > +      continuous set of data as defined by the EFI specification. Refer
> > +      to the specification for more details.
>
> Good comments. You can drop filename, since you subclass entry_Section
> which always provides this feature. But it's OK to mention it if you
> like, e.g. "All section properties are supported, include filename".

I have removed the filename property, and replaced it with the following text
"Since this is a subclass of Entry_section, all properties of the
parent class also apply here."

>
> > +
> > +    For more details on the description of the capsule format, and the capsule
> > +    update functionality, refer Section 8.5 and Chapter 23 in the UEFI
> > +    specification.
> > +    https://uefi.org/sites/default/files/resources/UEFI_Spec_2_10_Aug29.pdf
>
> I suppose this is OK, but can you make it into a text link? e.g. see
> how doc/usage/cmd/acpi.rst does it.

Done

>
> > +
> > +    The capsule parameters like image index and image GUID are passed as
> > +    properties in the entry. The payload to be used in the capsule is to be
> > +    provided as a subnode of the capsule entry.
> > +
> > +    A typical capsule entry node would then look something like this
> > +
> > +    capsule {
> > +            type = "efi-capsule";
> > +            image-index = <0x1>;
> > +            /* Image GUID for testing capsule update */
> > +            image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
> lower-case. Also please use a name here, e.g. a #fdefine above, if
> this isn't something already in U-Boot. I very much want to avoid
> people adding random UUIDs everywhere.
>
> > +            hardware-instance = <0x0>;
> > +            private-key = "tools/binman/test/key.key";
> > +            pub-key-cert = "tools/binman/test/key.pem";
>
> Drop the path for these

This is just an illustration. I have removed this specific path though.

>
> > +            capoemflags = <0x8000>;
> > +
> > +            u-boot {
> > +            };
> > +    };
> > +
> > +    In the above example, the capsule payload is the u-boot image. The
>
> U-Boot

Okay

>
> > +    capsule entry would read the contents of the payload and put them
> > +    into the capsule. Any external file can also be specified as the
> > +    payload using the blob-ext subnode.
> > +    """
> > +    def __init__(self, section, etype, node):
> > +        super().__init__(section, etype, node)
> > +        self.required_props = ['image-index', 'image-type-id']
> > +        self.image_index = 0
> > +        self.image_guid = ''
> > +        self.hardware_instance = 0
> > +        self.monotonic_count = 0
> > +        self.fw_version = 0
> > +        self.capoemflags = 0
> > +        self.private_key = ''
> > +        self.pub_key_cert = ''
> > +        self.auth = 0
> > +        self.capsule_fname = ''
>
> Please drop

Okay

>
> > +        self._entries = OrderedDict()
>
> entry_Section already does this

Removed

>
> > +
> > +    def ReadNode(self):
> > +        self.ReadEntries()
> > +        super().ReadNode()
> > +
> > +        self.image_index = fdt_util.GetInt(self._node, 'image-index')
> > +        self.image_guid = fdt_util.GetString(self._node, 'image-type-id')
> > +        self.fw_version = fdt_util.GetInt(self._node, 'fw-version')
> > +        self.hardware_instance = fdt_util.GetInt(self._node, 'hardware-instance')
> > +        self.monotonic_count = fdt_util.GetInt(self._node, 'monotonic-count')
> > +        self.capoemflags = fdt_util.GetInt(self._node, 'capoemflags')
> > +
> > +        self.private_key = fdt_util.GetString(self._node, 'private-key')
> > +        self.pub_key_cert = fdt_util.GetString(self._node, 'pub-key-cert')
> > +        if ((self.private_key and not self.pub_key_cert) or (self.pub_key_cert and not self.private_key)):
> > +            self.Raise('Both private key and public key certificate need to be provided')
> > +        elif not (self.private_key and self.pub_key_cert):
> > +            self.auth = 0
> > +        else:
> > +            self.auth = 1
> > +
> > +    def ReadEntries(self):
> > +        """Read the subnode to get the payload for this capsule"""
> > +        # We can have a single payload per capsule
> > +        if len(self._node.subnodes) != 1:
> > +            self.Raise('The capsule entry expects a single subnode for payload')
>
> No, we mustn't restruit this. Why would we?

The mkeficapsule tool that generates the capsule expects a single
image as it's input payload. Why do you see the need to support
multiple entries as payload?

mkeficapsule [options] <image blob> <output file>


>
> > +
> > +        for node in self._node.subnodes:
> > +            entry = Entry.Create(self, node)
> > +            entry.ReadNode()
> > +            self._entries[entry.name] = entry
> > +
> > +    def _GenCapsule(self):
> > +        return self.mkeficapsule.generate_capsule(self.image_index,
> > +                                                  self.image_guid,
> > +                                                  self.hardware_instance,
> > +                                                  self.payload,
> > +                                                  self.capsule_fname,
> > +                                                  self.private_key,
> > +                                                  self.pub_key_cert,
> > +                                                  self.monotonic_count,
> > +                                                  self.fw_version,
> > +                                                  self.capoemflags)
> > +
> > +    def BuildSectionData(self, required):
> > +        if self.auth:
> > +            if not os.path.isabs(self.private_key):
> > +                self.private_key =  tools.get_input_filename(self.private_key)
> > +            if not os.path.isabs(self.pub_key_cert):
> > +                self.pub_key_cert = tools.get_input_filename(self.pub_key_cert)
>
> These should be local vars, not class vars.

?

These are properties being passed by the user. Why should only these
be local vars?

>  Also these are properties
> passed in by the user, so you should not be converting them to
> filenames.

These properties are actually paths to the private and public key cert
files that a user will pass. In the normal usage, the user would be
expected to provide the absolute path to the private and public key
cert files. The above logic is needed for binman tests and out of tree
builds with relative paths.

>
> > +        data, self.payload, _ = self.collect_contents_to_file(
>
> Please don't add new things to the class in the middle of the class.
> Doesn't pylint warn about this? This can just be a local var.

Changed this to a local var.

> > +            self._entries.values(), 'capsule_payload')
> > +        outfile = self._filename if self._filename else self._node.name
> > +        self.capsule_fname = tools.get_output_filename(outfile)
>
> No, the class is for holding properties...you don't use this outside
> the function, so just:
>
> fname = tools....

Changed

>
> > +        if self._GenCapsule() is not None:
>
> Here you need to pass some of the params. Honestly, why not drop
> _GenCapsule() and just call generate_capsule() here? It seems easier.

Done

>
> > +            os.remove(self.payload)
> > +            return tools.read_file(self.capsule_fname)
>
> return tools.read_file(fname)
>
> > +
> > +    def ProcessContents(self):
> > +        ok = super().ProcessContents()
> > +        data = self.BuildSectionData(True)
> > +        ok2 = self.ProcessContentsUpdate(data)
> > +        return ok and ok2
>
> If that is the same as the entry_Section function, you can drop it

There are differences, so this is needed.

>
> > +
> > +    def SetImagePos(self, image_pos):
> > +        upto = 0
> > +        for entry in self.GetEntries().values():
> > +            entry.SetOffsetSize(upto, None)
> > +            upto += entry.size
> > +
> > +        super().SetImagePos(image_pos)
>
> This function looks the same as entry_Section. Drop it?

Yes

>
> > +
> > +    def AddBintools(self, btools):
> > +        self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')
> > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > index 1cfa349d38..7e7926b607 100644
> > --- a/tools/binman/ftest.py
> > +++ b/tools/binman/ftest.py
> > @@ -48,6 +48,7 @@ U_BOOT_VPL_DATA       = b'vpl76543210fedcbazywxyz_'
> >  BLOB_DATA             = b'89'
> >  ME_DATA               = b'0abcd'
> >  VGA_DATA              = b'vga'
> > +EFI_CAPSULE_DATA      = b'efi'
> >  U_BOOT_DTB_DATA       = b'udtb'
> >  U_BOOT_SPL_DTB_DATA   = b'spldtb'
> >  U_BOOT_TPL_DTB_DATA   = b'tpldtb'
> > @@ -119,6 +120,11 @@ COMP_BINTOOLS = ['bzip2', 'gzip', 'lz4', 'lzma_alone', 'lzop', 'xz', 'zstd']
> >
> >  TEE_ADDR = 0x5678
> >
> > +# Firmware Management Protocol(FMP) GUID
> > +FW_MGMT_GUID = 'edd5cb6d2de8444cbda17194199ad92a'
> > +# Image GUID specified in the DTS
> > +CAPSULE_IMAGE_GUID = '52cfd7092007104791d108469b7fe9c8'
> > +
> >  class TestFunctional(unittest.TestCase):
> >      """Functional tests for binman
> >
> > @@ -215,6 +221,7 @@ class TestFunctional(unittest.TestCase):
> >          TestFunctional._MakeInputFile('scp.bin', SCP_DATA)
> >          TestFunctional._MakeInputFile('rockchip-tpl.bin', ROCKCHIP_TPL_DATA)
> >          TestFunctional._MakeInputFile('ti_unsecure.bin', TI_UNSECURE_DATA)
> > +        TestFunctional._MakeInputFile('efi_capsule_payload.bin', EFI_CAPSULE_DATA)
> >
> >          # Add a few .dtb files for testing
> >          TestFunctional._MakeInputFile('%s/test-fdt1.dtb' % TEST_FDT_SUBDIR,
> > @@ -7087,5 +7094,120 @@ fdt         fdtmap                Extract the devicetree blob from the fdtmap
> >           self.assertEqual(fdt_util.GetString(key_node, "key-name-hint"),
> >                            "key")
> >
> > +    def _CheckCapsule(self, data, signed_capsule=False, version_check=False,
> > +                      capoemflags=False):
> > +        fmp_signature = "4d535331" # 'M', 'S', 'S', '1'
> > +        fmp_size = "10"
> > +        fmp_fw_version = "02"
> > +        oemflag = "0080"
> > +
> > +        payload_data = EFI_CAPSULE_DATA
> > +
> > +        # Firmware Management Protocol(FMP) GUID - offset(0 - 32)
> > +        self.assertEqual(FW_MGMT_GUID, data.hex()[:32])
> > +        # Image GUID - offset(96 - 128)
> > +        self.assertEqual(CAPSULE_IMAGE_GUID, data.hex()[96:128])
> > +
> > +        if capoemflags:
> > +            # OEM Flags - offset(40 - 44)
> > +            self.assertEqual(oemflag, data.hex()[40:44])
> > +        if signed_capsule and version_check:
> > +            # FMP header signature - offset(4770 - 4778)
> > +            self.assertEqual(fmp_signature, data.hex()[4770:4778])
> > +            # FMP header size - offset(4778 - 4780)
>
> Can you mention where these values came from and how you created them?
> This all seems very brittle. Is there a tool that prints them out,
> or..?

Like I mentioned earlier as well, the contents of a capsule are
dependent on the input parameters specified in it's generation. For
e.g. the capsule generated would be different when opting for passing
the version param. Similarly when generating a signed capsule -- even
with a signed capsule, the contents would depend on the keys. The
offsets therefore are not fixed but depend on the inputs specified. I
have added comments here for better understanding. Not sure what can
be done to improve this.

This is also one reason why the testing of the EFI capsule update
feature should use capsules generated as part of the sandbox platform
build. That makes the testing that much more robust.

>
> > +            self.assertEqual(fmp_size, data.hex()[4778:4780])
> > +            # firmware version - offset(4786 - 4788)
> > +            self.assertEqual(fmp_fw_version, data.hex()[4786:4788])
> > +            # payload offset signed capsule(4802 - 4808)
> > +            self.assertEqual(payload_data.hex(), data.hex()[4802:4808])
> > +        elif signed_capsule:
> > +            # payload offset signed capsule(4770 - 4776)
> > +            self.assertEqual(payload_data.hex(), data.hex()[4770:4776])
> > +        elif version_check:
> > +            # FMP header signature - offset(184 - 192)
> > +            self.assertEqual(fmp_signature, data.hex()[184:192])
> > +            # FMP header size - offset(192 - 194)
> > +            self.assertEqual(fmp_size, data.hex()[192:194])
> > +            # firmware version - offset(200 - 202)
> > +            self.assertEqual(fmp_fw_version, data.hex()[200:202])
> > +            # payload offset for non-signed capsule with version header(216 - 222)
> > +            self.assertEqual(payload_data.hex(), data.hex()[216:222])
> > +        else:
> > +            # payload offset for non-signed capsule with no version header(184 - 190)
> > +            self.assertEqual(payload_data.hex(), data.hex()[184:190])
> > +
> > +    def testCapsuleGen(self):
> > +        """Test generation of EFI capsule"""
> > +        data = self._DoReadFile('307_capsule.dts')
> > +
> > +        self._CheckCapsule(data)
> > +
> > +    def testSignedCapsuleGen(self):
> > +        """Test generation of EFI capsule"""
> > +        data = tools.read_file(self.TestFile("key.key"))
> > +        self._MakeInputFile("key.key", data)
> > +        data = tools.read_file(self.TestFile("key.pem"))
> > +        self._MakeInputFile("key.crt", data)
> > +
> > +        data = self._DoReadFile('308_capsule_signed.dts')
> > +
> > +        self._CheckCapsule(data, signed_capsule=True)
> > +
> > +    def testCapsuleGenVersionSupport(self):
> > +        """Test generation of EFI capsule with version support"""
> > +        data = self._DoReadFile('309_capsule_version.dts')
> > +
> > +        self._CheckCapsule(data, version_check=True)
> > +
> > +    def testCapsuleGenSignedVer(self):
> > +        """Test generation of signed EFI capsule with version information"""
> > +        data = tools.read_file(self.TestFile("key.key"))
> > +        self._MakeInputFile("key.key", data)
> > +        data = tools.read_file(self.TestFile("key.pem"))
> > +        self._MakeInputFile("key.crt", data)
> > +
> > +        data = self._DoReadFile('310_capsule_signed_ver.dts')
> > +
> > +        self._CheckCapsule(data, signed_capsule=True, version_check=True)
> > +
> > +    def testCapsuleGenCapOemFlags(self):
> > +        """Test generation of EFI capsule with OEM Flags set"""
> > +        data = self._DoReadFile('311_capsule_oemflags.dts')
> > +
> > +        self._CheckCapsule(data, capoemflags=True)
> > +
> > +
> > +    def testCapsuleGenKeyMissing(self):
> > +        """Test that binman errors out on missing key"""
> > +        with self.assertRaises(ValueError) as e:
> > +            self._DoReadFile('312_capsule_missing_key.dts')
> > +
> > +        self.assertIn("Both private key and public key certificate need to be provided",
> > +                      str(e.exception))
> > +
> > +    def testCapsuleGenIndexMissing(self):
> > +        """Test that binman errors out on missing image index"""
> > +        with self.assertRaises(ValueError) as e:
> > +            self._DoReadFile('313_capsule_missing_index.dts')
> > +
> > +        self.assertIn("entry is missing properties: image-index",
> > +                      str(e.exception))
> > +
> > +    def testCapsuleGenGuidMissing(self):
> > +        """Test that binman errors out on missing image GUID"""
> > +        with self.assertRaises(ValueError) as e:
> > +            self._DoReadFile('314_capsule_missing_guid.dts')
> > +
> > +        self.assertIn("entry is missing properties: image-type-id",
> > +                      str(e.exception))
> > +
> > +    def testCapsuleGenPayloadMissing(self):
> > +        """Test that binman errors out on missing input(payload)image"""
> > +        with self.assertRaises(ValueError) as e:
> > +            self._DoReadFile('315_capsule_missing_payload.dts')
> > +
> > +        self.assertIn("The capsule entry expects a single subnode for payload",
> > +                      str(e.exception))
> > +
> >  if __name__ == "__main__":
> >      unittest.main()
> > diff --git a/tools/binman/test/307_capsule.dts b/tools/binman/test/307_capsule.dts
> > new file mode 100644
> > index 0000000000..dd5b6f1c11
> > --- /dev/null
> > +++ b/tools/binman/test/307_capsule.dts
> > @@ -0,0 +1,21 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +       #address-cells = <1>;
> > +       #size-cells = <1>;
> > +
> > +       binman {
> > +               efi-capsule {
> > +                       image-index = <0x1>;
> > +                       /* Image GUID for testing capsule update */
> > +                       image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
> Please create a #include files for these (and use lower case). Please
> fix globally.

Okay. I am adding a file sandbox_efi_capsule.h which contains
information related to the capsule GUIDs and input files needed for
EFI capsule update functionality.

>
> > +                       hardware-instance = <0x0>;
> > +
> > +                       blob-ext {
> > +                               filename = "efi_capsule_payload.bin";
>
> Do you need this filename?

Yes, this is the payload that will be used in generating the capsule.

-sughosh

>
> > +                       };
> > +               };
> > +       };
> > +};
>
> [..]
>
> Regards,
> Simon


More information about the U-Boot mailing list