Remove FIXME about considering concurrent load/store and debug entry

in calculating the privilege of load/stores. This is safe because it
is only the *current* debug mode state which affects load/stores,
and some new properties have been added to ensure load/stores can
not be in aphase at the point debug mode is entered/exited (which
is achieved by delaying the trap). Therefore there is no way for
debug entry to inadvertently boost the privilege of an executing
U-mode load/store.

Also rename a confusingly-named signal for an unsquashable bus
transfer in stage 2 that delays IRQ entry.
This commit is contained in:
Luke Wren 2022-11-05 18:18:33 +00:00
parent 97bf2d06f6
commit c81666177e
2 changed files with 32 additions and 11 deletions

View File

@ -324,7 +324,7 @@ reg [W_EXCEPT-1:0] xm_except;
reg xm_except_to_d_mode;
reg xm_sleep_wfi;
reg xm_sleep_block;
reg xm_delay_irq_entry_on_ls_dphase;
reg xm_delay_irq_entry_on_ls_stagex;
// ----------------------------------------------------------------------------
// Stall logic
@ -893,7 +893,7 @@ reg prev_instr_was_32_bit;
always @ (posedge clk or negedge rst_n) begin
if (!rst_n) begin
xm_delay_irq_entry_on_ls_dphase <= 1'b0;
xm_delay_irq_entry_on_ls_stagex <= 1'b0;
prev_instr_was_32_bit <= 1'b0;
end else begin
// Must hold off IRQ if we are in the second cycle of an address phase or
@ -905,7 +905,7 @@ always @ (posedge clk or negedge rst_n) begin
// Also hold off on AMOs, unless the AMO is transitioning to an address
// phase or completing. ("completing" excludes transitions to error phase.)
xm_delay_irq_entry_on_ls_dphase <= bus_aph_req_d && !bus_aph_ready_d ||
xm_delay_irq_entry_on_ls_stagex <= bus_aph_req_d && !bus_aph_ready_d ||
d_memop_is_amo && !(
x_amo_phase == 3'h3 && bus_dph_ready_d && !bus_dph_err_d ||
// Read reservation failure failure also generates error
@ -944,7 +944,7 @@ wire m_dphase_in_flight = xm_memop != MEMOP_NONE && xm_memop != MEMOP_AMO;
// Need to delay IRQ entry on sleep exit because, for deep sleep states, we
// can't access the bus until the power handshake has completed.
wire m_delay_irq_entry = xm_delay_irq_entry_on_ls_dphase ||
wire m_delay_irq_entry = xm_delay_irq_entry_on_ls_stagex ||
((xm_sleep_wfi || xm_sleep_block) && !m_sleep_stall_release);
wire m_pwr_allow_sleep;
@ -999,6 +999,7 @@ hazard3_csr #(
.trap_enter_vld (m_trap_enter_vld),
.trap_enter_rdy (m_trap_enter_rdy),
.loadstore_dphase_pending (m_dphase_in_flight),
.delay_irq_entry (m_delay_irq_entry),
.mepc_in (m_exception_return_addr),
.pwr_allow_sleep (m_pwr_allow_sleep),
@ -1011,7 +1012,6 @@ hazard3_csr #(
.m_mode_trap_entry (m_mmode_trap_entry),
// IRQ and exception requests
.delay_irq_entry (m_delay_irq_entry),
.irq (irq),
.irq_software (soft_irq),
.irq_timer (timer_irq),
@ -1085,6 +1085,14 @@ always @ (posedge clk or negedge rst_n) begin
xm_sleep_wfi <= 1'b0; // TODO needed?
xm_sleep_block <= 1'b0;
end
`ifdef HAZARD3_ASSERTIONS
// Some of the exception->ls paths are cut as they are supposed to be
// impossible -- make sure there is no way to have a load/store that
// is not squashed. (This includes debug entry!)
if (m_trap_enter_vld) begin
assert(!bus_aph_req_d);
end
`endif
end
end

View File

@ -79,9 +79,12 @@ module hazard3_csr #(
// addresses.
output wire trap_enter_soon,
// We need to know about load/stores in data phase because their exception
// status is still unknown, so we fence off on them before entering debug
// mode.
// status is still unknown, so fence them off before entering debug mode.
input wire loadstore_dphase_pending,
// Asserted when the instruction in stage 2 can't be squashed (e.g. to
// keep an AHB address phase stable), or instruction fetch request can't
// be issued (e.g. waiting for wake from wfi state).
input wire delay_irq_entry,
input wire [XLEN-1:0] mepc_in,
// Power control signalling
@ -100,7 +103,6 @@ module hazard3_csr #(
input wire except_to_d_mode,
// Level-sensitive interrupt sources
input wire delay_irq_entry,
input wire [NUM_IRQS-1:0] irq,
input wire irq_software,
input wire irq_timer,
@ -1197,7 +1199,7 @@ wire want_halt_except = DEBUG_SUPPORT && !debug_mode && (
//
// We must mask halt_req with delay_irq_entry (true on cycle 2 and beyond of
// load/store address phase) because at that point we can't suppress the bus
// access..
// access. Others are fine because they are never mid-instruction in stage 2.
wire want_halt_irq_if_no_exception = DEBUG_SUPPORT && !debug_mode && !want_halt_except && (
(dbg_req_halt_prev && !delay_irq_entry) ||
(dbg_req_halt_on_reset && have_just_reset) ||
@ -1299,7 +1301,6 @@ assign trap_is_irq = DEBUG_SUPPORT && (want_halt_except || want_halt_irq) ?
// in progress, because that dphase could subsequently except, and sample the
// IRQ vector PC instead of the load/store instruction PC.
// delay_irq_entry also applies to IRQ-like debug entries.
assign trap_enter_vld =
CSR_M_TRAP && (exception_req_any ||
!delay_irq_entry && !debug_mode && irq_active && !loadstore_dphase_pending) ||
@ -1343,7 +1344,7 @@ assign m_mode_trap_entry = !U_MODE || (
// - Privilege level of load/stores on the bus
// - Checking load/stores against PMP read/write permission
assign m_mode_loadstore = !U_MODE || debug_mode || ( // FIXME how does this interact with load/store racing against debug entry?
assign m_mode_loadstore = !U_MODE || debug_mode || (
mstatus_mprv ? mstatus_mpp : m_mode
);
@ -1435,6 +1436,18 @@ always @ (posedge clk) begin
assert(except == EXCEPT_EBREAK);
end
// Exceptions should squash load/stores before they reach dphase.
if (except != EXCEPT_NONE && !(except == EXCEPT_LOAD_FAULT || except == EXCEPT_STORE_FAULT)) begin
assert(!loadstore_dphase_pending);
end
// Debug entries are trap entries! (we should also assert in core.v that
// trap entries are never coincident with active load/store address
// phases, which gives some confidence that debug entry does not gobble
// load/stores.)
if (enter_debug_mode) begin
assert(trap_enter_vld);
end
end
`endif