-
Notifications
You must be signed in to change notification settings - Fork 914
Description
Description
It is my understanding that rom_ctrl only uses the 32 data bits of the ROM when doing the verification via KMAC and it ignores the ECC bits:
opentitan/hw/ip/rom_ctrl/rtl/rom_ctrl.sv
Line 379 in 1f7db17
| .rom_data_i (checker_rom_rdata[31:0]), |
However, when reading from the ROM via the TLUL interface, the ECC bits are preserved.
This means that if there is a wrong bit in the data part of the ROM, e.g., due to manufacturing defects, then the ROM cannot be verified because the ECC bits are not being used to correct the data.
This also means that the ECC part of the ROM are currently wasted area. They are only used when reading data via the TLUL interface but to get to the point the ROM verification needs to happen first, which will always fail if there is an error in the data bits.
Open Questions:
I did not find the point where the ECC bits in the TLUL response are being used to correct any data. Is this the case or does ibex simply raise an alert when there is a (correctable) error?
Possible Solution:
I propose to use ECC and correct any data errors before passing the data to KMAC.
I think ideally this is being done close to the ROM itself similar to how prim_ram_1p_adv does ECC data correction before anyone can consume it.
However, the digest part of the ROM is currently not ECC encoded (or scrambled) according to this comment.
This means that the correction/decoding should only happen when not reading the digest, or we store ECC bits for the unscrambled digest in the ROM. I prefer the former in order not to break any existing flows. However we should discuss this and carefully consider our options.
More Context:
This issue has also been discussed in #16072 but mostly only from a security perspective.
I am arguing that we should make use of the ECC bits that we are storing anyway to increase reliability.
Implementation Notes:
When making this change we should probably also update chip_sw_rom_ctrl_integrity_check_vseq and corrupt non-digest data as well.