public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ni, Ray" <ray.ni@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
	"Ke, VincentX" <vincentx.ke@intel.com>
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: Re: [edk2-devel] [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS
Date: Wed, 31 May 2023 05:22:55 +0000	[thread overview]
Message-ID: <MN6PR11MB8244F3FA28637BFA219EA1EC8C489@MN6PR11MB8244.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230531024806.374-1-vincentx.ke@intel.com>

[-- Attachment #1: Type: text/plain, Size: 14419 bytes --]

Reviewed-by: Ray Ni <Ray.ni@intel.com>

thanks,
ray
________________________________
From: devel@edk2.groups.io <devel@edk2.groups.io> on behalf of VincentX Ke <vincentx.ke@intel.com>
Sent: Wednesday, May 31, 2023 10:48:06 AM
To: devel@edk2.groups.io <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: [edk2-devel] [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



-=-=-=-=-=-=
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#105475): https://edk2.groups.io/g/devel/message/105475
Mute This Topic: https://groups.io/mt/99234717/1712937
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
-=-=-=-=-=-=



[-- Attachment #2: Type: text/html, Size: 28338 bytes --]

  reply	other threads:[~2023-05-31  5:23 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 ` Ni, Ray [this message]
2023-05-31  5:37 ` Chiu, Chasel
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=MN6PR11MB8244F3FA28637BFA219EA1EC8C489@MN6PR11MB8244.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