summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorFurquan Shaikh <furquan@google.com>2018-06-26 18:10:07 -0700
committerFurquan Shaikh <furquan@google.com>2018-06-28 05:00:12 +0000
commita00c7774d8be5802268c0139a405f1dde38971ea (patch)
tree6f1bfcfa4c1dd1ba67e2414629d3633262180350
parent14e8f20edca4e33fcd5fda8d0977ee96864e60b4 (diff)
downloadcoreboot-a00c7774d8be5802268c0139a405f1dde38971ea.tar.xz
soc/intel/common: Disable GPEs just before enabling SMIs
Call to pmc_disable_all_gpe is required before enabling SMIs to ensure that we do not end up in a recursive SMI handler loop as mentioned in change 74145f7 (intel/common/pmc: Disable all GPEs during pmc_init). Thus, this call was added at the end of pmc_fill_power_state as we want to ensure that all the GPE registers are backed up before being cleared for identifying the wake source in ramstage. This resulted in a side-effect on APL where pmc_fixup_power_state was called much later in the boot process. Even though we have got rid of pmc_fixup_power_state, this change moves the call to pmc_disable_all_gpe to happen just before enabling SMIs. This helps to keep the disabling of GPEs logically before the enabling of SMIs and any clean ups that happen in pmc or soc-specific code should not affect the state of GPEs. BUG=b:110836465 TEST=Verified that wake sources are correctly identified on KBL and APL. Also, no SMI handler issues observed when resuming. Change-Id: I122a8118edcec117f25beee71a23c0a44ae862ed Signed-off-by: Furquan Shaikh <furquan@google.com> Reviewed-on: https://review.coreboot.org/27251 Tested-by: build bot (Jenkins) <no-reply@coreboot.org> Reviewed-by: Aaron Durbin <adurbin@chromium.org>
-rw-r--r--src/soc/intel/common/block/pmc/pmclib.c12
-rw-r--r--src/soc/intel/common/block/smm/smm.c9
2 files changed, 9 insertions, 12 deletions
diff --git a/src/soc/intel/common/block/pmc/pmclib.c b/src/soc/intel/common/block/pmc/pmclib.c
index 0795990332..339e674a6e 100644
--- a/src/soc/intel/common/block/pmc/pmclib.c
+++ b/src/soc/intel/common/block/pmc/pmclib.c
@@ -416,18 +416,6 @@ int pmc_fill_power_state(struct chipset_power_state *ps)
ps->prev_sleep_state = pmc_prev_sleep_state(ps);
printk(BIOS_DEBUG, "prev_sleep_state %d\n", ps->prev_sleep_state);
- /*
- * GPEs need to be disabled before enabling SMI. Otherwise, it could
- * lead to SMIs being triggered in coreboot preventing the progress of
- * normal boot-up. However, GPEs should not be disabled as part of
- * pmc_gpe_init which happens in bootblock. Otherwise,
- * pmc_fill_power_state would read GPE0_EN registers as all 0s thus
- * losing information about the wake source. Hence,
- * pmc_disable_all_gpe() is placed here after GPE0_EN registers are
- * saved in chipset_power_state.
- */
- pmc_disable_all_gpe();
-
return ps->prev_sleep_state;
}
diff --git a/src/soc/intel/common/block/smm/smm.c b/src/soc/intel/common/block/smm/smm.c
index e8c52459a1..6059995493 100644
--- a/src/soc/intel/common/block/smm/smm.c
+++ b/src/soc/intel/common/block/smm/smm.c
@@ -46,6 +46,15 @@ void smm_southbridge_enable(uint16_t pm1_events)
pmc_disable_std_gpe(PME_B0_EN);
/*
+ * GPEs need to be disabled before enabling SMI. Otherwise, it could
+ * lead to SMIs being triggered in coreboot preventing the progress of
+ * normal boot-up. This is done as late as possible so that
+ * pmc_fill_power_state can read the correct state of GPE0_EN* registers
+ * and not lose information about the wake source.
+ */
+ pmc_disable_all_gpe();
+
+ /*
* Enable SMI generation:
* - on APMC writes (io 0xb2)
* - on writes to SLP_EN (sleep states)