[coreboot] [PATCH] v3: printk buffer

Peter Stuge peter at stuge.se
Mon Feb 11 04:19:40 CET 2008


On Mon, Feb 11, 2008 at 03:01:39AM +0100, Carl-Daniel Hailfinger wrote:
> New version, without the recently merged stuff.
> 
> If you want to dump the buffer from the Qemu monitor after CAR has been
> disabled, use this command:
> memsave 0x90000 65536 memdump.bin

How do we fuse it with the kernel message buffer?


> Tested on Qemu, should work on LX. Code needs to get in mergeable
> shape before commit, but I'd appreciate testers on LX with FS2.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>

Acked-by: Peter Stuge <peter at stuge.se>

But please wait for comments from others too. I also have no way of
testing this.


> +struct printk_buffer {
> +	int len;
> +	int readoffset;
> +	int writeoffset;
> +	int realoffset;
> +#ifdef CONFIG_CHECK_STACK_USAGE	/* This config variable does not exit yet. */

typo exist


> +void printk_buffer_move(void *newaddr, int newsize)
> +{
> +	struct printk_buffer **p;
> +	struct printk_buffer *oldbuf, *newbuf;
> +	p = bottom_of_stack();
> +	oldbuf = *p;
> +	*p = newbuf = newaddr;

I think *p = newbuf should be moved to after the memcpy() so that
printk during memcpy() does not operate on an uninitialized struct.


> +	/* FIXME: The memcpy is wrong on so many levels.
> +	 * It should deal with wraparounds and with oldbuf->len > newbuf->len
> +	 */
> +	memcpy(newbuf, oldbuf, sizeof(struct printk_buffer) + oldbuf->len);

So deal with it now. :)

What kind of wraparound do you mean?

The comment suggests using newbuf->len before it has been
initialized, but I just realized it's refering to newsize. Easy fix:

int copylen = oldbuf->len < newsize ? oldbuf->len : newsize;
memcpy(.., sizeof(struct printk_buffer) + copylen);


> +#ifdef CONFIG_CHECK_STACK_USAGE	/* This config variable does not exit yet. */
> +	int i;
> +	/* stack usage checker */
> +	for (i = 0; i < 2048; i++)
> +		if (buf->buffer[buf->len + i] != 0x0)
> +			break;
> +	buf->loweststack = &buf->buffer[buf->len + i];

How does this work?


> Index: LinuxBIOSv3-printkbuffer/arch/x86/geodelx/stage0.S
> ===================================================================
> --- LinuxBIOSv3-printkbuffer/arch/x86/geodelx/stage0.S	(Revision 587)
> +++ LinuxBIOSv3-printkbuffer/arch/x86/geodelx/stage0.S	(Arbeitskopie)
> @@ -361,6 +361,11 @@
>  	movw	%ax, %ss
>  
>  lout:
> +	/* Store pointer to start of printk buffer, should really use
> +	 * PRINTK_BUF_ADDR_CAR instead.
> +	 */
> +	movl    $CONFIG_CARBASE, %eax

So why not do that?


> Index: LinuxBIOSv3-printkbuffer/arch/x86/stage1.c
> ===================================================================
> --- LinuxBIOSv3-printkbuffer/arch/x86/stage1.c	(Revision 587)
> +++ LinuxBIOSv3-printkbuffer/arch/x86/stage1.c	(Arbeitskopie)
> @@ -26,6 +26,7 @@
>  #include <tables.h>
>  #include <lib.h>
>  #include <mc146818rtc.h>
> +#include <cpu.h>
>  
>  /* ah, well, what a mess! This is a hard code. FIX ME but how? 
>   * By getting rid of ELF ...
> @@ -67,6 +68,13 @@
>  
>  }
>  
> +void *bottom_of_stack(void)
> +{
> +

Extra line snuck in here.


> +	/* -4-4 because CONFIG_CARBASE + CONFIG_CARSIZE - 4 is initial %esp */
> +	return (void *)(CONFIG_CARBASE + CONFIG_CARSIZE - 4 - 4);
> +}

Is it really bottom of stack and not top? esp decreases, and the
pointer is pushed before anything else?

Oh! It could be the bottom of the stack even though it has the
highest memory address. That's a bit confusing though.


//Peter




More information about the coreboot mailing list