[coreboot] New patch to review for coreboot: c7aaf88 haswell: adjust CAR usage
Stefan Reinauer (stefan.reinauer@coreboot.org)
gerrit at coreboot.org
Fri Mar 15 20:49:59 CET 2013
Stefan Reinauer (stefan.reinauer at coreboot.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/2752
-gerrit
commit c7aaf889cdeba2454c2d56abe86b7f5bec46def5
Author: Aaron Durbin <adurbin at chromium.org>
Date: Fri Jan 18 14:32:50 2013 -0600
haswell: adjust CAR usage
It was found that the Haswell reference code was smashing through the
stack into the reference code's heap implementation. The reason for this
is because our current CAR allocation is too small. Moreover there are
quite a few things to coordinate between 2 code bases to get correct.
This commit separates the CAR into 2 parts:
1. MRC CAR usage.
2. Coreboot CAR usage.
Pointers from one region can be passed between the 2 modules, but one
should not be able to affect the others as checking has been put into
place in both modules.
The CAR size has effectively been doubled from 0x20000 (128 KiB) to
0x40000 (256KiB). Not all of that increase was needed, but enforcing
a power of 2 size only utilizes 1 MTRR.
Old CAR layout with a single contiguous stack with the region starting
at CONFIG_DCACHE_RAM_BASE:
+---------------------------------------+ Offset CONFIG_DCACHE_RAM_SIZE
| MRC global variables |
| CONFIG_DCACHE_RAM_MRC_VAR_SIZE bytes |
+---------------------------------------+
| ROM stage stack |
| |
| |
+---------------------------------------+
| MRC Heap 30000 bytes |
+---------------------------------------+
| ROM stage console |
| CONFIG_CONSOLE_CAR_BUFFER_SIZE bytes |
+---------------------------------------+
| ROM stage CAR_GLOBAL variables |
+---------------------------------------+ Offset 0
There was some hard coded offsets in the reference code wrapper to start
the heap past the console buffer. Even with this commit the console
can smash into the following region depending on what size
CONFIG_CONSOLE_CAR_BUFFER_SIZE is.
As noted above This change splits the CAR region into 2 parts starting
at CONFIG_DCACHE_RAM_BASE:
+---------------------------------------+
| MRC Region |
| CONFIG_DCACHE_RAM_MRC_VAR_SIZE bytes |
+---------------------------------------+ Offset CONFIG_DCACHE_RAM_SIZE
| ROM stage stack |
| |
| |
+---------------------------------------+
| ROM stage console |
| CONFIG_CONSOLE_CAR_BUFFER_SIZE bytes |
+---------------------------------------+
| ROM stage CAR_GLOBAL variables |
+---------------------------------------+ Offset 0
Another variable was add, CONFIG_DCACHE_RAM_ROMSTAGE_STACK_SIZE,
which represents the expected stack usage for the romstage. A marker
is checked at the base of the stack to determine if either the stack
was smashed or the console encroached on the stack.
Change-Id: Id76f2fe4a5cf1c776c8f0019f406593f68e443a7
Signed-off-by: Aaron Durbin <adurbin at chromium.org>
---
src/cpu/intel/haswell/Makefile.inc | 1 +
src/cpu/intel/haswell/cache_as_ram.inc | 11 +++---
src/cpu/intel/haswell/haswell.h | 5 +++
src/cpu/intel/haswell/romstage.c | 53 +++++++++++++++++++++++++++++
src/mainboard/intel/baskingridge/romstage.c | 8 ++---
src/mainboard/intel/wtm1/romstage.c | 7 ++--
src/mainboard/intel/wtm2/romstage.c | 7 ++--
src/northbridge/intel/haswell/Kconfig | 19 +++++++++--
8 files changed, 89 insertions(+), 22 deletions(-)
diff --git a/src/cpu/intel/haswell/Makefile.inc b/src/cpu/intel/haswell/Makefile.inc
index 67b0954..b2116f2 100644
--- a/src/cpu/intel/haswell/Makefile.inc
+++ b/src/cpu/intel/haswell/Makefile.inc
@@ -1,5 +1,6 @@
ramstage-y += haswell_init.c
subdirs-y += ../../x86/name
+romstage-y += romstage.c
ramstage-$(CONFIG_GENERATE_ACPI_TABLES) += acpi.c
ramstage-$(CONFIG_HAVE_SMI_HANDLER) += smmrelocate.c
diff --git a/src/cpu/intel/haswell/cache_as_ram.inc b/src/cpu/intel/haswell/cache_as_ram.inc
index f5ee82e..72b4958 100644
--- a/src/cpu/intel/haswell/cache_as_ram.inc
+++ b/src/cpu/intel/haswell/cache_as_ram.inc
@@ -24,7 +24,11 @@
#include <cpu/x86/post_code.h>
#include <cbmem.h>
-#define CACHE_AS_RAM_SIZE CONFIG_DCACHE_RAM_SIZE
+/* The full cache-as-ram size includes the cache-as-ram portion from coreboot
+ * and the space used by the reference code. These 2 values combined should
+ * be a power of 2 because the MTRR setup assumes that. */
+#define CACHE_AS_RAM_SIZE \
+ (CONFIG_DCACHE_RAM_SIZE + CONFIG_DCACHE_RAM_MRC_VAR_SIZE)
#define CACHE_AS_RAM_BASE CONFIG_DCACHE_RAM_BASE
/* Cache 4GB - MRC_SIZE_KB for MRC */
@@ -166,9 +170,8 @@ clear_mtrrs:
andl $(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax
movl %eax, %cr0
- /* Set up the stack pointer below MRC variable space. */
- movl $(CACHE_AS_RAM_SIZE + CACHE_AS_RAM_BASE - \
- CONFIG_DCACHE_RAM_MRC_VAR_SIZE - 4), %eax
+ /* Setup the stack. */
+ movl $(CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE), %eax
movl %eax, %esp
/* Restore the BIST result. */
diff --git a/src/cpu/intel/haswell/haswell.h b/src/cpu/intel/haswell/haswell.h
index cb85078..8d91dba 100644
--- a/src/cpu/intel/haswell/haswell.h
+++ b/src/cpu/intel/haswell/haswell.h
@@ -101,6 +101,11 @@
#define PSS_LATENCY_BUSMASTER 10
#ifndef __ROMCC__
+
+#if defined(__PRE_RAM__)
+void romstage_main(unsigned long bist);
+#endif
+
#ifdef __SMM__
/* Lock MSRs */
void intel_cpu_haswell_finalize_smm(void);
diff --git a/src/cpu/intel/haswell/romstage.c b/src/cpu/intel/haswell/romstage.c
new file mode 100644
index 0000000..f954b91
--- /dev/null
+++ b/src/cpu/intel/haswell/romstage.c
@@ -0,0 +1,53 @@
+/*
+ * This file is part of the coreboot project.
+ *
+ * Copyright (C) 2012 ChromeOS Authors
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ */
+
+#include <stdint.h>
+#include <cbmem.h>
+#include <console/console.h>
+
+/* Mainboard needs to supply this symbol. */
+extern void romstage_main(unsigned long bist);
+
+void main(unsigned long bist)
+{
+ int i;
+ const int num_guards = 4;
+ const u32 stack_guard = 0xdeadbeef;
+ u32 *stack_base = (void *)(CONFIG_DCACHE_RAM_BASE +
+ CONFIG_DCACHE_RAM_SIZE -
+ CONFIG_DCACHE_RAM_ROMSTAGE_STACK_SIZE);
+
+ printk(BIOS_DEBUG, "Setting up stack guards.\n");
+ for (i = 0; i < num_guards; i++)
+ stack_base[i] = stack_guard;
+
+ romstage_main(bist);
+
+ /* Check the stack. */
+ for (i = 0; i < num_guards; i++) {
+ if (stack_base[i] == stack_guard)
+ continue;
+ printk(BIOS_DEBUG, "Smashed stack detected in romstage!\n");
+ }
+
+#if CONFIG_CONSOLE_CBMEM
+ /* Keep this the last thing this function does. */
+ cbmemc_reinit();
+#endif
+}
diff --git a/src/mainboard/intel/baskingridge/romstage.c b/src/mainboard/intel/baskingridge/romstage.c
index d47fbf1..45316f6 100644
--- a/src/mainboard/intel/baskingridge/romstage.c
+++ b/src/mainboard/intel/baskingridge/romstage.c
@@ -30,6 +30,7 @@
#include <pc80/mc146818rtc.h>
#include <cbmem.h>
#include <console/console.h>
+#include "cpu/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/raminit.h"
#include "southbridge/intel/lynxpoint/pch.h"
@@ -82,7 +83,7 @@ const struct rcba_config_instruction rcba_config[] = {
RCBA_END_CONFIG,
};
-void main(unsigned long bist)
+void romstage_main(unsigned long bist)
{
int boot_mode = 0;
int wake_from_s3;
@@ -236,8 +237,5 @@ void main(unsigned long bist)
timestamp_add(TS_AFTER_INITRAM, after_dram_time );
timestamp_add_now(TS_END_ROMSTAGE);
#endif
-#if CONFIG_CONSOLE_CBMEM
- /* Keep this the last thing this function does. */
- cbmemc_reinit();
-#endif
}
+
diff --git a/src/mainboard/intel/wtm1/romstage.c b/src/mainboard/intel/wtm1/romstage.c
index 0190964..c80c721 100644
--- a/src/mainboard/intel/wtm1/romstage.c
+++ b/src/mainboard/intel/wtm1/romstage.c
@@ -30,6 +30,7 @@
#include <pc80/mc146818rtc.h>
#include <cbmem.h>
#include <console/console.h>
+#include "cpu/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/raminit.h"
#include "southbridge/intel/lynxpoint/me.h"
@@ -87,7 +88,7 @@ const struct rcba_config_instruction rcba_config[] = {
RCBA_END_CONFIG,
};
-void main(unsigned long bist)
+void romstage_main(unsigned long bist)
{
int boot_mode = 0;
int wake_from_s3;
@@ -241,8 +242,4 @@ void main(unsigned long bist)
timestamp_add(TS_AFTER_INITRAM, after_dram_time );
timestamp_add_now(TS_END_ROMSTAGE);
#endif
-#if CONFIG_CONSOLE_CBMEM
- /* Keep this the last thing this function does. */
- cbmemc_reinit();
-#endif
}
diff --git a/src/mainboard/intel/wtm2/romstage.c b/src/mainboard/intel/wtm2/romstage.c
index 75a4e99..4737073 100644
--- a/src/mainboard/intel/wtm2/romstage.c
+++ b/src/mainboard/intel/wtm2/romstage.c
@@ -30,6 +30,7 @@
#include <pc80/mc146818rtc.h>
#include <cbmem.h>
#include <console/console.h>
+#include "cpu/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/haswell.h"
#include "northbridge/intel/haswell/raminit.h"
#include "southbridge/intel/lynxpoint/me.h"
@@ -87,7 +88,7 @@ const struct rcba_config_instruction rcba_config[] = {
RCBA_END_CONFIG,
};
-void main(unsigned long bist)
+void romstage_main(unsigned long bist)
{
int boot_mode = 0;
int wake_from_s3;
@@ -241,8 +242,4 @@ void main(unsigned long bist)
timestamp_add(TS_AFTER_INITRAM, after_dram_time );
timestamp_add_now(TS_END_ROMSTAGE);
#endif
-#if CONFIG_CONSOLE_CBMEM
- /* Keep this the last thing this function does. */
- cbmemc_reinit();
-#endif
}
diff --git a/src/northbridge/intel/haswell/Kconfig b/src/northbridge/intel/haswell/Kconfig
index a7f1f9b..f689780 100644
--- a/src/northbridge/intel/haswell/Kconfig
+++ b/src/northbridge/intel/haswell/Kconfig
@@ -59,15 +59,28 @@ config MRC_CACHE_SIZE
config DCACHE_RAM_BASE
hex
- default 0xff7e0000
+ default 0xff7c0000
config DCACHE_RAM_SIZE
hex
- default 0x20000
+ default 0x10000
+ help
+ The size of the cache-as-ram region required during bootblock
+ and/or romstage. Note DCACHE_RAM_SIZE and DCACHE_RAM_MRC_VAR_SIZE
+ must add up to a power of 2.
config DCACHE_RAM_MRC_VAR_SIZE
hex
- default 0x4000
+ default 0x30000
+ help
+ The amount of cache-as-ram region required by the reference code.
+
+config DCACHE_RAM_ROMSTAGE_STACK_SIZE
+ hex
+ default 0x2000
+ help
+ The amount of anticipated stack usage from the data cache
+ during pre-ram rom stage execution.
config HAVE_MRC
bool "Add a System Agent binary"
More information about the coreboot
mailing list