[imm_rom_ext] Match exec_env of ROM_EXT and IMM_ROM_EXT

* Add mechanism to select corresponding IMM_ROM_EXT sections when building ROM_EXT based on the exec_env * Add a new "none manifest" and skip the signing phase when building binaries if we use the none manifest * Add extra_bazel_features for IMM_ROM_EXT (as what we did for ROM_EXT targets) for code size optimization Signed-off-by: Chia-Wei Liu <[email protected]>
lowRISC · Jan 22, 2025 · 5547e44 · 5547e44
1 parent 0f5589d
commit 5547e44
Show file tree

Hide file tree

Showing 7 changed files with 102 additions and 42 deletions.
diff --git a/hw/top_earlgrey/BUILD b/hw/top_earlgrey/BUILD
@@ -2,6 +2,10 @@
 # Licensed under the Apache License, Version 2.0, see LICENSE for details.
 # SPDX-License-Identifier: Apache-2.0
 
+load(
+    "//rules:manifest.bzl",
+    "manifest",
+)
 load(
     "//rules/opentitan:defs.bzl",
     "CLEAR_KEY_SET",
@@ -27,6 +31,14 @@ filegroup(
     ],
 )
 
+# The following definition is used to define a null manifest in the signing
+# configuration for execution environments (exec_env) and opentitan_test
+# and opentitan_binary rules. When building the binaries, if the manifest equals
+# to this null manifest, then the signing will be skipped.
+CLEAR_MANIFEST = manifest(d = {
+    "name": "none_manifest",
+})
+
 ###########################################################################
 # FPGA CW310 Environments
 ###########################################################################

diff --git a/rules/opentitan/cc.bzl b/rules/opentitan/cc.bzl
@@ -182,6 +182,9 @@ def _build_binary(ctx, exec_env, name, deps, kind):
     )
 
     manifest = get_fallback(ctx, "file.manifest", exec_env)
+    if manifest and str(manifest.owner) == "@@//hw/top_earlgrey:none_manifest":
+        manifest = None
+
     ecdsa_key = get_fallback(ctx, "attr.ecdsa_key", exec_env)
     rsa_key = get_fallback(ctx, "attr.rsa_key", exec_env)
     spx_key = get_fallback(ctx, "attr.spx_key", exec_env)
@@ -226,10 +229,20 @@ def _opentitan_binary(ctx):
     providers = []
     default_info = []
     groups = {}
-    for exec_env in ctx.attr.exec_env:
-        exec_env = exec_env[ExecEnvInfo]
+    for exec_env_target in ctx.attr.exec_env:
+        exec_env = exec_env_target[ExecEnvInfo]
         name = _binary_name(ctx, exec_env)
         deps = ctx.attr.deps + exec_env.libs
+
+        imm_rom_ext_deps = []
+        for dep in ctx.attr.immutable_rom_ext_sections:
+            if exec_env_target.label.name not in dep.label.name:
+                continue
+            imm_rom_ext_deps.append(dep)
+        if ctx.attr.immutable_rom_ext_sections and len(imm_rom_ext_deps) != 1:
+            fail("When building for exec_env {}, found zero or more than one immutable ROM_EXT sections to link: {}".format(imm_rom_ext_deps, exec_env_target))
+        deps += imm_rom_ext_deps
+
         kind = ctx.attr.kind
         provides, signed = _build_binary(ctx, exec_env, name, deps, kind)
         providers.append(exec_env.provider(kind = kind, **provides))
@@ -329,6 +342,10 @@ common_binary_attrs = {
         doc = "Indicates whether the binary is intended for a chip with the immutable ROM_EXT feature enabled.",
         default = False,
     ),
+    "immutable_rom_ext_sections": attr.label_list(
+        providers = [CcInfo],
+        doc = "The list of immutable ROM_EXT sections to be linked in to the binary target.  Only the deps matched with the specific exec_env will be kept.",
+    ),
 }
 
 opentitan_binary = rv_rule(

diff --git a/sw/device/silicon_creator/imm_rom_ext/BUILD b/sw/device/silicon_creator/imm_rom_ext/BUILD
@@ -2,10 +2,15 @@
 # Licensed under the Apache License, Version 2.0, see LICENSE for details.
 # SPDX-License-Identifier: Apache-2.0
 
+load("@lowrisc_opentitan//rules/opentitan:exec_env.bzl", "ExecEnvInfo")
 load("@lowrisc_opentitan//rules/opentitan:transform.bzl", "obj_transform")
 load("//rules/opentitan:defs.bzl", "OPENTITAN_CPU", "opentitan_binary")
 load("//rules:linker.bzl", "ld_library")
-load("//sw/device/silicon_creator/imm_rom_ext:utils.bzl", "imm_rom_ext_section")
+load("//sw/device/silicon_creator/imm_rom_ext:defs.bzl", "DEFAULT_EXEC_ENV")
+load(
+    "//sw/device/silicon_creator/imm_rom_ext:utils.bzl",
+    "create_imm_rom_ext_targets",
+)
 
 package(default_visibility = ["//visibility:public"])
 
@@ -57,24 +62,27 @@ cc_library(
 
 opentitan_binary(
     name = "main_binaries",
-    # TODO(#24368): Support multiple executing environments. Currently all
-    # environments derive the same binary so only one environment is kept here,
-    # but we need to support multiple executing environments and make sure
-    # ROM_EXT targets choose the matched environment when linking IMM_ROM_EXT.
-    exec_env = [
-        "//hw/top_earlgrey:fpga_cw340",
+    exec_env = DEFAULT_EXEC_ENV,
+    extra_bazel_features = [
+        "minsize",
+        "use_lld",
     ],
     linker_script = ":ld_hello_world",
+    manifest = "//hw/top_earlgrey:none_manifest",
     deps = [
         ":main_lib",
         "//sw/device/lib/crt",
     ],
 )
 
-imm_rom_ext_section(
-    name = "main_section",
-    srcs = [":main_binaries"],
-)
+[
+    create_imm_rom_ext_targets(
+        src = ":main_binaries",
+        base_name = "main_section",
+        exec_env = env,
+    )
+    for env in DEFAULT_EXEC_ENV
+]
 
 ld_library(
     name = "ld_hello_world",
@@ -102,10 +110,6 @@ cc_library(
 
 opentitan_binary(
     name = "hello_world_binaries",
-    # TODO(#24368): Support multiple executing environments. Currently all
-    # environments derive the same binary so only one environment is kept here,
-    # but we need to support multiple executing environments and make sure
-    # ROM_EXT targets choose the matched environment when linking IMM_ROM_EXT.
     exec_env = [
         "//hw/top_earlgrey:fpga_cw340",
     ],
@@ -115,8 +119,3 @@ opentitan_binary(
         "//sw/device/lib/crt",
     ],
 )
-
-imm_rom_ext_section(
-    name = "hello_world_section",
-    srcs = [":hello_world_binaries"],
-)
diff --git a/sw/device/silicon_creator/imm_rom_ext/defs.bzl b/sw/device/silicon_creator/imm_rom_ext/defs.bzl
@@ -2,7 +2,21 @@
 # Licensed under the Apache License, Version 2.0, see LICENSE for details.
 # SPDX-License-Identifier: Apache-2.0
 
+DEFAULT_EXEC_ENV = [
+    "//hw/top_earlgrey:fpga_cw310",
+    "//hw/top_earlgrey:fpga_cw340",
+    "//hw/top_earlgrey:sim_dv_base",
+    "//hw/top_earlgrey:sim_verilator_base",
+    "//hw/top_earlgrey:silicon_creator",
+]
+
 # The target list should contian prebuilt artifacts and run-time build targets.
-IMM_ROM_EXT_TARGETS = {
-    "main": "//sw/device/silicon_creator/imm_rom_ext:main_section",
+IMM_ROM_EXT_SECTIONS = {
+    "main": [
+        "//sw/device/silicon_creator/imm_rom_ext:main_section_fpga_cw310",
+        "//sw/device/silicon_creator/imm_rom_ext:main_section_fpga_cw340",
+        "//sw/device/silicon_creator/imm_rom_ext:main_section_sim_dv_base",
+        "//sw/device/silicon_creator/imm_rom_ext:main_section_sim_verilator_base",
+        "//sw/device/silicon_creator/imm_rom_ext:main_section_silicon_creator",
+    ],
 }
diff --git a/sw/device/silicon_creator/imm_rom_ext/utils.bzl b/sw/device/silicon_creator/imm_rom_ext/utils.bzl
@@ -5,6 +5,7 @@
 load("@rules_cc//cc:action_names.bzl", "OBJ_COPY_ACTION_NAME")
 load("@rules_cc//cc:find_cc_toolchain.bzl", "find_cc_toolchain")
 load("@lowrisc_opentitan//rules:rv.bzl", "rv_rule")
+load("@lowrisc_opentitan//rules/opentitan:exec_env.bzl", "ExecEnvInfo")
 
 def _bin_to_imm_rom_ext_object_impl(ctx):
     cc_toolchain = find_cc_toolchain(ctx)
@@ -20,9 +21,8 @@ def _bin_to_imm_rom_ext_object_impl(ctx):
     )
 
     outputs = []
-    for src in ctx.files.srcs:
-        if src.extension != "bin":
-            continue
+    exec_env_name = ctx.attr.exec_env[ExecEnvInfo].exec_env
+    for src in ctx.attr.src.output_groups[exec_env_name + "_binary"].to_list():
         object = ctx.actions.declare_file(
             "{}.{}".format(
                 src.basename.replace("." + src.extension, ""),
@@ -45,23 +45,40 @@ def _bin_to_imm_rom_ext_object_impl(ctx):
             executable = objcopy,
         )
         outputs.append(object)
-    return [DefaultInfo(files = depset(outputs), runfiles = ctx.runfiles(files = outputs))]
+    if len(outputs) != 1:
+        fail("Generated zero or more than one binary: {}".format(outputs))
+    return [
+        DefaultInfo(
+            files = depset(outputs),
+            runfiles = ctx.runfiles(files = outputs),
+        ),
+    ]
 
 bin_to_imm_rom_ext_object = rv_rule(
     implementation = _bin_to_imm_rom_ext_object_impl,
     attrs = {
-        "srcs": attr.label_list(allow_files = True),
+        "src": attr.label(allow_files = True),
+        "exec_env": attr.label(
+            providers = [ExecEnvInfo],
+            doc = "The execution environment for this target.",
+        ),
         "_cc_toolchain": attr.label(default = Label("@bazel_tools//tools/cpp:current_cc_toolchain")),
     },
     fragments = ["cpp"],
     toolchains = ["@rules_cc//cc:toolchain_type"],
 )
 
-def imm_rom_ext_section(name, srcs):
-    object_target_name = name + "_object"
-    bin_to_imm_rom_ext_object(name = object_target_name, srcs = srcs)
+def create_imm_rom_ext_targets(src, exec_env, base_name):
+    exec_env_name = Label(exec_env).name
+    object_target_name = "{}_{}_object".format(base_name, exec_env_name)
+    cc_import_name = "{}_{}".format(base_name, exec_env_name)
+    bin_to_imm_rom_ext_object(
+        name = object_target_name,
+        src = src,
+        exec_env = exec_env,
+    )
     native.cc_import(
-        name = name,
+        name = cc_import_name,
         objects = [object_target_name],
         data = [object_target_name],
         alwayslink = 1,

diff --git a/sw/device/silicon_creator/rom_ext/BUILD b/sw/device/silicon_creator/rom_ext/BUILD
@@ -14,7 +14,7 @@ load(
 )
 load(
     "//sw/device/silicon_creator/imm_rom_ext:defs.bzl",
-    "IMM_ROM_EXT_TARGETS",
+    "IMM_ROM_EXT_SECTIONS",
 )
 
 package(default_visibility = ["//visibility:public"])
@@ -233,7 +233,6 @@ cc_library(
         "//sw/device/lib/base:memory",
         "//sw/device/lib/base:stdasm",
         "//sw/device/lib/runtime:hart",
-        "//sw/device/silicon_creator/imm_rom_ext:main_section",
         "//sw/device/silicon_creator/lib:boot_data",
         "//sw/device/silicon_creator/lib:boot_log",
         "//sw/device/silicon_creator/lib:dbg_print",
@@ -291,6 +290,7 @@ opentitan_binary(
         "minsize",
         "use_lld",
     ],
+    immutable_rom_ext_sections = IMM_ROM_EXT_SECTIONS["main"],
     linker_script = ":ld_slot_a",
     manifest = ":manifest",
     spx_key = {"//sw/device/silicon_creator/rom/keys/fake/spx:prod_key_0_spx": "prod_key_0"},
@@ -319,6 +319,7 @@ opentitan_binary(
         "minsize",
         "use_lld",
     ],
+    immutable_rom_ext_sections = IMM_ROM_EXT_SECTIONS["main"],
     linker_script = ":ld_slot_b",
     manifest = ":manifest",
     spx_key = {"//sw/device/silicon_creator/rom/keys/fake/spx:prod_key_0_spx": "prod_key_0"},
@@ -347,6 +348,7 @@ opentitan_binary(
         "minsize",
         "use_lld",
     ],
+    immutable_rom_ext_sections = IMM_ROM_EXT_SECTIONS["main"],
     linker_script = ":ld_slot_virtual",
     manifest = ":manifest",
     deps = [
@@ -374,6 +376,7 @@ opentitan_binary(
             "minsize",
             "use_lld",
         ],
+        immutable_rom_ext_sections = imm_rom_ext_sections,
         linker_script = ":ld_slot_virtual",
         manifest = ":manifest",
         deps = [
@@ -382,11 +385,9 @@ opentitan_binary(
             "//sw/device/silicon_creator/lib:manifest_def",
             "//sw/device/silicon_creator/lib/ownership:test_owner",
             "//sw/device/silicon_creator/lib/ownership/keys/fake",
-        ] + [
-            imm_rom_ext_target,
         ],
     )
-    for name, imm_rom_ext_target in IMM_ROM_EXT_TARGETS.items()
+    for name, imm_rom_ext_sections in IMM_ROM_EXT_SECTIONS.items()
 ]
 
 manifest(d = {

diff --git a/sw/device/silicon_creator/rom_ext/e2e/verified_boot/BUILD b/sw/device/silicon_creator/rom_ext/e2e/verified_boot/BUILD
@@ -23,7 +23,7 @@ load(
 )
 load(
     "//sw/device/silicon_creator/imm_rom_ext:defs.bzl",
-    "IMM_ROM_EXT_TARGETS",
+    "IMM_ROM_EXT_SECTIONS",
 )
 
 package(default_visibility = ["//visibility:public"])
@@ -48,7 +48,7 @@ filegroup(
         rom_ext = "//sw/device/silicon_creator/rom_ext:rom_ext_with_{}_imm_slot_virtual".format(name),
         visibility = ["//visibility:private"],
     )
-    for name in IMM_ROM_EXT_TARGETS
+    for name in IMM_ROM_EXT_SECTIONS
 ]
 
 [
@@ -62,7 +62,7 @@ filegroup(
         ],
         visibility = ["//visibility:private"],
     )
-    for name in IMM_ROM_EXT_TARGETS
+    for name in IMM_ROM_EXT_SECTIONS
 ]
 
 _POSITIONS = {
@@ -139,7 +139,7 @@ _POSITIONS = {
         "success": "rom_ext_slot = AA__\r\n",
         "otp_img": ":otp_img_with_{}_imm_romext_enabled".format(name),
     }
-    for name in IMM_ROM_EXT_TARGETS
+    for name in IMM_ROM_EXT_SECTIONS
 }
 
 [