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

Aon timer v3 dv changes #25559

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
20 changes: 17 additions & 3 deletions hw/dv/sv/cip_lib/seq_lib/cip_base_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -420,6 +420,7 @@ class cip_base_vseq #(
import dv_utils_pkg::interrupt_t;
dv_base_reg intr_csrs[$];
dv_base_reg intr_test_csrs[$];
dv_base_reg intr_state[$]; // convenience handle to intr_state regs

foreach (all_csrs[i]) begin
string csr_name = all_csrs[i].get_name();
Expand All @@ -431,6 +432,8 @@ class cip_base_vseq #(
if (!uvm_re_match("intr_test*", csr_name)) begin
intr_test_csrs.push_back(get_interrupt_csr(csr_name));
end
if (!uvm_re_match("intr_state*", csr_name))
intr_state.push_back(get_interrupt_csr(csr_name));
end

num_times = num_times * intr_csrs.size();
Expand All @@ -444,13 +447,15 @@ class cip_base_vseq #(
foreach (intr_csrs[i]) begin
uvm_reg_data_t data = $urandom();
`uvm_info(`gfn, $sformatf("Write %s: 0x%0h", intr_csrs[i].`gfn, data), UVM_MEDIUM)
foreach(intr_state[i])
intr_state[i].wait_for_prediction();
csr_wr(.ptr(intr_csrs[i]), .value(data));
end

// Read all intr related csr and check interrupt pins
intr_csrs.shuffle();
foreach (intr_csrs[i]) begin
uvm_reg_data_t exp_val = `gmv(intr_csrs[i]);
uvm_reg_data_t exp_val;
uvm_reg_data_t act_val;

interrupt_t irq_ro_mask = '0;
Expand All @@ -461,7 +466,16 @@ class cip_base_vseq #(
irq_ro_mask = intr_csrs[i].get_ro_mask();
end

exp_val &= ~irq_ro_mask;
// There may be a current intr_state access which impedes the TB from updating intr_state
// mirrored value. TB blocks here to ensure the predictions kick in time.
// We need to ensure the prediction has kicked in before we read the intr_state
// Users may set 'reg_name.predicting_value' before calling 'reg.predict' method, and
// unsetting it after 'reg.predict' returns to track whether a given reg is being predicted
foreach(intr_state[i])
intr_state[i].wait_for_prediction();
// we may need to update the exp_value here, since there may be a delay from the
// update above to ' exp_val'until here before we call the csr_rd
exp_val = `gmv(intr_csrs[i]) & ~irq_ro_mask;

csr_rd(.ptr(intr_csrs[i]), .value(act_val));
act_val &= ~irq_ro_mask;
Expand All @@ -478,7 +492,7 @@ class cip_base_vseq #(
`DV_CHECK_CASE_EQ(exp_intr_pin, act_intr_pin)
end // if (!uvm_re_match
end // foreach (intr_csrs[i])
end // for (int trans = 1; ...
end
// Write 0 to intr_test to clean up status interrupts, otherwise, status interrupts may remain
// active. And writing any value to a status interrupt CSR (intr_state) can't clear its value.
foreach (intr_test_csrs[i]) begin
Expand Down
14 changes: 14 additions & 0 deletions hw/dv/sv/dv_base_reg/dv_base_reg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,10 @@ class dv_base_reg extends uvm_reg;
// through the 1st/2nd (or both) writes
semaphore atomic_en_shadow_wr;

// Can be set before predicting a value for the case the register 'is_busy==1'
bit predicting_value;
uvm_event value_predicted_ev = new(); // Can be triggered to know when a prediction's finalised

function new(string name = "",
int unsigned n_bits,
int has_coverage);
Expand Down Expand Up @@ -479,4 +483,14 @@ class dv_base_reg extends uvm_reg;
return retval;
endfunction

// Blocks until a value is predicted assuming the flag 'predicting_value' was set prior to
// calling '.predict()' method
task wait_for_prediction();
if (predicting_value ) begin
value_predicted_ev.wait_ptrigger();
`uvm_info(`gfn, $sformatf("Reg_name: %0s, value=0x%0x; value_predicted_ev event triggered",
get_name(), get_mirrored_value()), UVM_DEBUG)
end
endtask

endclass
2 changes: 2 additions & 0 deletions hw/ip/aon_timer/dv/env/aon_timer_env.core
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,13 @@ filesets:
depend:
- lowrisc:dv:ralgen
- lowrisc:dv:cip_lib
- lowrisc:dv:usbdev_env
files:
- aon_timer_env_pkg.sv
- aon_timer_env_cfg.sv: {is_include_file: true}
- aon_timer_env_cov.sv: {is_include_file: true}
- aon_timer_virtual_sequencer.sv: {is_include_file: true}
- aon_timer_intr_timed_regs.sv: {is_include_file: true}
- aon_timer_scoreboard.sv: {is_include_file: true}
- aon_timer_env.sv: {is_include_file: true}
- seq_lib/aon_timer_vseq_list.sv: {is_include_file: true}
Expand Down
3 changes: 2 additions & 1 deletion hw/ip/aon_timer/dv/env/aon_timer_env_pkg.sv
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ package aon_timer_env_pkg;
import dv_base_reg_pkg::*;
import csr_utils_pkg::*;
import aon_timer_ral_pkg::*;

import usbdev_env_pkg::*;
// macro includes
`include "uvm_macros.svh"
`include "dv_macros.svh"
Expand All @@ -31,6 +31,7 @@ package aon_timer_env_pkg;
`include "aon_timer_env_cfg.sv"
`include "aon_timer_env_cov.sv"
`include "aon_timer_virtual_sequencer.sv"
`include "aon_timer_intr_timed_regs.sv"
`include "aon_timer_scoreboard.sv"
`include "aon_timer_env.sv"
`include "aon_timer_vseq_list.sv"
Expand Down
96 changes: 96 additions & 0 deletions hw/ip/aon_timer/dv/env/aon_timer_intr_timed_regs.sv
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
// Copyright lowRISC contributors (OpenTitan project).
// Licensed under the Apache License, Version 2.0, see LICENSE for details.
// SPDX-License-Identifier: Apache-2.0

// `Timed registers` cannot be guaranteed to change at an exact time, so a certain amount of
// variability must be expected in the timing.
class aon_timer_intr_timed_regs extends uvm_object;
`uvm_object_utils(aon_timer_intr_timed_regs)

extern function new (string name="");

typedef enum {
TimedIntrStateWkupExpired = 0,
TimedIntrStateWdogBark
} timed_reg_e;

// Access to DUT clock.
virtual clk_rst_if clk_rst_vif;
// Access to DUT registers.
aon_timer_reg_block ral;

timed_reg timed[timed_reg_e];

// Monotonic, wrapping cycle count; used to detect when expectations have not been met.
antmarzam marked this conversation as resolved.
Show resolved Hide resolved
// Incremented in 'check_predictions' task
int time_now = 0;

// Perform a read of the actual DUT register state, for checking against expectations.
// Note: we perform a backdoor read to avoid impacting the timing of the DUT and DV.
extern task read_act_data(timed_reg_e r, output uvm_reg_data_t act_data);
// Add a timed, predicted state change to the list of expectations for the given register.
extern function void predict(timed_reg_e r, uvm_reg_data_t prev_data, uvm_reg_data_t new_data);
// Check a DUT read from the specified register against any timed expectations.
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this comment: the function isn't checking anything, but returns the predicted value, I think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All of the timed_regs code reuses the code from Adrian adapting it to the aon_timer.
You can see the usb timed regs here with the same comment:

// Check a DUT read from the specified register against any timed expectations.

The read function does check and does not return anything (void), so I don't think the comment is misleading. See how this function calls the specific timed register read function which does the checking:

function uvm_reg_data_t read(int unsigned time_now, uvm_reg_data_t act_data);
// Properties of this register field.
uvm_reg_data_t f_mask = (1 << field.get_n_bits()) - 1;
int unsigned lsb = field.get_lsb_pos();
// Predicted value of this register field.
uvm_reg_data_t pred_val;
`uvm_info(`gfn, $sformatf(" - field %s : pred valid %d prev 0x%0x new 0x%0x",
field.get_name(), pred_valid, pred_latest.val_prev,
pred_latest.val_new), UVM_HIGH)
if (pred_valid) begin
uvm_reg_data_t act_val = (act_data >> lsb) & f_mask;
`uvm_info(`gfn, $sformatf(" (time_now 0x%0x latest_time 0x%0x)", pred_latest.latest_time,
time_now), UVM_HIGH)
if (act_val == pred_latest.val_new) begin
`uvm_info(`gfn, " Prediction met", UVM_HIGH)
pred_val = pred_latest.val_new;
// Prediction met; no longer required.
pred_valid = 1'b0;
end else begin
`uvm_info(`gfn, $sformatf(" Prediction not met; act 0x%0x", act_val), UVM_MEDIUM)
`DV_CHECK_EQ(act_val, pred_latest.val_prev)
`DV_CHECK_GT(pred_latest.latest_time - time_now, 0)
pred_val = pred_latest.val_prev;
end
// Retain the latest prediction.
read_latest = pred_val;
end else begin

I think you authored Adrian's PR which is the same :/

Copy link
Contributor

Choose a reason for hiding this comment

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

I just chatted to Adrian about this! (because I realised I didn't quite understand). And now I understand a bit more. And I'm convinced that the comment is correct (although the code is maybe a bit more confusing than it needs to be: not sure)

@alees24: Thanks for your help!

extern function uvm_reg_data_t read(timed_reg_e r, uvm_reg_data_t act_data);
// This process runs forever checking every prediction that is made, using backdoor csr_rd in zero
// time to avoid interfering with actual CSR reads and the timing of the simulation.
// It also resets the registers if a reset is seen.
extern task check_predictions(ref bit under_reset);

endclass : aon_timer_intr_timed_regs

function aon_timer_intr_timed_regs::new (string name="");
super.new(name);
endfunction : new

task aon_timer_intr_timed_regs::read_act_data(timed_reg_e r, output uvm_reg_data_t act_data);
case (r)
TimedIntrStateWkupExpired : begin
csr_rd(.ptr(ral.intr_state.wkup_timer_expired), .value(act_data), .backdoor(1));
end
TimedIntrStateWdogBark : begin
csr_rd(.ptr(ral.intr_state.wdog_timer_bark), .value(act_data), .backdoor(1));
end
default: `uvm_fatal(`gfn, "Invalid/unrecognized register")
endcase
`uvm_info(`gfn, $sformatf("Backdoor read of reg %p yielded 0x%0x", r, act_data), UVM_HIGH)
endtask : read_act_data

function void aon_timer_intr_timed_regs::predict(timed_reg_e r, uvm_reg_data_t prev_data,
uvm_reg_data_t new_data);
`uvm_info(`gfn, $sformatf("Expecting reg %p <= 0x%0x, from 0x%0x (mask 0x%0x), time_now %0d",
r, new_data, prev_data, prev_data ^ new_data, time_now), UVM_MEDIUM)
antmarzam marked this conversation as resolved.
Show resolved Hide resolved
timed[r].predict(time_now, new_data, prev_data);
endfunction : predict

function uvm_reg_data_t aon_timer_intr_timed_regs::read(timed_reg_e r, uvm_reg_data_t act_data);
`uvm_info(`gfn, $sformatf("Producing prediction for %p, act_data 0x%0x", r, act_data),
UVM_MEDIUM)
return timed[r].read(time_now, act_data);
endfunction : read

task aon_timer_intr_timed_regs::check_predictions(ref bit under_reset);
antmarzam marked this conversation as resolved.
Show resolved Hide resolved
// Collect the initial values post-reset.
wait (clk_rst_vif.rst_n === 1'b1);
forever begin
timed_reg_e r;
// Check on every negedge to avoid races with CSR changes
@(negedge clk_rst_vif.clk);
time_now++;
// Check each of the timed registers for expired expectations.
r = r.first();
for (int i=0; i < r.num(); i++) begin
rswarbrick marked this conversation as resolved.
Show resolved Hide resolved
if (under_reset) begin
`uvm_info(`gfn, $sformatf("Resetting timed register predictions at 0x%0x", time_now),
UVM_MEDIUM)
timed[r].reset();
end else if (timed[r].preds_pending()) begin
uvm_reg_data_t act_data;
// Something is expected to happen to this register field.
read_act_data(r, act_data);
timed[r].check_pred(time_now, $sformatf("%p", r), act_data);
end
r = r.next();
end // for (int i=0; i < r.num(); i++)
end
endtask : check_predictions
Loading
Loading