From: "Ma, Maurice" <maurice.ma@intel.com>
To: "Kesavan Balakrishnan,
ThiyaguX" <thiyagux.kesavan.balakrishnan@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Dong, Guo" <guo.dong@intel.com>, "Ni, Ray" <ray.ni@intel.com>,
"You, Benjamin" <benjamin.you@intel.com>,
"Liu, Zhiguang" <zhiguang.liu@intel.com>
Subject: Re: [PATCH] UefiPayloadPkg: Remove SystemTableInfo GUID.
Date: Mon, 11 Oct 2021 19:42:59 +0000 [thread overview]
Message-ID: <CO1PR11MB49458A71605CD93E728A397389B59@CO1PR11MB4945.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20211011042953.1248-1-thiyagux.kesavan.balakrishnan@intel.com>
Reviewed-by: Maurice Ma <maurice.ma@intel.com>
> -----Original Message-----
> From: Kesavan Balakrishnan, ThiyaguX
> <thiyagux.kesavan.balakrishnan@intel.com>
> Sent: Sunday, October 10, 2021 21:30
> To: devel@edk2.groups.io
> Cc: Kesavan Balakrishnan, ThiyaguX
> <thiyagux.kesavan.balakrishnan@intel.com>; Ma, Maurice
> <maurice.ma@intel.com>; Dong, Guo <guo.dong@intel.com>; Ni, Ray
> <ray.ni@intel.com>; You, Benjamin <benjamin.you@intel.com>; Liu,
> Zhiguang <zhiguang.liu@intel.com>
> Subject: [PATCH] UefiPayloadPkg: Remove SystemTableInfo GUID.
>
> SystemTableInfo GUID is not a Spec defined GUID.
> But the latest SBL uses SystemTableInfo to get ACPI and SMBIOS table
> information. So moving the SystemTableInfo GUID implementation to
> SblParseLib.
>
> Cc: Maurice Ma <maurice.ma@intel.com>
> Cc: Guo Dong <guo.dong@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Benjamin You <benjamin.you@intel.com>
> Cc: Zhiguang Liu <zhiguang.liu@intel.com>
>
> Signed-off-by: Guo Dong <guo.dong@intel.com>
> Signed-off-by: Thiyagu Kesavan Balakrishnan
> <thiyagux.kesavan.balakrishnan@intel.com>
> ---
> UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf | 1 -
> UefiPayloadPkg/Include/Library/BlParseLib.h | 2 ++
> UefiPayloadPkg/Library/CbParseLib/CbParseLib.c | 40
> +++++++++++++++++++++++++++++++---------
> UefiPayloadPkg/Library/SblParseLib/SblParseLib.c | 38
> +++++++++++++++++++++++++++++++++-----
> UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c | 22 +++++++++--
> -----------
> UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h | 1 -
> UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf | 1 -
> 7 files changed, 75 insertions(+), 30 deletions(-)
>
> diff --git a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> index 1ccb250991..96d85d2b1d 100644
> --- a/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> +++ b/UefiPayloadPkg/BlSupportDxe/BlSupportDxe.inf
> @@ -42,7 +42,6 @@
> HobLib
>
> [Guids]
> - gUefiSystemTableInfoGuid
> gUefiAcpiBoardInfoGuid
> gEfiGraphicsInfoHobGuid
>
> diff --git a/UefiPayloadPkg/Include/Library/BlParseLib.h
> b/UefiPayloadPkg/Include/Library/BlParseLib.h
> index 1244190d4e..de1da356e5 100644
> --- a/UefiPayloadPkg/Include/Library/BlParseLib.h
> +++ b/UefiPayloadPkg/Include/Library/BlParseLib.h
> @@ -12,6 +12,8 @@
> #include <Guid/SerialPortInfoGuid.h>
> #include <Guid/SystemTableInfoGuid.h>
> #include <Guid/AcpiBoardInfoGuid.h>
> +#include <UniversalPayload/AcpiTable.h> #include
> +<UniversalPayload/SmbiosTable.h>
>
> #ifndef __BOOTLOADER_PARSE_LIB__
> #define __BOOTLOADER_PARSE_LIB__
> diff --git a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
> b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
> index 4f90687e40..b260cfa04e 100644
> --- a/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
> +++ b/UefiPayloadPkg/Library/CbParseLib/CbParseLib.c
> @@ -410,9 +410,9 @@ ParseMemoryInfo (
>
>
> /**
> - Acquire acpi table and smbios table from coreboot
> + Acquire SMBIOS table from coreboot.
>
> - @param SystemTableInfo Pointer to the system table info
> + @param SmbiosTable Pointer to the SMBIOS table info.
>
> @retval RETURN_SUCCESS Successfully find out the tables.
> @retval RETURN_NOT_FOUND Failed to find the tables.
> @@ -420,8 +420,8 @@ ParseMemoryInfo (
> **/
> RETURN_STATUS
> EFIAPI
> -ParseSystemTable (
> - OUT SYSTEM_TABLE_INFO *SystemTableInfo
> +ParseSmbiosTable (
> + OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
> )
> {
> EFI_STATUS Status;
> @@ -432,17 +432,39 @@ ParseSystemTable (
> if (EFI_ERROR (Status)) {
> return EFI_NOT_FOUND;
> }
> - SystemTableInfo->SmbiosTableBase = (UINT64) (UINTN)MemTable;
> - SystemTableInfo->SmbiosTableSize = MemTableSize;
> + SmbiosTable->SmBiosEntryPoint = (UINT64) (UINTN)MemTable;
>
> + return RETURN_SUCCESS;
> +}
> +
> +
> +/**
> + Acquire ACPI table from coreboot.
> +
> + @param AcpiTableHob Pointer to the ACPI table info.
> +
> + @retval RETURN_SUCCESS Successfully find out the tables.
> + @retval RETURN_NOT_FOUND Failed to find the tables.
> +
> +**/
> +
> +RETURN_STATUS
> +EFIAPI
> +ParseAcpiTableInfo (
> + OUT UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob
> + )
> +{
> + EFI_STATUS Status;
> + VOID *MemTable;
> + UINT32 MemTableSize;
> +
> Status = ParseCbMemTable (SIGNATURE_32 ('I', 'P', 'C', 'A'), &MemTable,
> &MemTableSize);
> if (EFI_ERROR (Status)) {
> return EFI_NOT_FOUND;
> }
> - SystemTableInfo->AcpiTableBase = (UINT64) (UINTN)MemTable;
> - SystemTableInfo->AcpiTableSize = MemTableSize;
> + AcpiTableHob->Rsdp = (UINT64) (UINTN)MemTable;
>
> - return Status;
> + return RETURN_SUCCESS;
> }
>
>
> diff --git a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
> b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
> index 7214fd87d2..0f83771e6e 100644
> --- a/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
> +++ b/UefiPayloadPkg/Library/SblParseLib/SblParseLib.c
> @@ -110,9 +110,9 @@ ParseMemoryInfo (
> }
>
> /**
> - Acquire acpi table and smbios table from slim bootloader
> + Acquire SMBIOS table from slim bootloader.
>
> - @param SystemTableInfo Pointer to the system table info
> + @param SmbiosTable Pointer to the SMBIOS table info.
>
> @retval RETURN_SUCCESS Successfully find out the tables.
> @retval RETURN_NOT_FOUND Failed to find the tables.
> @@ -120,8 +120,8 @@ ParseMemoryInfo (
> **/
> RETURN_STATUS
> EFIAPI
> -ParseSystemTable (
> - OUT SYSTEM_TABLE_INFO *SystemTableInfo
> +ParseSmbiosTable (
> + OUT UNIVERSAL_PAYLOAD_SMBIOS_TABLE *SmbiosTable
> )
> {
> SYSTEM_TABLE_INFO *TableInfo;
> @@ -132,12 +132,40 @@ ParseSystemTable (
> return RETURN_NOT_FOUND;
> }
>
> - CopyMem (SystemTableInfo, TableInfo, sizeof (SYSTEM_TABLE_INFO));
> + SmbiosTable->SmBiosEntryPoint = TableInfo->SmbiosTableBase;
>
> return RETURN_SUCCESS;
> }
>
>
> +/**
> + Acquire ACPI table from slim bootloader.
> +
> + @param AcpiTableHob Pointer to the ACPI table info.
> +
> + @retval RETURN_SUCCESS Successfully find out the tables.
> + @retval RETURN_NOT_FOUND Failed to find the tables.
> +
> +**/
> +RETURN_STATUS
> +EFIAPI
> +ParseAcpiTableInfo (
> + OUT UNIVERSAL_PAYLOAD_ACPI_TABLE *AcpiTableHob
> + )
> +{
> + SYSTEM_TABLE_INFO *TableInfo;
> +
> + TableInfo = (SYSTEM_TABLE_INFO *)GetGuidHobDataFromSbl
> + (&gUefiSystemTableInfoGuid); if (TableInfo == NULL) {
> + ASSERT (FALSE);
> + return RETURN_NOT_FOUND;
> + }
> +
> + AcpiTableHob->Rsdp = TableInfo->AcpiTableBase;
> +
> + return RETURN_SUCCESS;
> +}
> +
> /**
> Find the serial port information
>
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> index f2ac3d2c69..ade81dbff5 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.c
> @@ -274,18 +274,6 @@ BuildHobFromBl (
> }
>
>
> - //
> - // Create guid hob for system tables like acpi table and smbios table
> - //
> - Status = ParseSystemTable(&SysTableInfo);
> - ASSERT_EFI_ERROR (Status);
> - if (!EFI_ERROR (Status)) {
> - NewSysTableInfo = BuildGuidHob (&gUefiSystemTableInfoGuid, sizeof
> (SYSTEM_TABLE_INFO));
> - ASSERT (NewSysTableInfo != NULL);
> - CopyMem (NewSysTableInfo, &SysTableInfo, sizeof
> (SYSTEM_TABLE_INFO));
> - DEBUG ((DEBUG_INFO, "Detected Acpi Table at 0x%lx, length 0x%x\n",
> SysTableInfo.AcpiTableBase, SysTableInfo.AcpiTableSize));
> - DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx, length 0x%x\n",
> SysTableInfo.SmbiosTableBase, SysTableInfo.SmbiosTableSize));
> - }
> //
> // Creat SmBios table Hob
> //
> @@ -295,6 +283,10 @@ BuildHobFromBl (
> SmBiosTableHob->Header.Length = sizeof
> (UNIVERSAL_PAYLOAD_SMBIOS_TABLE);
> SmBiosTableHob->SmBiosEntryPoint = SysTableInfo.SmbiosTableBase;
> DEBUG ((DEBUG_INFO, "Create smbios table
> gUniversalPayloadSmbiosTableGuid guid hob\n"));
> + Status = ParseSmbiosTable(SmBiosTableHob); if (!EFI_ERROR (Status))
> + {
> + DEBUG ((DEBUG_INFO, "Detected Smbios Table at 0x%lx\n",
> + SmBiosTableHob->SmBiosEntryPoint));
> + }
>
> //
> // Creat ACPI table Hob
> @@ -304,7 +296,11 @@ BuildHobFromBl (
> AcpiTableHob->Header.Revision =
> UNIVERSAL_PAYLOAD_ACPI_TABLE_REVISION;
> AcpiTableHob->Header.Length = sizeof
> (UNIVERSAL_PAYLOAD_ACPI_TABLE);
> AcpiTableHob->Rsdp = SysTableInfo.AcpiTableBase;
> - DEBUG ((DEBUG_INFO, "Create smbios table
> gUniversalPayloadAcpiTableGuid guid hob\n"));
> + DEBUG ((DEBUG_INFO, "Create ACPI table
> gUniversalPayloadAcpiTableGuid
> + guid hob\n")); Status = ParseAcpiTableInfo(AcpiTableHob); if
> + (!EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "Detected ACPI Table at 0x%lx\n",
> + AcpiTableHob->Rsdp)); }
>
> //
> // Create guid hob for acpi board information diff --git
> a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> index de51c2fba9..bc1ae10dcc 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UefiPayloadEntry.h
> @@ -27,7 +27,6 @@
> #include <IndustryStandard/Acpi.h>
> #include
> <IndustryStandard/MemoryMappedConfigurationSpaceAccessTable.h>
> #include <Guid/SerialPortInfoGuid.h>
> -#include <Guid/SystemTableInfoGuid.h>
> #include <Guid/MemoryMapInfoGuid.h>
> #include <Guid/AcpiBoardInfoGuid.h>
> #include <Guid/GraphicsInfoHob.h>
> diff --git a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> index 928bd2e42b..a8576305ad 100644
> --- a/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> +++ b/UefiPayloadPkg/UefiPayloadEntry/UniversalPayloadEntry.inf
> @@ -57,7 +57,6 @@
> [Guids]
> gEfiMemoryTypeInformationGuid
> gEfiFirmwareFileSystem2Guid
> - gUefiSystemTableInfoGuid
> gEfiGraphicsInfoHobGuid
> gEfiGraphicsDeviceInfoHobGuid
> gUefiAcpiBoardInfoGuid
> --
> 2.33.0.windows.2
prev parent reply other threads:[~2021-10-11 19:43 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-10-11 4:29 [PATCH] UefiPayloadPkg: Remove SystemTableInfo GUID thiyagukb
2021-10-11 19:42 ` Ma, Maurice [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB49458A71605CD93E728A397389B59@CO1PR11MB4945.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox