Caution: This message originated from an External Source. Use proper
caution when opening attachments, clicking links, or responding.
Hello Abdul,
On 3/4/24 16:43, Abdul Lateef Attar wrote:
From: Abdul Lateef Attar <abdullateef.at...@amd.com>
Adds generic ACPI SSDT HPET table generator library.
Register/Deregister HPET table.
Adds ACPI namespace object for HPET device.
Adds Address space for HPET device.
Cc: Sami Mujawar <sami.muja...@arm.com>
Cc: Pierre Gondois <pierre.gond...@arm.com>
Signed-off-by: Abdul Lateef Attar <abdullateef.at...@amd.com>`
---
DynamicTablesPkg/DynamicTables.dsc.inc | 2 +
DynamicTablesPkg/Include/AcpiTableGenerator.h | 2 +
.../Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf | 36 +++
.../Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c | 266 ++++++++++++++++++
4 files changed, 306 insertions(+)
create mode 100644
DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
create mode 100644
DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c
diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc
b/DynamicTablesPkg/DynamicTables.dsc.inc
index 477dc6b6a9..fc2ac5962e 100644
--- a/DynamicTablesPkg/DynamicTables.dsc.inc
+++ b/DynamicTablesPkg/DynamicTables.dsc.inc
@@ -36,6 +36,7 @@
DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
+ DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
(note for later):
The HPET table seems to be intel specific actually,
[Components.IA32, Components.X64]
#
@@ -46,6 +47,7 @@
NULL|DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib.inf
NULL|DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib.inf
NULL|DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib.inf
+ NULL|DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
}
[Components.ARM, Components.AARCH64]
diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h
b/DynamicTablesPkg/Include/AcpiTableGenerator.h
index a32ef46ecb..ef651aa2aa 100644
--- a/DynamicTablesPkg/Include/AcpiTableGenerator.h
+++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h
@@ -1,6 +1,7 @@
/** @file
Copyright (c) 2017 - 2022, Arm Limited. All rights reserved.<BR>
+ Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -101,6 +102,7 @@ typedef enum StdAcpiTableId {
EStdAcpiTableIdPcct, ///< PCCT Generator
EStdAcpiTableIdHpet, ///< HPET Generator
EStdAcpiTableIdWsmt, ///< WSMT Generator
+ EStdAcpiTableIdSsdtHpet, ///< SSDT HPET
Generator
EStdAcpiTableIdMax
} ESTD_ACPI_TABLE_ID;
diff --git
a/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
b/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
new file mode 100644
index 0000000000..7586b31adf
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf
@@ -0,0 +1,36 @@
+## @file
+# SSDT HPET Table Generator
+#
+# Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+#
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+##
+
+[Defines]
+ INF_VERSION = 1.27
+ BASE_NAME = AcpiSsdtHpetLib
+ FILE_GUID = 85262912-AD7F-4EE0-8BB1-EE177275A54E
+ VERSION_STRING = 1.0
+ MODULE_TYPE = DXE_DRIVER
+ LIBRARY_CLASS = NULL|DXE_DRIVER
+ CONSTRUCTOR = AcpiSsdtHpetLibConstructor
+ DESTRUCTOR = AcpiSsdtHpetLibDestructor
+
+[Sources]
+ SsdtHpetGenerator.c
+
+[Packages]
+ DynamicTablesPkg/DynamicTablesPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ PcAtChipsetPkg/PcAtChipsetPkg.dec
+
+[LibraryClasses]
+ AcpiHelperLib
+ AmlLib
+ BaseLib
+ DebugLib
+ PcdLib
+
+[Pcd]
+ gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress
Cf. [1], I think this should be removed along the dependency over the
PcAtChipsetPkg dependency.
diff --git
a/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c
b/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c
new file mode 100644
index 0000000000..3d401204ae
--- /dev/null
+++ b/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c
@@ -0,0 +1,266 @@
+/** @file
+ SSDT HPET Table Generator
+
+ Copyright (c) 2017 - 2023, Arm Limited. All rights reserved.
+ Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reserved.
+
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+ @par Reference(s):
+ - ACPI 6.5 Specification, Aug 29, 2022
Is it possible to reference the HPET spec. with a link aswell if
possible ?
Same comment for the other generators with their respective spec.
+
+**/
+
+#include <AcpiTableGenerator.h>
+#include <ConfigurationManagerHelper.h>
+#include <ConfigurationManagerObject.h>
+#include <Library/AcpiHelperLib.h>
+#include <Library/AmlLib/AmlLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/TableHelperLib.h>
+
+/** The Creator ID for the ACPI tables generated using
+ the standard ACPI table generators.
+*/
+#define TABLE_GENERATOR_CREATOR_ID_GENERIC SIGNATURE_32('D', 'Y',
'N', 'T')
+
+/** This macro defines the HPET Table Generator revision.
+*/
+#define HPET_GENERATOR_REVISION CREATE_REVISION (1, 0)
Is it possible to move this definition down, next to the
ACPI_TABLE_GENERATOR
definition (just for consistency with other generators).
Same remark for the other generators added in this patchset.
+
+#define SB_SCOPE "\\_SB_"
+
+/** Construct the SSDT HPET devices Table.
+
+ This function invokes the Configuration Manager protocol interface
+ to get the required hardware information for generating the ACPI
+ table if required.
+
+ If this function allocates any resources then they must be freed
+ in the FreeXXXXTableResources function.
+
+ @param [in] This Pointer to the table generator.
+ @param [in] AcpiTableInfo Pointer to the ACPI Table Info.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [out] Table Pointer to the constructed ACPI Table.
+
+ @retval EFI_SUCCESS Table generated successfully.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The required object was not found.
+ @retval EFI_BAD_BUFFER_SIZE The size returned by the Configuration
+ Manager is less than the Object size
for the
+ requested object.
+**/
+STATIC
+EFI_STATUS
+BuildSsdtHpetTable (
+ IN CONST ACPI_TABLE_GENERATOR *CONST This,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST AcpiTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
CfgMgrProtocol,
+ OUT EFI_ACPI_DESCRIPTION_HEADER **CONST Table
+ )
+{
+ EFI_STATUS Status;
+ AML_ROOT_NODE_HANDLE RootNode;
+ AML_OBJECT_NODE_HANDLE ScopeNode;
+ AML_OBJECT_NODE_HANDLE HpetNode;
+ AML_OBJECT_NODE_HANDLE CrsNode;
+ UINT32 EisaId;
+
+ ASSERT (This != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (Table != NULL);
+ ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
+ ASSERT (AcpiTableInfo->AcpiTableSignature ==
This->AcpiTableSignature);
+
+ Status = AddSsdtAcpiHeader (
+ CfgMgrProtocol,
+ This,
+ AcpiTableInfo,
+ &RootNode
+ );
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ Status = AmlCodeGenScope (SB_SCOPE, RootNode, &ScopeNode);
+ if (EFI_ERROR (Status)) {
+ goto exit_handler;
+ }
+
+ Status = AmlCodeGenDevice ("HPET", ScopeNode, &HpetNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlGetEisaIdFromString ("PNP0103", &EisaId);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlCodeGenNameInteger ("_HID", EisaId, HpetNode, NULL);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlCodeGenNameInteger ("_UID", 0x00, HpetNode, NULL);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlCodeGenNameResourceTemplate ("_CRS", HpetNode, &CrsNode);
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlCodeGenRdMemory32Fixed (FALSE, PcdGet32
(PcdHpetBaseAddress), SIZE_1KB, CrsNode, NULL);
I didn't find anything in the spec. stating the memory range should be
read only or
read-write. There are examples for both in edk2-platforms. Did you see
something
specific going in the way of a read only configuration ?
---
[1]
- to avoid making the DynamicTablesPkg dependent over the
PcAtChipsetPkg.dec
package (not that it is explicitly forbidden).
- to avoid making the DynamicTablesPkg fetch configuration information
from
PCDs instead of CmObjects (there is a counter example, but this
should be avoided
I think).
would it be possible to create a HPET object ? The base address would
be fetched
through this object instead. The object would contain:
- UINT32 BaseAddress
- (maybe, depending on whether this is usefull) BOOLEAN IsReadOnly
This would mean that the ConfigurationManager used for your platform
will have the dependency over the PcAtChipsetPkg and populate the HPET
CmObject with:
PcdGet32 (PcdHpetBaseAddress)
+ if (EFI_ERROR (Status)) {
+ ASSERT_EFI_ERROR (Status);
+ goto exit_handler;
+ }
+
+ Status = AmlSerializeDefinitionBlock (
+ RootNode,
+ Table
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "ERROR: SSDT-HPET: Failed to Serialize SSDT Table Data."
+ " Status = %r\n",
+ Status
+ ));
+ goto exit_handler;
+ }
+
+exit_handler:
+ // Delete the RootNode and its attached children.
+ return AmlDeleteTree (RootNode);
+}
+
+/** Free any resources allocated for constructing the
+ SSDT HPET ACPI table.
+
+ @param [in] This Pointer to the table generator.
+ @param [in] AcpiTableInfo Pointer to the ACPI Table Info.
+ @param [in] CfgMgrProtocol Pointer to the Configuration Manager
+ Protocol Interface.
+ @param [in, out] Table Pointer to the ACPI Table.
+
+ @retval EFI_SUCCESS The resources were freed successfully.
+ @retval EFI_INVALID_PARAMETER The table pointer is NULL or invalid.
+**/
+STATIC
+EFI_STATUS
+FreeSsdtHpetTableResources (
+ IN CONST ACPI_TABLE_GENERATOR *CONST This,
+ IN CONST CM_STD_OBJ_ACPI_TABLE_INFO *CONST
AcpiTableInfo,
+ IN CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL *CONST
CfgMgrProtocol,
+ IN OUT EFI_ACPI_DESCRIPTION_HEADER **CONST Table
+ )
+{
+ ASSERT (This != NULL);
+ ASSERT (AcpiTableInfo != NULL);
+ ASSERT (CfgMgrProtocol != NULL);
+ ASSERT (AcpiTableInfo->TableGeneratorId == This->GeneratorID);
+ ASSERT (AcpiTableInfo->AcpiTableSignature ==
This->AcpiTableSignature);
+
+ if ((Table == NULL) || (*Table == NULL)) {
+ DEBUG ((DEBUG_ERROR, "ERROR: SSDT-HPET: Invalid Table Pointer\n"));
+ ASSERT ((Table != NULL) && (*Table != NULL));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ FreePool (*Table);
+ *Table = NULL;
+ return EFI_SUCCESS;
+}
+
+/** The interface for the SSDT HPET Table Generator.
+*/
+STATIC
+CONST
+ACPI_TABLE_GENERATOR mSsdtPlatHpetGenerator = {
+ // Generator ID
+ CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtHpet),
+ // Generator Description
+ L"ACPI.STD.SSDT.HPET.GENERATOR",
+ // ACPI Table Signature
+ EFI_ACPI_6_5_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE,
+ // ACPI Table Revision - Unused
+ 0,
+ // Minimum ACPI Table Revision - Unused
+ 0,
+ // Creator ID
+ TABLE_GENERATOR_CREATOR_ID_GENERIC,
@Sami, shouldn't all the generators use this Creator Id instead of
'ARMH' ?
If yes the definition of:
TABLE_GENERATOR_CREATOR_ID_GENERIC
should be placed there I think:
DynamicTablesPkg/Include/AcpiTableGenerator.h
+ // Creator Revision
+ HPET_GENERATOR_REVISION,
+ // Build Table function
+ BuildSsdtHpetTable,
+ // Free Resource function
+ FreeSsdtHpetTableResources,
+ // FreeSsdtPlatDevicesTableResources,
Should be removed I think.
+ // Extended build function not needed
+ NULL,
+ // Extended build function not implemented by the generator.
+ // Hence extended free resource function is not required.
+ NULL
+};
+
+/** Register the Generator with the ACPI Table Factory.
+
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+
+ @retval EFI_SUCCESS The Generator is registered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_ALREADY_STARTED The Generator for the Table ID
+ is already registered.
+**/
+EFI_STATUS
+EFIAPI
+AcpiSsdtHpetLibConstructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = RegisterAcpiTableGenerator (&mSsdtPlatHpetGenerator);
+ DEBUG ((DEBUG_INFO, "SSDT-HPET: Register Generator. Status =
%r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
+
+/** Deregister the Generator from the ACPI Table Factory.
+
+ @param [in] ImageHandle The handle to the image.
+ @param [in] SystemTable Pointer to the System Table.
+
+ @retval EFI_SUCCESS The Generator is deregistered.
+ @retval EFI_INVALID_PARAMETER A parameter is invalid.
+ @retval EFI_NOT_FOUND The Generator is not registered.
+**/
+EFI_STATUS
+EFIAPI
+AcpiSsdtHpetLibDestructor (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = DeregisterAcpiTableGenerator (&mSsdtPlatHpetGenerator);
+ DEBUG ((DEBUG_INFO, "SSDT-HPET: Deregister Generator. Status =
%r\n", Status));
+ ASSERT_EFI_ERROR (Status);
+ return Status;
+}
Regards,
Pierre
