* [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS
@ 2023-05-10 9:58 VincentX Ke
2023-05-11 3:42 ` [edk2-devel] " Ni, Ray
0 siblings, 1 reply; 3+ messages in thread
From: VincentX Ke @ 2023-05-10 9:58 UTC (permalink / raw)
To: devel
Cc: VincentX Ke, Chasel Chiu, Nate DeSimone, Isaac Oram, Liming Gao,
Eric Dong, Ankit Sinha
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 filed 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 | 271 +++++++++++++++-----
Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
2 files changed, 209 insertions(+), 63 deletions(-)
diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
index e967031a3b..bb0f4a1f04 100644
--- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
+++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
@@ -1191,98 +1191,243 @@ 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.
+ This function calculates CRC based on each offset in the ACPI table.
+
+ @param[in] Table The ACPI table required to calculate CRC.
+
+ @retval CRC A pointer to allocate UINT32 that
+ contains the CRC32 data.
+**/
+UINT32
+AcpiTableCrcCalculator (
+ IN EFI_ACPI_COMMON_HEADER *Table
+ )
+{
+ EFI_STATUS Status;
+ UINT32 CRC;
+
+ Status = EFI_SUCCESS;
+ CRC = 0;
+
+ //
+ // Calculate CRC value.
+ //
+ 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;
+ }
+
+ Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length, &CRC);
+ return CRC;
+}
+
+/**
+ This function count ACPI tables in RSDT/XSDT and return the result.
+
+ @param[in] Sdt ACPI XSDT/RSDT.
+ @param[in] TablePointerSize Size of table pointer:
+ 4(RSDT) or 8(XSDT).
+
+ @retval TableCount The total number of ACPI tables in
+ RSDT or XSDT.
+**/
+UINTN
+CountTableInSDT (
+ IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
+ IN UINTN TablePointerSize
+ )
+{
+ UINTN Index;
+ UINTN TableCount;
+ UINTN EntryCount;
+ UINT64 EntryPtr;
+ UINTN BasePtr;
+ EFI_ACPI_COMMON_HEADER *Table;
+
+ EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / TablePointerSize;
+ BasePtr = (UINTN)(Sdt + 1);
+
+ for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
+ EntryPtr = 0;
+ Table = NULL;
+ CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize), TablePointerSize);
+ Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+ if (Table) {
+ TableCount++;
+ }
+
+ if (Table->Signature == EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
+ if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) {
+ TableCount++;
+ }
+
+ if (Fadt.Dsdt || Fadt.XDsdt) {
+ TableCount++;
+ }
+ }
+ }
+
+ return TableCount;
+}
+
+/**
+ This function calculates CRC based on each ACPI table.
+ It also calculates CRC and provides as HWSignature filed in FACS.
**/
VOID
-IsHardwareChange (
+IsAcpiTableChange (
VOID
)
{
- 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_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
- EFI_ACPI_6_3_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!
+ EFI_STATUS Status;
+ UINTN Index;
+ UINTN AcpiTableCount;
+ UINTN EntryCount;
+ UINTN BasePtr;
+ UINT64 EntryPtr;
+ UINT32 *TableCrcRecord;
+ UINT32 HWSignature;
+ EFI_ACPI_COMMON_HEADER *Table;
+ EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
+ EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
+ EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
+
+ Index = 0;
+ AcpiTableCount = 0;
+ EntryCount = 0;
+ BasePtr = 0;
+ EntryPtr = 0;
+ HWSignature = 0;
+ TableCrcRecord = NULL;
+ Rsdp = NULL;
+ Rsdt = NULL;
+ Xsdt = NULL;
+
+ DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
+
+ Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID **)&Rsdp);
+ if (EFI_ERROR (Status) || (Rsdp == NULL)) {
+ return;
}
//
- // Allocate memory for HWChange and add additional entrie for
- // pFADT->XDsdt
+ // ACPI table count starts with 0.
+ // Then add ACPI tables found by RSDT/XSDT and FADT.
//
- HWChangeSize = HandleCount + 1;
- HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
- ASSERT(HWChange != NULL);
+ Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
+ Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
- if (HWChange == NULL) return;
+ AcpiTableCount += CountTableInSDT (Rsdt, sizeof (UINT32));
+ AcpiTableCount += CountTableInSDT (Xsdt, sizeof (UINT64));
//
- // add HWChange inputs: PCI devices
+ // Allocate memory for founded ACPI tables.
+ //
+ TableCrcRecord = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
+ if (TableCrcRecord == NULL) {
+ return;
+ }
+
//
- 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;
+ // Search RSDT
+ //
+ AcpiTableCount = 0;
+ EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof (UINT32);
+ BasePtr = (UINTN)(Rsdt + 1);
+ for (Index = 0; Index < EntryCount; Index++) {
+ EntryPtr = 0;
+ Table = NULL;
+ CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof (UINT32)), sizeof (UINT32));
+ Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+ if (Table) {
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator (Table);
+ }
+
+ if (Table->Signature == EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
+ //
+ // Locate FACS in FADT
+ //
+ if (Fadt.FirmwareCtrl) {
+ CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl, sizeof (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
+ }
+
+ //
+ // Locate DSDT in FADT
+ //
+ if (Fadt.Dsdt) {
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
}
- HWChange[Index++] = PciId;
}
}
//
- // Locate FACP Table
+ // Search XSDT
//
- Handle = 0;
- Status = LocateAcpiTableBySignature (
- EFI_ACPI_6_3_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
+ EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) / sizeof (UINT64);
+ BasePtr = (UINTN)(Xsdt + 1);
+ for (Index = 0; Index < EntryCount; Index++) {
+ EntryPtr = 0;
+ Table = NULL;
+ CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof (UINT64)), sizeof (UINT64));
+ Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
+ if (Table) {
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator (Table);
+ }
+
+ if (Table->Signature == EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
+ //
+ // Locate FACS in FADT
+ //
+ CopyMem ((VOID *)&EntryPtr, &Fadt.XFirmwareCtrl, sizeof (UINT64));
+ if (EntryPtr != 0) {
+ CopyMem ((VOID *)&Facs, (VOID *)EntryPtr, sizeof (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
+ } else {
+ CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl, sizeof (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
+ }
+
+ //
+ // Locate DSDT in FADT
+ //
+ CopyMem ((VOID *)&EntryPtr, &Fadt.XDsdt, sizeof (UINT64));
+ if (EntryPtr != 0) {
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
+ } else {
+ TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
+ }
+ }
}
//
- // add HWChange inputs: others
+ // FADT/FACS not found
//
- HWChange[Index++] = (UINT32)pFADT->XDsdt;
+ if (Fadt.Header.Signature != EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
+ return;
+ }
+
+ if (Facs.Signature != EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
+ return;
+ }
//
- // Calculate CRC value with HWChange data.
+ // Calculate HWSignature data.
//
- Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC);
- DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
+ Status = gBS->CalculateCrc32 (TableCrcRecord, AcpiTableCount, &HWSignature);
+ DEBUG ((DEBUG_INFO, "HardwareSignature = %x and Status = %r\n", HWSignature, Status));
//
// Set HardwareSignature value based on CRC value.
//
- FacsPtr = (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)pFADT->FirmwareCtrl;
- FacsPtr->HardwareSignature = CRC;
- FreePool (HWChange);
+ Facs.HardwareSignature = HWSignature;
+ FreePool (TableCrcRecord);
+ DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
}
VOID
@@ -1329,7 +1474,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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS
2023-05-10 9:58 [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS VincentX Ke
@ 2023-05-11 3:42 ` Ni, Ray
2023-05-11 10:23 ` VincentX Ke
0 siblings, 1 reply; 3+ messages in thread
From: Ni, Ray @ 2023-05-11 3:42 UTC (permalink / raw)
To: devel@edk2.groups.io, Ke, VincentX
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Sinha, Ankit
Vincent,
ACPI spec contains following:
The XSDT provides identical functionality to the RSDT but accommodates physical addresses of DESCRIPTION
HEADERs that are larger than 32 bits. Notice that both the XSDT and the RSDT can be pointed to by the RSDP
structure. An ACPI-compatible OS must use the XSDT if present.
Which means, XSDT is used over RSDT when it exists.
But your code measures both XSDT and RSDT. It's not right.
This change is not a simple change. I suggest you review the ACPI spec in detail and close the opens within your team
before sending another version of patches.
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of VincentX Ke
> Sent: Wednesday, May 10, 2023 5:58 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: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed 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 filed 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 | 271
> +++++++++++++++-----
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 209 insertions(+), 63 deletions(-)
>
> diff --git a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..bb0f4a1f04 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,243 @@ 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.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The ACPI table required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> + contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN EFI_ACPI_COMMON_HEADER *Table
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 CRC;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + CRC = 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + 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;
>
> + }
>
> +
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length, &CRC);
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function count ACPI tables in RSDT/XSDT and return the result.
>
> +
>
> + @param[in] Sdt ACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> + 4(RSDT) or 8(XSDT).
>
> +
>
> + @retval TableCount The total number of ACPI tables in
>
> + RSDT or XSDT.
>
> +**/
>
> +UINTN
>
> +CountTableInSDT (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTN TablePointerSize
>
> + )
>
> +{
>
> + UINTN Index;
>
> + UINTN TableCount;
>
> + UINTN EntryCount;
>
> + UINT64 EntryPtr;
>
> + UINTN BasePtr;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> +
>
> + EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> TablePointerSize;
>
> + BasePtr = (UINTN)(Sdt + 1);
>
> +
>
> + for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCount++;
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) {
>
> + TableCount++;
>
> + }
>
> +
>
> + if (Fadt.Dsdt || Fadt.XDsdt) {
>
> + TableCount++;
>
> + }
>
> + }
>
> + }
>
> +
>
> + return TableCount;
>
> +}
>
> +
>
> +/**
>
> + This function calculates CRC based on each ACPI table.
>
> + It also calculates CRC and provides as HWSignature filed in FACS.
>
> **/
>
> VOID
>
> -IsHardwareChange (
>
> +IsAcpiTableChange (
>
> VOID
>
> )
>
> {
>
> - 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_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> - EFI_ACPI_6_3_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!
>
> + EFI_STATUS Status;
>
> + UINTN Index;
>
> + UINTN AcpiTableCount;
>
> + UINTN EntryCount;
>
> + UINTN BasePtr;
>
> + UINT64 EntryPtr;
>
> + UINT32 *TableCrcRecord;
>
> + UINT32 HWSignature;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
>
> +
>
> + Index = 0;
>
> + AcpiTableCount = 0;
>
> + EntryCount = 0;
>
> + BasePtr = 0;
>
> + EntryPtr = 0;
>
> + HWSignature = 0;
>
> + TableCrcRecord = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> + return;
>
> }
>
>
>
> //
>
> - // Allocate memory for HWChange and add additional entrie for
>
> - // pFADT->XDsdt
>
> + // ACPI table count starts with 0.
>
> + // Then add ACPI tables found by RSDT/XSDT and FADT.
>
> //
>
> - HWChangeSize = HandleCount + 1;
>
> - HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
>
> - ASSERT(HWChange != NULL);
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
>
>
>
> - if (HWChange == NULL) return;
>
> + AcpiTableCount += CountTableInSDT (Rsdt, sizeof (UINT32));
>
> + AcpiTableCount += CountTableInSDT (Xsdt, sizeof (UINT64));
>
>
>
> //
>
> - // add HWChange inputs: PCI devices
>
> + // Allocate memory for founded ACPI tables.
>
> + //
>
> + TableCrcRecord = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
>
> + if (TableCrcRecord == NULL) {
>
> + return;
>
> + }
>
> +
>
> //
>
> - 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;
>
> + // Search RSDT
>
> + //
>
> + AcpiTableCount = 0;
>
> + EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> sizeof (UINT32);
>
> + BasePtr = (UINTN)(Rsdt + 1);
>
> + for (Index = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof (UINT32)),
> sizeof (UINT32));
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator (Table);
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + //
>
> + // Locate FACS in FADT
>
> + //
>
> + if (Fadt.FirmwareCtrl) {
>
> + CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl, sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
>
> + }
>
> +
>
> + //
>
> + // Locate DSDT in FADT
>
> + //
>
> + if (Fadt.Dsdt) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
>
> }
>
> - HWChange[Index++] = PciId;
>
> }
>
> }
>
>
>
> //
>
> - // Locate FACP Table
>
> + // Search XSDT
>
> //
>
> - Handle = 0;
>
> - Status = LocateAcpiTableBySignature (
>
> - EFI_ACPI_6_3_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
>
> + EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> sizeof (UINT64);
>
> + BasePtr = (UINTN)(Xsdt + 1);
>
> + for (Index = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof (UINT64)),
> sizeof (UINT64));
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator (Table);
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + //
>
> + // Locate FACS in FADT
>
> + //
>
> + CopyMem ((VOID *)&EntryPtr, &Fadt.XFirmwareCtrl, sizeof (UINT64));
>
> + if (EntryPtr != 0) {
>
> + CopyMem ((VOID *)&Facs, (VOID *)EntryPtr, sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
>
> + } else {
>
> + CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl, sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
>
> + }
>
> +
>
> + //
>
> + // Locate DSDT in FADT
>
> + //
>
> + CopyMem ((VOID *)&EntryPtr, &Fadt.XDsdt, sizeof (UINT64));
>
> + if (EntryPtr != 0) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
>
> + } else {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
>
> + }
>
> + }
>
> }
>
>
>
> //
>
> - // add HWChange inputs: others
>
> + // FADT/FACS not found
>
> //
>
> - HWChange[Index++] = (UINT32)pFADT->XDsdt;
>
> + if (Fadt.Header.Signature !=
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + return;
>
> + }
>
> +
>
> + if (Facs.Signature !=
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> + return;
>
> + }
>
>
>
> //
>
> - // Calculate CRC value with HWChange data.
>
> + // Calculate HWSignature data.
>
> //
>
> - Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC);
>
> - DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
>
> + Status = gBS->CalculateCrc32 (TableCrcRecord, AcpiTableCount,
> &HWSignature);
>
> + DEBUG ((DEBUG_INFO, "HardwareSignature = %x and Status = %r\n",
> HWSignature, Status));
>
>
>
> //
>
> // Set HardwareSignature value based on CRC value.
>
> //
>
> - FacsPtr = (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)pFADT->FirmwareCtrl;
>
> - FacsPtr->HardwareSignature = CRC;
>
> - FreePool (HWChange);
>
> + Facs.HardwareSignature = HWSignature;
>
> + FreePool (TableCrcRecord);
>
> + DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
>
> }
>
>
>
> VOID
>
> @@ -1329,7 +1474,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 (#104537): https://edk2.groups.io/g/devel/message/104537
> Mute This Topic: https://groups.io/mt/98802410/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS
2023-05-11 3:42 ` [edk2-devel] " Ni, Ray
@ 2023-05-11 10:23 ` VincentX Ke
0 siblings, 0 replies; 3+ messages in thread
From: VincentX Ke @ 2023-05-11 10:23 UTC (permalink / raw)
To: Ni, Ray, devel@edk2.groups.io
Cc: Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming,
Dong, Eric, Sinha, Ankit
Hi, Ray
Thanks for the correction and those suggestion in previous patches.
As your suggestion, the best way is keep following the logic of ACPI spec definition.
[Patch V7] updated with https://edk2.groups.io/g/devel/message/104685
I apologize for those flaws in previous patches.
Sincerely,
Vincent
-----Original Message-----
From: Ni, Ray <ray.ni@intel.com>
Sent: Thursday, May 11, 2023 11:42 AM
To: devel@edk2.groups.io; Ke, VincentX <vincentx.ke@intel.com>
Cc: 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 v6] MinPlatformPkg: Update HWSignature filed in FACS
Vincent,
ACPI spec contains following:
The XSDT provides identical functionality to the RSDT but accommodates physical addresses of DESCRIPTION
HEADERs that are larger than 32 bits. Notice that both the XSDT and the RSDT can be pointed to by the RSDP
structure. An ACPI-compatible OS must use the XSDT if present.
Which means, XSDT is used over RSDT when it exists.
But your code measures both XSDT and RSDT. It's not right.
This change is not a simple change. I suggest you review the ACPI spec in detail and close the opens within your team before sending another version of patches.
Thanks,
Ray
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of
> VincentX Ke
> Sent: Wednesday, May 10, 2023 5:58 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: [edk2-devel] [PATCH v6] MinPlatformPkg: Update HWSignature
> filed 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 filed 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 | 271
> +++++++++++++++-----
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 209 insertions(+), 63 deletions(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..bb0f4a1f04 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1191,98 +1191,243 @@ 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.
>
> + This function calculates CRC based on each offset in the ACPI table.
>
> +
>
> + @param[in] Table The ACPI table required to calculate CRC.
>
> +
>
> + @retval CRC A pointer to allocate UINT32 that
>
> + contains the CRC32 data.
>
> +**/
>
> +UINT32
>
> +AcpiTableCrcCalculator (
>
> + IN EFI_ACPI_COMMON_HEADER *Table
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINT32 CRC;
>
> +
>
> + Status = EFI_SUCCESS;
>
> + CRC = 0;
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + 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;
>
> + }
>
> +
>
> + Status = gBS->CalculateCrc32 ((UINT8 *)Table, (UINTN)Table->Length,
> + &CRC);
>
> + return CRC;
>
> +}
>
> +
>
> +/**
>
> + This function count ACPI tables in RSDT/XSDT and return the result.
>
> +
>
> + @param[in] Sdt ACPI XSDT/RSDT.
>
> + @param[in] TablePointerSize Size of table pointer:
>
> + 4(RSDT) or 8(XSDT).
>
> +
>
> + @retval TableCount The total number of ACPI tables in
>
> + RSDT or XSDT.
>
> +**/
>
> +UINTN
>
> +CountTableInSDT (
>
> + IN EFI_ACPI_DESCRIPTION_HEADER *Sdt,
>
> + IN UINTN TablePointerSize
>
> + )
>
> +{
>
> + UINTN Index;
>
> + UINTN TableCount;
>
> + UINTN EntryCount;
>
> + UINT64 EntryPtr;
>
> + UINTN BasePtr;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> +
>
> + EntryCount = (Sdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> TablePointerSize;
>
> + BasePtr = (UINTN)(Sdt + 1);
>
> +
>
> + for (Index = 0, TableCount = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem (&EntryPtr, (VOID *)(BasePtr + Index * TablePointerSize),
> TablePointerSize);
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCount++;
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + if (Fadt.FirmwareCtrl || Fadt.XFirmwareCtrl) {
>
> + TableCount++;
>
> + }
>
> +
>
> + if (Fadt.Dsdt || Fadt.XDsdt) {
>
> + TableCount++;
>
> + }
>
> + }
>
> + }
>
> +
>
> + return TableCount;
>
> +}
>
> +
>
> +/**
>
> + This function calculates CRC based on each ACPI table.
>
> + It also calculates CRC and provides as HWSignature filed in FACS.
>
> **/
>
> VOID
>
> -IsHardwareChange (
>
> +IsAcpiTableChange (
>
> VOID
>
> )
>
> {
>
> - 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_3_FIRMWARE_ACPI_CONTROL_STRUCTURE *FacsPtr;
>
> - EFI_ACPI_6_3_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!
>
> + EFI_STATUS Status;
>
> + UINTN Index;
>
> + UINTN AcpiTableCount;
>
> + UINTN EntryCount;
>
> + UINTN BasePtr;
>
> + UINT64 EntryPtr;
>
> + UINT32 *TableCrcRecord;
>
> + UINT32 HWSignature;
>
> + EFI_ACPI_COMMON_HEADER *Table;
>
> + EFI_ACPI_6_5_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Rsdt;
>
> + EFI_ACPI_DESCRIPTION_HEADER *Xsdt;
>
> +
>
> + Index = 0;
>
> + AcpiTableCount = 0;
>
> + EntryCount = 0;
>
> + BasePtr = 0;
>
> + EntryPtr = 0;
>
> + HWSignature = 0;
>
> + TableCrcRecord = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> + return;
>
> }
>
>
>
> //
>
> - // Allocate memory for HWChange and add additional entrie for
>
> - // pFADT->XDsdt
>
> + // ACPI table count starts with 0.
>
> + // Then add ACPI tables found by RSDT/XSDT and FADT.
>
> //
>
> - HWChangeSize = HandleCount + 1;
>
> - HWChange = AllocateZeroPool (sizeof(UINT32) * HWChangeSize);
>
> - ASSERT(HWChange != NULL);
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp->XsdtAddress;
>
>
>
> - if (HWChange == NULL) return;
>
> + AcpiTableCount += CountTableInSDT (Rsdt, sizeof (UINT32));
>
> + AcpiTableCount += CountTableInSDT (Xsdt, sizeof (UINT64));
>
>
>
> //
>
> - // add HWChange inputs: PCI devices
>
> + // Allocate memory for founded ACPI tables.
>
> + //
>
> + TableCrcRecord = AllocateZeroPool (sizeof (UINT32) *
> + AcpiTableCount);
>
> + if (TableCrcRecord == NULL) {
>
> + return;
>
> + }
>
> +
>
> //
>
> - 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;
>
> + // Search RSDT
>
> + //
>
> + AcpiTableCount = 0;
>
> + EntryCount = (Rsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER)) /
> sizeof (UINT32);
>
> + BasePtr = (UINTN)(Rsdt + 1);
>
> + for (Index = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof
> + (UINT32)),
> sizeof (UINT32));
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> + (Table);
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + //
>
> + // Locate FACS in FADT
>
> + //
>
> + if (Fadt.FirmwareCtrl) {
>
> + CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl,
> + sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
>
> + }
>
> +
>
> + //
>
> + // Locate DSDT in FADT
>
> + //
>
> + if (Fadt.Dsdt) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
>
> }
>
> - HWChange[Index++] = PciId;
>
> }
>
> }
>
>
>
> //
>
> - // Locate FACP Table
>
> + // Search XSDT
>
> //
>
> - Handle = 0;
>
> - Status = LocateAcpiTableBySignature (
>
> - EFI_ACPI_6_3_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
>
> + EntryCount = (Xsdt->Length - sizeof (EFI_ACPI_DESCRIPTION_HEADER))
> + /
> sizeof (UINT64);
>
> + BasePtr = (UINTN)(Xsdt + 1);
>
> + for (Index = 0; Index < EntryCount; Index++) {
>
> + EntryPtr = 0;
>
> + Table = NULL;
>
> + CopyMem ((VOID *)&EntryPtr, (VOID *)(BasePtr + Index * sizeof
> + (UINT64)),
> sizeof (UINT64));
>
> + Table = (EFI_ACPI_COMMON_HEADER *)((UINTN)(EntryPtr));
>
> + if (Table) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> + (Table);
>
> + }
>
> +
>
> + if (Table->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + CopyMem ((VOID *)&Fadt, (VOID *)Table, sizeof
> (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE));
>
> + //
>
> + // Locate FACS in FADT
>
> + //
>
> + CopyMem ((VOID *)&EntryPtr, &Fadt.XFirmwareCtrl, sizeof
> + (UINT64));
>
> + if (EntryPtr != 0) {
>
> + CopyMem ((VOID *)&Facs, (VOID *)EntryPtr, sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
>
> + } else {
>
> + CopyMem ((VOID *)&Facs, (VOID *)(UINTN)Fadt.FirmwareCtrl,
> + sizeof
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE));
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.FirmwareCtrl);
>
> + }
>
> +
>
> + //
>
> + // Locate DSDT in FADT
>
> + //
>
> + CopyMem ((VOID *)&EntryPtr, &Fadt.XDsdt, sizeof (UINT64));
>
> + if (EntryPtr != 0) {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)EntryPtr);
>
> + } else {
>
> + TableCrcRecord[AcpiTableCount++] = AcpiTableCrcCalculator
> ((EFI_ACPI_COMMON_HEADER *)(UINTN)Fadt.Dsdt);
>
> + }
>
> + }
>
> }
>
>
>
> //
>
> - // add HWChange inputs: others
>
> + // FADT/FACS not found
>
> //
>
> - HWChange[Index++] = (UINT32)pFADT->XDsdt;
>
> + if (Fadt.Header.Signature !=
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + return;
>
> + }
>
> +
>
> + if (Facs.Signature !=
> EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE_SIGNATURE) {
>
> + return;
>
> + }
>
>
>
> //
>
> - // Calculate CRC value with HWChange data.
>
> + // Calculate HWSignature data.
>
> //
>
> - Status = gBS->CalculateCrc32(HWChange, HWChangeSize, &CRC);
>
> - DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
>
> + Status = gBS->CalculateCrc32 (TableCrcRecord, AcpiTableCount,
> &HWSignature);
>
> + DEBUG ((DEBUG_INFO, "HardwareSignature = %x and Status = %r\n",
> HWSignature, Status));
>
>
>
> //
>
> // Set HardwareSignature value based on CRC value.
>
> //
>
> - FacsPtr = (EFI_ACPI_6_3_FIRMWARE_ACPI_CONTROL_STRUCTURE
> *)(UINTN)pFADT->FirmwareCtrl;
>
> - FacsPtr->HardwareSignature = CRC;
>
> - FreePool (HWChange);
>
> + Facs.HardwareSignature = HWSignature;
>
> + FreePool (TableCrcRecord);
>
> + DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
>
> }
>
>
>
> VOID
>
> @@ -1329,7 +1474,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 (#104537):
> https://edk2.groups.io/g/devel/message/104537
> Mute This Topic: https://groups.io/mt/98802410/1712937
> Group Owner: devel+owner@edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni@intel.com]
> -=-=-=-=-=-=
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-11 10:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 9:58 [PATCH v6] MinPlatformPkg: Update HWSignature filed in FACS VincentX Ke
2023-05-11 3:42 ` [edk2-devel] " Ni, Ray
2023-05-11 10:23 ` VincentX Ke
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox