[coreboot] Patch set updated for coreboot: 8987184 SMM: Avoid use of global variables in SMI handler

Stefan Reinauer (stefan.reinauer@coreboot.org) gerrit at coreboot.org
Tue Nov 13 20:12:32 CET 2012


Stefan Reinauer (stefan.reinauer at coreboot.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1764

-gerrit

commit 89871848fdb567fdc3046494c2ce3dc5e7e3cb44
Author: Duncan Laurie <dlaurie at chromium.org>
Date:   Wed Oct 3 19:01:57 2012 -0700

    SMM: Avoid use of global variables in SMI handler
    
    Using global variables with the TSEG is a bad idea because
    they are not relocated properly right now.  Instead make
    the variables static and add accessor functions for the
    rest of SMM to use.
    
    At the same time drop the tcg/smi1 pointers as they are
    not setup or ever used.  (the debug output is added back
    in a subsequent commit)
    
    Change-Id: If0b2d47df4e482ead71bf713c1ef748da840073b
    Signed-off-by: Duncan Laurie <dlaurie at chromium.org>
---
 src/include/cpu/x86/smm.h                        |  3 +++
 src/mainboard/intel/emeraldlake2/mainboard_smi.c |  7 +------
 src/mainboard/samsung/lumpy/mainboard_smi.c      | 12 +++---------
 src/mainboard/samsung/stumpy/mainboard_smi.c     |  7 +------
 src/southbridge/intel/bd82x6x/nvs.h              |  4 ++++
 src/southbridge/intel/bd82x6x/smihandler.c       | 21 ++++++++++++---------
 6 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/src/include/cpu/x86/smm.h b/src/include/cpu/x86/smm.h
index 82a5a1a..b52a315 100644
--- a/src/include/cpu/x86/smm.h
+++ b/src/include/cpu/x86/smm.h
@@ -384,4 +384,7 @@ u32 smi_get_tseg_base(void);
 void tseg_relocate(void **ptr);
 #endif
 
+/* Get PMBASE address */
+u16 smm_get_pmbase(void);
+
 #endif
diff --git a/src/mainboard/intel/emeraldlake2/mainboard_smi.c b/src/mainboard/intel/emeraldlake2/mainboard_smi.c
index 340562a..03c505b 100644
--- a/src/mainboard/intel/emeraldlake2/mainboard_smi.c
+++ b/src/mainboard/intel/emeraldlake2/mainboard_smi.c
@@ -27,17 +27,12 @@
 #include <northbridge/intel/sandybridge/sandybridge.h>
 #include <cpu/intel/model_206ax/model_206ax.h>
 
-/* The southbridge SMI handler checks whether gnvs has a
- * valid pointer before calling the trap handler
- */
-extern global_nvs_t *gnvs;
-
 int mainboard_io_trap_handler(int smif)
 {
 	switch (smif) {
 	case 0x99:
 		printk(BIOS_DEBUG, "Sample\n");
-		gnvs->smif = 0;
+		smm_get_gnvs()->smif = 0;
 		break;
 	default:
 		return 0;
diff --git a/src/mainboard/samsung/lumpy/mainboard_smi.c b/src/mainboard/samsung/lumpy/mainboard_smi.c
index 2d396b0..4e73a57 100644
--- a/src/mainboard/samsung/lumpy/mainboard_smi.c
+++ b/src/mainboard/samsung/lumpy/mainboard_smi.c
@@ -29,17 +29,12 @@
 #include <ec/smsc/mec1308/ec.h>
 #include "ec.h"
 
-/* The southbridge SMI handler checks whether gnvs has a
- * valid pointer before calling the trap handler
- */
-extern global_nvs_t *gnvs;
-
 int mainboard_io_trap_handler(int smif)
 {
 	switch (smif) {
 	case 0x99:
 		printk(BIOS_DEBUG, "Sample\n");
-		gnvs->smif = 0;
+		smm_get_gnvs()->smif = 0;
 		break;
 	default:
 		return 0;
@@ -59,7 +54,6 @@ static u8 mainboard_smi_ec(void)
 {
 	u8 cmd;
 	u32 pm1_cnt;
-	extern u16 pmbase; /* Set in southbridge SMI handler */
 
 	cmd = read_ec_command_byte(EC_GET_SMI_CAUSE);
 
@@ -68,9 +62,9 @@ static u8 mainboard_smi_ec(void)
 		printk(BIOS_DEBUG, "LID CLOSED, SHUTDOWN\n");
 
 		/* Go to S5 */
-		pm1_cnt = inl(pmbase + PM1_CNT);
+		pm1_cnt = inl(smm_get_pmbase() + PM1_CNT);
 		pm1_cnt |= (0xf << 10);
-		outl(pm1_cnt, pmbase + PM1_CNT);
+		outl(pm1_cnt, smm_get_pmbase() + PM1_CNT);
 		break;
 	}
 
diff --git a/src/mainboard/samsung/stumpy/mainboard_smi.c b/src/mainboard/samsung/stumpy/mainboard_smi.c
index 0b3024b..660bb31 100644
--- a/src/mainboard/samsung/stumpy/mainboard_smi.c
+++ b/src/mainboard/samsung/stumpy/mainboard_smi.c
@@ -30,17 +30,12 @@
 /* Include romstage serial for SIO helper functions */
 #include <superio/ite/it8772f/early_serial.c>
 
-/* The southbridge SMI handler checks whether gnvs has a
- * valid pointer before calling the trap handler
- */
-extern global_nvs_t *gnvs;
-
 int mainboard_io_trap_handler(int smif)
 {
 	switch (smif) {
 	case 0x99:
 		printk(BIOS_DEBUG, "Sample\n");
-		gnvs->smif = 0;
+		smm_get_gnvs()->smif = 0;
 		break;
 	default:
 		return 0;
diff --git a/src/southbridge/intel/bd82x6x/nvs.h b/src/southbridge/intel/bd82x6x/nvs.h
index 3fa0093..b8506d4 100644
--- a/src/southbridge/intel/bd82x6x/nvs.h
+++ b/src/southbridge/intel/bd82x6x/nvs.h
@@ -152,3 +152,7 @@ typedef struct {
 	chromeos_acpi_t chromeos;
 } __attribute__((packed)) global_nvs_t;
 
+#ifdef __SMM__
+/* Used in SMM to find the ACPI GNVS address */
+global_nvs_t *smm_get_gnvs(void);
+#endif
diff --git a/src/southbridge/intel/bd82x6x/smihandler.c b/src/southbridge/intel/bd82x6x/smihandler.c
index 250035c..8fea33f 100644
--- a/src/southbridge/intel/bd82x6x/smihandler.c
+++ b/src/southbridge/intel/bd82x6x/smihandler.c
@@ -25,7 +25,6 @@
 #include <arch/romcc_io.h>
 #include <console/console.h>
 #include <cpu/x86/cache.h>
-#include <cpu/x86/smm.h>
 #include <device/pci_def.h>
 #include <cpu/x86/smm.h>
 #include <elog.h>
@@ -44,15 +43,22 @@
 /* While we read PMBASE dynamically in case it changed, let's
  * initialize it with a sane value
  */
-u16 pmbase = DEFAULT_PMBASE;
-u8 smm_initialized = 0;
+static u16 pmbase = DEFAULT_PMBASE;
+u16 smm_get_pmbase(void)
+{
+	return pmbase;
+}
+
+static u8 smm_initialized = 0;
 
 /* GNVS needs to be updated by an 0xEA PM Trap (B2) after it has been located
  * by coreboot.
  */
-global_nvs_t *gnvs = (global_nvs_t *)0x0;
-void *tcg = (void *)0x0;
-void *smi1 = (void *)0x0;
+static global_nvs_t *gnvs = (global_nvs_t *)0x0;
+global_nvs_t *smm_get_gnvs(void)
+{
+	return gnvs;
+}
 
 #if CONFIG_SMM_TSEG
 static u32 tseg_base = 0;
@@ -523,10 +529,7 @@ static void southbridge_smi_apmc(unsigned int node, smm_state_save_area_t *state
 			return;
 		}
 		gnvs = *(global_nvs_t **)0x500;
-		tcg  = *(void **)0x504;
-		smi1 = *(void **)0x508;
 		smm_initialized = 1;
-		printk(BIOS_DEBUG, "SMI#: Setting up structures to %p, %p, %p\n", gnvs, tcg, smi1);
 		break;
 #if CONFIG_ELOG_GSMI
 	case ELOG_GSMI_APM_CNT:




More information about the coreboot mailing list