[rtl] RRAM bkdr by gautschimi · Pull Request #30740 · lowRISC/opentitan

gautschimi · 2026-07-14T14:24:28Z

As per RFC , Earlgrey will be extended with support for RRAM based non-volatile memory.

This PR adds the backdoor access to the RRAM. This is used in some DV tests to either preload the RRAM with the firmware, or to examine certain memory locations during a test.

This PR is based on #30412

rswarbrick

Various requests for documentation comments (sorry) but this looks like a sensible implementation.

rswarbrick · 2026-07-16T10:54:37Z

+  localparam int unsigned RramDataWidth      = rram_ctrl_pkg::DataWidth;
+  localparam int unsigned RramKeySize        = rram_ctrl_pkg::KeySize;
+  localparam int unsigned RramAddrW          = rram_ctrl_pkg::AddrW;
+  localparam int unsigned RramStagesPerCycle = RramDataWidth / rram_ctrl_pkg::GfMultCycles;


What happens if this doesn't divide cleanly? (And do we care?)

Its a requirement of the gfmultiplier hardware module. so it must be a clean division

rswarbrick · 2026-07-16T10:56:56Z

+  localparam int unsigned RramAddrW          = rram_ctrl_pkg::AddrW;
+  localparam int unsigned RramStagesPerCycle = RramDataWidth / rram_ctrl_pkg::GfMultCycles;
+  localparam int unsigned RramNumRoundsHalf  = crypto_dpi_prince_pkg::NumRoundsHalf;
+  localparam int unsigned RramDataByteWidth  = $clog2(RramDataWidth / 8);


What happens if this doesn't divide cleanly? Do you want a +7 here? (Or maybe a check in the constructor? "These parameters don't make sense, so you can't make the class today")

RramDataWidth is always a multiple of a byte.

But I guess I can add a check to verify the parameters

rswarbrick · 2026-07-16T10:57:35Z

+  // Initialize the class instance.
+  // `extra_bits_per_subword` is the width of any additional metadata that is not captured in the
+  // secded package.


Can all the arguments be documented? (Possibly just by pointing to the base class)

added. Most of them are actually not used

rswarbrick · 2026-07-16T10:59:42Z

+    mult_out = operand[RramDataWidth-1] ? (operand << 1) ^
+        rram_ctrl_pkg::ScrambleIPoly : (operand << 1);


I know that not everyone agrees about indentation, but this looks wrong to me: surely we shouldn't out-dent the middle of a ternary case?

this looks indeed horrible:) sorry, I will fix it!

rswarbrick · 2026-07-16T11:00:09Z

+    end
+  endfunction
+
+  static function bit [RramDataWidth-1:0] rram_gf_mult2(bit [RramDataWidth-1:0] operand);


This function needs a documentation comment (which should describe the operand argument as well as the point of the function)

This is just the inner part of the gfmultiplier. I also don't really know what it is doing in detail. I took it over from flash_ctrl

rswarbrick · 2026-07-16T11:02:05Z

+  static function bit [RramStagesPerCycle-1:0][RramDataWidth-1:0] rram_gen_matrix(
+                                                                      bit [RramDataWidth-1:0] seed,
+                                                                      bit init);
+    bit [RramStagesPerCycle-1:0][RramDataWidth-1:0] matrix_out;


It might be nice to use a typedef for this type?

rswarbrick · 2026-07-16T11:02:42Z

+
+    matrix_out[0] = init ? seed : rram_gf_mult2(seed);
+
+    matrix_out[RramStagesPerCycle-1:1] = '0;


Does this line have any effect?

no, removed

rswarbrick · 2026-07-16T11:04:44Z

+  // Compute the tweak for the xex cipher. tweak = GfMult128(addr, addr_key)
+  static function bit [RramDataWidth-1:0] rram_galois_multiply(bit [RramKeySize-1:0] addr_key,
+                                                               bit [RramAddrW-1:0] addr);
+    bit [RramStagesPerCycle-1:0][RramDataWidth-1:0] current_matrix;


That current_matrix and mult_out probably need their scopes moved to inside the loop?

yes that would be cleaner. let me do that

rswarbrick · 2026-07-16T11:05:42Z

+    return total_product;
+  endfunction
+
+  function bit [RramDataWidth-1:0] rram_scramble_data(bit [RramDataWidth-1:0] data,


This function needs documenting.

rswarbrick · 2026-07-16T11:06:46Z

+    return scrambled_data ^ mask;
+  endfunction
+
+  function bit [RramDataWidth-1:0] rram_descramble_data(bit [RramDataWidth-1:0] data,


This function needs documenting.

This bkdr util is required to preload the RRAM or to examine certain memory location during tests. Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>

gautschimi requested review from rswarbrick, thommythomaso and vogelpi July 14, 2026 14:24

gautschimi marked this pull request as ready for review July 14, 2026 14:24

gautschimi requested a review from a team as a code owner July 14, 2026 14:24

gautschimi self-assigned this Jul 14, 2026

gautschimi force-pushed the rram_ctrl_bkdr branch from 439dfd2 to 1f0d7d1 Compare July 14, 2026 15:05

rswarbrick reviewed Jul 16, 2026

View reviewed changes

gautschimi force-pushed the rram_ctrl_bkdr branch from 1f0d7d1 to 252c9e0 Compare July 16, 2026 13:26

[rram_ctrl,dv] Add rram bkdr util

402fc3f

This bkdr util is required to preload the RRAM or to examine certain memory location during tests. Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>

gautschimi force-pushed the rram_ctrl_bkdr branch from 252c9e0 to 402fc3f Compare July 16, 2026 13:27

gautschimi added the CI:Rerun Rerun failed CI jobs label Jul 17, 2026

github-actions Bot removed the CI:Rerun Rerun failed CI jobs label Jul 17, 2026

		mult_out = operand[RramDataWidth-1] ? (operand << 1) ^
		rram_ctrl_pkg::ScrambleIPoly : (operand << 1);


		matrix_out[0] = init ? seed : rram_gf_mult2(seed);

		matrix_out[RramStagesPerCycle-1:1] = '0;

Uh oh!

Conversation

gautschimi commented Jul 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rswarbrick 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gautschimi commented Jul 14, 2026 •

edited

Loading