Skip to content

Conversation

AlexJones0
Copy link

This PR is the second of a series of 3 PRs to improve the OpenTitan Flash Controller emulation and make it more feature complete, with a focus on the features that are needed to support keymgr integration.

This PR focuses on refactoring and introducing additional logic to allow the flash controller to support both software- and hardware-initiated flash operations. For context, SW and HW operations use the same controller operation logic, but:

  1. HW operations are not visible to SW registers, except for fatal faults, REGWEN locking, etc.
  2. SW and HW operations share one program FIFO but have separate read FIFOs
  3. SW and HW operations have completely different data & info page protection mechanisms, where protection is based on the current flash controller's lifecycle state.

As such, this PR:

  • Implements some stubbed flash lifecycle management (support for initialization including loading flash secrets, but not for RMA entry).
  • Implements generalized support for HW flash operations.
  • Implements HW data page and info page protection mechanisms.
  • Implements additional lc_ctrl-backed secret protection (qualifications), though with no lc_ctrl connections in this PR.

See the commit messages for more details about each change.

Note also that the HW operations are currently unused in this PR, but will be utilized in a subsequent PR that will add an interface for retrieving keymgr secrets from the flash, and connect the flash to the lc_ctrl.

Copy link

@rivos-eblot rivos-eblot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I read nothing suspicious however I've forgotten how the flash controller works, so leaving to people with better knowledge the approval for these changes. Seems ok to me.

@AlexJones0
Copy link
Author

AlexJones0 commented Sep 8, 2025

Thanks for the reviews!

There are some tests //sw/device/tests:flash_ctrl_info_access_lc_{} for the dev, prod, prodend, rma and testunlocked0 states that at least test the lc_ctrl integration and SW qualification changes. Unfortunately due to some issues in the QEMU Bazel integration these are always passing (always boot into RMA state). Ideally this will be fixed on OpenTitan so we can actually test that QEMU is performing the correct behaviours.

The hardware operations cannot be trivially tested until keymgr is implemented, so testing that will unfortunately have to wait a few more PRs.

@AlexJones0 AlexJones0 force-pushed the eg-flash-hw-operations branch from 958a4a7 to 642e477 Compare September 9, 2025 10:31
This file was not fully formatted properly, run clang-format to re-order
and use the correct include orders.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Refactor FIFO status updates to all occur after finishing the current
execution of an operation, rather than handling all the individual
status and interrupt logic.

This is equivalent and easier to understand/maintain, and will
importantly allow easier implementation of hardware flash controller
reads in the future.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Add some basic flash lifecycle manager phases which are used for RMA
entry and initialization, starting in the `None` phase and moving to
the `Seed` phase during initialization.

Also take the opportunity to re-order the flash traces whilst adding a
new one as clean-up.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Hardware flash requests are processed in the same way as flash
controller requests, but have key differences in how they update the
state of software-visible status registers and behave on errors (they
fault with a fatal error that needs a reset).

Hardware operation requests also have different protection rules
depending on the lifecycle phase, but that is stubbed as unimplemented
for now (all HW-sourced operations are allowed).

Hardware and software operations share a program FIFO, but do not share
read buffers. To facilitate sharing a FIFO, software is locked out of
writing to the program FIFO while there is not an active *sw-initiated*
program operation.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
… checkpatch

`scripts/checkpatch.pl` wants there to be no spacing around colons in
BitFields, whereas `clang-format.yml` wants spaces on both sides. Change
`clang-format.yml` to match checkpatch, to better align with upstream
QEMU.

Applies the formatting changes to all existing files.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Some info pages that hold secrets (e.g. the creator and owner seeds for
key derivation by the keymgr, the isolated partition for provisioning,
etc.) are locked by an additional layer of protections on top of the
existing info page configuration registers, which depend on signals
broadcast by the lifecycle controller.

Since the lifecycle controller is not connected, these signals are
stubbed for now, but the logic for calculating the different permissions
on these pages and merging them with the existing sw register config is
all implemented to allow correct info page protection for software
operations.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
As with data pages, hardware requests bypass the sw-register-configured
protections and instead have a set of hardware protection rules applied
to them. The list of rules is searched for any matches (based on both
the specific info page and the current flash lcmgr phase), allowing
hardware only selective access to specific info pages containing secrets
during certain life cycle phases.

For these regions, the ECC and scramble properties can be optionally
disabled via the HW_INFO_CFG_OVERRIDE register to aid in recovery. This
is implemented as well. There are also OTP secrets that configure the
same properties, but these are left unimplemented for now.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This commit gates controller operations on initialization. It allows
writes to the `CONTROL` register, but the actual operation does not
start until the flash controller is initialized.

By default, the flash controller arbiter does not allow either SW or HW
requests. When the `INIT` command is sent and the flash physical
controller (around the flash macro) is initialized, the arbiter then
immediately switches to prioritising hardware to service the initial
hardware read requests to read the keymgr seeds out of the flash, and
then waits for software requests. Since reading the keymgr seeds is the
last part of the flash controller lcmgr initialization, this means that
when the flash controller is initialized, we can process a software op.

Specifically, if the `CONTROL` register was already written with
`start=true` before/while initializing, then this will cause the
operation to begin *as soon as initialization is finished* (and the
flash arbiter switches to prioritising SW requests over HW). Thus,
we must also begin an operation in the case that `CONTROL.START=true`
at the point in time where flash controller initialization finishes.

As well as moving the `CONTROL` processing logic into its own function,
this commit refactors the code slightly.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
As part of the logic for synchronizing the shared use of the program
FIFO between hardware and software flash operations (in addition to the
existing locking of SW writes to the PROG_FIFO), we always clear the
program FIFO at the start of a hardware operation, and clear any data
left over in the program FIFO at the end of a SW operation, regardless
of which operation it is.

To be sure that we only clear the program FIFO once when starting a HW
operation, create a new `ot_flash_op_start` function for starting an
operation, whereas `ot_flash_op_execute` can instead be used to continue
executing a started operation.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The `boot_data_functest` is now passing consistently with these
new flash changes - likely the additional SW qualification changes
are sufficient to meet the requirements of this test, even if not fully
modelled / connected with the lc_ctrl.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
@AlexJones0 AlexJones0 force-pushed the eg-flash-hw-operations branch from 5b682f6 to 1126484 Compare September 9, 2025 11:16
@AlexJones0
Copy link
Author

I've opened up a draft PR #178 that shows the changes to the flash_ctrl that will use the functionality introduced in this PR (keymgr secrets, lc_ctrl connections, a couple of other fixes).

The changes in this PR alone are sufficient to get //sw/device/silicon_creator/lib:boot_data_functest_sim_qemu_rom_with_fake_keys passing, and with the LC connections on top we also see //sw/device/tests:flash_ctrl_info_access_lc_dev_sim_qemu_rom_with_fake_keys and //sw/device/tests:flash_ctrl_info_access_lc_test_unlocked0_sim_qemu_rom_with_fake_keys passing as well.

@jwnrt jwnrt merged commit b3aa7b5 into lowRISC:ot-earlgrey-9.2.0 Sep 9, 2025
14 of 16 checks passed
@luismarques
Copy link

Progresses lowRISC/opentitan#28123

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants