search

VUnit / vunit

VUnit is a unit testing framework for VHDL/SystemVerilog
http://vunit.github.io/
Other
696 stars 250 forks source link

Add axi master #1017

Open DavidMartinPhios opened 1 month ago

DavidMartinPhios commented 1 month ago

Added axi master with equally powerful implementation as axi lite master has

LarsAsplund commented 1 month ago

Thanks for getting this started. I'm aware of several implementations over the years but non of them made it to open source. It so happens that I have my eyes on yet another implementation in progress that may or not make it to the public. Even if that doesn't become open source, I can keep track of what features it requires such that the VC released by VUnit can meet those as well.

I will get back when I have more info.

DavidMartinPhios commented 1 month ago

Thanks for getting this started. I'm aware of several implementations over the years but non of them made it to open source. It so happens that I have my eyes on yet another implementation in progress that may or not make it to the public. Even if that doesn't become open source, I can keep track of what features it requires such that the VC released by VUnit can meet those as well.

I will get back when I have more info.

As you might figured out already this implementation is not yet finished. I am still working on it. Basically I am now adding the functionallity to each single signal. At the moment I have deticated reserved time to implement this specific VC. It would be nice to know if the work will get obsolete some day or be replaced by another implementation. Otherwise I will keep going with the implementation. Hopefully with your early review / support.

@LarsAsplund Do you think that it is necessary to move the signals id, last, etc. to the bus_master record or is it ok to implement them around on a higher level as it is right now?

LarsAsplund commented 1 month ago

Since you've started and the other implementation may not make it to open source, I think we should continue on what you have. What I hope is to align on the required feature set such that you both can work on the open implementation in the end. I will come back and review a bit later,

DavidMartinPhios commented 1 month ago

Since you've started and the other implementation may not make it to open source, I think we should continue on what you have. What I hope is to align on the required feature set such that you both can work on the open implementation in the end. I will come back and review a bit later,

Thank you for the answer! I am currently working on the read burst behaviour. It is not finished yet but I think you could get an idea where the journey should go.

DavidMartinPhios commented 2 weeks ago

@LarsAsplund
Do you already had the change to review the axi master? At the moment a very basic version is implemented but it should provide basic read and write possibilities for burst and none burst.

LarsAsplund commented 2 weeks ago

I have yet to review it. The other implementation I'm keeping my eyes on is settling now so I think I will be in a position to review shortly and then make sure that the one you have is "feature compatible". With that I don't mean that it has to have all features but rather that the API is such that it can evolve and eventually cover all the things the other project is requiring. That way there is a chance that the two efforts can be merged in the end.

DavidMartinPhios commented 1 week ago

I have yet to review it. The other implementation I'm keeping my eyes on is settling now so I think I will be in a position to review shortly and then make sure that the one you have is "feature compatible". With that I don't mean that it has to have all features but rather that the API is such that it can evolve and eventually cover all the things the other project is requiring. That way there is a chance that the two efforts can be merged in the end.

@LarsAsplund I will wait for your review before I continue further implementations. Just to be sure that it is not completely running in the wrong direction.

LarsAsplund commented 6 days ago

So let's review this in a number of iterations, starting with the public interface. Since the first implementation won't implement all of AXI, it is important that we have a public API that can be extended in the future without breaking backward compatibility. There are two references to consider:

  1. The AXI standard (AXI5 is the latest)
  2. The unreleased VUnit VC standard. This is a work that started a few years ago but was paused since modifying existing VCs will break backward compatibility and has to be done with a major release of VUnit. Since the next release is planned to be a major one, v5.0, this is the time to make the modification. For your IP, we better do it according to this standard right from the beginning. This standard does not affect the details of your implementation but focuses on good VUnit practices.

The VUnit VC standard comes with a support package that you will need but I need to modify it slightly to take the identity package into account. I will start with that and push it to a branch that you can rebase on. To get a feel for the new VC standard you can read this but be aware that there will be some changes. Regardless of those changes you can start by looking at rule 13 (and rule 11):

Rule 13 A VC shall only have one generic.

Rationale: Representing a VC with a single object makes it easier to handle in code. Since all fields of the handle are private future updates have less risk of breaking backward compatibility.

Rather than a bus_master handle and a number of additional generics you will have to create another handle that contains a bus_master and the other parameters.

With respect to the AXI standard, I also see two things that you can start with:

  1. AXI5 has replaced the master/slave terminology with manager/subordinate
  2. To comply with the subset of the standard you've implemented, you would need the areset_n input. See chapter A3.1.2 of the standard.

Let's start with that and create a new iteration when I pushed that VC standard support package.