diff --git a/dv/uvm/icache/dv/env/ibex_icache_scoreboard.sv b/dv/uvm/icache/dv/env/ibex_icache_scoreboard.sv index 23abdba2..735d2698 100644 --- a/dv/uvm/icache/dv/env/ibex_icache_scoreboard.sv +++ b/dv/uvm/icache/dv/env/ibex_icache_scoreboard.sv @@ -133,14 +133,21 @@ class ibex_icache_scoreboard mem_fifo.get(item); `uvm_info(`gfn, $sformatf("received mem transaction:\n%0s", item.sprint()), UVM_HIGH) - if (item.is_response) begin - busy_check(); - assert(mem_trans_count > 0); - mem_trans_count -= 1; - end else begin - mem_trans_count += 1; - if (item.seed != 32'd0) mem_seeds.push_back(item.seed); - end + case (item.trans_type) + ICacheMemNewSeed: begin + mem_seeds.push_back(item.data); + end + + ICacheMemGrant: begin + mem_trans_count += 1; + end + + ICacheMemResponse: begin + busy_check(); + `DV_CHECK_FATAL(mem_trans_count > 0); + mem_trans_count -= 1; + end + endcase end endtask @@ -321,12 +328,11 @@ class ibex_icache_scoreboard // compressed instruction, in which case we don't care about the top bits anyway. lo_bits_to_drop = 8 * (address - addr_lo); - mem_model.set_seed(seed); - rdata = mem_model.read_data(addr_lo) >> lo_bits_to_drop; + rdata = mem_model.read_data(seed, addr_lo) >> lo_bits_to_drop; return is_fetch_compatible_1(seen_insn_data, seen_err, - mem_model.is_either_error(addr_lo), + mem_model.is_either_error(seed, addr_lo), rdata[31:0], seed, chatty); @@ -365,16 +371,14 @@ class ibex_icache_scoreboard lo_bits_to_drop = BusWidth - lo_bits_to_take; // Do the first read (from the low address) and shift right to drop the bits that we don't need. - mem_model.set_seed(seed_lo); - exp_err_lo = mem_model.is_either_error(addr_lo); - rdata = mem_model.read_data(addr_lo) >> lo_bits_to_drop; + exp_err_lo = mem_model.is_either_error(seed_lo, addr_lo); + rdata = mem_model.read_data(seed_lo, addr_lo) >> lo_bits_to_drop; exp_data = rdata[31:0]; // Now do the second read (from the upper address). Shift the result up by lo_bits_to_take, // which will discard some top bits. Then extract 32 bits and OR with what we have so far. - mem_model.set_seed(seed_hi); - exp_err_hi = mem_model.is_either_error(addr_hi); - rdata = mem_model.read_data(addr_hi) << lo_bits_to_take; + exp_err_hi = mem_model.is_either_error(seed_hi, addr_hi); + rdata = mem_model.read_data(seed_hi, addr_hi) << lo_bits_to_take; exp_data = exp_data | rdata[31:0]; return is_fetch_compatible_2(seen_insn_data, seen_err_plus2, diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_agent_pkg.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_agent_pkg.sv index f81f53aa..aebdfc69 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_agent_pkg.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_agent_pkg.sv @@ -12,7 +12,11 @@ package ibex_icache_mem_agent_pkg; `include "uvm_macros.svh" `include "dv_macros.svh" - // parameters + typedef enum { + ICacheMemNewSeed, + ICacheMemGrant, + ICacheMemResponse + } ibex_icache_mem_trans_type_e; // package sources `include "ibex_icache_mem_req_item.sv" diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_bus_item.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_bus_item.sv index fe57bdc5..043045d5 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_bus_item.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_bus_item.sv @@ -8,23 +8,20 @@ class ibex_icache_mem_bus_item extends uvm_sequence_item; - // Is this a request or a response? - logic is_response; + // What sort of transaction is this? (new seed, grant or response) + ibex_icache_mem_trans_type_e trans_type; - // Request address and possible new seed (only valid for request transactions) - logic [31:0] address; - bit [31:0] seed; + // This holds the new seed for a 'new seed' transaction, the request address for a grant + // transaction and the returned rdata for a response transaction. + logic [31:0] data; - // Response data and error flags (only valid for response transactions) - logic [31:0] rdata; + // Response error flag (only valid for response transactions) logic err; `uvm_object_utils_begin(ibex_icache_mem_bus_item) - `uvm_field_int(is_response, UVM_DEFAULT) - `uvm_field_int(address, UVM_DEFAULT | UVM_HEX) - `uvm_field_int(seed, UVM_DEFAULT | UVM_HEX) - `uvm_field_int(rdata, UVM_DEFAULT | UVM_HEX) - `uvm_field_int(err, UVM_DEFAULT) + `uvm_field_enum(ibex_icache_mem_trans_type_e, trans_type, UVM_DEFAULT) + `uvm_field_int(data, UVM_DEFAULT | UVM_HEX) + `uvm_field_int(err, UVM_DEFAULT) `uvm_object_utils_end `uvm_object_new diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_model.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_model.sv index 8d096c15..783c8b46 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_model.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_model.sv @@ -24,7 +24,6 @@ class ibex_icache_mem_model #(parameter int unsigned BusWidth = 32) extends uvm_object; - protected bit [31:0] seed = 32'd0; protected bit no_pmp_errs = 0; protected bit no_mem_errs = 0; @@ -34,49 +33,41 @@ class ibex_icache_mem_model #(parameter int unsigned BusWidth = 32) endfunction `uvm_object_utils_begin(ibex_icache_mem_model) - `uvm_field_int (seed, UVM_DEFAULT | UVM_HEX) `uvm_field_int (no_pmp_errs, UVM_DEFAULT) `uvm_field_int (no_mem_errs, UVM_DEFAULT) `uvm_object_utils_end - function void set_seed(bit [31:0] new_seed); - seed = new_seed; - endfunction - // Return true if reading from BusWidth bits from address should give an error - function automatic logic is_error(logic [31:0] address); + protected function automatic logic is_error(bit [31:0] seed, logic [31:0] address); logic [31:0] rng_lo, rng_hi, rng_w0, rng_w1; rng_lo = seed ^ 32'hdeadbeef; rng_w0 = 32'd1 << (32 - 3); rng_w1 = (~32'd0) - rng_lo; rng_hi = rng_lo + ((rng_w0 < rng_w1) ? rng_w0 : rng_w1); - return ranges_overlap(address, address + BusWidth / 8, rng_lo, rng_hi); - endfunction + // Return true iff [address, address + BusWidth / 8) and [rng_lo, rng_hi) have nonempty + // intersection. + return (address < rng_hi) && (rng_lo < address + BusWidth / 8); - // Return true iff [lo0, hi0) and [lo1, hi1) have nonempty intersection - function automatic logic ranges_overlap(logic [31:0] lo0, logic [31:0] hi0, - logic [31:0] lo1, logic [31:0] hi1); - return (lo0 < hi1) && (lo1 < hi0); endfunction // Return true if reading BusWidth bits from address should give a PMP error - function automatic logic is_pmp_error(logic [31:0] address); - return (! no_pmp_errs) && is_error(address ^ 32'h12344321); + function automatic logic is_pmp_error(bit [31:0] seed, logic [31:0] address); + return (! no_pmp_errs) && is_error(seed, address ^ 32'h12344321); endfunction // Return true if reading BusWidth bits from address should give a memory error - function automatic logic is_mem_error(logic [31:0] address); - return (! no_mem_errs) && is_error(address ^ 32'hf00dbeef); + function automatic logic is_mem_error(bit [31:0] seed, logic [31:0] address); + return (! no_mem_errs) && is_error(seed, address ^ 32'hf00dbeef); endfunction // Return true if reading BusWidth bits from address should give some sort of error - function automatic logic is_either_error(logic [31:0] address); - return is_pmp_error(address) || is_mem_error(address); + function automatic logic is_either_error(bit [31:0] seed, logic [31:0] address); + return is_pmp_error(seed, address) || is_mem_error(seed, address); endfunction // Return BusWidth bits of data from reading at address. - function automatic logic [BusWidth-1:0] read_data(logic [31:0] address); + function automatic logic [BusWidth-1:0] read_data(bit [31:0] seed, logic [31:0] address); logic [BusWidth-1:0] acc, word_data; int word_count, lo_idx, lo_bit; logic [29:0] word_addr; @@ -98,7 +89,7 @@ class ibex_icache_mem_model #(parameter int unsigned BusWidth = 32) // Note that the address sum might wrap (if we read off the top of memory), but that's not // really a problem. word_data = 0; - word_data[31:0] = read_word(word_addr + i[29:0]); + word_data[31:0] = read_word(seed, word_addr + i[29:0]); // The bottom valid byte in word_data is normally 0, but is positive if i is zero (the bottom) // word and the read was misaligned. In that case, it equals i & 3. @@ -115,7 +106,7 @@ class ibex_icache_mem_model #(parameter int unsigned BusWidth = 32) endfunction // Read 32 bits of data from reading at word_address. - function automatic logic [31:0] read_word(logic [29:0] address); + function automatic logic [31:0] read_word(bit [31:0] seed, logic [29:0] address); return hash({2'b0, address} ^ seed); endfunction diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv index 4c8b936f..55165827 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_monitor.sv @@ -76,11 +76,9 @@ class ibex_icache_mem_monitor forever begin if (cfg.vif.monitor_cb.rvalid) begin bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans"); - bus_trans.is_response = 1'b1; - bus_trans.address = '0; - bus_trans.seed = '0; - bus_trans.rdata = cfg.vif.monitor_cb.rdata; - bus_trans.err = cfg.vif.monitor_cb.err; + bus_trans.trans_type = ICacheMemResponse; + bus_trans.data = cfg.vif.monitor_cb.rdata; + bus_trans.err = cfg.vif.monitor_cb.err; analysis_port.write(bus_trans); end @@ -88,33 +86,42 @@ class ibex_icache_mem_monitor end endtask - // This is called immediately when an address is requested and is used to drive the PMP response + // This is called immediately when an address is requested and is used to drive the PMP response. + // If we decide to choose a new seed with this transaction, the seed also gets passed to the + // scoreboard. function automatic void new_request(logic [31:0] addr); + ibex_icache_mem_bus_item bus_trans; ibex_icache_mem_req_item item = new("item"); item.is_grant = 1'b0; item.address = addr; - item.seed = '0; + `DV_CHECK_RANDOMIZE_FATAL(item) request_port.write(item); + + if (item.seed != 0) begin + bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans"); + bus_trans.trans_type = ICacheMemNewSeed; + bus_trans.data = item.seed; + bus_trans.err = '0; + analysis_port.write(bus_trans); + end endfunction // This is called on a clock edge when an request is granted function automatic void new_grant(logic [31:0] addr); - ibex_icache_mem_req_item item; + ibex_icache_mem_req_item item; ibex_icache_mem_bus_item bus_trans; item = ibex_icache_mem_req_item::type_id::create("item"); item.is_grant = 1'b1; item.address = addr; - `DV_CHECK_RANDOMIZE_FATAL(item) + item.seed = '0; request_port.write(item); bus_trans = ibex_icache_mem_bus_item::type_id::create("bus_trans"); - bus_trans.is_response = 1'b0; - bus_trans.address = addr; - bus_trans.rdata = '0; - bus_trans.seed = item.seed; - bus_trans.err = 0; + bus_trans.trans_type = ICacheMemGrant; + bus_trans.data = addr; + bus_trans.err = 0; analysis_port.write(bus_trans); endfunction diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_req_item.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_req_item.sv index 87c5e452..8afa1e1f 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_req_item.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/ibex_icache_mem_req_item.sv @@ -6,11 +6,12 @@ // memory. // // When a request first comes in (via a posedge on the req line), it immediately generates a -// req_item with is_grant = 0. This is used by the driver to decide whether to generate a PMP error. +// req_item with is_grant = 0. This is used by the sequence to tell the driver whether to generate a +// PMP error. A req_item with is_grant = 0 may also have a seed update. // // Assuming that the request wasn't squashed by a PMP error, it will be granted on some later clock // edge. At that point, another req_item is generated with is_grant = 1. This is added to a queue in -// the driver and will be serviced at some later point. This item may also have a seed update. +// the driver and will be serviced at some later point. class ibex_icache_mem_req_item extends uvm_sequence_item; diff --git a/dv/uvm/icache/dv/ibex_icache_mem_agent/seq_lib/ibex_icache_mem_resp_seq.sv b/dv/uvm/icache/dv/ibex_icache_mem_agent/seq_lib/ibex_icache_mem_resp_seq.sv index 5fcfbef0..f4ecf6c7 100644 --- a/dv/uvm/icache/dv/ibex_icache_mem_agent/seq_lib/ibex_icache_mem_resp_seq.sv +++ b/dv/uvm/icache/dv/ibex_icache_mem_agent/seq_lib/ibex_icache_mem_resp_seq.sv @@ -17,34 +17,70 @@ class ibex_icache_mem_resp_seq extends ibex_icache_mem_base_seq; endtask task body(); + // We pick new seeds when we spot a request (rather than when we spot a grant) to ensure that + // any given fetch corresponds to exactly one seed. Unfortunately, there's a race if these two + // things happen at one clock edge: + // + // - Request for new address which gets a new seed + // - Grant of previous request + // + // If that happens, and the event scheduler happened to schedule the monitor to spot the request + // before the grant, we might accidentally handle the grant with the new seed rather than the + // old one, which would cause confusion in the scoreboard. + // + // To avoid this problem, we keep a FIFO of pending addresses along with their corresponding + // seeds. When a grant message comes in, we know that we can look up the correct seed there. + mailbox #(bit[63:0]) pending_grants = new("pending_grants"); + ibex_icache_mem_req_item req_item = new("req_item"); ibex_icache_mem_resp_item resp_item = new("resp_item"); + bit [63:0] gnt_data; + bit [31:0] gnt_addr, gnt_seed; + + bit [31:0] cur_seed = 0; + forever begin // Wait for a transaction request. p_sequencer.request_fifo.get(req_item); if (!req_item.is_grant) begin - // If this is a request (not a grant), check the memory model for a PMP error at this - // address. The other fields are ignored. + // If this is a request (not a grant), take any new seed and then check the memory model for + // a PMP error at this address. Add the address and its corresponding seed to the list of + // pending grants. + if (req_item.seed != 32'd0) begin + `uvm_info(`gfn, $sformatf("New memory seed: 0x%08h", req_item.seed), UVM_HIGH) + cur_seed = req_item.seed; + end + resp_item.is_grant = 1'b0; - resp_item.err = mem_model.is_pmp_error(req_item.address); + resp_item.err = mem_model.is_pmp_error(cur_seed, req_item.address); resp_item.address = req_item.address; resp_item.rdata = 'X; - end else begin - // If this is a grant, take any new seed then check the memory model for a (non-PMP) error - // at this address. On success, look up the memory data too. + pending_grants.put({req_item.address, cur_seed}); - if (req_item.seed != 32'd0) begin - `uvm_info(`gfn, $sformatf("New memory seed: 0x%08h", req_item.seed), UVM_HIGH) - mem_model.set_seed(req_item.seed); + end else begin + // If this is a grant, search through the pending_grants mailbox to find the seed from the + // request that matched. + gnt_addr = req_item.address + 1; + while (pending_grants.num() > 0) begin + pending_grants.get(gnt_data); + {gnt_addr, gnt_seed} = gnt_data; + if (gnt_addr == req_item.address) break; end + // If nothing went wrong, we should have found the right item and gnt_addr should now + // equal req_item.address + `DV_CHECK_FATAL(gnt_addr == req_item.address, + $sformatf("No pending grant for address 0x%08h.", req_item.address)) + + // Using the seed that we saw for the request, check the memory model for a (non-PMP) error + // at this address. On success, look up the memory data too. resp_item.is_grant = 1'b1; - resp_item.err = mem_model.is_mem_error(req_item.address); + resp_item.err = mem_model.is_mem_error(gnt_seed, req_item.address); resp_item.address = req_item.address; - resp_item.rdata = resp_item.err ? 'X : mem_model.read_data(req_item.address); + resp_item.rdata = resp_item.err ? 'X : mem_model.read_data(gnt_seed, req_item.address); end // Use the response item as an entry in the sequence, randomising any delay