[coreboot] [PATCH] v3: Work around broken MTRR setup in VIA CAR

[Corey Osgood]

On Sat, Nov 1, 2008 at 6:32 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006 at 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 at 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 at gmail.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.coreboot.org/pipermail/coreboot/attachments/20081101/fa5b6239/attachment.html>