Skip to content

Comments

[alert,dv] Fix timing for ping timeouts in alert handler test#29332

Merged
rswarbrick merged 1 commit intolowRISC:masterfrom
rswarbrick:ping-timeouts
Feb 18, 2026
Merged

[alert,dv] Fix timing for ping timeouts in alert handler test#29332
rswarbrick merged 1 commit intolowRISC:masterfrom
rswarbrick:ping-timeouts

Conversation

@rswarbrick
Copy link
Contributor

This was broken (I think) by the change in c254e65, which changed the monitor to see the ping signal change through a clocking block. Alert handler tests work by modelling the ping timeout in the alert monitor (??!!!) and this ended up a cycle late.

This was broken (I think) by the change in c254e65, which changed
the monitor to see the ping signal change through a clocking block.
Alert handler tests work by modelling the ping timeout in the alert
monitor (??!!!) and this ended up a cycle late.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
@rswarbrick rswarbrick requested a review from vogelpi February 17, 2026 19:16
@rswarbrick rswarbrick requested a review from a team as a code owner February 17, 2026 19:16
@rswarbrick rswarbrick requested review from eshapira and removed request for a team February 17, 2026 19:16
@rswarbrick rswarbrick added Component:DV DV issue: testbench, test case, etc. IP:alert_handler labels Feb 17, 2026
Copy link
Contributor

@vogelpi vogelpi left a comment

Choose a reason for hiding this comment

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

Thanks @rswarbrick , this is a big improvement already. I increases the pass rate of the alert_handler_ping_timeout test from 0% to 50% (at least when I tested it). The remaining failures I see are:

  • Either because the crashdump checking is still slighlty too early (increasing the wait_n_clks(2) to 3 on Line 571 can help, this is because this function checks the mirrored loc_alert_cause register, the prediction is updated based on the alert event sent to the scoreboard by the monitor, see process_alert_sig() task),
  • or because the intr_state register doesn't match the predicted value, it's also updated by the process_alert_sig() task .

So I think this solution is not perfect but it's a great step in the right direction and we can maybe do a follow up at some point to fix this for good?

@rswarbrick rswarbrick added this pull request to the merge queue Feb 18, 2026
Merged via the queue into lowRISC:master with commit 230a0e3 Feb 18, 2026
50 checks passed
@rswarbrick rswarbrick deleted the ping-timeouts branch February 18, 2026 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component:DV DV issue: testbench, test case, etc. IP:alert_handler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants