Wishbone BFM #312

slaweksiluk · 2018-03-02T15:21:05Z

I think my implementations of Wishbone Slave and Master Bus Functional Models are ready to be reviewed. Currently only pipelined mode is supported. Read/Write cycles lengths can be 1 (Single Read/Write) or more (Block Read/Write).

Wishbone Slave BFM

It is wrapper for Vunit memory VC. Currently memory is internal variable, but it should rather be passed as generic parameter. Manual tb with generic test cases is provided.

Wishbone Master BFM

Receives bus write and bus read messages from main sequencer. Consecutive groups of bus write/reads messages are merged into block cycles. Cycle is finished when next message is the opposite action (e.g write command after read) or there is no more messages in actor inbox. Test bench with Master and Slave connected is provided.

I didn't want to add new calls to bus_maser_pkg. However adding calls like: bus_write_block and bus_read_block might be good idea.

Conflicts: vunit/vhdl/verification_components/src/wishbone_master.vhd vunit/vhdl/verification_components/test/tb_wishbone_master.vhd

Conflicts: vunit/vhdl/verification_components/test/tb_wishbone_master.gtkw

One wb test failed

Delay needed between write and read

Use Vunit message passing tracing instead

But need to fix memory write read values (currently checking is disabled in tb). Also need to perfrom block write and read back to back

It is in wishbone slave module in ack process when the third (last?) msg is processed.

Call receive is not valid for such a use case (multi process BFM)

To remove the need for delays in tb. Not yes simulated.

kraigher

I have reviewed the code an written down my suggestions. Let me know if you need any help. The big issues are:

Copyright
Wishbone slave needs to get the memory_t instance from the outside to be usable.

kraigher · 2018-03-03T10:48:39Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+-- This Source Code Form is subject to the terms of the Mozilla Public
+-- License, v. 2.0. If a copy of the MPL was not distributed with this file,
+-- You can obtain one at http://mozilla.org/MPL/2.0/.
+--


Copyright notice needs to give copyright to Lars Asplund like all other copyright notices.
It is fine if you mention your name to give you credit for the the work but copyright needs to be given to Lars for this to be merged.

OK, no problem I will leave Copyright as in other vunit sources. I will also add line:

-- Author: Slawomir Siluk slaweksiluk@gazeta.pl

kraigher · 2018-03-03T10:48:52Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+library ieee;
+use ieee.std_logic_1164.all;
+
+--use work.axi_pkg.all;


kraigher · 2018-03-03T10:49:01Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+--use work.axi_pkg.all;
+use work.queue_pkg.all;
+use work.bus_master_pkg.all;
+--use work.axi_private_pkg.all;


kraigher · 2018-03-03T10:49:25Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+    variable status : com_status_t;
+  begin
+      -- Cannot use receive, as it deletes the message
+      --receive(net, bus_handle.p_actor, request_msg);


You can use receive if you set request_msg := null_message before.

Do you mean:

request_msg := null_message receive(net, bus_handle.p_actor, request_msg)

I sholud still manually delete the message, right?

The receive procedure will delete the request_msg if it is not already null_message.
By setting it to null_message before you inhibit receive from deleting the message.
You take the responsibility to delete the message yourself when you are done using it.
Ideally you should set the request_msg to null_message right after you push it to a queue since that is the point where ownership is semantically handed over to the queue. The process that pops the queue takes ownership and should delete it once it is done with it.

kraigher · 2018-03-03T10:50:37Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+    end loop;
+  end process;
+
+  acknowladge : process


misspelled acknowledge

kraigher · 2018-03-03T11:00:10Z

vunit/vhdl/verification_components/test/tb_wishbone_master.vhd

+-- This Source Code Form is subject to the terms of the Mozilla Public
+-- License, v. 2.0. If a copy of the MPL was not distributed with this file,
+-- You can obtain one at http://mozilla.org/MPL/2.0/.
+--


Copyright needs to be given to Lars. You can keep your name for crediting the contribution.

kraigher · 2018-03-03T11:01:09Z

vunit/vhdl/verification_components/test/tb_wishbone_master.vhd

+  show(tb_logger, display_handler, verbose);
+  show(default_logger, display_handler, verbose);
+  show(master_logger, display_handler, verbose);
+  show(com_logger, display_handler, verbose);


All these calls (except test_runnner_watchdog) should be after test_runner_setup in a process. I do not recommend making these calls as concurrent statements.

kraigher · 2018-03-03T11:01:44Z

vunit/vhdl/verification_components/test/tb_wishbone_slave.vhd

+-- This Source Code Form is subject to the terms of the Mozilla Public
+-- License, v. 2.0. If a copy of the MPL was not distributed with this file,
+-- You can obtain one at http://mozilla.org/MPL/2.0/.
+--


Copyright needs to be given to Lars, you can keep your names for crediting the work but have to give up copyright.

kraigher · 2018-03-03T11:02:45Z

vunit/vhdl/verification_components/test/tb_wishbone_slave.vhd

+  set_format(display_handler, verbose, true);
+  show(tb_logger, display_handler, verbose);
+  show(default_logger, display_handler, verbose);
+  show(com_logger, display_handler, verbose);


These calls (except test_runner_watchdog) should not be concurrent and should be located after test_runner_setup

kraigher · 2018-03-03T11:06:42Z

vunit/vhdl/verification_components/src/wishbone_slave.vhd

+    variable msg_type : msg_type_t;
+    variable data : std_logic_vector(dat_i'range);
+    variable addr : natural;
+    variable memory : memory_t := new_memory;


When using a memory_t from a VC you should convert it by using to_vc_interface. This enables permission checks on all read/write accesses which is not enabled by default as the test_bench should read and write freely but the VC should respect the permission settings.

I'm going to pass memory as entity generic. Should I still use to_vc_interface in that case? I will look for some examples in existing vunit VC's.

to_vc_interface should be used privately within the VC to create a new memory object which is identical to the one that was passed into the VC by the user. This has the effect of enabling read/write permission for all read/write operations to the memory when using the new value returned by to_vc_interface. This is to make it easier to support the two use cases of reading/writing memory from the test bench without permission checks and having axi_slave etc. perform memory operations with permission checks.

Also ideally you should create a wishbone_slave_t record type that is created with a new_wishbone_slave function like axi_slave. The new_wishbone_slave can take a memory_t as an argument and maybe also a logger_t. Internally in new_wishbone_slave an actor_t can be created that allows you to define command functions to the wishbone slave such as enable_random_stall(net : net_t, slave : wishbone_slave_t) or set_random_stall_probability(net : net_t, slave : wishbone_slave_t, probability : real);

kraigher · 2018-03-03T14:25:44Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+      get_message(net, bus_handle.p_actor, request_msg);
+      msg_type := message_type(request_msg);
+      if msg_type = bus_read_msg then
+        push(rd_request_queue, request_msg);


set request_msg to null_message after push since ownership was transferred into the queue.

kraigher · 2018-03-03T14:25:54Z

vunit/vhdl/verification_components/src/wishbone_master.vhd

+      if msg_type = bus_read_msg then
+        push(rd_request_queue, request_msg);
+      elsif msg_type = bus_write_msg then
+        push(wr_request_queue, request_msg);


set request_msg to null_message after push since ownership was transferred into the queue.

Memory is now external to slave

To not to prevent from multi master instances.

slaweksiluk · 2018-03-06T12:01:37Z

Ok, thank you for comments. I applied most of them, excluding random parameters set via msg. It is definitely better solution, however I need to have some deeper look at axi slaves to implement it.

kraigher · 2018-03-06T18:21:21Z

Regarding setting random parameters via message. The first you could to is just add them as fields in the wishbone_slave_t record and then rename them to initial_rand_stall_probability. That way it is forward compatible to a future where the rand_stall_probability can be set at run-time as well. In the axi_slaves we wrap all generics in the axi_slave_t record.

kraigher · 2018-03-06T19:06:35Z

vunit/vhdl/verification_components/src/wishbone_slave.vhd

+  generic (
+    wishbone_slave : wishbone_slave_t;
+    max_ack_dly : natural := 0;
+    rand_stall  : boolean := false


Rename max_ack_dly and rand_stall to initial_max_ack_delay and initial_rand_stall_probability : real and move them into wishbone_slave_t to make it forward compatible with changing them at runtime and more similar to axi_slave_t.

kraigher · 2018-03-06T19:08:10Z

vunit/vhdl/verification_components/src/wishbone_pkg.vhd

+package body wishbone_pkg is
+
+  impure function new_wishbone_slave(
+    memory : memory_t;


Add max_ack_delay and random_stall_probability parameters here and to wishbone_slave_t to make it similar to axi_slave_t and forward compatible with setting them at run-time.

Ok, I will go that way. I will also change both parameters to be probability of having value 1, E.g:

init_stall_high_probability : real := 0.0; init_ack_high_probability : real := 1.0;

It seems better defined this way. I also consider adding random stb signal stimulation option to master VC.

slaweksiluk · 2018-03-13T14:04:53Z

I changed both parameters to be probability 0.0 to 1.0 range. Also I moved them to wishbone_slave_t record. Still need to add support for runtime probability changes.

kraigher · 2018-03-13T17:10:17Z

Great work. I am ready to accept this pull request. You just need to fix the broken checks found by Travis CI: https://travis-ci.org/VUnit/vunit/jobs/352852061

I think the only problem is the Python PEP8 code style checks that are broken by the changes in the verification_components run.py file.

You can run this check locally using pytest vunit/test/lint/test_pep8.py.
First you need to install pytest and pep8 using pip install pytest pep8

kraigher · 2018-03-21T19:14:15Z

@slaweksiluk Do you need any help finishing this up?

slaweksiluk · 2018-03-22T07:14:09Z

I fixed coding style errors. I was using designed bfm's in my project to verify it in real conditions. Wishbone slave looks good, but I haven't got time to try wishbone master. Moreover I remember I saw small bug in wishbone master regarding cyc signal one clock cycle late termination. However it only occurs at the end of block cycle, so it is not harmful. I will create issues for it when I trace it. I think you should mentioned somewhere in documentation that wishbone bfms are still in testing.

kraigher · 2018-03-22T18:13:57Z

We write in the documentation that the entire verification component library is in beta so maybe the maturity of wishbone needs no additional disclaimer.

slaweksiluk added 29 commits February 26, 2018 11:33

Move ack handler in wb slave tb model to concurrent process

d3bb358

Conflicts: vunit/vhdl/verification_components/src/wishbone_master.vhd vunit/vhdl/verification_components/test/tb_wishbone_master.vhd

Pass block write (two cycles)

1d627fa

Pass block read double cycle test

e976028

Fix single write

99d11d0

Pass sim in both read write block cycles

5a380ee

Conflicts: vunit/vhdl/verification_components/test/tb_wishbone_master.gtkw

Compile with new vunit

f8c8b65

One wb test failed

Compile wishbone slave

09bb3d6

Pass single write read cycle

b68a3fc

Delay needed between write and read

Remove redundant log prints

a8fee8c

Use Vunit message passing tracing instead

Little changes to msg names

5dd39a4

Pass sim

b423930

But need to fix memory write read values (currently checking is disabled in tb). Also need to perfrom block write and read back to back

Add todo

88ee7f3

Find the origin of pop from empty queue error

1b61e10

It is in wishbone slave module in ack process when the third (last?) msg is processed.

Add wishbone slave tb template

7255f6a

Pass simple (manual) tb for wishbone slave

1a04fe0

Merge remote-tracking branch 'origin/master' into wishbone

08d0dd1

Fix Pop from empty queue error

0dddf5b

Call receive is not valid for such a use case (multi process BFM)

Delete request message

23feca8

Clean code

d2f6fb1

Remove to long delays

ff67efb

Clean todos

44533c4

Split request to write and read queue

ba15623

To remove the need for delays in tb. Not yes simulated.

Fix cycle termination

ef60350

Example random ack delay in wr path

a55fad0

Generate test for wishbone slave

90319c1

Random stall for wb slave test

4ecf82a

Update todos

aa19622

Random test for slave and master connected

c561515

Clean code

686acd1

kraigher reviewed Mar 3, 2018

View reviewed changes

slaweksiluk added 5 commits March 6, 2018 11:16

Fix copyright and code clean

6bc3c93

Change forwarded msg handling

6f22420

Move concurrent statements to process

3feeaca

Add wishbone pkg with wishbone slave record

8326364

Memory is now external to slave

Remove spcific name for ack actor

5170f97

To not to prevent from multi master instances.

kraigher reviewed Mar 6, 2018

View reviewed changes

slaweksiluk added 5 commits March 13, 2018 13:37

Use encoded generics to decode ack high probability

5c94377

Rand ack stim for reading

0b600d7

Move stall probability to tb cfg

e70626c

Add encoded generics to master tb

13c333e

Merge remote-tracking branch 'origin/master' into wishbone

0a5aa88

Apply coding style

2afd7b1

kraigher merged commit f7542ea into VUnit:master Mar 22, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wishbone BFM #312

Wishbone BFM #312

slaweksiluk commented Mar 2, 2018

kraigher left a comment

kraigher Mar 3, 2018

slaweksiluk Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

slaweksiluk Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018

slaweksiluk Mar 3, 2018

kraigher Mar 3, 2018

kraigher Mar 3, 2018 •

edited

Loading

kraigher Mar 3, 2018

kraigher Mar 3, 2018

slaweksiluk commented Mar 6, 2018 •

edited

Loading

kraigher commented Mar 6, 2018

kraigher Mar 6, 2018

kraigher Mar 6, 2018

slaweksiluk Mar 10, 2018

slaweksiluk commented Mar 13, 2018

kraigher commented Mar 13, 2018

kraigher commented Mar 21, 2018

slaweksiluk commented Mar 22, 2018

kraigher commented Mar 22, 2018

Wishbone BFM #312

Wishbone BFM #312

Conversation

slaweksiluk commented Mar 2, 2018

Wishbone Slave BFM

Wishbone Master BFM

kraigher left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraigher Mar 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slaweksiluk commented Mar 6, 2018 • edited Loading

kraigher commented Mar 6, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slaweksiluk commented Mar 13, 2018

kraigher commented Mar 13, 2018

kraigher commented Mar 21, 2018

slaweksiluk commented Mar 22, 2018

kraigher commented Mar 22, 2018

kraigher Mar 3, 2018 •

edited

Loading

slaweksiluk commented Mar 6, 2018 •

edited

Loading