lowRISC / lowRISC/opentitan

[dv,flash_ctrl] Fix initialization of secret pages

Open
#22,799 3 comments 0 reactions 2 assignees View on GitHub

@matutem is already working on this.

Since Apr 24, 2024.

Dominant language
SystemVerilog
Stars
3.6k
Forks
1.1k
Avg merge
2d 22h
Merged PRs (30d)
141

Description

### Description

The flash_ctrl block level environment initializes pages 1, 2, and 3 with random data with scrambling and ecc configured per the update_secret_partition function at https://cs.opensource.google/opentitan/opentitan/+/master:hw/ip/flash_ctrl/dv/env/seq_lib/flash_ctrl_base_vseq.sv;drc=30d7e787c753caaa03fe68a4a70da1bbcbc1d96f;l=1452. This is done in apply_reset.

Subsequent reads will instead derive the scrambling and ecc properties from the settings applied to the CSRs. These settings can be different from the ones used in update_secret_partition, which will cause ecc errors on reads, and there are tests that show this problem. Also notice the assumption that the setting of [hw_info_cfg_override](https://opentitan.org/book/hw/ip/flash_ctrl/doc/registers.html#hw_info_cfg_override), via cfg.ovrd_ecc_dis and cfg.ovrd_scr_dis, do not affect page 3. Interestingly, I have not found how these hw_info_cfg_override (or cfg.ovrd_???_dis) are used in DV: apparently we only use the configured per page settings.

The question is what is the use model of these pages? Should the CSRs be set to definitive values to match initialization? Should these pages be re-initialized on reset, as they are now?

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.