Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[pwrmgr] Don't lock CONTROL until observing core_sleep #23500

Merged
merged 2 commits into from
Jun 5, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
// SPDX-License-Identifier: Apache-2.0
// Decription:
// Create low power transition and wakeup a few times.
// When PWRMGR.CONTROL.LOW_POWER_HINT is set,
// When PWRMGR.CONTROL.LOW_POWER_HINT is set and core_sleep is high,
// issue random write to PWRMGR.CONTROL and check
// PWRMGR.CONTROL value is not changed.
// PWRMGR.CONTROL value is not changed, except for LOW_POWER_HINT.
class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
`uvm_object_utils(pwrmgr_sec_cm_ctrl_config_regwen_vseq)

Expand All @@ -17,7 +17,12 @@ class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
endtask : pre_start

task proc_illegal_ctrl_access();
uvm_reg_data_t wdata, expdata;
uvm_reg_data_t wdata, expdata, compare_mask;
// CONTROL.LOW_POWER_HINT is hardware-writeable, so mask it from checking.
// It gets cleared very quickly.
compare_mask = '1;
compare_mask = compare_mask - ral.control.low_power_hint.get_field_mask();

cfg.clk_rst_vif.wait_clks(1);
wait(cfg.pwrmgr_vif.lowpwr_cfg_wen == 0);

Expand All @@ -26,13 +31,18 @@ class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
expdata = ral.control.get();
`uvm_info(`gfn, $sformatf("csr start %x", ral.control.get()), UVM_HIGH)
csr_wr(.ptr(ral.control), .value(wdata));
csr_rd_check(.ptr(ral.control), .compare_value(expdata));
csr_rd_check(.ptr(ral.control), .compare_value(expdata), .compare_mask(compare_mask));
`uvm_info(`gfn, "csr done", UVM_HIGH)
end
endtask : proc_illegal_ctrl_access

virtual task wait_for_csr_to_propagate_to_slow_domain();
proc_illegal_ctrl_access();
super.wait_for_csr_to_propagate_to_slow_domain();
virtual task initiate_low_power_transition();
super.initiate_low_power_transition();
// The access checks can only happen if the bus is powered and the clock
// is active.
if ((ral.control.main_pd_n.get_mirrored_value() == 1'b1) &&
(ral.control.io_clk_en.get_mirrored_value() == 1'b1)) begin
proc_illegal_ctrl_access();
end
endtask
endclass : pwrmgr_sec_cm_ctrl_config_regwen_vseq
7 changes: 5 additions & 2 deletions hw/ip_templates/pwrmgr/dv/env/seq_lib/pwrmgr_wakeup_vseq.sv
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ class pwrmgr_wakeup_vseq extends pwrmgr_base_vseq;
update_control_csr();

// Initiate low power transition.
cfg.pwrmgr_vif.update_cpu_sleeping(1'b1);
set_nvms_idle();
initiate_low_power_transition();

if (ral.control.main_pd_n.get_mirrored_value() == 1'b0) begin
wait_for_reset_cause(pwrmgr_pkg::LowPwrEntry);
Expand Down Expand Up @@ -124,4 +123,8 @@ class pwrmgr_wakeup_vseq extends pwrmgr_base_vseq;
clear_wake_info();
endtask

virtual task initiate_low_power_transition();
cfg.pwrmgr_vif.update_cpu_sleeping(1'b1);
set_nvms_idle();
endtask
endclass : pwrmgr_wakeup_vseq
6 changes: 4 additions & 2 deletions hw/ip_templates/pwrmgr/rtl/pwrmgr.sv
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ module pwrmgr
prim_mubi_pkg::mubi4_t rom_ctrl_good;

logic core_sleeping;
logic low_power_entry;

////////////////////////////
/// clk_slow_i domain declarations
Expand Down Expand Up @@ -339,7 +340,7 @@ module pwrmgr
lowpwr_cfg_wen <= 1'b1;
end else if (!lowpwr_cfg_wen && (clr_cfg_lock || wkup)) begin
lowpwr_cfg_wen <= 1'b1;
end else if (low_power_hint) begin
end else if (low_power_entry) begin
lowpwr_cfg_wen <= 1'b0;
end
end
Expand Down Expand Up @@ -577,6 +578,7 @@ module pwrmgr
////////////////////////////

assign low_power_hint = reg2hw.control.low_power_hint.q == LowPower;
assign low_power_entry = core_sleeping & low_power_hint;

pwrmgr_fsm u_fsm (
.clk_i,
Expand All @@ -590,7 +592,7 @@ module pwrmgr
.ack_pwrup_o (ack_pwrup),
.req_pwrdn_o (req_pwrdn),
.ack_pwrdn_i (ack_pwrdn),
.low_power_entry_i (core_sleeping & low_power_hint),
.low_power_entry_i (low_power_entry),
.reset_reqs_i (peri_reqs_masked.rstreqs),
.fsm_invalid_i (fsm_invalid),
.clr_slow_req_o (clr_slow_req),
Expand Down
6 changes: 6 additions & 0 deletions hw/top_earlgrey/dv/chip_sim_cfg.hjson
Original file line number Diff line number Diff line change
Expand Up @@ -1854,6 +1854,12 @@
sw_images: ["//sw/device/tests:pwrmgr_normal_sleep_all_wake_ups:1:new_rules"]
en_run_modes: ["sw_test_mode_test_rom"]
}
{
name: chip_sw_pwrmgr_lowpower_cancel
uvm_test_seq: "chip_sw_base_vseq"
sw_images: ["//sw/device/tests:pwrmgr_lowpower_cancel_test:1:new_rules"]
en_run_modes: ["sw_test_mode_test_rom"]
}
{
name: chip_sw_pwrmgr_deep_sleep_all_wake_ups
uvm_test_seq: "chip_sw_pwrmgr_deep_sleep_all_wake_ups_vseq"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@
// SPDX-License-Identifier: Apache-2.0
// Decription:
// Create low power transition and wakeup a few times.
// When PWRMGR.CONTROL.LOW_POWER_HINT is set,
// When PWRMGR.CONTROL.LOW_POWER_HINT is set and core_sleep is high,
// issue random write to PWRMGR.CONTROL and check
// PWRMGR.CONTROL value is not changed.
// PWRMGR.CONTROL value is not changed, except for LOW_POWER_HINT.
class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
`uvm_object_utils(pwrmgr_sec_cm_ctrl_config_regwen_vseq)

Expand All @@ -17,7 +17,12 @@ class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
endtask : pre_start

task proc_illegal_ctrl_access();
uvm_reg_data_t wdata, expdata;
uvm_reg_data_t wdata, expdata, compare_mask;
// CONTROL.LOW_POWER_HINT is hardware-writeable, so mask it from checking.
// It gets cleared very quickly.
compare_mask = '1;
compare_mask = compare_mask - ral.control.low_power_hint.get_field_mask();

cfg.clk_rst_vif.wait_clks(1);
wait(cfg.pwrmgr_vif.lowpwr_cfg_wen == 0);

Expand All @@ -26,13 +31,18 @@ class pwrmgr_sec_cm_ctrl_config_regwen_vseq extends pwrmgr_wakeup_vseq;
expdata = ral.control.get();
`uvm_info(`gfn, $sformatf("csr start %x", ral.control.get()), UVM_HIGH)
csr_wr(.ptr(ral.control), .value(wdata));
csr_rd_check(.ptr(ral.control), .compare_value(expdata));
csr_rd_check(.ptr(ral.control), .compare_value(expdata), .compare_mask(compare_mask));
`uvm_info(`gfn, "csr done", UVM_HIGH)
end
endtask : proc_illegal_ctrl_access

virtual task wait_for_csr_to_propagate_to_slow_domain();
proc_illegal_ctrl_access();
super.wait_for_csr_to_propagate_to_slow_domain();
virtual task initiate_low_power_transition();
super.initiate_low_power_transition();
// The access checks can only happen if the bus is powered and the clock
// is active.
if ((ral.control.main_pd_n.get_mirrored_value() == 1'b1) &&
(ral.control.io_clk_en.get_mirrored_value() == 1'b1)) begin
proc_illegal_ctrl_access();
end
endtask
endclass : pwrmgr_sec_cm_ctrl_config_regwen_vseq
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,7 @@ class pwrmgr_wakeup_vseq extends pwrmgr_base_vseq;
update_control_csr();

// Initiate low power transition.
cfg.pwrmgr_vif.update_cpu_sleeping(1'b1);
set_nvms_idle();
initiate_low_power_transition();

if (ral.control.main_pd_n.get_mirrored_value() == 1'b0) begin
wait_for_reset_cause(pwrmgr_pkg::LowPwrEntry);
Expand Down Expand Up @@ -124,4 +123,8 @@ class pwrmgr_wakeup_vseq extends pwrmgr_base_vseq;
clear_wake_info();
endtask

virtual task initiate_low_power_transition();
cfg.pwrmgr_vif.update_cpu_sleeping(1'b1);
set_nvms_idle();
endtask
endclass : pwrmgr_wakeup_vseq
6 changes: 4 additions & 2 deletions hw/top_earlgrey/ip_autogen/pwrmgr/rtl/pwrmgr.sv
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ module pwrmgr
prim_mubi_pkg::mubi4_t rom_ctrl_good;

logic core_sleeping;
logic low_power_entry;

////////////////////////////
/// clk_slow_i domain declarations
Expand Down Expand Up @@ -339,7 +340,7 @@ module pwrmgr
lowpwr_cfg_wen <= 1'b1;
end else if (!lowpwr_cfg_wen && (clr_cfg_lock || wkup)) begin
lowpwr_cfg_wen <= 1'b1;
end else if (low_power_hint) begin
end else if (low_power_entry) begin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's important to have the configuration locked for the synchronization into the slow domain, we can probably do something about that. I noticed that is one other visible bit that is dropped.

If it's not important, we can move on!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it should be okay.

lowpwr_cfg_wen <= 1'b0;
end
end
Expand Down Expand Up @@ -577,6 +578,7 @@ module pwrmgr
////////////////////////////

assign low_power_hint = reg2hw.control.low_power_hint.q == LowPower;
assign low_power_entry = core_sleeping & low_power_hint;

pwrmgr_fsm u_fsm (
.clk_i,
Expand All @@ -590,7 +592,7 @@ module pwrmgr
.ack_pwrup_o (ack_pwrup),
.req_pwrdn_o (req_pwrdn),
.ack_pwrdn_i (ack_pwrdn),
.low_power_entry_i (core_sleeping & low_power_hint),
.low_power_entry_i (low_power_entry),
.reset_reqs_i (peri_reqs_masked.rstreqs),
.fsm_invalid_i (fsm_invalid),
.clr_slow_req_o (clr_slow_req),
Expand Down
25 changes: 25 additions & 0 deletions sw/device/tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -5908,6 +5908,31 @@ opentitan_test(
],
)

opentitan_test(
name = "pwrmgr_lowpower_cancel_test",
srcs = ["pwrmgr_lowpower_cancel_test.c"],
exec_env = dicts.add(
EARLGREY_TEST_ENVS,
EARLGREY_SILICON_OWNER_ROM_EXT_ENVS,
),
deps = [
"//hw/top_earlgrey/ip_autogen/pwrmgr/data:pwrmgr_c_regs",
"//hw/top_earlgrey/sw/autogen:top_earlgrey",
"//sw/device/lib/arch:device",
"//sw/device/lib/base:mmio",
"//sw/device/lib/dif:aon_timer",
"//sw/device/lib/dif:pwrmgr",
"//sw/device/lib/dif:rv_plic",
"//sw/device/lib/runtime:ibex",
"//sw/device/lib/runtime:irq",
"//sw/device/lib/runtime:log",
"//sw/device/lib/testing:aon_timer_testutils",
"//sw/device/lib/testing:pwrmgr_testutils",
"//sw/device/lib/testing:rv_plic_testutils",
"//sw/device/lib/testing/test_framework:ottf_main",
],
)

opentitan_test(
name = "sram_ctrl_lc_escalation_test",
srcs = ["sram_ctrl_lc_escalation_test.c"],
Expand Down
Loading
Loading