Change in ...coreboot[master]: sb/intel/ibexpeak/smihandler: Move finalizing to a common location
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33139
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
sb/intel/ibexpeak/smihandler: Move finalizing to a common location
TODO: There is no reason to do this in SMM.
Change-Id: I8bbb2f65bbe674bd1bc4ae8a4086bd1f5e9a79fa Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/packardbell/ms2290/smihandler.c M src/southbridge/intel/ibexpeak/smihandler.c 3 files changed, 7 insertions(+), 38 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/33139/1
diff --git a/src/mainboard/lenovo/x201/smihandler.c b/src/mainboard/lenovo/x201/smihandler.c index fc6a0e9..9043f8e 100644 --- a/src/mainboard/lenovo/x201/smihandler.c +++ b/src/mainboard/lenovo/x201/smihandler.c @@ -20,10 +20,7 @@ #include <cpu/x86/smm.h> #include <southbridge/intel/ibexpeak/nvs.h> #include <southbridge/intel/common/pmutil.h> -#include <southbridge/intel/ibexpeak/me.h> -#include <southbridge/intel/common/finalize.h> #include <northbridge/intel/nehalem/nehalem.h> -#include <cpu/intel/model_2065x/model_2065x.h> #include <ec/acpi/ec.h> #include <ec/lenovo/h8/h8.h> #include <delay.h> @@ -135,25 +132,9 @@ mainboard_smi_handle_ec_sci(); }
-static int mainboard_finalized = 0; - int mainboard_smi_apmc(u8 data) { switch (data) { - case APM_CNT_FINALIZE: - printk(BIOS_DEBUG, "APMC: FINALIZE\n"); - if (mainboard_finalized) { - printk(BIOS_DEBUG, "APMC#: Already finalized\n"); - return 0; - } - - intel_me_finalize_smm(); - intel_pch_finalize_smm(); - intel_nehalem_finalize_smm(); - intel_model_2065x_finalize_smm(); - - mainboard_finalized = 1; - break; case APM_CNT_ACPI_ENABLE: /* use 0x1600/0x1604 to prevent races with userspace */ ec_set_ports(0x1604, 0x1600); diff --git a/src/mainboard/packardbell/ms2290/smihandler.c b/src/mainboard/packardbell/ms2290/smihandler.c index 83562be..dfc66f9 100644 --- a/src/mainboard/packardbell/ms2290/smihandler.c +++ b/src/mainboard/packardbell/ms2290/smihandler.c @@ -18,11 +18,8 @@ #include <cpu/x86/smm.h> #include <device/pci_ops.h> #include <southbridge/intel/ibexpeak/nvs.h> -#include <southbridge/intel/ibexpeak/me.h> -#include <southbridge/intel/common/finalize.h> #include <southbridge/intel/common/pmutil.h> #include <northbridge/intel/nehalem/nehalem.h> -#include <cpu/intel/model_2065x/model_2065x.h> #include <ec/acpi/ec.h>
static void mainboard_smm_init(void) @@ -54,26 +51,10 @@ { }
-static int mainboard_finalized = 0; - int mainboard_smi_apmc(u8 data) { u8 tmp; switch (data) { - case APM_CNT_FINALIZE: - printk(BIOS_DEBUG, "APMC: FINALIZE\n"); - if (mainboard_finalized) { - printk(BIOS_DEBUG, "APMC#: Already finalized\n"); - return 0; - } - - intel_me_finalize_smm(); - intel_pch_finalize_smm(); - intel_nehalem_finalize_smm(); - intel_model_2065x_finalize_smm(); - - mainboard_finalized = 1; - break; case APM_CNT_ACPI_ENABLE: tmp = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xbb); tmp &= ~0x03; diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c index dbc412c..208075b 100644 --- a/src/southbridge/intel/ibexpeak/smihandler.c +++ b/src/southbridge/intel/ibexpeak/smihandler.c @@ -24,7 +24,10 @@ #include <elog.h> #include <halt.h> #include <pc80/mc146818rtc.h> +#include <cpu/intel/model_2065x/model_2065x.h> +#include <southbridge/intel/common/finalize.h> #include <southbridge/intel/common/pmbase.h> +#include <southbridge/intel/ibexpeak/me.h> #include "pch.h"
#include "nvs.h" @@ -182,4 +185,8 @@
void southbridge_finalize_all(void) { + intel_me_finalize_smm(); + intel_pch_finalize_smm(); + intel_nehalem_finalize_smm(); + intel_model_2065x_finalize_smm(); }
Angel Pons has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33139 )
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
Patch Set 1: Code-Review+1
Kyösti Mälkki has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33139 )
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
Patch Set 2: Code-Review+2
LGTM, although the log message "APMC: FINALIZE" is removed?
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33139 )
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
Patch Set 2:
Patch Set 2: Code-Review+2
LGTM, although the log message "APMC: FINALIZE" is removed?
I could add it to sb/intel/common/smihandler.c. OTOH on this platform it makes sense to move locking down out of SMM as no product ever depended on it.
HAOUAS Elyes has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/33139 )
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
Patch Set 2: Code-Review+1
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/33139 )
Change subject: sb/intel/ibexpeak/smihandler: Move finalizing to a common location ......................................................................
sb/intel/ibexpeak/smihandler: Move finalizing to a common location
TODO: There is no reason to do this in SMM.
Change-Id: I8bbb2f65bbe674bd1bc4ae8a4086bd1f5e9a79fa Signed-off-by: Arthur Heymans arthur@aheymans.xyz Reviewed-on: https://review.coreboot.org/c/coreboot/+/33139 Reviewed-by: Kyösti Mälkki kyosti.malkki@gmail.com Reviewed-by: HAOUAS Elyes ehaouas@noos.fr Reviewed-by: Angel Pons th3fanbus@gmail.com Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/mainboard/lenovo/x201/smihandler.c M src/mainboard/packardbell/ms2290/smihandler.c M src/southbridge/intel/ibexpeak/smihandler.c 3 files changed, 7 insertions(+), 38 deletions(-)
Approvals: build bot (Jenkins): Verified Kyösti Mälkki: Looks good to me, approved HAOUAS Elyes: Looks good to me, but someone else must approve Angel Pons: Looks good to me, but someone else must approve
diff --git a/src/mainboard/lenovo/x201/smihandler.c b/src/mainboard/lenovo/x201/smihandler.c index fc6a0e9..9043f8e 100644 --- a/src/mainboard/lenovo/x201/smihandler.c +++ b/src/mainboard/lenovo/x201/smihandler.c @@ -20,10 +20,7 @@ #include <cpu/x86/smm.h> #include <southbridge/intel/ibexpeak/nvs.h> #include <southbridge/intel/common/pmutil.h> -#include <southbridge/intel/ibexpeak/me.h> -#include <southbridge/intel/common/finalize.h> #include <northbridge/intel/nehalem/nehalem.h> -#include <cpu/intel/model_2065x/model_2065x.h> #include <ec/acpi/ec.h> #include <ec/lenovo/h8/h8.h> #include <delay.h> @@ -135,25 +132,9 @@ mainboard_smi_handle_ec_sci(); }
-static int mainboard_finalized = 0; - int mainboard_smi_apmc(u8 data) { switch (data) { - case APM_CNT_FINALIZE: - printk(BIOS_DEBUG, "APMC: FINALIZE\n"); - if (mainboard_finalized) { - printk(BIOS_DEBUG, "APMC#: Already finalized\n"); - return 0; - } - - intel_me_finalize_smm(); - intel_pch_finalize_smm(); - intel_nehalem_finalize_smm(); - intel_model_2065x_finalize_smm(); - - mainboard_finalized = 1; - break; case APM_CNT_ACPI_ENABLE: /* use 0x1600/0x1604 to prevent races with userspace */ ec_set_ports(0x1604, 0x1600); diff --git a/src/mainboard/packardbell/ms2290/smihandler.c b/src/mainboard/packardbell/ms2290/smihandler.c index 83562be..dfc66f9 100644 --- a/src/mainboard/packardbell/ms2290/smihandler.c +++ b/src/mainboard/packardbell/ms2290/smihandler.c @@ -18,11 +18,8 @@ #include <cpu/x86/smm.h> #include <device/pci_ops.h> #include <southbridge/intel/ibexpeak/nvs.h> -#include <southbridge/intel/ibexpeak/me.h> -#include <southbridge/intel/common/finalize.h> #include <southbridge/intel/common/pmutil.h> #include <northbridge/intel/nehalem/nehalem.h> -#include <cpu/intel/model_2065x/model_2065x.h> #include <ec/acpi/ec.h>
static void mainboard_smm_init(void) @@ -54,26 +51,10 @@ { }
-static int mainboard_finalized = 0; - int mainboard_smi_apmc(u8 data) { u8 tmp; switch (data) { - case APM_CNT_FINALIZE: - printk(BIOS_DEBUG, "APMC: FINALIZE\n"); - if (mainboard_finalized) { - printk(BIOS_DEBUG, "APMC#: Already finalized\n"); - return 0; - } - - intel_me_finalize_smm(); - intel_pch_finalize_smm(); - intel_nehalem_finalize_smm(); - intel_model_2065x_finalize_smm(); - - mainboard_finalized = 1; - break; case APM_CNT_ACPI_ENABLE: tmp = pci_read_config8(PCI_DEV(0, 0x1f, 0), 0xbb); tmp &= ~0x03; diff --git a/src/southbridge/intel/ibexpeak/smihandler.c b/src/southbridge/intel/ibexpeak/smihandler.c index dbc412c..208075b 100644 --- a/src/southbridge/intel/ibexpeak/smihandler.c +++ b/src/southbridge/intel/ibexpeak/smihandler.c @@ -24,7 +24,10 @@ #include <elog.h> #include <halt.h> #include <pc80/mc146818rtc.h> +#include <cpu/intel/model_2065x/model_2065x.h> +#include <southbridge/intel/common/finalize.h> #include <southbridge/intel/common/pmbase.h> +#include <southbridge/intel/ibexpeak/me.h> #include "pch.h"
#include "nvs.h" @@ -182,4 +185,8 @@
void southbridge_finalize_all(void) { + intel_me_finalize_smm(); + intel_pch_finalize_smm(); + intel_nehalem_finalize_smm(); + intel_model_2065x_finalize_smm(); }
-
Angel Pons (Code Review) -
Arthur Heymans (Code Review)
-
HAOUAS Elyes (Code Review)
-
Kyösti Mälkki (Code Review)
-
Patrick Georgi (Code Review)