From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id AB50C740040 for ; Wed, 13 Mar 2024 09:49:22 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=5AEh5eQxO1APAC7F+hPFphCY5jc3bEfm14YdYttQzvA=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:User-Agent:Subject:To:CC:References:From:In-Reply-To:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Resent-Date:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Language:Content-Type:Content-Transfer-Encoding; s=20240206; t=1710323361; v=1; b=jvbCWikWTltK0ZP1hVMV+Rqj7YwNuFIAS5Cwy8z48IGyZmvZfo2OS9KmD7G5MSvm5zSbveeR 6reSXX0yAfpnTgZxdlWxHl9bBSA9h7lrs3odtsLgT4bgF/HGLwqjwjQBgfACNNQVwFeUWebi1FH RNh0iZeMZ3wGsvGqHW33DNDoxdGvBGOY2doX2EBjMFcTTGg4DTfKyCc0Up5UMyvfGMqATyOsUxO lz531hA7UOR0z6siAu6B9uGPX8M0ngDHXIz5D2U+8zXihr0x6PaXmmf7dT6RLpHBgLzhdEasxat /44B6fjDZcKEr6+CRkMU5xTFFt/LUc0GCohZkJAQhLuKw== X-Received: by 127.0.0.2 with SMTP id NzDOYY7687511xWZTfe1EyMp; Wed, 13 Mar 2024 02:49:21 -0700 X-Received: from NAM11-DM6-obe.outbound.protection.outlook.com (NAM11-DM6-obe.outbound.protection.outlook.com [40.107.223.65]) by mx.groups.io with SMTP id smtpd.web10.12392.1710323360392469468 for ; Wed, 13 Mar 2024 02:49:20 -0700 X-Received: from IA1PR12MB6458.namprd12.prod.outlook.com (2603:10b6:208:3aa::22) by DS7PR12MB9041.namprd12.prod.outlook.com (2603:10b6:8:ea::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.7386.20; Wed, 13 Mar 2024 09:49:16 +0000 X-Received: from IA1PR12MB6458.namprd12.prod.outlook.com ([fe80::b913:7732:22e1:b71e]) by IA1PR12MB6458.namprd12.prod.outlook.com ([fe80::b913:7732:22e1:b71e%7]) with mapi id 15.20.7386.017; Wed, 13 Mar 2024 09:49:15 +0000 Message-ID: <11278586-09b3-49d8-b2bc-3e083e9afa49@amd.com> Date: Wed, 13 Mar 2024 15:19:08 +0530 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH v2 4/4] DynamicTablesPkg: Adds ACPI SSDT HPET Table generator To: Pierre Gondois , Abdul Lateef Attar , devel@edk2.groups.io CC: Sami Mujawar References: <514bf3127bf6d19396d323dda1d67672095a7b18.1709566848.git.AbdulLateef.Attar@amd.com> <405fb4ba-bfde-41a2-9f79-078d5c895d78@arm.com> From: "Abdul Lateef Attar via groups.io" In-Reply-To: <405fb4ba-bfde-41a2-9f79-078d5c895d78@arm.com> X-ClientProxiedBy: PN3PR01CA0116.INDPRD01.PROD.OUTLOOK.COM (2603:1096:c01:96::6) To IA1PR12MB6458.namprd12.prod.outlook.com (2603:10b6:208:3aa::22) MIME-Version: 1.0 X-MS-PublicTrafficType: Email X-MS-TrafficTypeDiagnostic: IA1PR12MB6458:EE_|DS7PR12MB9041:EE_ X-MS-Office365-Filtering-Correlation-Id: c53ebbaf-63c3-4cc7-9a40-08dc4342d833 X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Message-Info: Q99XRw+Kznc1k2SsCnYIgAX5dRsQrmhKYoKPv4hPe56b1RoCppLCLjMokOeMi/dgFEBnt/Ryny4f2318vsIJhTyPE4FAszYpK6fXVX01ecbFK5L0a2ogQEBLOTKkhAUqyW1OdjHfuNVs0ZmcPfyGcf6YK5KvM5xWW2uMG4b1c4K4CzF3GRqF76vNkF0rR03z8vFvE4m6HOonc+85MSWLh1BEr+4j8XFquNievc2RmdL8f+c0VbLtnyTxIF37CJ2P2jqruoi8FoKwu+EsVBxnQOwJoWbmu7lzGRPMYTAELCRyeZfVPmPpxTVsooDghzciV+APOzN/Ao3RP1PhbP37wDv8rO1DS8WAXhJaNhzCbTY4utHWnbOaXZ+0bBgfxASh9xjKD38z2+2wUISy7lYRVUURXEpPBwChNe/X+L+QILor8jxhx4KrZcAt/KJFG0oopVbxVZqtd34A01rnA7+yCWbHtJp2nkEBN3eK0CLdzbf/1AtylOqHQYk2fKSaPbG0GgCovIOqtSKOe3ubizsX+ZnmtlFpQxR6R18GK6v6Q88tGg5xT03wYj6R88clk8+7T2ENh32AJfpaEIElG0codwWZqyY+KJVNo7uSMqcDm1KrRA2VhXWcw+fQ61UWDpr4MjWet3tfaBEqqXdmq38bAEIrMQA9b642jN8iPsbKT0E= X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?meUneN8OF1i1gxr46tZxzWMsdsaRA7IeJMWxIPX8NxEjTKy3rS699HI4iuxG?= =?us-ascii?Q?K+nMmY4tEx873BsL+lX+IHXd7Z1Rb0fSeas+rBghflrEeMAnvc8Iy/OLjCGy?= =?us-ascii?Q?gYmQpVx2rdt2qZYELQHL5zorAzvyH+ErnDdMMA+1oQwDoj9Cu//IT6G2orOL?= =?us-ascii?Q?Z2R1qLbWtW0Xa1PMNRvpMyjnqL1yyHSvsRh36DV8bDxWcW4kHXbJavfENqfl?= =?us-ascii?Q?hF6h8fSPX3rkyFnuLHIw+EATEmOTXjPg+l44Gkmlc0LyvlmpdekS1BLJsZP3?= =?us-ascii?Q?zcS9lRbHqx0W/bqKu7AVt+qNRVct+Ufk5MXiFSku5jywQegR7ojW6NSp7XBd?= =?us-ascii?Q?a/VSdKWDwQHBJu4EE5kKGvYuNk/9gV2PBVLgJuhmR9Qth/pk/IzyLmVb2xbT?= =?us-ascii?Q?WotXwb9oD6MKW5TSxTfF6+EQQxBbSM5h7DWxe/GwxNYg6zAKzkCJwg0niVJK?= =?us-ascii?Q?24Scrxcbhp+PLT88Jm4/gpzjrT74yLafcpH+BVfn/6lfzyMDeg4cT0FzcXDo?= =?us-ascii?Q?MTjmkWpT8Rhk/NE1YxDlszy9v6QBRUYO0KWzMirM6ClqqK9JdsNopxkxMQjh?= =?us-ascii?Q?wXArYcCZ7pPmNMBd2vU+DdQcgpQRrNcFKZUosp9dH8wXL02WZ21fqYA0RhQd?= =?us-ascii?Q?nTqyjOcyzsIrCyHyhRNB31wKwDCz9fgLmVJ2sKebzIPXrJm872gwETyGLMIf?= =?us-ascii?Q?d9YW7xSyRQGHYBokMP5F9uH/DMMlJ0URG1VjI4VaJT89hvsS6zY7D0jutkL1?= =?us-ascii?Q?TpJekd0t/xXMXPlnviTCVSbAGoAtdXMwRITRIdcm5yVe/u1BjAWy2LQAm2uO?= =?us-ascii?Q?a7DVC/t3Dohml/a+YXn8u72lrWo3CozUCCOlku6LBGevxA0un4LZ1yXk5OfV?= =?us-ascii?Q?TFKlMEFodjPi0NYeksDAW8f152pQFQ4pMFJYp+0nYtxx6V6QT/Azbh2xpY7K?= =?us-ascii?Q?aUJz4kJmTKWmk3mQnFGm/6HH/lgGnFRBXdEmHQq1ze9zCQyo26YQW+FigxZS?= =?us-ascii?Q?GsUqkp/PvU5X3ecfwXt4MJ5vw53NvZEkm9qoWh1mF0BrKj0j94ytSpvqQ0Ik?= =?us-ascii?Q?DH1gavVGgOGaS6Gnq50BhvDNG6YFBXpEPNRgrfomzP+qK8lpGadxViYpyU+c?= =?us-ascii?Q?sFMUKULHAWAs0JwnV32S7LZ5Er5Bbdo81cknsLLmfUtPiyZjSX0qSFtMz8l0?= =?us-ascii?Q?rdXKFXupiprHeK61KZ1R8v2W24wIz3tobP9MRRbQe1ASF6WN1KV0SqHN1ckg?= =?us-ascii?Q?jLSl0fj1uvw/dg712bA4RvJUEnuQuOoEEt3tkXy5GQ5AaUmG5xLSl5+Nd2k7?= =?us-ascii?Q?HBFUQXKUAs7MdeHeiLIWwqzdcsMcuHOJa64xLnA1Ux1Fm2U1Ly/E/Wk0G5CE?= =?us-ascii?Q?GSbkqtlxvuFJiPyFEPB5hAu91mCYy53tTkRm+vkJQ6s1W9XMeObY/A296jtX?= =?us-ascii?Q?lIM2m/w6A7pvOfD62xdxkUkijujgcWpiPc7JpJ+HBiDhoZSi6b5vQhStv/qe?= =?us-ascii?Q?Wsfdk5Z+oqxr0pebTDPK+zMPhAkEdbef877UMBN4j326j2j9jVDh19MMoTAo?= =?us-ascii?Q?L5GRA8EuV4yxIbVQAwA4eetM7lqs1EJxPBH+i0dI?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: c53ebbaf-63c3-4cc7-9a40-08dc4342d833 X-MS-Exchange-CrossTenant-AuthSource: IA1PR12MB6458.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Mar 2024 09:49:15.6831 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: bSosFX7q8kyHkRHRDFUou5vzNOPjj1HK1lBRCU4cOWXioF8RviyhJ99eylsz9T934z6odVCdDpk9GSXRXR/zUw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR12MB9041 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Resent-Date: Wed, 13 Mar 2024 02:49:20 -0700 Reply-To: devel@edk2.groups.io,AbdulLateef.Attar@amd.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: mSHA11UJOuBlRishpO75ooRHx7686176AA= Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20240206 header.b=jvbCWikW; dmarc=pass (policy=none) header.from=groups.io; spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Thanks Pierre for the review, I'll address the review comments. Please see inline for my reply On 11-03-2024 19:46, Pierre Gondois wrote: > Caution: This message originated from an External Source. Use proper=20 > caution when opening attachments, clicking links, or responding. > > > Hello Abdul, > > On 3/4/24 16:43, Abdul Lateef Attar wrote: >> From: Abdul Lateef Attar >> >> 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 >> Cc: Pierre Gondois >> Signed-off-by: Abdul Lateef Attar ` >> --- >> =C2=A0 DynamicTablesPkg/DynamicTables.dsc.inc=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 |=C2=A0=C2=A0 2 + >> =C2=A0 DynamicTablesPkg/Include/AcpiTableGenerator.h |=C2=A0=C2=A0 2 + >> =C2=A0 .../Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf=C2=A0 |=C2=A0 36 +++ >> =C2=A0 .../Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c=C2=A0 | 266 ++++++++= ++++++++++ >> =C2=A0 4 files changed, 306 insertions(+) >> =C2=A0 create mode 100644=20 >> DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf >> =C2=A0 create mode 100644=20 >> DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c >> >> diff --git a/DynamicTablesPkg/DynamicTables.dsc.inc=20 >> b/DynamicTablesPkg/DynamicTables.dsc.inc >> index 477dc6b6a9..fc2ac5962e 100644 >> --- a/DynamicTablesPkg/DynamicTables.dsc.inc >> +++ b/DynamicTablesPkg/DynamicTables.dsc.inc >> @@ -36,6 +36,7 @@ >> =C2=A0=C2=A0=C2=A0 DynamicTablesPkg/Library/Acpi/AcpiFadtLib/AcpiFadtLib= .inf >> =C2=A0=C2=A0=C2=A0 DynamicTablesPkg/Library/Acpi/AcpiHpetLib/AcpiHpetLib= .inf >> =C2=A0=C2=A0=C2=A0 DynamicTablesPkg/Library/Acpi/AcpiWsmtLib/AcpiWsmtLib= .inf >> + DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf > > (note for later): > The HPET table seems to be intel specific actually, > >> >> =C2=A0 [Components.IA32, Components.X64] >> =C2=A0=C2=A0=C2=A0 # >> @@ -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 >> =C2=A0=C2=A0=C2=A0 } >> >> =C2=A0 [Components.ARM, Components.AARCH64] >> diff --git a/DynamicTablesPkg/Include/AcpiTableGenerator.h=20 >> b/DynamicTablesPkg/Include/AcpiTableGenerator.h >> index a32ef46ecb..ef651aa2aa 100644 >> --- a/DynamicTablesPkg/Include/AcpiTableGenerator.h >> +++ b/DynamicTablesPkg/Include/AcpiTableGenerator.h >> @@ -1,6 +1,7 @@ >> =C2=A0 /** @file >> >> =C2=A0=C2=A0=C2=A0 Copyright (c) 2017 - 2022, Arm Limited. All rights re= served.
>> +=C2=A0 Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reser= ved. >> >> =C2=A0=C2=A0=C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent >> >> @@ -101,6 +102,7 @@ typedef enum StdAcpiTableId { >> =C2=A0=C2=A0=C2=A0 EStdAcpiTableIdPcct,=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ///< PCCT Generator >> =C2=A0=C2=A0=C2=A0 EStdAcpiTableIdHpet,=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ///< HPET Generator >> =C2=A0=C2=A0=C2=A0 EStdAcpiTableIdWsmt,=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ///< WSMT Generator >> +=C2=A0 EStdAcpiTableIdSsdtHpet,=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 ///< SSDT HPET=20 >> Generator >> =C2=A0=C2=A0=C2=A0 EStdAcpiTableIdMax >> =C2=A0 } ESTD_ACPI_TABLE_ID; >> >> diff --git=20 >> a/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/AcpiSsdtHpetLib.inf=20 >> 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 >> +#=C2=A0 SSDT HPET Table Generator >> +# >> +#=C2=A0 Copyright (C) 2024 Advanced Micro Devices, Inc. All rights rese= rved. >> +# >> +#=C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent >> +## >> + >> +[Defines] >> +=C2=A0 INF_VERSION=C2=A0=C2=A0=C2=A0 =3D 1.27 >> +=C2=A0 BASE_NAME=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D AcpiSsdtHpetLib >> +=C2=A0 FILE_GUID=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 =3D 85262912-AD7F-4EE0-8= BB1-EE177275A54E >> +=C2=A0 VERSION_STRING =3D 1.0 >> +=C2=A0 MODULE_TYPE=C2=A0=C2=A0=C2=A0 =3D DXE_DRIVER >> +=C2=A0 LIBRARY_CLASS=C2=A0 =3D NULL|DXE_DRIVER >> +=C2=A0 CONSTRUCTOR=C2=A0=C2=A0=C2=A0 =3D AcpiSsdtHpetLibConstructor >> +=C2=A0 DESTRUCTOR=C2=A0=C2=A0=C2=A0=C2=A0 =3D AcpiSsdtHpetLibDestructor >> + >> +[Sources] >> +=C2=A0 SsdtHpetGenerator.c >> + >> +[Packages] >> +=C2=A0 DynamicTablesPkg/DynamicTablesPkg.dec >> +=C2=A0 MdeModulePkg/MdeModulePkg.dec >> +=C2=A0 MdePkg/MdePkg.dec >> +=C2=A0 PcAtChipsetPkg/PcAtChipsetPkg.dec >> + >> +[LibraryClasses] >> +=C2=A0 AcpiHelperLib >> +=C2=A0 AmlLib >> +=C2=A0 BaseLib >> +=C2=A0 DebugLib >> +=C2=A0 PcdLib >> + >> +[Pcd] >> +=C2=A0 gPcAtChipsetPkgTokenSpaceGuid.PcdHpetBaseAddress > > Cf. [1], I think this should be removed along the dependency over the > PcAtChipsetPkg dependency. > >> diff --git=20 >> a/DynamicTablesPkg/Library/Acpi/AcpiSsdtHpetLib/SsdtHpetGenerator.c=20 >> 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 >> +=C2=A0 SSDT HPET Table Generator >> + >> +=C2=A0 Copyright (c) 2017 - 2023, Arm Limited. All rights reserved. >> +=C2=A0 Copyright (C) 2024 Advanced Micro Devices, Inc. All rights reser= ved. >> + >> +=C2=A0 SPDX-License-Identifier: BSD-2-Clause-Patent >> + >> +=C2=A0 @par Reference(s): >> +=C2=A0 - ACPI 6.5 Specification, Aug 29, 2022 > > Is it possible to reference the HPET spec. with a link aswell if=20 > possible ? > Same comment for the other generators with their respective spec. > >> + >> +**/ >> + >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> +#include >> + >> +/** The Creator ID for the ACPI tables generated using >> +=C2=A0 the standard ACPI table generators. >> +*/ >> +#define TABLE_GENERATOR_CREATOR_ID_GENERIC=C2=A0 SIGNATURE_32('D', 'Y',= =20 >> 'N', 'T') >> + >> +/** This macro defines the HPET Table Generator revision. >> +*/ >> +#define HPET_GENERATOR_REVISION=C2=A0 CREATE_REVISION (1, 0) > > Is it possible to move this definition down, next to the=20 > ACPI_TABLE_GENERATOR > definition (just for consistency with other generators). > Same remark for the other generators added in this patchset. > >> + >> +#define SB_SCOPE=C2=A0 "\\_SB_" >> + >> +/** Construct the SSDT HPET devices Table. >> + >> +=C2=A0 This function invokes the Configuration Manager protocol interfa= ce >> +=C2=A0 to get the required hardware information for generating the ACPI >> +=C2=A0 table if required. >> + >> +=C2=A0 If this function allocates any resources then they must be freed >> +=C2=A0 in the FreeXXXXTableResources function. >> + >> +=C2=A0 @param [in]=C2=A0 This=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0 Pointer to the table generator. >> +=C2=A0 @param [in]=C2=A0 AcpiTableInfo=C2=A0 Pointer to the ACPI Table = Info. >> +=C2=A0 @param [in]=C2=A0 CfgMgrProtocol Pointer to the Configuration Ma= nager >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Protocol Interface. >> +=C2=A0 @param [out] Table=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 Pointer to the constructed ACPI Table. >> + >> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 Table generated successfully. >> +=C2=A0 @retval EFI_INVALID_PARAMETER A parameter is invalid. >> +=C2=A0 @retval EFI_NOT_FOUND=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 The required object was not found. >> +=C2=A0 @retval EFI_BAD_BUFFER_SIZE=C2=A0=C2=A0 The size returned by the= Configuration >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Manager is less than the Object = size=20 >> for the >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 requested object. >> +**/ >> +STATIC >> +EFI_STATUS >> +BuildSsdtHpetTable ( >> +=C2=A0 IN=C2=A0 CONST ACPI_TABLE_GENERATOR=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= *CONST=C2=A0 This, >> +=C2=A0 IN=C2=A0 CONST CM_STD_OBJ_ACPI_TABLE_INFO=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *CONST AcpiTableInfo, >> +=C2=A0 IN=C2=A0 CONST EDKII_CONFIGURATION_MANAGER_PROTOCOL=C2=A0 *CONST= =20 >> CfgMgrProtocol, >> +=C2=A0 OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EFI_ACPI_DESCRIPTION_HEA= DER=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 **CONST=C2=A0 Tab= le >> +=C2=A0 ) >> +{ >> +=C2=A0 EFI_STATUS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 Status; >> +=C2=A0 AML_ROOT_NODE_HANDLE=C2=A0=C2=A0=C2=A0 RootNode; >> +=C2=A0 AML_OBJECT_NODE_HANDLE=C2=A0 ScopeNode; >> +=C2=A0 AML_OBJECT_NODE_HANDLE=C2=A0 HpetNode; >> +=C2=A0 AML_OBJECT_NODE_HANDLE=C2=A0 CrsNode; >> +=C2=A0 UINT32=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EisaId; >> + >> +=C2=A0 ASSERT (This !=3D NULL); >> +=C2=A0 ASSERT (AcpiTableInfo !=3D NULL); >> +=C2=A0 ASSERT (CfgMgrProtocol !=3D NULL); >> +=C2=A0 ASSERT (Table !=3D NULL); >> +=C2=A0 ASSERT (AcpiTableInfo->TableGeneratorId =3D=3D This->GeneratorID= ); >> +=C2=A0 ASSERT (AcpiTableInfo->AcpiTableSignature =3D=3D=20 >> This->AcpiTableSignature); >> + >> +=C2=A0 Status =3D AddSsdtAcpiHeader ( >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 CfgMgrProtocol, >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 This, >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 AcpiTableInfo, >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 &RootNode >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 ); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 return Status; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenScope (SB_SCOPE, RootNode, &ScopeNode); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenDevice ("HPET", ScopeNode, &HpetNode); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlGetEisaIdFromString ("PNP0103", &EisaId); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenNameInteger ("_HID", EisaId, HpetNode, NULL= ); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenNameInteger ("_UID", 0x00, HpetNode, NULL); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenNameResourceTemplate ("_CRS", HpetNode, &Cr= sNode); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlCodeGenRdMemory32Fixed (FALSE, PcdGet32=20 >> (PcdHpetBaseAddress), SIZE_1KB, CrsNode, NULL); > > I didn't find anything in the spec. stating the memory range should be=20 > read only or > read-write. There are examples for both in edk2-platforms. Did you see=20 > something > specific going in the way of a read only configuration ? > [Abdul], Yes I also didnt find any information. I think we can go with=20 read only configuration which is safe. > --- > > [1] > > - to avoid making the DynamicTablesPkg dependent over the=20 > PcAtChipsetPkg.dec > =C2=A0=C2=A0=C2=A0 package (not that it is explicitly forbidden). > - to avoid making the DynamicTablesPkg fetch configuration information=20 > from > =C2=A0=C2=A0=C2=A0 PCDs instead of CmObjects (there is a counter example,= but this=20 > should be avoided > =C2=A0=C2=A0=C2=A0 I think). > would it be possible to create a HPET object ? The base address would=20 > 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: > =C2=A0=C2=A0 PcdGet32 (PcdHpetBaseAddress) > > >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +=C2=A0 Status =3D AmlSerializeDefinitionBlock ( >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 RootNode, >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 Table >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0 ); >> +=C2=A0 if (EFI_ERROR (Status)) { >> +=C2=A0=C2=A0=C2=A0 DEBUG (( >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 DEBUG_ERROR, >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 "ERROR: SSDT-HPET: Failed to Serialize S= SDT Table Data." >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 " Status =3D %r\n", >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Status >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 )); >> +=C2=A0=C2=A0=C2=A0 goto exit_handler; >> +=C2=A0 } >> + >> +exit_handler: >> +=C2=A0 // Delete the RootNode and its attached children. >> +=C2=A0 return AmlDeleteTree (RootNode); >> +} >> + >> +/** Free any resources allocated for constructing the >> +=C2=A0=C2=A0=C2=A0 SSDT HPET ACPI table. >> + >> +=C2=A0 @param [in]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 This=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Pointer to the table generator. >> +=C2=A0 @param [in]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 AcpiTableInfo=C2=A0 Po= inter to the ACPI Table Info. >> +=C2=A0 @param [in]=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CfgMgrProtocol Pointer= to the Configuration Manager >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Protocol Interface. >> +=C2=A0 @param [in, out] Table=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0 Pointer to the ACPI Table. >> + >> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 The resources were freed successfully. >> +=C2=A0 @retval EFI_INVALID_PARAMETER The table pointer is NULL or inval= id. >> +**/ >> +STATIC >> +EFI_STATUS >> +FreeSsdtHpetTableResources ( >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CONST ACPI_TABLE_GENERATOR=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0 *CONST This, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CONST CM_STD_OBJ_ACPI_TABLE_INF= O=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 *CONST= =20 >> AcpiTableInfo, >> +=C2=A0 IN=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 CONST EDKII_CONFIGURATION_MANAG= ER_PROTOCOL=C2=A0 *CONST=20 >> CfgMgrProtocol, >> +=C2=A0 IN OUT=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 EFI_ACPI_DESCRI= PTION_HEADER=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 **CONST = Table >> +=C2=A0 ) >> +{ >> +=C2=A0 ASSERT (This !=3D NULL); >> +=C2=A0 ASSERT (AcpiTableInfo !=3D NULL); >> +=C2=A0 ASSERT (CfgMgrProtocol !=3D NULL); >> +=C2=A0 ASSERT (AcpiTableInfo->TableGeneratorId =3D=3D This->GeneratorID= ); >> +=C2=A0 ASSERT (AcpiTableInfo->AcpiTableSignature =3D=3D=20 >> This->AcpiTableSignature); >> + >> +=C2=A0 if ((Table =3D=3D NULL) || (*Table =3D=3D NULL)) { >> +=C2=A0=C2=A0=C2=A0 DEBUG ((DEBUG_ERROR, "ERROR: SSDT-HPET: Invalid Tabl= e Pointer\n")); >> +=C2=A0=C2=A0=C2=A0 ASSERT ((Table !=3D NULL) && (*Table !=3D NULL)); >> +=C2=A0=C2=A0=C2=A0 return EFI_INVALID_PARAMETER; >> +=C2=A0 } >> + >> +=C2=A0 FreePool (*Table); >> +=C2=A0 *Table =3D NULL; >> +=C2=A0 return EFI_SUCCESS; >> +} >> + >> +/** The interface for the SSDT HPET Table Generator. >> +*/ >> +STATIC >> +CONST >> +ACPI_TABLE_GENERATOR=C2=A0 mSsdtPlatHpetGenerator =3D { >> +=C2=A0 // Generator ID >> +=C2=A0 CREATE_STD_ACPI_TABLE_GEN_ID (EStdAcpiTableIdSsdtHpet), >> +=C2=A0 // Generator Description >> +=C2=A0 L"ACPI.STD.SSDT.HPET.GENERATOR", >> +=C2=A0 // ACPI Table Signature >> +=C2=A0 EFI_ACPI_6_5_SECONDARY_SYSTEM_DESCRIPTION_TABLE_SIGNATURE, >> +=C2=A0 // ACPI Table Revision - Unused >> +=C2=A0 0, >> +=C2=A0 // Minimum ACPI Table Revision - Unused >> +=C2=A0 0, >> +=C2=A0 // Creator ID >> +=C2=A0 TABLE_GENERATOR_CREATOR_ID_GENERIC, > > @Sami, shouldn't all the generators use this Creator Id instead of=20 > 'ARMH' ? > If yes the definition of: > =C2=A0=C2=A0=C2=A0 TABLE_GENERATOR_CREATOR_ID_GENERIC > should be placed there I think: > =C2=A0=C2=A0=C2=A0 DynamicTablesPkg/Include/AcpiTableGenerator.h > >> +=C2=A0 // Creator Revision >> +=C2=A0 HPET_GENERATOR_REVISION, >> +=C2=A0 // Build Table function >> +=C2=A0 BuildSsdtHpetTable, >> +=C2=A0 // Free Resource function >> +=C2=A0 FreeSsdtHpetTableResources, >> +=C2=A0 // FreeSsdtPlatDevicesTableResources, > > Should be removed I think. > >> +=C2=A0 // Extended build function not needed >> +=C2=A0 NULL, >> +=C2=A0 // Extended build function not implemented by the generator. >> +=C2=A0 // Hence extended free resource function is not required. >> +=C2=A0 NULL >> +}; >> + >> +/** Register the Generator with the ACPI Table Factory. >> + >> +=C2=A0 @param [in]=C2=A0 ImageHandle=C2=A0 The handle to the image. >> +=C2=A0 @param [in]=C2=A0 SystemTable=C2=A0 Pointer to the System Table. >> + >> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 The Generator is registered. >> +=C2=A0 @retval EFI_INVALID_PARAMETER A parameter is invalid. >> +=C2=A0 @retval EFI_ALREADY_STARTED=C2=A0=C2=A0 The Generator for the Ta= ble ID >> +=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 is already registered. >> +**/ >> +EFI_STATUS >> +EFIAPI >> +AcpiSsdtHpetLibConstructor ( >> +=C2=A0 IN=C2=A0 EFI_HANDLE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Im= ageHandle, >> +=C2=A0 IN=C2=A0 EFI_SYSTEM_TABLE=C2=A0 *SystemTable >> +=C2=A0 ) >> +{ >> +=C2=A0 EFI_STATUS=C2=A0 Status; >> + >> +=C2=A0 Status =3D RegisterAcpiTableGenerator (&mSsdtPlatHpetGenerator); >> +=C2=A0 DEBUG ((DEBUG_INFO, "SSDT-HPET: Register Generator. Status =3D= =20 >> %r\n", Status)); >> +=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0 return Status; >> +} >> + >> +/** Deregister the Generator from the ACPI Table Factory. >> + >> +=C2=A0 @param [in]=C2=A0 ImageHandle=C2=A0 The handle to the image. >> +=C2=A0 @param [in]=C2=A0 SystemTable=C2=A0 Pointer to the System Table. >> + >> +=C2=A0 @retval EFI_SUCCESS=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 The Generator is deregistered. >> +=C2=A0 @retval EFI_INVALID_PARAMETER A parameter is invalid. >> +=C2=A0 @retval EFI_NOT_FOUND=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0 The Generator is not registered. >> +**/ >> +EFI_STATUS >> +EFIAPI >> +AcpiSsdtHpetLibDestructor ( >> +=C2=A0 IN=C2=A0 EFI_HANDLE=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 Im= ageHandle, >> +=C2=A0 IN=C2=A0 EFI_SYSTEM_TABLE=C2=A0 *SystemTable >> +=C2=A0 ) >> +{ >> +=C2=A0 EFI_STATUS=C2=A0 Status; >> + >> +=C2=A0 Status =3D DeregisterAcpiTableGenerator (&mSsdtPlatHpetGenerator= ); >> +=C2=A0 DEBUG ((DEBUG_INFO, "SSDT-HPET: Deregister Generator. Status =3D= =20 >> %r\n", Status)); >> +=C2=A0 ASSERT_EFI_ERROR (Status); >> +=C2=A0 return Status; >> +} > > > Regards, > Pierre -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#116712): https://edk2.groups.io/g/devel/message/116712 Mute This Topic: https://groups.io/mt/104724534/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-