michaelh/zephyr
1
0
Fork 0
Code Releases Activity

intel_adsp: remove rimage sign() from west flash

Browse source 
`west sign` has been invoked by `west build` (through CMake) since
commit fad2da39aa, almost one year ago. During that time, this new
workflow has been refined and successfully used by at least two vendors,
multiple CIs across both SOF and Zephyr and many developers.

At the time, the ability to sign from `west flash` was preserved for
backwards compatibility. This means rimage parameters can come from many
different places at once and that rimage can be invoked twice during a
single `west flash` invocation!

Now that Zephyr 3.5 has been released, we need to reduce the number of
rimage use cases and the corresponding validation complexity and
maintenance workload to simplify and accelerate new features like
splitting rimage configuration files (#65411)

Signed-off-by: Marc Herbert <marc.herbert@intel.com>
This commit is contained in:
Marc Herbert 2023-12-08 00:34:21 +00:00 • committed by Carles Cufí
commit 039e5ef1b8
5 changed files with 17 additions and 68 deletions
Download patch file Download diff file Expand all files Collapse all files

5
boards/common/intel_adsp.board.cmake
View file

@ -1,5 +0,0 @@
# SPDX-License-Identifier: Apache-2.0
board_runner_args(intel_adsp "--default-key=${RIMAGE_SIGN_KEY}")
board_finalize_runner_args(intel_adsp)

2
boards/xtensa/intel_adsp_ace15_mtpm/board.cmake
View file

@ -6,4 +6,4 @@ board_set_rimage_target(mtl)
set(RIMAGE_SIGN_KEY "otc_private_key_3k.pem" CACHE STRING "default in ace15_mtpm/board.cmake")
include(${ZEPHYR_BASE}/boards/common/intel_adsp.board.cmake)
board_finalize_runner_args(intel_adsp)

2
boards/xtensa/intel_adsp_cavs25/board.cmake
View file

@ -17,4 +17,4 @@ if(CONFIG_BOARD_INTEL_ADSP_CAVS25_TGPH)
board_set_rimage_target(tgl-h)
endif()
include(${ZEPHYR_BASE}/boards/common/intel_adsp.board.cmake)
board_finalize_runner_args(intel_adsp)

10
boards/xtensa/intel_adsp_cavs25/doc/intel_adsp_generic.rst
View file

@ -143,12 +143,10 @@ undocumented rimage precedence rules it's best to use only one way at a time.
``boards/my/board/board.cmake``, see example in
``soc/xtensa/intel_adsp/common/CMakeLists.txt``
For backwards compatibility reasons, you can also pass rimage parameters like
the path to the tool binary as arguments to
``west flash`` if the flash target exists for your board. To see a list
of all arguments to the Intel ADSP runner, run the following after you have
built the binary. There are multiple arguments related to signing, including a
key argument.
Starting with Zephyr 3.6.0, ``west flash`` does not invoke ``west sign``
anymore and you cannot pass rimage parameters to ``west flash`` anymore. To
see an up-to-date list of all arguments to the Intel ADSP runner, run the
following after you have built the binary:
.. code-block:: console

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

@ -4,6 +4,7 @@
'''Runner for flashing with the Intel ADSP boards.'''
import argparse
import os
import sys
import re
@ -14,11 +15,12 @@ from runners.core import ZephyrBinaryRunner, RunnerCaps
from zephyr_ext_common import ZEPHYR_BASE
DEFAULT_CAVSTOOL='soc/xtensa/intel_adsp/tools/cavstool_client.py'
DEFAULT_SOF_MOD_DIR=os.path.join(ZEPHYR_BASE, '../modules/audio/sof')
DEFAULT_RIMAGE_TOOL=shutil.which('rimage')
DEFAULT_CONFIG_DIR=os.path.join(DEFAULT_SOF_MOD_DIR, 'tools/rimage/config')
DEFAULT_KEY_DIR=os.path.join(DEFAULT_SOF_MOD_DIR, 'keys')
class SignParamError(argparse.Action):
'User-friendly feedback when trying to sign with west flash'
def __call__(self, parser, namespace, values, option_string=None):
parser.error(f'Cannot use "west flash {option_string} ..." any more. ' +
'"west sign" is now called from CMake, see "west sign -h"')
class IntelAdspBinaryRunner(ZephyrBinaryRunner):
'''Runner front-end for the intel ADSP boards.'''
@ -26,32 +28,19 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
def __init__(self,
cfg,
remote_host,
rimage_tool,
config_dir,
default_key,
key,
pty,
tool_opt,
do_sign,
):
super().__init__(cfg)
self.remote_host = remote_host
self.rimage_tool = rimage_tool
self.config_dir = config_dir
self.bin_fw = os.path.join(cfg.build_dir, 'zephyr', 'zephyr.ri')
self.cavstool = os.path.join(ZEPHYR_BASE, DEFAULT_CAVSTOOL)
self.platform = os.path.basename(cfg.board_dir)
self.pty = pty
if key:
self.key = key
else:
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):
@ -65,18 +54,14 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
def do_add_parser(cls, parser):
parser.add_argument('--remote-host',
help='hostname of the remote targeting ADSP board')
parser.add_argument('--rimage-tool', default=DEFAULT_RIMAGE_TOOL,
help='path to the rimage tool')
parser.add_argument('--config-dir', default=DEFAULT_CONFIG_DIR,
help='path to the toml config file')
parser.add_argument('--default-key',
help='the default basename of the key store in board.cmake')
parser.add_argument('--key',
help='specify where the signing key is')
parser.add_argument('--pty', nargs='?', const="remote-host", type=str,
help=''''Capture the output of cavstool.py running on --remote-host \
and stream it remotely to west's standard output.''')
for old_sign_param in [ '--rimage-tool', '--config-dir', '--default-key', '--key']:
parser.add_argument(old_sign_param, action=SignParamError,
help='do not use, "west sign" is now called from CMake, see "west sign -h"')
@classmethod
def tool_opt_help(cls) -> str:
return """Additional options for run/request service tool,
@ -84,35 +69,15 @@ 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,
config_dir=args.config_dir,
default_key=args.default_key,
key=args.key,
pty=args.pty,
tool_opt=args.tool_opt,
do_sign=do_sign,
)
def do_run(self, command, **kwargs):
self.logger.info('Starting Intel ADSP runner')
if self.do_sign:
self.sign(**kwargs)
if re.search("intel_adsp", self.platform):
self.require(self.cavstool)
self.flash(**kwargs)
@ -120,17 +85,8 @@ class IntelAdspBinaryRunner(ZephyrBinaryRunner):
self.logger.error("No suitable platform for running")
sys.exit(1)
def sign(self, **kwargs):
path_opt = ['-p', f'{self.rimage_tool}'] if self.rimage_tool else []
sign_cmd = (
['west', 'sign', '-d', f'{self.cfg.build_dir}', '-t', 'rimage']
+ path_opt + ['-D', f'{self.config_dir}', '--', '-k', f'{self.key}']
)
self.logger.info(" ".join(sign_cmd))
self.check_call(sign_cmd)
def flash(self, **kwargs):
# Generate a hash string for appending to the sending ri file
'Generate a hash string for appending to the sending ri file'
hash_object = hashlib.md5(self.bin_fw.encode())
random_str = f"{random.getrandbits(64)}".encode()
hash_object.update(random_str)