[bazel] Pass copt and features through the OT transition

1. Pass the `copt` and `features` values through the OT platform transition. 2. Convert the aes_smoketest to the `opentitan_test` rule. 3. Refine the verilator binary query for the englishbreakfast model. Signed-off-by: Chris Frantz <[email protected]> Signed-off-by: Chris Frantz <[email protected]>
lowRISC · Jan 12, 2024 · 66ee21a · 66ee21a
1 parent cf38c1d
commit 66ee21a
Show file tree

Hide file tree

Showing 8 changed files with 37 additions and 19 deletions.
diff --git a/azure-pipelines.yml b/azure-pipelines.yml
@@ -339,12 +339,17 @@ jobs:
     displayName: Build & execute tests
   - template: ci/publish-bazel-test-results.yml
   # TODO: build and cache the verilator model to avoid building twice (#12574)
+  # NOTE: The new build/test rules reference verilator as a dependency, but under the
+  #       platforms transition of that rule.  Therefore, verilator is built under a
+  #       configuration like 'k8-opt-exec-$host-FOR-$target'.  In order to get the
+  #       verilator binary, we query the output of one of the verilated tests and ask
+  #       for the verilator binary, which is in a subdir named 'build.verilator_<stuff>'.
   - bash: |
       . util/build_consts.sh
       mkdir -p "$BIN_DIR/hw/top_earlgrey/"
-      cp $(ci/scripts/target-location.sh //hw:verilator) \
+      cp $(./bazelisk.sh outquery-build.verilator //sw/device/tests:uart_smoketest_sim_verilator) \
         "$BIN_DIR/hw/top_earlgrey/Vchip_earlgrey_verilator"
-    displayName: Copy //hw:verilator to $BIN_DIR
+    displayName: Copy verilated binary to $BIN_DIR
   - template: ci/upload-artifacts-template.yml
     parameters:
       includePatterns:

diff --git a/bazelisk.sh b/bazelisk.sh
@@ -79,16 +79,16 @@ function outquery_starlark_expr() {
             echo "target.files.to_list()[0].path"
             ;;
         -all)
-            echo "\"\\n\".join([f.path for f in target.files.to_list()])"
+            echo "\"\\n\".join([f.path for f in depset(transitive=[target.files, target.default_runfiles.files]).to_list()])"
             ;;
         -providers)
             echo "providers(target)"
             ;;
         -*)
-            echo "\"\\n\".join([f.path for f in target.files.to_list() if \"$q\"[1:] in f.path])"
+            echo "\"\\n\".join([f.path for f in depset(transitive=[target.files, target.default_runfiles.files]).to_list() if \"$q\"[1:] in f.path])"
             ;;
         .*)
-            echo "\"\\n\".join([f.path for f in target.files.to_list() if f.path.endswith(\"$q\")])"
+            echo "\"\\n\".join([f.path for f in depset(transitive=[target.files, target.default_runfiles.files]).to_list() if f.path.endswith(\"$q\")])"
             ;;
     esac
 }

diff --git a/ci/scripts/run-english-breakfast-verilator-tests.sh b/ci/scripts/run-english-breakfast-verilator-tests.sh
@@ -25,13 +25,16 @@ ci/bazelisk.sh build  \
 
 # Run the one test.
 # This needs to be run outside the bazel sandbox, so we do not use `bazel run`
+#
+# NOTE: we specify `-type f` in the find commands to avoid finding any
+# additional symlinks bazel might have prepared when building the test targets.
 bazel-bin/sw/host/opentitantool/opentitantool \
     --rcfile="" \
     --logging=info \
     --interface=verilator \
     --verilator-bin=$BIN_DIR/hw/top_englishbreakfast/Vchip_englishbreakfast_verilator \
-    --verilator-rom="$(find bazel-out/* -name 'test_rom_sim_verilator.32.vmem')" \
-    --verilator-flash="$(find bazel-out/* -name 'aes_smoketest_prog_sim_verilator.64.scr.vmem')" \
+    --verilator-rom="$(find bazel-out/* -type f -name 'test_rom_sim_verilator.32.vmem')" \
+    --verilator-flash="$(find bazel-out/* -type f -name 'aes_smoketest_sim_verilator.64.vmem')" \
     console \
     --exit-failure="(FAIL|FAULT).*\n" \
     --exit-success="PASS.*\n" \

diff --git a/hw/top_earlgrey/dv/chip_sim_cfg.hjson b/hw/top_earlgrey/dv/chip_sim_cfg.hjson
@@ -1220,14 +1220,14 @@
     {
       name: chip_sw_aes_enc
       uvm_test_seq: chip_sw_base_vseq
-      sw_images: ["//sw/device/tests:aes_smoketest:1"]
+      sw_images: ["//sw/device/tests:aes_smoketest:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=22_000_000"]
     }
     {
       name: chip_sw_aes_enc_jitter_en
       uvm_test_seq: chip_sw_base_vseq
-      sw_images: ["//sw/device/tests:aes_smoketest:1"]
+      sw_images: ["//sw/device/tests:aes_smoketest:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=26_000_000", "+en_jitter=1"]
     }
@@ -1912,7 +1912,7 @@
     {
       name: chip_sw_rv_core_ibex_lockstep_glitch
       uvm_test_seq: chip_sw_rv_core_ibex_lockstep_glitch_vseq
-      sw_images: ["sw/device/tests:aes_smoketest:1"]
+      sw_images: ["sw/device/tests:aes_smoketest:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       // This test currently stops without completing all transactions, so we
       // have to disable the final assertions.
@@ -1980,7 +1980,7 @@
     {
       name: chip_sw_aes_enc_jitter_en_reduced_freq
       uvm_test_seq: chip_sw_base_vseq
-      sw_images: ["//sw/device/tests:aes_smoketest:1"]
+      sw_images: ["//sw/device/tests:aes_smoketest:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
       run_opts: ["+sw_test_timeout_ns=26_000_000", "+en_jitter=1", "+cal_sys_clk_70mhz=1"]
     }

diff --git a/hw/top_earlgrey/dv/chip_smoketests.hjson b/hw/top_earlgrey/dv/chip_smoketests.hjson
@@ -10,7 +10,7 @@
     {
       name: chip_sw_aes_smoketest
       uvm_test_seq: chip_sw_base_vseq
-      sw_images: ["//sw/device/tests:aes_smoketest:1"]
+      sw_images: ["//sw/device/tests:aes_smoketest:1:new_rules"]
       en_run_modes: ["sw_test_mode_test_rom"]
     }
     {

diff --git a/hw/top_englishbreakfast/util/prepare_sw.py b/hw/top_englishbreakfast/util/prepare_sw.py
@@ -30,7 +30,7 @@
     '//sw/device/lib/testing/test_rom',
     '//sw/device/sca:aes_serial',
     '//sw/device/examples/hello_world',
-    '//sw/device/tests:aes_smoketest_prog',
+    '//sw/device/tests:aes_smoketest_sim_verilator',
 ]
 
 
@@ -182,6 +182,7 @@ def main():
         'build',
         '--features=-rv32_bitmanip',
         '--copt=-DOT_IS_ENGLISH_BREAKFAST_REDUCED_SUPPORT_FOR_INTERNAL_USE_ONLY_',
+        '--define=DISABLE_VERILATOR_BUILD=true',
     ] + BAZEL_BINARIES)
 
 

diff --git a/rules/rv.bzl b/rules/rv.bzl
@@ -21,16 +21,28 @@ PER_DEVICE_DEPS = {
 def _opentitan_transition_impl(settings, attr):
     return {
         "//command_line_option:platforms": attr.platform,
+        "//command_line_option:copt": settings["//command_line_option:copt"],
+        "//command_line_option:features": settings["//command_line_option:features"],
         "//hw/bitstream/universal:rom": "//hw/bitstream/universal:none",
         "//hw/bitstream/universal:otp": "//hw/bitstream/universal:none",
         "//hw/bitstream/universal:env": "//hw/bitstream/universal:none",
     }
 
 opentitan_transition = transition(
     implementation = _opentitan_transition_impl,
-    inputs = [],
+    # In order to build the englishbreakfast binaries, we need to pass through
+    # the `--copt` and `--features` flags:
+    # - The copt flag defines a preprocessor symbol indicating englishbreakfast.
+    # - The features flags turn off compiler support for CPU extensions not
+    #   present in the englishbreakfast rv32i implementation.
+    inputs = [
+        "//command_line_option:copt",
+        "//command_line_option:features",
+    ],
     outputs = [
         "//command_line_option:platforms",
+        "//command_line_option:copt",
+        "//command_line_option:features",
         "//hw/bitstream/universal:rom",
         "//hw/bitstream/universal:otp",
         "//hw/bitstream/universal:env",

diff --git a/sw/device/tests/BUILD b/sw/device/tests/BUILD
@@ -140,13 +140,10 @@ alias(
     visibility = ["//visibility:private"],
 )
 
-# TODO(#19926): The `opentitan_test` rule doesn't propagate --copt accross all
-# dependencies, causing issues when compiling targets against
-# --copt=-DOT_IS_ENGLISH_BREAKFAST_REDUCED_SUPPORT_FOR_INTERNAL_USE_ONLY_.
-# Using `opentitan_functest` as a workaround.
-opentitan_functest(
+opentitan_test(
     name = "aes_smoketest",
     srcs = ["aes_smoketest.c"],
+    exec_env = EARLGREY_TEST_ENVS,
     deps = [
         "//hw/ip/aes:model",
         "//hw/top_earlgrey/sw/autogen:top_earlgrey",