[coreboot] patch: add support for blobs; example SMM blob included

[Carl-Daniel Hailfinger]

On 26.11.2008 22:49, ron minnich wrote:
> Index: arch/x86/intel/core2/stage1.c
> ===================================================================
> --- arch/x86/intel/core2/stage1.c	(revision 1059)
> +++ arch/x86/intel/core2/stage1.c	(working copy)
> @@ -2,6 +2,7 @@
>   * This file is part of the coreboot project.
>   *
>   * Copyright (C) 2008 Carl-Daniel Hailfinger
> + * Copyright (C) 2008 coresystems GmbH
>   *
>   * This program is free software; you can redistribute it and/or modify
>   * it under the terms of the GNU General Public License as published by
> @@ -29,30 +30,23 @@
>  #include <string.h>
>  #include <mtrr.h>
>  
> -/**
> - * Disable Cache As RAM (CAR) after memory is setup.
> - */
>  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
> -	 * Global variables
> -	 *
> - 	 * Align the result to 8 bytes
> -	 */
> -	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;
> +	struct global_vars *const new_globals = 
> +		(struct global_vars *)((RAM_STACK_BASE - sizeof(struct global_vars *) - sizeof(struct global_vars)) & ~0x7);
>  
> -	printk(BIOS_DEBUG, "disable_car global_vars pointer adjusted\n");
> -	printk(BIOS_DEBUG, "entering asm code now\n");
> +	/* Copy global variables */
> +	memcpy(new_globals, global_vars(), sizeof(struct global_vars));
>  
> +	/* Update global variable pointer */
> +	*(struct global_vars **)(RAM_STACK_BASE - sizeof(struct global_vars *))
> +		= new_globals;
> +
> +	printk(BIOS_DEBUG, "Disabling cache-as-ram\n");
> +
>  	__asm__ __volatile__(
> +"movb   $0x30, %%al\noutb %%al, $0x80\n"
> +
>  	"	movl	%[newesp], %%esp	\n"
>  
>  	/* We don't need cache as ram for now on */
> @@ -61,6 +55,8 @@
>  	"	orl    $(0x1<<30),%%eax		\n"
>  	"	movl    %%eax, %%cr0		\n"
>  
> +"movb   $0x31, %%al\noutb %%al, $0x80\n"
> +
>  	/* disable fixed mtrr from now on, it will be enabled by coreboot_ram again*/
>  	/* clear sth */
>  	"	xorl    %%eax, %%eax		\n"
> @@ -70,6 +66,7 @@
>  	"	movl    $0x200, %%ecx		\n"
>  	"	wrmsr				\n"
>  
> +"movb   $0x32, %%al\noutb %%al, $0x80\n"
>  	/* Set the default memory type and disable fixed and enable variable MTRRs */
>  	"	movl    %[_MTRRdefType_MSR], %%ecx	\n"
>  	"	xorl    %%edx, %%edx		\n"
> @@ -77,17 +74,22 @@
>  	"	movl    $0x00000800, %%eax	\n"
>  	"	wrmsr				\n"
>  
> +"movb   $0x33, %%al\noutb %%al, $0x80\n"
>  	/* enable cache */
>  	"	movl    %%cr0, %%eax		\n"
>  	"	andl    $0x9fffffff,%%eax	\n"
>  	"	movl    %%eax, %%cr0		\n"
>  
> -	"	wbinvd				\n"
> +"movb   $0x34, %%al\noutb %%al, $0x80\n"
> +	"	invd				\n"
>  
> +"movb   $0x35, %%al\noutb %%al, $0x80\n"
>  	"	call stage1_phase3		\n"
> -	:: [newesp] "i" (newlocation),
> +	:: [newesp] "i" (new_globals),
>  	 [_MTRRdefType_MSR] "i" (MTRRdefType_MSR)
>  	: "memory");
> +
> +	printk(BIOS_DEBUG, "Something went wrong. Still here.\n");
>  }
>  
>  void stop_ap(void)
>
>   

That part of the patch is really funny. It rewrites a few comments and
printk messages (no effect on execution), renames one variable and
changes some whitespace. A few POST codes are added for good measure.

The only _functional_ change (except cosmetics and debugging) is the
change from wbinvd to invd.

May I suggest to drop all arch/x86/intel/core2/stage1.c changes except
the one below (and this wbinvd->invd change should have no effect
according to the scarce publicly available docs).

Index: arch/x86/intel/core2/stage1.c
===================================================================
--- arch/x86/intel/core2/stage1.c	(Revision 1061)
+++ arch/x86/intel/core2/stage1.c	(Arbeitskopie)
@@ -82,7 +82,7 @@
 	"	andl    $0x9fffffff,%%eax	\n"
 	"	movl    %%eax, %%cr0		\n"
 
-	"	wbinvd				\n"
+	"	invd				\n"
 
 	"	call stage1_phase3		\n"
 	:: [newesp] "i" (newlocation),


This keeps the changes to a minium (and Stefan opposed changing
whitespace, printk and comments just for the sake of it).

If the machine boots even without the change, we could drop it
altogether. Having disable_car implementations diverge without reasons
enforced by the hardware is suboptimal.

Thanks.

Regards,
Carl-Daniel

-- 
http://www.hailfinger.org/