From: "Chiu, Chasel" <chasel.chiu@intel.com>
To: "Ke, VincentX" <vincentx.ke@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "Desimone, Nathaniel L" <nathaniel.l.desimone@intel.com>,
"Oram, Isaac W" <isaac.w.oram@intel.com>,
"Gao, Liming" <gaoliming@byosoft.com.cn>,
"Dong, Eric" <eric.dong@intel.com>,
"Sinha, Ankit" <ankit.sinha@intel.com>,
"Ni, Ray" <ray.ni@intel.com>
Subject: Re: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
Date: Wed, 31 May 2023 05:37:47 +0000 [thread overview]
Message-ID: <CH0PR11MB54750194975E0D4DC9BD6D8FE6489@CH0PR11MB5475.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230531024806.374-1-vincentx.ke@intel.com>
Patch merged:
https://github.com/tianocore/edk2-platforms/commit/1a7bd150d39007bfb72c4727feda3184c23efe96
Thanks,
Chasel
> -----Original Message-----
> From: Ke, VincentX <vincentx.ke@intel.com>
> Sent: Tuesday, May 30, 2023 7:48 PM
> To: devel@edk2.groups.io
> Cc: Ke, VincentX <vincentx.ke@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Oram, Isaac W <isaac.w.oram@intel.com>;
> Gao, Liming <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>;
> Sinha, Ankit <ankit.sinha@intel.com>
> Subject: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
>
> From: VincentX Ke <vincentx.ke@intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on each ACPI table.
> Update HWSignature field in FACS based on CRC while ACPI table changed.
>
> Change-Id: Ic0ca66ff10cda0fbcd0683020fab1bc9aea9b78c
> Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Isaac Oram <isaac.w.oram@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ankit Sinha<ankit.sinha@intel.com>
> Signed-off-by: VincentX Ke <vincentx.ke@intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 284
> +++++++++++++++-----
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 217 insertions(+), 68 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index 2f2c96f907..2a833ec99c 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,246 @@ PlatformUpdateTables (
> }
>
>
>
> /**
>
> - This function calculates RCR based on PCI Device ID and Vendor ID from the
> devices
>
> - available on the platform.
>
> - It also includes other instances of BIOS change to calculate CRC and provides
> as
>
> - HWSignature filed in FADT table.
>
> + Function prototype for GetAcpiTableCount/CalculateAcpiTableCrc.
>
> +
>
> + @param[in] Table The pointer to ACPI table.
>
> + @param[in] TableIndex The ACPI table index.
>
> + @param[in] Context The pointer to UINTN for GetAcpiTableCount.
>
> + The pointer to UINT32 array for CalculateAcpiTableCrc.
>
> **/
>
> +typedef
>
> VOID
>
> -IsHardwareChange (
>
> - VOID
>
> +(EFIAPI *ACPI_TABLE_CALLBACK)(
>
> + IN EFI_ACPI_COMMON_HEADER *Table,
>
> + IN UINTN TableIndex,
>
> + IN VOID *Context
>
> + );
>
> +
>
> +/**
>
> + Enumerate all ACPI tables in RSDT/XSDT.
>
> +
>
> + @param[in] Sdt ACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> + 4(RSDT) or 8(XSDT).
>
> + @param[in] CallbackFunction The pointer to
> GetAcpiTableCount/CalculateAcpiTableCrc.
>
> + @param[in] Context The pointer to UINTN for GetAcpiTableCount.
>
> + The pointer to UINT32 array for CalculateAcpiTableCrc.
>
> +**/
>
> +VOID
>
> +EnumerateAllAcpiTables (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTN TablePointerSize,
>
> + IN ACPI_TABLE_CALLBACK CallbackFunction,
>
> + IN VOID *Context
>
> )
>
> {
>
> - EFI_STATUS Status;
>
> - UINTN Index;
>
> - UINTN HandleCount;
>
> - EFI_HANDLE *HandleBuffer;
>
> - EFI_PCI_IO_PROTOCOL *PciIo;
>
> - UINT32 CRC;
>
> - UINT32 *HWChange;
>
> - UINTN HWChangeSize;
>
> - UINT32 PciId;
>
> - UINTN Handle;
>
> - EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> - EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
>
> -
>
> - HandleCount = 0;
>
> - HandleBuffer = NULL;
>
> -
>
> - Status = gBS->LocateHandleBuffer (
>
> - ByProtocol,
>
> - &gEfiPciIoProtocolGuid,
>
> - NULL,
>
> - &HandleCount,
>
> - &HandleBuffer
>
> - );
>
> - if (EFI_ERROR (Status)) {
>
> - return; // PciIO protocol not installed yet!
>
> + UINTN Index;
>
> + UINTN TableIndex;
>
> + UINTN EntryCount;
>
> + UINT64 EntryPtr;
>
> + UINTN BasePtr;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *FadtPtr;
>
> +
>
> + Index = 0;
>
> + TableIndex = 0;
>
> + EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> TablePointerSize;
>
> + EntryPtr = 0;
>
> + BasePtr = (UINTN)(Sdt + 1);
>
> + Table = NULL;
>
> + FadtPtr = NULL;
>
> +
>
> + if (Sdt == NULL) {
>
> + ASSERT (Sdt != NULL);
>
> + return;
>
> }
>
>
>
> - //
>
> - // Allocate memory for HWChange and add additional entrie for
>
> - // pFADT->XDsdt
>
> - //
>
> - HWChangeSize = HandleCount + 1;
>
> - HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
>
> - ASSERT(HWChange != NULL);
>
> + for (Index = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table != NULL) {
>
> + CallbackFunction (Table, TableIndex++, Context);
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + FadtPtr = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *)Table;
>
> + if (FadtPtr->Header.Revision <
> EFI_ACPI_2_0_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) {
>
> + //
>
> + // Locate FACS/DSDT in FADT
>
> + //
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr-
> >FirmwareCtrl, TableIndex++, Context);
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr->Dsdt,
> TableIndex++, Context);
>
> + } else {
>
> + //
>
> + // Locate FACS in FADT
>
> + //
>
> + if (FadtPtr->XFirmwareCtrl != 0) {
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr-
> >XFirmwareCtrl, TableIndex++, Context);
>
> + } else {
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr-
> >FirmwareCtrl, TableIndex++, Context);
>
> + }
>
> +
>
> + //
>
> + // Locate DSDT in FADT
>
> + //
>
> + if (FadtPtr->XDsdt != 0) {
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr-
> >XDsdt, TableIndex++, Context);
>
> + } else {
>
> + CallbackFunction ((EFI_ACPI_COMMON_HEADER *)(UINTN)FadtPtr->Dsdt,
> TableIndex++, Context);
>
> + }
>
> + }
>
> + }
>
> + }
>
> +}
>
> +
>
> +/**
>
> + Count the number of ACPI tables.
>
> +
>
> + @param[in] Table The pointer to ACPI table.
>
> + @param[in] TableIndex The ACPI table index.
>
> + @param[in] Context The pointer to UINTN.
>
> +**/
>
> +VOID
>
> +EFIAPI
>
> +GetAcpiTableCount (
>
> + IN EFI_ACPI_COMMON_HEADER *Table,
>
> + IN UINTN TableIndex,
>
> + IN VOID *Context
>
> + )
>
> +{
>
> + UINTN *TableCount;
>
> +
>
> + TableCount = (UINTN *)Context;
>
> +
>
> + if (Table == NULL) {
>
> + ASSERT (Table != NULL);
>
> + return;
>
> + }
>
> +
>
> + (*TableCount)++;
>
> +}
>
>
>
> - if (HWChange == NULL) return;
>
> +/**
>
> + Calculate CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The pointer to ACPI table.
>
> + @param[in] TableIndex The ACPI table index.
>
> + @param[in] Context The pointer to UINT32 array.
>
> +**/
>
> +VOID
>
> +EFIAPI
>
> +CalculateAcpiTableCrc (
>
> + IN EFI_ACPI_COMMON_HEADER *Table,
>
> + IN UINTN TableIndex,
>
> + IN VOID *Context
>
> + )
>
> +{
>
> + UINT32 *TableCrcRecord;
>
> +
>
> + TableCrcRecord = (UINT32 *)Context;
>
> +
>
> + if (Table == NULL) {
>
> + ASSERT (Table != NULL);
>
> + return;
>
> + }
>
>
>
> //
>
> - // add HWChange inputs: PCI devices
>
> + // Calculate CRC value.
>
> //
>
> - for (Index = 0; HandleCount > 0; HandleCount--) {
>
> - PciId = 0;
>
> - Status = gBS->HandleProtocol (HandleBuffer[Index], &gEfiPciIoProtocolGuid,
> (VOID **) &PciIo);
>
> - if (!EFI_ERROR (Status)) {
>
> - Status = PciIo->Pci.Read (PciIo, EfiPciIoWidthUint32, 0, 1, &PciId);
>
> - if (EFI_ERROR (Status)) {
>
> - continue;
>
> - }
>
> - HWChange[Index++] = PciId;
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> + //
>
> + // Zero HardwareSignature field before Calculating FACS CRC
>
> + //
>
> + ((EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)Table)-
> >HardwareSignature = 0;
>
> + }
>
> +
>
> + gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,
> &TableCrcRecord[TableIndex]);
>
> +}
>
> +
>
> +/**
>
> + This function calculates CRC based on each ACPI table.
>
> + It also calculates CRC and provides as HardwareSignature field in FACS.
>
> +**/
>
> +VOID
>
> +IsAcpiTableChange (
>
> + VOID
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + BOOLEAN IsRsdt;
>
> + UINTN AcpiTableCount;
>
> + UINT32 *TableCrcRecord;
>
> + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
>
> + EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> +
>
> + IsRsdt = FALSE;
>
> + AcpiTableCount = 0;
>
> + TableCrcRecord = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> + FacsPtr = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> + return;
>
> + }
>
> +
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
>
> + if (Xsdt == NULL) {
>
> + if (Rsdt != NULL) {
>
> + IsRsdt = TRUE;
>
> + } else {
>
> + return;
>
> }
>
> }
>
>
>
> + FacsPtr = (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)EfiLocateFirstAcpiTable
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE);
>
> + if (FacsPtr == NULL) {
>
> + return;
>
> + }
>
> +
>
> //
>
> - // Locate FACP Table
>
> + // Count the ACPI tables found by RSDT/XSDT and FADT.
>
> //
>
> - Handle = 0;
>
> - Status = LocateAcpiTableBySignature (
>
> - EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE,
>
> - (EFI_ACPI_DESCRIPTION_HEADER **) &pFADT,
>
> - &Handle
>
> - );
>
> - if (EFI_ERROR (Status) || (pFADT == NULL)) {
>
> - return; //Table not found or out of memory resource for pFADT table
>
> + if (IsRsdt) {
>
> + EnumerateAllAcpiTables (Rsdt, sizeof (UINT32), GetAcpiTableCount, (VOID
> *)&AcpiTableCount);
>
> + } else {
>
> + EnumerateAllAcpiTables (Xsdt, sizeof (UINT64), GetAcpiTableCount, (VOID
> *)&AcpiTableCount);
>
> }
>
>
>
> //
>
> - // add HWChange inputs: others
>
> + // Allocate memory for founded ACPI tables.
>
> //
>
> - HWChange[Index++] = (UINT32)pFADT->XDsdt;
>
> + TableCrcRecord = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
>
> + if (TableCrcRecord == NULL) {
>
> + return;
>
> + }
>
>
>
> //
>
> - // Calculate CRC value with HWChange data.
>
> + // Calculate CRC for each ACPI table and set record.
>
> //
>
> - Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC);
>
> - DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
>
> + if (IsRsdt) {
>
> + EnumerateAllAcpiTables (Rsdt, sizeof (UINT32), CalculateAcpiTableCrc, (VOID
> *)TableCrcRecord);
>
> + } else {
>
> + EnumerateAllAcpiTables (Xsdt, sizeof (UINT64), CalculateAcpiTableCrc, (VOID
> *)TableCrcRecord);
>
> + }
>
>
>
> //
>
> - // Set HardwareSignature value based on CRC value.
>
> + // Calculate and set HardwareSignature data.
>
> //
>
> - FacsPtr = (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)
> pFADT->FirmwareCtrl;
>
> - FacsPtr->HardwareSignature = CRC;
>
> - FreePool (HWChange);
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)TableCrcRecord, AcpiTableCount,
> &(FacsPtr->HardwareSignature));
>
> + DEBUG ((DEBUG_INFO, "HardwareSignature = %x and Status = %r\n", FacsPtr-
> >HardwareSignature, Status));
>
> +
>
> + FreePool (TableCrcRecord);
>
> + DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
>
> }
>
>
>
> VOID
>
> @@ -1329,7 +1477,7 @@ AcpiEndOfDxeEvent (
> //
>
> // Calculate Hardware Signature value based on current platform
> configurations
>
> //
>
> - IsHardwareChange ();
>
> + IsAcpiTableChange ();
>
> }
>
>
>
> /**
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> index 694492112b..f47cc3908d 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf
> @@ -128,6 +128,7 @@
> gEfiGlobalVariableGuid ## CONSUMES
>
> gEfiHobListGuid ## CONSUMES
>
> gEfiEndOfDxeEventGroupGuid ## CONSUMES
>
> + gEfiAcpiTableGuid ## CONSUMES
>
>
>
> [Depex]
>
> gEfiAcpiTableProtocolGuid AND
>
> --
> 2.39.2.windows.1
next prev parent reply other threads:[~2023-05-31 5:37 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-31 2:48 [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS VincentX Ke
2023-05-31 5:22 ` [edk2-devel] " Ni, Ray
2023-05-31 5:37 ` Chiu, Chasel [this message]
2023-05-31 6:02 ` VincentX Ke
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=CH0PR11MB54750194975E0D4DC9BD6D8FE6489@CH0PR11MB5475.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