On Sat, Nov 1, 2008 at 6:32 PM, Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> wrote:
On 01.11.2008 23:21, Carl-Daniel Hailfinger wrote:
> Once we touch the MTRRs in VIA disable_car(), the CPU resets. Since
> workarounds are better than instant reboots, mangle the code so that it
> only switches stacks and flushes the cache.
>
> There are two genuine fixes in there as well: Switch %esp before CAR is
> disabled. That way, debugging becomes easier and the stack is always valid.
>

And one of the nastier bugs easily happening in C: We had a pointer to a
const struct, but we wanted a const pointer to a struct. This kills the
(correct) warning about that code.

> Many thanks to Corey for testing countless iterations of that code.
>
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>
> Attached for poor gmail users.
>

New version attached. It also fixes a missing semicolon that somehow
snuck into the earlier patch.

Index: corebootv3-via_car/arch/x86/via/stage1.c
===================================================================
--- corebootv3-via_car/arch/x86/via/stage1.c    (Revision 977)
+++ corebootv3-via_car/arch/x86/via/stage1.c    (Arbeitskopie)
@@ -35,6 +35,7 @@
  */
 void disable_car(void)
 {
+    printk(BIOS_DEBUG, "disable_car entry\n");
     /* Determine new global variable location. Stack organization from top
      * Top 4 bytes are reserved
      * Pointer to global variables
@@ -42,19 +43,28 @@
      *
       * Align the result to 8 bytes
      */
-    const struct global_vars *newlocation = (struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);
+    struct global_vars *const newlocation = (struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);
     /* Copy global variables to new location. */
     memcpy(newlocation, global_vars(), sizeof(struct global_vars));
+    printk(BIOS_DEBUG, "disable_car global_vars copy done\n");
     /* Set the new global variable pointer. */
     *(struct global_vars **)(RAM_STACK_BASE - sizeof(struct global_vars *)) = newlocation;
 
+    printk(BIOS_DEBUG, "disable_car global_vars pointer adjusted\n");
+    printk(BIOS_DEBUG, "entering asm code now\n");
+
+#define HALT_AFTER 2

I don't think we need that anymore ;) Tested and
Acked-by: Corey Osgood <corey.osgood@gmail.com>