[PATCH v4 6/9] x86: Move coreboot-table detection into common code

[Bin Meng]

Hi Simon,

On Sun, Apr 26, 2020 at 11:13 PM Simon Glass <sjg at chromium.org> wrote:
>
> To support detecting booting from coreboot, move the code which locates
> the coreboot tables into a common place. Adjust the algorithm slightly to
> use a word comparison instead of string, since it is faster.
>
> Signed-off-by: Simon Glass <sjg at chromium.org>
> ---
>
> Changes in v4:
> - Add new patch to move coreboot-table detection into common code
>
> Changes in v3: None
> Changes in v2: None
>
>  arch/x86/cpu/coreboot/tables.c         | 24 +++++++++---------------
>  arch/x86/cpu/i386/cpu.c                | 25 +++++++++++++++++++++++++
>  arch/x86/include/asm/coreboot_tables.h |  7 +++++++
>  3 files changed, 41 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/cpu/coreboot/tables.c b/arch/x86/cpu/coreboot/tables.c
> index 37e0424b5e..0f04c4f8e9 100644
> --- a/arch/x86/cpu/coreboot/tables.c
> +++ b/arch/x86/cpu/coreboot/tables.c
> @@ -115,20 +115,11 @@ __weak void cb_parse_unhandled(u32 tag, unsigned char *ptr)
>
>  static int cb_parse_header(void *addr, int len, struct sysinfo_t *info)
>  {
> +       unsigned char *ptr = addr;

This variable is not necessary

>         struct cb_header *header;
> -       unsigned char *ptr = (unsigned char *)addr;
>         int i;
>
> -       for (i = 0; i < len; i += 16, ptr += 16) {
> -               header = (struct cb_header *)ptr;
> -               if (!strncmp((const char *)header->signature, "LBIO", 4))
> -                       break;
> -       }
> -
> -       /* We walked the entire space and didn't find anything. */
> -       if (i >= len)
> -               return -1;
> -
> +       header = (struct cb_header *)ptr;

and assign addr to header directly

>         if (!header->table_bytes)
>                 return 0;
>
> @@ -231,10 +222,13 @@ static int cb_parse_header(void *addr, int len, struct sysinfo_t *info)
>
>  int get_coreboot_info(struct sysinfo_t *info)
>  {
> -       int ret = cb_parse_header((void *)0x00000000, 0x1000, info);
> +       long addr;
> +       int ret;
>
> -       if (ret != 1)
> -               ret = cb_parse_header((void *)0x000f0000, 0x1000, info);
> +       addr = locate_coreboot_table();
> +       if (addr < 0)
> +               return addr;
> +       ret = cb_parse_header((void *)addr, 0x1000, info);
>
> -       return (ret == 1) ? 0 : -1;
> +       return ret == 1 ? 0 : -ENOENT;
>  }
> diff --git a/arch/x86/cpu/i386/cpu.c b/arch/x86/cpu/i386/cpu.c
> index c8da7f10e9..e52fd686d9 100644
> --- a/arch/x86/cpu/i386/cpu.c
> +++ b/arch/x86/cpu/i386/cpu.c
> @@ -447,6 +447,31 @@ int x86_cpu_init_f(void)
>         return 0;
>  }
>
> +long detect_coreboot_table_at(ulong start, ulong size)
> +{
> +       u32 *ptr, *end;
> +
> +       size /= 4;
> +       for (ptr = (void *)start, end = ptr + size; ptr < end; ptr += 4) {
> +               if (*ptr == 0x4f49424c) /* "LBIO" */
> +                       return (long)ptr;
> +       }
> +
> +       return -ENOENT;
> +}
> +
> +long locate_coreboot_table(void)
> +{
> +       long addr;
> +
> +       /* We look for LBIO in the first 4K of RAM and again at 60KB */

It should be 960KB

> +       addr = detect_coreboot_table_at(0x0, 0x1000);
> +       if (addr < 0)
> +               addr = detect_coreboot_table_at(0xf0000, 0x1000);
> +
> +       return addr;
> +}
> +

Looks good otherwise:

Reviewed-by: Bin Meng <bmeng.cn at gmail.com>

Regards,
Bin