-
Notifications
You must be signed in to change notification settings - Fork 638
Fix incorrect privilege mode detection causing failed dret riscv-dv tests #2301
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Fix incorrect privilege mode detection causing failed dret riscv-dv tests #2301
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making these changes, just marking them as request changes because they are currently not correct and will affect other tests as well.
@@ -1125,7 +1125,7 @@ class core_ibex_directed_test extends core_ibex_debug_intr_basic_test; | |||
// we are blocking the current instruction until the instruction from WB stage is ready. | |||
wait (dut_vif.dut_cb.ctrl_fsm_cs == FLUSH); | |||
clk_vif.wait_clks(2); | |||
check_priv_mode(PRIV_LVL_M); | |||
check_priv_mode(init_operating_mode); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When you get an illegal instruction it must trap in M-mode regardless of what the initial operating mode is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think the time of checking was on trap exit. If the design here was to test the mode on trap entry, we'll probably have to discuss how to rewrite this module. The wait on line 1126 here can not capture the flush signal on trap entry. In this specific testcase(riscv_dret_test), the previous functions before the wait line have a handshake mechanism and blocks the wait line until handshake finished, so the first flush on trap entry signal will not be captured in this case. That's probably why I think this checking is wrong.
@@ -1657,7 +1657,7 @@ class core_ibex_dret_test extends core_ibex_directed_test; | |||
|
|||
virtual task check_stimulus(); | |||
forever begin | |||
wait (dut_vif.dut_cb.dret === 1'b1); | |||
@(negedge dut_vif.dut_cb.dret); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're changing what this test does now. This test sees whether there is a DRET instruction and makes sure it is detected as illegal. Have you configured your Ibex to support DRET?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I have the dret enabled, and it is only a legal instruction in debug mode.
I don't think I am changing the test here. Somehow, the wait line failed to capture here sometime, only negedge can correctly triggers all the time.
The testbench of the riscv-dv flow failed to compare the privilege mode of the DUT correctly and failing the riscv_dret_test. The testbench assumes the DUT will always be in machine mode while different random seeds can generate a testcase that would bring the DUT into either user mode or machine mode at the point of comparison.
Some seeds of the riscv_dret_test that leads to failure:
VCS: 1574
XLM: 28930
This commit fixed the comparison and make riscv_dret_test of the riscv-dv pass under either circumstances.
