From 16b5876ef60c77f53a8d41119597d653bed0f294 Mon Sep 17 00:00:00 2001 From: Pascal Nasahl Date: Thu, 14 Nov 2024 14:50:31 +0000 Subject: [PATCH] [sram_ctrl,dv] Adapt throughput tests for readback feature When the readback feature is enabled, reads and writes take longer. This commit adapts the cycle prediction to account for that delay. Closes lowRISC/opentitan#23321 Signed-off-by: Pascal Nasahl --- hw/ip/sram_ctrl/data/sram_ctrl_testplan.hjson | 4 +- .../dv/env/seq_lib/sram_ctrl_base_vseq.sv | 14 ++++--- .../env/seq_lib/sram_ctrl_throughput_vseq.sv | 37 +++++++++++++++++-- .../sram_ctrl/dv/sram_ctrl_base_sim_cfg.hjson | 5 +++ 4 files changed, 51 insertions(+), 9 deletions(-) diff --git a/hw/ip/sram_ctrl/data/sram_ctrl_testplan.hjson b/hw/ip/sram_ctrl/data/sram_ctrl_testplan.hjson index e7d7f946206e6..0782c095e2a40 100644 --- a/hw/ip/sram_ctrl/data/sram_ctrl_testplan.hjson +++ b/hw/ip/sram_ctrl/data/sram_ctrl_testplan.hjson @@ -140,9 +140,11 @@ Without partial write, if driver doesn't introduce any delay, it takes N+1 cycles to finish N SRAM read/write accesses. With partial write, it needs 2 extra cycles per partial write. + With readback enabled, each read needs 2 cycles and each write 3 cycles. ''' stage: V2 - tests: ["{name}_max_throughput", "{name}_throughput_w_partial_write"] + tests: ["{name}_max_throughput", "{name}_throughput_w_partial_write", + "{name}_throughput_w_readback"] } { name: regwen diff --git a/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_base_vseq.sv b/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_base_vseq.sv index eab7b80f85b06..c141274a34afb 100644 --- a/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_base_vseq.sv +++ b/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_base_vseq.sv @@ -26,6 +26,10 @@ class sram_ctrl_base_vseq #( int partial_access_pct = 10; + // Used by the throughput_w_readback test. When 1'b1, the readback feature is + // enabled at the beginning of the test. + bit init_w_readback = 1'b0; + constraint readback_en_c { soft readback_en inside {MuBi4True, MuBi4False}; } @@ -34,6 +38,8 @@ class sram_ctrl_base_vseq #( super.pre_start(); void'($value$plusargs("partial_access_pct=%0d", partial_access_pct)); `DV_CHECK_LE(partial_access_pct, 100) + void'($value$plusargs("init_w_readback=%0d", init_w_readback)); + `DV_CHECK_LE(init_w_readback, 1) // Wait for ram initialization to be done, since it blocks memory accesses. `DV_WAIT(cfg.in_init == 1'b0, "Timed out waiting for initialization done") // Make sure that the sram_readback_en task only runs once during the test. @@ -51,11 +57,9 @@ class sram_ctrl_base_vseq #( if (!readback_running && do_readback_en) sram_readback_en(); endtask - // Enable readback feature only for non-throughput and non-sec_cm tests. The - // readback feature is randomly initialized to on/off at the start of the test - // and randomly switched during the tests. - // TODO(#23321) Adapt the troughput tests to take the delay caused by the - // readback feature into account. + // Randomly disable or enable the readback feature during a test run. + // If a subclass wants to manually enable the readback feature, it should + // directly write to the readback enable register. virtual protected task sram_readback_en(); readback_running = 1; if (uvm_re_match("*throughput*", get_type_name()) && diff --git a/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_throughput_vseq.sv b/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_throughput_vseq.sv index b2724e0441a03..b946357f4e8aa 100644 --- a/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_throughput_vseq.sv +++ b/hw/ip/sram_ctrl/dv/env/seq_lib/sram_ctrl_throughput_vseq.sv @@ -12,11 +12,22 @@ class sram_ctrl_throughput_vseq extends sram_ctrl_smoke_vseq; int num_partial_write; + // Track the number of writes and reads and whether the most recent transaction + // was a write. This is updated in get_rand_mask. + int num_writes; + int num_reads; + int last_was_write; + task body(); // In order to have max throughput, don't drop a_valid cfg.m_tl_agent_cfgs[cfg.sram_ral_name].allow_a_valid_drop_wo_a_ready = 0; req_mem_init(); + if (init_w_readback) begin + // Init the sequence with the SRAM readback feature enabled. + csr_wr(.ptr(ral.readback), .value(MuBi4True)); + end + // And wait for side_effects of ram_init to be done, since detection of the end is not very // accurate and memory transactions wll be blocked until init is completely done. The number // 20 below is not special, it is just a sensible guess. @@ -27,6 +38,10 @@ class sram_ctrl_throughput_vseq extends sram_ctrl_smoke_vseq; num_partial_write = 0; + num_writes = 0; + num_reads = 0; + last_was_write = 0; + `DV_CHECK_MEMBER_RANDOMIZE_FATAL(num_ops) `uvm_info(`gfn, $sformatf("iteration: %0d, issuing %0d ops", i, num_ops), UVM_LOW) @@ -42,16 +57,32 @@ class sram_ctrl_throughput_vseq extends sram_ctrl_smoke_vseq; `uvm_info(`gfn, $sformatf("num_cycles: %0d, num_ops: %0d, num_partial_write: %0d", num_cycles, num_ops, num_partial_write), UVM_MEDIUM) - - `DV_CHECK_EQ(num_cycles, num_ops + 1 + num_partial_write * 2); + if (init_w_readback) begin + // In throughput_w_readback test, if the last operation was a write, subtract + // one as the write already gets acknowledged over TLUL while the readback error + // is still doing the readback of the written value. + `DV_CHECK_EQ(num_cycles, num_writes * 3 + num_reads * 2 - last_was_write); + end else begin + `DV_CHECK_EQ(num_cycles, num_ops + 1 + num_partial_write * 2); + end end endtask : body - // override the function to count how many partial writes are sent + // override the function to count how many reads, writes, and partial writes are sent virtual function bit[bus_params_pkg::BUS_DBW-1:0] get_rand_mask(bit write); bit [bus_params_pkg::BUS_DBW-1:0] mask = super.get_rand_mask(write); if (write && mask != '1) num_partial_write++; + + // Keep track of number of writes and reads for readback test. + if (write) begin + num_writes++; + last_was_write = 1; + end else begin + num_reads++; + last_was_write = 0; + end + return mask; endfunction diff --git a/hw/ip/sram_ctrl/dv/sram_ctrl_base_sim_cfg.hjson b/hw/ip/sram_ctrl/dv/sram_ctrl_base_sim_cfg.hjson index 9070afaaef1da..b2bc6b7ff6b08 100644 --- a/hw/ip/sram_ctrl/dv/sram_ctrl_base_sim_cfg.hjson +++ b/hw/ip/sram_ctrl/dv/sram_ctrl_base_sim_cfg.hjson @@ -108,6 +108,11 @@ uvm_test_seq: sram_ctrl_throughput_vseq run_opts: ["+zero_delays=1 +partial_access_pct=20"] } + { + name: "{name}_throughput_w_readback" + uvm_test_seq: sram_ctrl_throughput_vseq + run_opts: ["+zero_delays=1 +partial_access_pct=0 +init_w_readback=1"] + } { name: "{name}_lc_escalation" uvm_test_seq: sram_ctrl_lc_escalation_vseq