This changed a few years ago to include an overflow flag. Bring in the new structure.
This comes from coreboot commit:
6f5ead14b4 ("mb/google/nissa/var/joxer: Update eMMC DLL settings")
Note: There are several implementations of this in coreboot. I have chosen to follow the one in src/lib/cbmem_console.c
Signed-off-by: Simon Glass sjg@chromium.org ---
Changes in v3: - Drop __packed as it does nothing useful
arch/x86/include/asm/coreboot_tables.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index 4de137fbab9d..0dfb64babb96 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -299,11 +299,24 @@ struct cb_vdat { #define CB_TAG_TIMESTAMPS 0x0016 #define CB_TAG_CBMEM_CONSOLE 0x0017
+#define CBMC_CURSOR_MASK ((1 << 28) - 1) +#define CBMC_OVERFLOW BIT(31) + +/* + * struct cbmem_console - In-memory console buffer for coreboot + * + * Structure describing console buffer. It is overlaid on a flat memory area, + * with body covering the extent of the memory. Once the buffer is full, + * output will wrap back around to the start of the buffer. The high bit of the + * cursor field gets set to indicate that this happened. If the underlying + * storage allows this, the buffer will persist across multiple boots and append + * to the previous log. + */ struct cbmem_console { u32 size; u32 cursor; - char body[0]; -} __packed; + u8 body[0]; +};
#define CB_TAG_MRC_CACHE 0x0018
This driver is not actually built since a Kconfig was never created for it.
Add a Kconfig (which is already implied by COREBOOT) and update the implementation to avoid using unnecessary memory. Drop the #ifdef at the top since we can rely on Kconfig to get that right.
To enable it (in addition to serial and video), use:
setenv stdout serial,vidconsole,cbmem
Signed-off-by: Simon Glass sjg@chromium.org ---
(no changes since v2)
Changes in v2: - Update to support the new overflow mechanism
drivers/misc/Kconfig | 8 ++++++++ drivers/misc/cbmem_console.c | 38 +++++++++++++++++++----------------- 2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index a6e3f62ecb09..c930e4a361bf 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -122,6 +122,14 @@ config VEXPRESS_CONFIG configuration bus on the Arm Versatile Express boards via a sysreg driver.
+config CBMEM_CONSOLE + bool "Write console output to coreboot cbmem" + depends on X86 + help + Enables console output to the cbmem console, which is a memory + region set up by coreboot to hold a record of all console output. + Enable this only if booting from coreboot. + config CMD_CROS_EC bool "Enable crosec command" depends on CROS_EC diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c index 8bbe33d414da..7a47a814f638 100644 --- a/drivers/misc/cbmem_console.c +++ b/drivers/misc/cbmem_console.c @@ -5,27 +5,30 @@
#include <common.h> #include <console.h> -#ifndef CONFIG_SYS_COREBOOT -#error This driver requires coreboot -#endif - #include <asm/cb_sysinfo.h>
-struct cbmem_console { - u32 buffer_size; - u32 buffer_cursor; - u8 buffer_body[0]; -} __attribute__ ((__packed__)); - -static struct cbmem_console *cbmem_console_p; - void cbmemc_putc(struct stdio_dev *dev, char data) { - int cursor; - - cursor = cbmem_console_p->buffer_cursor++; - if (cursor < cbmem_console_p->buffer_size) - cbmem_console_p->buffer_body[cursor] = data; + const struct sysinfo_t *sysinfo = cb_get_sysinfo(); + struct cbmem_console *cons; + uint pos, flags; + + if (!sysinfo) + return; + cons = sysinfo->cbmem_cons; + if (!cons) + return; + + pos = cons->cursor & CBMC_CURSOR_MASK; + flags = cons->cursor & ~CBMC_CURSOR_MASK; + + cons->body[pos++] = data; + if (pos >= cons->size) { + pos = 0; + flags |= CBMC_OVERFLOW; + } + + cons->cursor = flags | pos; }
void cbmemc_puts(struct stdio_dev *dev, const char *str) @@ -40,7 +43,6 @@ int cbmemc_init(void) { int rc; struct stdio_dev cons_dev; - cbmem_console_p = lib_sysinfo.cbmem_cons;
memset(&cons_dev, 0, sizeof(cons_dev));
Hi Simon,
On Thu, Aug 17, 2023 at 9:58 AM Simon Glass sjg@chromium.org wrote:
This driver is not actually built since a Kconfig was never created for it.
Add a Kconfig (which is already implied by COREBOOT) and update the implementation to avoid using unnecessary memory. Drop the #ifdef at the top since we can rely on Kconfig to get that right.
To enable it (in addition to serial and video), use:
setenv stdout serial,vidconsole,cbmem
Signed-off-by: Simon Glass sjg@chromium.org
(no changes since v2)
Changes in v2:
- Update to support the new overflow mechanism
drivers/misc/Kconfig | 8 ++++++++ drivers/misc/cbmem_console.c | 38 +++++++++++++++++++----------------- 2 files changed, 28 insertions(+), 18 deletions(-)
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig index a6e3f62ecb09..c930e4a361bf 100644 --- a/drivers/misc/Kconfig +++ b/drivers/misc/Kconfig @@ -122,6 +122,14 @@ config VEXPRESS_CONFIG configuration bus on the Arm Versatile Express boards via a sysreg driver.
+config CBMEM_CONSOLE
bool "Write console output to coreboot cbmem"depends on X86helpEnables console output to the cbmem console, which is a memoryregion set up by coreboot to hold a record of all console output.Enable this only if booting from coreboot.config CMD_CROS_EC bool "Enable crosec command" depends on CROS_EC diff --git a/drivers/misc/cbmem_console.c b/drivers/misc/cbmem_console.c index 8bbe33d414da..7a47a814f638 100644 --- a/drivers/misc/cbmem_console.c +++ b/drivers/misc/cbmem_console.c @@ -5,27 +5,30 @@
#include <common.h> #include <console.h> -#ifndef CONFIG_SYS_COREBOOT -#error This driver requires coreboot -#endif
#include <asm/cb_sysinfo.h>
-struct cbmem_console {
u32 buffer_size;u32 buffer_cursor;u8 buffer_body[0];-} __attribute__ ((__packed__));
-static struct cbmem_console *cbmem_console_p;
void cbmemc_putc(struct stdio_dev *dev, char data) {
int cursor;cursor = cbmem_console_p->buffer_cursor++;if (cursor < cbmem_console_p->buffer_size)cbmem_console_p->buffer_body[cursor] = data;
const struct sysinfo_t *sysinfo = cb_get_sysinfo();struct cbmem_console *cons;uint pos, flags;if (!sysinfo)return;cons = sysinfo->cbmem_cons;if (!cons)return;pos = cons->cursor & CBMC_CURSOR_MASK;flags = cons->cursor & ~CBMC_CURSOR_MASK;
This line does not seem useful to me, as we don't use the extracted flag from the higher bits of cons->cursor.
I guess we can just drop it.
cons->body[pos++] = data;if (pos >= cons->size) {pos = 0;flags |= CBMC_OVERFLOW;
Who is supposed to clear the overflow flag?
}cons->cursor = flags | pos;}
void cbmemc_puts(struct stdio_dev *dev, const char *str) @@ -40,7 +43,6 @@ int cbmemc_init(void) { int rc; struct stdio_dev cons_dev;
cbmem_console_p = lib_sysinfo.cbmem_cons; memset(&cons_dev, 0, sizeof(cons_dev));--
Regards, Bin
Hi Simon,
On Thu, Aug 17, 2023 at 9:58 AM Simon Glass sjg@chromium.org wrote:
This changed a few years ago to include an overflow flag. Bring in the new structure.
s/This/Coreboot
"Bring in the new structure" does not sound correct to me. The structure is not changed. It's adding a comment block and some bit fields.
The commit summary is not accurately reflecting what is changed in this patch. How about:
x86: coreboot: Document cbmem console struct
This comes from coreboot commit:
6f5ead14b4 ("mb/google/nissa/var/joxer: Update eMMC DLL settings")
Note: There are several implementations of this in coreboot. I have chosen to follow the one in src/lib/cbmem_console.c
Signed-off-by: Simon Glass sjg@chromium.org
Changes in v3:
- Drop __packed as it does nothing useful
arch/x86/include/asm/coreboot_tables.h | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)
diff --git a/arch/x86/include/asm/coreboot_tables.h b/arch/x86/include/asm/coreboot_tables.h index 4de137fbab9d..0dfb64babb96 100644 --- a/arch/x86/include/asm/coreboot_tables.h +++ b/arch/x86/include/asm/coreboot_tables.h @@ -299,11 +299,24 @@ struct cb_vdat { #define CB_TAG_TIMESTAMPS 0x0016 #define CB_TAG_CBMEM_CONSOLE 0x0017
+#define CBMC_CURSOR_MASK ((1 << 28) - 1) +#define CBMC_OVERFLOW BIT(31)
+/*
- struct cbmem_console - In-memory console buffer for coreboot
- Structure describing console buffer. It is overlaid on a flat memory area,
- with body covering the extent of the memory. Once the buffer is full,
- output will wrap back around to the start of the buffer. The high bit of the
- cursor field gets set to indicate that this happened. If the underlying
- storage allows this, the buffer will persist across multiple boots and append
- to the previous log.
- */
struct cbmem_console { u32 size; u32 cursor;
char body[0];-} __packed;
u8 body[0];+};
#define CB_TAG_MRC_CACHE 0x0018
--
Regards, Bin
-
Bin Meng -
Simon Glass