Merge pull request #141 from antmicro/mkurc/fix-axi2ahb

Fix  AXI4 to AHB converter issues

.github/scripts/run_regression_test.sh

@@ -7,25 +7,37 @@ run_regression_test(){
     # Run a regression test with coverage collection enabled
     # Args:
     # RESULTS_DIR -
+    # BUS -
     # NAME -
     # COVERAGE -
-    check_args_count $# 3
+    check_args_count $# 4
     RESULTS_DIR=$1
-    NAME=$2
-    COVERAGE=$3
+    BUS=$2
+    NAME=$3
+    COVERAGE=$4
     echo -e "${COLOR_WHITE}========== running test '${NAME}' =========${COLOR_CLEAR}"
     echo -e "${COLOR_WHITE} RESULTS_DIR = ${RESULTS_DIR}${COLOR_CLEAR}"
+    echo -e "${COLOR_WHITE} SYSTEM BUS  = ${BUS}${COLOR_CLEAR}"
     echo -e "${COLOR_WHITE} NAME        = ${NAME}${COLOR_CLEAR}"
     echo -e "${COLOR_WHITE} COVERAGE    = ${COVERAGE}${COLOR_CLEAR}"
 
+    if [[ "${BUS}" == "axi" ]]; then
+        PARAMS="-set build_axi4"
+    elif [[ "${BUS}" == "ahb" ]]; then
+        PARAMS="-set build_ahb_lite"
+    else
+        echo -e "${COLOR_RED}Unknown system bus type '${BUS}'${COLOR_CLEAR}"
+        exit 1
+    fi
+
     mkdir -p ${RESULTS_DIR}
     LOG="${RESULTS_DIR}/test_${NAME}_${COVERAGE}.log"
     touch ${LOG}
     DIR="run_${NAME}_${COVERAGE}"
 
     # Run the test
     mkdir -p ${DIR}
-    make -j`nproc` -C ${DIR} -f $RV_ROOT/tools/Makefile verilator TEST=${NAME} COVERAGE=${COVERAGE} 2>&1 | tee ${LOG}
+    make -j`nproc` -C ${DIR} -f $RV_ROOT/tools/Makefile verilator CONF_PARAMS="${PARAMS}" TEST=${NAME} COVERAGE=${COVERAGE} 2>&1 | tee ${LOG}
     if [ ! -f "${DIR}/coverage.dat" ]; then
         echo -e "${COLOR_WHITE}Test '${NAME}' ${COLOR_RED}FAILED${COLOR_CLEAR}"
         exit 1
@@ -38,11 +50,12 @@ run_regression_test(){
 
 # Example usage
 # RESULTS_DIR=results
+# BUS=axi
 # NAME=hello_world
 # COVERAGE=branch
-# run_regression_test.sh $RESULTS_DIR $NAME $COVERAGE
+# run_regression_test.sh $RESULTS_DIR $BUS $NAME $COVERAGE
 
-check_args_count $# 3
+check_args_count $# 4
 run_regression_test "$@"
 
 

.github/workflows/test-regression.yml

@@ -10,6 +10,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
+        bus: ["axi", "ahb"]
         test: ["hello_world", "hello_world_dccm", "hello_world_iccm", "cmark", "cmark_dccm", "cmark_iccm", "dhry", "pmp"]
         coverage: ["branch", "toggle"] #TODO: add functional coverage
     env:
@@ -82,15 +83,15 @@ jobs:
         run: |
           export PATH=/opt/verilator/bin:$PATH
           export RV_ROOT=`pwd`
-          .github/scripts/run_regression_test.sh $TEST_PATH ${{ matrix.test}} ${{ matrix.coverage }}
+          .github/scripts/run_regression_test.sh $TEST_PATH ${{ matrix.bus }} ${{ matrix.test}} ${{ matrix.coverage }}
 
       - name: Prepare coverage data
         run: |
           .github/scripts/convert_coverage_data.sh ${TEST_PATH}/
           echo "convert_coverage_data.sh exited with RET_CODE = "$?
           mkdir -p results
           mv ${TEST_PATH}/coverage.info \
-             results/coverage_${{ matrix.test }}_${{ matrix.coverage }}.info
+             results/coverage_${{ matrix.bus }}_${{ matrix.test }}_${{ matrix.coverage }}.info
 
       - name: Pack artifacts
         if: always()

design/lib/axi4_to_ahb.sv

@@ -88,11 +88,21 @@ import el2_pkg::*;
 
    localparam ID   = 1;
    localparam PRTY = 1;
-   typedef enum logic [2:0] {IDLE=3'b000, CMD_RD=3'b001, CMD_WR=3'b010, DATA_RD=3'b011, DATA_WR=3'b100, DONE=3'b101, STREAM_RD=3'b110, STREAM_ERR_RD=3'b111} state_t;
+   typedef enum logic [3:0] {
+        IDLE            = 4'b0000,
+        CMD_RD          = 4'b0001,
+        CMD_WR          = 4'b1001,
+        DATA_RD         = 4'b0010,
+        DATA_WR         = 4'b1010,
+        DONE_RD         = 4'b0011,
+        DONE_WR         = 4'b1011,
+        STREAM_RD       = 4'b0101,
+        STREAM_ERR_RD   = 4'b0110
+    } state_t;
+
    state_t buf_state, buf_nxtstate;
 
    logic             slave_valid;
-   logic             slave_ready;
    logic [TAG-1:0]   slave_tag;
    logic [63:0]      slave_rdata;
    logic [3:0]       slave_opc;
@@ -245,15 +255,14 @@ import el2_pkg::*;
    assign master_wdata[63:0]  = wrbuf_data[63:0];
 
    // AXI response channel signals
-   assign axi_bvalid       = slave_valid & slave_ready & slave_opc[3];
+   assign axi_bvalid       = slave_valid & slave_opc[3];
    assign axi_bresp[1:0]   = slave_opc[0] ? 2'b10 : (slave_opc[1] ? 2'b11 : 2'b0);
    assign axi_bid[TAG-1:0] = slave_tag[TAG-1:0];
 
-   assign axi_rvalid       = slave_valid & slave_ready & (slave_opc[3:2] == 2'b0);
+   assign axi_rvalid       = slave_valid & (slave_opc[3:2] == 2'b0);
    assign axi_rresp[1:0]   = slave_opc[0] ? 2'b10 : (slave_opc[1] ? 2'b11 : 2'b0);
    assign axi_rid[TAG-1:0] = slave_tag[TAG-1:0];
    assign axi_rdata[63:0]  = slave_rdata[63:0];
-   assign slave_ready        = axi_bready & axi_rready;
 
  // FIFO state machine
    always_comb begin
@@ -324,7 +333,7 @@ import el2_pkg::*;
                   ahb_htrans[1:0] = 2'b10 & {2{~buf_state_en}};
          end
          DATA_RD: begin
-                  buf_nxtstate   = DONE;
+                  buf_nxtstate   = DONE_RD;
                   buf_state_en   = (ahb_hready_q | ahb_hresp_q);
                   buf_data_wr_en = buf_state_en;
                   slvbuf_error_in= ahb_hresp_q;
@@ -345,8 +354,8 @@ import el2_pkg::*;
          end
          DATA_WR: begin
                   buf_state_en = (cmd_doneQ & ahb_hready_q) | ahb_hresp_q;
-                  master_ready = buf_state_en & ~ahb_hresp_q & slave_ready;   // Ready to accept new command if current command done and no error
-                  buf_nxtstate = (ahb_hresp_q | ~slave_ready) ? DONE :
+                  master_ready = buf_state_en & ~ahb_hresp_q & axi_bready;   // Ready to accept new command if current command done and no error
+                  buf_nxtstate = (ahb_hresp_q | ~axi_bready) ? DONE_WR :
                                   ((master_valid & master_ready) ? ((master_opc[2:1] == 2'b01) ? CMD_WR : CMD_RD) : IDLE);
                   slvbuf_error_in = ahb_hresp_q;
                   slvbuf_error_en = buf_state_en;
@@ -359,19 +368,29 @@ import el2_pkg::*;
                                  ((buf_cmd_byte_ptrQ == 3'b111) | (buf_byteen[get_nxtbyte_ptr(buf_cmd_byte_ptrQ[2:0],buf_byteen[7:0],1'b1)] == 1'b0))));
                   bypass_en       = buf_state_en & buf_write_in & (buf_nxtstate == CMD_WR);   // Only bypass for writes for the time being
                   ahb_htrans[1:0] = {2{(~(cmd_done | cmd_doneQ) | bypass_en)}} & 2'b10;
-                  slave_valid_pre  = buf_state_en & (buf_nxtstate != DONE);
+                  slave_valid_pre  = buf_state_en & (buf_nxtstate != DONE_WR);
 
                   trxn_done = ahb_hready_q & ahb_hwrite_q & (ahb_htrans_q[1:0] != 2'b0);
                   buf_cmd_byte_ptr_en = trxn_done | bypass_en;
                   buf_cmd_byte_ptr = bypass_en ? get_nxtbyte_ptr(3'b0,buf_byteen_in[7:0],1'b0) :
                                                  trxn_done ? get_nxtbyte_ptr(buf_cmd_byte_ptrQ[2:0],buf_byteen[7:0],1'b1) : buf_cmd_byte_ptrQ;
-            end
-         DONE: begin
+         end
+         DONE_WR: begin
+                  buf_nxtstate = IDLE;
+                  buf_state_en = axi_bvalid & axi_bready;
+                  slvbuf_error_en = 1'b1;
+                  slave_valid_pre = 1'b1;
+         end
+         DONE_RD: begin
                   buf_nxtstate = IDLE;
-                  buf_state_en = slave_ready;
+                  buf_state_en = axi_rvalid & axi_rready; // axi_rlast == 1
                   slvbuf_error_en = 1'b1;
                   slave_valid_pre = 1'b1;
          end
+         default: begin
+                  buf_nxtstate = IDLE;
+                  buf_state_en = 1'b1;
+         end
       endcase
    end
 
@@ -403,7 +422,7 @@ import el2_pkg::*;
    assign slave_valid          = slave_valid_pre;// & (~slvbuf_posted_write | slvbuf_error);
    assign slave_opc[3:2]       = slvbuf_write ? 2'b11 : 2'b00;
    assign slave_opc[1:0]       = {2{slvbuf_error}} & 2'b10;
-   assign slave_rdata[63:0]    = slvbuf_error ? {2{last_bus_addr[31:0]}} : ((buf_state == DONE) ? buf_data[63:0] : ahb_hrdata_q[63:0]);
+   assign slave_rdata[63:0]    = slvbuf_error ? {2{last_bus_addr[31:0]}} : ((buf_state == DONE_RD) ? buf_data[63:0] : ahb_hrdata_q[63:0]);
    assign slave_tag[TAG-1:0]   = slvbuf_tag[TAG-1:0];
 
    assign last_addr_en = (ahb_htrans[1:0] != 2'b0) & ahb_hready & ahb_hwrite ;

testbench/tb_top.sv

@@ -328,8 +328,15 @@ module tb_top (
 
 `define DEC rvtop.veer.dec
 
+`ifdef RV_BUILD_AXI4
     assign mailbox_write    = lmem.awvalid && lmem.awaddr == mem_mailbox && rst_l;
     assign mailbox_data     = lmem.wdata;
+`endif
+`ifdef RV_BUILD_AHB_LITE
+    assign mailbox_write    = lmem.write   && lmem.laddr  == mem_mailbox && rst_l;
+    assign mailbox_data     = lmem.HWDATA;
+`endif
+
     assign mailbox_data_val = mailbox_data[7:0] > 8'h5 && mailbox_data[7:0] < 8'h7f;
 
     parameter MAX_CYCLES = 2_000_000;

tools/Makefile

@@ -14,7 +14,7 @@
 # limitations under the License.
 #
 
-CONF_PARAMS = -set build_axi4
+CONF_PARAMS ?= -set build_axi4
 
 TEST_CFLAGS = -g -O3 -funroll-all-loops
 ABI = -mabi=ilp32

verification/block/lib_axi4_to_ahb/axi_r_bfm.py

@@ -109,9 +109,10 @@ async def rsp_monitor_q_bfm(self):
                 await RisingEdge(self.rst_n)
             await RisingEdge(self.clk)
             if get_int(self.dut.axi_rvalid):
-                sigs = get_signals(AXI_R_CHAN_RSP_SIGNALS, self.dut)
-                values = tuple(sig.value for sig in sigs)
-                await self.rsp_monitor_q.put(values)
+                if get_int(self.dut.axi_rready):
+                    sigs = get_signals(AXI_R_CHAN_RSP_SIGNALS, self.dut)
+                    values = tuple(sig.value for sig in sigs)
+                    await self.rsp_monitor_q.put(values)
 
     def start_bfm(self):
         cocotb.start_soon(self.drive())

verification/block/lib_axi4_to_ahb/coordinator_seq.py

@@ -6,7 +6,7 @@
 import cocotb
 from ahb_lite_pkg import AHB_LITE_NOTIFICATION
 from ahb_lite_seq import AHBLiteAcceptReadSeq, AHBLiteAcceptWriteSeq
-from axi_r_seq import AXIReadTransactionRequestSeq
+from axi_r_seq import AXIReadTransactionRequestSeq, AXIReadTransactionResponseSeq
 from axi_w_seq import (
     AXIWriteDataSeq,
     AXIWriteResponseSeq,
@@ -37,6 +37,7 @@ async def axi_write(self, axi_seqr, ahb_seqr):
 
     async def axi_read(self, axi_seqr, ahb_seqr):
         axi_trq_seq = AXIReadTransactionRequestSeq()
+        axi_rresp_seq = AXIReadTransactionResponseSeq()
 
         # Read Request
         await axi_trq_seq.start(axi_seqr)
@@ -46,6 +47,9 @@ async def axi_read(self, axi_seqr, ahb_seqr):
         await self.ahb_response_handler(ahb_seqr=ahb_seqr, is_read=True)
         await self.delay(5)
 
+        # Read Response
+        await axi_rresp_seq.start(axi_seqr)
+
     async def delay(self, i):
         for _ in range(i):
             await RisingEdge(cocotb.top.clk)

verification/block/lib_axi4_to_ahb/test_axi.py

@@ -6,45 +6,8 @@
 from coordinator_seq import TestBothChannelsSeq
 from testbench import BaseTest
 
-# FIXME       : This test is expected to fail.
-# Reason      : Handshake sequence is non-compliant with specification
-# Faulty code : axi4_to_ahb.sv#L248-256
-#
-# Issue #1 BVALID/BREADY Handshake
-# Handshake is meant to occur on the Write Response Channel in order:
-# * subordinate asserts BVALID
-# * manager responds with BREADY
-# Quote: "The Subordinate must not wait for the Manager to assert BREADY
-# before asserting BVALID"
-# Source: AMBA AXI Protocol Specification A3.5.1 Write transaction dependencies
-#
-# In RTL:
-#
-# assign axi_bvalid  = slave_valid & slave_ready & slave_opc[3];
-# assign slave_ready = axi_bready & axi_rready;
-#
-# BVALID is calculated from BREADY and RREADY, which is wrong for 2 reasons:
-# * BVALID should not depend on RREADY
-# * BVALID should be asserted before BREADY. BREADY should depend on BVALID.
-#
-# Issue #2 RVALID/RREADY Handshake
-# Handshake is meant to occur on the Read Response Channel in order:
-# * subordinate asserts RVALID
-# * manager responds with RREADY
-# Quote: "The Subordinate must not wait for the Manager to assert RREADY
-# before asserting RVALID"
-# Source: AMBA AXI Protocol Specification A3.5.2 Read transaction dependencies
-#
-# In RTL:
-# assign axi_rvalid  = slave_valid & slave_ready & (slave_opc[3:2] == 2'b0);
-# assign slave_ready = axi_bready & axi_rready;
-#
-# RVALID is calculated from BREADY and RREADY, which is wrong for 2 reasons:
-# * RVALID should not depend on BREADY
-# * RVALID should be asserted before RREADY. RREADY should depend on RVALID.
 
-
-@pyuvm.test(expect_error=(TimeoutError, QueueFull))
+@pyuvm.test()
 class TestAXI(BaseTest):
     def end_of_elaboration_phase(self):
         self.seq = TestBothChannelsSeq.create("stimulus")

verification/block/lib_axi4_to_ahb/test_axi_read_channel.py

@@ -6,11 +6,8 @@
 from coordinator_seq import TestReadChannelSeq
 from testbench import BaseTest
 
-# FIXME       : This test is expected to fail.
-# See description in `test_axi.py`
 
-
-@pyuvm.test(expect_error=QueueFull)
+@pyuvm.test()
 class TestAXIReadChannel(BaseTest):
     def end_of_elaboration_phase(self):
         self.seq = TestReadChannelSeq.create("stimulus")

verification/block/lib_axi4_to_ahb/test_axi_write_channel.py

@@ -5,11 +5,8 @@
 from coordinator_seq import TestWriteChannelSeq
 from testbench import BaseTest
 
-# FIXME       : This test is expected to fail.
-# See description in `test_axi.py`
 
-
-@pyuvm.test(expect_error=TimeoutError)
+@pyuvm.test()
 class TestAXIWriteChannel(BaseTest):
     def end_of_elaboration_phase(self):
         self.seq = TestWriteChannelSeq.create("stimulus")