[coreboot] proposal to enable running vsa in geode northbridge

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sat Jan 26 02:57:47 CET 2008


On 26.01.2008 00:30, ron minnich wrote:
> Attached.
>
> It's pretty simple and short and untested :-)
>
> ron
>   

Comments below.

> This change will support stage2 running LAR files. The initial example is running the VSA in 
> the geode lx northbridge. 
>
> lar.h: make LAR functions SHARED
> lar.c: make process_file non-static
> vsmsetup.c: modify to use LAR functions. 
> stage1.c: new function, init_archive, which is SHARED and will set up the initial archive struct. 
>
> Signed-off-by: Ronald G. Minnich <rminnich at gmail.com>
>
> Index: include/lar.h
> ===================================================================
> --- include/lar.h	(revision 560)
> +++ include/lar.h	(working copy)
> @@ -51,6 +51,7 @@
>  #define LAR_H
>  
>  #include <types.h>
> +#include <shared.h>
>  
>  #define MAGIC "LARCHIVE"
>  #define MAX_PATHLEN 1024
> @@ -82,11 +83,15 @@
>  };
>  
>  /* Prototypes. */
> -int find_file(const struct mem_file *archive, const char *filename, struct mem_file *result);
> -int copy_file(const struct mem_file *archive, const char *filename, void *where);
> -int run_file(const struct mem_file *archive, const char *filename, void *where);
> -int execute_in_place(const struct mem_file *archive, const char *filename);
> -int run_address(void *f);
> -void *load_file(const struct mem_file *archive, const char *filename);
> -void *load_file_segments(const struct mem_file *archive, const char *filename);
> +/* architecture-defined */
> +SHARED(init_archive, void, struct mem_file *);
> +/* architecture-independent */
> +SHARED(find_file, int, const struct mem_file *archive, const char *filename, struct mem_file *result);
> +SHARED(copy_file, int, const struct mem_file *archive, const char *filename, void *where);
> +SHARED(run_file, int, const struct mem_file *archive, const char *filename, void *where);
> +SHARED(execute_in_place, int, const struct mem_file *archive, const char *filename);
> +SHARED(run_address, int, void *f);
> +SHARED(load_file, void *, const struct mem_file *archive, const char *filename);
> +SHARED(load_file_segments, void *, const struct mem_file *archive, const char *filename);
> +SHARED(process_file, int, const struct mem_file *archive, void *where);
>  #endif /* LAR_H */
> Index: lib/lar.c
> ===================================================================
> --- lib/lar.c	(revision 560)
> +++ lib/lar.c	(working copy)
> @@ -145,7 +145,7 @@
>  	return 1;
>  }
>  
> -static int process_file(const struct mem_file *archive, void *where)
> +int process_file(const struct mem_file *archive, void *where)
>  {
>  	printk(BIOS_SPEW, "LAR: Compression algorithm #%i used\n", archive->compression);
>  	/* no compression */
> Index: northbridge/amd/geodelx/vsmsetup.c
> ===================================================================
> --- northbridge/amd/geodelx/vsmsetup.c	(revision 560)
> +++ northbridge/amd/geodelx/vsmsetup.c	(working copy)
> @@ -29,6 +29,7 @@
>  #include <device/pci_ids.h>
>  #include <msr.h>
>  #include <amd_geodelx.h>
> +#include <lar.h>
>  
>  #define VSA2_BUFFER			0x60000
>  #define VSA2_ENTRY_POINT	0x60020
> @@ -169,25 +170,32 @@
>  
>  void do_vsmbios(void *bios)
>  {
> -	struct device *dev;
> -	unsigned long busdevfn;
> -	unsigned int rom = 0;
>  	unsigned char *buf;
> -	unsigned int size = SMM_SIZE * 1024;
>  	int i;
> -	unsigned long ilen, olen;
> +	unsigned long olen;
> +	struct mem_file archive;
> +	struct mem_file file;
>  
>  	printk(BIOS_ERR, "do_vsmbios\n");
>  	/* clear vsm bios data area */
>  	for (i = 0x400; i < 0x500; i++) {
>  		*(volatile unsigned char *)i = 0;
>  	}
> +	init_archive(&archive);
>  
> -	rom = (unsigned long) bios;
> +	if (find_file(&archive, "/blob/vsa", &file)){
> +		printk(BIOS_ERR, "NO VSA found!\n");
> +		return;
> +	}
> +
> +	if (process_file(&file, (void *)VSA2_BUFFER)) {
> +		printk(BIOS_ERR, "Processing /blob/vsa failed\n");
> +		return;
> +	}
> +
>  	buf = (unsigned char *)VSA2_BUFFER;
> -	// FIXME
> -	//olen = unrv2b((u8 *) rom, buf, &ilen);
> -	printk(BIOS_DEBUG, "buf ilen %d olen%d\n", ilen, olen);
> +	olen = file.reallen;
> +	printk(BIOS_DEBUG, "buf ilen %d olen%ld\n", file.len, olen);
>   

Hm. Why is olen an unsigned long when reallen is u32?

>  	printk(BIOS_DEBUG, "buf %p *buf %d buf[256k] %d\n",
>  		     buf, buf[0], buf[SMM_SIZE * 1024]);
>  	printk(BIOS_DEBUG, "buf[0x20] signature is %x:%x:%x:%x\n",
> @@ -253,16 +261,16 @@
>  		     eax, ebx, ecx, edx);
>  	printk(BIOS_DEBUG, "biosint: ebp 0x%lx esp 0x%lx edi 0x%lx esi 0x%lx\n",
>  		     ebp, esp, edi, esi);
> -	printk(BIOS_DEBUG, "biosint:  ip 0x%x   cs 0x%x  flags 0x%x\n",
> +	printk(BIOS_DEBUG, "biosint:  ip 0x%lx   cs 0x%lx  flags 0x%lx\n",
>  		     ip, cs, flags);
> -	printk(BIOS_DEBUG, "biosint: gs 0x%x fs 0x%x ds 0x%x es 0x%x\n",
> +	printk(BIOS_DEBUG, "biosint: gs 0x%lx fs 0x%lx ds 0x%lx es 0x%lx\n",
>  		     gsfs >> 16, gsfs & 0xffff, dses >> 16, dses & 0xffff);
>   

If cs, ds, es, fs are all 16bit, then dses and gsfs should be explicitly
32bit and not long. That would fix compile warnings as well. I
understand that biosint is x86emu code, though.

>  	// cases in a good compiler are just as good as your own tables.
>  	switch (intnumber) {
>  	case 0 ... 15:
>  		// These are not BIOS service, but the CPU-generated exceptions
> -		printk(BIOS_INFO, "biosint: Oops, exception %u\n", intnumber);
> +		printk(BIOS_INFO, "biosint: Oops, exception %lu\n", intnumber);
>  		if (esp < 0x1000) {
>  			printk(BIOS_DEBUG, "Stack contents: ");
>  			while (esp < 0x1000) {
> @@ -290,7 +298,7 @@
>  				  &ebx, &edx, &ecx, &eax, &flags);
>  		break;
>  	default:
> -	  printk(BIOS_INFO, "BIOSINT: Unsupport int #0x%x\n", intnumber);
> +	  printk(BIOS_INFO, "BIOSINT: Unsupport int #0x%lx\n", intnumber);
>  		break;
>  	}
>  	if (ret)
> Index: arch/x86/stage1.c
> ===================================================================
> --- arch/x86/stage1.c	(revision 560)
> +++ arch/x86/stage1.c	(working copy)
> @@ -50,7 +50,21 @@
>  	post_code(POST_STAGE1_ENABLE_ROM);
>  }
>  
> +void init_archive(struct mem_file *archive)
> +{
> +	// FIXME this should be defined in the VPD area
> +	// but NOT IN THE CODE.
>  
> +	/* The len field starts behind the reset vector on x86.
> +	 * The start is not correct for all platforms. sc520 will
> +	 * need some hands on here.
> +	 */
> +	archive->len = *(u32 *)0xfffffff4;
> +	archive->start =(void *)(0UL-archive->len);
> +
> +	// FIXME check integrity
> +
> +}
>  /* until we get rid of elf */
>  int legacy(struct mem_file *archive, char *name, void *where, struct lb_memory *mem)
>  {
> @@ -121,19 +135,8 @@
>  
>  	// location and size of image.
>  
> -	// FIXME this should be defined in the VPD area
> -	// but NOT IN THE CODE.
> +	init_archive(&archive);
>  
> -	/* The len field starts behind the reset vector on x86.
> -	 * The start is not correct for all platforms. sc520 will
> -	 * need some hands on here.
> -	 */
> -	archive.len = *(u32 *)0xfffffff4;
> -	archive.start =(void *)(0UL-archive.len);
> -
> -	// FIXME check integrity
> -
> -
>  	// find first initram
>  	if (check_normal_boot_flag()) {
>  		printk(BIOS_DEBUG, "Choosing normal boot.\n");
>   


Nice refactoring.

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

Regards,
Carl-Daniel




More information about the coreboot mailing list