From cb7dbb5bbe8a5105d98a6e1d7bdbef84126a5424 Mon Sep 17 00:00:00 2001 From: Pirmin Vogel Date: Thu, 25 Jul 2024 15:10:03 +0200 Subject: [PATCH] [prim_esc_receiver] Register esc_req_o to avoid potential CDC issues This is related to lowRISC/OpenTitan#24119. Signed-off-by: Pirmin Vogel --- hw/ip/prim/dv/prim_esc/tb/prim_esc_tb.sv | 16 +++++++++++++-- .../prim/fpv/vip/prim_esc_rxtx_assert_fpv.sv | 8 ++++---- hw/ip/prim/rtl/prim_esc_receiver.sv | 20 ++++++++++++++----- 3 files changed, 33 insertions(+), 11 deletions(-) diff --git a/hw/ip/prim/dv/prim_esc/tb/prim_esc_tb.sv b/hw/ip/prim/dv/prim_esc/tb/prim_esc_tb.sv index e3572df1dcde6..234fbe7cac11f 100644 --- a/hw/ip/prim/dv/prim_esc/tb/prim_esc_tb.sv +++ b/hw/ip/prim/dv/prim_esc/tb/prim_esc_tb.sv @@ -110,6 +110,8 @@ module prim_esc_tb; // Drive random length of esc_req and check `esc_req_out` and `integ_fail` outputs. main_clk.wait_clks($urandom_range(1, 20)); if (integ_fail) test_error("Esc_req unexpected signal integrity error!"); + // Wait for the escalation request to pass through the output register. + main_clk.wait_clks(1); if (!esc_req_out) test_error("Esc_req did not set esc_req!"); esc_req = 0; @@ -148,6 +150,8 @@ module prim_esc_tb; `DV_SPINWAIT(wait (integ_fail == 1);, , , "Wait for esc_tx.esc_n integ_fail timeout"); main_clk.wait_clks(1); release esc_tx.esc_n; + // Wait for the escalation request to pass through the output register. + main_clk.wait_clks(1); // Wait #1ps to avoid a race condition on clock edge. #1ps; if (!esc_req_out) test_error("Signal integrity error should set esc_req!"); @@ -161,6 +165,8 @@ module prim_esc_tb; `DV_SPINWAIT(wait (integ_fail == 1);, , , "Wait for esc_rx.resp_n integ_fail timeout"); main_clk.wait_clks(1); release esc_rx.resp_n; + // Wait for the escalation request to pass through the output register. + main_clk.wait_clks(1); // Wait #1ps to avoid a race condition on clock edge. #1ps; if (!esc_req_out) test_error("Signal integrity error should set esc_req!"); @@ -176,8 +182,12 @@ module prim_esc_tb; release esc_tx.esc_n; // Wait #1ps to avoid a race condition on clock edge. #1ps; - if (!esc_req_out) test_error("Signal integrity error should set esc_req!"); if (ping_ok) test_error("Ping error!"); + // Wait for the escalation request to pass through the output register. + main_clk.wait_clks(1); + // Wait #1ps to avoid a race condition on clock edge. + #1ps; + if (!esc_req_out) test_error("Signal integrity error should set esc_req!"); ping_req = 0; $display("[prim_esc_seq] Escalation esc_p/n integrity sequence during ping request finished!"); @@ -191,7 +201,7 @@ module prim_esc_tb; // After one ping_req, esc_receiver will start a counter to expect next ping_req. If the // counter reaches its max value but no ping_req has been received, design will set // `esc_req_out` signal. - main_clk.wait_clks(TIMEOUT_CYCLES + 1); + main_clk.wait_clks(TIMEOUT_CYCLES + 2); if (!esc_req_out) test_error("Design failed to detect ping request timeout!"); end begin @@ -200,6 +210,8 @@ module prim_esc_tb; main_clk.wait_clks(2); ping_req = 0; if (integ_fail) test_error("Ping_req unexpected signal integrity error!"); + // Wait for the escalation request to pass through the output register. + main_clk.wait_clks(1); if (esc_req_out) test_error("Ping request should not set esc_req_out!"); end join diff --git a/hw/ip/prim/fpv/vip/prim_esc_rxtx_assert_fpv.sv b/hw/ip/prim/fpv/vip/prim_esc_rxtx_assert_fpv.sv index d39fae3ff1ca3..86d8b4ce0790e 100644 --- a/hw/ip/prim/fpv/vip/prim_esc_rxtx_assert_fpv.sv +++ b/hw/ip/prim/fpv/vip/prim_esc_rxtx_assert_fpv.sv @@ -103,11 +103,11 @@ module prim_esc_rxtx_assert_fpv !rst_ni || error_present) - // check correct transmission of escalation within 0-1 cycles + // check correct transmission of escalation within 1-2 cycles `ASSERT(EscCheck_A, ##1 esc_req_i |-> - ##[0:1] esc_req_o, + ##[1:2] esc_req_o, clk_i, !rst_ni || error_present) @@ -155,7 +155,7 @@ module prim_esc_rxtx_assert_fpv !esc_req_o ##1 !ping_req_i [*0 : 2**TimeoutCntDw - 4] |-> - !esc_req_o, + ##1 !esc_req_o, clk_i, !rst_ni || error_d || @@ -169,7 +169,7 @@ module prim_esc_rxtx_assert_fpv !esc_req_o ##1 !ping_req_i [* 2**TimeoutCntDw - 3 : $] |-> - esc_req_o, + ##1 esc_req_o, clk_i, !rst_ni || error_d || diff --git a/hw/ip/prim/rtl/prim_esc_receiver.sv b/hw/ip/prim/rtl/prim_esc_receiver.sv index 2f01b93f0c74d..d537716575cbb 100644 --- a/hw/ip/prim/rtl/prim_esc_receiver.sv +++ b/hw/ip/prim/rtl/prim_esc_receiver.sv @@ -126,11 +126,21 @@ module prim_esc_receiver // - requested via the escalation sender/receiver path, // - the ping monitor timeout is reached, // - the two ping monitor counters are in an inconsistent state. - logic esc_req; + // Register the escalation request to avoid potential CDC issues downstream. + logic esc_req, esc_req_d, esc_req_q; + assign esc_req_d = esc_req || (&timeout_cnt) || timeout_cnt_error; + always_ff @(posedge clk_i or negedge rst_ni) begin + if (!rst_ni) begin + esc_req_q <= 1'b0; + end else begin + esc_req_q <= esc_req_d; + end + end + prim_sec_anchor_buf #( .Width(1) ) u_prim_buf_esc_req ( - .in_i(esc_req || (&timeout_cnt) || timeout_cnt_error), + .in_i (esc_req_q), .out_o(esc_req_o) ); @@ -257,7 +267,7 @@ module prim_esc_receiver `ASSERT(SigIntCheck0_A, esc_tx_i.esc_p == esc_tx_i.esc_n |=> esc_rx_o.resp_p == esc_rx_o.resp_n) `ASSERT(SigIntCheck1_A, esc_tx_i.esc_p == esc_tx_i.esc_n |=> state_q == SigInt) // auto-escalate in case of signal integrity issue - `ASSERT(SigIntCheck2_A, esc_tx_i.esc_p == esc_tx_i.esc_n |=> esc_req_o) + `ASSERT(SigIntCheck2_A, esc_tx_i.esc_p == esc_tx_i.esc_n |=> esc_req_d) // correct diff encoding `ASSERT(DiffEncCheck_A, esc_tx_i.esc_p ^ esc_tx_i.esc_n |=> esc_rx_o.resp_p ^ esc_rx_o.resp_n) // disable in case of signal integrity issue @@ -271,10 +281,10 @@ module prim_esc_receiver // detect escalation pulse `ASSERT(EscEnCheck_A, esc_tx_i.esc_p && (esc_tx_i.esc_p ^ esc_tx_i.esc_n) && state_q != SigInt - ##1 esc_tx_i.esc_p && (esc_tx_i.esc_p ^ esc_tx_i.esc_n) |-> esc_req_o) + ##1 esc_tx_i.esc_p && (esc_tx_i.esc_p ^ esc_tx_i.esc_n) |-> esc_req_d) // make sure the counter does not wrap around `ASSERT(EscCntWrap_A, &timeout_cnt |=> timeout_cnt != 0) // if the counter expires, escalation should be asserted - `ASSERT(EscCntEsc_A, &timeout_cnt |-> esc_req_o) + `ASSERT(EscCntEsc_A, &timeout_cnt |-> esc_req_d) endmodule : prim_esc_receiver