Skip to content

Commit 7cb9804

Browse files
nasahlpanasahlpa
committed
[chip,dv] Fix monitoring alert_major_internal
Currently, the `chip_sw_rv_core_ibex_lockstep_glitch` test fails with: ``` Check failed alert_major_internal == exp_alert_major_internal (0 [0x0] vs 1 [0x1]) Major alert did not match expectation. ``` which could be quite a serious issue if the lockstep comparison would not detect mismatches. However, it turns out that the testbench simply reads the `alert_major_internal` signal too late. As this signal is generated purely combinational, the logic asserts this signal as soon as we are flipping a bit. In the testbench, we though first wait a couple of cycles before reading this signal, which is often already too late. Signed-off-by: Pascal Nasahl <[email protected]>
1 parent 16463ab commit 7cb9804

File tree

1 file changed

+106
-97
lines changed

1 file changed

+106
-97
lines changed

‎hw/top_earlgrey/dv/env/seq_lib/chip_sw_rv_core_ibex_lockstep_glitch_vseq.sv‎

Lines changed: 106 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -583,6 +583,15 @@ class chip_sw_rv_core_ibex_lockstep_glitch_vseq extends chip_sw_base_vseq;
583583
endcase
584584
end
585585

586+
// Path to the alert_major_internal signal inside the lockstep.
587+
alert_major_internal_path = $sformatf("%s.alert_major_internal_o", lockstep_path);
588+
alert_major_internal = 1'b0;
589+
590+
// An alert should be triggered, so we check for that. Depending on the glitched signal and
591+
// core it may take several clock cycles for a potential alert to fire. We wait for at most
592+
// max_delay_clks cycles.
593+
max_delay_clks = 10 + lockstep_offset;
594+
586595
// The MuBi encoded enable_cmp_q signal is responsible for enabling/disabling the
587596
// lockstep comparison. Due to the encoding and the implemented enabling/disabling
588597
// logic, we tolerate a fault into this signal. Even when faulting this signal,
@@ -602,115 +611,115 @@ class chip_sw_rv_core_ibex_lockstep_glitch_vseq extends chip_sw_base_vseq;
602611
// Force the glitched value onto the port for one cycle, then release it again.
603612
`DV_CHECK_FATAL(uvm_hdl_force(glitch_path, glitched_val));
604613
`uvm_info(`gfn, $sformatf("Forcing %s to value 'h%0x.", glitch_path, glitched_val), UVM_LOW)
605-
if (!glitch_lockstep_core && glitched_port_is_inp) begin
606-
// The input ports of the ibex_core module are defined as `logic` without an explicit net
607-
// type. According to the standard, simulation tools are thus supposed to model these inputs
608-
// using a `var` type. However, it turns out that some tools collapse input ports into a
609-
// single object to reduce the number of assignments to improve simulation performance.
610-
// As a result, glitches inserted to inputs of the non-lockstep core may propagate back and
611-
// also change the input of the lockstep core. If this happens, both cores are glitched
612-
// simultaneously without any alerts firing.
613-
//
614-
// To avoid this, we also glitch the corresponding input of the ibex_lockstep instance to
615-
// the opposite value. The ibex_lockstep instance is embedded inside prim_buf cells across
616-
// which glitches don't propagate back. Also, the delay lines are embedded inside the
617-
// ibex_lockstep instance. It's thus fine to apply the glitch simultaneously.
618-
//
619-
// It's further worth noting that:
620-
//
621-
// 1. For some top-level inputs there may exist single points of failure, e.g. some inputs
622-
// without integrity protection, some control signals without spurious enable detection.
623-
// There are always such single points of failures. Where and how many there are depends
624-
// on the surrounding modules, backend etc. and is out of scope for the lockstep
625-
// countermeasure.
626-
// 2. To verify the lockstep countermeasure, we should actually be glitching internal core
627-
// signals instead of inputs to the ibex_core module. However, the problem with this is
628-
// that it's infeasible to always correctly model how the glitching of any internal signal
629-
// impacts core behavior and ultimately whether this should be detected. Forcing inputs
630-
// of ibex_core is a way to make the test feasible in the first place.
631-
//
632-
// For more details refer to https://github.com/lowRISC/ibex/pull/1967 .
633-
`DV_CHECK_FATAL(uvm_hdl_force(glitch_path_lockstep, orig_val));
634-
`uvm_info(`gfn, $sformatf("Forcing %s to value 'h%0x.", glitch_path_lockstep, orig_val),
635-
UVM_LOW)
636-
end
637-
cfg.chip_vif.cpu_clk_rst_if.wait_n_clks(1);
638-
if ((uvm_re_match("irq_*_i", port_name) != 0) && (port_name != "debug_req_i")) begin
639-
// If the port is not an interrupt or debug request, which are level-sensitive signals,
640-
// release the forcing at this point.
641-
`DV_CHECK_FATAL(uvm_hdl_release(glitch_path));
642-
`uvm_info(`gfn, $sformatf("Releasing force of %s.", glitch_path), UVM_LOW)
643-
if (!glitch_lockstep_core && glitched_port_is_inp) begin
644-
// In case we glitched an input port of the non-lockstep core, we must now also release
645-
// the force applied to the corresponding port of the ibex_lockstep instance.
646-
`DV_CHECK_FATAL(uvm_hdl_release(glitch_path_lockstep));
647-
`uvm_info(`gfn, $sformatf("Releasing force of %s.", glitch_path_lockstep), UVM_LOW)
648-
end
649-
end
650614

651-
// An alert should be triggered, so we check for that. Depending on the glitched signal and
652-
// core it may take several clock cycles for a potential alert to fire. We wait for at most
653-
// max_delay_clks cycles.
654-
max_delay_clks = 10 + lockstep_offset;
655-
656-
// Assert that `enable_cmp_q` in `ibex_lockstep` is 1. When coming out of reset and
657-
// starting execution, it takes `LockstepOffset` clock cycles for this to happen.
658-
for (int i = 0; i < lockstep_offset; i++) begin
659-
`DV_CHECK_FATAL(uvm_hdl_read(enable_cmp_path, enable_cmp))
660-
if (enable_cmp == ibex_pkg::IbexMuBiOn) begin
661-
break;
662-
end else begin
615+
fork
616+
begin : monitor_alert_major_internal
617+
// Check that `alert_major_internal_o` of `ibex_lockstep` matches our expectation. Depending
618+
// on the glitched signal and core it may take several clock cycles for a potential alert to
619+
// fire. We wait for at most max_delay_clks cycles.
620+
for (int i = 0; i <= max_delay_clks; i++) begin
621+
`uvm_info(`gfn, $sformatf("Checking for potential alert in cycle %0d.", i), UVM_LOW)
622+
`DV_CHECK_FATAL(uvm_hdl_read(alert_major_internal_path, alert_major_internal))
623+
if (alert_major_internal) begin
624+
break;
625+
end
626+
cfg.chip_vif.cpu_clk_rst_if.wait_n_clks(1);
627+
end
628+
end : monitor_alert_major_internal
629+
begin : determine_response
630+
if (!glitch_lockstep_core && glitched_port_is_inp) begin
631+
// The input ports of the ibex_core module are defined as `logic` without an explicit net
632+
// type. According to the standard, simulation tools are thus supposed to model these inputs
633+
// using a `var` type. However, it turns out that some tools collapse input ports into a
634+
// single object to reduce the number of assignments to improve simulation performance.
635+
// As a result, glitches inserted to inputs of the non-lockstep core may propagate back and
636+
// also change the input of the lockstep core. If this happens, both cores are glitched
637+
// simultaneously without any alerts firing.
638+
//
639+
// To avoid this, we also glitch the corresponding input of the ibex_lockstep instance to
640+
// the opposite value. The ibex_lockstep instance is embedded inside prim_buf cells across
641+
// which glitches don't propagate back. Also, the delay lines are embedded inside the
642+
// ibex_lockstep instance. It's thus fine to apply the glitch simultaneously.
643+
//
644+
// It's further worth noting that:
645+
//
646+
// 1. For some top-level inputs there may exist single points of failure, e.g. some inputs
647+
// without integrity protection, some control signals without spurious enable detection.
648+
// There are always such single points of failures. Where and how many there are depends
649+
// on the surrounding modules, backend etc. and is out of scope for the lockstep
650+
// countermeasure.
651+
// 2. To verify the lockstep countermeasure, we should actually be glitching internal core
652+
// signals instead of inputs to the ibex_core module. However, the problem with this is
653+
// that it's infeasible to always correctly model how the glitching of any internal signal
654+
// impacts core behavior and ultimately whether this should be detected. Forcing inputs
655+
// of ibex_core is a way to make the test feasible in the first place.
656+
//
657+
// For more details refer to https://github.com/lowRISC/ibex/pull/1967 .
658+
`DV_CHECK_FATAL(uvm_hdl_force(glitch_path_lockstep, orig_val));
659+
`uvm_info(`gfn, $sformatf("Forcing %s to value 'h%0x.", glitch_path_lockstep, orig_val),
660+
UVM_LOW)
661+
end
663662
cfg.chip_vif.cpu_clk_rst_if.wait_n_clks(1);
664-
end
665-
end
666-
`DV_CHECK_NE_FATAL(enable_cmp, ibex_pkg::IbexMuBiOff,
667-
"Lockstep comparison disabled, which is illegal.")
663+
if ((uvm_re_match("irq_*_i", port_name) != 0) && (port_name != "debug_req_i")) begin
664+
// If the port is not an interrupt or debug request, which are level-sensitive signals,
665+
// release the forcing at this point.
666+
`DV_CHECK_FATAL(uvm_hdl_release(glitch_path));
667+
`uvm_info(`gfn, $sformatf("Releasing force of %s.", glitch_path), UVM_LOW)
668+
if (!glitch_lockstep_core && glitched_port_is_inp) begin
669+
// In case we glitched an input port of the non-lockstep core, we must now also release
670+
// the force applied to the corresponding port of the ibex_lockstep instance.
671+
`DV_CHECK_FATAL(uvm_hdl_release(glitch_path_lockstep));
672+
`uvm_info(`gfn, $sformatf("Releasing force of %s.", glitch_path_lockstep), UVM_LOW)
673+
end
674+
end
668675

669-
// Calculate whether we expect a major alert.
670-
exp_alert_major_internal = 1'b0;
671-
if (glitched_port_is_inp) begin
672-
// Expect a major alert for a *used* glitched input.
673-
if (glitched_inp_used) begin
674-
exp_alert_major_internal = 1'b1;
675-
`uvm_info(`gfn, "Expecting an internal major alert because glitched input is used.",
676-
UVM_LOW)
677-
end else begin
676+
// An alert should be triggered, so we check for that. Depending on the glitched signal and
677+
// core it may take several clock cycles for a potential alert to fire. We wait for at most
678+
// max_delay_clks cycles.
679+
max_delay_clks = 10 + lockstep_offset;
680+
681+
// Assert that `enable_cmp_q` in `ibex_lockstep` is 1. When coming out of reset and
682+
// starting execution, it takes `LockstepOffset` clock cycles for this to happen.
683+
for (int i = 0; i < lockstep_offset; i++) begin
684+
`DV_CHECK_FATAL(uvm_hdl_read(enable_cmp_path, enable_cmp))
685+
if (enable_cmp == ibex_pkg::IbexMuBiOn) begin
686+
break;
687+
end else begin
688+
cfg.chip_vif.cpu_clk_rst_if.wait_n_clks(1);
689+
end
690+
end
691+
`DV_CHECK_NE_FATAL(enable_cmp, ibex_pkg::IbexMuBiOff,
692+
"Lockstep comparison disabled, which is illegal.")
693+
694+
// Calculate whether we expect a major alert.
678695
exp_alert_major_internal = 1'b0;
679-
`uvm_info(`gfn, "Expecting no internal major alert because glitched input is not used.",
680-
UVM_LOW)
681-
end
682-
end else begin
683-
// Always expect a major alert for a glitched output.
684-
exp_alert_major_internal = 1'b1;
685-
`uvm_info(`gfn, "Expecting an internal major alert due to glitched output.", UVM_LOW)
686-
end
696+
if (glitched_port_is_inp) begin
697+
// Expect a major alert for a *used* glitched input.
698+
if (glitched_inp_used) begin
699+
exp_alert_major_internal = 1'b1;
700+
`uvm_info(`gfn, "Expecting an internal major alert because glitched input is used.",
701+
UVM_LOW)
702+
end else begin
703+
exp_alert_major_internal = 1'b0;
704+
`uvm_info(`gfn, "Expecting no internal major alert because glitched input is not used.",
705+
UVM_LOW)
706+
end
707+
end else begin
708+
// Always expect a major alert for a glitched output.
709+
exp_alert_major_internal = 1'b1;
710+
`uvm_info(`gfn, "Expecting an internal major alert due to glitched output.", UVM_LOW)
711+
end
712+
end : determine_response
713+
join
687714

715+
// If we expect a major internal alert, check if we have seen it.
688716
if (exp_alert_major_internal) begin
689717
seen_top_level_alert = 1'b0;
690718
// Give the rv_core_ibex_fatal_hw_err alert a few more cycles to propagate out compared to the
691719
// alert signal at the ibex top level.
692720
check_alert_occurs("rv_core_ibex_fatal_hw_err", max_delay_clks + 5);
693721
end
694722

695-
// Check that `alert_major_internal_o` of `ibex_lockstep` matches our expectation. Depending on
696-
// the glitched signal and core it may take several clock cycles for a potential alert to fire.
697-
// We wait for at most max_delay_clks cycles.
698-
alert_major_internal_path = $sformatf("%s.alert_major_internal_o", lockstep_path);
699-
for (int i = 0; i <= max_delay_clks; i++) begin
700-
`uvm_info(`gfn, $sformatf("Checking for potential alert in cycle %0d.", i), UVM_MEDIUM)
701-
`DV_CHECK_FATAL(uvm_hdl_read(alert_major_internal_path, alert_major_internal))
702-
if (exp_alert_major_internal) begin
703-
if (alert_major_internal) begin
704-
`uvm_info(`gfn, $sformatf("Major alert expectedly fired in cycle %0d.", i), UVM_LOW)
705-
break;
706-
end
707-
end else begin
708-
`DV_CHECK_EQ_FATAL(alert_major_internal, exp_alert_major_internal,
709-
$sformatf("Major alert unexpectedly fired in cycle %0d.", i))
710-
end
711-
cfg.chip_vif.cpu_clk_rst_if.wait_n_clks(1);
712-
end
713-
714723
`DV_CHECK_EQ_FATAL(alert_major_internal, exp_alert_major_internal,
715724
"Major alert did not match expectation.")
716725

0 commit comments

Comments
 (0)