[PATCH] smbios: Fix BIOS Characteristics Extension Byte 2

[Ilias Apalodimas]

Hi Heinrich


On Thu, 10 Jun 2021 at 13:16, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>
> On 6/9/21 5:14 PM, Ilias Apalodimas wrote:
> > We currently define the EFI support of an SMBIOS table as the third bit of
> > "BIOS Characteristics Extension Byte 1".  The latest DMTF spec defines it
> > on "BIOS Characteristics Extension Byte 2".
> >
> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > ---
> >   include/smbios.h | 2 +-
> >   lib/smbios.c     | 5 +++--
> >   2 files changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/include/smbios.h b/include/smbios.h
> > index ffeefb47372d..fc49fc10b9d7 100644
> > --- a/include/smbios.h
> > +++ b/include/smbios.h
> > @@ -60,7 +60,7 @@ struct __packed smbios_entry {
> >   #define BIOS_CHARACTERISTICS_SELECTABLE_BOOT        (1 << 16)
> >
> >   #define BIOS_CHARACTERISTICS_EXT1_ACPI              (1 << 0)
> > -#define BIOS_CHARACTERISTICS_EXT1_UEFI               (1 << 3)
> > +#define BIOS_CHARACTERISTICS_EXT2_UEFI               (1 << 3)
> >   #define BIOS_CHARACTERISTICS_EXT2_TARGET    (1 << 2)
> >
> >   struct __packed smbios_type0 {
> > diff --git a/lib/smbios.c b/lib/smbios.c
> > index 9eb226ec9fbd..abdd157a7084 100644
> > --- a/lib/smbios.c
> > +++ b/lib/smbios.c
> > @@ -214,6 +214,7 @@ static int smbios_write_type0(ulong *current, int handle,
> >               gd->smbios_version = ctx->last_str;
> >       log_debug("smbios_version = %p: '%s'\n", gd->smbios_version,
> >                 gd->smbios_version);
> > +     t->bios_characteristics_ext2 = 0;
> >   #ifdef LOG_DEBUG
> >       print_buffer((ulong)gd->smbios_version, gd->smbios_version,
> >                    1, strlen(gd->smbios_version) + 1, 0);
> > @@ -229,9 +230,9 @@ static int smbios_write_type0(ulong *current, int handle,
> >       t->bios_characteristics_ext1 = BIOS_CHARACTERISTICS_EXT1_ACPI;
>
> This looks fishy. Where is t->bios_characteristics_ext1 initialized if
> CONFIG_GENERATE_ACPI_TABLE=n? efi_smbios_register() does not zero out
> the memory.
>
> We should initialize the field irrespective of the configuration and
> then use a bitwise or here.
>
> >   #endif
> >   #ifdef CONFIG_EFI_LOADER
> > -     t->bios_characteristics_ext1 |= BIOS_CHARACTERISTICS_EXT1_UEFI;
> > +     t->bios_characteristics_ext2 |= BIOS_CHARACTERISTICS_EXT2_UEFI;
> >   #endif
> > -     t->bios_characteristics_ext2 = BIOS_CHARACTERISTICS_EXT2_TARGET;
> > +     t->bios_characteristics_ext2 |= BIOS_CHARACTERISTICS_EXT2_TARGET;
>
> Where is t->bios_characteristics_ext2 initialized?
> We don't want random bytes.

A few lines above, in this patchset

>
> @Simon:
> The usage of ulong instead of pointers in this module does not make
> sense to me. If the sandbox needs to call it, it should map its virtual
> addresses. We should not spill those conversion into non-sandbox code.
>
> Best regards
>
> Heinrich
>
> >
> >       /* bios_major_release has only one byte, so drop century */
> >       t->bios_major_release = U_BOOT_VERSION_NUM % 100;
> >
>