Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Parameterized SECURE features #498

Merged

Conversation

halfdan-dolva
Copy link
Member

LEC clean with SECURE=1.

With SECURE=0 and some (to be analyzed) changes in the controller and hacked marchid, this is also LEC clean against the equivalent configuration of CV32E40X.

Signed-off-by: Halfdan Bechmann Dolva <halfdan.bechmann@silabs.com>
@halfdan-dolva halfdan-dolva added the Component:RTL For issues in the RTL (e.g. for files in the rtl directory) label Sep 11, 2023
@halfdan-dolva
Copy link
Member Author

Additional info:

The changes required in the controller for cv32e40x equivalence are the following:

cv32e40s_controller_bypass.sv: assign csr_impl_rd_unqual_id = sys_mret_unqual_id || sys_wfi_unqual_id || sys_wfe_unqual_id || tbljmp_unqual_id;
cv32e40x_controller_bypass.sv: assign csr_impl_rd_unqual_id = sys_mret_unqual_id || tbljmp_unqual_id;

cv32e40s_controller_fsm.sv: assign branch_taken_ex = branch_in_ex && branch_decision_ex_i && !branch_taken_q;
cv32e40x_controller_fsm.sv: assign branch_taken_ex = branch_in_ex && !branch_taken_q;

cv32e40s_controller_fsm.sv: assign async_debug_allowed = lsu_interruptible_i && !fencei_ongoing && !clic_ptr_in_pipeline && sequence_interruptible &&
                                                         !woke_to_interrupt_q && !csr_flush_ack_q && !(ctrl_fsm_cs == SLEEP);
cv32e40x_controller_fsm.sv:                              !woke_to_interrupt_q && !(ctrl_fsm_cs == SLEEP);

cv32e40s_controller_fsm.sv: assign nmi_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !clic_ptr_in_pipeline &&
                                                 sequence_interruptible && !(woke_to_debug_q || woke_to_interrupt_q) && !csr_flush_ack_q && !(ctrl_fsm_cs == SLEEP);
cv32e40x_controller_fsm.sv:                      sequence_interruptible && !(woke_to_debug_q || woke_to_interrupt_q) && !(ctrl_fsm_cs == SLEEP);

cv32e40s_controller_fsm.sv: assign branch_in_ex = id_ex_pipe_i.alu_bch && id_ex_pipe_i.alu_en && id_ex_pipe_i.instr_valid;
cv32e40x_controller_fsm.sv: assign branch_in_ex = id_ex_pipe_i.alu_bch && id_ex_pipe_i.alu_en && id_ex_pipe_i.instr_valid && branch_decision_ex_i;

cv32e40s_controller_fsm.sv: assign interrupt_allowed = lsu_interruptible_i && debug_interruptible && !fencei_ongoing && !clic_ptr_in_pipeline &&
                                                       sequence_interruptible && !interrupt_blanking_q && !csr_flush_ack_q && !csr_flush_ack_q && !(ctrl_fsm_cs == SLEEP);
cv32e40x_controller_fsm.sv:                            sequence_interruptible && !interrupt_blanking_q && !(ctrl_fsm_cs == SLEEP);

@@ -85,8 +85,13 @@ module cv32e40s_data_obi_interface import cv32e40s_pkg::*;

always_comb begin
resp_o = m_c_obi_data_if.resp_payload;
resp_o.integrity_err = rvalidpar_err_resp || gntpar_err_resp || rchk_err_resp;
resp_o.integrity = integrity_resp;
if (SECURE) begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use scope names here (is that possible without generate)?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As in passing the parameter through the hierarchy?
That should be equivalent both with and without a generate.

if (instr_rdata_i[14:12] == 3'b001) begin // CSRRW
if ((instr_rdata_i[19:15] == 5'b0)) begin
// rs1 is zero, flag instruction as illegal
if (SECURE) begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use scope names here (is that possible without generate)?

@@ -87,8 +87,13 @@ module cv32e40s_instr_obi_interface import cv32e40s_pkg::*;

always_comb begin
resp_o = m_c_obi_instr_if.resp_payload;
resp_o.integrity_err = rvalidpar_err_resp || gntpar_err_resp || rchk_err_resp;
resp_o.integrity = integrity_resp;
if (SECURE) begin
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use scope names here (is that possible without generate)?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean: if (SECURE) begin : some_name

No need to add parameters, just sensible names for introduced scopes

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added names to these ifs, but they won't be visible in formal/sim tools as they are just part of the combinatorial logic in the always blocks.

Signed-off-by: Halfdan Bechmann Dolva <halfdan.bechmann@silabs.com>
@Silabs-ArjanB Silabs-ArjanB merged commit d6048f8 into openhwgroup:master Sep 13, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component:RTL For issues in the RTL (e.g. for files in the rtl directory)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants