[rom_ctrl] ROM verification does not make use ECC bits
- Dominant language
- SystemVerilog
- Stars
- 3.6k
- Forks
- 1.1k
- Avg merge
- 2d 22h
- Merged PRs (30d)
- 141
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: https://github.com/lowRISC/opentitan/blob/1f7db17392086db69b4f3f161e3d02f59f7dfb0d/hw/ip/rom_ctrl/rtl/rom_ctrl.sv#L379
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](https://github.com/lowRISC/opentitan/issues/16072#issuecomment-1306170540).
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.
Contributor guide
Research direction
Start with hw/ip/rom_ctrl/rtl/rom_ctrl.sv around the KMAC verification path linked in the issue, then compare its ECC handling with prim_ram_1p_adv. Read chip_sw_rom_ctrl_integrity_check_vseq and the discussion in #16072 to resolve how digest data differs from other ROM data. Done means the chosen ECC behavior is implemented and the sequence covers corruption of non-digest data.
Written by the indexing model from the issue text.
Assessment
- Domain
- embedded-iot, security
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100