Add support for `west sign` for Silabs SoCs #92181

jerome-pouiller · 2025-06-25T13:08:59Z

Silabs siwx91x support various security options. This PR exposes these options in west sign.

This PR is a rework of #87669 opened by @Aasimshaik11.

pdgendt · 2025-06-25T13:20:43Z

Re-assigned to Silabs Platforms maintainers.

jhedberg · 2025-06-25T13:32:15Z

doc/develop/west/sign.rst

+silabs_commander
+***************


minor: one more * seems to be needed to be the same length as the subtitle

marc-hb

I'm surprised @Aasimshaik11 is not credited as an author, not even in the commit messages...

https://docs.zephyrproject.org/latest/contribute/modifying_contributions.html

https://www.kernel.org/doc/html/latest/process/submitting-patches.html#when-to-use-acked-by-cc-and-co-developed-by

jerome-pouiller · 2025-06-25T15:45:44Z

I'm surprised @Aasimshaik11 is not credited as an author, not even in the commit messages...

Definitely. Fixed.

marc-hb

The ${CMAKE_COMMAND} -E env ZEPHYR_BASE=${ZEPHYR_BASE} change needs at the very least a separate commit and explanation in the commit message; preferably a separate PR.

A few other, non-blocking requests.

marc-hb · 2025-06-25T18:00:53Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

+    set_target_properties(runners_yaml_props_target
+        PROPERTIES bin_file ${PROJECT_BINARY_DIR}/${KERNEL_NAME}.signed.bin.rps
+    )
+endif()


Please use else() instead of overwriting, much clearer IMHO

marc-hb · 2025-06-25T18:07:45Z

cmake/flash/CMakeLists.txt

@@ -196,7 +196,7 @@ foreach(target flash debug debugserver attach rtt)
        -DDEPENDENCIES="$<TARGET_PROPERTY:${target},MANUALLY_ADDED_DEPENDENCIES>"
        -P ${CMAKE_CURRENT_LIST_DIR}/check_runner_dependencies.cmake
      COMMAND
-        ${CMAKE_COMMAND} -E env
+        ${CMAKE_COMMAND} -E env ZEPHYR_BASE=${ZEPHYR_BASE}


Errr... I really don't think a functional change like that should be hidden in a documentation commit.

While you may require this for some reason (a commit message would be useful), this code change is 100% generic and totally unrelated to Silab or even west sign. Please submit this in a separate PR, get it merged and then wait 1-2 days for nightly test runs to demonstrate it does not break anything or anyone's CI.

This change was not expected (I expect to create another PR for this one). I did a mistake when I did my first rebase.

marc-hb · 2025-06-25T18:09:36Z

scripts/west_commands/sign.py

-        in_file = f'{kernel_prefix}.bin.rps'
-        out_file = command.args.sbin or f'{kernel_prefix}.signed.bin.rps'
+        in_file = f'{kernel_prefix}.rps'
+        out_file = command.args.sbin or f'{kernel_prefix}.signed.rps'


.bin: indeed the most overrated name providing the least information ever...

✔️

This looks fun but you sent a notification for each one :-)

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers

(

marc-hb · 2025-06-25T18:10:55Z

soc/silabs/silabs_siwx91x/CMakeLists.txt

 )
+set_target_properties(runners_yaml_props_target


The name runners_yaml_props_target is quite cryptic, does not seem related to flashing at all. So can you add a one-line comment?

Use runners_yaml_props_target to set the name of the file to flash, rather than hard-coding it for each board. Co-developed-by: Aksel Skauge Mellbye <[email protected]> Signed-off-by: Aksel Skauge Mellbye <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

Disable generation of .hex output, since the Commander runner prefers it over the .bin output but SiWx91x needs a .bin.rps file. Signed-off-by: Aksel Skauge Mellbye <[email protected]> Co-developed-by: Aksel Skauge Mellbye <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

Silabs siwx91x support signed and encrypted firmwares. This PR includes support for these features in "west sign" Co-developed-by: Aasim Shaik <[email protected]> Signed-off-by: Aasim Shaik <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

Once the keys has been provisioned on the Silabs siwx91x, the chip expects the firmware to be properly signed. This PR automate the signing process. Hence, "west flash" will work as expected. Co-developed-by: Aasim Shaik <[email protected]> Signed-off-by: Aasim Shaik <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

Add the relevant documentation for the silabs_commander runner. Co-developed-by: Aasim Shaik <[email protected]> Signed-off-by: Aasim Shaik <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

The siwx91x need a specific firmware image format. These image end with .rps extension. The current name of the image is zephyr.bin.rps. However, the .bin suffix is not relevant. It makes even more sense if we consider the output of west sign: zephyr.signed.bin.rps. We can simplify these name by remove the .bin suffix. Signed-off-by: Jérôme Pouiller <[email protected]>

sonarqubecloud · 2025-06-25T19:23:28Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

Done

marc-hb · 2025-06-25T19:24:20Z

doc/develop/west/sign.rst

+``silabs_commander`` require `Simpliciy Commander`_ to be install on the host. The provisionning of the
+key on the device is described in `UG574 SiWx917 SoC Manufacturing Utility User Guide`_.
+
+.. _Simpliciy Commander:


marc-hb · 2025-06-25T20:26:06Z

scripts/west_commands/sign.py

-        in_file = f'{kernel_prefix}.bin.rps'
-        out_file = command.args.sbin or f'{kernel_prefix}.signed.bin.rps'
+        in_file = f'{kernel_prefix}.rps'
+        out_file = command.args.sbin or f'{kernel_prefix}.signed.rps'


✔️

This looks fun but you sent a notification for each one :-)

https://docs.zephyrproject.org/latest/contribute/contributor_expectations.html#workflow-suggestions-that-help-reviewers

(

github-actions bot added area: Documentation platform: Silabs Silicon Labs area: West West utility labels Jun 25, 2025

github-actions bot requested review from asmellby, carlescufi, jhedberg, kartben, Martinhoff-maker, nashif, pdgendt and swinslow June 25, 2025 13:10

github-actions bot assigned pdgendt Jun 25, 2025

jerome-pouiller force-pushed the west-sign branch from 8a0aa87 to 1819b4a Compare June 25, 2025 13:14

jerome-pouiller requested a review from marc-hb June 25, 2025 13:16

jerome-pouiller added this to the v4.2.0 milestone Jun 25, 2025

pdgendt assigned jhedberg and asmellby and unassigned pdgendt Jun 25, 2025

jhedberg reviewed Jun 25, 2025

View reviewed changes

jerome-pouiller force-pushed the west-sign branch from 1819b4a to 1d87b03 Compare June 25, 2025 13:41

github-actions bot added the area: Build System label Jun 25, 2025

github-actions bot requested review from 57300, jeremybettis, nordicjm and tejlmand June 25, 2025 13:42

jhedberg previously approved these changes Jun 25, 2025

View reviewed changes

marc-hb reviewed Jun 25, 2025

View reviewed changes

marc-hb mentioned this pull request Jun 25, 2025

scripts: west: sign: Add support for image signing for Silabs SiWx91x #87669

Open

jerome-pouiller force-pushed the west-sign branch from 1d87b03 to 0457320 Compare June 25, 2025 15:44

marc-hb previously requested changes Jun 25, 2025

View reviewed changes

jerome-pouiller added 6 commits June 25, 2025 21:18

doc: sign: Describe usage of silabs_commander tool

26222a4

Add the relevant documentation for the silabs_commander runner. Co-developed-by: Aasim Shaik <[email protected]> Signed-off-by: Aasim Shaik <[email protected]> Signed-off-by: Jérôme Pouiller <[email protected]>

jerome-pouiller dismissed jhedberg’s stale review via 3e6eef8 June 25, 2025 19:19

jerome-pouiller force-pushed the west-sign branch from 0457320 to 3e6eef8 Compare June 25, 2025 19:19

marc-hb approved these changes Jun 25, 2025

View reviewed changes

		silabs_commander
		***************

Add support for west sign for Silabs SoCs #92181

Are you sure you want to change the base?

Add support for west sign for Silabs SoCs #92181

Conversation

jerome-pouiller commented Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pdgendt commented Jun 25, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marc-hb left a comment

Choose a reason for hiding this comment

Uh oh!

jerome-pouiller commented Jun 25, 2025

Uh oh!

marc-hb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jerome-pouiller Jun 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Jun 25, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Add support for `west sign` for Silabs SoCs #92181

Add support for `west sign` for Silabs SoCs #92181

jerome-pouiller commented Jun 25, 2025 •

edited

Loading

jerome-pouiller Jun 25, 2025 •

edited

Loading