[coreboot-gerrit] Change in coreboot[master]: cpu/intel/car/cache_as_ram.inc: Fix long standing issues

[Keith Hui (Code Review)]

Keith Hui has uploaded this change for review. ( https://review.coreboot.org/21503


Change subject: cpu/intel/car/cache_as_ram.inc: Fix long standing issues
......................................................................

cpu/intel/car/cache_as_ram.inc: Fix long standing issues

Do not set %ebp before and switch directly to stack returned by
romstage_main().

Remove an unneeded 4-byte gap in CAR stack.

The caching strategy for flash XIP area should be WRPROT.

Clarify the various comments in the file on the logic.

Together they lay the groundwork for bringing EARLY_CBMEM_INIT to
intel/slot_1 boards.

Change-Id: Ibb6cf6a2adbe3a1f28bf2903d852ddc19e09b484
Signed-off-by: Keith Hui <buurin at gmail.com>
---
M src/cpu/intel/car/cache_as_ram.inc
1 file changed, 23 insertions(+), 35 deletions(-)



  git pull ssh://review.coreboot.org:29418/coreboot refs/changes/03/21503/1

diff --git a/src/cpu/intel/car/cache_as_ram.inc b/src/cpu/intel/car/cache_as_ram.inc
index 7cf55b3..d208cee 100644
--- a/src/cpu/intel/car/cache_as_ram.inc
+++ b/src/cpu/intel/car/cache_as_ram.inc
@@ -22,9 +22,6 @@
 #include <cpu/x86/lapic_def.h>
 #include <cpu/x86/post_code.h>
 
-#define CacheSize		CONFIG_DCACHE_RAM_SIZE
-#define CacheBase		(0xd0000 - CacheSize)
-
 	/* Save the BIST result. */
 	movl	%eax, %ebp
 
@@ -128,32 +125,29 @@
 	 */
 .endm
 
-#if CacheSize > 0x10000
+#if CONFIG_DCACHE_RAM_SIZE > 0x10000
 #error Invalid CAR size, must be at most 64k.
 #endif
-#if CacheSize < 0x1000
+#if CONFIG_DCACHE_RAM_SIZE < 0x1000
 #error Invalid CAR size, must be at least 4k. This is a processor limitation.
 #endif
-#if (CacheSize & (0x1000 - 1))
+#if (CONFIG_DCACHE_RAM_SIZE & (0x1000 - 1))
 #error Invalid CAR size, is not a multiple of 4k. This is a processor limitation.
 #endif
 
-#if CacheSize > 0x8000
+#if CONFIG_DCACHE_RAM_SIZE > 0x8000
 	/* Enable caching for 32K-64K using fixed MTRR. */
 	movl	$MTRR_FIX_4K_C0000, %ecx
-	simplemask CacheSize, 0x8000
+	simplemask CONFIG_DCACHE_RAM_SIZE, 0x8000
 	wrmsr
 #endif
 
 	/* Enable caching for 0-32K using fixed MTRR. */
 	movl	$MTRR_FIX_4K_C8000, %ecx
-	simplemask CacheSize, 0
+	simplemask CONFIG_DCACHE_RAM_SIZE, 0
 	wrmsr
 
-	/*
-	 * Enable write base caching so we can do execute in place (XIP)
-	 * on the flash ROM.
-	 */
+	/* Enable cache for our code in Flash because we do XIP here. */
 	movl	$MTRR_PHYS_BASE(1), %ecx
 	xorl	%edx, %edx
 	/*
@@ -162,7 +156,7 @@
 	 */
 	movl	$copy_and_run, %eax
 	andl	$(~(CONFIG_XIP_ROM_SIZE - 1)), %eax
-	orl	$MTRR_TYPE_WRBACK, %eax
+	orl	$MTRR_TYPE_WRPROT, %eax
 	wrmsr
 
 	movl	$MTRR_PHYS_MASK(1), %ecx
@@ -175,33 +169,31 @@
 	andl	$(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax
 	movl	%eax, %cr0
 
-	/* Read the range with lodsl. */
-	movl	$CacheBase, %esi
+	/* Read the CAR region. This will also fill up the cache.
+	 * IMPORTANT: This step is mandatory.
+	 */
+	movl	$CONFIG_DCACHE_RAM_BASE, %esi
 	cld
-	movl	$(CacheSize >> 2), %ecx
+	movl	$(CONFIG_DCACHE_RAM_SIZE >> 2), %ecx
 	rep	lodsl
 
-	/* Clear the range. */
-	movl	$CacheBase, %edi
-	movl	$(CacheSize >> 2), %ecx
+	/* Clear the CAR region. */
+	movl	$CONFIG_DCACHE_RAM_BASE, %edi
+	movl	$(CONFIG_DCACHE_RAM_SIZE >> 2), %ecx
 	xorl	%eax, %eax
 	rep	stosl
 
-	movl	$(CacheBase + CacheSize - 4), %eax
+	movl	$(CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE), %eax
 	movl	%eax, %esp
 lout:
 	/* Restore the BIST result. */
 	movl	%ebp, %eax
 
-	/* We need to set EBP? No need. */
-	movl	%esp, %ebp
 	pushl	%eax  /* BIST */
 	call	romstage_main
 
-	/* Save return value from romstage_main. It contains the stack to use
-	 * after cache-as-ram is torn down.
-	 */
-	movl	%eax, %ebx
+	/* Setup stack as indicated by return value from romstage_main(). */
+	movl	%eax, %esp
 
 	/* We don't need CAR from now on. */
 
@@ -210,7 +202,7 @@
 	orl	$CR0_CacheDisable, %eax
 	movl	%eax, %cr0
 
-	/* Clear sth. */
+	/* Clear the fixed MTRR we used. */
 	movl	$MTRR_FIX_4K_C8000, %ecx
 	xorl	%edx, %edx
 	xorl	%eax, %eax
@@ -222,12 +214,12 @@
 #endif
 
 	/*
-	 * Set the default memory type and disable fixed
-	 * and enable variable MTRRs.
+	 * Enable variable and disable fixed MTRRs.
+	 * Default memory type will be UC.
 	 */
 	movl	$MTRR_DEF_TYPE_MSR, %ecx
 	xorl	%edx, %edx
-	movl	$MTRR_DEF_TYPE_EN, %eax /* Enable variable and disable fixed MTRRs. */
+	movl	$MTRR_DEF_TYPE_EN, %eax
 	wrmsr
 
 	/* Enable cache. */
@@ -238,10 +230,6 @@
 __main:
 	post_code(POST_PREPARE_RAMSTAGE)
 	cld			/* Clear direction flag. */
-
-	/* Setup stack as indicated by return value from romstage_main(). */
-	movl	%ebx, %esp
-	movl	%esp, %ebp
 	call	copy_and_run
 
 .Lhlt:

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

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ibb6cf6a2adbe3a1f28bf2903d852ddc19e09b484
Gerrit-Change-Number: 21503
Gerrit-PatchSet: 1
Gerrit-Owner: Keith Hui <buurin at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.coreboot.org/pipermail/coreboot-gerrit/attachments/20170911/bc4b75ac/attachment-0001.html>