openhwgroup / cv32e40p

CV32E40P is an in-order 4-stage RISC-V RV32IMFCXpulp CPU based on RI5CY from PULP-Platform
https://docs.openhwgroup.org/projects/cv32e40p-user-manual/en/latest
Other
941 stars 412 forks source link

cv32e40p_obi_interface: When TRANS_STABLE=0, unstable transaction can still pass through to obi interface. #950

Closed bdiz closed 7 months ago

bdiz commented 7 months ago

Bug Title

cv32e40p_obi_interface: When TRANS_STABLE=0, unstable transaction can still pass through to obi interface.

RTL does not register transaction while in the REGISTERED state.

Component

cv32e40p_obi_interface

Component:RTL:

Steps to Reproduce

  1. git hash: 6adc6b3d71d93dfbb5006cd3fa095331df38e8fd
  2. Command line: proprietary
  3. Logfile and/or wave-dump info (screen shots can be useful) image

It can be seen in the waveform that trans_addr_i 'h7453_603e gets registered and is driven stable on obi_addr_o until obi_gnt_i is asserted. So far, so good. During the cycle that obi_gnt_i is asserted and the RTL is in the REGISTERED state, a transaction with trans_addr_i of 'hcc3b_c3f6 gets put on the bus. Being that this transaction is unstable, trans_addr_i goes to 'x on the next cycle. The RTL did not register 'hcc3b_c3f6 and thus drives unstable 'x data on obi_addr_o for a few cycles until obi_gnt_i is asserted.

My expectation is that 'hcc3b_c3f6 should have been driven on obi_addr_o instead of 'x.

pascalgouedo commented 7 months ago

Hi, The "unstable" transaction at time 380.000 is retracted while trans_ready_o is 0 which means the design is still managing previous one. FSM state is always switching between TRANSPARENT and REGISTERED states. It does not stay continuously in REGISTERED state and take any new transaction to register it if obi_gnt_i is 0. This is the responsibility of the transaction generator to use/check trans_ready_o to figure out if the transaction has been taken into account by the design.

pascalgouedo commented 7 months ago

As the design is working as expected I close this issue.

bdiz commented 7 months ago

Ok, I think I am fundamentally off as to the intended function of this module. I thought it was that if ever trans_valid_o was asserted, the entire address channel would be registered and ensure it eventually arrives on obi. But I'm thinking by your reply this is not correct.

Can you give me a short description of this modules function? Thanks.

pascalgouedo commented 7 months ago

The entire address channel is registered only if obi_req_o is not immediately granted by obi_gnt_i. A burst of granted request will go through it in transparent mode to avoid to add any latency and don't need to be registered. As soon as obi_gnt_i is 0 when obi_req_o =1 then the entire channel is memorized to have stable information on obi side as long as obi_gnt_i is not 1. And no new request is accepted from trans side.