Conversation
olofk
left a comment
olofk
left a comment
There was a problem hiding this comment.
Looks good! Most of the changes are just renaming and a little cleaning up.
rtl/serv_bufreg.v
Outdated
| //External | ||
| output wire [31:0] o_dbus_adr); | ||
| output wire [31:0] o_dbus_adr, | ||
| output wire [31:0] o_mdu_rs1); |
There was a problem hiding this comment.
Rename o_mdu_rs1 to o_ext_rs1 instead since this can potentially be shared between different extensions
rtl/serv_decode.v
Outdated
| output reg o_slt_op, | ||
| output reg o_rd_op, | ||
| output reg o_mdu_op, | ||
| output reg [2:0] o_mdu_opcode, |
There was a problem hiding this comment.
Let's rename o_mdu_opcode to o_ext_funct3 to show what the value comes from and that the signal can be useful for multiple extensions
rtl/serv_mem_if.v
Outdated
|
|
||
| generate | ||
| if(MDU) begin | ||
| wire mdu_rd = i_mdu_op & (dat_valid ? dat_cur : signbit & i_signed); |
There was a problem hiding this comment.
I think that dat_valid will always be raised for MDU operations. Please check if we can simplify this expression to just i_mdu_op & dat_cur
rtl/serv_top.v
Outdated
| wire [3:0] immdec_ctrl; | ||
| wire [3:0] immdec_en; | ||
|
|
||
| .i_csr_rd (csr_rd), | ||
| .i_rd_csr_en (rd_csr_en), | ||
| .i_mem_rd (mem_rd), | ||
|
|
serv.core
Outdated
| parameters: | ||
| MDU: | ||
| datatype : int | ||
| description: Enables RISC-V standard M-extension |
There was a problem hiding this comment.
Enables interface for RISC-V standard M-extension
servant.core
Outdated
| - servant/servant.v | ||
| file_type : verilogSource | ||
| depend : [serv] | ||
| depend : {serv, mdu} |
There was a problem hiding this comment.
Use [ ] instead of { }. I actually didn't know it was allowed to use { }. Seems to work fine but made me a bit confused :)
|
Thanks for reviewing, the code is modified according to the requested changes. |
olofk
left a comment
olofk
left a comment
There was a problem hiding this comment.
Close now, but I missed one thing from my first review
servant.core
Outdated
| filesets : [soc, servant_tb] | ||
| parameters : | ||
| - RISCV_FORMAL | ||
| - MDU |
There was a problem hiding this comment.
This should also be "- mdu? (MDU=1)"
There was a problem hiding this comment.
Oh yes, this should be.
rtl/serv_rf_top.v
Outdated
| assign dbus_ack = i_dbus_ack; | ||
| end | ||
| assign o_ext_rs2 = o_dbus_dat; | ||
| endgenerate |
There was a problem hiding this comment.
I just realized that we need to move this generate block inside serv_top.v so that we can expose the extension interface for designs that uses serv_top directly instead of serv_rf_top (e.g. serving)
|
Awesome! I have nothing more to complain about now. :) Well done and thank you for your contributions |
2 participants
This project was the part of GSoC'21. Completed by @zeeshanrafique23 under the mentorship of @olofk.
The RTL of M-extension and changes made in serv are verified with RISC-V compliance tests. Follow the README to run the tests.
The parent repository of MDU can be found here: https://github.com/zeeshanrafique23/mdu.git
To run the serv with M-extension you have to add mdu in fusesoc core list, run this command to check the existing core:
and to add mdu as a fusesoc core run this command:
Now run this command to build the serv core with mdu:
Add
--vcdflag to generate.vcdfile to see the waveforms.For more details check this Blog.