Skip to content

Conversation

AlexJones0
Copy link

This should fix failures in Earlgrey ROM_EXT regressions after lowRISC/opentitan@abac206 was (correctly) merged in upstream earlgrey_1.0.0. Also see discussion on the relevant RTL and SW drivers in lowRISC/opentitan#27683. This is a case of the SW being made more stringent and catching an issue in the QEMU implementation.

The keymgr should only change its OP_STATUS to idle upon reset, or if it is explicitly cleared by SW. The DONE_ERROR and DONE_SUCCESS operation status values need to be latched to be queried by software, but subsequent FSM ticks (e.g. scheduled with a timer) can then change the operation status back to idle.

In HW, this value is latched and only updated when op_start, i.e. when the operation is ongoing. When the operation is not ongoing, the value is thus not reset back to idle outside of SW writes/resets. See the relevant RTL.

This wasn't being caught before because SW was either reading fast enough / only in cases before any subsequent FSM ticks, and/or was just discarding the incorrect IDLE state.

The keymgr should only change its `OP_STATUS` to idle upon reset, or if
it is explicitly cleared by SW. The `DONE_ERROR` and `DONE_SUCCESS`
operation status values need to be latched to be queried by software,
but subsequent FSM ticks (e.g. scheduled by the BH) can then change the
operation status back to idle.

In HW, this value is latched and only updated when `op_start`, i.e.
when the operation is ongoing. When the operation is not ongoing, the
value is thus not reset back to idle outside of SW writes/resets.

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
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.

(LGTM, looking at the description vs. change)

@AlexJones0
Copy link
Author

I am expecting the //sw/device/tests:coverage_test_sim_qemu_rom_with_fake_keys Earlgrey target to fail on this PR because the test was removed from upstream Earlgrey, I will remove it in #203 instead however.

@AlexJones0 AlexJones0 merged commit 9a0f025 into lowRISC:ot-9.2.0 Oct 1, 2025
7 of 8 checks passed
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.

3 participants