Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[bazel] Update the binary/test rules for building ROMs #19710

Closed
wants to merge 4 commits into from

Conversation

cfrantz
Copy link
Contributor

@cfrantz cfrantz commented Sep 20, 2023

  1. Add a kind attribute to the binary and test rules to indicate
    whether the target is for flash, ram or rom.
  2. Plumb flash scrambling support into the exec_env and binary rules.
  3. Update the rom attribute in the exec environments and binary rules.
    Allow the label to be any provider.
  4. Implement flash and rom paths for fpga_cw310 and sim_{dv,verilator}.
    4a. When building, apply the scrambling transform to the ROM binary.
    4b. When testing, retrieve the supplied ROM and substitute it into the
    test flow. Currently, only verilator can run ROM tests.
  5. Build the test_rom with the new rules.
  6. Update the rom_epmp_test to the new rules.

This PR depends on #19650 and #19697. You need only review the last 3 commits.

@@ -113,6 +138,12 @@ def _test_dispatch(ctx, exec_env, provider):

# Get the pre-test_cmd args.
args = get_fallback(ctx, "attr.args", exec_env)
if ctx.attr.kind == "rom":
# FIXME: This is a bit of inside-baseball: We know the opentitantool
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be more complicated but maybe the param dict should have two test commands, one for flash-based tests and one for rom-based tests? Say, if args is an array, it is valid for all kind, but it can also be a dictionary of key/values where the keys are the kind.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have two alternate ideas to explore rather than making the test_env more complex:

  • Allow opentitantool to accept an empty parameter for --verilator-flash, meaning there is no flash payload. This is the current behavior with --verilator-{rom,otp} and used to be the behavior with flash. It changed when opentitantool was updated to accept multiple flash args to load multiple banks.
  • Provide an "empty" VMEM file to load into flash.

@cfrantz cfrantz force-pushed the bazel-rom branch 5 times, most recently from ba0a8ad to f090160 Compare October 4, 2023 22:09
1. Add a `kind` attribute to the binary and test rules to indicate
   whether the target is for flash, ram or rom.
2. Plumb flash scrambling support into the exec_env and binary rules.
3. Update the `rom` attribute in the exec environments and binary rules.
   Allow the label to be any provider.
4. Implement flash and rom paths for fpga_cw310 and sim_{dv,verilator}.
4a. When building, apply the scrambling transform to the ROM binary.
4b. When testing, retrieve the supplied ROM and substitute it into the
    test flow.  Currently, only verilator can run ROM tests.
5. Update `sim.mk` to collect the correct file resources for the
   test_rom.

Signed-off-by: Chris Frantz <[email protected]>
1. Re-arrange the exec_envs so that there are environments that do not
   depend on the ROM targets.
2. Convert the test_rom target to use `opentitan_binary`.
3. Update the `test_rom_test` to use the standard exec_envs.
4. Add tentative CW340 support (build only - testing not yet supported).

Signed-off-by: Chris Frantz <[email protected]>
Demonstrate how to convert a `test_in_rom` test to the new
opentitan_test rule.

Signed-off-by: Chris Frantz <[email protected]>
1. Define a basic CW305 exec_env.The build/release pipeline expects to
   find cw305 artifacts for the test_rom.
2. The englishbreakfast verilator model does not understand ROM scrambling,
   so we also create a non-scrambled VMEM file.

Signed-off-by: Chris Frantz <[email protected]>
@cfrantz cfrantz marked this pull request as ready for review October 5, 2023 18:02
@cfrantz cfrantz requested review from rswarbrick and a team as code owners October 5, 2023 18:02
@cfrantz cfrantz requested review from moidx and removed request for a team and rswarbrick October 5, 2023 18:02
@cfrantz
Copy link
Contributor Author

cfrantz commented Oct 6, 2023

The commits in this PR were merged as part of #19719.

@cfrantz cfrantz closed this Oct 6, 2023
@cfrantz cfrantz deleted the bazel-rom branch October 6, 2023 20:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants