From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) by mx.groups.io with SMTP id smtpd.web12.7858.1639756587996330811 for ; Fri, 17 Dec 2021 07:56:28 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@nuviainc-com.20210112.gappssmtp.com header.s=20210112 header.b=Oh2Zrkos; spf=pass (domain: nuviainc.com, ip: 209.85.221.47, mailfrom: leif@nuviainc.com) Received: by mail-wr1-f47.google.com with SMTP id i22so4840681wrb.13 for ; Fri, 17 Dec 2021 07:56:27 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nuviainc-com.20210112.gappssmtp.com; s=20210112; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=nY8JhBBuCYukwJvZ561VpoxPKxtnvH+epmPFbzqhHV0=; b=Oh2ZrkosR7zb0HFAkT5dtn9qAEo9UnteIHuWXkIpSJpucbnTv9Vt/s4e8QaA9yD/7t 6mERTtSk8QSILPKV3PKBM+YDIQ2CO9bJv7OYoZjQwtZzEKPkCw4t1eYFSzkAhG0X2wyc cBs0PQwXc2kKdy8+zxBRFq6XY4m7bHo6wgnlVDp+4vP+YtKaGma3UTLPEGNPogT8YXsd HbzG1PxokdAh8SIIiywuZYUGYbHKa92wXN5sMTfuaitQAQZJfT0Ie7FlRDrJqYSAm/Iu CaJ4KyIqybMNAB4NWnR0CGhZtiZvppTDICJ3ArpqPq55Br4kDtefiKIP9cLMwYpHH7gC lYiQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=nY8JhBBuCYukwJvZ561VpoxPKxtnvH+epmPFbzqhHV0=; b=AiSDzDq8I2W1gJ/y6A5MzWUD7fPyA5ftKs2j63lFbGNi4fDJvp76uSKjPXnpyBk/VR uRApZqFdhjQ9ok+A/u7Pft5V5Ruh+KhI3seyFjbmgBcK9NS9M0yTNWWjKxgJnJembPRz wG76BNBFBEnbYYetX/gPXqRzM8ii+FGA7+bMc+VPOs+GkfWAlGI84w3kel6WBd2cjsHk CMUGRB68FmQ+Q3CrLvpsrcH+a3tiYYy/7sEQoNnD9UWIN+QY5kRJDcIIVu9zXzCohr48 DVmBWfz+CGN+si7dz+NwLD9NBXbC8ck34qZmKfHtRk4EKhps5W9d+8gVnJdVJVGPmvkp Svhg== X-Gm-Message-State: AOAM531CjdwBbGeDPRzztUoSyp8hfBqDf1haO3/clk2eHn/aNDdHvwdl ygHgt/mvDaW7UL92eErTIAggkw== X-Google-Smtp-Source: ABdhPJwqA+zhCK558spqsIBL17lCrbugFBNZG78Az9oxdqoU0KnCoIuwgTq+E4h3kcJVnosm2dKgFA== X-Received: by 2002:a5d:4ccc:: with SMTP id c12mr3034724wrt.453.1639756586551; Fri, 17 Dec 2021 07:56:26 -0800 (PST) Return-Path: Received: from leviathan (cpc92314-cmbg19-2-0-cust559.5-4.cable.virginm.net. [82.11.186.48]) by smtp.gmail.com with ESMTPSA id j85sm12878862wmj.3.2021.12.17.07.56.25 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 17 Dec 2021 07:56:26 -0800 (PST) Date: Fri, 17 Dec 2021 15:56:24 +0000 From: "Leif Lindholm" To: Nhi Pham Cc: "Chang, Abner (HPS SW/FW Technologist)" , "devel@edk2.groups.io" , "patches@amperecomputing.com" , Ard Biesheuvel , "Schaefer, Daniel" Subject: Re: [edk2-devel] [PATCH v2 1/1] EmbeddedPkg/AcpiLib: Add more helper functions Message-ID: References: <20210903154423.32619-1-nhi@os.amperecomputing.com> <4cc27fb8-6640-f9f8-374d-4424c789a98c@os.amperecomputing.com> MIME-Version: 1.0 In-Reply-To: <4cc27fb8-6640-f9f8-374d-4424c789a98c@os.amperecomputing.com> Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Hi Nhi, I have now pushed this patch as c63a10ecb7d6, and will get back to the rest of the Altra port. / Leif On Thu, Sep 09, 2021 at 00:02:36 +0700, Nhi Pham wrote: > Thanks Abner. > > I will correct the description for the function AcpiLocateTableBySignature > in the v3. > > Best regards, > Nhi > > On 08/09/2021 11:53, Chang, Abner (HPS SW/FW Technologist) wrote: > > After below comments are addressed, > > > > Reviewed-by: Abner Chang > > > > > -----Original Message----- > > > From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of > > > Nhi Pham via groups.io > > > Sent: Friday, September 3, 2021 11:44 PM > > > To: devel@edk2.groups.io > > > Cc: patches@amperecomputing.com; Nhi Pham > > > ; Leif Lindholm ; Ard > > > Biesheuvel ; Chang, Abner (HPS SW/FW > > > Technologist) ; Schaefer, Daniel > > > > > > Subject: [edk2-devel] [PATCH v2 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 > > > Cc: Ard Biesheuvel > > > Cc: Abner Chang > > > Cc: Daniel Schaefer > > > Signed-off-by: Nhi Pham > > > --- > > > Changes since v1: > > > + Add copyright [Abner] > > > + Improve the AcpiLocateTableBySignature function to remove the caution > > > for the usage of SSDT table. [Abner] > > > + AcpiAmlObjectUpdateInteger: Use the AcpiSdtProtocol->SetOption to > > > update > > > the value of data object. [Abner] > > > > > > EmbeddedPkg/Library/AcpiLib/AcpiLib.inf | 3 + > > > EmbeddedPkg/Include/Library/AcpiLib.h | 69 +++++++ > > > EmbeddedPkg/Library/AcpiLib/AcpiLib.c | 214 ++++++++++++++++++++ > > > 3 files changed, 286 insertions(+) > > > > > > diff --git a/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf > > > b/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf > > > index 538fe09cca29..01b12c9423a9 100644 > > > --- a/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf > > > +++ b/EmbeddedPkg/Library/AcpiLib/AcpiLib.inf > > > @@ -1,6 +1,7 @@ > > > #/** @file > > > # > > > # Copyright (c) 2014, ARM Ltd. All rights reserved. > > > +# Copyright (c) 2021, Ampere Computing LLC. All rights reserved. > > > # > > > # SPDX-License-Identifier: BSD-2-Clause-Patent > > > # > > > @@ -23,6 +24,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..933582b7f607 100644 > > > --- a/EmbeddedPkg/Include/Library/AcpiLib.h > > > +++ b/EmbeddedPkg/Include/Library/AcpiLib.h > > > @@ -2,6 +2,7 @@ > > > Helper Library for ACPI > > > > > > Copyright (c) 2014-2016, ARM Ltd. All rights reserved. > > > + Copyright (c) 2021, Ampere Computing LLC. All rights reserved. > > > > > > SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > @@ -13,6 +14,7 @@ > > > #include > > > > > > #include > > > +#include > > > > > > // > > > // Macros for the Generic Address Space > > > @@ -128,4 +130,71 @@ LocateAndInstallAcpiFromFv ( > > > IN CONST EFI_GUID* AcpiFile > > > ); > > > > > > +/** > > > + This function calculates and updates a UINT8 checksum > > > + in an ACPI description table header. > > > + > > > + @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 OUT 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. > > > + > > > + @param AcpiTableSdtProtocol Pointer to ACPI SDT protocol. > > > + @param TableSignature ACPI table signature. > > > + @param Index The zero-based index of the table where to > > > search the table. > > > + @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, > > > + IN OUT UINTN *Index, > > > + 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..393133f54381 100644 > > > --- a/EmbeddedPkg/Library/AcpiLib/AcpiLib.c > > > +++ b/EmbeddedPkg/Library/AcpiLib/AcpiLib.c > > > @@ -1,6 +1,7 @@ > > > /** @file > > > * > > > * Copyright (c) 2014-2015, ARM Limited. All rights reserved. > > > +* Copyright (c) 2021, Ampere Computing LLC. All rights reserved. > > > * > > > * SPDX-License-Identifier: BSD-2-Clause-Patent > > > * > > > @@ -9,9 +10,12 @@ > > > #include > > > > > > #include > > > +#include > > > +#include > > > #include > > > #include > > > > > > +#include > > > #include > > > #include > > > > > > @@ -170,3 +174,213 @@ LocateAndInstallAcpiFromFv ( > > > { > > > return LocateAndInstallAcpiFromFvConditional (AcpiFile, NULL); > > > } > > > + > > > +/** > > > + This function calculates and updates a UINT8 checksum > > > + in an ACPI description table header. > > > + > > > + @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 OUT 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. > > Is the description correct? I think this function can be used to search > > the given signature which could have multiple instances, right? > > > > > + > > > + @param AcpiTableSdtProtocol Pointer to ACPI SDT protocol. > > > + @param TableSignature ACPI table signature. > > > + @param Index The zero-based index of the table where to > > > search the table. > > Could you please mention that the index will be updated to the next instance > > if the table is found with the matched TableSignature? > > > > > > > + @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, > > > + IN OUT UINTN *Index, > > > + 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 = *Index; > > > + 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; > > > + *Index = TableIndex; > > > + 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_HANDLE DataHandle; > > > + EFI_ACPI_DATA_TYPE DataType; > > > + UINT8 *Buffer; > > > + UINTN BufferSize; > > > + UINTN DataSize; > > > + > > > + if (AcpiSdtProtocol == NULL || AsciiObjectPath == NULL) { > > > + return EFI_INVALID_PARAMETER; > > > + } > > > + > > > + ObjectHandle = NULL; > > > + 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)) { > > > + Status = EFI_NOT_FOUND; > > > + goto Exit; > > > + } > > > + ASSERT (DataType == EFI_ACPI_DATA_TYPE_OPCODE); > > > + ASSERT (Buffer != NULL); > > > + > > > + if (Buffer[0] != AML_NAME_OP) { > > > + Status = EFI_NOT_FOUND; > > > + goto Exit; > > > + } > > > + > > > + // > > > + // Get handle of data object > > > + // > > > + DataHandle = NULL; > > > + Status = AcpiSdtProtocol->GetChild (ObjectHandle, &DataHandle); > > > + ASSERT_EFI_ERROR (Status); > > > + > > > + Status = AcpiSdtProtocol->GetOption (DataHandle, 0, &DataType, (VOID > > Ok, that is fine to use 0 as you mentioned AML_OP_PARSE_INDEX_GET_OPCODE was defined privately. The better way > > is to define this in EmbeddedPkg/Library/AcpiLib.h. Sorry I don't have chance to reply your last mail. > > > > Abner > > > > > *)&Buffer, &BufferSize); > > > + ASSERT (DataType == EFI_ACPI_DATA_TYPE_OPCODE); > > > + ASSERT (Buffer != NULL); > > > + > > > + if (Buffer[0] == AML_ZERO_OP || Buffer[0] == AML_ONE_OP) { > > > + Status = AcpiSdtProtocol->SetOption (DataHandle, 0, (VOID *)&Value, > > > sizeof (UINT8)); > > > + ASSERT_EFI_ERROR (Status); > > > + } else { > > > + // > > > + // Check the size of data object > > > + // > > > + switch (Buffer[0]) { > > > + case AML_BYTE_PREFIX: > > > + DataSize = sizeof (UINT8); > > > + break; > > > + > > > + case AML_WORD_PREFIX: > > > + DataSize = sizeof (UINT16); > > > + break; > > > + > > > + case AML_DWORD_PREFIX: > > > + DataSize = sizeof (UINT32); > > > + break; > > > + > > > + case AML_QWORD_PREFIX: > > > + DataSize = sizeof (UINT64); > > > + break; > > > + > > > + default: > > > + // The data type of the ACPI object is not an integer > > > + Status = EFI_INVALID_PARAMETER; > > > + goto Exit; > > > + } > > > + > > > + Status = AcpiSdtProtocol->SetOption (DataHandle, 1, (VOID *)&Value, > > > DataSize); > > > + ASSERT_EFI_ERROR (Status); > > > + } > > > + > > > +Exit: > > > + AcpiSdtProtocol->Close (DataHandle); > > > + AcpiSdtProtocol->Close (ObjectHandle); > > > + > > > + return Status; > > > +} > > > -- > > > 2.17.1 > > > > > > > > > > > > > > >