search

bespoke-silicon-group / bsg_replicant

BSG Replicant: Cosimulation and Emulation Infrastructure for HammerBlade
BSD 3-Clause "New" or "Revised" License
26 stars 20 forks source link

RVALID should be asserted after ARVALID and ARREADY in bsg_manycore_link_to_axil #585

Open gaozihou opened 4 years ago

gaozihou commented 4 years ago

Original implementation results in same-cycle assertion of arvalid, arready and rvalid, which violates AXI read transaction dependencies. In AMBA AXI and ACE Protocol Specification AXI3, AXI4, and AXI4-Lite ACE and ACE-Lite page A3-41 figure A3-5, arvalid and arready has double-headed arrows to rvalid, where " double-headed arrows point to signals that must be asserted only after assertion of the signal at the start of the arrow." Here the word "after" means "at least one cycle after", as clarified here by ARM Support.

This makes the adapter incompatible with Xilinx xdma IP, which results in weird behavior on both VU37P FPGA and ASIC motherboard. When executing "lspci" in command line, the PC freezes and needs a hard-restart. It should be because the AXI protocol violation triggers illegal behavior in xdma IP, which breaks communication between FPGA and PC.

Suggested fix: https://github.com/bespoke-silicon-group/bsg_replicant/blob/b7ea81cad6447837e1b8b2cebc0a6f198908d2b3/hardware/bsg_manycore_link_to_axil.v#L197-L198 Should be changed to assign axil_rdata_o = rdata_r; assign axil_rvalid_o = rvalid_r;

leonardxiang commented 4 years ago

Hi, @gaozihou good finding about the definition of double-headed arrows! Previously, I was using the AXI protocol checker from aws_f1, and it did not find any violation in the simulation. https://github.com/aws/aws-fpga/blob/ed564e7a87be1ad75b629d628b6e933f27b5bf26/hdk/common/verif/models/sh_bfm/sh_bfm.sv#L431

Does it pass with your fix?

gaozihou commented 4 years ago

Thanks @leonardxiang. Seems that implementation of bvalid is fine (it also has double-headed arrows), only rvalid has this issue.

Not sure why aws AXI checker didn't find this issue, but cosim is running without problem on F1 right? Maybe aws-checker is doing is job to ensure things are compatible with aws-F1 framework, but not ensuring it is fully compatible with AMBA AXI protocol?

When it comes to Xilinx it is a different story... As we discussed before sometimes lspci freezes the PC, and I think this should be the root cause.

drichmond commented 4 years ago

Good catch!

drichmond commented 4 years ago

Is the fix as simple as changing those two lines? Did you confirm this?