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

Removed POINTER_FETCH state from controller_fsm #696

Conversation

silabs-oysteink
Copy link
Contributor

POINTER_FETCH fsm state replaced by a separate state flop. Controller stays in FUNCTIONAL while handling CLIC SHV interrupts.

Mret which restarts CLIC pointer fetch due to mcause.minhv and mcause.mpp will now get its 'last_op' bit set to 0 in the ID stage, and the following CLIC pointer will complete the sequence. CSR updates only happen when the pointer reaches WB.

NOT SEC clean

  • Master has bugs (possible deadlock while single stepping mret with CLIC pointers, wrong pipeline stage used for excluding CLIC pointers from incrementing minstret).
  • Minstret will now only update for mret when the CLIC pointer is done (or when the mret reaches WB if no CLIC pointer fetch is restarted)

…a status bit (flop). Mrets which spawn a CLIC pointer fetch will not retire until the pointe reaches WB (last_op=0 when the mret itself reaches WB).

Updated RVFI to enable signaling completion of the mret when the pointer reaches WB.

Signed-off-by: Oystein Knauserud <[email protected]>
…LIC pointer reaches WB (last_op == 1).

Signed-off-by: Oystein Knauserud <[email protected]>
@silabs-oysteink silabs-oysteink added the Component:RTL For issues in the RTL (e.g. for files in the rtl directory) label Nov 8, 2022
rtl/cv32e40x_alignment_buffer.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_controller_fsm.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_controller_fsm.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_cs_registers.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_id_stage.sv Show resolved Hide resolved
rtl/cv32e40x_if_stage.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_if_stage.sv Outdated Show resolved Hide resolved
Restructured the csr_save unique case and updated comments and signal names for clarity.

Signed-off-by: Oystein Knauserud <[email protected]>
@silabs-oysteink
Copy link
Contributor Author

SEC clean updates pushed.

rtl/include/cv32e40x_pkg.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_controller_fsm.sv Outdated Show resolved Hide resolved
rtl/cv32e40x_controller_fsm.sv Outdated Show resolved Hide resolved

// Set flag to avoid further jumps to the same target
// if we are stalled
branch_taken_n = 1'b1;

// Clear flag for halting IF in case of single stepping
Copy link
Contributor

Choose a reason for hiding this comment

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

Add comment that flag was cleared too early because last_op_if_i information is not accurate (and gets corrected only in ID) for an mret that causes a CLIC pointer fetch.

rtl/cv32e40x_cs_registers.sv Outdated Show resolved Hide resolved
sva/cv32e40x_controller_fsm_sva.sv Outdated Show resolved Hide resolved
sva/cv32e40x_controller_fsm_sva.sv Outdated Show resolved Hide resolved
sva/cv32e40x_rvfi_sva.sv Outdated Show resolved Hide resolved
@@ -681,6 +687,11 @@ module cv32e40x_rvfi

rvfi_obi_instr_t obi_instr_if;

// Detect mret initiated CLIC pointer in WB
logic mret_clic_pointer_wb;
Copy link
Contributor

Choose a reason for hiding this comment

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

mret_clic_pointer_wb -> mret_clic_ptr_wb

bhv/cv32e40x_rvfi.sv Outdated Show resolved Hide resolved
Removed flushing of EX and WB when an mret which restarts pointer fetches is in ID. An mret can now be killed at any stage,  and if it restarted a pointer fetch the 'sequence_in_progress_wb' will block interrupts and debug until the pointer arrives in WB.

Signed-off-by: Oystein Knauserud <[email protected]>
…progress_is.

SEC clean

Signed-off-by: Oystein Knauserud <[email protected]>
sva/cv32e40x_controller_fsm_sva.sv Show resolved Hide resolved
sva/cv32e40x_controller_fsm_sva.sv Show resolved Hide resolved
sva/cv32e40x_decoder_sva.sv Outdated Show resolved Hide resolved
end

// CLIC/mret pointer in ID clears pointer fetch flag
if ((clic_ptr_in_id || mret_ptr_in_id) && id_valid_i && ex_ready_i) begin
Copy link
Contributor

Choose a reason for hiding this comment

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

mret_ptr_in_id should not affect clic_ptr_in_progress_id(_clear) I think

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, removing mret_ptr_in_id is also SEC clean (no mret should be in the pipeline at all when clic_ptr_in_id is 1)

rtl/cv32e40x_controller_fsm.sv Show resolved Hide resolved
rtl/cv32e40x_controller_fsm.sv Outdated Show resolved Hide resolved
bhv/cv32e40x_rvfi.sv Outdated Show resolved Hide resolved
Added assertions related to clic pointers and decoder deassertion and that ID stage can never be killed when a clic pointer is in progress as seen from the ID stage.

Signed-off-by: Oystein Knauserud <[email protected]>
@silabs-oysteink
Copy link
Contributor Author

Updated according to latest feedback, SEC clean for both ZC_EXT=0 and ZC_EXT=1.

@Silabs-ArjanB Silabs-ArjanB merged commit 0c9a3a8 into openhwgroup:master Nov 14, 2022
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.

2 participants