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.

Sign up for GitHub

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

Fix for issue #341. #725

Merged
merged 2 commits into from
Dec 13, 2022
Merged

Fix for issue #341. #725

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5b4a669
Fix for issue #341.
silabs-oysteink Dec 13, 2022
37a96f1
Removed ungated clock from controller.
silabs-oysteink Dec 13, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions rtl/cv32e40x_controller.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
)
(
input logic clk, // Gated clock
input logic clk_ungated_i, // Ungated clock
input logic rst_n,

input logic fetch_enable_i, // Start the decoding
Expand Down Expand Up @@ -150,7 +149,6 @@ module cv32e40x_controller import cv32e40x_pkg::*;
(
// Clocks and reset
.clk ( clk ),
.clk_ungated_i ( clk_ungated_i ),
.rst_n ( rst_n ),

.fetch_enable_i ( fetch_enable_i ),
Expand Down
24 changes: 6 additions & 18 deletions rtl/cv32e40x_controller_fsm.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
(
// Clocks and reset
input logic clk, // Gated clock
input logic clk_ungated_i, // Ungated clock
input logic rst_n,

input logic fetch_enable_i, // Start executing
Expand Down Expand Up @@ -137,9 +136,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
// Debug state
debug_state_e debug_fsm_cs, debug_fsm_ns;

// Sticky version of debug_req_i
logic debug_req_q;

// Sticky version of lsu_err_wb_i
logic nmi_pending_q;
logic nmi_is_store_q; // 1 for store, 0 for load
Expand Down Expand Up @@ -482,8 +478,12 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
(ebreak_in_wb && dcsr_i.ebreakm && !debug_mode_q) || // Ebreak with dcsr.ebreakm==1
(ebreak_in_wb && debug_mode_q); // Ebreak during debug_mode restarts execution from dm_halt_addr, as a regular debug entry without CSR updates.

// Debug pending for external debug request
assign pending_async_debug = ((debug_req_i || debug_req_q) && !debug_mode_q);
// Debug pending for external debug request, only if not already in debug mode
// Ideally the !debug_mode_q below should be factored into async_debug_allowed, but
// that can currently cause a deadlock if debug_req_i gets asserted while in debug mode, as
// a pending but not allowed async debug will cause the ID stage to halt forever while trying
// to get to an interruptible state.
assign pending_async_debug = debug_req_i && !debug_mode_q;
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove clk_ungated_i from this module as well as it is no longer needed here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed ungated clock from controller.


// Determine cause of debug. Set for all causes of debug entry.
// In case of ebreak during debug mode, the entry code in DEBUG_TAKEN will
Expand Down Expand Up @@ -1129,18 +1129,6 @@ module cv32e40x_controller_fsm import cv32e40x_pkg::*;
assign ctrl_fsm_o.debug_mode_if = debug_mode_n;
assign ctrl_fsm_o.debug_mode = debug_mode_q;

// sticky version of debug_req (must be on clk_ungated_i such that incoming pulse before core is enabled is not missed)
always_ff @(posedge clk_ungated_i, negedge rst_n) begin
if (rst_n == 1'b0) begin
debug_req_q <= 1'b0;
end else begin
if (debug_req_i) begin
debug_req_q <= 1'b1;
end else if (debug_mode_q) begin
debug_req_q <= 1'b0;
end
end
end

// Sticky version of lsu_err_wb_i
always_ff @(posedge clk, negedge rst_n) begin
Expand Down
1 change: 0 additions & 1 deletion rtl/cv32e40x_core.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -846,7 +846,6 @@ module cv32e40x_core import cv32e40x_pkg::*;
controller_i
(
.clk ( clk ), // Gated clock
.clk_ungated_i ( clk_i ), // Ungated clock
.rst_n ( rst_ni ),

.fetch_enable_i ( fetch_enable ),
Expand Down
3 changes: 2 additions & 1 deletion sva/cv32e40x_controller_fsm_sva.sv
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,10 @@ module cv32e40x_controller_fsm_sva
else `uvm_error("controller", "Fencei request when no fencei in writeback")

// Assert that the fencei request is set the cycle after fencei instruction enters WB (if fencei_ready=1 and there are no higher priority events)
// Only check when no higher priority event is pending (nmi, async debug or interrupts) and WB stage is not killed
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you explain why this assertion was required to be changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When debug_req_q was still in the controller, it would remain set until we actually entered debug mode (causing pending_async_debug to be 1). That would cause the antecedent of the assertion not to trig. Now that the sticky debug is gone, we may go to DEBUG_TAKEN due to a short pulse on debug_req_i and pending_async_debug may be 0 while in the DEBUG_TAKEN state. We are then killing the WB stage, but the antecedent didn't take killing of WB into account and only looked at signals originating from the ex_wb_pipe.

a_fencei_hndshk_req_when_fencei_wb :
assert property (@(posedge clk) disable iff (!rst_n)
$rose(fencei_in_wb && fencei_ready) && !(pending_nmi || (pending_async_debug && async_debug_allowed) || (pending_interrupt && interrupt_allowed))
$rose(fencei_in_wb && fencei_ready) && !ctrl_fsm_o.kill_wb && !(pending_nmi || (pending_async_debug && async_debug_allowed) || (pending_interrupt && interrupt_allowed))
|=> $rose(fencei_flush_req_o))
else `uvm_error("controller", "Fencei in WB did not result in fencei_flush_req_o")

Expand Down