[PATCH v3 06/11] binman: capsule: Add support for generating capsules

Simon Glass sjg at chromium.org
Mon Jul 10 23:38:34 CEST 2023


Hi Sughosh,

On Sun, 9 Jul 2023 at 07:34, 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.
>
> Signed-off-by: Sughosh Ganu <sughosh.ganu at linaro.org>
> ---
> Changes since V2:
> * New patch which generates capsules through binman replacing the
>   earlier make target.
>
>  tools/binman/btool/mkeficapsule.py |  91 +++++++++++++++++++++++++
>  tools/binman/entries.rst           |  27 ++++++++
>  tools/binman/etype/capsule.py      | 102 +++++++++++++++++++++++++++++
>  3 files changed, 220 insertions(+)
>  create mode 100644 tools/binman/btool/mkeficapsule.py
>  create mode 100644 tools/binman/etype/capsule.py

Please do check test coverage (binman test -T). You are missing quite
a lot in two two files you have added.

>
> diff --git a/tools/binman/btool/mkeficapsule.py b/tools/binman/btool/mkeficapsule.py
> new file mode 100644
> index 0000000000..9f656c12cf
> --- /dev/null
> +++ b/tools/binman/btool/mkeficapsule.py
> @@ -0,0 +1,91 @@
> +# 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
> +       -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):

"""Function comment

Args:
   cfg_file (str): ...
"""

Please fix throughout

> +
> +        args = [
> +            f'--cfg-file={cfg_file}'
> +        ]
> +        self.run_cmd(*args)
> +
> +    def cmdline_capsule(self, image_index, image_guid, hardware_instance,
> +                        payload, output_fname):
> +
> +        args = [
> +            f'--index={image_index}',
> +            f'--guid={image_guid}',
> +            f'--instance={hardware_instance}',
> +            payload,
> +            output_fname
> +        ]
> +        self.run_cmd(*args)
> +
> +    def cmdline_auth_capsule(self, image_index, image_guid, hardware_instance,
> +                             monotonic_count, priv_key, pub_key,
> +                             payload, output_fname):
> +
> +        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
> +        ]
> +        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
> +        result = self.build_from_git(
> +            'https://source.denx.de/u-boot/u-boot.git',
> +            'tools',
> +            'tools/mkeficapsule')
> +        return result

Can we just install u-boot-tools? But if not, this is OK.

But it does not actually work:

$ binman tool -f mkeficapsule
Fetch: mkeficapsule
- trying method: binary download
- trying method: build from source
- clone git repo 'https://source.denx.de/u-boot/u-boot.git' to
'/tmp/binmanf.chfbsxc0'
- build target 'tools'
Exception: Error 2 running 'make -C /tmp/binmanf.chfbsxc0 -j 64 tools': ***
*** Configuration file ".config" not found!
***
*** Please run some configurator (e.g. "make oldconfig" or
*** "make menuconfig" or "make xconfig").
***
make[2]: *** [scripts/kconfig/Makefile:75: syncconfig] Error 1
make[1]: *** [Makefile:576: syncconfig] Error 2
make: *** No rule to make target 'include/config/auto.conf', needed by
'include/config/uboot.release'.  Stop.

- failed to fetch with all methods

I think you need a make tools_defconfig in there.

> diff --git a/tools/binman/entries.rst b/tools/binman/entries.rst
> index b71af801fd..9a263e8691 100644
> --- a/tools/binman/entries.rst
> +++ b/tools/binman/entries.rst
> @@ -283,6 +283,33 @@ 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:

Please add more detail to answer below questions.

> +    - cfg-file: Config file for providing capsule
> +      parameters.

What parameters? Please provide a link

> +    - image-index: Unique number for identifying
> +      corresponding payload image.

Is this indexed from 0 ?

> +    - image-type-id: Image GUID which will be used
> +      for identifying the image.

In what way is it identifying it? Is it identifying the board that the
image is for, or something else?

> +    - hardware-instance: Optional number for identifying
> +      unique hardware instance of a device in the system.

Is this numbered from 0?

> +    - monotomic-count: Count used when signing an image.

monotonic

What count? What is it used for?

> +    - private-key: Path to private key file.

File format?

> +    - pub-key-cert: Path to public key certificate file.

File format?

> +    - filename: Path to the input(payload) file.

This is just a binary file to be signed, right?

> +    - capsule: Path to the output capsule file.

File format?

> +
> +
> +
>  .. _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..a5ada885a6
> --- /dev/null
> +++ b/tools/binman/etype/capsule.py
> @@ -0,0 +1,102 @@
> +# SPDX-License-Identifier: GPL-2.0+
> +# Copyright (c) 2023 Linaro Limited
> +#
> +# Entry-type module for producing a capsule
> +#
> +
> +import os

blank line here (I think it is the convention to put a blank line
after OS includes)

> +from u_boot_pylib import tools
> +from dtoc import fdt_util
> +from binman.entry import Entry

We should order these in reverse. I suspect that the conversion I did
from patman to u_boot_pylib was not correct, as I probably forgot to
fix the order.

> +
> +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.
> +        - image-index: Unique number for identifying
> +          corresponding payload image.
> +        - image-type-id: Image GUID which will be used
> +          for identifying the image.
> +        - hardware-instance: Optional number for identifying
> +          unique hardware instance of a device in the system.
> +        - monotomic-count: Count used when signing an image.
> +        - private-key: Path to private key file.
> +        - pub-key-cert: Path to public key certificate file.
> +        - filename: Path to the input(payload) file.
> +        - capsule: Path to the output capsule file.

copy docs from above to here, when updated

> +
> +    """
> +    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.private_key = ""
> +        self.pub_key_cert = ""
> +        self.auth = 0
> +        self.payload = ""
> +        self.capsule_fname = ""
> +
> +    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')

You can handle this with something like this in your __init__():

self.required_props = ['image-index', 'image-type-id', ...]

Sadly the docs for required_props are wrong.

> +
> +            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)')
> +
> +            self.capsule_fname = fdt_util.GetString(self._node, 'capsule')
> +            if not self.capsule_fname:
> +                self.capsule_fname = os.path.splitext(self.payload)[0] + '.capsule'
> +
> +    def ObtainContents(self):
> +        if self.cfg_file:
> +            self.mkeficapsule.capsule_cfg_file(self.cfg_file)
> +        elif self.auth:
> +            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)
> +        else:
> +            self.mkeficapsule.cmdline_capsule(self.image_index,
> +                                              self.image_guid,
> +                                              self.hardware_instance,
> +                                              self.payload,
> +                                              self.capsule_fname)

This does not actually obtain the contents. I think you need:

self.SetContents(...)

here.

> +
> +    def AddBintools(self, btools):
> +        self.mkeficapsule = self.AddBintool(btools, 'mkeficapsule')
> --
> 2.34.1
>

Regards,
Simon


More information about the U-Boot mailing list