ibex icon indicating copy to clipboard operation
ibex copied to clipboard

regfile_latch: oh_raddr_a_err signal generated based on raddr_b_int

Open glaserf opened this issue 1 year ago • 4 comments

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).

https://github.com/lowRISC/ibex/blob/53888bcdf4ca3c07e5e715fb6386cb4cc643a61b/rtl/ibex_register_file_latch.sv#L110-L125

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)

glaserf avatar Sep 17 '24 13:09 glaserf

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

rswarbrick avatar Sep 17 '24 13:09 rswarbrick

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.

GregAC avatar Sep 17 '24 13:09 GregAC

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 👍🏼

glaserf avatar Sep 17 '24 13:09 glaserf

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

Fixed in #2216.

nasahlpa avatar Sep 17 '24 13:09 nasahlpa