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.

Sign up for GitHub

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

Jump to bottom

[dv] Add riscv_rf_ctrl_intg_test #2182

Merged
merged 1 commit into from
Jul 3, 2024
Merged

[dv] Add riscv_rf_ctrl_intg_test #2182

merged 1 commit into from
Jul 3, 2024

Conversation

GregAC
Copy link
Collaborator

@GregAC GregAC commented Jul 1, 2024

This tests new hardening added to the register file around read and write control signals.

@GregAC GregAC requested review from rswarbrick and nasahlpa July 1, 2024 13:18
@GregAC GregAC mentioned this pull request Jul 1, 2024
Closed
rswarbrick
rswarbrick approved these changes Jul 1, 2024
View reviewed changes
Copy link
Contributor

@rswarbrick rswarbrick left a comment

Choose a reason for hiding this comment

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

A couple of nitty style comments, but this looks good. I particularly like the "early stop" at the bottom: I hadn't seen that trick before.

dv/uvm/core_ibex/tests/core_ibex_test_lib.sv Outdated
`DV_CHECK_STD_RANDOMIZE_WITH_FATAL(ctrl_signal_idx, ctrl_signal_idx < ctrl_signals.size();)
`DV_CHECK_STD_RANDOMIZE_WITH_FATAL(rnd_delay, rnd_delay > 1000; rnd_delay < 10_000;)

ctrl_signal = ctrl_signals[ctrl_signal_idx];
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: I'd probably inline this into the computation of glitch_path and drop a variable.

dv/uvm/core_ibex/tests/core_ibex_test_lib.sv Outdated
// Check that the alert matches our expectation.
alert_major_internal_path = $sformatf("%s.alert_major_internal_o", top_path);
`DV_CHECK_FATAL(uvm_hdl_read(alert_major_internal_path, alert_major_internal))
`DV_CHECK_EQ_FATAL(alert_major_internal, 1'b1, "Major alert did not fire!")
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For this sort of thing, I'd probably use something like DV_CHECK_FATAL(alert_major_internal, "blah") because otherwise the reader needs to figure out how the first two arguments work. Honestly, I'm rather unconvinced about DV_CHECK_EQ in general, but the default print message is slightly useful, espeically if the actual and expected values are variable and/or multi-bit.

nasahlpa
nasahlpa approved these changes Jul 1, 2024
View reviewed changes
Copy link
Member

@nasahlpa nasahlpa left a comment

Choose a reason for hiding this comment

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

Thanks Greg for working on this, LGTM.

Will use this as a base when working on issue #2173.

@GregAC GregAC mentioned this pull request Jul 2, 2024
Closed
3 tasks
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
8ea000f 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
@nasahlpa nasahlpa mentioned this pull request Jul 2, 2024
Merged
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
cf6f8b9 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
3eecbe0 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
4b7c5aa 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
bb81156 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 2, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
29c0fae 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
@GregAC GregAC force-pushed the riscv_rf_ctrl_intg_test branch from 2a26740 to e4b389f Compare July 3, 2024 08:44
@GregAC
[dv] Add riscv_rf_ctrl_intg_test
71eea96 
This tests new hardening added to the register file around read and
write control signals.
@GregAC GregAC force-pushed the riscv_rf_ctrl_intg_test branch from e4b389f to 71eea96 Compare July 3, 2024 09:39
@GregAC GregAC added this pull request to the merge queue Jul 3, 2024
Merged via the queue into lowRISC:master with commit 2c13211 Jul 3, 2024
11 checks passed
nasahlpa added a commit to nasahlpa/ibex that referenced this pull request Jul 4, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
8feab56 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on lowRISC#2182 and closes lowRISC#2173.

Signed-off-by: Pascal Nasahl <[email protected]>
github-merge-queue bot pushed a commit that referenced this pull request Jul 4, 2024
@nasahlpa
[dv] Add riscv_ram_intg_test
0e0f27a 
This test injects a fault into different MuBi encoded signals within
the prim_ram_1p_scr and prim_ram_1p_adv and checks whether a fatal
alert is triggered.

I have excluded the addr_match signal from FI as its encoding
is not directly checked. If the signal was a MuBi True, a
fault into it is treated by the mubi4_and_hi as a False.
If the signal was a MuBi False, a fault into it is treated
by the mubi4_and_hi also as a False. Hence, no address
collision occurs and the holding register is not returned.

This PR is based on #2182 and closes #2173.

Signed-off-by: Pascal Nasahl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nasahlpa nasahlpa nasahlpa approved these changes

@rswarbrick rswarbrick rswarbrick approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@GregAC @rswarbrick @nasahlpa