[coreboot] r858 - coreboot-v3/arch/x86/amd/k8

[svn at coreboot.org]

Author: hailfinger
Date: 2008-09-05 12:29:33 +0200 (Fri, 05 Sep 2008)
New Revision: 858

Modified:
   coreboot-v3/arch/x86/amd/k8/stage1.c
Log:
The current K8 stack preservation code in disable_car() works by chance,
but that's not something we should rely on.

The new code is entirely rewritten, fixes a few missing constraints in
the asm and should be a lot more readable. However, the generated code
is NOT identical. The old code was broken because of the missing ecx
clobber constraint and it did not copy the stack back (ecx was zero at
the beginning of the copy-back loop and so the loop executed exactly
zero times).
So this is a genuine bug fix.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Ron writes:
wow! nice catch!

Acked-by: Ronald G. Minnich <rminnich at gmail.com>

We also need disable_car_and_halt, which only disables car and halts,
for the APs (i.e. no need to copy stack back)


Modified: coreboot-v3/arch/x86/amd/k8/stage1.c
===================================================================
--- coreboot-v3/arch/x86/amd/k8/stage1.c	2008-09-04 15:35:49 UTC (rev 857)
+++ coreboot-v3/arch/x86/amd/k8/stage1.c	2008-09-05 10:29:33 UTC (rev 858)
@@ -34,20 +34,31 @@
  */
 void disable_car(void)
 {
-	/* OK, here is the theory: we should be able to copy 
-	 * the data back over itself, and the wbinvd should then 
-	 * flush to memory. Let's see. 
-	 */
-	/* nope. 
-	__asm__ __volatile__("cld; rep movsl" ::"D" (CONFIG_CARBASE), "S" (CONFIG_CARBASE), "c" (CONFIG_CARSIZE/4): "memory");
-	 */
 	/* call the inlined function */
 	disable_cache_as_ram();
 
-	/* copy it down, wbinvd, copy it back? */
-	__asm__ __volatile__("cld; rep movsl" ::"D" (0x88000), "S" (CONFIG_CARBASE), "c" (CONFIG_CARSIZE/4): "memory");
-	__asm__ __volatile__ ("wbinvd\n");
-	__asm__ __volatile__("cld; rep movsl" ::"D" (CONFIG_CARBASE), "S" (0x88000), "c" (CONFIG_CARSIZE/4): "memory");
+	/* The BKDG says that a WBINVD will not flush CAR to RAM (because the
+	 * cache tags are not dirty).
+	 * Solution:
+	 * - Two subsequent memcpy in the same inline asm block, one from stack
+	 *   to backup, one from backup to stack.
+	 * The solution for geode of using a inline asm memcpy of the stack
+	 * onto itself will not mark the cache tags as dirty on K8.
+	 */
+	__asm__ __volatile__(
+	"	movl %[carbase], %%esi		\n"
+	"	movl %[backuplocation], %%edi	\n"
+	"	movl %[carsizequads], %%ecx	\n"
+	"	cld				\n"
+	"	rep movsl			\n"
+	"	wbinvd				\n"
+	"	movl %[backuplocation], %%esi	\n"
+	"	movl %[carbase], %%edi		\n"
+	"	movl %[carsizequads], %%ecx	\n"
+	"	rep movsl			\n"
+	:: [carbase] "i" (CONFIG_CARBASE), [backuplocation] "i" (0x88000),
+	   [carsizequads] "i" (CONFIG_CARSIZE/4)
+	: "memory", "%edi", "%esi", "%ecx");
 	banner(BIOS_DEBUG, "Disable_car: done wbinvd");
 	banner(BIOS_DEBUG, "disable_car: done");
 }