[coreboot] [PATCH] v3: printk buffer
Carl-Daniel Hailfinger
c-d.hailfinger.devel.2006 at gmx.net
Mon Feb 11 16:19:06 CET 2008
On 11.02.2008 04:19, Peter Stuge wrote:
> 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?
>
That problem still needs to be solved.
>> 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.
>
You could test on Qemu. I'll write up a really short Howto.
>> +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
>
Thanks. Fixed the other occurrences as well.
>> +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.
>
Agreed. However, once we begin to make coreboot threadsafe, this is
rather difficult to solve.
>> + /* 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. :)
>
Agreed. Will do so in a later patch. Remind me if I forget.
> What kind of wraparound do you mean?
>
The buffer is a ring buffer. Once writing reaches the end, you start
again at the beginning. However, if such a wraparound has happened
before relocation/resizing, we want to be more clever than just copying
the buffer and reorder the buffer contents into correct chronological
order, so that we do not lose all of the contents before wraparound.
> 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);
>
Thanks. As written above, the situation is a bit more complicated and
I'll address this later.
>> +#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?
>
The loop was looking for the lowest nonzero byte on the stack and stored
the address of that byte as maximum stack usage. The code was only
halfway correct before the printk buffer move and now it needs to be
rewritten. Will do that later.
>> 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?
>
We'd have to import PRINTK_BUF_ADDR_CAR from some header. We need to
create such a platform-specific header which exists on all platforms.
Maybe cpu.h?
>> 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.
>
Thanks.
>> + /* -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.
>
Yes, I'll add a better comment later.
New patch with printk buffer as default-on Kconfig variable:
Tested on Qemu, should work on Geode as well.
Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Index: LinuxBIOSv3-printkbuffer/include/console.h
===================================================================
--- LinuxBIOSv3-printkbuffer/include/console.h (Revision 587)
+++ LinuxBIOSv3-printkbuffer/include/console.h (Arbeitskopie)
@@ -37,6 +37,10 @@
unsigned char console_rx_byte(void);
int console_tst_byte(void);
void die(const char *msg);
+#ifdef CONFIG_CONSOLE_BUFFER
+void printk_buffer_init(void);
+void printk_buffer_move(void *newaddr, int newsize);
+#endif
struct console_driver {
void (*init)(void);
@@ -46,6 +50,19 @@
int (*tst_byte)(void);
};
+#ifdef CONFIG_CONSOLE_BUFFER
+struct printk_buffer {
+ int len;
+ int readoffset;
+ int writeoffset;
+ int realoffset;
+#ifdef CONFIG_CHECK_STACK_USAGE /* This config variable does not exist yet. */
+ void *loweststack;
+#endif
+ char buffer[];
+};
+#endif
+
SHARED_WITH_ATTRIBUTES(printk, int, __attribute__((format (printf, 2, 3))),
int msg_level, const char *fmt, ...);
SHARED(banner, void, int msg_level, const char *msg);
Index: LinuxBIOSv3-printkbuffer/include/arch/x86/cpu.h
===================================================================
--- LinuxBIOSv3-printkbuffer/include/arch/x86/cpu.h (Revision 587)
+++ LinuxBIOSv3-printkbuffer/include/arch/x86/cpu.h (Arbeitskopie)
@@ -24,6 +24,7 @@
#include <types.h>
#include <device/device.h>
+#include <shared.h>
#define X86_VENDOR_INTEL 0
#define X86_VENDOR_CYRIX 1
@@ -196,4 +197,13 @@
__asm__ __volatile__("hlt" : : : "memory");
}
+SHARED(bottom_of_stack, void *, void);
+
+#ifdef CONFIG_CONSOLE_BUFFER
+#define PRINTK_BUF_SIZE_CAR (CONFIG_CARSIZE / 2)
+#define PRINTK_BUF_ADDR_CAR CONFIG_CARBASE
+#define PRINTK_BUF_SIZE_RAM 65536
+#define PRINTK_BUF_ADDR_RAM 0x90000
+#endif
+
#endif /* ARCH_X86_CPU_H */
Index: LinuxBIOSv3-printkbuffer/lib/Kconfig
===================================================================
--- LinuxBIOSv3-printkbuffer/lib/Kconfig (Revision 587)
+++ LinuxBIOSv3-printkbuffer/lib/Kconfig (Arbeitskopie)
@@ -223,5 +223,12 @@
When you enable this option, coreboot will prefix each line of
console output with '(LB)'.
+config CONSOLE_BUFFER
+ boolean "Console memory buffer support"
+ default y
+ depends CONSOLE
+ help
+ Save coreboot output in a memory buffer.
+
endmenu
Index: LinuxBIOSv3-printkbuffer/lib/console.c
===================================================================
--- LinuxBIOSv3-printkbuffer/lib/console.c (Revision 587)
+++ LinuxBIOSv3-printkbuffer/lib/console.c (Arbeitskopie)
@@ -3,6 +3,7 @@
#include <console.h>
#include <uart8250.h>
#include <stdarg.h>
+#include <string.h>
int vtxprintf(void (*)(unsigned char, void *arg),
void *arg, const char *, va_list);
@@ -12,8 +13,65 @@
return CONFIG_DEFAULT_CONSOLE_LOGLEVEL;
}
+#ifdef CONFIG_CONSOLE_BUFFER
+void printk_buffer_move(void *newaddr, int newsize)
+{
+ struct printk_buffer **p;
+ struct printk_buffer *oldbuf, *newbuf;
+ p = bottom_of_stack();
+ oldbuf = *p;
+ newbuf = newaddr;
+ /* 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);
+ newbuf->len = newsize;
+ *p = newbuf;
+ return;
+}
+
+struct printk_buffer *printk_buffer_addr(void)
+{
+ struct printk_buffer **p;
+ p = bottom_of_stack();
+ return *p;
+}
+
+void printk_buffer_init(void)
+{
+ struct printk_buffer *buf = printk_buffer_addr();
+ buf->len = PRINTK_BUF_SIZE_CAR - sizeof(struct printk_buffer);
+ buf->readoffset = 0;
+ buf->writeoffset = 0;
+#ifdef CONFIG_CHECK_STACK_USAGE /* This config variable does not exist yet. */
+ buf->loweststack = 0;
+#endif
+ return;
+}
+
+void buffer_tx_byte(unsigned char byte, void *arg)
+{
+ struct printk_buffer *buf = printk_buffer_addr();
+ buf->buffer[buf->writeoffset++] = byte;
+ buf->writeoffset %= buf->len;
+#ifdef CONFIG_CHECK_STACK_USAGE /* This config variable does not exist yet. */
+ int i;
+ /* stack usage checker, works only during CAR, need to rewrite */
+ for (i = 0; i < 2048; i++)
+ if (buf->buffer[buf->len + i] != 0x0)
+ break;
+ buf->loweststack = &buf->buffer[buf->len + i];
+#endif
+ return;
+}
+#endif
+
void console_tx_byte(unsigned char byte, void *arg)
{
+#ifdef CONFIG_CONSOLE_BUFFER
+ buffer_tx_byte(byte, arg);
+#endif
+#ifdef CONFIG_CONSOLE_SERIAL
if (byte == '\n') {
uart8250_tx_byte(TTYSx_BASE, '\r');
#if defined(CONFIG_CONSOLE_PREFIX) && (CONFIG_CONSOLE_PREFIX == 1)
@@ -26,8 +84,8 @@
return;
#endif
}
-
uart8250_tx_byte(TTYSx_BASE, byte);
+#endif
}
int printk(int msg_level, const char *fmt, ...)
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,13 @@
movw %ax, %ss
lout:
+#ifdef CONFIG_CONSOLE_BUFFER
+ /* Store pointer to start of printk buffer, should really use
+ * PRINTK_BUF_ADDR_CAR instead.
+ */
+ movl $CONFIG_CARBASE, %eax
+ pushl %eax /* printk buffer */
+#endif
/* Restore the BIST result. */
movl %ebp, %eax
Index: LinuxBIOSv3-printkbuffer/arch/x86/stage0_i586.S
===================================================================
--- LinuxBIOSv3-printkbuffer/arch/x86/stage0_i586.S (Revision 587)
+++ LinuxBIOSv3-printkbuffer/arch/x86/stage0_i586.S (Arbeitskopie)
@@ -433,7 +433,13 @@
movw %ax, %ss
lout:
-
+#ifdef CONFIG_CONSOLE_BUFFER
+ /* Store pointer to start of printk buffer, should really use
+ * PRINTK_BUF_ADDR_CAR instead.
+ */
+ movl $CONFIG_CARBASE, %eax
+ pushl %eax /* printk buffer */
+#endif
/* Restore the BIST result */
movl %ebp, %eax
/* We need to set ebp ? No need */
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,12 @@
}
+void *bottom_of_stack(void)
+{
+ /* -4-4 because CONFIG_CARBASE + CONFIG_CARSIZE - 4 is initial %esp */
+ return (void *)(CONFIG_CARBASE + CONFIG_CARSIZE - 4 - 4);
+}
+
void dump_mem_range(int msg_level, unsigned char *buf, int size)
{
int i;
@@ -129,6 +136,11 @@
// We have cache as ram running and can start executing code in C.
+#ifdef CONFIG_CONSOLE_BUFFER
+ /* Initialize the printk buffer. */
+ printk_buffer_init();
+#endif
+
hardware_stage1();
//
@@ -173,10 +185,14 @@
printk(BIOS_DEBUG, "Done RAM init code\n");
-
/* Turn off Cache-As-Ram */
disable_car();
+#ifdef CONFIG_CONSOLE_BUFFER
+ /* Move the printk buffer to PRINTK_BUF_ADDR_RAM */
+ printk_buffer_move((void *)PRINTK_BUF_ADDR_RAM, PRINTK_BUF_SIZE_RAM);
+#endif
+
entry = load_file_segments(&archive, "normal/stage2");
if (entry == (void *)-1)
die("FATAL: Failed loading stage2.");
--
http://www.hailfinger.org
More information about the coreboot
mailing list