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

AXI-Stream Formal Properties #3

Open
wants to merge 38 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c70a583
First pass at AXI Stream formal properties
rlee287 Dec 5, 2020
ce90a76
Fix README typo
rlee287 Dec 6, 2020
b60c3c5
Add the property that TVALID cannot be deasserted until transfer
rlee287 Dec 6, 2020
0871812
Add note about lack of requirement for no combinational paths to AXIS
rlee287 Dec 9, 2020
985e786
Remove .keep from property_sets now that the directory is populated
rlee287 Dec 9, 2020
40c1b6c
Fix some reset related issues with AXIS properties
rlee287 Dec 9, 2020
06330b2
Fix missing negation of reset in an AXIS slave property
rlee287 Dec 10, 2020
34408c1
Add more checks for past_valid where they are needed
rlee287 Dec 10, 2020
f8921ca
Use nonblocking assignment for past_valid value
rlee287 Dec 11, 2020
43984c5
Fix reset things and use preprocessor to encourage code reuse
rlee287 Dec 16, 2020
cc16378
Check for positive width parameter before assuming/asserting stall st…
rlee287 Dec 19, 2020
7692bac
Adjust reset handling in AXI-Stream properties
rlee287 Dec 22, 2020
6b40ac0
Create a utility for assuming async resets are synchronously released
rlee287 Dec 22, 2020
8579c5c
Allow non-data bytes to vary when stream is stalled
rlee287 Dec 26, 2020
8b36055
Add script to compare AXI-Stream properties and ensure they match cor…
rlee287 Dec 27, 2020
0c478fa
Add README state for "complete but untested" and update statues
rlee287 Dec 27, 2020
ece2d1a
Fix README.md typo
rlee287 Dec 27, 2020
e19f039
Label the AXI-Stream properties with the version and issue of the spec
rlee287 Dec 27, 2020
bdd5b19
Bypass default input tready value when linting with Verilator
rlee287 Dec 27, 2020
674c736
Revert "Allow non-data bytes to vary when stream is stalled"
rlee287 Dec 27, 2020
5657264
Move up definition of TX_ASSERT (so that diff reference can be update…
rlee287 Dec 27, 2020
b1256c8
Move up Section 3.1.5 note and add keep_width for when byte_width=0
rlee287 Dec 27, 2020
22ba3fc
Account for the possibility of byte_width=0 in stall $stable checks
rlee287 Dec 27, 2020
870ac0b
Print out the diff output if the check_differences script fails
rlee287 Dec 29, 2020
0cebe36
Remove extraneous colon from Optional TREADY comment
rlee287 Dec 29, 2020
a4753bf
Add a property to ensure TVALID is raised "in a timely manner"
rlee287 Dec 29, 2020
149bcac
Reset handling should be fixed now...
rlee287 Jan 1, 2021
55f635a
Update diff file for AXI-Stream
rlee287 Jan 1, 2021
dc45ddf
Using not_in_reset requires past_valid
rlee287 Jan 2, 2021
447b415
Assert that valid is raised with backpressure only when no reset happens
rlee287 Jan 2, 2021
9f3b8f4
Update TODO notes in AXI-Stream properties
rlee287 Jan 2, 2021
0eb4261
Update comparison diff for two property sets
rlee287 Jan 2, 2021
aafee53
Merge branch 'main' into axi_stream
rlee287 Jan 3, 2021
e759e47
Rename clk and resetn to aclk and aresetn
rlee287 Jan 3, 2021
9ea22fa
Add TODO note about assuming master readiness in slave properties
rlee287 Jan 4, 2021
1bae26b
Allow valid-before-ready check to be disabled by parameters
rlee287 Jan 7, 2021
d5cd1e6
Add a .gitignore for SymbiYosys and note naming convention in CONTRIB…
rlee287 Jan 15, 2021
bf50f86
Update CONTRIBUTING.md
rlee287 Feb 10, 2021
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
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
# Output folders from SymbiYosys (by task name convention)
*_bmc/
*_cover/
*_prove/
3 changes: 2 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,6 @@ Before submitting a pull request, please note the following points:

* Before starting work, make sure an issue has been created first, and assign yourself (or leave some other marker in the issue) to indicate that you are working on the issue. This reduces duplication of work.
* If you are submitting a bugfix, link the original issue(s) that you are aiming to fix. If you are adding new features, link the issue(s) in which the features were discussed.
* Always use SymbiYosys tasks (even when there is only one), and choose task names that end with `_bmc`, `_cover`, or `_prove`. This allows the SymbiYosys output directories to be picked up by the `.gitignore` file.
* Public-facing interfaces to a module follow the naming conventions of the standard they are based on, but names for internal items do not have to. An example of this is that AXI-Stream properties use the terms "master" and "slave" in their name (as these are the names used in the specification), but internal signals should use alternative labels such as "tx"/"send" and "rx"/"receive".
* Pull requests are not merged until they have been reviewed and approved by at least one other person.
* Pull requests into `main` are not merged until they have been reviewed and approved by at least one other person. Commits to `main` should only be used to update the status table or to update meta documents like `CONTRIBUTING.md` (though such changes should be discussed with the community first).
9 changes: 5 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,17 @@ Key to emojis:

* :x: : Item is not started
* :heavy_minus_sign: : Item is in progress
* :heavy_check_mark: : Item is completed
* :heavy_plus_sign: : Item is completed but has not been tested sufficiently
* :heavy_check_mark: : Item is completed and has been tested sufficiently

All entries may contain an issue number tracking progress on the item.

Item | Wishbone 4 Classic | Wishbone 4 Pipelined | AXI4-Stream | AXI4-Lite | AXI4
---- | ------------------ | -------------------- | ----------- | --------- | ----
Formal Property Set (SVA) | :x: | :x: | :x: | :x: | :x:
Formal Property Set (PSL) | :x: | :x: | :x: | :x: | :x:
Formal Property Set (SVA) | :x: | :x: | :heavy_plus_sign: (#1, #3, #4) | :x: | :x:
Formal Property Set (PSL) | :x: | :x: | :heavy_minus_sign: (#1, awaiting #3 merge) | :x: | :x:
Bus Summary | :x: | :x: | :x: | :x: | :x:
Example Cores (expand list later) | :x: | :x: | :x: | :x: | :x:
Example Cores (expand list later) | :x: | :x: | :x: (#5) | :x: | :x:

## Bus Standards

Expand Down
Empty file removed property_sets/.keep
Empty file.
202 changes: 202 additions & 0 deletions property_sets/axi_stream/axi_stream_master_monitor.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,202 @@
`default_nettype none

/*
* A set of formal properties for the master port of AXI-Stream.
* AXI4-Stream, Version 1.0, Issue A
*/

/*
* Section 2.1 Signal list
* Section 3.1 Default value signaling
* Notes on defaults for specific signals in port list below
*/
module axi_stream_master_monitor #(
parameter byte_width = 4,
parameter id_width = 0,
parameter dest_width = 0,
parameter user_width = 0,
// Section 3.1.5 Optional TDATA
// no TDATA -> no TSTRB (but TKEEP can still exist)
// TKEEP width when byte_width=0
parameter keep_width = 0,

// TODO: properties untested for multiclock environments
parameter USE_ASYNC_RESET = 1'b0,
// Section 2.2.1 Handshake process
// Master cannot wait on TREADY to signal TVALID
// Cycle count for TREADY to be kept low initially, after which TVALID should be high
// Set to 0 to disable this check
parameter MAX_DELAY_TREADY_NO_TVALID = 16
) (
input wire aclk,
input wire aresetn,

input wire tvalid,
// Section 3.1.1 Optional TREADY
// Having a tready port is recommended
`ifndef VERILATOR
input wire tready = 1'b1,
`else
input wire tready,
`endif

input wire [(8*byte_width-1):0] tdata,
// Section 3.1.2 Optional TKEEP and TSTRB
// TODO: fix optional value declarations
input wire [(byte_width-1):0] tstrb,// = tkeep,
input wire [(byte_width>0 ? (byte_width-1) : keep_width):0] tkeep,// = {(byte_width-1){1'b1}},

// Section 3.1.3 Optional TLAST
// Recommended default TLAST value situation-dependent
input wire tlast,

// Section 3.1.4 Optional TID, TDEST, and TUSER
// Signals omitted by setting widths to 0
input wire [(id_width-1):0] tid,
input wire [(dest_width-1):0] tdest,
input wire [(user_width-1):0] tuser

// TODO add output signals for byte counts, etc.
);
`define TX_ASSERT assert
`define NOT_RESET_FOR_REGISTERED ($past(aresetn) && (!USE_ASYNC_RESET || aresetn))

reg past_valid = 1'b0;
always @(posedge aclk)
past_valid <= 1'b1;

reg not_in_reset;

/*
* If in async mode, reset is registered by async flip flop
* If in sync mode, reset is registered by sync flip flop
*
* WARNING: Only use this in combinational blocks
* Use in registered blocks makes this incorrect by a clock cycle delay
* Use `NOT_RESET_FOR_REGISTERED for registered blocks
*/
generate
if (USE_ASYNC_RESET)
always @(posedge aclk or negedge aresetn)
begin
if (!aresetn)
not_in_reset <= 1'b0;
else
not_in_reset <= aresetn;
end
else
always @(posedge aclk)
begin
not_in_reset <= aresetn;
end
endgenerate
// TODO reset signal generation is quite messy right now

// Section 2.2.1 Handshake process

// Once TVALID is asserted it must be held until TVALID && TREADY
always @(posedge aclk)
begin
// Write this as (TVALID falls implies previous data transfer or reset)
if (past_valid && $fell(tvalid))
begin
`TX_ASSERT($past(tvalid && tready) || !`NOT_RESET_FOR_REGISTERED);
end
end

// Master cannot wait on TREADY to signal TVALID
// Property below modified from @awygle's suggestion
generate
if (MAX_DELAY_TREADY_NO_TVALID < 4096 && MAX_DELAY_TREADY_NO_TVALID > 0)
begin
reg f_ever_ready = 1'b0;
reg [11:0] delay_counter = 0;
always @(posedge aclk)
begin
if (!aresetn)
f_ever_ready <= 1'b0;
else if (tready)
f_ever_ready <= 1'b1;
end
always @(posedge aclk)
begin
// Else if check to prevent weirdness with overflow
// 4096 cycles should be plenty enough though
if (!aresetn)
delay_counter <= 0;
else if (delay_counter < 12'hfff)
delay_counter <= delay_counter + 1;
end

always @(*)
begin
// Helper assert to justify lack of past_valid for below property
if (delay_counter > 0)
assert(past_valid);
end

// Check that TVALID has been raised even if TREADY has been held low
// In isolation, this checks the required property
// In a full system (where TREADY may be tied high) a false hypothesis allows the property to pass vacuously
// TODO: optional assumption for slave properties?
always @(*)
begin
if (delay_counter == MAX_DELAY_TREADY_NO_TVALID
&& !f_ever_ready && not_in_reset)
assert(tvalid);
end
end
else if (MAX_DELAY_TREADY_NO_TVALID != 0)
$error("MAX_DELAY_TREADY_NO_TVALID too large");
endgenerate

// When TVALID && !TREADY, data signals must be stable
always @(posedge aclk)
begin
if (past_valid && `NOT_RESET_FOR_REGISTERED && $past(tvalid && !tready))
begin
if (byte_width > 0)
begin
`TX_ASSERT($stable(tdata));
`TX_ASSERT($stable(tstrb));
end
if (byte_width > 0 || keep_width > 0)
`TX_ASSERT($stable(tkeep));

`TX_ASSERT($stable(tlast));

if (id_width > 0)
`TX_ASSERT($stable(tid));
if (dest_width > 0)
`TX_ASSERT($stable(tdest));
if (user_width > 0)
`TX_ASSERT($stable(tuser));
end
end

// Section 2.7.1 Clock
// Unlike regular AXI there is no requirement for no combinational paths

// Section 2.7.2 Reset
// "A master interface must only begin driving TVALID at a rising ACLK edge following a rising edge at which ARESETn is asserted HIGH."
// This timing is handled by the definition of not_in_reset above
always @(*)
begin
if (past_valid && !not_in_reset)
begin
`TX_ASSERT(!tvalid);
end
end

// Section 2.4.3 TKEEP and TSTRB combinations
// TSTRB can be asserted only if TKEEP is asserted
always @(*)
begin
if (tvalid)
begin
`TX_ASSERT(!(~tkeep & tstrb));
end
end
endmodule
`undef TX_ASSERT
`undef NOT_RESET_FOR_REGISTERED
153 changes: 153 additions & 0 deletions property_sets/axi_stream/axi_stream_slave_monitor.v
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
`default_nettype none

/*
* A set of formal properties for the slave port of AXI-Stream.
* AXI4-Stream, Version 1.0, Issue A
*/

/*
* Section 2.1 Signal list
* Section 3.1 Default value signaling
* Notes on defaults for specific signals in port list below
*/
module axi_stream_slave_monitor #(
parameter byte_width = 4,
parameter id_width = 0,
parameter dest_width = 0,
parameter user_width = 0,
// Section 3.1.5 Optional TDATA
// no TDATA -> no TSTRB (but TKEEP can still exist)
// TKEEP width when byte_width=0
parameter keep_width = 0,

// TODO: properties untested for multiclock environments
parameter USE_ASYNC_RESET = 1'b0
) (
input wire aclk,
input wire aresetn,

input wire tvalid,
// Section 3.1.1 Optional TREADY
// Having a tready port is recommended
`ifndef VERILATOR
input wire tready = 1'b1,
`else
input wire tready,
`endif

input wire [(8*byte_width-1):0] tdata,
// Section 3.1.2 Optional TKEEP and TSTRB
// TODO: fix optional value declarations
input wire [(byte_width-1):0] tstrb,// = tkeep,
input wire [(byte_width>0 ? (byte_width-1) : keep_width):0] tkeep,// = {(byte_width-1){1'b1}},

// Section 3.1.3 Optional TLAST
// Recommended default TLAST value situation-dependent
input wire tlast,

// Section 3.1.4 Optional TID, TDEST, and TUSER
// Signals omitted by setting widths to 0
input wire [(id_width-1):0] tid,
input wire [(dest_width-1):0] tdest,
input wire [(user_width-1):0] tuser

// TODO add output signals for byte counts, etc.
);
`define TX_ASSERT assume
`define NOT_RESET_FOR_REGISTERED ($past(aresetn) && (!USE_ASYNC_RESET || aresetn))

reg past_valid = 1'b0;
always @(posedge aclk)
past_valid <= 1'b1;

reg not_in_reset;

/*
* If in async mode, reset is registered by async flip flop
* If in sync mode, reset is registered by sync flip flop
*
* WARNING: Only use this in combinational blocks
* Use in registered blocks makes this incorrect by a clock cycle delay
* Use `NOT_RESET_FOR_REGISTERED for registered blocks
*/
generate
if (USE_ASYNC_RESET)
always @(posedge aclk or negedge aresetn)
begin
if (!aresetn)
not_in_reset <= 1'b0;
else
not_in_reset <= aresetn;
end
else
always @(posedge aclk)
begin
not_in_reset <= aresetn;
end
endgenerate
// TODO reset signal generation is quite messy right now

// Section 2.2.1 Handshake process

// Once TVALID is asserted it must be held until TVALID && TREADY
always @(posedge aclk)
begin
// Write this as (TVALID falls implies previous data transfer or reset)
if (past_valid && $fell(tvalid))
begin
`TX_ASSERT($past(tvalid && tready) || !`NOT_RESET_FOR_REGISTERED);
end
end

// Slave can wait on TVALID to signal TREADY

// When TVALID && !TREADY, data signals must be stable
always @(posedge aclk)
begin
if (past_valid && `NOT_RESET_FOR_REGISTERED && $past(tvalid && !tready))
begin
if (byte_width > 0)
begin
`TX_ASSERT($stable(tdata));
`TX_ASSERT($stable(tstrb));
end
if (byte_width > 0 || keep_width > 0)
`TX_ASSERT($stable(tkeep));

`TX_ASSERT($stable(tlast));

if (id_width > 0)
`TX_ASSERT($stable(tid));
if (dest_width > 0)
`TX_ASSERT($stable(tdest));
if (user_width > 0)
`TX_ASSERT($stable(tuser));
end
end

// Section 2.7.1 Clock
// Unlike regular AXI there is no requirement for no combinational paths

// Section 2.7.2 Reset
// "A master interface must only begin driving TVALID at a rising ACLK edge following a rising edge at which ARESETn is asserted HIGH."
// This timing is handled by the definition of not_in_reset above
always @(*)
begin
if (past_valid && !not_in_reset)
begin
`TX_ASSERT(!tvalid);
end
end

// Section 2.4.3 TKEEP and TSTRB combinations
// TSTRB can be asserted only if TKEEP is asserted
always @(*)
begin
if (tvalid)
begin
`TX_ASSERT(!(~tkeep & tstrb));
end
end
endmodule
`undef TX_ASSERT
`undef NOT_RESET_FOR_REGISTERED
Loading