Patrick Rudolph has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/55114 )
Change subject: [RFC]src/soc/intel/common/block/pmc: Don't link get_int_option in SMM ......................................................................
[RFC]src/soc/intel/common/block/pmc: Don't link get_int_option in SMM
The option backend used in SMM has several disadvantages: - Restoring the initial state when entering SMM is difficult - Accessing hardware is slow and steals time from ring 0 - When using PCI hardware additional checks on decoded memory regions would be necessary - It links in a lot of code, which defeats the aim to have a minimal SMM handler
Pass the power_on_after_fail option to SMM by using GNVS instead of including the option backend into SMM. With this change the option can no longer be updated at runtime.
Change-Id: I60986979d7f7905b312b7d03abd46ac0465ae4cf Signed-off-by: Patrick Rudolph patrick.rudolph@9elements.com --- M src/soc/intel/alderlake/pmc.c M src/soc/intel/apollolake/pmc.c M src/soc/intel/cannonlake/pmc.c M src/soc/intel/common/block/include/intelblocks/nvs.h M src/soc/intel/common/block/include/intelblocks/pmclib.h M src/soc/intel/common/block/pmc/pmclib.c M src/soc/intel/common/block/smm/smihandler.c M src/soc/intel/elkhartlake/pmc.c M src/soc/intel/icelake/pmc.c M src/soc/intel/jasperlake/acpi.c M src/soc/intel/jasperlake/pmc.c M src/soc/intel/skylake/pmc.c M src/soc/intel/tigerlake/pmc.c M src/soc/intel/xeon_sp/pmc.c 14 files changed, 51 insertions(+), 20 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/14/55114/1
diff --git a/src/soc/intel/alderlake/pmc.c b/src/soc/intel/alderlake/pmc.c index 03399c3..154d1c5b 100644 --- a/src/soc/intel/alderlake/pmc.c +++ b/src/soc/intel/alderlake/pmc.c @@ -14,6 +14,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <soc/soc_chip.h> @@ -76,7 +77,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); diff --git a/src/soc/intel/apollolake/pmc.c b/src/soc/intel/apollolake/pmc.c index 4cc7670..d2ba86c 100644 --- a/src/soc/intel/apollolake/pmc.c +++ b/src/soc/intel/apollolake/pmc.c @@ -7,6 +7,7 @@ #include <device/pci.h> #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> +#include <option.h> #include <soc/iomap.h> #include <soc/pm.h> #include <timer.h> @@ -94,5 +95,7 @@ /* Now that things have been logged clear out the PMC state. */ pmc_clear_prsts();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); } diff --git a/src/soc/intel/cannonlake/pmc.c b/src/soc/intel/cannonlake/pmc.c index d6c30a8..778b86c 100644 --- a/src/soc/intel/cannonlake/pmc.c +++ b/src/soc/intel/cannonlake/pmc.c @@ -7,6 +7,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h>
@@ -74,7 +75,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); diff --git a/src/soc/intel/common/block/include/intelblocks/nvs.h b/src/soc/intel/common/block/include/intelblocks/nvs.h index c98fa01..bd41ea9 100644 --- a/src/soc/intel/common/block/include/intelblocks/nvs.h +++ b/src/soc/intel/common/block/include/intelblocks/nvs.h @@ -13,7 +13,7 @@ u8 ppcm; /* 0x04 - Max PPC State */ u8 tlvl; /* 0x05 - Throttle Level Limit */ u8 lids; /* 0x06 - LID State */ - u8 unused_was_pwrs; /* 0x07 - AC Power State */ + u8 pag3; /* 0x07 - Power State After G3 */ u32 cbmc; /* 0x08 - 0xb coreboot Memory Console */ u64 pm1i; /* 0x0c - 0x13 PM1 wake status bit */ u64 gpei; /* 0x14 - 0x1b GPE wake status bit */ diff --git a/src/soc/intel/common/block/include/intelblocks/pmclib.h b/src/soc/intel/common/block/include/intelblocks/pmclib.h index ecc8166..3d2451c 100644 --- a/src/soc/intel/common/block/include/intelblocks/pmclib.h +++ b/src/soc/intel/common/block/include/intelblocks/pmclib.h @@ -198,7 +198,7 @@ * 2 == Keep Previous State * Keep in sync with `config MAINBOARD_POWER_FAILURE_STATE`. */ -enum { +enum pmc_after_g3_power_state { MAINBOARD_POWER_STATE_OFF, MAINBOARD_POWER_STATE_ON, MAINBOARD_POWER_STATE_PREVIOUS, @@ -209,6 +209,7 @@ * system should go into after power is reapplied. */ void pmc_soc_set_afterg3_en(bool on); + /* * Configure power state to go into when power is reapplied. * @@ -218,7 +219,8 @@ * `target_on` signifies that we are currently powering on, so that * MAINBOARD_POWER_STATE_PREVIOUS can be handled accordingly. */ -void pmc_set_power_failure_state(bool target_on); +void pmc_set_power_failure_state(bool target_on, + const enum pmc_after_g3_power_state power_on_after_fail);
/* * This function ensures that the duration programmed in the PchPmPwrCycDur will never be diff --git a/src/soc/intel/common/block/pmc/pmclib.c b/src/soc/intel/common/block/pmc/pmclib.c index aa5b362..a04a3ff 100644 --- a/src/soc/intel/common/block/pmc/pmclib.c +++ b/src/soc/intel/common/block/pmc/pmclib.c @@ -11,7 +11,6 @@ #include <intelblocks/pmclib.h> #include <intelblocks/gpio.h> #include <intelblocks/tco.h> -#include <option.h> #include <security/vboot/vboot_common.h> #include <soc/pm.h> #include <stdint.h> @@ -579,14 +578,12 @@ gpio_route_gpe(dw0, dw1, dw2); }
-void pmc_set_power_failure_state(const bool target_on) +void pmc_set_power_failure_state(const bool target_on, + const enum pmc_after_g3_power_state power_on_after_fail) { bool on;
- const unsigned int state = get_uint_option("power_on_after_fail", - CONFIG_MAINBOARD_POWER_FAILURE_STATE); - - switch (state) { + switch (power_on_after_fail) { case MAINBOARD_POWER_STATE_OFF: printk(BIOS_INFO, "Set power off after power failure.\n"); on = false; @@ -600,7 +597,8 @@ on = target_on; break; default: - printk(BIOS_WARNING, "WARNING: Unknown power-failure state: %d\n", state); + printk(BIOS_WARNING, "WARNING: Unknown power-failure state: %d\n", + power_on_after_fail); on = false; break; } diff --git a/src/soc/intel/common/block/smm/smihandler.c b/src/soc/intel/common/block/smm/smihandler.c index ff87d71..5d33043 100644 --- a/src/soc/intel/common/block/smm/smihandler.c +++ b/src/soc/intel/common/block/smm/smihandler.c @@ -215,7 +215,7 @@ /* Disable all GPE */ pmc_disable_all_gpe(); /* Set which state system will be after power reapplied */ - pmc_set_power_failure_state(false); + pmc_set_power_failure_state(false, gnvs->pag3); /* also iterates over all bridges on bus 0 */ busmaster_disable_on_bus(0);
diff --git a/src/soc/intel/elkhartlake/pmc.c b/src/soc/intel/elkhartlake/pmc.c index 45a368c..bc6b99d 100644 --- a/src/soc/intel/elkhartlake/pmc.c +++ b/src/soc/intel/elkhartlake/pmc.c @@ -7,6 +7,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <soc/soc_chip.h> @@ -58,7 +59,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); diff --git a/src/soc/intel/icelake/pmc.c b/src/soc/intel/icelake/pmc.c index ee40fee..b331a9f 100644 --- a/src/soc/intel/icelake/pmc.c +++ b/src/soc/intel/icelake/pmc.c @@ -7,6 +7,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <soc/soc_chip.h> @@ -58,7 +59,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
pmc_set_acpi_mode(); diff --git a/src/soc/intel/jasperlake/acpi.c b/src/soc/intel/jasperlake/acpi.c index f73766a..1f77128 100644 --- a/src/soc/intel/jasperlake/acpi.c +++ b/src/soc/intel/jasperlake/acpi.c @@ -259,6 +259,10 @@ gnvs->u2we = config->usb2_wake_enable_bitmap; gnvs->u3we = config->usb3_wake_enable_bitmap;
+ /* Set Power State after G3 */ + gnvs->pag3 = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + /* Fill in Above 4GB MMIO resource */ sa_fill_gnvs(gnvs); } diff --git a/src/soc/intel/jasperlake/pmc.c b/src/soc/intel/jasperlake/pmc.c index dce791c..3b0b51d 100644 --- a/src/soc/intel/jasperlake/pmc.c +++ b/src/soc/intel/jasperlake/pmc.c @@ -7,6 +7,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <soc/soc_chip.h> @@ -58,7 +59,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); diff --git a/src/soc/intel/skylake/pmc.c b/src/soc/intel/skylake/pmc.c index e797b8f..1ed3cad 100644 --- a/src/soc/intel/skylake/pmc.c +++ b/src/soc/intel/skylake/pmc.c @@ -8,6 +8,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <reg_script.h> #include <soc/pci_devs.h> #include <soc/pm.h> @@ -94,7 +95,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
/* Note that certain bits may be cleared from running script as diff --git a/src/soc/intel/tigerlake/pmc.c b/src/soc/intel/tigerlake/pmc.c index 190f3ff..38a56c1 100644 --- a/src/soc/intel/tigerlake/pmc.c +++ b/src/soc/intel/tigerlake/pmc.c @@ -15,6 +15,7 @@ #include <intelblocks/pmclib.h> #include <intelblocks/pmc_ipc.h> #include <intelblocks/rtc.h> +#include <option.h> #include <soc/pci_devs.h> #include <soc/pm.h> #include <soc/soc_chip.h> @@ -76,7 +77,9 @@
rtc_init();
- pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
config_deep_s3(config->deep_s3_enable_ac, config->deep_s3_enable_dc); diff --git a/src/soc/intel/xeon_sp/pmc.c b/src/soc/intel/xeon_sp/pmc.c index b0c067b..07de002 100644 --- a/src/soc/intel/xeon_sp/pmc.c +++ b/src/soc/intel/xeon_sp/pmc.c @@ -6,6 +6,7 @@ #include <intelblocks/pmc.h> #include <intelblocks/pmclib.h> #include <intelblocks/rtc.h> +#include <option.h> #include <reg_script.h> #include <soc/pci_devs.h> #include <soc/pm.h> @@ -42,7 +43,9 @@
void pmc_soc_init(struct device *dev) { - pmc_set_power_failure_state(true); + const unsigned int state = get_uint_option("power_on_after_fail", + CONFIG_MAINBOARD_POWER_FAILURE_STATE); + pmc_set_power_failure_state(true, state); pmc_gpe_init();
/* Note that certain bits may be cleared from running script as