Skip to content

Conversation

AlexJones0
Copy link

The fatal alert was connected for the different fatal faults, but the recoverable alert for recoverable errors (such as bad instruction address errors, invalid sideloaded key errors, illegal instructions, etc.) was not connected. This commit simply adds the logic for this alert.

@luismarques
Copy link

@AlexJones0 Do we have OT test coverage for this behaviour?

@AlexJones0
Copy link
Author

AlexJones0 commented Sep 3, 2025

@luismarques I think we might? The OTBN smoketest started failing after the recent alert catching changes were merged to OpenTitan earlgrey_1.0.0, so I would expect that test would probably pass again (I think we can wait for CI to see?)

Edit: yes, looks like there are two OT tests that are now passing with this change:

  //sw/device/tests:otbn_irq_test_sim_qemu_rom_with_fake_keys
  //sw/device/tests:otbn_smoketest_sim_qemu_rom_with_fake_keys

…bits

The Fatal alert was connected for the different fatal faults, but the
recoverable alert for recoverable errors such as bad instruction address
errors, invalid sideloaded key errors, illegal instructions, etc. was
not connected. This commit simply adds the logic for this alert.

Also take this opportunity to add a trace on OTBN alerts, and to modify
the implementation so that ALERT_TEST and recoverable alerts are
transient (i.e. immediately cleared backed to 0 after being sent once),
whereas fatal alerts are sticky (also fatal, as they can't be cleared).

Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
@AlexJones0
Copy link
Author

@rivos-eblot is this ok to merge for you?

@rivos-eblot
Copy link

@rivos-eblot is this ok to merge for you?

Sorry I missed your comment. Can you take care of the alert_test transient thing, for ex. ot_ibex_wrapper_write_alert_test? Thanks.

@AlexJones0
Copy link
Author

Sorry I missed your comment. Can you take care of the alert_test transient thing, for ex. ot_ibex_wrapper_write_alert_test? Thanks.

The alert_test transience should have already been added by the last force push - after replying to your comment I added it 😃

@rivos-eblot
Copy link

The alert_test transience should have already been added by the last force push - after replying to your comment I added it 😃

Ooops, I missed it. Ok then.

@rivos-eblot rivos-eblot self-requested a review September 10, 2025 12:24
@AlexJones0 AlexJones0 merged commit 5a8ee31 into lowRISC:ot-earlgrey-9.2.0 Sep 10, 2025
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.

4 participants