ZipCPU / wb2axip

Bus bridges and other odds and ends
495 stars 100 forks source link

axisafety module fails tests with ZYNQ + AXI BRAM controller #29

Closed madorskya closed 4 years ago

madorskya commented 4 years ago

Hello, first of all, this is a very useful collection of utilities, thanks a lot for making them available. My issue: I'm trying to use axisafety module to prevent AXI bus lockups. As a first step, I tried testing it with a known-good module, which is Xilinx standard AXI BRAM controller. Steps are below:

  1. Had to convert all localparams into params in axisafety.v source, otherwise Vivado schematic editor does not recognize the module as valid for inclusion into schematics. (Using Vivado 2020.1)
  2. Connected AXI BRAM controller with BRAM attached directly to ZYNQ AXI interconnect, verified that it works by writing and reading from Linux console using devmem utility. Writing a number into address 0x50000000 and then reading it back returns the number that was written. (0x50000000 is BRAM base address)
  3. Attached axisafety module between AXI interconnect and AXI BRAM controller. See attached pictures for details. BRAM depth is 8KB.
  4. When trying to write/read the BRAM using devmem utility, both read and write operations are successful, but the value read back is always 0. It does not depend on what was previously written into BRAM.

Could you let me know if axisafety was attached and configured correctly? If not, what must be changed?

thanks. Alex Madorsky University of Florida/Physics.

image image image

ZipCPU commented 4 years ago

Thank you for reporting this.

The bug is in line 637, where the downstream write data is assigned to M_AXI_WDATA (the downstream write data), rather than S_AXI_WDATA. Expect an update shortly.

Dan

ZipCPU commented 4 years ago

Fixed in 77ab90a.

Dan

ZipCPU commented 4 years ago

I should also point out that it looks like you have promoted several localparam's to full blown parameters. These were designed as localparam's to guarantee proper functionality. While I understand that Vivado may require to you turn them into parameters, if you actually override their values--or set them to values inconsistent with their original (sometimes equation) values, you may break the core. For example, IW is shorthand for C_S_AXI_ID_WIDTH. If the two are ever out of synch, the core will not work. DW and AW have similar definitions. Likewise if you change LSB, EXOKAY, or SLAVE_ERROR, the design may not be protocol compliant.

Dan

madorskya commented 4 years ago

Thanks for the fix, it's now working, at least with BRAM controller.

Regarding localparam vs parameter: I left the equations as they were for localparams, so the link between parameters is still there. But if you have a better suggestion, please let me know. Vivado definitely does not like localparams

Another question, if you don't mind: Do I understand correctly that M_AXI_ARESETN output from axisafety should be connected to S_AXI_ARESETN input of the unreliable core?

thanks. Alex.

ZipCPU commented 4 years ago

If you use the OPT_SELF_RESET capability then M_AXI_ARESETN needs to be connected to the downstream/unreliable slave, yes. If not, M_AXI_ARESETN is just a simple passthrough and you can choose to connect it or not at your leisure.

The self reset capability is one of the big benefits of this particular design: on any fault, it can reset the slave and try to start over. This could be valuable if you want to discover what conditions cause the violation and the first trace you get doesn't tell you enough and so you want to try again. I'm unaware of any other firewalls with this capability.

Incidentally, don't forget to trigger a trace on one of the fault outputs. That'll help you understand what's going on when/if your design does fail.

Dan

madorskya commented 4 years ago

thanks Dan, I'm actually using axisafety with Xilinx Chip2chip IP. That IP is pretty much guaranteed to lock up the system if the other side of the link is missing, for example in case of the other FPGA being unconfigured. It entirely evades me why Xilinx does not care about this glaring omission. So axisafety is invaluable in this scenario. I do use it with OPT_SELF_RESET=1 and have connected M_AXI_ARESETN output to Chip2chip reset input. thanks for your help. Alex.

ZipCPU commented 4 years ago

Awesome! That's what it axisafety was made for. I'd love to hear how it works for you once you get far enough to be able to tell.

Dan