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

[entropy_src/dv] Separate external health test (XHT) verification from V2 tests #17638

Merged
merged 4 commits into from
Mar 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions hw/ip/entropy_src/data/entropy_src_testplan.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,15 @@
stage: V2
tests: ["entropy_src_functional_errors"]
}
{
name: external_health_tests
desc: '''
Verify the external health test (XHT) interface and functionality, including continuous
and windowed functionality and different high/low thresholds and watermarks.
'''
stage: V3
tests: ["entropy_src_rng_with_xht_rsps"], // TODO(#16276): Complete XHT verification
}
]
covergroups: [
{
Expand Down
7 changes: 7 additions & 0 deletions hw/ip/entropy_src/dv/entropy_src_sim_cfg.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@
uvm_test_seq: entropy_src_rng_vseq
}

{
name: entropy_src_rng_with_xht_rsps
Copy link
Contributor Author

Choose a reason for hiding this comment

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

NB: This test has the default number of reseeds (50), whereas entropy_src_rng still has 300. The number of reseeds can be increased later depending on XHT coverage (#16276)

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good.

uvm_test: entropy_src_rng_test
uvm_test_seq: entropy_src_rng_vseq
run_opts: ["+xht_only_default_rsp=0"]
}

andreaskurth marked this conversation as resolved.
Show resolved Hide resolved
{
name: entropy_src_stress_all
uvm_test: entropy_src_stress_all_test
Expand Down
3 changes: 3 additions & 0 deletions hw/ip/entropy_src/dv/env/entropy_src_env_cfg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,9 @@ class entropy_src_env_cfg extends cip_base_env_cfg #(.RAL_T(entropy_src_reg_bloc
// alert within alert_max_delay clock cycles.
int alert_max_delay;

// Whether to keep the default response on the XHT interface at all time.
bit xht_only_default_rsp = 1;

///////////////////////
// Randomized fields //
///////////////////////
Expand Down
16 changes: 16 additions & 0 deletions hw/ip/entropy_src/dv/env/entropy_src_scoreboard.sv
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ class entropy_src_scoreboard extends cip_base_scoreboard#(
process_interrupts();
process_fifo_exceptions();
health_test_scoring_thread();
process_xht();
join_none
end
endtask
Expand Down Expand Up @@ -1107,6 +1108,21 @@ class entropy_src_scoreboard extends cip_base_scoreboard#(
end
endtask

task process_xht();
Copy link
Contributor

Choose a reason for hiding this comment

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

Great idea to add such a check!

Nit: should this task rather be named check_xht_response()?

Copy link
Contributor Author

@andreaskurth andreaskurth Mar 22, 2023

Choose a reason for hiding this comment

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

Yeah, at some point the scoreboard will have to take XHT transactions into account for its modeling and checking, as it already does for TL accesses, for example. By convention, these tasks seem to start with process_, e.g., process_tl_access is defined for our scoreboards derived from cip_base_scoreboard. So I think process_xht is appropriate.

Note that in a proper UVM design, the scoreboard would not sample I/Os directly but use TLM analysis FIFOs (again using the TL example, the scoreboard calls process_tl_fifo -> process_tl_a_item -> process_tl_access). I decided against this design for now for two main reasons:

  • It's overly complicated for the check we need right now.
  • As we want to ensure the DUT sees a fixed value in every cycle, directly sampling I/O is less error-prone than the indirection through a monitor and analysis FIFOs.

When implementing XHT tests, the checks in this PR can be complemented by or moved to TLM-based checks.

// Process XHT transactions.
forever begin
@(cfg.m_xht_agent_cfg.vif.mon_cb);
if (cfg.under_reset) continue;

if (cfg.xht_only_default_rsp) begin
// If the environment is configured to maintain the default XHT response at all time, ensure
// that this is really the case.
`DV_CHECK_EQ(cfg.m_xht_agent_cfg.vif.mon_cb.rsp,
entropy_src_pkg::ENTROPY_SRC_XHT_RSP_DEFAULT)
end
end
endtask

// All the HT threshold registers are one-way: they can only become more strict unless
// the DUT is reset. This function encapsulates this behavior.
//
Expand Down
4 changes: 3 additions & 1 deletion hw/ip/entropy_src/dv/env/seq_lib/entropy_src_rng_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,9 @@ class entropy_src_rng_vseq extends entropy_src_base_vseq;
fork
m_rng_push_seq.start(p_sequencer.rng_sequencer_h);
m_csrng_pull_seq.start(p_sequencer.csrng_sequencer_h);
m_xht_seq.start(p_sequencer.xht_sequencer);
if (!cfg.xht_only_default_rsp) begin
m_xht_seq.start(p_sequencer.xht_sequencer);
end
andreaskurth marked this conversation as resolved.
Show resolved Hide resolved
join_none
endtask

Expand Down
7 changes: 7 additions & 0 deletions hw/ip/entropy_src/dv/tests/entropy_src_base_test.sv
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,13 @@ class entropy_src_base_test extends cip_base_test #(
// Overrides should happen in the specific testcase.

virtual function void configure_env();
// Take plusargs into account.
bit xht_only_default_rsp;
if ($value$plusargs("xht_only_default_rsp=%0b", xht_only_default_rsp)) begin
`uvm_info(`gfn, $sformatf("+xht_only_default_rsp specified"), UVM_MEDIUM)
cfg.xht_only_default_rsp = xht_only_default_rsp;
end

// seed_cnt only used by smoke test
// so there is no need to randomize it.
cfg.seed_cnt = 1;
Expand Down