* [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions
@ 2021-08-17 13:24 Nhi Pham
2021-08-22 14:42 ` Abner Chang
0 siblings, 1 reply; 4+ messages in thread
From: Nhi Pham @ 2021-08-17 13:24 UTC (permalink / raw)
To: devel
Cc: patches, Nhi Pham, Leif Lindholm, Ard Biesheuvel, Abner Chang,
Daniel Schaefer
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@nuviainc.com>
Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
Cc: Abner Chang <abner.chang@hpe.com>
Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
Signed-off-by: Nhi Pham <nhi@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.
+
+ @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 *)&Buffer, &BufferSize);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ if (Buffer[0] != AML_NAME_OP) {
+ return EFI_NOT_FOUND;
+ }
+
+ switch (Buffer[5]) {
+ 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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions
2021-08-17 13:24 [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions Nhi Pham
@ 2021-08-22 14:42 ` Abner Chang
2021-09-03 13:13 ` Nhi Pham
2021-09-03 13:34 ` Nhi Pham
0 siblings, 2 replies; 4+ messages in thread
From: Abner Chang @ 2021-08-22 14:42 UTC (permalink / raw)
To: Nhi Pham, devel@edk2.groups.io
Cc: patches@amperecomputing.com, Leif Lindholm, Ard Biesheuvel,
Schaefer, Daniel
Hi Pham,
First at all, you have to update the copyright. Other comments are in below,
> -----Original Message-----
> From: Nhi Pham [mailto:nhi@os.amperecomputing.com]
> Sent: Tuesday, August 17, 2021 9:24 PM
> To: devel@edk2.groups.io
> Cc: patches@amperecomputing.com; Nhi Pham
> <nhi@os.amperecomputing.com>; Leif Lindholm <leif@nuviainc.com>; Ard
> Biesheuvel <ardb+tianocore@kernel.org>; Chang, Abner (HPS SW/FW
> Technologist) <abner.chang@hpe.com>; Schaefer, Daniel
> <daniel.schaefer@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@nuviainc.com>
> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
> Cc: Abner Chang <abner.chang@hpe.com>
> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
> Signed-off-by: Nhi Pham <nhi@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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions
2021-08-22 14:42 ` Abner Chang
@ 2021-09-03 13:13 ` Nhi Pham
2021-09-03 13:34 ` Nhi Pham
1 sibling, 0 replies; 4+ messages in thread
From: Nhi Pham @ 2021-09-03 13:13 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist), devel@edk2.groups.io
Cc: patches@amperecomputing.com, Leif Lindholm, Ard Biesheuvel,
Schaefer, Daniel
Hi Abner,
Thank you so much for reviewing this patch.
On 22/08/2021 21:42, Chang, Abner (HPS SW/FW Technologist) wrote:
> Hi Pham,
> First at all, you have to update the copyright. Other comments are in below,
Will update.
>
>> -----Original Message-----
>> From: Nhi Pham [mailto:nhi@os.amperecomputing.com]
>> Sent: Tuesday, August 17, 2021 9:24 PM
>> To: devel@edk2.groups.io
>> Cc: patches@amperecomputing.com; Nhi Pham
>> <nhi@os.amperecomputing.com>; Leif Lindholm <leif@nuviainc.com>; Ard
>> Biesheuvel <ardb+tianocore@kernel.org>; Chang, Abner (HPS SW/FW
>> Technologist) <abner.chang@hpe.com>; Schaefer, Daniel
>> <daniel.schaefer@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@nuviainc.com>
>> Cc: Ard Biesheuvel <ardb+tianocore@kernel.org>
>> Cc: Abner Chang <abner.chang@hpe.com>
>> Cc: Daniel Schaefer <daniel.schaefer@hpe.com>
>> Signed-off-by: Nhi Pham <nhi@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.
Thanks for your idea. Yes, I was intended to add a new structure
definition like EFI_ACPI_TABLE_WITH_KEY like your suggestion. But it
looks more complicated in consuming. Only the SSDT table is happy with
that :). Well, to support searching SSDT properly in this function, I
will add one more IN/OUT parameter "*Index" to this function to help
consumers can search the ACPI tables continuously. So, consumers can
call this function multiple times (continuously) if there are many SSDT
tables in the platform. What are your thoughts about this approach?
>
>> +
>> + @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.
I find the AML_OP_PARSE_INDEX_GET_OPCODE is defined for private use in
the MdeModulePkg/Universal/Acpi/AcpiTableDxe module. Do we need to make
a copy for usage in the AcpiLib?
>
>> *)&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?
Yes, thank you. Let me update it to use the SDT->SetOption function for
updating the data object in an AML name object.
Best regards,
Nhi
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions
2021-08-22 14:42 ` Abner Chang
2021-09-03 13:13 ` Nhi Pham
@ 2021-09-03 13:34 ` Nhi Pham
1 sibling, 0 replies; 4+ messages in thread
From: Nhi Pham @ 2021-09-03 13:34 UTC (permalink / raw)
To: Chang, Abner (HPS SW/FW Technologist), devel@edk2.groups.io
Cc: patches@amperecomputing.com, Leif Lindholm, Ard Biesheuvel,
Schaefer, Daniel
Hi Abner,
Thank you so much for reviewing this patch.
On 22/08/2021 21:42, Chang, Abner (HPS SW/FW Technologist) wrote:
> Hi Pham,
> First at all, you have to update the copyright. Other comments are in below,
Will update.
>> -----Original Message-----
>> From: Nhi Pham [mailto:nhi@os.amperecomputing.com]
>> Sent: Tuesday, August 17, 2021 9:24 PM
>> To:devel@edk2.groups.io
>> Cc:patches@amperecomputing.com; Nhi Pham
>> <nhi@os.amperecomputing.com>; Leif Lindholm<leif@nuviainc.com>; Ard
>> Biesheuvel<ardb+tianocore@kernel.org>; Chang, Abner (HPS SW/FW
>> Technologist)<abner.chang@hpe.com>; Schaefer, Daniel
>> <daniel.schaefer@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@nuviainc.com>
>> Cc: Ard Biesheuvel<ardb+tianocore@kernel.org>
>> Cc: Abner Chang<abner.chang@hpe.com>
>> Cc: Daniel Schaefer<daniel.schaefer@hpe.com>
>> Signed-off-by: Nhi Pham<nhi@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.
Thank you for a good idea. So, I was intended to add a new
EFI_ACPI_TABLE_WITH_KEY structure your suggestion. But it looks more
complicated in consuming. Only the SSDT table is happy with that. Well,
now my idea is that, to remove the caution, I want to add one more
IN/OUT parameter "*Index", which is the ACPI table index where to
search, to this API function. So, consumers can rely on this index to
search continuously the tables if there are multiple ACPI table
instances installed like SSDT. Does it look good to you?
>> +
>> + @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.
I find the AML_OP_PARSE_INDEX_GET_OPCODE is defined for private use in
the MdeModulePkg/Universal/Acpi/AcpiTableDxe module. Do we need to make
a copy for usage in the AcpiLib?
>> *)&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?
Yes, thank you. Let me update it to use the SDT->SetOption function for
updating the data object in an AML name object.
Best regards,
Nhi
>
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-09-03 13:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-17 13:24 [PATCH 1/1] EmbeddedPkg/AcpiLib: Add more helper functions Nhi Pham
2021-08-22 14:42 ` Abner Chang
2021-09-03 13:13 ` Nhi Pham
2021-09-03 13:34 ` Nhi Pham
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox