Skip to content
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

[rom_ctrl,dv] Pass the .prior argument for some mirror calls #25839

Merged
merged 1 commit into from
Jan 24, 2025

Conversation

rswarbrick
Copy link
Contributor

Confusingly, the default priority (-1) seems to mostly work. Unfortunately, this doesn't work properly with the rom_ctrl_stress_all_with_rand_reset sequence for some seeds for particular iterations of the stressed sequence.

In this situation, we end up with an item from a call to mirror() from the end of one iteration racing against some other register item from the start of the next one. Because there is more than once item, the sequencer tries to decide which one to grant... and explodes because they have a negative priority.

The error message is rather confusing:

UVM_FATAL @ 2122662017 ps: uvm_test_top.env.m_tl_agent_rom_ctrl_regs_reg_block.sequencer [SEQDEFPRI] Sequence m_tl_host_base_seq has illegal priority: -1

and the point is that if a sequence item has a priority of -1, this means "look at the priority of the sequence as a fall-back". This priority happens to be -1 (because the parent_sequence object didn't have a priority in the first place, and its do_copy function didn't copy that across either).

But it turns out that the correct fix has nothing to do with the sequence itself. That took... a LONG time to debug.

Confusingly, the default priority (-1) seems to mostly work.
Unfortunately, this *doesn't* work properly with the
rom_ctrl_stress_all_with_rand_reset sequence for some seeds for
particular iterations of the stressed sequence.

In this situation, we end up with an item from a call to mirror() from
the end of one iteration racing against some other register item from
the start of the next one. Because there is more than once item, the
sequencer tries to decide which one to grant... and explodes because
they have a negative priority.

The error message is rather confusing:

    UVM_FATAL @ 2122662017 ps: uvm_test_top.env.m_tl_agent_rom_ctrl_regs_reg_block.sequencer [SEQDEFPRI] Sequence m_tl_host_base_seq has illegal priority: -1

and the point is that if a sequence item has a priority of -1, this
means "look at the priority of the *sequence* as a fall-back". This
priority happens to be -1 (because the parent_sequence object didn't
have a priority in the first place, and its do_copy function didn't
copy that across either).

But it turns out that the correct fix has nothing to do with the
sequence itself. That took... a LONG time to debug.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
@rswarbrick rswarbrick added Component:DV DV issue: testbench, test case, etc. IP:rom_ctrl labels Jan 9, 2025
@rswarbrick rswarbrick requested a review from a team as a code owner January 9, 2025 19:57
@rswarbrick rswarbrick requested review from eshapira and hcallahan-lowrisc and removed request for a team January 9, 2025 19:57
@rswarbrick rswarbrick requested a review from alees24 January 23, 2025 19:32
Copy link
Contributor

@alees24 alees24 left a comment

Choose a reason for hiding this comment

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

Thanks for the explanation.

@rswarbrick rswarbrick merged commit 002bb67 into lowRISC:master Jan 24, 2025
42 checks passed
@rswarbrick rswarbrick deleted the rom-ctrl-priority-argument branch January 24, 2025 15:49
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:rom_ctrl
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants