[hw] rom_ctrl added to mocha by KolosKoblasz-Semify · Pull Request #389 · lowRISC/mocha

KolosKoblasz-Semify · 2026-03-27T14:03:07Z

existing source files vendored in
rom_ctrl integrated at topl level, AXI and TL busnetwork extended
new smoketest written for reading rom init values
xbar_peri regenerated include new rom_ctrl_regs TL interface

marnovandermaas

Some initial comments from my end. Please also have a look at the CI failures.

ziuziakowska

Just some nits on the software.

Something to consider might also be modelling the ROM as a const char * instead of a void * typedef, as that could prevent writing through the pointer (although I believe that should cause a fault anyway).

raylau1

Thanks for adding this, I have added some comment.

marnovandermaas

Great work! Some more comments from my end before we can merge this.

marnovandermaas

Some more comments on the software side.

* rom_ctrl_regs if added to xbapr_peri cfg

* kmac vendored in * lowrisc_ip.vendor.hjson modified * Apply patch on prim_xilinx_rom.sv. Vivado synth failed due to string type parameter.

* axi bus network expanded to handle rom_ctrl data interface * xbar_peri module connected to rom_ctrl register interface * verilator rule exclusions added * Path to ROM init file added to tb.sv * clk and reset of the new axi bus and rom_ctrl fixed on top level * backdoor access for ROM config added

* new testcase implemented among top level sw tests * the new test reads the compile time initialized ROM values * if any comparision fails (expected values differs from read value) test is terminated with an error * sw test files reformated to complie with clang format * fixed issues highlighted during PR review

KolosKoblasz-Semify · 2026-04-17T06:36:08Z

Just some nits on the software.

Something to consider might also be modelling the ROM as a const char * instead of a void * typedef, as that could prevent writing through the pointer (although I believe that should cause a fault anyway).

If we do these changes for rom_ctrl tests then it will be inconsistent with other sw tests like: spi_host, rstmgr, mailbox, etc..
It is also working as is, so I don't think it worth the effort and further delays.

* xbar_peri IP regenerated with new paths during CI/CD tests * ROM backdoor init command added to sw regression tests

* FPGA tests need ROM initialization to pass * RomInitFile argument and init file paths added to the nix/fpga.nix script

raylau1

Thanks for adding all the fixes! I found a few small issues with DV and software, otherwise should be ready to merge.

raylau1 · 2026-04-17T12:40:49Z

      name: rom_ctrl
      tests: [
-        "rom_ctrl_integrity_check"
+        "rom_ctrl_integrity_check",
+        "rom_ctrl_smoke"


rom_ctrl_smoke_cheri should be added to the rom_ctrl tests

raylau1 · 2026-04-17T12:56:42Z

+rom_t mocha_system_rom(void);
+rom_ctrl_t mocha_system_rom_ctrl(void);


Since this is sorted in order of memory map, mocha_system_rom should be before mailbox and mocha_system_rom_ctrl between rstmgr and uart

raylau1 · 2026-04-17T13:00:17Z

+#include "hal/mocha.h"
+#include <stdint.h>
+
+/* Read specific word_idx-th word from ROM memory */


This comment seems outdated, there is no word_idx argument and rel_addr specifies a byte offset

raylau1 · 2026-04-17T13:03:56Z

+bool test_main()
+{
+    rom_t rom = mocha_system_rom();
+    uart_t console = mocha_system_uart();
+
+    uart_init(console);
+
+    return rom_read_test(rom, console);
+}


There is no need to reinitialize UART, you can get the console handle with:

bool test_main(uart_t console) { ... }

raylau1 · 2026-04-17T13:05:58Z

        "mailbox_smoke",
        "mailbox_smoke_cheri",
        "axi_sram_smoke",
-        "axi_sram_smoke_cheri",
+        "axi_sram_smoke_cheri"
      ]
    }


rom_ctrl_smoke and rom_ctrl_smoke_cheri should be added to the smoke regression tests. Also do not remove the comma if it is not necessary.

marnovandermaas

A few more comments from my end. Would you mind checking that all your lint rules are necessary? It looks like your disabling more than you need to still, which is undesirable.

marnovandermaas · 2026-04-17T13:39:08Z

    : _sram(sram_hier_path, sram_size_words, 8),
-      _dram(dram_hier_path, dram_size_words, 8) {}
+      _dram(dram_hier_path, dram_size_words, 8),
+      _rom("TOP.top_chip_verilator.u_top_chip_system.u_rom_ctrl.gen_rom_scramble_disabled.u_rom.u_prim_rom", 8192, 4) {}


We should add rom_hier_path and rom_size_words in here.

marnovandermaas · 2026-04-17T13:49:18Z

+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*kmac_rom_last_outer*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*kmac_rom_last_outer*"
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*kmac_err*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*kmac_err*"


Are you sure about this undriven?

mocha/hw/vendor/lowrisc_ip/ip/rom_ctrl/rtl/rom_ctrl.sv

Lines 150 to 154 in fac6f9b

assign kmac_data_o = '0;

assign kmac_rom_rdy_outer = 1'b0;

assign kmac_done = 1'b0;

assign kmac_digest = '0;

assign kmac_err = 1'b0;

marnovandermaas · 2026-04-17T13:49:35Z

+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*kmac_rom_rdy*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*kmac_rom_rdy*"
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*kmac_rom_rdy_outer*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*kmac_rom_rdy_outer*"


Are you sure about this undriven?

mocha/hw/vendor/lowrisc_ip/ip/rom_ctrl/rtl/rom_ctrl.sv

Lines 150 to 154 in fac6f9b

assign kmac_data_o = '0;

assign kmac_rom_rdy_outer = 1'b0;

assign kmac_done = 1'b0;

assign kmac_digest = '0;

assign kmac_err = 1'b0;

marnovandermaas · 2026-04-17T13:54:17Z

+
+// rom_ctrl.sv
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*checker_rom_rdata*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*checker_rom_rdata*"


Are you sure about this undriven?

mocha/hw/vendor/lowrisc_ip/ip/rom_ctrl/rtl/rom_ctrl.sv

Line 250 in fac6f9b

.chk_rdata_o (checker_rom_rdata),

marnovandermaas · 2026-04-17T13:55:29Z

+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*checker_rom_rdata*"
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*checker_rom_rdata_outer*"
+lint_off -rule UNDRIVEN     -file "*/rom_ctrl.sv" -match "*checker_rom_rdata_outer*"
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*kmac_rom_rdy*"


Are you sure about this unused?

mocha/hw/vendor/lowrisc_ip/ip/rom_ctrl/rtl/rom_ctrl.sv

Line 454 in fac6f9b

assign unused_fsm_inputs = ^{kmac_rom_rdy, kmac_done, kmac_digest, digest_q, exp_digest_q};

marnovandermaas · 2026-04-17T13:56:47Z

+lint_off -rule WIDTHTRUNC -file "*/lowrisc_prim_count_0/rtl/prim_count.sv"
+
+// rom_ctrl.sv
+lint_off -rule UNUSEDSIGNAL -file "*/rom_ctrl.sv" -match "*checker_rom_rdata*"


Do you mind changing all of these files to "*/rom_ctrl/src/rom_ctrl.sv"

marnovandermaas

Did you mean to create an empty commit?

KolosKoblasz-Semify marked this pull request as ready for review March 27, 2026 14:55

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 2affd6c to 499b597 Compare March 30, 2026 08:07

KolosKoblasz-Semify changed the title ~~rom_ctrl added to mocha~~ [hw] rom_ctrl added to mocha Mar 30, 2026

marnovandermaas requested review from marnovandermaas and raylau1 March 30, 2026 12:25

marnovandermaas reviewed Mar 30, 2026

View reviewed changes

marnovandermaas mentioned this pull request Mar 30, 2026

rom_ctrl integration #351

Closed

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch 3 times, most recently from 71fd63b to e3eebff Compare April 1, 2026 09:23

ziuziakowska reviewed Apr 2, 2026

View reviewed changes

Comment thread sw/device/tests/rom_ctrl/smoketest.c Outdated

Comment thread sw/device/lib/hal/rom_ctrl.h Outdated

Comment thread sw/device/lib/hal/CMakeLists.txt Outdated

Comment thread sw/device/tests/rom_ctrl/smoketest.c Outdated

Comment thread sw/device/tests/rom_ctrl/smoketest.c Outdated

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from e3eebff to 6f80dee Compare April 7, 2026 08:34

raylau1 reviewed Apr 7, 2026

View reviewed changes

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch 2 times, most recently from e72eb24 to 555b922 Compare April 7, 2026 13:42

This comment was marked as resolved.

Sign in to view

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 555b922 to 8075c5c Compare April 8, 2026 11:23

marnovandermaas mentioned this pull request Apr 8, 2026

Integrate ROM #135

Open

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch 2 times, most recently from 33172ca to 4ee0a2b Compare April 9, 2026 12:36

KolosKoblasz-Semify requested review from marnovandermaas, raylau1 and ziuziakowska April 9, 2026 13:36

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 4ee0a2b to 3090824 Compare April 9, 2026 13:37

martin-velay mentioned this pull request Apr 10, 2026

ROM DV - Top-level basic register access #148

Open

marnovandermaas reviewed Apr 13, 2026

View reviewed changes

Comment thread sw/device/tests/rom_ctrl/smoketest.c Outdated

Comment thread sw/device/tests/rom_ctrl/smoketest.c Outdated

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 3090824 to 6cf5ade Compare April 14, 2026 11:46

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 6cf5ade to d7fc6d6 Compare April 15, 2026 07:48

KolosKoblasz-Semify added 4 commits April 15, 2026 09:03

[xbar_peri, feat] xbar_peri interconnect regenerated

bf89f97

* rom_ctrl_regs if added to xbapr_peri cfg

[rom_ctrl, vendoring] rom_ctrl dependencies vendored in

4197ec8

* kmac vendored in * lowrisc_ip.vendor.hjson modified * Apply patch on prim_xilinx_rom.sv. Vivado synth failed due to string type parameter.

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from d7fc6d6 to 9e41b05 Compare April 15, 2026 08:04

KolosKoblasz-Semify added 2 commits April 17, 2026 08:09

[rom_ctrl, script] rom_ctrl related script upgrades

a571643

* xbar_peri IP regenerated with new paths during CI/CD tests * ROM backdoor init command added to sw regression tests

[rom_ctrl, fpga] Add ROM init in fpga bit file

91b1f74

* FPGA tests need ROM initialization to pass * RomInitFile argument and init file paths added to the nix/fpga.nix script

KolosKoblasz-Semify force-pushed the kk_rom_ctrl branch from 9e41b05 to 91b1f74 Compare April 17, 2026 07:10

raylau1 reviewed Apr 17, 2026

View reviewed changes

marnovandermaas reviewed Apr 17, 2026

View reviewed changes

		rom_t mocha_system_rom(void);
		rom_ctrl_t mocha_system_rom_ctrl(void);

	assign kmac_data_o = '0;
	assign kmac_rom_rdy_outer = 1'b0;
	assign kmac_done = 1'b0;
	assign kmac_digest = '0;
	assign kmac_err = 1'b0;

Conversation

KolosKoblasz-Semify commented Mar 27, 2026

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ziuziakowska left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

raylau1 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as resolved.

This comment was marked as resolved.

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

KolosKoblasz-Semify commented Apr 17, 2026

Uh oh!

raylau1 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

marnovandermaas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!