Skip to content

[multitop] Make the codebase more ready to support multiple tops #31072

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.

Sign up for GitHub

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

Open
mfnch wants to merge 7 commits into
base: master
Choose a base branch
from
Open

[multitop] Make the codebase more ready to support multiple tops #31072

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
548a1da
[ottf] Remove unnecessary dependencies in ujson_ottf
mfnch Aug 17, 2026
c7ad328
[bazel,test] Remove dependency on simple_serial
mfnch Aug 20, 2026
4b08177
[ottf] Tolerate lack of a console backend
mfnch Aug 17, 2026
9416636
[multitop] Make dif_clkmgr multitop friendly
mfnch Aug 13, 2026
289c380
[multitop] Improve multitop support in dif_rstmgr
mfnch Aug 13, 2026
f05014d
[multitop] Make multi-register names more uniform
mfnch Aug 20, 2026
8166db6
[multitop] Make the test_rom multitop friendly
mfnch Aug 13, 2026
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
21 changes: 21 additions & 0 deletions sw/device/lib/dif/dif_clkmgr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include <assert.h>

#include "sw/device/lib/base/bitfield.h"
#include "sw/device/lib/base/macros.h"
#include "sw/device/lib/base/mmio.h"
#include "sw/device/lib/base/multibits.h"
#include "sw/device/lib/dif/dif_base.h"
Expand Down Expand Up @@ -173,11 +174,15 @@ dif_result_t dif_clkmgr_gateable_clock_get_enabled(
return kDifBadArg;
}

#if CLKMGR_PARAM_NUM_SW_GATEABLE_CLOCKS == 0
OT_UNREACHABLE();
#else
uint32_t clk_enables_val =
mmio_region_read32(clkmgr->base_addr, CLKMGR_CLK_ENABLES_REG_OFFSET);
*state = dif_bool_to_toggle(bitfield_bit32_read(clk_enables_val, clock));

return kDifOk;
#endif
}

dif_result_t dif_clkmgr_gateable_clock_set_enabled(
Expand All @@ -188,6 +193,9 @@ dif_result_t dif_clkmgr_gateable_clock_set_enabled(
return kDifBadArg;
}

#if CLKMGR_PARAM_NUM_SW_GATEABLE_CLOCKS == 0
OT_UNREACHABLE();
#else
bool new_clk_enables_bit = dif_toggle_to_bool(new_state);
uint32_t clk_enables_val =
mmio_region_read32(clkmgr->base_addr, CLKMGR_CLK_ENABLES_REG_OFFSET);
Expand All @@ -197,6 +205,7 @@ dif_result_t dif_clkmgr_gateable_clock_set_enabled(
clk_enables_val);

return kDifOk;
#endif
}

OT_WARN_UNUSED_RESULT
Expand Down Expand Up @@ -225,11 +234,15 @@ dif_result_t dif_clkmgr_hintable_clock_get_enabled(
return kDifBadArg;
}

#if CLKMGR_PARAM_NUM_HINTABLE_CLOCKS == 0
OT_UNREACHABLE();
#else
uint32_t clk_hints_val =
mmio_region_read32(clkmgr->base_addr, CLKMGR_CLK_HINTS_STATUS_REG_OFFSET);
*state = dif_bool_to_toggle(bitfield_bit32_read(clk_hints_val, clock));

return kDifOk;
#endif
}

dif_result_t dif_clkmgr_hintable_clock_set_hint(
Expand All @@ -240,6 +253,9 @@ dif_result_t dif_clkmgr_hintable_clock_set_hint(
return kDifBadArg;
}

#if CLKMGR_PARAM_NUM_HINTABLE_CLOCKS == 0
OT_UNREACHABLE();
#else
bool new_clk_hints_bit = dif_toggle_to_bool(new_state);
uint32_t clk_hints_val =
mmio_region_read32(clkmgr->base_addr, CLKMGR_CLK_HINTS_REG_OFFSET);
Expand All @@ -248,6 +264,7 @@ dif_result_t dif_clkmgr_hintable_clock_set_hint(
clk_hints_val);

return kDifOk;
#endif
}

dif_result_t dif_clkmgr_hintable_clock_get_hint(
Expand All @@ -257,11 +274,15 @@ dif_result_t dif_clkmgr_hintable_clock_get_hint(
return kDifBadArg;
}

#if CLKMGR_PARAM_NUM_HINTABLE_CLOCKS == 0
OT_UNREACHABLE();
#else
uint32_t clk_hints_val =
mmio_region_read32(clkmgr->base_addr, CLKMGR_CLK_HINTS_REG_OFFSET);
*state = dif_bool_to_toggle(bitfield_bit32_read(clk_hints_val, clock));

return kDifOk;
#endif
}

#if OPENTITAN_CLKMGR_HAS_SW_EXTCLK_REGWEN
Expand Down
10 changes: 0 additions & 10 deletions sw/device/lib/dif/dif_pinmux.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,21 +48,11 @@ static bool dif_pinmux_get_sleep_status_bit(dif_pinmux_pad_kind_t kind,
switch (kind) {
case kDifPinmuxPadKindMio:
num_pads = PINMUX_PARAM_N_MIO_PADS;
// Only platforms with few MIOs, there is a single register with no index.
#ifdef PINMUX_MIO_PAD_SLEEP_STATUS_0_REG_OFFSET
reg_base = PINMUX_MIO_PAD_SLEEP_STATUS_0_REG_OFFSET;
#else
reg_base = PINMUX_MIO_PAD_SLEEP_STATUS_REG_OFFSET;
#endif
break;
case kDifPinmuxPadKindDio:
num_pads = PINMUX_PARAM_N_DIO_PADS;
// Only platforms with few DIOs, there is a single register with no index.
#ifdef PINMUX_DIO_PAD_SLEEP_STATUS_0_REG_OFFSET
reg_base = PINMUX_DIO_PAD_SLEEP_STATUS_0_REG_OFFSET;
#else
reg_base = PINMUX_DIO_PAD_SLEEP_STATUS_REG_OFFSET;
#endif
break;
default:
return false;
Expand Down
50 changes: 24 additions & 26 deletions sw/device/lib/dif/dif_rstmgr.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,25 @@

#include "hw/top/rstmgr_regs.h" // Generated.

// These assertions are only defined for the Earl Grey chip.
#if defined(OPENTITAN_IS_EARLGREY)
// This macro simplifies the `static_assert` check to make sure that the
// public reset info register bitfield matches register bits.
#define RSTMGR_RESET_INFO_CHECK(pub_name, priv_name) \
static_assert(kDifRstmgrResetInfo##pub_name == \
(0x1 << RSTMGR_RESET_##priv_name##_BIT), \
"kDifRstmgrResetInfo" #pub_name \
#define RSTMGR_RESET_INFO_CHECK(pub_name, priv_name) \
static_assert(kDifRstmgrResetInfo##pub_name == \
(0x1 << RSTMGR_RESET_INFO_##priv_name##_BIT), \
"kDifRstmgrResetInfo" #pub_name \
" must match the register definition!")

RSTMGR_RESET_INFO_CHECK(Por, INFO_POR);
RSTMGR_RESET_INFO_CHECK(LowPowerExit, INFO_LOW_POWER_EXIT);
// These assertions are only defined for the Earl Grey chip.
#if defined(OPENTITAN_IS_EARLGREY)

RSTMGR_RESET_INFO_CHECK(Por, POR);
RSTMGR_RESET_INFO_CHECK(LowPowerExit, LOW_POWER_EXIT);
RSTMGR_RESET_INFO_CHECK(Sw, SW_RESET);

static_assert(kDifRstmgrResetInfoHwReq == (RSTMGR_RESET_INFO_HW_REQ_MASK
<< RSTMGR_RESET_INFO_HW_REQ_OFFSET),
"kDifRstmgrResetInfoHwReq must match the register definition!");

static_assert(
RSTMGR_PARAM_NUM_SW_RESETS == 8,
"Number of software resets has changed, please update this file!");

// The Reset Manager implementation will have to be updated if the number
// of software resets grows, as it would span across multiple registers, so
// there will be multiple of Reset Enable and Reset Control registers. The
Expand All @@ -54,7 +52,7 @@ static_assert(
DIF_RSTMGR_ALERT_INFO_MAX_SIZE == RSTMGR_ALERT_INFO_CTRL_INDEX_MASK,
"Alert info dump max size has grown, please update the public define!");
#elif defined(OPENTITAN_IS_DARJEELING)
// TODO: equivalent assertations are not yet defined for Darjeeling
// TODO: equivalent assertions are not yet defined for Darjeeling
#else
#error "dif_rstmgr does not support this top"
#endif
Expand All @@ -81,13 +79,20 @@ static bool cpu_capture_is_locked(mmio_region_t base_addr) {
return !bitfield_bit32_read(bitfield, RSTMGR_CPU_REGWEN_EN_BIT);
}

static inline ptrdiff_t get_regwen_reg_offset(dif_rstmgr_peripheral_t p) {
return RSTMGR_SW_RST_REGWEN_0_REG_OFFSET + 4 * (ptrdiff_t)p;
}

/**
* Checks whether the software reset is disabled for a `peripheral`.
*/
static bool rstmgr_software_reset_is_locked(
mmio_region_t base_addr, dif_rstmgr_peripheral_t peripheral) {
return !mmio_region_read32(
base_addr, RSTMGR_SW_RST_REGWEN_0_REG_OFFSET + 4 * (ptrdiff_t)peripheral);
return !mmio_region_read32(base_addr, get_regwen_reg_offset(peripheral));
}

static inline ptrdiff_t get_ctrl_n_reg_offset(dif_rstmgr_peripheral_t p) {
return RSTMGR_SW_RST_CTRL_N_0_REG_OFFSET + 4 * (ptrdiff_t)p;
}

/**
Expand All @@ -97,9 +102,7 @@ static void rstmgr_software_reset_hold(mmio_region_t base_addr,
dif_rstmgr_peripheral_t peripheral,
bool hold) {
bool value = hold ? false : true;
mmio_region_write32(
base_addr, RSTMGR_SW_RST_CTRL_N_0_REG_OFFSET + 4 * (ptrdiff_t)peripheral,
value);
mmio_region_write32(base_addr, get_ctrl_n_reg_offset(peripheral), value);
}

/**
Expand All @@ -122,9 +125,7 @@ dif_result_t dif_rstmgr_reset(const dif_rstmgr_t *handle) {

// Set bits to stop holding all peripherals in the reset state.
for (uint32_t i = 0; i < RSTMGR_PARAM_NUM_SW_RESETS; i++) {
mmio_region_write32(base_addr,
RSTMGR_SW_RST_CTRL_N_0_REG_OFFSET + (ptrdiff_t)i * 4,
UINT32_MAX);
mmio_region_write32(base_addr, get_ctrl_n_reg_offset(i), UINT32_MAX);
}

return kDifOk;
Expand All @@ -138,9 +139,7 @@ dif_result_t dif_rstmgr_reset_lock(const dif_rstmgr_t *handle,

mmio_region_t base_addr = handle->base_addr;

mmio_region_write32(
base_addr, RSTMGR_SW_RST_REGWEN_0_REG_OFFSET + 4 * (ptrdiff_t)peripheral,
0);
mmio_region_write32(base_addr, get_regwen_reg_offset(peripheral), 0);

return kDifOk;
}
Expand Down Expand Up @@ -400,8 +399,7 @@ dif_result_t dif_rstmgr_software_reset_is_held(

// When the bit is cleared - peripheral is held in reset.
*asserted =
!mmio_region_read32(handle->base_addr, RSTMGR_SW_RST_CTRL_N_0_REG_OFFSET +
4 * (ptrdiff_t)peripheral);
!mmio_region_read32(handle->base_addr, get_ctrl_n_reg_offset(peripheral));

return kDifOk;
}
Expand Down
5 changes: 1 addition & 4 deletions sw/device/lib/testing/test_framework/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,11 +327,8 @@ dual_cc_library(
deps = dual_inputs(
device = [
":ottf_console",
"//sw/device/lib/dif:uart",
"//sw/device/lib/runtime:print",
],
shared = [
"//sw/device/lib/base:status",
"//sw/device/lib/testing/json:ottf",
"//sw/device/lib/ujson",
],
Expand Down Expand Up @@ -371,6 +368,7 @@ OTTF_CONSOLE_BACKENDS = {
"//sw/device/lib/dif:spi_device",
"//sw/device/lib/dif:pinmux",
"//sw/device/lib/testing:spi_device_testutils",
"//sw/device/lib/dif:gpio",
],
"defines": ["OTTF_CONSOLE_HAS_SPI_DEVICE"],
},
Expand Down Expand Up @@ -414,7 +412,6 @@ cc_library(
]) + [
":ottf_console_types",
"//sw/device/lib/base:status",
"//sw/device/lib/dif:gpio",
],
)

Expand Down
2 changes: 2 additions & 0 deletions sw/device/lib/testing/test_framework/ottf_console.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,10 @@ void ottf_console_configure_null(ottf_console_t *console) {
}

void ottf_console_init(void) {
#if defined(OTTF_CONSOLE_HAS_UART) || defined(OTTF_CONSOLE_HAS_SPI_DEVICE)
// Initialize/Configure the console device.
uintptr_t base_addr = kOttfTestConfig.console.base_addr;
#endif

switch (kOttfTestConfig.console.type) {
#ifdef OTTF_CONSOLE_HAS_UART
Expand Down
3 changes: 2 additions & 1 deletion sw/device/lib/testing/test_framework/ottf_console.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
#include <stdint.h>

#include "sw/device/lib/base/status.h"
#include "sw/device/lib/dif/dif_gpio.h"
#ifdef OTTF_CONSOLE_HAS_SPI_DEVICE
#include "sw/device/lib/testing/test_framework/ottf_console_spi.h"
#endif
Expand Down Expand Up @@ -38,6 +37,7 @@ struct ottf_console {
size_t buf_size;
/** Where to write next to the staging buffer. */
size_t buf_end;
#if defined(OTTF_CONSOLE_HAS_UART) || defined(OTF_CONSOLE_HAS_SPI_DEVICE)
/** Auxiliary data, per console type */
union {
#ifdef OTTF_CONSOLE_HAS_UART
Expand All @@ -49,6 +49,7 @@ struct ottf_console {
ottf_console_spi_t spi;
#endif // OTTF_CONSOLE_HAS_SPI_DEVICE
} data;
#endif // defined(OTTF_CONSOLE_HAS_UART) || defined(OTF_CONSOLE_HAS_SPI_DEVICE)
};

/**
Expand Down
2 changes: 0 additions & 2 deletions sw/device/lib/testing/test_framework/ujson_ottf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

#include "sw/device/lib/testing/test_framework/ujson_ottf.h"

#include "sw/device/lib/base/status.h"
#include "sw/device/lib/dif/dif_uart.h"
#include "sw/device/lib/testing/test_framework/ottf_console.h"
#include "sw/device/lib/ujson/ujson.h"

Expand Down
39 changes: 23 additions & 16 deletions sw/device/lib/testing/test_rom/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -183,9 +183,14 @@ cc_library(
"test_rom_start.S",
],
local_defines = opentitan_for_ip({
"flash_ctrl": ["HAS_NVM"],
"rram_ctrl": ["HAS_NVM"],
"otp_ctrl": ["HAS_OTP_CTRL"],
"pinmux": ["HAS_PINMUX"],
"uart": ["HAS_UART"],
}) + opentitan_select_top(
{
"darjeeling": ["SKIP_BOOTSTRAP"], # FIXME bootstrap disabled for now
"englishbreakfast": [],
},
["HAS_RETENTION_RAM"],
Expand All @@ -194,31 +199,37 @@ cc_library(
deps = [
":target_test_rom_lib",
":test_rom_manifest",
"//hw/top:ast_c_regs",
"//hw/top:clkmgr_c_regs",
"//hw/top:sram_ctrl_c_regs",
"//hw/top:top_lib",
"//sw/device/lib/arch:device",
"//sw/device/lib/base:abs_mmio",
"//sw/device/lib/base:bitfield",
"//sw/device/lib/base:mmio",
"//sw/device/lib/crt",
"//sw/device/lib/dif:clkmgr",
"//sw/device/lib/dif:gpio",
"//sw/device/lib/dif:pinmux",
"//sw/device/lib/dif:rv_core_ibex",
"//sw/device/lib/dif:spi_device",
"//sw/device/lib/dif:uart",
"//sw/device/lib/runtime:hart",
"//sw/device/lib/runtime:log",
"//sw/device/lib/runtime:print_uart",
"//sw/device/lib/testing:pinmux_testutils",
"//sw/device/lib/testing/test_framework:check",
"//sw/device/lib/testing/test_framework:status",
"//sw/device/silicon_creator/lib:build_info",
"//sw/device/silicon_creator/lib/base:sec_mmio",
"//sw/device/silicon_creator/lib/base:static_critical",
] + opentitan_for_ip({
"ast": ["//hw/top:ast_c_regs"],
"clkmgr": [
"//hw/top:clkmgr_c_regs",
"//sw/device/lib/dif:clkmgr",
],
"gpio": ["//sw/device/lib/dif:gpio"],
"pinmux": [
"//sw/device/lib/dif:pinmux",
"//sw/device/lib/testing:pinmux_testutils",
],
"spi_device": ["//sw/device/lib/dif:spi_device"],
"uart": [
"//sw/device/lib/dif:uart",
"//sw/device/lib/runtime:print_uart",
],
"entropy_src": ["//hw/top:entropy_src_c_regs"],
"csrng": ["//hw/top:csrng_c_regs"],
"edn": ["//hw/top:edn_c_regs"],
Expand All @@ -227,13 +238,9 @@ cc_library(
"//hw/top:otp_ctrl_c_regs",
"//hw/top/dt:otp_ctrl",
],
}) + opentitan_select_top(
{
"earlgrey": ["//sw/device/lib/testing:nvm_testutils"],
"englishbreakfast": ["//sw/device/lib/testing:nvm_testutils"],
},
[],
),
"flash_ctrl": ["//sw/device/lib/testing:nvm_testutils"],

@mfnch mfnch Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I need to have a look at how to handle nvm_testutils. I think this library should shield its users from having to check whether the hardware is supported... a bit like the ottf_console?

@mfnch mfnch Aug 21, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

One option would be to change nvm_testutils so that it always compiles, even when there is no hardware (flash/rram). The library would then fail only when trying to write or read to NVM. In practice, however, there aren't many users of nvm_testutils - apart from this one - that would benefit from this change (most users want to to actually write from or read to NVM rather than optionally initialize it).

However, the problem of how we pull a NVM library in as a dependency is a real one. It'd be nice to have an "alias" IP that just tells us whether a NVM IP is present in the current top. That would make handling dependencies to this library nicer.

"rram_ctrl": ["//sw/device/lib/testing:nvm_testutils"],
}),
)

opentitan_test(
Expand Down
Loading
Loading