[edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables

Patrick Rudolph posted 1 patch 3 years, 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/edk2 tags/patchew/20210120160157.3343911-1-patrick.rudolph@9elements.com
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.c   | 111 +++++++++++++++++++-
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.h   |   3 +
UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf |   5 +-
3 files changed, 115 insertions(+), 4 deletions(-)
[edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables
Posted by Patrick Rudolph 3 years, 2 months ago
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.rudolph@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>
 
 #endif
diff --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 (#70587): https://edk2.groups.io/g/devel/message/70587
Mute This Topic: https://groups.io/mt/79981721/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables
Posted by Ma, Maurice 3 years, 2 months ago
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.rudolph@9elements.com>
> Sent: Wednesday, January 20, 2021 8:02
> To: devel@edk2.groups.io
> Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>;
> You, Benjamin <benjamin.you@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.rudolph@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/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables
Posted by Patrick Rudolph 3 years, 2 months ago
Hi Maurice,
I'll adapt the function names to match EDK2 naming conventions.

The SmbiosDxe is already part of UefiPayloadPkg, so it's not optional
(right now). I don't see how registering gEfiSmbiosProtocolGuid could
fail.
If you think Depex must be true, there should be
a) a comment stating the reasons why Depex must not be changed
b) I'll have to move the SMBIOS code out of BlSupportDxe into
BlSupportSmbiosDxe and add the Depex section there.
    A failed dispatch of BlSupportSmbiosDxe would then be non critical.

Do you think this would be a better solution?

I don't want to use RegisterProtocolNotify() as it's discouraged, isn't it?

Kind Regards,
Patrick Rudolph

On Thu, Jan 21, 2021 at 2:14 AM Ma, Maurice <maurice.ma@intel.com> wrote:
>
> 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.rudolph@9elements.com>
> > Sent: Wednesday, January 20, 2021 8:02
> > To: devel@edk2.groups.io
> > Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>;
> > You, Benjamin <benjamin.you@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.rudolph@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 (#70611): https://edk2.groups.io/g/devel/message/70611
Mute This Topic: https://groups.io/mt/79981721/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables
Posted by Ma, Maurice 3 years, 2 months ago
Hi, Patrick,

Since BlSupportDxe does some very basic initialization for UEFI payload (EX:  Initialize PcdPciExpressBaseAddress dynamic PCD), and it needs to be done in very early phase.  So it is desired not to push the dispatching order by adding more dependencies.  Otherwise, drivers dispatched before it might get incorrect PCD values.

Yes, I know RegisterProtocolNotify() is discouraged for UEFI driver.   This driver is not UEFI driver, so we cannot use UEFI driver model to address it here.   Do we have any alternative way without adding new dependencies in INF ?  

Thanks
Maurice

> -----Original Message-----
> From: Patrick Rudolph <patrick.rudolph@9elements.com>
> Sent: Thursday, January 21, 2021 0:11
> To: Ma, Maurice <maurice.ma@intel.com>
> Cc: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; You, Benjamin
> <benjamin.you@intel.com>
> Subject: Re: [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to
> install tables
> 
> Hi Maurice,
> I'll adapt the function names to match EDK2 naming conventions.
> 
> The SmbiosDxe is already part of UefiPayloadPkg, so it's not optional (right now).
> I don't see how registering gEfiSmbiosProtocolGuid could fail.
> If you think Depex must be true, there should be
> a) a comment stating the reasons why Depex must not be changed
> b) I'll have to move the SMBIOS code out of BlSupportDxe into
> BlSupportSmbiosDxe and add the Depex section there.
>     A failed dispatch of BlSupportSmbiosDxe would then be non critical.
> 
> Do you think this would be a better solution?
> 
> I don't want to use RegisterProtocolNotify() as it's discouraged, isn't it?
> 
> Kind Regards,
> Patrick Rudolph
> 
> On Thu, Jan 21, 2021 at 2:14 AM Ma, Maurice <maurice.ma@intel.com> wrote:
> >
> > 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.rudolph@9elements.com>
> > > Sent: Wednesday, January 20, 2021 8:02
> > > To: devel@edk2.groups.io
> > > Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> > > <guo.dong@intel.com>; You, Benjamin <benjamin.you@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.rudolph@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 (#70652): https://edk2.groups.io/g/devel/message/70652
Mute This Topic: https://groups.io/mt/79981721/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-


Re: [edk2-devel] [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol to install tables
Posted by Guo Dong 3 years, 2 months ago
How about update SmbiosDxe to detect if SMBIOS table is installed in Configuration Table in its entry point?
If it is not installed, no change to current behavior (install SmbiosProtocol on an empty SMBIOS table).
If it is installed, install SmbiosProtocol with the SMBIOS table from Configuration Table.

Thanks,
Guo

> -----Original Message-----
> From: Ma, Maurice <maurice.ma@intel.com>
> Sent: Thursday, January 21, 2021 8:33 AM
> To: Patrick Rudolph <patrick.rudolph@9elements.com>
> Cc: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; You,
> Benjamin <benjamin.you@intel.com>
> Subject: RE: [PATCH] UefiPayloadPkg/BlSupportDxe: Use EfiSmbiosProtocol
> to install tables
> 
> Hi, Patrick,
> 
> Since BlSupportDxe does some very basic initialization for UEFI payload (EX:
> Initialize PcdPciExpressBaseAddress dynamic PCD), and it needs to be done
> in very early phase.  So it is desired not to push the dispatching order by
> adding more dependencies.  Otherwise, drivers dispatched before it might
> get incorrect PCD values.
> 
> Yes, I know RegisterProtocolNotify() is discouraged for UEFI driver.   This
> driver is not UEFI driver, so we cannot use UEFI driver model to address it
> here.   Do we have any alternative way without adding new dependencies in
> INF ?
> 
> Thanks
> Maurice
> 
> > -----Original Message-----
> > From: Patrick Rudolph <patrick.rudolph@9elements.com>
> > Sent: Thursday, January 21, 2021 0:11
> > To: Ma, Maurice <maurice.ma@intel.com>
> > Cc: devel@edk2.groups.io; Dong, Guo <guo.dong@intel.com>; You,
> Benjamin
> > <benjamin.you@intel.com>
> > Subject: Re: [PATCH] UefiPayloadPkg/BlSupportDxe: Use
> EfiSmbiosProtocol to
> > install tables
> >
> > Hi Maurice,
> > I'll adapt the function names to match EDK2 naming conventions.
> >
> > The SmbiosDxe is already part of UefiPayloadPkg, so it's not optional (right
> now).
> > I don't see how registering gEfiSmbiosProtocolGuid could fail.
> > If you think Depex must be true, there should be
> > a) a comment stating the reasons why Depex must not be changed
> > b) I'll have to move the SMBIOS code out of BlSupportDxe into
> > BlSupportSmbiosDxe and add the Depex section there.
> >     A failed dispatch of BlSupportSmbiosDxe would then be non critical.
> >
> > Do you think this would be a better solution?
> >
> > I don't want to use RegisterProtocolNotify() as it's discouraged, isn't it?
> >
> > Kind Regards,
> > Patrick Rudolph
> >
> > On Thu, Jan 21, 2021 at 2:14 AM Ma, Maurice <maurice.ma@intel.com>
> wrote:
> > >
> > > 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.rudolph@9elements.com>
> > > > Sent: Wednesday, January 20, 2021 8:02
> > > > To: devel@edk2.groups.io
> > > > Cc: Ma, Maurice <maurice.ma@intel.com>; Dong, Guo
> > > > <guo.dong@intel.com>; You, Benjamin <benjamin.you@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.rudolph@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 (#70707): https://edk2.groups.io/g/devel/message/70707
Mute This Topic: https://groups.io/mt/79981721/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-