Re: [PATCH v3 1/2] x86: coreboot: Update cbmem table

Hi Simon,

On Thu, Aug 17, 2023 at 9:58 AM Simon Glass <[email protected]> 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 <[email protected]>
> ---
>
> 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