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: [email protected] <[email protected]> On Behalf Of > VincentX Ke > Sent: Friday, April 28, 2023 10:57 AM > To: [email protected] > Cc: Ke, VincentX <[email protected]>; Chiu, Chasel > <[email protected]>; Desimone, Nathaniel L > <[email protected]>; Oram, Isaac W > <[email protected]>; Gao, Liming <[email protected]>; Dong, > Eric <[email protected]> > Subject: [edk2-devel] [PATCH v3] MinPlatformPkg: Update HWSignature > filed in FADT > > From: VincentX Ke <[email protected]> > > 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 <[email protected]> > Cc: Chasel Chiu <[email protected]> > Cc: Nate DeSimone <[email protected]> > Cc: Isaac Oram <[email protected]> > Cc: Liming Gao <[email protected]> > Cc: Eric Dong <[email protected]> > --- > 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: [email protected] > Unsubscribe: https://edk2.groups.io/g/devel/unsub [[email protected]] > -=-=-=-=-=-= > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#103736): https://edk2.groups.io/g/devel/message/103736 Mute This Topic: https://groups.io/mt/98551423/21656 Group Owner: [email protected] Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [[email protected]] -=-=-=-=-=-=-=-=-=-=-=-
