On Sat, Nov 1, 2008 at 6:32 PM, Carl-Daniel Hailfinger <span dir="ltr"><<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>></span> wrote:<br><div class="gmail_quote"><blockquote class="gmail_quote" style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;">
<div class="Ih2E3d">On 01.11.2008 23:21, Carl-Daniel Hailfinger wrote:<br>
> Once we touch the MTRRs in VIA disable_car(), the CPU resets. Since<br>
> workarounds are better than instant reboots, mangle the code so that it<br>
> only switches stacks and flushes the cache.<br>
><br>
</div>> There are two genuine fixes in there as well: Switch %esp before CAR is<br>
<div class="Ih2E3d">> disabled. That way, debugging becomes easier and the stack is always valid.<br>
><br>
<br>
</div>And one of the nastier bugs easily happening in C: We had a pointer to a<br>
const struct, but we wanted a const pointer to a struct. This kills the<br>
(correct) warning about that code.<br>
<div class="Ih2E3d"><br>
> Many thanks to Corey for testing countless iterations of that code.<br>
><br>
> Signed-off-by: Carl-Daniel Hailfinger <<a href="mailto:c-d.hailfinger.devel.2006@gmx.net">c-d.hailfinger.devel.2006@gmx.net</a>><br>
> Attached for poor gmail users.<br>
><br>
<br>
</div>New version attached. It also fixes a missing semicolon that somehow<br>
snuck into the earlier patch.</blockquote><div><br><blockquote style="border-left: 1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;" class="gmail_quote">Index: corebootv3-via_car/arch/x86/via/stage1.c<br>
===================================================================<br>--- corebootv3-via_car/arch/x86/via/stage1.c    (Revision 977)<br>+++ corebootv3-via_car/arch/x86/via/stage1.c    (Arbeitskopie)<br>@@ -35,6 +35,7 @@<br>
  */<br> void disable_car(void)<br> {<br>+    printk(BIOS_DEBUG, "disable_car entry\n");<br>     /* Determine new global variable location. Stack organization from top<br>      * Top 4 bytes are reserved<br>      * Pointer to global variables<br>
@@ -42,19 +43,28 @@<br>      *<br>       * Align the result to 8 bytes<br>      */<br>-    const struct global_vars *newlocation = (struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);<br>
+    struct global_vars *const newlocation = (struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);<br>     /* Copy global variables to new location. */<br>     memcpy(newlocation, global_vars(), sizeof(struct global_vars));<br>
+    printk(BIOS_DEBUG, "disable_car global_vars copy done\n");<br>     /* Set the new global variable pointer. */<br>     *(struct global_vars **)(RAM_STACK_BASE - sizeof(struct global_vars *)) = newlocation;<br>
 <br>+    printk(BIOS_DEBUG, "disable_car global_vars pointer adjusted\n");<br>+    printk(BIOS_DEBUG, "entering asm code now\n");<br>+<br>+#define HALT_AFTER 2</blockquote><div><br>I don't think we need that anymore ;) Tested and<br>
Acked-by: Corey Osgood <<a href="mailto:corey.osgood@gmail.com">corey.osgood@gmail.com</a>><br></div></div></div>