patrick/zephyr
1
0
Fork 0
Code Issues Pull requests Actions 7 Packages Projects Releases Wiki Activity

intel_adsp: move west sign from west flash to earlier west build

Browse source 
Invoking `west sign` in `west build` accelerates twister because `west
build` is run in parallel, see rationale in superseded and very
different (CMake-based) PR #52942.

To maximize backwards compatibility:
- `west sign` is optional in `west build`
- `west flash` will sign (again) if any rimage --option is passed

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
This commit is contained in:
Marc Herbert 2023-02-24 07:13:23 +00:00 committed by Anas Nashif
parent 2c80c4daa4
commit fad2da39aa
3 changed files with 89 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

51
doc/develop/west/sign.rst
View file

@ -117,3 +117,54 @@ signing system or extending the default behaviour.
.. _imgtool:
https://pypi.org/project/imgtool/
rimage
******
rimage configuration uses a different approach that does not rely on Kconfig or CMake
but on :ref:`west config<west-config>` instead, similar to
:ref:`west-building-cmake-config`.
Signing involves a number of "wrapper" scripts stacked on top of each other: ``west
flash`` invokes ``west build`` which invokes ``cmake`` and ``ninja`` which invokes
``west sign`` which invokes ``imgtool`` or `rimage`_. As long as the signing
parameters desired are the default ones and fairly static, these indirections are
not a problem. On the other hand, passing ``imgtool`` or ``rimage`` options through
all these layers can causes issues typical when the layers don't abstract
anything. First, this usually requires boilerplate code in each layer. Quoting
whitespace or other special characters through all the wrappers can be
difficult. Reproducing a lower ``west sign`` command to debug some build-time issue
can be very time-consuming: it requires at least enabling and searching verbose
build logs to find which exact options were used. Copying these options from the
build logs can be unreliable: it may produce different results because of subtle
environment differences. Last and worst: new signing feature and options are
impossible to use until more boilerplate code has been added in each layer.
To avoid these issues, ``rimage`` parameters can bet set in ``west config``
instead. Here's a ``workspace/.west/config`` example:
.. code-block:: ini
[sign]
# Not needed when invoked from CMake
tool = rimage
[rimage]
# Quoting is optional and works like in Unix shells
# Not needed when rimage can be found in the default PATH
path = "/home/me/zworkspace/build-rimage/rimage"
# Not needed when using the default development key
extra-args = -i 4 -k 'keys/key argument with space.pem'
In order to support quoting, values are parsed by Python's ``shlex.split()`` like in
:ref:`west-building-cmake-args`.
The ``extra-args`` are passed directly to the ``rimage`` command. The example
above has the same effect as appending them on command line after ``--`` like this:
``west sign --tool rimage -- -i 4 -k 'keys/key argument with space.pem'``. In case
both are used, the command-line arguments go last.
.. _rimage:
https://github.com/thesofproject/rimage

30
scripts/west_commands/runners/intel_adsp.py
View file

@ -31,7 +31,8 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
default_key,
key,
pty,
tool_opt
tool_opt,
do_sign,
):
super().__init__(cfg)
@ -50,6 +51,7 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
self.key = os.path.join(DEFAULT_KEY_DIR, default_key)
self.tool_opt_args = tool_opt
self.do_sign = do_sign
@classmethod
def name(cls):
@ -82,6 +84,18 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
@classmethod
def do_create(cls, cfg, args):
# We now have `west flash` -> `west build` -> `west sign` so
# `west flash` -> `west sign` is not needed anymore; it's also
# slower because not concurrent. However, for backwards
# compatibility keep signing here if some explicit rimage
# --option was passed. Some of these options may differ from the
# current `west sign` configuration; we take "precedence" by
# running last.
do_sign = (
args.rimage_tool != DEFAULT_RIMAGE_TOOL or
args.config_dir != DEFAULT_CONFIG_DIR or
args.key is not None
)
return IntelAdspBinaryRunner(cfg,
remote_host=args.remote_host,
rimage_tool=args.rimage_tool,
@ -89,21 +103,15 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
default_key=args.default_key,
key=args.key,
pty=args.pty,
tool_opt=args.tool_opt
tool_opt=args.tool_opt,
do_sign=do_sign,
)
def do_run(self, command, **kwargs):
self.logger.info('Starting Intel ADSP runner')
# Always re-sign because here we cannot know whether `west
# flash` was invoked with `--skip-rebuild` or not and we have no
# way to tell whether there was any code change either.
#
# Signing does not belong here; it should be invoked either from
# some CMakeLists.txt file or an optional hook in some generic
# `west flash` code but right now it's in neither so we have to
# do this.
self.sign(**kwargs)
if self.do_sign:
self.sign(**kwargs)
if re.search("intel_adsp_cavs", self.platform):
self.require(self.cavstool)

19
soc/xtensa/intel_adsp/common/CMakeLists.txt
View file

@ -118,3 +118,22 @@ add_custom_command(
$<TARGET_PROPERTY:bintools,strip_flag_final>
)
endif()
# west sign
add_custom_target(zephyr.ri ALL
DEPENDS ${CMAKE_BINARY_DIR}/zephyr/zephyr.ri
)
add_custom_command(
OUTPUT ${CMAKE_BINARY_DIR}/zephyr/zephyr.ri
COMMENT "west sign --if-tool-available --tool rimage ..."
# Use --if-tool-available so we don't force every CI to install
# rimage. We don't want to break build-only and other tests that don't
# require signing. When rimage is missing, `west flash` fails with a
# clear "zephyr.ri missing" error with an "rimage not found" warning
# from west sign immediately before it.
COMMAND west sign --if-tool-available --tool rimage --build-dir ${CMAKE_BINARY_DIR}
DEPENDS gen_modules
${CMAKE_BINARY_DIR}/zephyr/boot.mod ${CMAKE_BINARY_DIR}/zephyr/main.mod
)