Hi, Patrick
In this patch I noticed that we changed the BlSupportDxe dependency from True
to gEfiSmbiosProtocolGuid.
Since BlSupportDxe is considered as critical for UEFI payload, and on the
other side SMBIOS driver could be optional in some case, do you think it is
better to handle it through RegisterProtocolNotify() ? In this way, if
gEfiSmbiosProtocolGuid is not installed for any reason, BlSupportDxe can still
be dispatched and the boot flow can continue.
Some other comments:
- Please add function and parameter description for BlDxeInstallSMBIOStables().
- To follow the naming convention in EDK2, maybe use
BlDxeInstallSmbiosTables instead of BlDxeInstallSMBIOStables().
Thanks
Maurice
> -----Original Message-----
> From: Patrick Rudolph <patrick.rudo...@9elements.com>
> Sent: Wednesday, January 20, 2021 8:02
> To: devel@edk2.groups.io
> Cc: Ma, Maurice <maurice...@intel.com>; Dong, Guo <guo.d...@intel.com>;
> You, Benjamin <benjamin....@intel.com>
> Subject: [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to
> install tables
>
> The default EfiSmbiosProtocol operates on an empty SMBIOS table.
> As the SMBIOS tables are provided by the bootloader, install the SMBIOS tables
> using the EfiSmbiosProtocol.
>
> This fixes the settings menu not showing any hardware information, instead
> only
> "0 MB RAM" was displayed.
>
> Tests showed that the OS can still see the SMBIOS tables.
>
> Signed-off-by: Patrick Rudolph <patrick.rudo...@9elements.com>
> ---
> UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c | 111
> +++++++++++++++++++-
> UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h | 3 +
> UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 5 +-
> 3 files changed, 115 insertions(+), 4 deletions(-)
>
> diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
> b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
> index a746d0581e..db478c1abc 100644
> --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
> +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c
> @@ -79,6 +79,107 @@ ReserveResourceInGcd (
> return Status; } +EFI_STATUS+EFIAPI+BlDxeInstallSMBIOStables(+ IN UINT64
> SmbiosTableBase,+ IN UINT32 SmbiosTableSize+)+{+ EFI_STATUS
> Status;+ SMBIOS_TABLE_ENTRY_POINT *SmbiosTable;+
> SMBIOS_TABLE_3_0_ENTRY_POINT *Smbios30Table;+
> SMBIOS_STRUCTURE_POINTER Smbios;+ SMBIOS_STRUCTURE_POINTER
> SmbiosEnd;+ CHAR8 *String;+ EFI_SMBIOS_HANDLE
> SmbiosHandle;+ EFI_SMBIOS_PROTOCOL *SmbiosProto;++ //+ // Locate
> Smbios protocol.+ //+ Status = gBS->LocateProtocol (&gEfiSmbiosProtocolGuid,
> NULL, (VOID **)&SmbiosProto);+ if (EFI_ERROR (Status)) {+ DEBUG
> ((DEBUG_ERROR, "%a: Failed to locate gEfiSmbiosProtocolGuid\n",+
> __FUNCTION__));+ return Status;+ }++ Smbios30Table =
> (SMBIOS_TABLE_3_0_ENTRY_POINT *)(UINTN)(SmbiosTableBase);+
> SmbiosTable = (SMBIOS_TABLE_ENTRY_POINT *)(UINTN)(SmbiosTableBase);++
> if (CompareMem (Smbios30Table->AnchorString, "_SM3_", 5) == 0) {+
> Smbios.Hdr = (SMBIOS_STRUCTURE *) (UINTN) Smbios30Table-
> >TableAddress;+ SmbiosEnd.Raw = (UINT8 *) (UINTN) (Smbios30Table-
> >TableAddress + Smbios30Table->TableMaximumSize);+ if (Smbios30Table-
> >TableMaximumSize > SmbiosTableSize) {+ DEBUG((DEBUG_INFO, "%a:
> SMBIOS table size greater than reported by bootloader\n",+
> __FUNCTION__));+ }+ } else if (CompareMem (SmbiosTable->AnchorString,
> "_SM_", 4) == 0) {+ Smbios.Hdr = (SMBIOS_STRUCTURE *) (UINTN)
> SmbiosTable->TableAddress;+ SmbiosEnd.Raw = (UINT8 *) ((UINTN)
> SmbiosTable->TableAddress + SmbiosTable->TableLength);++ if (SmbiosTable-
> >TableLength > SmbiosTableSize) {+ DEBUG((DEBUG_INFO, "%a: SMBIOS
> table size greater than reported by bootloader\n",+
> __FUNCTION__));+ }+ } else {+ DEBUG ((DEBUG_ERROR, "%a: No valid
> SMBIOS table found\n", __FUNCTION__ ));+ return EFI_NOT_FOUND;+ }++
> do {+ // Check for end marker+ if (Smbios.Hdr->Type == 127) {+
> break;+ }++ // Install the table+ SmbiosHandle =
> SMBIOS_HANDLE_PI_RESERVED;+ Status = SmbiosProto->Add (+
> SmbiosProto,+ gImageHandle,+
> &SmbiosHandle,+
> Smbios.Hdr+ );+ ASSERT_EFI_ERROR (Status);+ if
> (EFI_ERROR
> (Status)) {+ return Status;+ }+ //+ // Go to the next SMBIOS
> structure.
> Each SMBIOS structure may include 2 parts:+ // 1. Formatted section; 2.
> Unformatted string section. So, 2 steps are needed+ // to skip one SMBIOS
> structure.+ //++ //+ // Step 1: Skip over formatted section.+ //+
> String =
> (CHAR8 *) (Smbios.Raw + Smbios.Hdr->Length);++ //+ // Step 2: Skip over
> unformatted string section.+ //+ do {+ //+ // Each string is
> terminated
> with a NULL(00h) BYTE and the sets of strings+ // is terminated with an
> additional NULL(00h) BYTE.+ //+ for ( ; *String != 0; String++) {+
> }++ if
> (*(UINT8*)++String == 0) {+ //+ // Pointer to the next SMBIOS
> structure.+ //+ Smbios.Raw = (UINT8 *)++String;+ break;+
> }+ }
> while (TRUE);+ } while (Smbios.Raw < SmbiosEnd.Raw);++ return
> EFI_SUCCESS;+} /** Main entry for the bootloader support DXE module.@@ -
> 133,9 +234,13 @@ BlDxeEntryPoint (
> // Install Smbios Table // if (SystemTableInfo->SmbiosTableBase != 0 &&
> SystemTableInfo->SmbiosTableSize != 0) {- DEBUG ((DEBUG_ERROR, "Install
> Smbios Table at 0x%lx, length 0x%x\n", SystemTableInfo->SmbiosTableBase,
> SystemTableInfo->SmbiosTableSize));- Status =
> gBS->InstallConfigurationTable
> (&gEfiSmbiosTableGuid, (VOID *)(UINTN)SystemTableInfo->SmbiosTableBase);-
> ASSERT_EFI_ERROR (Status);+ DEBUG ((DEBUG_ERROR, "Install Smbios Table
> at 0x%lx, length 0x%x\n",+ SystemTableInfo->SmbiosTableBase,
> SystemTableInfo->SmbiosTableSize));++ if
> (BlDxeInstallSMBIOStables(SystemTableInfo->SmbiosTableBase,
> SystemTableInfo->SmbiosTableSize) != EFI_SUCCESS) {+ Status = gBS-
> >InstallConfigurationTable (&gEfiSmbiosTableGuid, (VOID
> *)(UINTN)SystemTableInfo->SmbiosTableBase);+ ASSERT_EFI_ERROR
> (Status);+ } } //diff --git
> a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
> b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
> index 512105fafd..a5216cd2e9 100644
> --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
> +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h
> @@ -10,6 +10,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
> #include <PiDxe.h> +#include <Protocol/Smbios.h>+ #include
> <Library/UefiDriverEntryPoint.h> #include <Library/UefiBootServicesTableLib.h>
> #include <Library/DxeServicesTableLib.h>@@ -26,5 +28,6 @@ SPDX-License-
> Identifier: BSD-2-Clause-Patent #include <Guid/GraphicsInfoHob.h> #include
> <IndustryStandard/Acpi.h>+#include <IndustryStandard/SmBios.h> #endifdiff -
> -git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> index cebc811355..d26a75248b 100644
> --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> @@ -56,5 +56,8 @@
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress
> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseSize +[Protocols]+
> gEfiSmbiosProtocolGuid+ [Depex]- TRUE+ gEfiSmbiosProtocolGuid--
> 2.26.2
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70601): https://edk2.groups.io/g/devel/message/70601
Mute This Topic: https://groups.io/mt/79981721/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
