[otbn,dv] Add WFI testpoint and tests by etterli · Pull Request #30918 · lowRISC/opentitan

etterli · 2026-08-04T14:13:40Z

This PR is based on #30917 and can only be merged afterwards (thus a draft PR). Only the last 4 commits are relevant.

This PR adds a testpoint for the WFI feature (or here) as well as the required tests.

Please note that I have limited DV experience. Feel free to discard the current vseqs. These serve as starting points.

rswarbrick · 2026-08-04T15:13:21Z

+   * After resuming, load the word at DMEM address 0. This tests that the ISS observes the same new
+   * value as the RTL (if the host wrote to DMEM).


I'd probably switch the order to "RTL ... the same as the ISS", which sounds a bit more like the ISS is defined to be correct :-)

This test is now obsolete as I switched the smoke test to use the already existing top level test.

rswarbrick · 2026-08-04T15:19:48Z

+            `DV_CHECK_FATAL(uvm_hdl_read(status_path, rtl_status),
+                            $sformatf("Failed to read STATUS from `%0s'", status_path))


I'm not a big fan of this! Can't we wait by repeatedly reading the STATUS register with no backdoors?

True, this is ugly. I've replaced it with a simpler STATUS register polling.

rswarbrick · 2026-08-04T15:20:30Z

+    // Stay paused for a random number of cycles.
+    resume_delay = $urandom_range(max_resume_delay, min_resume_delay);
+    `uvm_info(`gfn, $sformatf("Paused, waiting %0d cycles before RESUME", resume_delay), UVM_LOW)
+    cfg.clk_rst_vif.wait_clks(resume_delay);


Why add an extra wait? If there's a good reason, it will need describing carefully here and also in the testpoint. If not, I suggest dropping the unnecessary code.

Hm, I added this just to not always read on the same cycle. But its meaningless. The only really good edge case would be to read in the very first pause cycle. But this code does not do this anyway. I removed it.

rswarbrick · 2026-08-04T15:23:09Z

+
+    // Wait until the program has ended.
+    `DV_WAIT(cfg.model_agent_cfg.vif.status == otbn_pkg::StatusIdle)
+    `uvm_info(`gfn, "OTBN finished after RESUME", UVM_LOW)


Don't we need the OTBN application to finish the handshake? I would suggest it does something like copying the value that we wrote and putting it at address 0x4. Then the virtual sequence can load that word and check it's the value that we wrote in the first place.

That is a good point. This smoke test here was my very first test. But in the meantime I wrote a top level test which is more evolved. I have now changed the smoke vseq to use this test. The test can be found at sw/otbn/wfi/wfi_test.s.

etterli

Thanks @rswarbrick for the good inputs. The smoke tests now uses the top level test which checks more stuff.

etterli · 2026-08-04T17:38:20Z

+   * After resuming, load the word at DMEM address 0. This tests that the ISS observes the same new
+   * value as the RTL (if the host wrote to DMEM).


This test is now obsolete as I switched the smoke test to use the already existing top level test.

etterli · 2026-08-04T17:38:46Z

+            `DV_CHECK_FATAL(uvm_hdl_read(status_path, rtl_status),
+                            $sformatf("Failed to read STATUS from `%0s'", status_path))


True, this is ugly. I've replaced it with a simpler STATUS register polling.

etterli · 2026-08-04T17:39:38Z

+
+    // Wait until the program has ended.
+    `DV_WAIT(cfg.model_agent_cfg.vif.status == otbn_pkg::StatusIdle)
+    `uvm_info(`gfn, "OTBN finished after RESUME", UVM_LOW)


That is a good point. This smoke test here was my very first test. But in the meantime I wrote a top level test which is more evolved. I have now changed the smoke vseq to use this test. The test can be found at sw/otbn/wfi/wfi_test.s.

etterli · 2026-08-04T17:41:11Z

+    // Stay paused for a random number of cycles.
+    resume_delay = $urandom_range(max_resume_delay, min_resume_delay);
+    `uvm_info(`gfn, $sformatf("Paused, waiting %0d cycles before RESUME", resume_delay), UVM_LOW)
+    cfg.clk_rst_vif.wait_clks(resume_delay);


Hm, I added this just to not always read on the same cycle. But its meaningless. The only really good edge case would be to read in the very first pause cycle. But this code does not do this anyway. I removed it.

etterli · 2026-08-05T06:50:30Z

Moved the WFI test programs to the sw/otbn/wfi folder. Now all test software is at the same location.

Adds new testpoints to test the WaitForInterrupt feature. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

This adds a simple sequence which runs an OTBN program with a WFI instruction. It then lets OTBN resume and checks that it did resume properly. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

This adds a directed test to check that an escalation is properly handled whilst OTBN is paused by a WFI instruction. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

etterli · 2026-08-10T12:21:25Z

Rebased on merged #30917. This is now ready.

etterli requested a review from a team as a code owner August 4, 2026 14:13

etterli requested review from rswarbrick and removed request for a team August 4, 2026 14:13

etterli marked this pull request as draft August 4, 2026 14:14

etterli requested review from nasahlpa, victornwaliLowRISC and vogelpi August 4, 2026 14:14

etterli force-pushed the otbn-wfi-testing branch from 3566ffb to 7921cbb Compare August 4, 2026 14:21

rswarbrick reviewed Aug 4, 2026

View reviewed changes

etterli force-pushed the otbn-wfi-testing branch from 7921cbb to 9545a6f Compare August 4, 2026 19:18

etterli commented Aug 4, 2026

View reviewed changes

etterli force-pushed the otbn-wfi-testing branch from 9545a6f to 8b9506d Compare August 5, 2026 06:50

etterli added 4 commits August 10, 2026 14:20

[otbn,dv] Extend OTBN's testplan for WFI feature

9816f61

Adds new testpoints to test the WaitForInterrupt feature. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

[otbn,dv] Add smoke test for WFI feature

1301f5d

This adds a simple sequence which runs an OTBN program with a WFI instruction. It then lets OTBN resume and checks that it did resume properly. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

[otbn,dv] Add test to check escalation during WFI pause

bda5759

This adds a directed test to check that an escalation is properly handled whilst OTBN is paused by a WFI instruction. Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

[otbn,dv] Add test to check multiple back-to-back WFI instructions

0156513

Signed-off-by: Pascal Etterli <pascal.etterli@lowrisc.org>

etterli force-pushed the otbn-wfi-testing branch from 8b9506d to 0156513 Compare August 10, 2026 12:21

etterli marked this pull request as ready for review August 10, 2026 12:21

etterli requested a review from rswarbrick August 10, 2026 12:22

		* After resuming, load the word at DMEM address 0. This tests that the ISS observes the same new
		* value as the RTL (if the host wrote to DMEM).

		`DV_CHECK_FATAL(uvm_hdl_read(status_path, rtl_status),
		$sformatf("Failed to read STATUS from `%0s'", status_path))

Uh oh!

Conversation

etterli commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etterli left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etterli commented Aug 5, 2026

Uh oh!

etterli commented Aug 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

etterli commented Aug 4, 2026 •

edited

Loading