add X-IF 1.0 #284
add X-IF 1.0 #284
Conversation
There was a problem hiding this comment.
I'm glad for the addition of the eXtension Interface to the CVE2 :)
Also, sorry for the delay on the review.
I added some comments basically regarding changes that should be necessary to keep the CVE2 behave the same way when the X-IF is not used, after @davideschiavone suggestion to run a Logical Equivalence Checking tool.
I used Yosys EQY for it, you can run the same test applying the patch in attachment and running it with cd scripts/sec && ./sec.sh -t yosys
| @@ -646,7 +660,6 @@ module cve2_decoder #( | |||
| // insufficient privileges), or when accessing non-available registers in RV32E, | |||
| // these cases are not handled here | |||
| if (illegal_insn) begin | |||
| rf_we = 1'b0; | |||
There was a problem hiding this comment.
Is the removal of this line necessary? If yes, is it due to a bug or it is needed to allow the eXtension Interface to work?
| @@ -690,6 +800,7 @@ module cve2_id_stage #( | |||
| stall_multdiv = multdiv_en_dec; | |||
| stall_branch = branch_in_dec; | |||
| stall_jump = jump_in_dec; | |||
| stall_coproc = illegal_insn_dec; | |||
There was a problem hiding this comment.
I would add the condition of when the XInterface parameter is activated, so that it is fully compatible with the original design with the X-IF.
| @@ -626,6 +710,8 @@ module cve2_id_stage #( | |||
| stall_jump = 1'b0; | |||
| stall_branch = 1'b0; | |||
| stall_alu = 1'b0; | |||
| stall_coproc = 1'b0; | |||
| stall_coproc = 1'b0; | |||
| default: rf_wdata_id_o = result_ex_i; | ||
| RF_WD_EX: rf_wdata_id_o = result_ex_i; | ||
| RF_WD_CSR: rf_wdata_id_o = csr_rdata_i; | ||
| RF_WD_COPROC: rf_wdata_id_o = x_result_i.data; |
There was a problem hiding this comment.
It would be better to limit the case RF_WD_COPROC: rf_wdata_id_o = x_result_i.data; to only when X-IF is used, i.e. when XInterface is enabled.
No description provided.