Skip to content

Commit

Permalink
hw: Improve debug parameterization, fix debug CSR RW (pulp-platform#27)
Browse files Browse the repository at this point in the history 
* hw: Make `dcsr`, `dpc`, and `dscratch0` CSRs accessible

* hw: Fully gate debug support in Snitch core

* hw: Assert `irq_i.debug` is raised only if debug is supported

---------

Co-authored-by: dbekatli <dbekatli@student.ethz.ch>
Co-authored-by: Luca Colagrande <luca.colagrande3@gmail.com>
  • Loading branch information
@paulsc96 @colluca
3 people committed Sep 28, 2023
1 parent 8ae6d3b commit 4174fa7
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 14 deletions.
52 changes: 42 additions & 10 deletions hw/snitch/src/snitch.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
parameter int unsigned NumDTLBEntries = 0,
parameter int unsigned NumITLBEntries = 0,
parameter snitch_pma_pkg::snitch_pma_t SnitchPMACfg = '{default: 0},
/// Enable debug support.
parameter bit DebugSupport = 1,
/// Derived parameter *Do not override*
parameter type addr_t = logic [AddrWidth-1:0],
parameter type data_t = logic [DataWidth-1:0]
Expand Down Expand Up @@ -289,10 +291,17 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
`FFAR(ssip_q, ssip_d, '0, clk_i, rst_i)
`FFAR(scip_q, scip_d, '0, clk_i, rst_i)

`FFAR(dcsr_q, dcsr_d, '0, clk_i, rst_i)
`FFNR(dpc_q, dpc_d, clk_i)
`FFNR(dscratch_q, dscratch_d, clk_i)
`FFAR(debug_q, debug_d, '0, clk_i, rst_i) // Debug mode
if (DebugSupport) begin : gen_debug
`FFAR(dcsr_q, dcsr_d, '0, clk_i, rst_i)
`FFNR(dpc_q, dpc_d, clk_i)
`FFNR(dscratch_q, dscratch_d, clk_i)
`FFAR(debug_q, debug_d, '0, clk_i, rst_i) // Debug mode
end else begin : gen_no_debug
assign dcsr_q = '0;
assign dpc_q = '0;
assign dscratch_q = '0;
assign debug_q = '0;
end

typedef struct packed {
fpnew_pkg::fmt_mode_t fmode;
Expand Down Expand Up @@ -475,7 +484,8 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
DmBaseAddress + dm::HaltAddress : DmBaseAddress + dm::ExceptionAddress;
end else begin
end
if (!debug_q && (irq_i.debug || dcsr_q.step)) pc_d = DmBaseAddress + dm::HaltAddress;
if (!debug_q && ((DebugSupport && irq_i.debug) || dcsr_q.step))
pc_d = DmBaseAddress + dm::HaltAddress;
end
end

Expand Down Expand Up @@ -523,7 +533,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
debug_d = (!debug_q && (
// the external debugger or an ebreak instruction triggerd the
// request to debug.
irq_i.debug ||
(DebugSupport && irq_i.debug) ||
// We encountered an ebreak and the default ebreak behaviour is switched off
(dcsr_q.ebreakm && inst_data_i == EBREAK) ||
// This was a single-step
Expand All @@ -533,7 +543,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
csr_en = 1'b0;
// Debug request and wake up are possibilties to move out of
// the low power state.
wfi_d = (irq_i.debug || debug_q || any_interrupt_pending) ? 1'b0 : wfi_q;
wfi_d = ((DebugSupport && irq_i.debug) || debug_q || any_interrupt_pending) ? 1'b0 : wfi_q;

unique casez (inst_data_i)
ADD: begin
Expand Down Expand Up @@ -2239,7 +2249,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(

// CSR logic
always_comb begin
csr_rvalue = 1'b0;
csr_rvalue = '0;
csr_dump = 1'b0;
illegal_csr = '0;
priv_lvl_d = priv_lvl_q;
Expand Down Expand Up @@ -2280,7 +2290,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
if (valid_instr && inst_data_i == EBREAK) begin
dpc_d = pc_q;
dcsr_d.cause = dm::CauseBreakpoint;
end else if (irq_i.debug) begin
end else if (DebugSupport && irq_i.debug) begin
dpc_d = npc;
dcsr_d.cause = dm::CauseRequest;
end else if (valid_instr && dcsr_q.step) begin
Expand Down Expand Up @@ -2321,6 +2331,25 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
CSR_MHARTID: begin
csr_rvalue = hart_id_i;
end
CSR_DCSR: begin
if (DebugSupport) begin
csr_rvalue = dcsr_q;
dcsr_d.ebreakm = alu_result[15];
dcsr_d.step = alu_result[2];
end else illegal_csr = 1'b1;
end
CSR_DPC: begin
if (DebugSupport) begin
csr_rvalue = dpc_q;
dpc_d = alu_result;
end else illegal_csr = 1'b1;
end
CSR_DSCRATCH0: begin
if (DebugSupport) begin
csr_rvalue = dscratch_q;
dscratch_d = alu_result;
end else illegal_csr = 1'b1;
end
`ifdef SNITCH_ENABLE_PERF
CSR_MCYCLE: begin
csr_rvalue = cycle_q[31:0];
Expand Down Expand Up @@ -2471,7 +2500,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
end else illegal_csr = 1'b1;
end
default: begin
csr_rvalue = 1'b0;
csr_rvalue = '0;
csr_dump = 1'b1;
end
endcase
Expand Down Expand Up @@ -2902,4 +2931,7 @@ module snitch import snitch_pkg::*; import riscv_instr::*; #(
// Make sure that without virtual memory support, translation is never enabled
`ASSERT(NoVMSupportNoTranslation, (~VMSupport |-> ~trans_active), clk_i, rst_i)

// Make sure debug IRQ line is not raised when debug mode is not supported
`ASSERT(DebugModeUnsupported, irq_i.debug == 1'b1 |-> DebugSupport == 1, clk_i, rst_i)

endmodule
5 changes: 4 additions & 1 deletion hw/snitch_cluster/src/snitch_cc.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,8 @@ module snitch_cc #(
/// Insert Pipeline registers immediately after FPU datapath
parameter bit RegisterFPUOut = 0,
parameter snitch_pma_pkg::snitch_pma_t SnitchPMACfg = '{default: 0},
/// Enable debug support.
parameter bit DebugSupport = 1,
/// Derived parameter *Do not override*
parameter int unsigned TCDMPorts = (NumSsrs > 1 ? NumSsrs : 1),
parameter type addr_t = logic [AddrWidth-1:0],
Expand Down Expand Up @@ -222,7 +224,8 @@ module snitch_cc #(
.XFVEC (XFVEC),
.XFDOTP (XFDOTP),
.XFAUX (XFauxMerged),
.FLEN (FLEN)
.FLEN (FLEN),
.DebugSupport (DebugSupport)
) i_snitch (
.clk_i ( clk_d2_i ), // if necessary operate on half the frequency
.rst_i ( ~rst_ni ),
Expand Down
7 changes: 5 additions & 2 deletions hw/snitch_cluster/src/snitch_cluster.sv
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,9 @@ module snitch_cluster
// value here. This only applies to the TCDM. The instruction cache macros will break!
// In case you are using the `RegisterTCDMCuts` feature this adds an
// additional cycle latency, which is taken into account here.
parameter int unsigned MemoryMacroLatency = 1 + RegisterTCDMCuts
parameter int unsigned MemoryMacroLatency = 1 + RegisterTCDMCuts,
/// Enable debug support.
parameter bit DebugSupport = 1
) (
/// System clock. If `IsoCrossing` is enabled this port is the _fast_ clock.
/// The slower, half-frequency clock, is derived internally.
Expand Down Expand Up @@ -868,7 +870,8 @@ module snitch_cluster
.RegisterSequencer (RegisterSequencer),
.RegisterFPUIn (RegisterFPUIn),
.RegisterFPUOut (RegisterFPUOut),
.TCDMAddrWidth (TCDMAddrWidth)
.TCDMAddrWidth (TCDMAddrWidth),
.DebugSupport (DebugSupport)
) i_snitch_cc (
.clk_i,
.clk_d2_i (clk_d2),
Expand Down
3 changes: 2 additions & 1 deletion hw/snitch_cluster/src/snitch_cluster_wrapper.sv.tpl
View file
Original file line number Diff line number Diff line change
Expand Up @@ -331,7 +331,8 @@ module ${cfg['name']}_wrapper (
.NarrowMaxMstTrans (${cfg['narrow_trans']}),
.NarrowMaxSlvTrans (${cfg['narrow_trans']}),
.sram_cfg_t (${cfg['pkg_name']}::sram_cfg_t),
.sram_cfgs_t (${cfg['pkg_name']}::sram_cfgs_t)
.sram_cfgs_t (${cfg['pkg_name']}::sram_cfgs_t),
.DebugSupport (${int(cfg['enable_debug'])})
) i_cluster (
.clk_i,
.rst_ni,
Expand Down

0 comments on commit 4174fa7

Please sign in to comment.