[PATCH v4 1/4] efi_loader: add SMBIOS table measurement

[Masahisa Kojima]

Hi Simon,

On Thu, 4 Nov 2021 at 11:49, Simon Glass <sjg at chromium.org> wrote:
>
> Hi Masahisa,
>
> On Wed, 3 Nov 2021 at 19:16, Masahisa Kojima <masahisa.kojima at linaro.org> wrote:
> >
> > Hi Simon,
> >
> > On Tue, 2 Nov 2021 at 23:56, Simon Glass <sjg at chromium.org> wrote:
> > >
> > >  Hi Masahisa,
> > >
> > > On Tue, 26 Oct 2021 at 02:26, Masahisa Kojima
> > > <masahisa.kojima at linaro.org> wrote:
> > > >
> > > > TCG PC Client Platform Firmware Profile Specification
> > > > requires to measure the SMBIOS table that contains static
> > > > configuration information (e.g. Platform Manufacturer
> > > > Enterprise Number assigned by IANA, platform model number,
> > > > Vendor and Device IDs for each SMBIOS table).
> > > >
> > > > The device- and environment-dependent information such as
> > > > serial number is cleared to zero or space character for
> > > > the measurement.
> > > >
> > > > Existing smbios_string() function returns pointer to the string
> > > > with const qualifier, but exisintg use case is updating version
> > > > string and const qualifier must be removed.
> > > > This commit removes const qualifier from smbios_string()
> > > > return value and reuses to clear the strings for the measurement.
> > > >
> > > > This commit also fixes the following compiler warning:
> > > >
> > > > lib/smbios-parser.c:59:39: warning: cast to pointer from integer of
> > > > different size [-Wint-to-pointer-cast]
> > > >   const struct smbios_header *header = (struct smbios_header *)entry->struct_table_address;
> > > >
> > > > Signed-off-by: Masahisa Kojima <masahisa.kojima at linaro.org>
> > > > ---
> > > > Changes in v4:
> > > > - update commit message
> > > >
> > > > Changes in v3:
> > > > - TCG spec says EV_SEPARATOR must be the last,
> > > >   swap the order of measurement
> > > >
> > > > Changes in v2:
> > > > - use flexible array for table_entry field
> > > > - modify funtion name to find_smbios_table()
> > > > - remove unnecessary const qualifier from smbios_string()
> > > > - create non-const version of next_header()
> > > >
> > > >  include/efi_loader.h          |   2 +
> > > >  include/efi_tcg2.h            |  15 ++++
> > > >  include/smbios.h              |  17 +++-
> > > >  lib/efi_loader/Kconfig        |   1 +
> > > >  lib/efi_loader/efi_boottime.c |   2 +
> > > >  lib/efi_loader/efi_smbios.c   |   2 -
> > > >  lib/efi_loader/efi_tcg2.c     |  84 +++++++++++++++++++
> > > >  lib/smbios-parser.c           | 152 +++++++++++++++++++++++++++++++---
> > > >  8 files changed, 261 insertions(+), 14 deletions(-)
> > >
> > > I don't understand what efi_system_table is. Could someone add a
> > > comment about that to the code?
> >
> > efi_system_table is defined in the UEFI Specification.
> >
> > >
> > > Also efi_configuration_table is an array of void * to what? Again I
> > > don't see any comment in the header file...
> >
> > It is also defined in the UEFI Specification.
> >
> > typedef struct{
> >     EFI_GUID VendorGuid;
> >     VOID *VendorTable;
> > } EFI_CONFIGURATION_TABLE;
> >
> > efi_configuration_table will contain the guid and pointer to the industry
> > standard configuration tables such as APCI and SMBIOS.
> >
> > >
> > > How does this match up with the SMBIOS table already in U-Boot? Is it
> > > a pointer to it?
> >
> > efi_system_table has array of efi_configuration_table, SMBIOS tables is
> > installed(saving pointer into the array of efi_configuration_table
> > together with guid)
> > by calling efi_install_configuration_table().
> >
>
> OK thank you. So while you are here could you add a patch to add
> comments to this obtuse code? The specification is a mile long and the
> poor punters need to be able to figure out what is going on,
> particularly with void pointers.
>
> Reviewed-by: Simon Glass <sjg at chromium.org>

Thank you for your review.

OK, I will add the comment for the struct efi_configuration_table
and struct efi_system_table.

Thanks,
Masahisa Kojima


Thanks,
Masahisa Kojima

>
> Regards,
> Simon