Backport 28259 to master #29152

engdoreis · 2026-01-19T16:58:14Z

This PR is a manual Backport of #28259.

The conflict was caused by commit 4790641, which also modified sim.mk. I have resolved the conflict in an additional commit within this PR, which can be squashed after the review if preferred.

marnovandermaas

Just some comments from an initial read through.

hw/dv/tools/dvsim/sim.mk

util/py/scripts/build_sw_collateral_for_sim.py

marnovandermaas · 2026-01-21T09:55:44Z

Any idea why CI is complaining about?
ERROR: Skipping '//hw/ip/otp_ctrl/data:img_seed': no such package 'hw/ip/otp_ctrl/data': BUILD file not found in any of the following directories. Add a BUILD file to a directory to mark it as a package.

Replace some heavily-nested Makefile target code with a dedicated python script that invokes Bazel to build the needed software collateral for a simulation. This script does not fundamentally change any of logic for building software, but repackages it in a form that allows proper documentation and modularity. Signed-off-by: Harry Callahan <[email protected]> (cherry picked from commit 9989803)

Signed-off-by: Harry Callahan <[email protected]> (cherry picked from commit 6437f96)

This creates a new run_mode 'sw_test_mode_base', which currently only provides the following run opts: +sw_build_device and +sw_images Signed-off-by: Harry Callahan <[email protected]> (cherry picked from commit 85b1517)

This commit replicate the changes made to the sim.mk file in the commit 4790641 Signed-off-by: Douglas Reis <[email protected]>

engdoreis · 2026-01-22T18:08:44Z

There were some problems with darjeeling that @hcallahan-lowrisc helped me to investigate and fix.
I should pass CI now.

marnovandermaas

Ideally, @hcallahan-lowrisc would review this code.

marnovandermaas · 2026-01-28T16:35:41Z

util/py/scripts/build_sw_collateral_for_sim.py

-    # If build_seed is provided, feed this value into bazel when building the OTP pre-load images.
-    # This overrides the default seed value and is needed for reproducibility to match the
-    # synthesized hardware from the 'build' stage. (sw build happens in the 'run' stage)
-    if args.build_seed != "None":
-        bazel_runner.build_opts += [
-            f"--//hw/ip/otp_ctrl/data:lc_seed={args.build_seed}",
-            f"--//hw/ip/otp_ctrl/data:otp_seed={args.build_seed}",
-        ]
-


Why are you removing the build seed here? The commit message says you're making changes to sim.mk but this is a different file.

The original PR extracted the shell script inside the sim.mk to a python script build_sw_collateral_for_sim.py. So in order to backport this PR to master, all the changes made to sim.mk after the earlgrey_1.0.0 forked must be replicated in build_sw_collateral_for_sim.py.

jwnrt · 2026-02-03T11:08:06Z

As a backport it looks okay, but I don't know enough about this flow to give a good review

AlexJones0

Thanks for the backport, this is definitely a big improvement over the original Makefile logic. I've primarily focused on reviewing the new Python script, but have briefly looked over other changes. The majority of the comments are about the original PR, not this backport specifically. As this is a backport (and a big improvement), I'm okay for it to go in without most the comments being addressed and we can defer these to an issue / separate PR.

Generally most of the comments should be quick fixes or suggestions, but there are a couple that ideally I would like to be resolved before approving this PR - specifically, the comments about the docs on the sw_type enum and about the de-duplication in the last commit that passes the sw build opt to cquery.

AlexJones0 · 2026-02-05T10:57:20Z

hw/dv/tools/dvsim/bazel.hjson

 // SPDX-License-Identifier: Apache-2.0
 {
-  sw_build_cmd: "bazel"
+  sw_build_cmd: [


Minor nit: maybe this should be called sw_build.hjson, or even bazel_sw.hjson instead? At the moment it's slightly confusing to just have it named bazel.hjson when its purpose is actually just to build the sw collateral (via Bazel) - "Bazel" should not be synonymous with "SW collateral". I think the naming issue here just becomes more pronounced with the change to a python script that uses Bazel.

util/py/scripts/build_sw_collateral_for_sim.py

AlexJones0 · 2026-02-05T11:59:07Z

util/py/scripts/build_sw_collateral_for_sim.py

+        type=_space_sep_str,
+        default=[],


Nit (repeated for --sw-build-opts): the idiomatic way would be to have nargs="*" with no specified type and then just have your shell and argparse handle splitting on whitespace for you, and then give the args like e.g.

./util/py/scripts/build_sw_collateral_for_sim.py --sw-images img_one img_two img_three

etc., but that might require some small changes to the HJSON to support. At the moment this works well for the makefile flow but won't play particularly nice with manual invocations.

util/py/scripts/build_sw_collateral_for_sim.py

AlexJones0 · 2026-02-05T13:09:33Z

util/py/scripts/build_sw_collateral_for_sim.py

+        dev = args.sw_build_device
+
+        def _f_endswith_dev(f: Path) -> bool:
+            f_stem = str(f).split(".")[0]
+            return f_stem.endswith(dev)


It looks like this --sw-build-device argument should be marked as required=True in the argparse parser? Or this code should handle the None case.

The same is true with args.run_dir later on in the copy_files_to_run_dir call, which doesn't handle the None case for its run_dir arg.

AlexJones0 · 2026-02-05T13:13:57Z

util/py/scripts/build_sw_collateral_for_sim.py

+    if logger.level <= logging.INFO:
+        location_query = bazel_runner.query(iq.label, ["--output=location"])
+        location = location_query[0].split(" ")[0]
+        logger.info(f"bazel target location: {location}")


I wonder if this script should default to logging.WARNING and take a -v/--verbose option? As right now _main always configures to logging.INFO which means that we are making these additional expensive Bazel queries per image for any SW collateral build we do (despite good logging practice here!) This could add up over lots of SW images.

util/py/scripts/build_sw_collateral_for_sim.py

This is needed on master branch only because of darjeeling, otherwise cquery would not find darjeeling artefacts. Signed-off-by: Douglas Reis <[email protected]>

AlexJones0

Looks OK to me modulo a couple of quick comments on your fixes 😅

As already mentioned, I'm happy to defer my remaining comments to an issue or future PR in the interest of making the backporting easier. If you haven't already, it would probably be a good idea to check the DV flow with the latest changes before merging.

util/py/scripts/build_sw_collateral_for_sim.py

Signed-off-by: Douglas Reis <[email protected]>

AlexJones0

Thanks for the changes! As before, I am happy for the rest of the comments to be deferred so we can get this backported.

engdoreis requested review from hcallahan-lowrisc and pamaury January 19, 2026 16:58

engdoreis requested a review from a team as a code owner January 19, 2026 16:58

engdoreis force-pushed the backport-28259-to-master branch from 4a1ec76 to 128ee8c Compare January 19, 2026 17:28

marnovandermaas reviewed Jan 19, 2026

View reviewed changes

hw/dv/tools/dvsim/sim.mk Show resolved Hide resolved

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

engdoreis force-pushed the backport-28259-to-master branch from 128ee8c to 444fe63 Compare January 20, 2026 18:08

engdoreis force-pushed the backport-28259-to-master branch 5 times, most recently from 2150a5a to 99e9194 Compare January 21, 2026 18:04

hcallahan-lowrisc and others added 4 commits January 22, 2026 14:25

[dv] Cleanup prints in sim.mk

bf1555d

Signed-off-by: Harry Callahan <[email protected]> (cherry picked from commit 6437f96)

[util] Replicate the commit 4790641

1ea3bed

This commit replicate the changes made to the sim.mk file in the commit 4790641 Signed-off-by: Douglas Reis <[email protected]>

engdoreis force-pushed the backport-28259-to-master branch 2 times, most recently from 2ef8be7 to eea84dd Compare January 22, 2026 14:35

engdoreis requested a review from marnovandermaas January 22, 2026 18:07

marnovandermaas reviewed Jan 28, 2026

View reviewed changes

engdoreis requested review from jwnrt and marnovandermaas February 2, 2026 08:52

hcallahan-lowrisc requested a review from AlexJones0 February 4, 2026 12:03

AlexJones0 reviewed Feb 5, 2026

View reviewed changes

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

[dv] Add bazel opts to cquery

c8c4366

This is needed on master branch only because of darjeeling, otherwise cquery would not find darjeeling artefacts. Signed-off-by: Douglas Reis <[email protected]>

engdoreis force-pushed the backport-28259-to-master branch 2 times, most recently from 6776896 to 800d7f1 Compare February 6, 2026 14:31

engdoreis force-pushed the backport-28259-to-master branch from 800d7f1 to 1ca7e87 Compare February 6, 2026 14:39

AlexJones0 approved these changes Feb 6, 2026

View reviewed changes

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

util/py/scripts/build_sw_collateral_for_sim.py Outdated Show resolved Hide resolved

engdoreis added 2 commits February 9, 2026 10:11

[dv] Small refactor based on feedback from backport PR.

141af19

Signed-off-by: Douglas Reis <[email protected]>

[dv] Add more documentation to the sw_type_e

13ff019

Signed-off-by: Douglas Reis <[email protected]>

engdoreis force-pushed the backport-28259-to-master branch from 1ca7e87 to 13ff019 Compare February 9, 2026 10:11

AlexJones0 approved these changes Feb 9, 2026

View reviewed changes

engdoreis added this pull request to the merge queue Feb 9, 2026

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 9, 2026

engdoreis added this pull request to the merge queue Feb 9, 2026

Merged via the queue into lowRISC:master with commit 75229e1 Feb 9, 2026
77 of 78 checks passed

Backport 28259 to master #29152

Backport 28259 to master #29152

Uh oh!

Conversation

engdoreis commented Jan 19, 2026

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marnovandermaas commented Jan 21, 2026

Uh oh!

engdoreis commented Jan 22, 2026

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

marnovandermaas Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

engdoreis Feb 2, 2026

Choose a reason for hiding this comment

Uh oh!

jwnrt commented Feb 3, 2026

Uh oh!

AlexJones0 left a comment • 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!

AlexJones0 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

AlexJones0 Feb 5, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

AlexJones0 Feb 5, 2026 • 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!

AlexJones0 Feb 5, 2026 • 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!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

AlexJones0 left a comment • 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!

Uh oh!

Uh oh!

Uh oh!

AlexJones0 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

AlexJones0 left a comment •

edited

Loading

AlexJones0 Feb 5, 2026 •

edited

Loading

AlexJones0 Feb 5, 2026 •

edited

Loading

AlexJones0 left a comment •

edited

Loading