[edk2-devel] [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions

Abner Chang abner.chang at hpe.com
Sun Aug 22 14:42:35 UTC 2021


Hi Pham,
First at all, you have to update the copyright. Other comments are in below,

> -----Original Message-----
> From: Nhi Pham [mailto:nhi at os.amperecomputing.com]
> Sent: Tuesday, August 17, 2021 9:24 PM
> To: devel at edk2.groups.io
> Cc: patches at amperecomputing.com; Nhi Pham
> <nhi at os.amperecomputing.com>; Leif Lindholm <leif at nuviainc.com>; Ard
> Biesheuvel <ardb+tianocore at kernel.org>; Chang, Abner (HPS SW/FW
> Technologist) <abner.chang at hpe.com>; Schaefer, Daniel
> <daniel.schaefer at hpe.com>
> Subject: [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions
> 
> This adds more helper functions that assist in calculating the checksum,
> locating an ACPI table by signature, and updating an AML integer object.
> 
> Cc: Leif Lindholm <leif at nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore at kernel.org>
> Cc: Abner Chang <abner.chang at hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer at hpe.com>
> Signed-off-by: Nhi Pham <nhi at os.amperecomputing.com>
> ---
>  EmbeddedPkg/Library/AcpiLib/AcpiLib.inf |   2 +
>  EmbeddedPkg/Include/Library/AcpiLib.h   |  68 ++++++++
>  EmbeddedPkg/Library/AcpiLib/AcpiLib.c   | 183 ++++++++++++++++++++
>  3 files changed, 253 insertions(+)
> 
> diff --git a/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
> b/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
> index 538fe09cca29..154cb1eebc80 100644
> --- a/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
> +++ b/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf
> @@ -23,6 +23,8 @@ [Packages]
>    EmbeddedPkg/EmbeddedPkg.dec
> 
>  [LibraryClasses]
> +  BaseLib
> +  BaseMemoryLib
>    DebugLib
>    UefiBootServicesTableLib
> 
> diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h
> b/EmbeddedPkg/Include/Library/AcpiLib.h
> index c142446d9d59..cdb6ea410c54 100644
> --- a/EmbeddedPkg/Include/Library/AcpiLib.h
> +++ b/EmbeddedPkg/Include/Library/AcpiLib.h
> @@ -13,6 +13,7 @@
>  #include <Uefi.h>
> 
>  #include <IndustryStandard/Acpi10.h>
> +#include <Protocol/AcpiSystemDescriptionTable.h>
> 
>  //
>  // Macros for the Generic Address Space
> @@ -128,4 +129,71 @@ LocateAndInstallAcpiFromFv (
>    IN CONST EFI_GUID* AcpiFile
>    );
> 
> +/**
> +  This function calculates and updates an UINT8 checksum.
> +
> +  @param  Buffer          Pointer to buffer to checksum
> +  @param  Size            Number of bytes to checksum
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiUpdateChecksum (
> +  IN UINT8      *Buffer,
> +  IN UINTN      Size
> +  );
> +
> +/**
> +  This function uses the ACPI SDT protocol to locate an ACPI table
> +  with a given signature that only have a single instance.
> +
> +  Caution: This function does not act correctly with tables having
> +  more than 2 instances like SSDT table.
> +
> +  @param  AcpiTableSdtProtocol    Pointer to ACPI SDT protocol.
> +  @param  TableSignature          ACPI table signature.
> +  @param  Table                   Pointer to the table.
> +  @param  TableKey                Pointer to the table key.
> +
> +  @return EFI_SUCCESS             The function completed successfully.
> +  @return EFI_INVALID_PARAMETER   At least one of parameters is invalid.
> +  @retval EFI_NOT_FOUND           The requested index is too large and a
> table was not found.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiLocateTableBySignature (
> +  IN  EFI_ACPI_SDT_PROTOCOL           *AcpiSdtProtocol,
> +  IN  UINT32                          TableSignature,
> +  OUT EFI_ACPI_DESCRIPTION_HEADER     **Table,
> +  OUT UINTN                           *TableKey
> +  );
> +
> +/**
> +  This function updates the integer value of an AML Object.
> +
> +  @param  AcpiTableSdtProtocol    Pointer to ACPI SDT protocol.
> +  @param  TableHandle             Points to the table representing the starting
> point
> +                                  for the object path search.
> +  @param  AsciiObjectPath         Pointer to the ACPI path of the object being
> updated.
> +  @param  Value                   New value to write to the object.
> +
> +  @return EFI_SUCCESS             The function completed successfully.
> +  @return EFI_INVALID_PARAMETER   At least one of parameters is invalid
> or the data type
> +                                  of the ACPI object is not an integer value.
> +  @retval EFI_NOT_FOUND           The object is not found with the given path.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiAmlObjectUpdateInteger (
> +  IN  EFI_ACPI_SDT_PROTOCOL           *AcpiSdtProtocol,
> +  IN  EFI_ACPI_HANDLE                 TableHandle,
> +  IN  CHAR8                           *AsciiObjectPath,
> +  IN  UINTN                           Value
> +  );
> +
>  #endif // __ACPI_LIB_H__
> diff --git a/EmbeddedPkg/Library/AcpiLib/AcpiLib.c
> b/EmbeddedPkg/Library/AcpiLib/AcpiLib.c
> index ff7d678433d5..e07919ae323f 100644
> --- a/EmbeddedPkg/Library/AcpiLib/AcpiLib.c
> +++ b/EmbeddedPkg/Library/AcpiLib/AcpiLib.c
> @@ -9,9 +9,12 @@
>  #include <Uefi.h>
> 
>  #include <Library/AcpiLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
>  #include <Library/DebugLib.h>
>  #include <Library/UefiBootServicesTableLib.h>
> 
> +#include <Protocol/AcpiSystemDescriptionTable.h>
>  #include <Protocol/AcpiTable.h>
>  #include <Protocol/FirmwareVolume2.h>
> 
> @@ -170,3 +173,183 @@ LocateAndInstallAcpiFromFv (
>  {
>    return LocateAndInstallAcpiFromFvConditional (AcpiFile, NULL);
>  }
> +
> +/**
> +  This function calculates and updates an UINT8 checksum.
> +
> +  @param  Buffer          Pointer to buffer to checksum
> +  @param  Size            Number of bytes to checksum
> +
> +  @retval EFI_SUCCESS             The function completed successfully.
> +  @retval EFI_INVALID_PARAMETER   Invalid parameter.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiUpdateChecksum (
> +  IN UINT8      *Buffer,
> +  IN UINTN      Size
> +  )
> +{
> +  UINTN ChecksumOffset;
> +
> +  if (Buffer == NULL || Size == 0) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  ChecksumOffset = OFFSET_OF (EFI_ACPI_DESCRIPTION_HEADER,
> Checksum);
> +
> +  //
> +  // Set checksum to 0 first
> +  //
> +  Buffer[ChecksumOffset] = 0;
> +
> +  //
> +  // Update checksum value
> +  //
> +  Buffer[ChecksumOffset] = CalculateCheckSum8 (Buffer, Size);
> +
> +  return EFI_SUCCESS;
> +}
> +
> +/**
> +  This function uses the ACPI SDT protocol to locate an ACPI table
> +  with a given signature that only have a single instance.
> +
> +  Caution: This function does not act correctly with tables having
> +  more than 2 instances like SSDT table.
Instead of having this caution, why not returning an array of table pointers and TableKeys, also another new parameter indicates how many tables are matched to the signature?
How to use the array depends on the caller.

> +
> +  @param  AcpiTableSdtProtocol    Pointer to ACPI SDT protocol.
> +  @param  TableSignature          ACPI table signature.
> +  @param  Table                   Pointer to the table.
> +  @param  TableKey                Pointer to the table key.
> +
> +  @return EFI_SUCCESS             The function completed successfully.
> +  @return EFI_INVALID_PARAMETER   At least one of parameters is invalid.
> +  @retval EFI_NOT_FOUND           The requested index is too large and a
> table was not found.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiLocateTableBySignature (
> +  IN  EFI_ACPI_SDT_PROTOCOL           *AcpiSdtProtocol,
> +  IN  UINT32                          TableSignature,
> +  OUT EFI_ACPI_DESCRIPTION_HEADER     **Table,
> +  OUT UINTN                           *TableKey
> +  )
> +{
> +  EFI_STATUS              Status;
> +  EFI_ACPI_SDT_HEADER     *TempTable;
> +  EFI_ACPI_TABLE_VERSION  TableVersion;
> +  UINTN                   TableIndex;
> +
> +  if (AcpiSdtProtocol == NULL
> +      || Table == NULL
> +      || TableKey == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = EFI_SUCCESS;
> +
> +  //
> +  // Search for ACPI Table with matching signature
> +  //
> +  TableVersion = 0;
> +  TableIndex = 0;
> +  while (!EFI_ERROR (Status)) {
> +    Status = AcpiSdtProtocol->GetAcpiTable (
> +                                TableIndex,
> +                                &TempTable,
> +                                &TableVersion,
> +                                TableKey
> +                                );
> +    if (!EFI_ERROR (Status)) {
> +      TableIndex++;
> +
> +      if (((EFI_ACPI_DESCRIPTION_HEADER *)TempTable)->Signature ==
> TableSignature) {
> +        *Table = (EFI_ACPI_DESCRIPTION_HEADER *)TempTable;
> +        break;
> +      }
> +    }
> +  }
> +
> +  return Status;
> +}
> +
> +/**
> +  This function updates the integer value of an AML Object.
> +
> +  @param  AcpiTableSdtProtocol    Pointer to ACPI SDT protocol.
> +  @param  TableHandle             Points to the table representing the starting
> point
> +                                  for the object path search.
> +  @param  AsciiObjectPath         Pointer to the ACPI path of the object being
> updated.
> +  @param  Value                   New value to write to the object.
> +
> +  @return EFI_SUCCESS             The function completed successfully.
> +  @return EFI_INVALID_PARAMETER   At least one of parameters is invalid
> or the data type
> +                                  of the ACPI object is not an integer value.
> +  @retval EFI_NOT_FOUND           The object is not found with the given path.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +AcpiAmlObjectUpdateInteger (
> +  IN  EFI_ACPI_SDT_PROTOCOL           *AcpiSdtProtocol,
> +  IN  EFI_ACPI_HANDLE                 TableHandle,
> +  IN  CHAR8                           *AsciiObjectPath,
> +  IN  UINTN                           Value
> +  )
> +{
> +  EFI_STATUS            Status;
> +  EFI_ACPI_HANDLE       ObjectHandle;
> +  EFI_ACPI_DATA_TYPE    DataType;
> +  UINT8                 *Buffer;
> +  UINTN                 BufferSize;
> +
> +  if (AcpiSdtProtocol == NULL || AsciiObjectPath == NULL) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  Status = AcpiSdtProtocol->FindPath (TableHandle, AsciiObjectPath,
> &ObjectHandle);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  Status = AcpiSdtProtocol->GetOption (ObjectHandle, 0, &DataType, (VOID
We can use AML_OP_PARSE_INDEX_GET_OPCODE instead of '0', which is more readable.

> *)&Buffer, &BufferSize);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (Buffer[0] != AML_NAME_OP) {
> +    return EFI_NOT_FOUND;
> +  }
> +
> +  switch (Buffer[5]) {
After  getting the opcode from ACPI handle, the BufferSize suppose is in value of 1 or 2 if I interpret the code correctly. You can access to Buffer[5] because the buffer points the AML node.
Below can be executed correctly but seems to me not quite proper. Furthermore, can we leverage AcpiSdtProtocol ->SetOpion for updating value instead of having below code block?

Abner

> +  case AML_ZERO_OP:
> +  case AML_ONE_OP:
> +    Buffer[5] = (UINT8)Value;
> +    break;
> +
> +  case AML_BYTE_PREFIX:
> +    Buffer[6] = (UINT8)Value;
> +    break;
> +
> +  case AML_WORD_PREFIX:
> +    CopyMem ((VOID *)&Buffer[6], (VOID *)&Buffer, sizeof (UINT16));
> +    break;
> +
> +  case AML_DWORD_PREFIX:
> +    CopyMem ((VOID *)&Buffer[6], (VOID *)&Buffer, sizeof (UINT32));
> +    break;
> +
> +  case AML_QWORD_PREFIX:
> +    CopyMem ((VOID *)&Buffer[6], (VOID *)&Buffer, sizeof (UINT64));
> +    break;
> +
> +  default:
> +    // The data type of the ACPI object is not an integer value.
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  return Status;
> +}
> --
> 2.17.1



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#79686): https://edk2.groups.io/g/devel/message/79686
Mute This Topic: https://groups.io/mt/84947529/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