[edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
VincentX Ke
vincentx.ke at intel.com
Fri Apr 28 03:46:55 UTC 2023
Dear All,
Patch v4 updated. Please let me know if there is any suggestion. Thanks
[PATCH v4] https://edk2.groups.io/g/devel/message/103738
BR,
Vincent
-----Original Message-----
From: Ke, VincentX
Sent: Friday, April 28, 2023 11:30 AM
To: Ni, Ray <ray.ni at intel.com>; devel at edk2.groups.io
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Oram, Isaac W <isaac.w.oram at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Dong, Eric <eric.dong at intel.com>
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
Hi, Ray
Based on ACPI Spec 6.5, "Hardware Signature" filed changed the description.
Here is the new description.
"The only thing that determines the hardware signature is the ACPI tables.
If any content or structure of the ACPI tables has changed, including adding or removing of tables, then the hardware signature must change."
I keep version 6.3 only for Intel code base test. It should be updated to 6.4.
Thanks for the reminder.
BR,
Vincent
-----Original Message-----
From: Ni, Ray <ray.ni at intel.com>
Sent: Friday, April 28, 2023 11:08 AM
To: devel at edk2.groups.io; Ke, VincentX <vincentx.ke at intel.com>
Cc: Chiu, Chasel <chasel.chiu at intel.com>; Desimone, Nathaniel L <nathaniel.l.desimone at intel.com>; Oram, Isaac W <isaac.w.oram at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>; Dong, Eric <eric.dong at intel.com>
Subject: RE: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature filed in FADT
Vincent,
It's an interesting patch.
The original logic is to use the HardwareSignature field to indicate any changes in adding/removing PCI devices.
Your new logic is to expand this field to indicate any changes in any tables when FADT version is > 6.3.
Why?
> -----Original Message-----
> From: devel at edk2.groups.io <devel at edk2.groups.io> On Behalf Of
> VincentX Ke
> Sent: Friday, April 28, 2023 10:57 AM
> To: devel at edk2.groups.io
> Cc: Ke, VincentX <vincentx.ke at intel.com>; Chiu, Chasel
> <chasel.chiu at intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone at intel.com>; Oram, Isaac W
> <isaac.w.oram at intel.com>; Gao, Liming <gaoliming at byosoft.com.cn>;
> Dong, Eric <eric.dong at intel.com>
> Subject: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature
> filed in FADT
>
> From: VincentX Ke <vincentx.ke at intel.com>
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=4428
>
> Calculating CRC based on checksum from all ACPI tables.
> Update HWSignature filed in FADT based on CRC while ACPI table changed.
>
> Signed-off-by: VincentX Ke <vincentx.ke at intel.com>
> Cc: Chasel Chiu <chasel.chiu at intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone at intel.com>
> Cc: Isaac Oram <isaac.w.oram at intel.com>
> Cc: Liming Gao <gaoliming at byosoft.com.cn>
> Cc: Eric Dong <eric.dong at intel.com>
> ---
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c | 110
> +++++++++++++++++++-
> Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.inf | 1 +
> 2 files changed, 110 insertions(+), 1 deletion(-)
>
> diff --git
> a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> index e967031a3b..d84c1d4f6d 100644
> --- a/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> +++ b/Platform/Intel/MinPlatformPkg/Acpi/AcpiTables/AcpiPlatform.c
> @@ -1285,6 +1285,108 @@ IsHardwareChange (
> FreePool (HWChange);
>
> }
>
>
>
> +/**
>
> + This function calculates RCR based on Checksum from all ACPI tables.
>
> + It also calculates CRC and provides as HWSignature filed in FADT table.
>
> +**/
>
> +VOID
>
> +IsAcpiTableChange (
>
> + VOID
>
> + )
>
> +{
>
> + EFI_STATUS Status;
>
> + UINTN Index;
>
> + UINTN AcpiTableCount;
>
> + UINT32 Table;
>
> + UINT32 CRC;
>
> + UINT32 *AcpiTable;
>
> + 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;
>
> + EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE *pFADT;
>
> +
>
> + AcpiTableCount = 0;
>
> + AcpiTable = NULL;
>
> + Rsdp = NULL;
>
> + Rsdt = NULL;
>
> + Xsdt = NULL;
>
> + FacsPtr = NULL;
>
> + pFADT = NULL;
>
> +
>
> + DEBUG ((DEBUG_INFO, "%a() - Start\n", __FUNCTION__));
>
> +
>
> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID
> **)&Rsdp);
>
> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {
>
> + return;
>
> + }
>
> +
>
> + //
>
> + // ACPI table count starts with 2 as RSDT and XSDT are already located.
>
> + // Then add ACPI tables found by XSDT and FADT.
>
> + //
>
> + Rsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp-
> >RsdtAddress;
>
> + Xsdt = (EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Rsdp-
> >XsdtAddress;
>
> + AcpiTableCount = AcpiTableCount + 2;
>
> + AcpiTableCount = AcpiTableCount + (Xsdt->Length - sizeof
> (EFI_ACPI_DESCRIPTION_HEADER))/sizeof (UINT64);
>
> +
>
> + for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt-
> >Length); Index = Index + sizeof (UINT64)) {
>
> + Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
>
> + if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE
> *)(UINTN)Table;
>
> + if ((pFADT->XDsdt != 0) || (pFADT->Dsdt != 0)) {
>
> + AcpiTableCount = AcpiTableCount + 1;
>
> + }
>
> + }
>
> + }
>
> +
>
> + //
>
> + // Allocate memory for founded ACPI tables.
>
> + //
>
> + AcpiTable = AllocateZeroPool (sizeof (UINT32) * AcpiTableCount);
>
> + if (AcpiTable == NULL) {
>
> + return;
>
> + }
>
> +
>
> + AcpiTableCount = 0;
>
> + AcpiTable[AcpiTableCount++] = Rsdt->Checksum;
>
> + AcpiTable[AcpiTableCount++] = Xsdt->Checksum;
>
> +
>
> + for (Index = sizeof (EFI_ACPI_DESCRIPTION_HEADER); Index < (Xsdt-
> >Length); Index = Index + sizeof (UINT64)) {
>
> + Table = *((UINT32 *)((UINT8 *)Xsdt + Index));
>
> + AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
> *)(UINTN)Table)->Checksum;
>
> + if (((EFI_ACPI_DESCRIPTION_HEADER *)(UINTN)Table)->Signature ==
> EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE_SIGNATURE) {
>
> + pFADT = (EFI_ACPI_6_5_FIXED_ACPI_DESCRIPTION_TABLE
> *)(UINTN)Table;
>
> + if (pFADT->XDsdt != 0) {
>
> + AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
> *)(UINTN)pFADT->XDsdt)->Checksum;
>
> + } else {
>
> + AcpiTable[AcpiTableCount++] = ((EFI_ACPI_DESCRIPTION_HEADER
> *)(UINTN)pFADT->Dsdt)->Checksum;
>
> + }
>
> + }
>
> + }
>
> +
>
> + //
>
> + // pFADT table not found.
>
> + //
>
> + if (pFADT == NULL) {
>
> + return;
>
> + }
>
> +
>
> + //
>
> + // Calculate CRC value.
>
> + //
>
> + Status = gBS->CalculateCrc32 (AcpiTable, AcpiTableCount, &CRC);
>
> + DEBUG ((DEBUG_INFO, "CRC = %x and Status = %r\n", CRC, Status));
>
> +
>
> + //
>
> + // Set HardwareSignature value based on CRC value.
>
> + //
>
> + FacsPtr =
> (EFI_ACPI_6_5_FIRMWARE_ACPI_CONTROL_STRUCTURE *)(UINTN)pFADT-
> >FirmwareCtrl;
>
> + FacsPtr->HardwareSignature = CRC;
>
> + FreePool (AcpiTable);
>
> + DEBUG ((DEBUG_INFO, "%a() - End\n", __FUNCTION__));
>
> +}
>
> +
>
> VOID
>
> UpdateLocalTable (
>
> VOID
>
> @@ -1329,7 +1431,13 @@ AcpiEndOfDxeEvent (
> //
>
> // Calculate Hardware Signature value based on current platform
> configurations
>
> //
>
> - IsHardwareChange ();
>
> + if ((PcdGet8 (PcdFadtMajorVersion) <=
> EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_REVISION) &&
>
> + (PcdGet8 (PcdFadtMinorVersion) <=
> EFI_ACPI_6_3_FIXED_ACPI_DESCRIPTION_TABLE_MINOR_REVISION))
>
> + {
>
> + IsHardwareChange ();
>
> + } else {
>
> + 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 (#103734):
> https://edk2.groups.io/g/devel/message/103734
> Mute This Topic: https://groups.io/mt/98551423/1712937
> Group Owner: devel+owner at edk2.groups.io
> Unsubscribe: https://edk2.groups.io/g/devel/unsub [ray.ni at intel.com]
> -=-=-=-=-=-=
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#103739): https://edk2.groups.io/g/devel/message/103739
Mute This Topic: https://groups.io/mt/98551423/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list