[coreboot-gerrit] Change in coreboot[master]: nb/intel/*/northbridge.c: add a final() to trigger finalize_smm() at ...

[Bill XIE (Code Review)]

Bill XIE has uploaded this change for review. ( https://review.coreboot.org/21129


Change subject: nb/intel/*/northbridge.c: add a final() to trigger finalize_smm() at boot time
......................................................................

nb/intel/*/northbridge.c: add a final() to trigger finalize_smm() at boot time

Several registers inside intel northbridge from nehalem should be locked
down during boot and s3 resume. coreboot do have a function implemented
in src/northbridge/intel/*/finalize.c to be called during an #SMI triggered
with outb(APM_CNT_FINALIZE, APM_CNT), but currently this #SMI is only
triggered during s3 resume, and not during boot. This problem has beed
discussed in
https://mail.coreboot.org/pipermail/coreboot/2017-August/084924.html .

I believe that a final() function inside struct chip_operations is a good
place to do the lockdown during boot time.

Only the change on sandybridge is well tested.

Change-Id: I43d4142291c8737b29738c41e8c484328b297b55
Signed-off-by: Bill XIE <persmule at gmail.com>
---
M src/northbridge/intel/fsp_sandybridge/northbridge.c
M src/northbridge/intel/haswell/northbridge.c
M src/northbridge/intel/nehalem/northbridge.c
M src/northbridge/intel/sandybridge/northbridge.c
4 files changed, 44 insertions(+), 1 deletion(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/21129/1

diff --git a/src/northbridge/intel/fsp_sandybridge/northbridge.c b/src/northbridge/intel/fsp_sandybridge/northbridge.c
index a565b8d..d3eb670 100644
--- a/src/northbridge/intel/fsp_sandybridge/northbridge.c
+++ b/src/northbridge/intel/fsp_sandybridge/northbridge.c
@@ -33,6 +33,7 @@
 #include "northbridge.h"
 #include <fsp_util.h>
 #include <cpu/intel/smm/gen1/smi.h>
+#include <cpu/x86/smm.h>
 
 static int bridge_revision_id = -1;
 
@@ -352,7 +353,17 @@
 	}
 }
 
+static void final(void *chip_info)
+{
+	/*
+	 * Trigger APM_CNT_FINALIZE to lock down several
+	 * registers on the northbridge.
+	 */
+	outb(APM_CNT_FINALIZE, APM_CNT);
+}
+
 struct chip_operations northbridge_intel_fsp_sandybridge_ops = {
 	CHIP_NAME("Intel i7 (SandyBridge/IvyBridge) integrated Northbridge")
 	.enable_dev = enable_dev,
+	.final = final,
 };
diff --git a/src/northbridge/intel/haswell/northbridge.c b/src/northbridge/intel/haswell/northbridge.c
index 32be916..5defba2 100644
--- a/src/northbridge/intel/haswell/northbridge.c
+++ b/src/northbridge/intel/haswell/northbridge.c
@@ -480,7 +480,17 @@
 	}
 }
 
+static void final(void *chip_info)
+{
+	/*
+	 * Trigger APM_CNT_FINALIZE to lock down several
+	 * registers on the northbridge.
+	 */
+	outb(APM_CNT_FINALIZE, APM_CNT);
+}
+
 struct chip_operations northbridge_intel_haswell_ops = {
 	CHIP_NAME("Intel i7 (Haswell) integrated Northbridge")
 	.enable_dev = enable_dev,
+	.final = final,
 };
diff --git a/src/northbridge/intel/nehalem/northbridge.c b/src/northbridge/intel/nehalem/northbridge.c
index b09460c..581c3a2 100644
--- a/src/northbridge/intel/nehalem/northbridge.c
+++ b/src/northbridge/intel/nehalem/northbridge.c
@@ -33,6 +33,7 @@
 #include "chip.h"
 #include "nehalem.h"
 #include <cpu/intel/smm/gen1/smi.h>
+#include <cpu/x86/smm.h>
 
 static int bridge_revision_id = -1;
 
@@ -322,7 +323,17 @@
 	}
 }
 
+static void final(void *chip_info)
+{
+	/*
+	 * Trigger APM_CNT_FINALIZE to lock down several
+	 * registers on the northbridge.
+	 */
+	outb(APM_CNT_FINALIZE, APM_CNT);
+}
+
 struct chip_operations northbridge_intel_nehalem_ops = {
 	CHIP_NAME("Intel i7 (Nehalem) integrated Northbridge")
-	    .enable_dev = enable_dev,
+	.enable_dev = enable_dev,
+	.final = final,
 };
diff --git a/src/northbridge/intel/sandybridge/northbridge.c b/src/northbridge/intel/sandybridge/northbridge.c
index 3c0b9ec..aa67db7 100644
--- a/src/northbridge/intel/sandybridge/northbridge.c
+++ b/src/northbridge/intel/sandybridge/northbridge.c
@@ -32,6 +32,7 @@
 #include "chip.h"
 #include "sandybridge.h"
 #include <cpu/intel/smm/gen1/smi.h>
+#include <cpu/x86/smm.h>
 
 static int bridge_revision_id = -1;
 
@@ -540,7 +541,17 @@
 	}
 }
 
+static void final(void *chip_info)
+{
+	/*
+	 * Trigger APM_CNT_FINALIZE to lock down several
+	 * registers on the northbridge.
+	 */
+	outb(APM_CNT_FINALIZE, APM_CNT);
+}
+
 struct chip_operations northbridge_intel_sandybridge_ops = {
 	CHIP_NAME("Intel SandyBridge/IvyBridge integrated Northbridge")
 	.enable_dev = enable_dev,
+	.final = final,
 };

-- 
To view, visit https://review.coreboot.org/21129
To unsubscribe, visit https://review.coreboot.org/settings

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I43d4142291c8737b29738c41e8c484328b297b55
Gerrit-Change-Number: 21129
Gerrit-PatchSet: 1
Gerrit-Owner: Bill XIE <persmule at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20170821/0ba55ffb/attachment.html>