* [PATCH v4 1/2] x86: coreboot: Document cbmem console struct
@ 2023-09-10 19:13 Simon Glass
2023-09-10 19:13 ` [PATCH v4 2/2] x86: Update cbmem driver Simon Glass
2023-09-19 8:47 ` [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Bin Meng
0 siblings, 2 replies; 6+ messages in thread
From: Simon Glass @ 2023-09-10 19:13 UTC (permalink / raw)
To: U-Boot Mailing List; +Cc: Alex Sadovsky, Simon Glass, Bin Meng, Paul Menzel
Coreboot changed a few years ago to include an overflow flag. Update the
structure to match this.
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 v4:
- Reword commit and change title
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
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v4 2/2] x86: Update cbmem driver
2023-09-10 19:13 [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Simon Glass
@ 2023-09-10 19:13 ` Simon Glass
2023-09-19 8:47 ` Bin Meng
2023-09-19 8:47 ` [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Bin Meng
1 sibling, 1 reply; 6+ messages in thread
From: Simon Glass @ 2023-09-10 19:13 UTC (permalink / raw)
To: U-Boot Mailing List
Cc: Alex Sadovsky, Simon Glass, Bin Meng, Paul Menzel,
Jean-Marie Lemetayer, Jonas Karlman, Kever Yang, Konrad Dybcio,
Peng Fan, Vladimir Zapolskiy
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>
---
Changes in v4:
- Add some comments to help understand the overflow mechanism
Changes in v2:
- Update to support the new overflow mechanism
drivers/misc/Kconfig | 8 +++++++
drivers/misc/cbmem_console.c | 43 ++++++++++++++++++++++--------------
2 files changed, 34 insertions(+), 17 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..3cbe9fb1a46a 100644
--- a/drivers/misc/cbmem_console.c
+++ b/drivers/misc/cbmem_console.c
@@ -5,27 +5,37 @@
#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;
+ 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;
+
+ /* preserve the overflow flag if present */
+ flags = cons->cursor & ~CBMC_CURSOR_MASK;
+
+ cons->body[pos++] = data;
+
+ /*
+ * Deal with overflow - the flag may be cleared by another program which
+ * reads the buffer out, e.g. Linux
+ */
+ if (pos >= cons->size) {
+ pos = 0;
+ flags |= CBMC_OVERFLOW;
+ }
- cursor = cbmem_console_p->buffer_cursor++;
- if (cursor < cbmem_console_p->buffer_size)
- cbmem_console_p->buffer_body[cursor] = data;
+ cons->cursor = flags | pos;
}
void cbmemc_puts(struct stdio_dev *dev, const char *str)
@@ -40,7 +50,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));
--
2.42.0.283.g2d96d420d3-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] x86: Update cbmem driver
2023-09-10 19:13 ` [PATCH v4 2/2] x86: Update cbmem driver Simon Glass
@ 2023-09-19 8:47 ` Bin Meng
2023-09-20 2:59 ` Simon Glass
0 siblings, 1 reply; 6+ messages in thread
From: Bin Meng @ 2023-09-19 8:47 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alex Sadovsky, Paul Menzel,
Jean-Marie Lemetayer, Jonas Karlman, Kever Yang, Konrad Dybcio,
Peng Fan, Vladimir Zapolskiy
Hi Simon,
On Mon, Sep 11, 2023 at 3:13 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>
> ---
>
> Changes in v4:
> - Add some comments to help understand the overflow mechanism
>
> Changes in v2:
> - Update to support the new overflow mechanism
>
> drivers/misc/Kconfig | 8 +++++++
> drivers/misc/cbmem_console.c | 43 ++++++++++++++++++++++--------------
> 2 files changed, 34 insertions(+), 17 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..3cbe9fb1a46a 100644
> --- a/drivers/misc/cbmem_console.c
> +++ b/drivers/misc/cbmem_console.c
> @@ -5,27 +5,37 @@
>
> #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;
> + 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;
> +
> + /* preserve the overflow flag if present */
> + flags = cons->cursor & ~CBMC_CURSOR_MASK;
> +
> + cons->body[pos++] = data;
> +
> + /*
> + * Deal with overflow - the flag may be cleared by another program which
> + * reads the buffer out, e.g. Linux
> + */
U-Boot is not memory resident, so this does not sound like a correct
overflow mechanism to me.
> + if (pos >= cons->size) {
> + pos = 0;
> + flags |= CBMC_OVERFLOW;
> + }
>
> - cursor = cbmem_console_p->buffer_cursor++;
> - if (cursor < cbmem_console_p->buffer_size)
> - cbmem_console_p->buffer_body[cursor] = data;
> + cons->cursor = flags | pos;
> }
>
> void cbmemc_puts(struct stdio_dev *dev, const char *str)
> @@ -40,7 +50,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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 1/2] x86: coreboot: Document cbmem console struct
2023-09-10 19:13 [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Simon Glass
2023-09-10 19:13 ` [PATCH v4 2/2] x86: Update cbmem driver Simon Glass
@ 2023-09-19 8:47 ` Bin Meng
1 sibling, 0 replies; 6+ messages in thread
From: Bin Meng @ 2023-09-19 8:47 UTC (permalink / raw)
To: Simon Glass; +Cc: U-Boot Mailing List, Alex Sadovsky, Paul Menzel
On Mon, Sep 11, 2023 at 3:13 AM Simon Glass <sjg@chromium.org> wrote:
>
> Coreboot changed a few years ago to include an overflow flag. Update the
> structure to match this.
>
> 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 v4:
> - Reword commit and change title
>
> Changes in v3:
> - Drop __packed as it does nothing useful
>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] x86: Update cbmem driver
2023-09-19 8:47 ` Bin Meng
@ 2023-09-20 2:59 ` Simon Glass
2023-09-21 5:00 ` Bin Meng
0 siblings, 1 reply; 6+ messages in thread
From: Simon Glass @ 2023-09-20 2:59 UTC (permalink / raw)
To: Bin Meng
Cc: U-Boot Mailing List, Alex Sadovsky, Paul Menzel,
Jean-Marie Lemetayer, Jonas Karlman, Kever Yang, Konrad Dybcio,
Peng Fan, Vladimir Zapolskiy
Hi Bin,
On Tue, 19 Sept 2023 at 02:47, Bin Meng <bmeng.cn@gmail.com> wrote:
>
> Hi Simon,
>
> On Mon, Sep 11, 2023 at 3:13 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>
> > ---
> >
> > Changes in v4:
> > - Add some comments to help understand the overflow mechanism
> >
> > Changes in v2:
> > - Update to support the new overflow mechanism
> >
> > drivers/misc/Kconfig | 8 +++++++
> > drivers/misc/cbmem_console.c | 43 ++++++++++++++++++++++--------------
> > 2 files changed, 34 insertions(+), 17 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..3cbe9fb1a46a 100644
> > --- a/drivers/misc/cbmem_console.c
> > +++ b/drivers/misc/cbmem_console.c
> > @@ -5,27 +5,37 @@
> >
> > #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;
> > + 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;
> > +
> > + /* preserve the overflow flag if present */
> > + flags = cons->cursor & ~CBMC_CURSOR_MASK;
> > +
> > + cons->body[pos++] = data;
> > +
> > + /*
> > + * Deal with overflow - the flag may be cleared by another program which
> > + * reads the buffer out, e.g. Linux
> > + */
>
> U-Boot is not memory resident, so this does not sound like a correct
> overflow mechanism to me.
I am not sure what you mean. This logic is used in coreboot and some
payloads, so I want to do the same in U-Boot. It basically let's
U-Boot handle an overflow properly by setting the overflow flag. A
later program (e.g. Linux) can then tell that that an overflow
occurred.
>
> > + if (pos >= cons->size) {
> > + pos = 0;
> > + flags |= CBMC_OVERFLOW;
> > + }
> >
> > - cursor = cbmem_console_p->buffer_cursor++;
> > - if (cursor < cbmem_console_p->buffer_size)
> > - cbmem_console_p->buffer_body[cursor] = data;
> > + cons->cursor = flags | pos;
> > }
> >
> > void cbmemc_puts(struct stdio_dev *dev, const char *str)
> > @@ -40,7 +50,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,
Simon
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v4 2/2] x86: Update cbmem driver
2023-09-20 2:59 ` Simon Glass
@ 2023-09-21 5:00 ` Bin Meng
0 siblings, 0 replies; 6+ messages in thread
From: Bin Meng @ 2023-09-21 5:00 UTC (permalink / raw)
To: Simon Glass
Cc: U-Boot Mailing List, Alex Sadovsky, Paul Menzel,
Jean-Marie Lemetayer, Jonas Karlman, Kever Yang, Konrad Dybcio,
Peng Fan, Vladimir Zapolskiy
Hi Simon,
On Wed, Sep 20, 2023 at 10:59 AM Simon Glass <sjg@chromium.org> wrote:
>
> Hi Bin,
>
> On Tue, 19 Sept 2023 at 02:47, Bin Meng <bmeng.cn@gmail.com> wrote:
> >
> > Hi Simon,
> >
> > On Mon, Sep 11, 2023 at 3:13 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>
> > > ---
> > >
> > > Changes in v4:
> > > - Add some comments to help understand the overflow mechanism
> > >
> > > Changes in v2:
> > > - Update to support the new overflow mechanism
> > >
> > > drivers/misc/Kconfig | 8 +++++++
> > > drivers/misc/cbmem_console.c | 43 ++++++++++++++++++++++--------------
> > > 2 files changed, 34 insertions(+), 17 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..3cbe9fb1a46a 100644
> > > --- a/drivers/misc/cbmem_console.c
> > > +++ b/drivers/misc/cbmem_console.c
> > > @@ -5,27 +5,37 @@
> > >
> > > #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;
> > > + 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;
> > > +
> > > + /* preserve the overflow flag if present */
> > > + flags = cons->cursor & ~CBMC_CURSOR_MASK;
> > > +
> > > + cons->body[pos++] = data;
> > > +
> > > + /*
> > > + * Deal with overflow - the flag may be cleared by another program which
> > > + * reads the buffer out, e.g. Linux
> > > + */
> >
> > U-Boot is not memory resident, so this does not sound like a correct
> > overflow mechanism to me.
>
> I am not sure what you mean. This logic is used in coreboot and some
> payloads, so I want to do the same in U-Boot. It basically let's
> U-Boot handle an overflow properly by setting the overflow flag. A
> later program (e.g. Linux) can then tell that that an overflow
> occurred.
>
Okay, I modified the comment to
/*
* Deal with overflow - the flag may be cleared by another program which
* reads the buffer out later, e.g. Linux
*/
which makes more sense.
applied to u-boot-x86/next, thanks!
Regards,
Bin
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-21 5:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-10 19:13 [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Simon Glass
2023-09-10 19:13 ` [PATCH v4 2/2] x86: Update cbmem driver Simon Glass
2023-09-19 8:47 ` Bin Meng
2023-09-20 2:59 ` Simon Glass
2023-09-21 5:00 ` Bin Meng
2023-09-19 8:47 ` [PATCH v4 1/2] x86: coreboot: Document cbmem console struct Bin Meng
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.