<p>Keith Hui has uploaded this change for <strong>review</strong>.</p><p><a href="https://review.coreboot.org/21329">View Change</a></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">cpu/intel/car/cache_as_ram.inc: Simplify code path<br><br>Make all CAR-related calculations refer to CONFIG_DCACHE_RAM_BASE and<br>CONFIG_DCACHE_RAM_SIZE for consistency. This fixes a bug that prevents<br>migrating cpu/intel/slot_1 & nb/intel/i440bx to EARLY_CBMEM_INIT.<br><br>Exclude a Hyperthreading-specific code path when building for<br>slot_1 or pga370, which predates HT.<br><br>Do not clear MTRRs that will be programmed immediately afterwards.<br><br>Remove a block of CAR testing code currently blocked out by #if.<br>Newer CAR code don't even do it anymore.<br><br>Do not set %ebp before calling romstage_main(). We know it's not<br>needed.<br><br>The check for CONFIG_DCACHE_RAM_SIZE < 0x1000 is not needed;<br>checking that it is multiples of 4k will catch it.<br><br>Clarify the purpose of various code in the file.<br><br>Boot tested on ASUS P2B-LS mainboard.<br><br>Brought to you by https://review.coreboot.org/c/20977/.<br><br>Change-Id: I9ab996e46e4f96320143022938477a5fd2046ed7<br>Signed-off-by: Keith Hui <buurin@gmail.com><br>---<br>M src/cpu/intel/car/cache_as_ram.inc<br>1 file changed, 31 insertions(+), 73 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">git pull ssh://review.coreboot.org:29418/coreboot refs/changes/29/21329/1</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">diff --git a/src/cpu/intel/car/cache_as_ram.inc b/src/cpu/intel/car/cache_as_ram.inc<br>index ac17571..ee1d577 100644<br>--- a/src/cpu/intel/car/cache_as_ram.inc<br>+++ b/src/cpu/intel/car/cache_as_ram.inc<br>@@ -22,13 +22,15 @@<br> #include <cpu/x86/lapic_def.h><br> #include <cpu/x86/post_code.h><br> <br>-#define CacheSize CONFIG_DCACHE_RAM_SIZE<br>-#define CacheBase (0xd0000 - CacheSize)<br>-<br> /* Save the BIST result. */<br> movl %eax, %ebp<br> <br> CacheAsRam:<br>+/*<br>+ * Only SLOT_1, SOCKET_(FC_)PGA370 and SOCKET_MPGA479M use this code and HT<br>+ * was only introduced for 479. Include this HT-specific code only on 479.<br>+ */<br>+#if IS_ENABLED(CONFIG_CPU_INTEL_SOCKET_MPGA479M)<br> /* Check whether the processor has HT capability. */<br> movl $01, %eax<br> cpuid<br>@@ -104,6 +106,7 @@<br> jz LogicalAP_SIPINotdone<br> <br> NotHtProcessor:<br>+#endif<br> /* Set the default memory type and enable fixed and variable MTRRs. */<br> movl $MTRR_DEF_TYPE_MSR, %ecx<br> xorl %edx, %edx<br>@@ -130,8 +133,13 @@<br> .long MTRR_FIX_64K_00000<br> .long MTRR_FIX_16K_80000<br> .long MTRR_FIX_16K_A0000<br>+ /* C0000 will be set later if DCACHE_RAM_BASE is set below 0xc8000.<br>+ * Don't try to set it twice.<br>+ */<br>+#if CONFIG_DCACHE_RAM_BASE >= 0xc8000<br> .long MTRR_FIX_4K_C0000<br>- .long MTRR_FIX_4K_C8000<br>+#endif<br>+ /* C8000 will be set later. */<br> .long MTRR_FIX_4K_D0000<br> .long MTRR_FIX_4K_D8000<br> .long MTRR_FIX_4K_E0000<br>@@ -142,8 +150,7 @@<br> /* var MTRR MSRs */<br> .long MTRR_PHYS_BASE(0)<br> .long MTRR_PHYS_MASK(0)<br>- .long MTRR_PHYS_BASE(1)<br>- .long MTRR_PHYS_MASK(1)<br>+ /* var MTRR #1 will be set next for XIP ROM. Don't try to set it twice. */<br> .long MTRR_PHYS_BASE(2)<br> .long MTRR_PHYS_MASK(2)<br> .long MTRR_PHYS_BASE(3)<br>@@ -203,32 +210,27 @@<br> */<br> .endm<br> <br>-#if CacheSize > 0x10000<br>+#if CONFIG_DCACHE_RAM_SIZE > 0x10000<br> #error Invalid CAR size, must be at most 64k.<br> #endif<br>-#if CacheSize < 0x1000<br>-#error Invalid CAR size, must be at least 4k. This is a processor limitation.<br>-#endif<br>-#if (CacheSize & (0x1000 - 1))<br>+/* This check will also catch DCACHE_RAM_SIZE < 0x1000 */<br>+#if (CONFIG_DCACHE_RAM_SIZE & (0x1000 - 1))<br> #error Invalid CAR size, is not a multiple of 4k. This is a processor limitation.<br> #endif<br> <br>-#if CacheSize > 0x8000<br>+#if CONFIG_DCACHE_RAM_SIZE > 0x8000<br> /* Enable caching for 32K-64K using fixed MTRR. */<br> movl $MTRR_FIX_4K_C0000, %ecx<br>- simplemask CacheSize, 0x8000<br>+ simplemask CONFIG_DCACHE_RAM_SIZE, 0x8000<br> wrmsr<br> #endif<br> <br> /* Enable caching for 0-32K using fixed MTRR. */<br> movl $MTRR_FIX_4K_C8000, %ecx<br>- simplemask CacheSize, 0<br>+ simplemask CONFIG_DCACHE_RAM_SIZE, 0<br> wrmsr<br> <br>- /*<br>- * Enable write base caching so we can do execute in place (XIP)<br>- * on the flash ROM.<br>- */<br>+ /* Enable cache for our code in Flash because we do XIP here. */<br> movl $MTRR_PHYS_BASE(1), %ecx<br> xorl %edx, %edx<br> /*<br>@@ -237,7 +239,7 @@<br> */<br> movl $copy_and_run, %eax<br> andl $(~(CONFIG_XIP_ROM_SIZE - 1)), %eax<br>- orl $MTRR_TYPE_WRBACK, %eax<br>+ orl $MTRR_TYPE_WRPROT, %eax<br> wrmsr<br> <br> movl $MTRR_PHYS_MASK(1), %ecx<br>@@ -250,70 +252,26 @@<br> andl $(~(CR0_CacheDisable | CR0_NoWriteThrough)), %eax<br> movl %eax, %cr0<br> <br>- /* Read the range with lodsl. */<br>- movl $CacheBase, %esi<br>+ /* Read the CAR region. This will also fill up the cache.<br>+ * IMPORTANT: This step is mandatory.<br>+ */<br>+ movl $CONFIG_DCACHE_RAM_BASE, %esi<br> cld<br>- movl $(CacheSize >> 2), %ecx<br>+ movl $(CONFIG_DCACHE_RAM_SIZE >> 2), %ecx<br> rep lodsl<br> <br>- /* Clear the range. */<br>- movl $CacheBase, %edi<br>- movl $(CacheSize >> 2), %ecx<br>+ /* Clear the CAR region. */<br>+ movl $CONFIG_DCACHE_RAM_BASE, %edi<br>+ movl $(CONFIG_DCACHE_RAM_SIZE >> 2), %ecx<br> xorl %eax, %eax<br> rep stosl<br> <br>-#if 0<br>- /* Check the cache as ram. */<br>- movl $CacheBase, %esi<br>- movl $(CacheSize >> 2), %ecx<br>-.xin1:<br>- movl %esi, %eax<br>- movl %eax, (%esi)<br>- decl %ecx<br>- je .xout1<br>- add $4, %esi<br>- jmp .xin1<br>-.xout1:<br>-<br>- movl $CacheBase, %esi<br>- // movl $(CacheSize >> 2), %ecx<br>- movl $4, %ecx<br>-.xin1x:<br>- movl %esi, %eax<br>-<br>- movl $0x4000, %edx<br>- movb %ah, %al<br>-.testx1:<br>- outb %al, $0x80<br>- decl %edx<br>- jnz .testx1<br>-<br>- movl (%esi), %eax<br>- cmpb 0xff, %al<br>- je .xin2 /* Don't show. */<br>-<br>- movl $0x4000, %edx<br>-.testx2:<br>- outb %al, $0x80<br>- decl %edx<br>- jnz .testx2<br>-<br>-.xin2:<br>- decl %ecx<br>- je .xout1x<br>- add $4, %esi<br>- jmp .xin1x<br>-.xout1x:<br>-#endif<br>-<br>- movl $(CacheBase + CacheSize - 4), %eax<br>+ movl $(CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE), %eax<br> movl %eax, %esp<br> lout:<br> /* Restore the BIST result. */<br> movl %ebp, %eax<br> <br>- /* We need to set EBP? No need. */<br>- movl %esp, %ebp<br> pushl %eax /* BIST */<br> call romstage_main<br> <br>@@ -329,7 +287,7 @@<br> orl $CR0_CacheDisable, %eax<br> movl %eax, %cr0<br> <br>- /* Clear sth. */<br>+ /* Clear the fixed MTRR we used. */<br> movl $MTRR_FIX_4K_C8000, %ecx<br> xorl %edx, %edx<br> xorl %eax, %eax<br></pre><p>To view, visit <a href="https://review.coreboot.org/21329">change 21329</a>. To unsubscribe, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/21329"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: newchange </div>
<div style="display:none"> Gerrit-Change-Id: I9ab996e46e4f96320143022938477a5fd2046ed7 </div>
<div style="display:none"> Gerrit-Change-Number: 21329 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: Keith Hui <buurin@gmail.com> </div>