[PATCH v4 06/12] binman: capsule: Add support for generating capsules
Sughosh Ganu
sughosh.ganu at linaro.org
Mon Jul 17 12:44:20 CEST 2023
hi Simon,
On Sun, 16 Jul 2023 at 05:12, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Sughosh,
>
> On Sat, 15 Jul 2023 at 07:46, Sughosh Ganu <sughosh.ganu at linaro.org> wrote:
> >
> > Add support in binman for generating capsules. The capsule parameters
> > can be specified either through a config file or through the capsule
> > binman entry. Also add test cases in binman for capsule generation,
> > and enable this testing on the sandbox_spl variant.
>
> Can you use sandbox instead, or perhaps sandbox_spl? SPL is really for
> SPL testing.
Er, I am actually using the sandbox_spl variant.
>
> >
> > Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> > ---
> > Changes since V3:
> > * Add test cases for covering the various capsule generation
> > scenarios.
> > * Add function comments in the mkeficapsule bintool.
> > * Fix the fetch method of the mkeficapsule bintool to enable building
> > the tool.
> > * Add more details about the capsule parameters in the documentation
> > as well as the code.
> > * Fix order of module imports, and addition of blank lines in the
> > capsule.py file.
> > * Use SetContents in the ObtainContents method.
> >
> > configs/sandbox_spl_defconfig | 1 +
> > tools/binman/btool/mkeficapsule.py | 158 ++++++++++++++++++
> > tools/binman/entries.rst | 37 ++++
> > tools/binman/etype/capsule.py | 132 +++++++++++++++
> > tools/binman/ftest.py | 127 ++++++++++++++
> > tools/binman/test/282_capsule.dts | 18 ++
> > tools/binman/test/283_capsule_signed.dts | 20 +++
> > tools/binman/test/284_capsule_conf.dts | 14 ++
> > tools/binman/test/285_capsule_missing_key.dts | 19 +++
> > .../binman/test/286_capsule_missing_index.dts | 17 ++
> > .../binman/test/287_capsule_missing_guid.dts | 17 ++
> > .../test/288_capsule_missing_payload.dts | 17 ++
> > tools/binman/test/289_capsule_missing.dts | 17 ++
> > tools/binman/test/290_capsule_version.dts | 19 +++
> > tools/binman/test/capsule_cfg.txt | 6 +
> > 15 files changed, 619 insertions(+)
> > create mode 100644 tools/binman/btool/mkeficapsule.py
> > create mode 100644 tools/binman/etype/capsule.py
> > create mode 100644 tools/binman/test/282_capsule.dts
> > create mode 100644 tools/binman/test/283_capsule_signed.dts
> > create mode 100644 tools/binman/test/284_capsule_conf.dts
> > create mode 100644 tools/binman/test/285_capsule_missing_key.dts
> > create mode 100644 tools/binman/test/286_capsule_missing_index.dts
> > create mode 100644 tools/binman/test/287_capsule_missing_guid.dts
> > create mode 100644 tools/binman/test/288_capsule_missing_payload.dts
> > create mode 100644 tools/binman/test/289_capsule_missing.dts
> > create mode 100644 tools/binman/test/290_capsule_version.dts
> > create mode 100644 tools/binman/test/capsule_cfg.txt
>
> This looks pretty good to me. Some nits below
>
> >
> > diff --git a/configs/sandbox_spl_defconfig b/configs/sandbox_spl_defconfig
> > index dd848c57c6..2fcc789347 100644
> > --- a/configs/sandbox_spl_defconfig
> > +++ b/configs/sandbox_spl_defconfig
> > @@ -248,3 +248,4 @@ CONFIG_UNIT_TEST=y
> > CONFIG_SPL_UNIT_TEST=y
> > CONFIG_UT_TIME=y
> > CONFIG_UT_DM=y
> > +CONFIG_TOOLS_MKEFICAPSULE=y
>
> Why enabling this here? I don't think it is needed in sandbox_spl, but
> in any case it should be in a different patch if needed.
The binman tests run on the sandbox_spl variant. When running the
capsule generation tests, the mkeficapsule tool should be present on
the board variant no?
>
> > diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py
> > new file mode 100644
> > index 0000000000..ba6b666714
> > --- /dev/null
> > +++ b/tools/binman/btool/mkeficapsule.py
> > @@ -0,0 +1,158 @@
> > +# 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
> > + -f, --cfg-file <config file> config file with capsule parameters
> > + -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.
> > +
> > + """
> > + def __init__(self, name):
> > + super().__init__(name, 'mkeficapsule tool for generating capsules')
> > +
> > + def capsule_cfg_file(self, cfg_file):
> > + """Generate a capsule reading parameters from config file
> > +
> > + Args:
> > + cfg_file (str): Path to the config file
> > +
> > + Returns:
> > + str: Tool output
> > + """
> > +
>
> nit: drop blank lines after """ function comment (please fix throughout)
Okay
>
> > + args = [
> > + f'--cfg-file={cfg_file}'
> > + ]
> > + return self.run_cmd(*args)
> > +
> > + def cmdline_capsule(self, image_index, image_guid, hardware_instance,
> > + payload, output_fname, version=0):
> > + """Generate a capsule through commandline provided parameters
> > +
> > + 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
> > + version (int): Image version (Optional)
> > +
> > + Returns:
> > + str: Tool output
> > + """
> > +
> > + if version:
> > + args = [
> > + f'--index={image_index}',
> > + f'--fw-version={version}',
> > + f'--guid={image_guid}',
> > + f'--instance={hardware_instance}',
> > + payload,
> > + output_fname
> > + ]
> > + else:
> > + args = [
> > + f'--index={image_index}',
> > + f'--guid={image_guid}',
> > + f'--instance={hardware_instance}',
> > + payload,
> > + output_fname
> > + ]
> > +
> > + return self.run_cmd(*args)
> > +
> > + def cmdline_auth_capsule(self, image_index, image_guid, hardware_instance,
> > + monotonic_count, priv_key, pub_key,
> > + payload, output_fname, version=0):
> > + """Generate a signed capsule through commandline provided parameters
> > +
> > + 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
> > + monotonic_count (int): Count used when signing an image
> > + priv_key (str): Path to the private key
> > + pub_key(str): Path to the public key
> > + payload (str): Path to the input payload image
> > + output_fname (str): Path to the output capsule file
> > + version (int): Image version (Optional)
> > +
> > + Returns:
> > + str: Tool output
> > + """
> > +
> > + if version:
> > + args = [
> > + f'--index={image_index}',
> > + f'--guid={image_guid}',
> > + f'--instance={hardware_instance}',
> > + f'--monotonic-count={monotonic_count}',
> > + f'--private-key={priv_key}',
> > + f'--certificate={pub_key}',
> > + f'--fw-version={version}',
> > + payload,
> > + output_fname
> > + ]
> > + else:
> > + args = [
> > + f'--index={image_index}',
> > + f'--guid={image_guid}',
> > + f'--instance={hardware_instance}',
> > + f'--monotonic-count={monotonic_count}',
> > + f'--private-key={priv_key}',
> > + f'--certificate={pub_key}',
> > + 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 b71af801fd..523c8222f5 100644
> > --- a/tools/binman/entries.rst
> > +++ b/tools/binman/entries.rst
> > @@ -283,6 +283,43 @@ entry; similarly for SPL.
> >
> >
> >
> > +.. _etype_capsule:
> > +
> > +Entry: capsule: Entry for generating EFI Capsule files
> > +------------------------------------------------------
> > +
> > +This is an entry for generating EFI capsules.
> > +
> > +The parameters needed for generation of the capsules can either be
> > +provided separately, or through a config file.
> > +
> > +Properties / Entry arguments:
> > + - cfg-file: Config file for providing capsule
> > + parameters. These are parameters needed for generating the
> > + capsules. The parameters can be listed by running the
> > + './tools/mkeficapsule -h' command.
> > + - 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.
> > + - monotomic-count: Count used when signing an image.
> > + - private-key: Path to PEM formatted .key private key file.
> > + - pub-key-cert: Path to PEM formatted .crt public key certificate
> > + file.
> > + - filename: Path to the input(payload) file. File can be any
> > + format, a binary or an elf, platform specific.
> > + - capsule: Path to the output capsule file. A capsule is a
> > + continous set of data as defined by the EFI specification. Refer
> > + to the specification for more details.
> > +
> > +
> > +
> > .. _etype_cbfs:
> >
> > Entry: cbfs: Coreboot Filesystem (CBFS)
> > diff --git a/tools/binman/etype/capsule.py b/tools/binman/etype/capsule.py
> > new file mode 100644
> > index 0000000000..46e5507704
> > --- /dev/null
> > +++ b/tools/binman/etype/capsule.py
> > @@ -0,0 +1,132 @@
> > +# SPDX-License-Identifier: GPL-2.0+
> > +# Copyright (c) 2023 Linaro Limited
> > +#
> > +# Entry-type module for producing a capsule
> > +#
> > +
> > +import os
> > +
> > +from binman.entry import Entry
> > +from dtoc import fdt_util
> > +from u_boot_pylib import tools
> > +
> > +class Entry_capsule(Entry):
> > + """Entry for generating EFI capsules
> > +
> > + This is an entry for generating EFI capsules.
> > +
> > + The parameters needed for generation of the capsules can
> > + either be provided separately, or through a config file.
> > +
> > + Properties / Entry arguments:
> > + - cfg-file: Config file for providing capsule
> > + parameters. These are parameters needed for generating the
> > + capsules. The parameters can be listed by running the
> > + './tools/mkeficapsule -h' command.
> > + - 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.
> > + - monotomic-count: Count used when signing an image.
>
> monotonic
>
> > + - private-key: Path to PEM formatted .key private key file.
> > + - pub-key-cert: Path to PEM formatted .crt public key certificate
> > + file.
> > + - filename: Path to the input(payload) file. File can be any
> > + format, a binary or an elf, platform specific.
> > + - capsule: Path to the output capsule file. A capsule is a
> > + continous set of data as defined by the EFI specification. Refer
>
> continuous
>
> Can you add a link to EFI spec so it appears in the docs here?
Will fix the above typos and add the link.
>
> > + to the specification for more details.
> > +
> > + """
> > + def __init__(self, section, etype, node):
> > + super().__init__(section, etype, node)
> > + self.image_index = 0
> > + self.image_guid = ""
> > + self.hardware_instance = 0
> > + self.monotonic_count = 0
> > + self.fw_version = 0
> > + self.private_key = ""
> > + self.pub_key_cert = ""
> > + self.auth = 0
> > + self.payload = ""
> > + self.capsule_fname = ""
>
> Please remember to use single quotes
Okay
>
> > +
> > + def ReadNode(self):
> > + super().ReadNode()
> > +
> > + self.cfg_file = fdt_util.GetString(self._node, 'cfg-file')
> > + if not self.cfg_file:
> > + self.image_index = fdt_util.GetInt(self._node, 'image-index')
> > + if not self.image_index:
> > + self.Raise('mkeficapsule must be provided an Image Index')
> > +
> > + self.image_guid = fdt_util.GetString(self._node, 'image-type-id')
> > + if not self.image_guid:
> > + self.Raise('mkeficapsule must be provided an Image GUID')
>
> Use self.required_props = ['image-type-id', ...] in your __init__().
> Then this is automatic
I should have clarified this during the earlier version itself. So
these parameters are mandatory only when not using the config file. In
the scenario of generating the capsules through config files, all
these parameters are provided through the config file. Hence these
explicit checks.
>
> > +
> > + 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.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
> > +
> > + self.payload = fdt_util.GetString(self._node, 'filename')
> > + if not self.payload:
> > + self.Raise('mkeficapsule must be provided an input filename(payload)')
> > +
> > + if not os.path.isabs(self.payload):
> > + self.payload_path = tools.get_input_filename(self.payload)
> > + if not os.path.exists(self.payload_path):
> > + self.Raise('Cannot resolve path to the input filename(payload)')
> > + else:
> > + self.payload = self.payload_path
> > +
> > + self.capsule_fname = fdt_util.GetString(self._node, 'capsule')
> > + if not self.capsule_fname:
> > + self.Raise('Specify the output capsule file')
> > +
> > + if not os.path.isabs(self.capsule_fname):
> > + self.capsule_path = tools.get_output_filename(self.capsule_fname)
> > + self.capsule_fname = self.capsule_path
> > +
> > + def _GenCapsule(self):
>
> What if some of the inputs are missing? Does this handle missing blobs?
Any missing input parameters are checked earlier itself.
> > + if self.cfg_file:
> > + return self.mkeficapsule.capsule_cfg_file(self.cfg_file)
> > + elif self.auth:
> > + return self.mkeficapsule.cmdline_auth_capsule(self.image_index,
> > + self.image_guid,
> > + self.hardware_instance,
> > + self.monotonic_count,
> > + self.private_key,
> > + self.pub_key_cert,
> > + self.payload,
> > + self.capsule_fname,
> > + self.fw_version)
> > + else:
> > + return self.mkeficapsule.cmdline_capsule(self.image_index,
> > + self.image_guid,
> > + self.hardware_instance,
> > + self.payload,
> > + self.capsule_fname,
> > + self.fw_version)
> > +
> > + def ObtainContents(self):
> > + self.SetContents(tools.to_bytes(self._GenCapsule()))
> > + return True
> > +
> > + def AddBintools(self, btools):
> > + self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')
> > diff --git a/tools/binman/ftest.py b/tools/binman/ftest.py
> > index 43b4f850a6..f5dd62d028 100644
> > --- a/tools/binman/ftest.py
> > +++ b/tools/binman/ftest.py
> > @@ -6676,6 +6676,133 @@ fdt fdtmap Extract the devicetree blob from the fdtmap
> > ['fit'])
> > self.assertIn("Node '/fit': Missing tool: 'mkimage'", str(e.exception))
> >
> > + def _CheckCapsule(self, signed_capsule=False, version_check=False):
> > + # Firmware Management Protocol GUID used in capsule header
> > + self.capsule_guid = "edd5cb6d2de8444cbda17194199ad92a"
>
> Please add a constant FW_MGMT_GUID = '' at the top of the file
Okay. Will add for the capsule_guid and image_guid values.
>
> We really should not have GUIDs in the code...they are a mess.
You want the UEFI capsule generation to happen through binman, and not
mention GUIDs. That ain't happening :)
>
> > + # Image GUID specified in the DTS
> > + self.image_guid = "52cfd7092007104791d108469b7fe9c8"
> > + self.fmp_signature = "4d535331"
> > + self.fmp_size = "10"
> > + self.fmp_fw_version = "02"
>
> These should really be local vars, not members.
Okay
>
> > +
> > + self.capsule_data = tools.read_file(self.capsule_fname)
>
> Pass the data in here and then you don't need to read the file
So the file needs to be read here since the actual capsule generation
tool(tools/mkeficapsule) does not return any capsule data. Instead,
the data gets written to the capsule file, and the tool just returns a
pass/fail status.
>
> > +
> > + self.assertEqual(self.capsule_guid, self.capsule_data.hex()[:32])
> > + self.assertEqual(self.image_guid, self.capsule_data.hex()[96:128])
> > +
> > + if version_check:
> > + self.assertEqual(self.fmp_signature, self.capsule_data.hex()[184:192])
> > + self.assertEqual(self.fmp_size, self.capsule_data.hex()[192:194])
> > + self.assertEqual(self.fmp_fw_version, self.capsule_data.hex()[200:202])
> > +
> > + if signed_capsule:
> > + self.assertEqual(self.payload_data.hex(), self.capsule_data.hex()[4770:4778])
>
> Where do these integer offsets come from? Please add a comment
So, these are simply offsets in the output capsule file, which get
impacted based on the contents being put in the capsule, like
presence/absence of optional headers. I don't think putting a comment
really wll add any value, because these offsets are variable.
>
> > + elif version_check:
> > + self.assertEqual(self.payload_data.hex(), self.capsule_data.hex()[216:224])
> > + else:
> > + self.assertEqual(self.payload_data.hex(), self.capsule_data.hex()[184:192])
> > +
> > + def _GenCapsuleCfgPayload(self, fname, contents):
> > + capsule_dir = '/tmp/capsules/'
>
> You can't write to /tmp
>
> Please use self._indir for input files - see how other tests do it
For all other tests, I am indeed using _indir and outdir. But for
generation of capsules through a config file, we need to specify the
location where the output capsule file will be written to. Which is
the reason for the /tmp/capsules/. We are using this directory for
collating all capsule related files.
>
> > + pathname = os.path.join(capsule_dir, fname)
> > + dirname = os.path.dirname(pathname)
> > + if dirname and not os.path.exists(dirname):
> > + os.makedirs(dirname)
> > +
> > + with open(pathname, 'wb') as fd:
> > + fd.write(contents)
>
> tools.write_file(pathname, contents)
Okay
>
> > +
> > + def testCapsuleGen(self):
> > + """Test generation of EFI capsule"""
> > + self.payload_data = tools.to_bytes(TEXT_DATA)
>
> Please can you use your own test data, like EFI_DATA ? Also if you
> declare it as a binary string you can drop the call.
>
> For example:
>
> EFI_DATA = b'efi'
I don't know why text_data cannot be used, but I will add the EFI_DATA.
>
> > +
> > + TestFunctional._MakeInputFile('payload.txt', self.payload_data)
> > +
> > + self._DoReadFile('282_capsule.dts')
>
> data = self...
Please see above. We need to read the capsule file. This applies for
all the related comments about using the data = self._DoReadFile...
> > +
> > + self.capsule_fname = tools.get_output_filename('test.capsule')
> > + self.assertTrue(os.path.exists(self.capsule_fname))
>
> You can drop that line
>
> > +
> > + self._CheckCapsule()
>
> self._CheckCapsule(data)
>
> > +
> > + def testSignedCapsuleGen(self):
> > + """Test generation of EFI capsule"""
> > + self.payload_data = tools.to_bytes(TEXT_DATA)
> > +
> > + TestFunctional._MakeInputFile('payload.txt', self.payload_data)
> > +
> > + self._DoReadFile('283_capsule_signed.dts')
>
> data = self...
>
> That is the actual data, so you don't need to read it below.
>
> > +
> > + self.capsule_fname = tools.get_output_filename('test.capsule')
> > + self.assertTrue(os.path.exists(self.capsule_fname))
> > +
> > + self._CheckCapsule(signed_capsule=True)
> > +
> > + def testCapsuleGenCfgFile(self):
> > + """Test generation of EFI capsule through config file"""
> > + self.payload_data = tools.to_bytes(TEXT_DATA)
> > +
> > + self._GenCapsuleCfgPayload('payload.txt', self.payload_data)
> > +
> > + self._DoReadFile('284_capsule_conf.dts')
> > +
> > + self.capsule_fname = '/tmp/capsules/test.capsule'
> > + self.assertTrue(os.path.exists(self.capsule_fname))
> > +
> > + self._CheckCapsule()
> > +
> > + def testCapsuleGenVersionSupport(self):
> > + """Test generation of EFI capsule with version support"""
> > + self.payload_data = tools.to_bytes(TEXT_DATA)
> >
> > + TestFunctional._MakeInputFile('payload.txt', self.payload_data)
> > +
> > + self._DoReadFile('290_capsule_version.dts')
> > +
> > + self.capsule_fname = tools.get_output_filename('test.capsule')
> > + self.assertTrue(os.path.exists(self.capsule_fname))
> > +
> > + self._CheckCapsule(version_check=True)
> > +
> > + def testCapsuleGenKeyMissing(self):
> > + """Test that binman errors out on missing key"""
> > + with self.assertRaises(ValueError) as e:
> > + self._DoReadFile('285_capsule_missing_key.dts')
> > +
> > + self.assertIn("Both private-key and public key Certificate need to be provided",
>
> private key
> certificate
Okay
>
> > + str(e.exception))
> > +
> > + def testCapsuleGenIndexMissing(self):
> > + """Test that binman errors out on missing image index"""
> > + with self.assertRaises(ValueError) as e:
> > + self._DoReadFile('286_capsule_missing_index.dts')
> > +
> > + self.assertIn("mkeficapsule must be provided an 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('287_capsule_missing_guid.dts')
> > +
> > + self.assertIn("mkeficapsule must be provided an Image GUID",
> > + str(e.exception))
> > +
> > + def testCapsuleGenPayloadMissing(self):
> > + """Test that binman errors out on missing input(payload)image"""
> > + with self.assertRaises(ValueError) as e:
> > + self._DoReadFile('288_capsule_missing_payload.dts')
> > +
> > + self.assertIn("mkeficapsule must be provided an input filename(payload)",
> > + str(e.exception))
> > +
> > + def testCapsuleGenCapsuleFileMissing(self):
> > + """Test that binman errors out on missing output capsule file"""
> > + with self.assertRaises(ValueError) as e:
> > + self._DoReadFile('289_capsule_missing.dts')
> > +
> > + self.assertIn("Specify the output capsule file",
> > + str(e.exception))
>
> This looks good. It is a pain that you need to cover each missing arg.
> I'm not sure I can think of a better way.
>
> > +
> > if __name__ == "__main__":
> > unittest.main()
> > diff --git a/tools/binman/test/282_capsule.dts b/tools/binman/test/282_capsule.dts
> > new file mode 100644
> > index 0000000000..0e7fcdf8ab
> > --- /dev/null
> > +++ b/tools/binman/test/282_capsule.dts
> > @@ -0,0 +1,18 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > + #address-cells = <1>;
> > + #size-cells = <1>;
> > +
> > + binman {
> > + capsule {
> > + image-index = <0x1>;
> > + image-type-id = "09D7CF52-0720-4710-91D1-08469B7FE9C8";
>
> Is there a #define somewhere for this? Perhaps you can add a #define,
> or a comment as to what this is.
I will put a comment.
>
> Also, please use lower case.
Okay
-sughosh
>
> > + hardware-instance = <0x0>;
> > + filename = "payload.txt";
> > + capsule = "test.capsule";
> > + };
> > + };
> > +};
>
> Regards,
> Simon
More information about the U-Boot
mailing list