Axi4 passive vip by csabakiss-semify · Pull Request #391 · lowRISC/mocha

csabakiss-semify · 2026-03-30T17:44:20Z

Developed AXI4 VIP supporting passive mode only
VIP integrated to the top level environment
Added scoreboard to the AXI XBAR

Tested with top level test cases.

martin-velay

I have added some comments, but I am still not done, I'll do another batch later. There are still some coding style issues

martin-velay · 2026-03-31T12:28:04Z

hw/top_chip/dv/tb/axi_vip_connections.sv

@@ -0,0 +1,306 @@
+  // This AXI4 VIP shall be always UVM_PASSIVE on top level


The copyright header is missing

martin-velay · 2026-03-31T12:34:54Z

hw/top_chip/dv/env/top_chip_dv_env_pkg.sv

+  typedef enum bit[2:0] {
+    mst0 = 0,
+    slv0 = 1,
+    slv1 = 2,
+    slv2 = 3,
+    slv3 = 4
+  } axi_if_t;


Would it be possible to change the field names into something more explicit like that maybe:

Suggested change

typedef enum bit[2:0] {

mst0 = 0,

slv0 = 1,

slv1 = 2,

slv2 = 3,

slv3 = 4

} axi_if_t;

typedef enum bit[2:0] {

mst0_cva6 = 0,

slv0_sram = 1,

slv1_mailbox = 2,

slv2_tl_xbar = 3,

slv3_dram = 4

} axi_if_t;

top_pkg field names are used.

martin-velay · 2026-03-31T12:39:11Z

hw/top_chip/dv/test/top_chip_dv_base_test.sv

+  // AXI VIP configuration
+  axi_cfg = new[NUM_OF_AXI_IFS];
+  env.cfg.m_axi_cfg = new[NUM_OF_AXI_IFS];
+  foreach(axi_cfg[i]) begin


Nit: you need a space here and same below for the case. Can you please check else where in your code that you follow this rule.
https://github.com/lowRISC/style-guides/blob/master/VerilogCodingStyle.md#space-around-keywords

martin-velay · 2026-03-31T12:49:56Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+
+  foreach (actual_q[s]) foreach (actual_q[s][i]) if (actual_q[s][i].size() > 0)
+    `uvm_error("SCB_DRAIN", $sformatf("Slave trans on %s (ID %h) never reached Master", s, i))
+endfunction : check_phase


You need to configure your editor to make sure you insert an empty line at the end of each file (it's also part of the coding rules somewhere)

martin-velay · 2026-03-31T12:52:34Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+    string     slave_name;
+    bit [63:0] start_addr;
+    bit [63:0] end_addr;
+  } local_addr_range_t;


Can you move this typedef into the env_pkg

martin-velay · 2026-03-31T12:58:58Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+  int unsigned       master_id_width  = 4; 
+
+  // Analysis Implementation Ports
+  `uvm_analysis_imp_decl(_mst0)


In the names if we can make it clearer which block is connected, as commented in the env_pkg, that would be better for debug purposes

Updated using the top_pkg names

martin-velay · 2026-03-31T13:00:56Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+
+  // Queues to handle out-of-order monitor arrivals
+  // [SlaveName][MaskedID]
+  axi4_vip_item expect_q[string][bit[63:0]][$]; // Master arrived first


Is this 64 bits width linked with top_pkg::AxiDataWidth? It's better to point to a pkg value, this should be applied wherever required in this PR

This is ID width to handle when the interconnect extends the ID width to be able to back-route the transactions to the managers. However, good spot, the scoreboard cuts the unnecessary bits before anything is pushed to this queue. Replaced by parameter.

martin-velay · 2026-03-31T13:03:51Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+  mem_map.push_back('{"slv0", 64'h1000_0000, 64'h1001_FFFF});
+  mem_map.push_back('{"slv1", 64'h2001_0000, 64'h2001_FFFF});
+  mem_map.push_back('{"slv2", 64'h4000_0000, 64'h4FFF_FFFF});
+  mem_map.push_back('{"slv3", 64'h8000_0000, 64'hBF7F_FFFF});


Here, the same applies, could you point at the pkg values please axi_addr_start_t

martin-velay · 2026-03-31T13:07:01Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+
+function string top_chip_dv_axi_scoreboard::decode_addr(bit [63:0] addr);
+  foreach (mem_map[i]) begin
+    if (addr >= mem_map[i].start_addr && addr <= mem_map[i].end_addr)


You need to add the begin...end for each if whenever it doesn't fit on a single line as a rule. But as we talk about DV here, I'd prefer to add the begin...end and always jump a line this will facilitate breakpoint debug

martin-velay · 2026-03-31T13:54:39Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+
+function void top_chip_dv_axi_scoreboard::check_phase(uvm_phase phase);
+  super.check_phase(phase);
+  foreach (expect_q[s]) foreach (expect_q[s][i]) if (expect_q[s][i].size() > 0)


This should be break down into multi lines and I think one foreach is useless:

Suggested change

foreach (expect_q[s]) foreach (expect_q[s][i]) if (expect_q[s][i].size() > 0)

foreach (expect_q[s][i]) begin

if (expect_q[s][i].size() > 0) begin

`uvm_error("SCB_DRAIN", $sformatf("Master req for %s (ID %h) never reached Slave", s, i))

end

thommythomaso · 2026-04-01T06:53:00Z

hw/dv/vip/axi4_vip/axi4_vip_defines.svh

+// maximum supported bus widths
+`define AXI4_MAX_ID_WIDTH     16
+`define AXI4_MAX_ADDR_WIDTH   64
+`define AXI4_MAX_DATA_WIDTH   512


Is there a reason why DATA_WIDTH 1024 is not supported?

Updated to 1024.

thommythomaso · 2026-04-01T07:31:38Z

hw/dv/vip/axi4_vip/axi4_vip_monitor.svh

+      if (current_burst == null) current_burst = axi4_vip_item::type_id::create("w_burst");
+
+      current_burst.dir = AXI_WRITE;
+      current_burst.wdata.push_back(vif.monitor_cb.wdata & ((64'h1 << m_cfg.m_data_width) - 1));


Shouldn't this be 512'h1 (or 1024'h1) for all rdata and wdata types?

Updated with the given define.

thommythomaso · 2026-04-01T07:45:09Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+
+  // Local Address Map Struct
+  typedef struct {
+    string     slave_name;


Nit: the new nomenclature of AXI uses manager and subordinate.

Yes, we talked about this. Is it better if the naming matches the AXI standard or the current RTL. I know, the new standard uses manager and subordinate, but the RTL uses master and slave.

Good point. I'd say that we should follow the latest nomenclature with manager/subordinate. So it might be that the RTL needs to be changed? But from our concern in this VIP, as its purpose goes beyond this particular DUT eventually, I'd suggest to use the new wording. Do you think there will be some confusing piece of code where the 2 kinds will live together?

Updated names.

thommythomaso · 2026-04-01T07:54:10Z

hw/top_chip/dv/env/top_chip_dv_axi_scoreboard.sv

+  begin
+    bit [63:0] addr = (tr.dir == AXI_WRITE) ? tr.awaddr : tr.araddr;
+    bit [63:0] id   = (tr.dir == AXI_WRITE) ? tr.awid   : tr.arid;
+    // Note: resp logic simplified here for example; normally bit-sliced per beat


Can you please formulate this comment more precise?

This is a legacy comment from a previous version. Thanks for spotting it. Removed.

Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>

…top UVM environment Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>

martin-velay self-requested a review March 31, 2026 07:15

martin-velay assigned csabakiss-semify Mar 31, 2026

martin-velay requested a review from marnovandermaas March 31, 2026 07:16

martin-velay mentioned this pull request Mar 31, 2026

Axi4 passive vip and scoreboard #356

Closed

martin-velay reviewed Mar 31, 2026

View reviewed changes

thommythomaso reviewed Apr 1, 2026

View reviewed changes

csabakiss-semify added 3 commits April 2, 2026 15:39

[axi,dv] Initial AXI4 VIP development with passive mode support

a689fe1

Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>

[top,script] Added AXI4 VIP files to the top chip simulation build

317515e

Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>

[top,dv] Integrated AXI4 passive VIP and added AXI scoreboard to the …

78b7a22

…top UVM environment Signed-off-by: Csaba Kiss <csaba.kiss@semify-eda.com>

csabakiss-semify force-pushed the axi4_passive_vip branch from f698608 to 78b7a22 Compare April 2, 2026 14:40

		@@ -0,0 +1,306 @@
		// This AXI4 VIP shall be always UVM_PASSIVE on top level

-  foreach (expect_q[s]) foreach (expect_q[s][i]) if (expect_q[s][i].size() > 0)
+  foreach (expect_q[s][i]) begin
+    if (expect_q[s][i].size() > 0) begin
+      `uvm_error("SCB_DRAIN", $sformatf("Master req for %s (ID %h) never reached Slave", s, i))
+    end

Conversation

csabakiss-semify commented Mar 30, 2026

Uh oh!

martin-velay left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thommythomaso Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martin-velay Apr 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

thommythomaso Apr 1, 2026 •

edited

Loading

martin-velay Apr 2, 2026 •

edited

Loading