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

[multitop] Add better support for linker scripts (preprocessing) #25908

Merged
merged 6 commits into from
Jan 20, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 39 additions & 0 deletions hw/top/defs.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@
load("//rules/opentitan:hw.bzl", "opentitan_top")
load("//hw/top_earlgrey/data/autogen:defs.bzl", "EARLGREY")
load("//hw/top_darjeeling/data/autogen:defs.bzl", "DARJEELING")
load("//hw/top_englishbreakfast/data/autogen:defs.bzl", "ENGLISHBREAKFAST")

ALL_TOPS = [
EARLGREY,
DARJEELING,
ENGLISHBREAKFAST,
]

ALL_TOP_NAMES = [
Expand Down Expand Up @@ -52,3 +54,40 @@ def opentitan_require_ip(ip):
)
"""
return opentitan_if_ip(ip, [], ["@platforms//:incompatible"])

def opentitan_select_top(values, default):
"""
Select a value based on the top. If no top matches, a default
value is returned. The values must be a dictionary where each key
is either a string, or an array of string.

Example:
alias(
name = "my_alias",
actual = opentitan_select_top({
"earlgrey": "//something:earlgrey",
["english_breakfast", "darjeeling"]: "//something:else",
}, "//something:error")
)
"""
branches = {}
for (tops, value) in values.items():
if type(tops) == "string":
tops = [tops]
for top in tops:
branches["//hw/top:is_{}".format(top)] = value
branches["//conditions:default"] = default
return select(branches)

def opentitan_require_top(top):
"""
Return a value that can be used with `target_compatible_with` to
express that this target only works on the requested top.

Example:
cc_library(
name = "my_library",
target_compatible_with = opentitan_require_top("darjeeling"),
)
"""
return opentitan_select_top({top: []}, ["@platforms//:incompatible"])
4 changes: 4 additions & 0 deletions hw/top_darjeeling/sw/autogen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# -o hw/top_darjeeling

load("//rules:linker.bzl", "ld_library")
load("//hw/top:defs.bzl", "opentitan_require_top")

package(default_visibility = ["//visibility:public"])

Expand All @@ -20,9 +21,12 @@ cc_library(
"top_darjeeling.h",
"top_darjeeling_memory.h",
],
target_compatible_with = opentitan_require_top("darjeeling"),
)

ld_library(
name = "top_darjeeling_memory",
defines = ["OPENTITAN_TOP_MEMORY_LD=top_darjeeling_memory.ld"],
includes = ["top_darjeeling_memory.ld"],
target_compatible_with = opentitan_require_top("darjeeling"),
)
4 changes: 4 additions & 0 deletions hw/top_earlgrey/sw/autogen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# -o hw/top_earlgrey

load("//rules:linker.bzl", "ld_library")
load("//hw/top:defs.bzl", "opentitan_require_top")

package(default_visibility = ["//visibility:public"])

Expand All @@ -20,9 +21,12 @@ cc_library(
"top_earlgrey.h",
"top_earlgrey_memory.h",
],
target_compatible_with = opentitan_require_top("earlgrey"),
)

ld_library(
name = "top_earlgrey_memory",
defines = ["OPENTITAN_TOP_MEMORY_LD=top_earlgrey_memory.ld"],
includes = ["top_earlgrey_memory.ld"],
target_compatible_with = opentitan_require_top("earlgrey"),
)
1 change: 1 addition & 0 deletions hw/top_englishbreakfast/ip/ast/BUILD
1 change: 1 addition & 0 deletions hw/top_englishbreakfast/ip/ast/defs.bzl
4 changes: 4 additions & 0 deletions hw/top_englishbreakfast/sw/autogen/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
# -o hw/top_englishbreakfast

load("//rules:linker.bzl", "ld_library")
load("//hw/top:defs.bzl", "opentitan_require_top")

package(default_visibility = ["//visibility:public"])

Expand All @@ -20,9 +21,12 @@ cc_library(
"top_englishbreakfast.h",
"top_englishbreakfast_memory.h",
],
target_compatible_with = opentitan_require_top("englishbreakfast"),
)

ld_library(
name = "top_englishbreakfast_memory",
defines = ["OPENTITAN_TOP_MEMORY_LD=top_englishbreakfast_memory.ld"],
includes = ["top_englishbreakfast_memory.ld"],
target_compatible_with = opentitan_require_top("englishbreakfast"),
)
106 changes: 94 additions & 12 deletions rules/linker.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,73 @@

"""Rules for declaring linker scripts and linker script fragments."""

load("@rules_cc//cc:find_cc_toolchain.bzl", "find_cc_toolchain")
load("@rules_cc//cc:action_names.bzl", "C_COMPILE_ACTION_NAME")

def _preprocess_linker_file(ctx):
cc_toolchain = find_cc_toolchain(ctx)
features = cc_common.configure_features(
ctx = ctx,
cc_toolchain = cc_toolchain,
requested_features = ctx.features,
unsupported_features = ctx.disabled_features,
)
compilation_context = cc_common.merge_compilation_contexts(
compilation_contexts = [dep[CcInfo].compilation_context for dep in ctx.attr.deps],
)

# FIXME could not get cc_common.compile to work because it returns no object.
cxx_compiler_path = cc_common.get_tool_for_action(
feature_configuration = features,
action_name = C_COMPILE_ACTION_NAME,
)
c_compile_variables = cc_common.create_compile_variables(
feature_configuration = features,
cc_toolchain = cc_toolchain,
user_compile_flags = ctx.fragments.cpp.copts + ctx.fragments.cpp.conlyopts,
include_directories = compilation_context.includes,
quote_include_directories = compilation_context.quote_includes,
system_include_directories = compilation_context.system_includes,
preprocessor_defines = depset(ctx.attr.defines, transitive = [compilation_context.defines]),
)
cmd_line = cc_common.get_memory_inefficient_command_line(
feature_configuration = features,
action_name = C_COMPILE_ACTION_NAME,
variables = c_compile_variables,
)
env = cc_common.get_environment_variables(
feature_configuration = features,
action_name = C_COMPILE_ACTION_NAME,
variables = c_compile_variables,
)
output_script = ctx.actions.declare_file(ctx.label.name + ".ld")
ctx.actions.run(
outputs = [output_script],
inputs = depset(
[ctx.file.script],
transitive = [compilation_context.headers, cc_toolchain.all_files],
),
executable = cxx_compiler_path,
arguments = [
"-E", # Preprocess only.
"-P", # Avoid line markers in output.
"-C", # Keep comments
"-xc", # Force C language.
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this mean that C++ comments (// foo ) are not permitted?

Considering that is already the case for assembly language files, its probably not an issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes only C comments are supported. I think this is more consistent because linker scripts only allow /* */ for comments when they are not preprocessed.

Copy link
Contributor

@a-will a-will Jan 17, 2025

Choose a reason for hiding this comment

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

They should pass through the preprocessor with -C (or maybe it needs -CC... I forget!), so you don't have to maintain that restriction, I think. Doesn't matter to me, though!

ctx.file.script.path,
"-o",
output_script.path,
] + cmd_line,
env = env,
)
return output_script

def _ld_library_impl(ctx):
files = []
user_link_flags = []
files = []

# Disable non-volatile scratch region and counters if building for english
# breakfast. This should appear before the linker script.
# FIXME Get rid of this.
if "-DOT_IS_ENGLISH_BREAKFAST_REDUCED_SUPPORT_FOR_INTERNAL_USE_ONLY_" in ctx.fragments.cpp.copts:
user_link_flags += [
"-Wl,--defsym=no_ottf_nv_scratch=1",
Expand All @@ -20,31 +81,40 @@ def _ld_library_impl(ctx):
"-Wl,-nmagic",
]

if ctx.files.includes:
files += ctx.files.includes
user_link_flags += [
"-Wl,-L,{}".format(include.dirname)
for include in ctx.files.includes
]
files += ctx.files.includes
user_link_flags += [
"-Wl,-L,{}".format(include.dirname)
for include in ctx.files.includes
]

if ctx.file.script:
files += ctx.files.script
output_script = _preprocess_linker_file(ctx)
files.append(output_script)

user_link_flags += [
"-Wl,-T,{}".format(ctx.file.script.path),
"-Wl,-T,{}".format(output_script.path),
]

return [
DefaultInfo(
files = depset(files),
),
cc_common.merge_cc_infos(
direct_cc_infos = [CcInfo(
# Order is important! We list dependencies first so that any
# linker flags set by dependencies (such as -Wl,-L) appear before
# the files that depend on them.
cc_infos = [dep[CcInfo] for dep in ctx.attr.deps] + [CcInfo(
linking_context = cc_common.create_linking_context(
linker_inputs = depset([cc_common.create_linker_input(
owner = ctx.label,
additional_inputs = depset(files),
user_link_flags = depset(user_link_flags),
)]),
),
compilation_context = cc_common.create_compilation_context(
defines = depset(ctx.attr.defines),
),
)],
cc_infos = [dep[CcInfo] for dep in ctx.attr.deps],
),
]

Expand All @@ -66,10 +136,21 @@ ld_library = rule(
for more details.
""",
attrs = {
"script": attr.label(allow_single_file = True),
"script": attr.label(
allow_single_file = True,
doc = "Main linker script. This file will be preprocessed.",
),
"defines": attr.string_list(
doc = "C preprocessor defines. These defines are subject to Make variable substitution.",
),
"includes": attr.label_list(
default = [],
allow_files = True,
doc = """
Link script libraries. Those files will automatically be added
to the linker search paths and will also be available in the
preprocessing context of the main link script.
""",
),
"deps": attr.label_list(
default = [],
Expand All @@ -79,4 +160,5 @@ ld_library = rule(
default = False,
),
},
toolchains = ["@rules_cc//cc:toolchain_type"],
)
12 changes: 6 additions & 6 deletions sw/device/lib/testing/test_framework/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ ld_library(
script = "ottf_silicon_creator_a.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand All @@ -104,7 +104,7 @@ ld_library(
script = "ottf_silicon_creator_b.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand All @@ -113,7 +113,7 @@ ld_library(
script = "ottf_silicon_creator_virtual.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand All @@ -122,7 +122,7 @@ ld_library(
script = "ottf_silicon_owner_a.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand All @@ -131,7 +131,7 @@ ld_library(
script = "ottf_silicon_owner_b.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand All @@ -140,7 +140,7 @@ ld_library(
script = "ottf_silicon_owner_virtual.ld",
deps = [
":ottf_ld_common",
"//hw/top_earlgrey/sw/autogen:top_earlgrey_memory",
"//hw/top:top_ld",
],
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* This linker script generates a binary to run rom.
*/

INCLUDE hw/top_earlgrey/sw/autogen/top_earlgrey_memory.ld
/* This linker script is preprocessed, OPENTITAN_TOP_MEMORY_LD is a define
* set by Bazel to point to the top's linker file. */
INCLUDE OPENTITAN_TOP_MEMORY_LD
engdoreis marked this conversation as resolved.
Show resolved Hide resolved

/**
* Symbols to be used in the setup of the address translation for the OTTF run
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* This linker script generates a binary to run rom.
*/

INCLUDE hw/top_earlgrey/sw/autogen/top_earlgrey_memory.ld
/* This linker script is preprocessed, OPENTITAN_TOP_MEMORY_LD is a define
* set by Bazel to point to the top's linker file. */
INCLUDE OPENTITAN_TOP_MEMORY_LD

/**
* Symbols to be used in the setup of the address translation for the OTTF run
Expand Down
4 changes: 3 additions & 1 deletion sw/device/lib/testing/test_framework/ottf_silicon_owner_a.ld
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* This linker script generates a binary to run BL0 tests.
*/

INCLUDE hw/top_earlgrey/sw/autogen/top_earlgrey_memory.ld
/* This linker script is preprocessed, OPENTITAN_TOP_MEMORY_LD is a define
* set by Bazel to point to the top's linker file. */
INCLUDE OPENTITAN_TOP_MEMORY_LD

/**
* Symbols to be used in the setup of the address translation for the OTTF run
Expand Down
6 changes: 4 additions & 2 deletions sw/device/lib/testing/test_framework/ottf_silicon_owner_b.ld
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* This linker script generates a binary to run BL0 tests.
*/

INCLUDE hw/top_earlgrey/sw/autogen/top_earlgrey_memory.ld
/* This linker script is preprocessed, OPENTITAN_TOP_MEMORY_LD is a define
* set by Bazel to point to the top's linker file. */
INCLUDE OPENTITAN_TOP_MEMORY_LD

/**
* Symbols to be used in the setup of the address translation for the OTTF run
Expand All @@ -22,4 +24,4 @@ _ottf_start_address = ORIGIN(eflash) + (LENGTH(eflash) / 2) + 0x10000;

REGION_ALIAS("ottf_flash", eflash);

INCLUDE sw/device/lib/testing/test_framework/ottf_common.ld
INCLUDE ottf_common.ld
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
* This linker script generates a binary to run BL0 tests.
*/

INCLUDE hw/top_earlgrey/sw/autogen/top_earlgrey_memory.ld
/* This linker script is preprocessed, OPENTITAN_TOP_MEMORY_LD is a define
* set by Bazel to point to the top's linker file. */
INCLUDE OPENTITAN_TOP_MEMORY_LD

/**
* Symbols to be used in the setup of the address translation for ROM_EXT.
Expand Down
Loading
Loading