* [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS @ 2023-05-31 2:48 VincentX Ke 2023-05-31 5:22 ` [edk2-devel] " Ni, Ray 2023-05-31 5:37 ` Chiu, Chasel 0 siblings, 2 replies; 4+ messages in thread From: VincentX Ke @ 2023-05-31 2:48 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 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS 2023-05-31 2:48 [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS VincentX Ke @ 2023-05-31 5:22 ` Ni, Ray 2023-05-31 5:37 ` Chiu, Chasel 1 sibling, 0 replies; 4+ messages in thread From: Ni, Ray @ 2023-05-31 5:22 UTC (permalink / raw) To: devel@edk2.groups.io, Ke, VincentX Cc: Ke, VincentX, Chiu, Chasel, Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming, Dong, Eric, Sinha, Ankit [-- 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 --] ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS 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 2023-05-31 6:02 ` VincentX Ke 1 sibling, 1 reply; 4+ messages in thread From: Chiu, Chasel @ 2023-05-31 5:37 UTC (permalink / raw) To: Ke, VincentX, devel@edk2.groups.io Cc: Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming, Dong, Eric, Sinha, Ankit, Ni, Ray 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH v10] MinPlatformPkg: Update HWSignature field in FACS 2023-05-31 5:37 ` Chiu, Chasel @ 2023-05-31 6:02 ` VincentX Ke 0 siblings, 0 replies; 4+ messages in thread From: VincentX Ke @ 2023-05-31 6:02 UTC (permalink / raw) To: Chiu, Chasel, devel@edk2.groups.io Cc: Desimone, Nathaniel L, Oram, Isaac W, Gao, Liming, Dong, Eric, Sinha, Ankit, Ni, Ray Thanks for review and merge. -----Original Message----- From: Chiu, Chasel <chasel.chiu@intel.com> Sent: Wednesday, May 31, 2023 1:38 PM To: Ke, VincentX <vincentx.ke@intel.com>; 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 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2023-05-31 6:03 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 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 2023-05-31 6:02 ` VincentX Ke
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox