Thank you Chasel, I've updated the GUID to all capitals to match.

@Desimone, Nathaniel L could you review the patch as well, I would like to get this in by Friday if possible.

Thank you,

Zack

-----Original Message-----
From: Chiu, Chasel <chasel.chiu@intel.com>
Sent: Wednesday, February 7, 2024 7:29 PM
To: Clark-williams, Zachary <zachary.clark-williams@intel.com>; devel@edk2.groups.io
Cc: Desimone, Nathaniel L <nathaniel.l.desimone@intel.com>; Liming Gao <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>
Subject: RE: [PATCH] MinPlatformPkg/PhatAcpiLib: adding Phat library

Hi Zack,

Change looks good, just one minor thing that we may correct DxePhatAcpiLib.inf FILE_GUID format during merging this patch.

Reviewed-by: Chasel Chiu <chasel.chiu@intel.com>

Thanks,

Chasel

> -----Original Message-----

> From: Clark-williams, Zachary <zachary.clark-williams@intel.com>

> Sent: Tuesday, February 6, 2024 3:32 PM

> To: devel@edk2.groups.io

> Cc: Clark-williams, Zachary <zachary.clark-williams@intel.com>; Chiu,

> Chasel <chasel.chiu@intel.com>; Desimone, Nathaniel L

> <nathaniel.l.desimone@intel.com>; Liming Gao

> <gaoliming@byosoft.com.cn>; Dong, Eric <eric.dong@intel.com>

> Subject: [PATCH] MinPlatformPkg/PhatAcpiLib: adding Phat library

> From: Zachary clark-Williams <zachary.clark-williams@intel.com>

> Adding a Platform Health Assessment Table (PHAT) ACPI library for

> general feature interface with table. MemoryHealthInsights, FSPv and

> Chasmfalls all use this table and will migrate to utilize the library for accessing and appending the PHAT.

> Change-Id: I715fa543e81403ee8e159c2c2afd8b8709234a11

> Hsd-es-id: 14019467886

> Cc: Chasel Chiu <chasel.chiu@intel.com>

> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>

> Cc: Liming Gao <gaoliming@byosoft.com.cn>

> Cc: Eric Dong <eric.dong@intel.com>

> Signed-off-by: Zachary Clark-Williams

> <zachary.clark-williams@intel.com>

> ---

> .../Acpi/Library/PhatAcpiLib/DxePhatAcpiLib.c | 292 ++++++++++++++++++

> .../Library/PhatAcpiLib/DxePhatAcpiLib.inf | 45 +++

> .../Include/Library/PhatAcpiLib.h | 40 +++

> .../Intel/MinPlatformPkg/MinPlatformPkg.dec | 2 +

> .../Intel/MinPlatformPkg/MinPlatformPkg.dsc | 3 +

> 5 files changed, 382 insertions(+)

> create mode 100644

> Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLib.

> c

> create mode 100644

> Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLib.

> inf

> create mode 100644

> Platform/Intel/MinPlatformPkg/Include/Library/PhatAcpiLib.h

> diff --git

> a/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLi

> b.c

> b/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLi

> b.c

> new file mode 100644

> index 000000000..925f8484b

> --- /dev/null

> +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAc

> +++ pi

> +++ Lib.c

> @@ -0,0 +1,292 @@

> +/** @file

> + Dxe Platform Health Assessment Table Library

> +

> + SPDX-License-Identifier: BSD-2-Clause-Patent **/

> +

> +#include <Library/PhatAcpiLib.h>

> +

> +#include <PiDxe.h>

> +#include <Base.h>

> +#include <Library/UefiLib.h>

> +#include <Library/DebugLib.h>

> +#include <Library/BaseMemoryLib.h>

> +#include <Library/MemoryAllocationLib.h> #include

> +<Library/UefiBootServicesTableLib.h>

> +

> +#include <Protocol/AcpiTable.h>

> +#include <Protocol/AcpiSystemDescriptionTable.h>

> +

> +GLOBAL_REMOVE_IF_UNREFERENCED EFI_ACPI_TABLE_PROTOCOL

> +*mAcpiTableProtocol = NULL;

> +

> +/**

> + Initialize the header of the Platform Health Assessment Table.

> +

> + @param[out] Header The header of the ACPI Table.

> + @param[in] OemId The OEM ID.

> + @param[in] OemTableId The OEM table ID for the Phat.

> +**/

> +VOID

> +InitPhatTableHeader (

> + OUT EFI_ACPI_DESCRIPTION_HEADER *Header,

> + IN UINT8 *OemId,

> + IN UINT64 *OemTableId

> + )

> +{

> + ZeroMem (Header, sizeof (EFI_ACPI_DESCRIPTION_HEADER));

> +

> + Header->Signature =

> +EFI_ACPI_6_5_PLATFORM_HEALTH_ASSESSMENT_TABLE_SIGNATURE;

> + //

> + // total length (FVI, Driver Health).

> + //

> + Header->Length = 0;

> + Header->Revision =

> EFI_ACPI_6_5_PLATFORM_HEALTH_ASSESSMENT_TABLE_REVISION;

> + Header->Checksum = 0;

> + CopyMem (Header->OemId, OemId, sizeof (Header->OemId)); CopyMem

> + (&Header->OemTableId, OemTableId, sizeof (UINT64));

> + Header->OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision);

> + Header->CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId);

> + Header->CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision);

> + }

> +

> +/**

> + This function scan ACPI table entry point.

> +

> + @retval ACPI table entry pointer

> +**/

> +VOID *

> +SearchAcpiTablePointer (

> + VOID

> + )

> +{

> + EFI_ACPI_3_0_ROOT_SYSTEM_DESCRIPTION_POINTER *Rsdp;

> + EFI_ACPI_DESCRIPTION_HEADER *Entry;

> + EFI_STATUS Status;

> +

> + Entry = NULL;

> +

> + Status = gBS->LocateProtocol (&gEfiAcpiTableProtocolGuid, NULL,

> + (VOID

> + **) &mAcpiTableProtocol); if (EFI_ERROR (Status)) {

> + DEBUG ((DEBUG_INFO, "[%a] Locate gEfiAcpiTableProtocolGuid failed

> + with

> status: [%r].\n", __FUNCTION__, Status));

> + return NULL;

> + }

> +

> + //

> + // Find ACPI table RSD_PTR from the system table.

> + //

> + Status = EfiGetSystemConfigurationTable (&gEfiAcpiTableGuid, (VOID

> + **) &Rsdp); if (EFI_ERROR (Status)) {

> + Status = EfiGetSystemConfigurationTable (&gEfiAcpi10TableGuid,

> + (VOID **) &Rsdp); }

> +

> + if (EFI_ERROR (Status) || (Rsdp == NULL)) {

> + DEBUG ((DEBUG_INFO, "[%a] Can't find RSD_PTR from system table!

> + \n",

> __FUNCTION__));

> + return NULL;

> + } else if (Rsdp->Revision >=

> EFI_ACPI_2_0_ROOT_SYSTEM_DESCRIPTION_POINTER_REVISION && Rsdp-

> >XsdtAddress != 0) {

> + Entry = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN)

> + Rsdp->XsdtAddress; } else if (Rsdp->RsdtAddress != 0) {

> + Entry = (EFI_ACPI_DESCRIPTION_HEADER *) (UINTN)

> + Rsdp->RsdtAddress; }

> +

> + if (Entry == NULL) {

> + DEBUG ((DEBUG_INFO, "[%a] XsdtAddress and RsdtAddress are NULL!

> + \n",

> __FUNCTION__));

> + return NULL;

> + }

> +

> + return Entry;

> +}

> +

> +/**

> + This function calculates and updates an UINT8 checksum.

> +

> + @param[in] Buffer Pointer to buffer to checksum

> + @param[in] Size Number of bytes to checksum

> +**/

> +VOID

> +AcpiPlatformChecksum (

> + IN UINT8 *Buffer,

> + IN UINTN Size

> + )

> +{

> + UINTN ChecksumOffset;

> +

> + if (Buffer == NULL) {

> + return;

> + }

> +

> + ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER, Checksum);

> +

> + // Set checksum to 0 first

> + Buffer[ChecksumOffset] = 0;

> +

> + // Update checksum value

> + Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size); }

> +

> +/**

> + Convert AIP data block to PHAT ACPI style, and publish it onto

> + an existing ACPI PHAT structure or initialize and install a new

> + instance.

> +

> + @param[in] InfoBlock Point to AIP data block.

> + @param[in] InfoBlockSize The size of AIP data.

> +

> + @retval EFI_SUCCESS Success

> + @retval EFI_OUT_OF_RESOURCES Out of memory space.

> + @retval EFI_INVALID_PARAMETER Either InfoBlock is NULL,

> + TableKey is NULL, or

> + AcpiTableBufferSize and the size

> + field embedded in the ACPI table

> + pointed to by AcpiTableBuffer

> + are not in sync.

> + @retval EFI_ACCESS_DENIED The table signature matches a table already

> + present in the system and platform policy

> + does not allow duplicate tables of this type.

> + @retval EFI_NOT_FOUND AcpiEntry is NULL.

> +**/

> +EFI_STATUS

> +EFIAPI

> +InstallPhatTable (

> + IN VOID *InfoBlock,

> + IN UINTN InfoBlockSize

> + )

> +{

> + EFI_STATUS Status;

> + EFI_ACPI_DESCRIPTION_HEADER *AcpiEntry;

> + EFI_ACPI_SDT_PROTOCOL *AcpiSdtProtocol;

> + EFI_ACPI_DESCRIPTION_HEADER *PhatHeader;

> + UINT8 *PhatTable;

> + UINT32 PhatLen;

> + UINTN TableIndex;

> + UINT8 *TableHeader;

> + EFI_ACPI_TABLE_VERSION TableVersion;

> + UINTN TableKey;

> +

> + if ((InfoBlock == NULL) || (InfoBlockSize == 0)) {

> + DEBUG ((DEBUG_ERROR, "[%a] Table Data Invalid!\n", __FUNCTION__));

> + return EFI_INVALID_PARAMETER;

> + }

> +

> + Status = EFI_SUCCESS;

> + TableIndex = 0;

> + TableKey = 0;

> + TableHeader = NULL;

> +

> + AcpiEntry = SearchAcpiTablePointer (); if (AcpiEntry == NULL) {

> + DEBUG((DEBUG_ERROR, "[%a] ACPI table pointer not found\n",

> __FUNCTION__));

> + return EFI_NOT_FOUND;

> + }

> +

> + //

> + // Locate the EFI_ACPI_SDT_PROTOCOL.

> + //

> + Status = gBS->LocateProtocol (

> + &gEfiAcpiSdtProtocolGuid,

> + NULL,

> + (VOID **)&AcpiSdtProtocol

> + );

> + if (EFI_ERROR (Status)) {

> + DEBUG ((DEBUG_ERROR, "[%a] Failed to locate AcpiSdt with status:

> + %r\n",

> __FUNCTION__, Status));

> + return Status;

> + }

> +

> + // Search ACPI table for PHAT

> + while (!EFI_ERROR (Status)) {

> + Status = AcpiSdtProtocol->GetAcpiTable (

> + TableIndex,

> + (EFI_ACPI_SDT_HEADER **)&TableHeader,

> + &TableVersion,

> + &TableKey

> + );

> + if (!EFI_ERROR (Status)) {

> + TableIndex++;

> +

> + if (((EFI_ACPI_SDT_HEADER *) TableHeader)->Signature ==

> + EFI_ACPI_6_5_PLATFORM_HEALTH_ASSESSMENT_TABLE_SIGNATURE)

> + {

> + DEBUG ((DEBUG_INFO, "[%a] Existing Phat AcpiTable is

> + found.\n",

> __FUNCTION__));

> + break;

> + }

> +

> + if (!EFI_ERROR (Status)) {

> + //

> + // A PHAT is already in the ACPI table, update existing table and re-install

> + //

> + PhatHeader = (EFI_ACPI_DESCRIPTION_HEADER *) TableHeader;

> + PhatLen = PhatHeader->Length + (UINT32) InfoBlockSize;

> + PhatTable = (UINT8 *) AllocateZeroPool (PhatLen);

> + if (PhatTable == NULL) {

> + DEBUG ((DEBUG_ERROR, "[%a] Failed to allocated new PHAT pool

> + with.\n",

> __FUNCTION__));

> + return EFI_OUT_OF_RESOURCES;

> + }

> +

> + // Copy original table content to the new PHAT table pool

> + CopyMem (PhatTable, TableHeader, PhatHeader->Length);

> +

> + // Append InfoBlock in the end of the origin PHAT

> + CopyMem (PhatTable + PhatHeader->Length, InfoBlock,

> + InfoBlockSize);

> +

> + // Update the PHAT head pointer.

> + PhatHeader = (EFI_ACPI_DESCRIPTION_HEADER *) PhatTable;

> +

> + // Update the length field to found table plus appended new data

> + PhatHeader->Length = PhatLen;

> +

> + // Uninstall the origin PHAT from the ACPI table.

> + Status = mAcpiTableProtocol->UninstallAcpiTable (

> + mAcpiTableProtocol,

> + TableKey

> + );

> + ASSERT_EFI_ERROR (Status);

> +

> + if (EFI_ERROR (Status)) {

> + DEBUG ((DEBUG_ERROR, "[%a] Failed to uninstall existing PHAT

> + ACPI table

> with status: %r\n", __FUNCTION__, Status));

> + FreePool (PhatTable);

> + return Status;

> + }

> + } else {

> + //

> + // PHAT ACPI table does not exist, install new one

> + //

> + PhatTable = AllocateZeroPool (InfoBlockSize + sizeof

> (EFI_ACPI_DESCRIPTION_HEADER));

> + if (PhatTable == NULL) {

> + DEBUG ((DEBUG_ERROR, "[%a] Failed to allocate new PHAT

> + pool.\n",

> __FUNCTION__));

> + return EFI_OUT_OF_RESOURCES;

> + }

> + PhatHeader = (EFI_ACPI_DESCRIPTION_HEADER *) PhatTable;

> +

> + // Initialize the header of the Platform Health Assessment Table.

> + InitPhatTableHeader (PhatHeader, AcpiEntry->OemId,

> + &AcpiEntry->OemTableId);

> +

> + PhatHeader->Length = sizeof (EFI_ACPI_DESCRIPTION_HEADER) +

> + (UINT32)InfoBlockSize;

> +

> + // Connect a telemetry data to ACPI table header.

> + CopyMem (PhatTable + sizeof (EFI_ACPI_DESCRIPTION_HEADER),

> + InfoBlock, InfoBlockSize); }

> +

> + // Update table checksum

> + AcpiPlatformChecksum ((UINT8 *) PhatTable,

> + ((EFI_ACPI_DESCRIPTION_HEADER *) PhatHeader)->Length);

> +

> + // Install or update the Phat table.

> + Status = mAcpiTableProtocol->InstallAcpiTable (

> + mAcpiTableProtocol,

> + PhatTable,

> + PhatHeader->Length,

> + &TableKey

> + );

> + if (EFI_ERROR (Status)) {

> + DEBUG ((DEBUG_INFO, "[%a] Install Phat AcpiTable failed, Status =

> + [%r]. \n", __FUNCTION__, Status)); }

> +

> + if (PhatTable != NULL) {

> + FreePool (PhatTable);

> + }

> +

> + DEBUG ((DEBUG_INFO, "[%a] Install PHAT table, status: %r \n",

> +__FUNCTION__, Status));

> + return Status;

> +}

> diff --git

> a/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLi

> b.inf

> b/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAcpiLi

> b.inf

> new file mode 100644

> index 000000000..18374b265

> --- /dev/null

> +++ b/Platform/Intel/MinPlatformPkg/Acpi/Library/PhatAcpiLib/DxePhatAc

> +++ pi

> +++ Lib.inf

> @@ -0,0 +1,45 @@

> +## @file

> +# Dxe Platform Health Assessment Table Library # # Copyright (c)

> +SPDX-License-Identifier: BSD-2-Clause-Patent ###

> +

> +[Defines]

> + INF_VERSION = 0x00010017

> + BASE_NAME = DxePhatAcpiLib

> + FILE_GUID = 3932-bb84-adfb-4c7a-bc59-e33fc7ad6e20

> + VERSION_STRING = 1.0

> + MODULE_TYPE = DXE_DRIVER

> + LIBRARY_CLASS = DxePhatAcpiLib|DXE_DRIVER

> +

> +[Sources]

> + DxePhatAcpiLib.c

> +

> +[Packages]

> + MdePkg/MdePkg.dec

> + MdeModulePkg/MdeModulePkg.dec

> + MinPlatformPkg/MinPlatformPkg.dec

> +

> +[LibraryClasses]

> + DebugLib

> + BaseMemoryLib

> + MemoryAllocationLib

> + UefiBootServicesTableLib

> +

> +[Guids]

> + gEfiAcpiTableGuid ## CONSUMES

> + gEfiAcpi10TableGuid ## CONSUMES

> +

> +[Protocols]

> + gEfiAcpiTableProtocolGuid ## CONSUMES

> + gEfiAcpiSdtProtocolGuid ## CONSUMES

> +

> +[Pcd]

> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId ##

> CONSUMES

> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision ##

> CONSUMES

> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision ##

> CONSUMES

> +

> +[Depex]

> + TRUE

> diff --git

> a/Platform/Intel/MinPlatformPkg/Include/Library/PhatAcpiLib.h

> b/Platform/Intel/MinPlatformPkg/Include/Library/PhatAcpiLib.h

> new file mode 100644

> index 000000000..fe4fe5f5b

> --- /dev/null

> +++ b/Platform/Intel/MinPlatformPkg/Include/Library/PhatAcpiLib.h

> @@ -0,0 +1,40 @@

> +/** @file

> + Platform Health Assessment Table Library Definitions

> +

> + SPDX-License-Identifier: BSD-2-Clause-Patent **/

> +

> +#ifndef _PHAT_ACPI_LIB_H_

> +#define _PHAT_ACPI_LIB_H_

> +

> +/**

> + Convert AIP data block to PHAT ACPI style, and publish it onto

> + an existing ACPI PHAT structure or initialize and install a new

> + instance.

> +

> + @param[in] InfoBlock Point to AIP data block.

> + @param[in] InfoBlockSize The size of AIP data.

> +

> + @retval EFI_SUCCESS Success

> + @retval EFI_OUT_OF_RESOURCES Out of memory space.

> + @retval EFI_INVALID_PARAMETER Either InfoBlock is NULL,

> + TableKey is NULL, or

> + AcpiTableBufferSize and the size

> + field embedded in the ACPI table

> + pointed to by AcpiTableBuffer

> + are not in sync.

> + @retval EFI_ACCESS_DENIED The table signature matches a table already

> + present in the system and platform policy

> + does not allow duplicate tables of this type.

> + @retval EFI_NOT_FOUND AcpiEntry is NULL.

> +**/

> +EFI_STATUS

> +EFIAPI

> +InstallPhatTable (

> + IN VOID *InfoBlock,

> + IN UINTN InfoBlockSize

> + );

> +

> +#endif // _PHAT_ACPI_LIB_H_

> \ No newline at end of file

> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec

> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec

> index 74e1bce87..09312d329 100644

> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec

> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dec

> @@ -88,6 +88,8 @@

> LargeVariableReadLib|Include/Library/LargeVariableReadLib.h

> LargeVariableWriteLib|Include/Library/LargeVariableWriteLib.h

> + PhatAcpiLib|Include/Library/PhatAcpiLib.h

> +

> [PcdsFixedAtBuild, PcdsPatchableInModule]

> gMinPlatformPkgTokenSpaceGuid.PcdFspMaxUpdSize|0x00000000|UINT32|0x80

> 000000

> diff --git a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc

> b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc

> index ee5d21112..4b295babf 100644

> --- a/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc

> +++ b/Platform/Intel/MinPlatformPkg/MinPlatformPkg.dsc

> @@ -64,6 +64,7 @@

> PciSegmentInfoLib|MinPlatformPkg/Pci/Library/PciSegmentInfoLibSimple/P

> PciSegmentInfoLib|ciSeg

> mentInfoLibSimple.inf

> PlatformBootManagerLib|MinPlatformPkg/Bds/Library/DxePlatformBootManag

> PlatformBootManagerLib|er

> Lib/DxePlatformBootManagerLib.inf

> AslUpdateLib|MinPlatformPkg/Acpi/Library/DxeAslUpdateLib/DxeAslUpdateL

> AslUpdateLib|ib.in

> f

> +

> + PhatAcpiLib|MinPlatformPkg/Acpi/Library/DxePhatAcpiLib/DxePhatAcpiLib.

> + inf

> #

> # Misc

> @@ -217,5 +218,7 @@

> MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableReadLib.inf

> MinPlatformPkg/Library/BaseLargeVariableLib/BaseLargeVariableWriteLib.

> inf

> + MinPlatformPkg/Acpi/Library/DxePhatAcpiLib/DxePhatAcpiLib.inf

> +

> [BuildOptions]

> *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES

> --

> 2.39.1.windows.1