regfile_latch: oh_raddr_a_err signal generated based on raddr_b_int

[glaserf]

Observed Behavior

The onehot read address error signal for read port a in ibex_register_file_latch.sv is generated based on the read address for read port b (raddr_b_int).

ibex/rtl/ibex_register_file_latch.sv

Lines 110 to 125 in 53888bc

	// Check the one-hot encoded signals for glitches.
	prim_onehot_check #(
	.AddrWidth(ADDR_WIDTH),
	.OneHotWidth(NUM_WORDS),
	.AddrCheck(1),
	// When AddrCheck=1 also EnableCheck needs to be 1.
	.EnableCheck(1)
	) u_prim_onehot_check_raddr_a (
	.clk_i,
	.rst_ni,
	.oh_i (raddr_onehot_a_buf),
	.addr_i (raddr_b_int),
	// Set enable=1 as address is always valid.
	.en_i (1'b1),
	.err_o (oh_raddr_a_err)
	);

Expected Behavior

The onehot read address error signal for read port a should be generated based on the read address for read port a, like in ibex_register_file_ff.sv and ibex_register_file_fpga.sv.

Version of the Ibex source code

53888bc (part of master)

[rswarbrick]

Thanks for this report, and it looks like there was a typo in 35bbdb7. @nasahlpa: Have I understood correctly and do we just need to change a character?

[GregAC]

Well spotted @glaserf!

Looks like this only effects the latch implementation of the register file, which is why I think we didn't find it previously (as we simulate with the FF based register file). We should consider refactoring here so the security hardening is factored out into some common module that works equally against the FF/Latch/FPGA implementation options.

[glaserf]

Looks like this only effects the latch implementation of the register file, which is why I think we didn't find it previously (as we simulate with the FF based register file).

This is what I supposed - I stumbled upon it as this file caused a conflict during merging to an internal development branch were we added some test structures for the non-secure Ibex variant.

Refactoring to avoid code duplification (well, tripflification, currently) and ease verification coverage sure sounds very reasonable to me 👍🏼

[nasahlpa]

Nicely spotted @glaserf! Indeed, this is a typo introduced with the recent changes in the RF hardening.

Fixed in #2216.