diff --git a/docs/news.d/1127.breaking.rst b/docs/news.d/1127.breaking.rst new file mode 100644 index 000000000..5e30ca084 --- /dev/null +++ b/docs/news.d/1127.breaking.rst @@ -0,0 +1,11 @@ +The AXI stream standard requires tdata to be a multiple of 8 bits. VUnit VCs allowed non-standard widths but the tkeep and tstrb +signals didn't cover the most significant bits beyond the last full byte. For example, if the tdata width was 12 bits, tkeep and tstrb +were one bit corresponding to the least significant byte in tdata. The 4 most significant bits didn't have a corresponding tkeep +bit. + +Starting in VUnit v5.0.0-dev2, the check features for AXI stream were updated such that comparison of the actual tdata value with the +expected one, only considers bytes for which tkeep and tstrb are set. As a consequence, the upper bits of non-standard width tdata, +for which there is no tkeep and tstrb bits, were not considered for comparison. + +The VCs have now been updated to extend tkeep and tstrb in these cases such that all bits in tdata are considered for comparison. +Existing testbenches using non-standard widths *and* tkeep or tstrb will have to be updated as the ports are one bit wider. diff --git a/vunit/vhdl/verification_components/run.py b/vunit/vhdl/verification_components/run.py index 66178fca4..1cd498c4b 100644 --- a/vunit/vhdl/verification_components/run.py +++ b/vunit/vhdl/verification_components/run.py @@ -134,7 +134,7 @@ def gen_avalon_master_tests(obj, *args): for id_length in [0, 8]: for dest_length in [0, 8]: for user_length in [0, 8]: - for data_length in [8, 16]: + for data_length in [0, 3, 8, 11, 16]: for test in TB_AXI_STREAM.get_tests("*check"): test.add_config( name=f"id_l={id_length} dest_l={dest_length} user_l={user_length} data_l={data_length}", @@ -150,7 +150,7 @@ def gen_avalon_master_tests(obj, *args): TB_AXI_STREAM_PROTOCOL_CHECKER = LIB.test_bench("tb_axi_stream_protocol_checker") -for data_length in [0, 8, 32]: +for data_length in [0, 3, 8, 11, 32]: for test in TB_AXI_STREAM_PROTOCOL_CHECKER.get_tests("*passing*tdata*"): test.add_config(name="data_length=%d" % data_length, generics=dict(data_length=data_length)) diff --git a/vunit/vhdl/verification_components/src/axi_stream_master.vhd b/vunit/vhdl/verification_components/src/axi_stream_master.vhd index 87e2551c2..ed002c3a7 100644 --- a/vunit/vhdl/verification_components/src/axi_stream_master.vhd +++ b/vunit/vhdl/verification_components/src/axi_stream_master.vhd @@ -37,8 +37,8 @@ entity axi_stream_master is tready : in std_logic := '1'; tdata : out std_logic_vector(data_length(master)-1 downto 0) := (others => '0'); tlast : out std_logic := '0'; - tkeep : out std_logic_vector(data_length(master)/8-1 downto 0) := (others => '1'); - tstrb : out std_logic_vector(data_length(master)/8-1 downto 0) := (others => '1'); + tkeep : out std_logic_vector(keep_strb_length(master)-1 downto 0) := (others => '1'); + tstrb : out std_logic_vector(keep_strb_length(master)-1 downto 0) := (others => '1'); tid : out std_logic_vector(id_length(master)-1 downto 0) := (others => '0'); tdest : out std_logic_vector(dest_length(master)-1 downto 0) := (others => '0'); tuser : out std_logic_vector(user_length(master)-1 downto 0) := (others => '0') @@ -55,8 +55,8 @@ architecture a of axi_stream_master is procedure drive_invalid_output(signal l_tdata : out std_logic_vector(data_length(master)-1 downto 0); - signal l_tkeep : out std_logic_vector(data_length(master)/8-1 downto 0); - signal l_tstrb : out std_logic_vector(data_length(master)/8-1 downto 0); + signal l_tkeep : out std_logic_vector(keep_strb_length(master)-1 downto 0); + signal l_tstrb : out std_logic_vector(keep_strb_length(master)-1 downto 0); signal l_tid : out std_logic_vector(id_length(master)-1 downto 0); signal l_tdest : out std_logic_vector(dest_length(master)-1 downto 0); signal l_tuser : out std_logic_vector(user_length(master)-1 downto 0)) diff --git a/vunit/vhdl/verification_components/src/axi_stream_monitor.vhd b/vunit/vhdl/verification_components/src/axi_stream_monitor.vhd index 08b0e8012..ce275c3f4 100644 --- a/vunit/vhdl/verification_components/src/axi_stream_monitor.vhd +++ b/vunit/vhdl/verification_components/src/axi_stream_monitor.vhd @@ -24,8 +24,8 @@ entity axi_stream_monitor is tready : in std_logic := '1'; tdata : in std_logic_vector(data_length(monitor) - 1 downto 0); tlast : in std_logic := '1'; - tkeep : in std_logic_vector(data_length(monitor)/8-1 downto 0) := (others => '1'); - tstrb : in std_logic_vector(data_length(monitor)/8-1 downto 0) := (others => 'U'); + tkeep : in std_logic_vector(keep_strb_length(monitor)-1 downto 0) := (others => '1'); + tstrb : in std_logic_vector(keep_strb_length(monitor)-1 downto 0) := (others => 'U'); tid : in std_logic_vector(id_length(monitor)-1 downto 0) := (others => '0'); tdest : in std_logic_vector(dest_length(monitor)-1 downto 0) := (others => '0'); tuser : in std_logic_vector(user_length(monitor)-1 downto 0) := (others => '0') diff --git a/vunit/vhdl/verification_components/src/axi_stream_pkg.vhd b/vunit/vhdl/verification_components/src/axi_stream_pkg.vhd index 26a6cd75d..0e554f9f5 100644 --- a/vunit/vhdl/verification_components/src/axi_stream_pkg.vhd +++ b/vunit/vhdl/verification_components/src/axi_stream_pkg.vhd @@ -207,6 +207,10 @@ package axi_stream_pkg is impure function data_length(slave : axi_stream_slave_t) return natural; impure function data_length(monitor : axi_stream_monitor_t) return natural; impure function data_length(protocol_checker : axi_stream_protocol_checker_t) return natural; + impure function keep_strb_length(master : axi_stream_master_t) return natural; + impure function keep_strb_length(slave : axi_stream_slave_t) return natural; + impure function keep_strb_length(monitor : axi_stream_monitor_t) return natural; + impure function keep_strb_length(protocol_checker : axi_stream_protocol_checker_t) return natural; impure function id_length(master : axi_stream_master_t) return natural; impure function id_length(slave : axi_stream_slave_t) return natural; impure function id_length(monitor : axi_stream_monitor_t) return natural; @@ -516,6 +520,26 @@ package body axi_stream_pkg is return protocol_checker.p_data_length; end; + impure function keep_strb_length(master : axi_stream_master_t) return natural is + begin + return (master.p_data_length + 7) / 8 ; + end; + + impure function keep_strb_length(slave : axi_stream_slave_t) return natural is + begin + return (slave.p_data_length + 7) / 8; + end; + + impure function keep_strb_length(monitor : axi_stream_monitor_t) return natural is + begin + return (monitor.p_data_length + 7) / 8; + end; + + impure function keep_strb_length(protocol_checker : axi_stream_protocol_checker_t) return natural is + begin + return (protocol_checker.p_data_length + 7) / 8; + end; + impure function id_length(master : axi_stream_master_t) return natural is begin return master.p_id_length; @@ -609,8 +633,8 @@ package body axi_stream_pkg is ) is variable msg : msg_t := new_msg(push_axi_stream_msg); variable normalized_data : std_logic_vector(data_length(axi_stream)-1 downto 0) := (others => '0'); - variable normalized_keep : std_logic_vector(data_length(axi_stream)/8-1 downto 0) := (others => '1'); - variable normalized_strb : std_logic_vector(data_length(axi_stream)/8-1 downto 0) := (others => '1'); + variable normalized_keep : std_logic_vector(keep_strb_length(axi_stream)-1 downto 0) := (others => '1'); + variable normalized_strb : std_logic_vector(keep_strb_length(axi_stream)-1 downto 0) := (others => '1'); variable normalized_id : std_logic_vector(id_length(axi_stream)-1 downto 0) := (others => '0'); variable normalized_dest : std_logic_vector(dest_length(axi_stream)-1 downto 0) := (others => '0'); variable normalized_user : std_logic_vector(user_length(axi_stream)-1 downto 0) := (others => '0'); @@ -733,8 +757,8 @@ package body axi_stream_pkg is constant expected_normalized : std_logic_vector(expected'length - 1 downto 0) := expected; variable got_tdata : std_logic_vector(data_length(axi_stream)-1 downto 0); variable got_tlast : std_logic; - variable got_tkeep : std_logic_vector(data_length(axi_stream)/8-1 downto 0); - variable got_tstrb : std_logic_vector(data_length(axi_stream)/8-1 downto 0); + variable got_tkeep : std_logic_vector(keep_strb_length(axi_stream)-1 downto 0); + variable got_tstrb : std_logic_vector(keep_strb_length(axi_stream)-1 downto 0); variable got_tid : std_logic_vector(id_length(axi_stream)-1 downto 0); variable got_tdest : std_logic_vector(dest_length(axi_stream)-1 downto 0); variable got_tuser : std_logic_vector(user_length(axi_stream)-1 downto 0); @@ -746,7 +770,8 @@ package body axi_stream_pkg is mismatch := false; for idx in got_tkeep'range loop if got_tkeep(idx) and got_tstrb(idx) then - mismatch := got_tdata(8 * idx + 7 downto 8 * idx) /= expected_normalized(8 * idx + 7 downto 8 * idx); + mismatch := got_tdata(minimum(8 * idx + 7, got_tdata'left) downto 8 * idx) /= + expected_normalized(minimum(8 * idx + 7, got_tdata'left) downto 8 * idx); exit when mismatch; end if; end loop; diff --git a/vunit/vhdl/verification_components/src/axi_stream_protocol_checker.vhd b/vunit/vhdl/verification_components/src/axi_stream_protocol_checker.vhd index 9c3c05ed9..3905b4851 100644 --- a/vunit/vhdl/verification_components/src/axi_stream_protocol_checker.vhd +++ b/vunit/vhdl/verification_components/src/axi_stream_protocol_checker.vhd @@ -30,8 +30,8 @@ entity axi_stream_protocol_checker is tready : in std_logic := '1'; tdata : in std_logic_vector(data_length(protocol_checker) - 1 downto 0); tlast : in std_logic := '1'; - tkeep : in std_logic_vector(data_length(protocol_checker)/8-1 downto 0) := (others => '1'); - tstrb : in std_logic_vector(data_length(protocol_checker)/8-1 downto 0) := (others => 'U'); + tkeep : in std_logic_vector(keep_strb_length(protocol_checker)-1 downto 0) := (others => '1'); + tstrb : in std_logic_vector(keep_strb_length(protocol_checker)-1 downto 0) := (others => 'U'); tid : in std_logic_vector(id_length(protocol_checker)-1 downto 0) := (others => '0'); tdest : in std_logic_vector(dest_length(protocol_checker)-1 downto 0) := (others => '0'); tuser : in std_logic_vector(user_length(protocol_checker)-1 downto 0) := (others => '0') @@ -85,7 +85,7 @@ architecture a of axi_stream_protocol_checker is ret := data; for i in keep'range loop if keep(i) = '0' or strb(i) = '0' then - ret(i*8+7 downto i*8) := (others => '0'); + ret(minimum(i*8+7, ret'left) downto i*8) := (others => '0'); end if; end loop; return ret; diff --git a/vunit/vhdl/verification_components/src/axi_stream_slave.vhd b/vunit/vhdl/verification_components/src/axi_stream_slave.vhd index fcb381649..5a0a43670 100644 --- a/vunit/vhdl/verification_components/src/axi_stream_slave.vhd +++ b/vunit/vhdl/verification_components/src/axi_stream_slave.vhd @@ -34,8 +34,8 @@ entity axi_stream_slave is tready : out std_logic := '0'; tdata : in std_logic_vector(data_length(slave)-1 downto 0); tlast : in std_logic := '1'; - tkeep : in std_logic_vector(data_length(slave)/8-1 downto 0) := (others => '1'); - tstrb : in std_logic_vector(data_length(slave)/8-1 downto 0) := (others => 'U'); + tkeep : in std_logic_vector(keep_strb_length(slave)-1 downto 0) := (others => '1'); + tstrb : in std_logic_vector(keep_strb_length(slave)-1 downto 0) := (others => 'U'); tid : in std_logic_vector(id_length(slave)-1 downto 0) := (others => '0'); tdest : in std_logic_vector(dest_length(slave)-1 downto 0) := (others => '0'); tuser : in std_logic_vector(user_length(slave)-1 downto 0) := (others => '0') @@ -146,7 +146,8 @@ begin mismatch := false; for idx in tkeep'range loop if tkeep(idx) and tstrb_resolved(idx) then - mismatch := tdata(8 * idx + 7 downto 8 * idx) /= expected_tdata(8 * idx + 7 downto 8 * idx); + mismatch := tdata(minimum(8 * idx + 7, tdata'left) downto 8 * idx) /= + expected_tdata(minimum(8 * idx + 7, tdata'left) downto 8 * idx); exit when mismatch; end if; end loop; diff --git a/vunit/vhdl/verification_components/test/tb_axi_stream.vhd b/vunit/vhdl/verification_components/test/tb_axi_stream.vhd index 899aee853..536c1b86d 100644 --- a/vunit/vhdl/verification_components/test/tb_axi_stream.vhd +++ b/vunit/vhdl/verification_components/test/tb_axi_stream.vhd @@ -23,7 +23,7 @@ use osvvm.RandomPkg.all; entity tb_axi_stream is generic( runner_cfg : string; - g_data_length : positive := 8; + g_data_length : natural := 8; g_id_length : natural := 8; g_dest_length : natural := 8; g_user_length : natural := 8; @@ -75,8 +75,8 @@ architecture a of tb_axi_stream is signal tready : std_logic; signal tdata : std_logic_vector(data_length(slave_axi_stream)-1 downto 0); signal tlast : std_logic; - signal tkeep, tkeep_from_master : std_logic_vector(data_length(slave_axi_stream)/8-1 downto 0); - signal tstrb, tstrb_from_master : std_logic_vector(data_length(slave_axi_stream)/8-1 downto 0); + signal tkeep, tkeep_from_master : std_logic_vector(keep_strb_length(slave_axi_stream)-1 downto 0); + signal tstrb, tstrb_from_master : std_logic_vector(keep_strb_length(slave_axi_stream)-1 downto 0); signal tid : std_logic_vector(id_length(slave_axi_stream)-1 downto 0); signal tdest : std_logic_vector(dest_length(slave_axi_stream)-1 downto 0); signal tuser : std_logic_vector(user_length(slave_axi_stream)-1 downto 0); @@ -447,15 +447,21 @@ begin check_axi_stream(net, slave_axi_stream, not data, tlast => '0', tkeep => not keep, tstrb => not strb, tid => id, tdest => dest, tuser => user, msg => "checking axi stream"); - check_log(mocklogger, "TDATA mismatch, checking axi stream - Got " & - to_nibble_string(data) & " (" & to_string(to_integer(data)) & "). Expected " & - to_nibble_string(not data) & " (" & to_string(to_integer(not data)) & ").", error); - check_log(mocklogger, "TKEEP mismatch, checking axi stream - Got " & - to_nibble_string(keep) & " (" & to_string(to_integer(keep)) & "). Expected " & - to_nibble_string(not keep) & " (" & to_string(to_integer(not keep)) & ").", error); - check_log(mocklogger, "TSTRB mismatch, checking axi stream - Got " & - to_nibble_string(strb) & " (" & to_string(to_integer(strb)) & "). Expected " & - to_nibble_string(not strb) & " (" & to_string(to_integer(not strb)) & ").", error); + if data'length > 0 then + check_log(mocklogger, "TDATA mismatch, checking axi stream - Got " & + to_nibble_string(data) & " (" & to_string(to_integer(data)) & "). Expected " & + to_nibble_string(not data) & " (" & to_string(to_integer(not data)) & ").", error); + end if; + if tkeep'length > 0 then + check_log(mocklogger, "TKEEP mismatch, checking axi stream - Got " & + to_nibble_string(keep) & " (" & to_string(to_integer(keep)) & "). Expected " & + to_nibble_string(not keep) & " (" & to_string(to_integer(not keep)) & ").", error); + end if; + if tstrb'length > 0 then + check_log(mocklogger, "TSTRB mismatch, checking axi stream - Got " & + to_nibble_string(strb) & " (" & to_string(to_integer(strb)) & "). Expected " & + to_nibble_string(not strb) & " (" & to_string(to_integer(not strb)) & ").", error); + end if; check_log(mocklogger, "TLAST mismatch, checking axi stream - Got 1. Expected 0.", error); if id'length > 0 then check_log(mocklogger, "TID mismatch, checking axi stream - Got 0010_0010 (34). Expected 0010_0011 (35).", error); @@ -587,15 +593,21 @@ begin wait until rising_edge(aclk) and tvalid = '1'; wait for 1 ps; - check_log(mocklogger, "TDATA mismatch, check non-blocking - Got " & - to_nibble_string(data) & " (" & to_string(to_integer(data)) & "). Expected " & - to_nibble_string(not data) & " (" & to_string(to_integer(not data)) & ").", error); - check_log(mocklogger, "TKEEP mismatch, check non-blocking - Got " & - to_nibble_string(keep) & " (" & to_string(to_integer(keep)) & "). Expected " & - to_nibble_string(not keep) & " (" & to_string(to_integer(not keep)) & ").", error); - check_log(mocklogger, "TSTRB mismatch, check non-blocking - Got " & - to_nibble_string(keep) & " (" & to_string(to_integer(keep)) & "). Expected " & - to_nibble_string(not keep) & " (" & to_string(to_integer(not keep)) & ").", error); + if data'length > 0 then + check_log(mocklogger, "TDATA mismatch, check non-blocking - Got " & + to_nibble_string(data) & " (" & to_string(to_integer(data)) & "). Expected " & + to_nibble_string(not data) & " (" & to_string(to_integer(not data)) & ").", error); + end if; + if tkeep'length > 0 then + check_log(mocklogger, "TKEEP mismatch, check non-blocking - Got " & + to_nibble_string(keep) & " (" & to_string(to_integer(keep)) & "). Expected " & + to_nibble_string(not keep) & " (" & to_string(to_integer(not keep)) & ").", error); + end if; + if tstrb'length > 0 then + check_log(mocklogger, "TSTRB mismatch, check non-blocking - Got " & + to_nibble_string(strb) & " (" & to_string(to_integer(strb)) & "). Expected " & + to_nibble_string(not strb) & " (" & to_string(to_integer(not strb)) & ").", error); + end if; check_log(mocklogger, "TLAST mismatch, check non-blocking - Got 1. Expected 0.", error); if id'length > 0 then check_log(mocklogger, "TID mismatch, check non-blocking - Got 0010_1010 (42). Expected 0010_1100 (44).", error); @@ -685,7 +697,7 @@ begin tkeep <= tkeep_from_master when connected_tkeep else (others => '1'); tstrb <= tstrb_from_master when connected_tstrb else (others => 'U'); - not_valid <= not tvalid; + not_valid <= not tvalid when g_data_length > 0 else '0'; not_valid_data <= '1' when is_x(tdata) else '0'; check_true(aclk, not_valid, not_valid_data, "Invalid data not X"); diff --git a/vunit/vhdl/verification_components/test/tb_axi_stream_protocol_checker.vhd b/vunit/vhdl/verification_components/test/tb_axi_stream_protocol_checker.vhd index b62107601..5437f9fcf 100644 --- a/vunit/vhdl/verification_components/test/tb_axi_stream_protocol_checker.vhd +++ b/vunit/vhdl/verification_components/test/tb_axi_stream_protocol_checker.vhd @@ -35,8 +35,8 @@ architecture a of tb_axi_stream_protocol_checker is signal tlast : std_logic := '1'; signal tdest : std_logic_vector(dest_length - 1 downto 0) := (others => '0'); signal tid : std_logic_vector(id_length - 1 downto 0) := (others => '0'); - signal tstrb : std_logic_vector(data_length/8 - 1 downto 0) := (others => '0'); - signal tkeep : std_logic_vector(data_length/8 - 1 downto 0) := (others => '0'); + signal tstrb : std_logic_vector((data_length + 7) / 8 - 1 downto 0) := (others => '0'); + signal tkeep : std_logic_vector((data_length + 7) / 8 - 1 downto 0) := (others => '0'); signal tuser : std_logic_vector(user_length - 1 downto 0) := (others => '0'); constant logger : logger_t := get_logger("protocol_checker");