Make WDT more modular for MCI reuse (#679)

* 1. Make WDT more moduler by removing the soc_ifc_pkg import and adding parameters to the module. - Better reusability with MCI 2. Move the wdt_timer*_timeout_serviced write restrictions from soc_ifc_top -> wdt. Better reusability with MCI. 3. Simplify moduler the timer*_count logic by creating new *_qual and *_restart signals that are separate from the timer*_count logic. - Code clean up 4. HW enfore the timer2_count cannot be reset by timer2_restart unless we are in independent mode. - New functionality 5. Add new fatal_error to WDT that is equivalent to nmi_intr logic in soc_ifc_top making it more resuable for MCI WDT. - Better reusability with MCI * Fix lint issues within wdt.sv and remove bitwise and boolean mixed logic * Add SOC_IFC_ prefix to WDT params in the soc_ifc_pkg.sv to avoid clashing with the wdt.sv params - PR request from Caleb. * Fix TB files due to WDT updates and wdt_error_t*_intr_serviced behavior has changed. The qualification now lives in the WDT module. * MICROSOFT AUTOMATED PIPELINE: Stamp 'ckuchta-msft-wdt-reuse' with updated timestamp and hash after successful run * 1. Make WDT more moduler by removing the soc_ifc_pkg import and adding parameters to the module. - Better reusability with MCI 2. Move the wdt_timer*_timeout_serviced write restrictions from soc_ifc_top -> wdt. Better reusability with MCI. 3. Simplify moduler the timer*_count logic by creating new *_qual and *_restart signals that are separate from the timer*_count logic. - Code clean up 4. HW enfore the timer2_count cannot be reset by timer2_restart unless we are in independent mode. - New functionality 5. Add new fatal_error to WDT that is equivalent to nmi_intr logic in soc_ifc_top making it more resuable for MCI WDT. - Better reusability with MCI * Fix lint issues within wdt.sv and remove bitwise and boolean mixed logic * Add SOC_IFC_ prefix to WDT params in the soc_ifc_pkg.sv to avoid clashing with the wdt.sv params - PR request from Caleb. * Fix TB files due to WDT updates and wdt_error_t*_intr_serviced behavior has changed. The qualification now lives in the WDT module. * 1. Make WDT more moduler by removing the soc_ifc_pkg import and adding parameters to the module. - Better reusability with MCI 2. Move the wdt_timer*_timeout_serviced write restrictions from soc_ifc_top -> wdt. Better reusability with MCI. 3. Simplify moduler the timer*_count logic by creating new *_qual and *_restart signals that are separate from the timer*_count logic. - Code clean up 4. HW enfore the timer2_count cannot be reset by timer2_restart unless we are in independent mode. - New functionality 5. Add new fatal_error to WDT that is equivalent to nmi_intr logic in soc_ifc_top making it more resuable for MCI WDT. - Better reusability with MCI * Fix lint issues within wdt.sv and remove bitwise and boolean mixed logic * Add SOC_IFC_ prefix to WDT params in the soc_ifc_pkg.sv to avoid clashing with the wdt.sv params - PR request from Caleb. * Fix TB files due to WDT updates and wdt_error_t*_intr_serviced behavior has changed. The qualification now lives in the WDT module. * 1. Make WDT more moduler by removing the soc_ifc_pkg import and adding parameters to the module. - Better reusability with MCI 2. Move the wdt_timer*_timeout_serviced write restrictions from soc_ifc_top -> wdt. Better reusability with MCI. 3. Simplify moduler the timer*_count logic by creating new *_qual and *_restart signals that are separate from the timer*_count logic. - Code clean up 4. HW enfore the timer2_count cannot be reset by timer2_restart unless we are in independent mode. - New functionality 5. Add new fatal_error to WDT that is equivalent to nmi_intr logic in soc_ifc_top making it more resuable for MCI WDT. - Better reusability with MCI * Fix typo in comment * Update pr_hash and pr_timestamp manually since pipeline is blocked by timingout regression
chipsalliance · Jan 9, 2025 · 6bff4e4 · 6bff4e4
1 parent 5381cfa
commit 6bff4e4
Show file tree

Hide file tree

Showing 8 changed files with 64 additions and 28 deletions.
diff --git a/.github/workflow_metadata/pr_hash b/.github/workflow_metadata/pr_hash
@@ -1 +1 @@
-43dfab7a70b0c74173df4e0d3da7140d71725ba41e9fff0d4a55f8ceb5ab777a805162ebdfe08c0595d99644268926de
+7563196b9c5ba8be0297cf9b6d1655ef81b98fe018fbd4ad37d05a539e7d5effb280748ac7b019c205e2d15bbbe7254f
diff --git a/.github/workflow_metadata/pr_timestamp b/.github/workflow_metadata/pr_timestamp
@@ -1 +1 @@
-1736301153
+1736449012
diff --git a/src/integration/asserts/caliptra_top_sva.sv b/src/integration/asserts/caliptra_top_sva.sv
@@ -586,13 +586,13 @@ module caliptra_top_sva
 
   cascade_wdt_t1_service: assert property (
     @(posedge `SVA_RDC_CLK)
-    (`WDT_PATH.wdt_timer1_timeout_serviced && !`WDT_PATH.timer2_en && !`WDT_PATH.t2_timeout) |=> (`WDT_PATH.timer1_count == 'h0)
+    (`WDT_PATH.wdt_timer1_timeout_serviced_qual && !`WDT_PATH.timer2_en && !`WDT_PATH.t2_timeout) |=> (`WDT_PATH.timer1_count == 'h0)
   )
   else $display("SVA ERROR: [Cascade] WDT Timer1 did not restart after interrupt service");
 
   cascade_wdt_t2_service: assert property (
     @(posedge `SVA_RDC_CLK)
-    (`WDT_PATH.wdt_timer2_timeout_serviced && !`WDT_PATH.timer2_en) |=> (`WDT_PATH.timer2_count == 'h0)
+    (`WDT_PATH.wdt_timer2_timeout_serviced_qual && !`WDT_PATH.timer2_en) |=> (`WDT_PATH.timer2_count == 'h0)
   )
   else $display("SVA ERROR: [Cascade] WDT Timer2 did not restart after interrupt service");
 
@@ -610,13 +610,13 @@ module caliptra_top_sva
 
   independent_wdt_t1_service: assert property (
     @(posedge `SVA_RDC_CLK)
-    (`WDT_PATH.wdt_timer1_timeout_serviced && `WDT_PATH.timer2_en && !`WDT_PATH.t2_timeout) |=> (`WDT_PATH.timer1_count == 'h0)
+    (`WDT_PATH.wdt_timer1_timeout_serviced_qual && `WDT_PATH.timer2_en && !`WDT_PATH.t2_timeout) |=> (`WDT_PATH.timer1_count == 'h0)
   )
   else $display("SVA ERROR: [Independent] WDT Timer1 did not restart after interrupt service");
 
   independent_wdt_t2_service: assert property (
     @(posedge `SVA_RDC_CLK)
-    (`WDT_PATH.wdt_timer2_timeout_serviced && `WDT_PATH.timer2_en) |=> (`WDT_PATH.timer2_count == 'h0)
+    (`WDT_PATH.wdt_timer2_timeout_serviced_qual && `WDT_PATH.timer2_en) |=> (`WDT_PATH.timer2_count == 'h0)
   )
   else $display("SVA ERROR: [Independent] WDT Timer2 did not restart after interrupt service");
 

diff --git a/src/integration/tb/caliptra_top_tb_services.sv b/src/integration/tb/caliptra_top_tb_services.sv
@@ -1343,10 +1343,10 @@ endgenerate //IV_NO
         end
         else begin
             if (!UVM_TB) begin
-                if(caliptra_top_dut.soc_ifc_top1.i_wdt.wdt_timer1_timeout_serviced) begin
+                if(caliptra_top_tb.caliptra_top_dut.soc_ifc_top1.i_wdt.wdt_timer1_timeout_serviced_qual) begin
                     set_wdt_timer1_period <= 'b1;
                 end
-                if(caliptra_top_dut.soc_ifc_top1.i_wdt.wdt_timer2_timeout_serviced) begin
+                if(caliptra_top_tb.caliptra_top_dut.soc_ifc_top1.i_wdt.wdt_timer2_timeout_serviced_qual) begin
                     set_wdt_timer2_period <= 'b1;
                 end
             end

diff --git a/src/soc_ifc/rtl/soc_ifc_external_reg.rdl b/src/soc_ifc/rtl/soc_ifc_external_reg.rdl
@@ -359,7 +359,7 @@ reg {
             [br]Caliptra Access: RW
             [br]SOC Access:      RO";
     field {desc = "WDT timer1 timeout period"; hw = r; sw = rw; swwel = soc_req; resetsignal = cptra_rst_b;} timer1_timeout_period[32] = 32'hFFFFFFFF;
-} CPTRA_WDT_TIMER1_TIMEOUT_PERIOD[2]; //This reflects WDT_TIMEOUT_PERIOD_NUM_DWORDS in soc_ifc_pkg
+} CPTRA_WDT_TIMER1_TIMEOUT_PERIOD[2]; //This reflects SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS in soc_ifc_pkg
 
 //Timer2
 reg {
@@ -385,7 +385,7 @@ reg {
             [br]Caliptra Access: RW
             [br]SOC Access:      RO";
     field {desc = "WDT timer2 timeout period"; hw = r; sw = rw; swwel = soc_req; resetsignal = cptra_rst_b;} timer2_timeout_period[32] = 32'hFFFFFFFF;
-} CPTRA_WDT_TIMER2_TIMEOUT_PERIOD[2]; //This reflects WDT_TIMEOUT_PERIOD_NUM_DWORDS in soc_ifc_pkg
+} CPTRA_WDT_TIMER2_TIMEOUT_PERIOD[2]; //This reflects SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS in soc_ifc_pkg
 
 //Status
 reg {

diff --git a/src/soc_ifc/rtl/soc_ifc_pkg.sv b/src/soc_ifc/rtl/soc_ifc_pkg.sv
@@ -38,8 +38,8 @@ package soc_ifc_pkg;
     parameter CPTRA_AXI_DMA_ID_WIDTH   = 5; // FIXME related to CALIPTRA_AXI_ID_WIDTH?
     parameter CPTRA_AXI_DMA_USER_WIDTH = 32;
 
-    parameter WDT_TIMEOUT_PERIOD_NUM_DWORDS = 2;
-    parameter WDT_TIMEOUT_PERIOD_W = WDT_TIMEOUT_PERIOD_NUM_DWORDS * 32;
+    parameter SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS = 2;
+    parameter SOC_IFC_WDT_TIMEOUT_PERIOD_W = SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS * 32;
 
     parameter SOC_IFC_REG_OFFSET = 32'h3000_0000;
 

diff --git a/src/soc_ifc/rtl/soc_ifc_top.sv b/src/soc_ifc/rtl/soc_ifc_top.sv
@@ -262,8 +262,8 @@ soc_ifc_reg__in_t soc_ifc_reg_hwif_in;
 soc_ifc_reg__out_t soc_ifc_reg_hwif_out;
 
 //WDT signals
-logic [WDT_TIMEOUT_PERIOD_NUM_DWORDS-1:0][31:0] timer1_timeout_period;
-logic [WDT_TIMEOUT_PERIOD_NUM_DWORDS-1:0][31:0] timer2_timeout_period;
+logic [SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS-1:0][31:0] timer1_timeout_period;
+logic [SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS-1:0][31:0] timer2_timeout_period;
 logic timer1_en;
 logic timer2_en;
 logic timer1_restart;
@@ -974,7 +974,6 @@ assign soc_ifc_notif_intr = soc_ifc_reg_hwif_out.intr_block_rf.notif_global_intr
 assign nmi_vector = soc_ifc_reg_hwif_out.internal_nmi_vector.vec.value;
 assign iccm_lock  = soc_ifc_reg_hwif_out.internal_iccm_lock.lock.value;
 assign clk_gating_en = soc_ifc_reg_hwif_out.CPTRA_CLK_GATING_EN.clk_gating_en.value;
-assign nmi_intr = t2_timeout && !timer2_en;             //Only issue nmi if WDT timers are cascaded and t2 times out
 
 // Interrupt output is set, for any enabled conditions, when a new write
 // sets CPTRA_FW_ERROR_FATAL or when a HW condition occurs that sets a bit
@@ -1159,7 +1158,7 @@ assign timer2_en = soc_ifc_reg_hwif_out.CPTRA_WDT_TIMER2_EN.timer2_en.value;
 assign timer1_restart = soc_ifc_reg_hwif_out.CPTRA_WDT_TIMER1_CTRL.timer1_restart.value;
 assign timer2_restart = soc_ifc_reg_hwif_out.CPTRA_WDT_TIMER2_CTRL.timer2_restart.value;
 
-for (genvar i = 0; i < WDT_TIMEOUT_PERIOD_NUM_DWORDS; i++) begin
+for (genvar i = 0; i < SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS; i++) begin
   assign timer1_timeout_period[i] = soc_ifc_reg_hwif_out.CPTRA_WDT_TIMER1_TIMEOUT_PERIOD[i].timer1_timeout_period.value;
   assign timer2_timeout_period[i] = soc_ifc_reg_hwif_out.CPTRA_WDT_TIMER2_TIMEOUT_PERIOD[i].timer2_timeout_period.value;
 end
@@ -1196,16 +1195,18 @@ always_ff @(posedge soc_ifc_clk_cg or negedge cptra_noncore_rst_b) begin
         wdt_error_t2_intr_serviced <= 1'b0;
     end
     else if (soc_ifc_reg_req_dv && soc_ifc_reg_req_data.write && (soc_ifc_reg_req_data.addr[SOC_IFC_REG_ADDR_WIDTH-1:0] == SOC_IFC_REG_ADDR_WIDTH'(`SOC_IFC_REG_INTR_BLOCK_RF_ERROR_INTERNAL_INTR_R))) begin
-        wdt_error_t1_intr_serviced <= soc_ifc_reg_req_data.wdata[`SOC_IFC_REG_INTR_BLOCK_RF_ERROR_INTERNAL_INTR_R_ERROR_WDT_TIMER1_TIMEOUT_STS_LOW] && t1_timeout;
-        wdt_error_t2_intr_serviced <= soc_ifc_reg_req_data.wdata[`SOC_IFC_REG_INTR_BLOCK_RF_ERROR_INTERNAL_INTR_R_ERROR_WDT_TIMER2_TIMEOUT_STS_LOW] && t2_timeout && timer2_en;
+        wdt_error_t1_intr_serviced <= soc_ifc_reg_req_data.wdata[`SOC_IFC_REG_INTR_BLOCK_RF_ERROR_INTERNAL_INTR_R_ERROR_WDT_TIMER1_TIMEOUT_STS_LOW];
+        wdt_error_t2_intr_serviced <= soc_ifc_reg_req_data.wdata[`SOC_IFC_REG_INTR_BLOCK_RF_ERROR_INTERNAL_INTR_R_ERROR_WDT_TIMER2_TIMEOUT_STS_LOW];
     end
     else begin
         wdt_error_t1_intr_serviced <= 1'b0;
         wdt_error_t2_intr_serviced <= 1'b0;
     end
 end
 
-wdt i_wdt (
+wdt #(
+    .WDT_TIMEOUT_PERIOD_NUM_DWORDS(SOC_IFC_WDT_TIMEOUT_PERIOD_NUM_DWORDS)
+)i_wdt (
     .clk(rdc_clk_cg),
     .cptra_rst_b(cptra_noncore_rst_b),
     .timer1_en(timer1_en),
@@ -1217,7 +1218,8 @@ wdt i_wdt (
     .wdt_timer1_timeout_serviced(wdt_error_t1_intr_serviced),
     .wdt_timer2_timeout_serviced(wdt_error_t2_intr_serviced),
     .t1_timeout(t1_timeout),
-    .t2_timeout(t2_timeout)
+    .t2_timeout(t2_timeout), 
+    .fatal_timeout(nmi_intr)   //Only issue nmi if WDT timers are cascaded and t2 times out
 );
 
 ////////////////////////////////////////////////////////

diff --git a/src/soc_ifc/rtl/wdt.sv b/src/soc_ifc/rtl/wdt.sv
@@ -13,7 +13,10 @@
 // limitations under the License.
 
 module wdt
-    import soc_ifc_pkg::*;
+    #(
+    parameter  WDT_TIMEOUT_PERIOD_NUM_DWORDS = 2,
+    localparam WDT_TIMEOUT_PERIOD_W = WDT_TIMEOUT_PERIOD_NUM_DWORDS * 32
+    )
 (
     input  logic clk,
     input  logic cptra_rst_b,
@@ -29,24 +32,54 @@ module wdt
     input  logic wdt_timer2_timeout_serviced,
     //WDT STATUS bits
     output logic t1_timeout,
-    output logic t2_timeout
+    output logic t2_timeout,
+    output logic fatal_timeout
+
 );
 
 //Timer1
 logic [WDT_TIMEOUT_PERIOD_W-1:0] timer1_count;
 logic timer1_count_en;
+logic wdt_timer1_timeout_serviced_qual;
+logic wdt_timer1_timeout_serviced_restart;
 
 //Timer2
 logic [WDT_TIMEOUT_PERIOD_W-1:0] timer2_count;
 logic timer2_count_en;
+logic wdt_timer2_timeout_serviced_qual;
+logic wdt_timer2_timeout_serviced_restart;
 
 //Output assignments
 assign t1_timeout = (timer1_count == timer1_timeout_period);
 assign t2_timeout = (timer2_count == timer2_timeout_period);
 
+
 assign timer1_count_en = timer1_en;
 assign timer2_count_en = !timer2_en ? t1_timeout : timer2_en;
 
+// In cascade mode if T2 timesout it is a fatal error.
+assign fatal_timeout = t2_timeout && !timer2_en;          
+
+
+// Timeout Service Logic
+
+// Only acknowledge the T1 timeout servicing request if T1 has timeout
+assign wdt_timer1_timeout_serviced_qual = wdt_timer1_timeout_serviced & t1_timeout; 
+
+// Only restart T1 via timeout service request if:
+//  1. In independent mode and T1 has timed out
+//  2. In cascade mode and T2 hasn't timed out
+assign wdt_timer1_timeout_serviced_restart = wdt_timer1_timeout_serviced_qual & (timer2_en | ~t2_timeout);
+
+
+// Only acknowledge the T2 timeout servicing request if T2 has timeout and in parallel mode
+assign wdt_timer2_timeout_serviced_qual = wdt_timer2_timeout_serviced & t2_timeout & timer2_en; 
+
+// Only restart T2 via timeout service request if:
+//  1. In cascade mode T2 hasn't timeout out and T1 has been serviced
+//  2. In independent mode and T2 has timed out
+assign wdt_timer2_timeout_serviced_restart = (wdt_timer1_timeout_serviced_qual & ~t2_timeout & ~timer2_en) | wdt_timer2_timeout_serviced_qual;
+
 
 //Timer1
 always_ff @(posedge clk or negedge cptra_rst_b) begin
@@ -60,7 +93,7 @@ always_ff @(posedge clk or negedge cptra_rst_b) begin
         //start count from 0 again. In all other cases, hold the count
         //Note: if t2 has timed out, expect a warm reset to re-init timers
         timer1_count <= timer1_restart && !t1_timeout ? 'h0 
-                                                    : (wdt_timer1_timeout_serviced && (timer2_en || !t2_timeout)) ? 'h0 
+                                                    : wdt_timer1_timeout_serviced_restart ? 'h0 
                                                     : !t1_timeout ? (timer1_count + 'h1)
                                                     : timer1_count;
     end
@@ -77,11 +110,12 @@ always_ff @(posedge clk or negedge cptra_rst_b) begin
         //hold the count. In case of time out, nmi and fatal error are triggered
         //Warm reset is expected to re-init timers
         //Note: if timer1 and timer2 are independently enabled, we want to remove the dependency of timer1 on timer2 and vice versa
-        timer2_count <= timer2_restart && !t2_timeout ? 'h0 
-                                                    : ((wdt_timer1_timeout_serviced && !t2_timeout && !timer2_en) || wdt_timer2_timeout_serviced) ? 'h0 
-                                                    : !t2_timeout ? (timer2_count + 'h1) 
-                                                    : timer2_count;
+        //Note: Only allow timer2_restart to reset T2 if in independent mode 
+        timer2_count <= timer2_restart && timer2_en && !t2_timeout ? 'h0 
+                                                                    : wdt_timer2_timeout_serviced_restart ? 'h0 
+                                                                    : !t2_timeout ? (timer2_count + 'h1) 
+                                                                    : timer2_count;
     end
 end
 
-endmodule
+endmodule
Original file line number	Diff line number	Diff line change
		@@ -1 +1 @@
		43dfab7a70b0c74173df4e0d3da7140d71725ba41e9fff0d4a55f8ceb5ab777a805162ebdfe08c0595d99644268926de
		7563196b9c5ba8be0297cf9b6d1655ef81b98fe018fbd4ad37d05a539e7d5effb280748ac7b019c205e2d15bbbe7254f