Wrapping the core with a axi4 lite slave interface

[Sanjay-A-Menon]

I am trying to use this IP for creating a SHA256 cracker using a ZYNQ FPGA(PYNQ-Z2). I have partially completed the wrapping of the IP with the interface but I am facing some issues in writing a testbench for the IP.

Is there a way to contact you? Could you provide a suggestion on what bus interface to wrap the IP with?

[secworks]

This works well as a way of contacting me. I'm not really sure what you mean by "Could you provide a suggestion on what bus interface to wrap the IP with?"

The sha256 core comes with a simple wrapper (the file sha256.v). It can either be replaced by an AXI4 slave interface (that instantiates sha256_core.v), or it can in turn be wrapped by a module that have an AXI4 Slave interface. What strategy are you considering.

Is it an AXI4 slave testbench you are asking about?

[Sanjay-A-Menon]

Yes, i am talking about an AXI4-lite slave testbench. I edited the file sha256.v to incorporate the otherwise individual i/o ports into an axi4-lite slave interface, but are facing some difficulties. Would it be possible for you to review the code(the axi4-lite wrapper and testbench) if I put it in here?(It is a bit large)

[secworks]

Can you describe what specific difficulties you have?

I can take a loot at your code. But for doing that I think it is better if you fork the core and check in the changes you have done in your fork. Then add a link to the changes. That way the Github diff tool will display the changes correctly.

I searched for AXI4 slaves on Github, and there are quite a few existing project. Including projects that provides interface generators, testbenches etc. One idea is to look at those and see if they can be useful for you to generate what you need.

https://github.com/pulp-platform/axi https://github.com/k0nze/zedboard_axi4_master_burst_example https://github.com/OSVVM/AXI4 https://github.com/PyHDI/ipgen https://github.com/smartfoxdata/axi4lite_gpio

This is not an exhaustive list, just five min searching. I've tried PyHDL before and seems to be good.

[Sanjay-A-Menon]

Wow, the sources for existing projects you provided were great. I have already completed all of the wrapped IP core(I believe so), but are facing some issues with the testbench for the IP.

These are my links to the AXI4 wrapped IP core and testbench:- IP core :- https://github.com/Sanjay-A-Menon/sha256/blob/master/src/rtl/sha256.v https://github.com/Sanjay-A-Menon/sha256/blob/master/src/rtl/SHA256_v2_0_S00_AXI.v

testbench :- https://github.com/Sanjay-A-Menon/sha256/blob/master/src/tb/sha256v2_tb.v

Comparison:- https://github.com/secworks/sha256/compare/master...Sanjay-A-Menon:master

I am currently facing issues in reading from the IP other than that writing into the IP and process initiation of the IP is fine(found that out by printing the internal register values). I did a behavioral simulation of the IP using Vivado 2019.1 and I have attached the waveform.(Note the tb_rdata waveform-Presence of don't care's instead of 1's).

Thanks a lot for helping me out.

[Sanjay-A-Menon]

Are you sure this is the best way to communicate with you?.

[secworks]

This channel works. I will try and help you. But you can't expect a response immediately. I do this on my spare time. I will look at the things you posted above the coming week.

[Sanjay-A-Menon]

Yeah sorry thought you forgot about this.

[secworks]

Ok, I've got a few minutes for this.

One issue I see is that you have blocking and non_blocking assignments in the same process. This is incorrect code. You have moved the default assignment of tmp_read_data from the combinational process api_logic to the reset part of reg_update. But you are still using a blocking assignment (=), not a non-blocking assigment (<=):

https://github.com/Sanjay-A-Menon/sha256/blob/a86f48ddf4449e50bdaed25a000c2c5f4db450eb/src/rtl/sha256.v#L198

Also the tmp_read_data is not updated in the else-part of the reg_update, which will lead to latches. Furthermore, tmp_read_data is still being updated in the api_logic process. This means that you update tmp_read_data in two processes (reg_update and api_logic). This is not correct code and will cause conflicts since the wire is driven by two processes at the same time. This may well explain that you are getting errors on the read data.

I would suggest that you restore the default assignment of tmp_read_data in api_logic and removing it from the reg_update process. tmp_read_data is the wire connecting the api read mux to the read_data port. It is not a register.

If you need a register on the s00_axi_rdata port I would suggest creating a s00_axi_rdata_reg and having it sampled (s00_axi_rdata_reg <= tmp_read_data) in the reg_update port. And then having an assignment assign s00_axi_rdata = s00_axi_rdata_reg;

Try that and see if it helps.

[secworks]

Since you are basically keeping the sha256.v (the modifications are quite low) I would suggest that you instead wrap the sha256.v in an AXI_wrapper. keep the sha256.v as it is and add logic, regs as needed around it to get it to work as an AXI slave. This is probably easier and you have a known good part (sha256.v) as a component.

[Sanjay-A-Menon]

Thanks for reviewing my code. Is there a way I can map the output of the tmp_read_data register to the s00_axi_rdata wire, which was what I was trying to achieve. To note a point tmp_read_data was declared as a register in the old code as well. Got the same waveform even after restoring the default assignment of tmp_read_data into api_logic. Comparison :- https://github.com/secworks/sha256/compare/master...Sanjay-A-Menon:master

[secworks]

It seems you may be confused regarding Verilog syntax.

The variable type "reg" in Verilog does not mean it will become a register. [0] A variable of type reg is something that can be assigned a value, for example in a task or in a process (an always-something block). If you look at the code there are for example several variable declared with the type reg that are clearly not registers, mode_we and block_we for example. A variable (or a port) declared as as the type wire can not hold a value and only be assigned to with an assign statement or a port map.

Therefore, if I have an API implemented in a process, but have the port declared as a wire (which you want), I need to have a temporary wire (tmp_read_data) that is assigned in the read part of the API. I then need to have an assign statement that connects the read_data port to the tmp_read_data wire.

In short: There is no tmp_read_data register, it is a wire (of the type reg). It is the output from the MUX that selects what is being read out based on the given address. The wire tmp_read_data is connected to the read_data port of the type wire by a continuous assignment.

I would really suggest that you create a module, for example sha256_axi.v and in it instantiate sha256.v. Do not try and modify it. Simply connect the axi-address, write_data and read_data to address, write_data and read_data ports of the sha256 module. You may have to implement some simple state machine to perform the correct strobing of chip select (cs) and write enable (we). But it will quite probably be much easier. Don't try to modify the functionality in sha256.v. It works really well and you will have much less complexity to deal with.

[0] a register is created when a variable is assigned in a clocked process using non-blocking assignment (<=). I have a personal code style where I name variables that I want to be registers "_reg", for example mode_reg. The variable that becomes a register must be of type "reg" because it is assigned inside (the clocked) process. But it is the assignment and the clock trigger that makes it a register, not the variable type.

Some other explanation: https://stackoverflow.com/questions/33459048/what-is-the-difference-between-reg-and-wire-in-a-verilog-module

A good reference: https://sutherland-hdl.com/pdfs/verilog_2001_ref_guide.pdf

[Sanjay-A-Menon]

Thank you so much, That cleared a lot of my doubts. Yes, what you said is correct I should probably instantiate the sha256 module in another sha256_AXI4.v file. But, after reviewing my existing code for quite a while I have identified the problem to be due to the inherent function of the AXI4 protocol itself which is also trying to drive my output(s00_axi_rdata).

See :- https://github.com/Sanjay-A-Menon/sha256/blob/dd6ad3a85ab1a0ea9e6ba55f8fd34d31509b4f72/src/rtl/SHA256_v2_0_S00_AXI.v#L1055

1.Wouldn't this issue exist even if I were to instantiate sha256 as a module rather than modifying it? 2.How could I possibly solve this issue?

[secworks]

You seem to be implementing the AXI interface twice.

(1) remove your file sha256.v with the original file from my repo. (2) Instantiate the interface in the original sha256.v into the file SHA256_v2_0_S00_AXI.v (3) Connect the ports.

You will only have one driver of the read_data port. BTW: You seem to allocate a lot of slave registers in SHA256_v2_0_S00_AXI.v or it is possibly template registers. But there is a lot of cleanup to do. You should basically only have registers as needed to support the handshake and possibly read_data hold registers.

[Sanjay-A-Menon]

Actually the slave registers existed as part of the template I got for the implementation of the AXI4 lite slave interface on the IP. The writing into the IP seems to be working fine with all the handshaking signals proper. But, somehow the read part is the issue so didn't try fiddling with the registers. Will provide you with an update after trying what what you have suggested so far. You said "You seem to be implementing the AXI interface twice." . Could you explain why you think so?.

Facing issue similar to what is described in this:- https://forums.xilinx.com/t5/Design-Entry/Modifing-AXI4-lite-IP-packager-verilog-code/td-p/919401

Finally got it working. Funnily enough by changing this line:- https://github.com/Sanjay-A-Menon/sha256/blob/3a786e686a42098bf4e8b12efd21ad1a81874948/src/rtl/sha256.v#L69 Thanks for your help. Isn't it better that I keep this issue open so that I can use it if I have doubts?. I am trying to create a fast SHA256 cracker using this core. I would like to contribute to your project. Should I add a pull request?.

[secworks]

Cool! Congratulations to getting it to work. I think we should close the issue since you got it to work. We can open up the issue again if the need arise.

I saw the merge request. I need to look at it and will respond there.