From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from EUR04-HE1-obe.outbound.protection.outlook.com (EUR04-HE1-obe.outbound.protection.outlook.com [40.107.7.52]) by mx.groups.io with SMTP id smtpd.web11.19144.1627908631759546461 for ; Mon, 02 Aug 2021 05:50:32 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@armh.onmicrosoft.com header.s=selector2-armh-onmicrosoft-com header.b=3qIP21xf; spf=pass (domain: arm.com, ip: 40.107.7.52, mailfrom: sami.mujawar@arm.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=BpriCFZFJknTpA8hoG2AxkNZGAHPEqderyf+X+uyhlk=; b=3qIP21xfxGhZnPKyGztYJVDA118VlDgykgRVaV8sDRHB6D9ISgLifE2af9yhUH8YG3uw34pjPvq+sS8/bXgHGwvC3BawiIK6GSGzAzHWNVdjdEnmhLClO5VngxPyFW672ST4JjSCjemHyXPO3CvBLKYAAhPe/rKB/z4WpNN7moI= Received: from AM6P192CA0036.EURP192.PROD.OUTLOOK.COM (2603:10a6:209:83::49) by AM0PR08MB5523.eurprd08.prod.outlook.com (2603:10a6:208:17f::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.18; Mon, 2 Aug 2021 12:50:28 +0000 Received: from AM5EUR03FT051.eop-EUR03.prod.protection.outlook.com (2603:10a6:209:83:cafe::fa) by AM6P192CA0036.outlook.office365.com (2603:10a6:209:83::49) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.18 via Frontend Transport; Mon, 2 Aug 2021 12:50:28 +0000 X-MS-Exchange-Authentication-Results: spf=pass (sender IP is 63.35.35.123) smtp.mailfrom=arm.com; edk2.groups.io; dkim=pass (signature was verified) header.d=armh.onmicrosoft.com;edk2.groups.io; dmarc=pass action=none header.from=arm.com; Received-SPF: Pass (protection.outlook.com: domain of arm.com designates 63.35.35.123 as permitted sender) receiver=protection.outlook.com; client-ip=63.35.35.123; helo=64aa7808-outbound-1.mta.getcheckrecipient.com; Received: from 64aa7808-outbound-1.mta.getcheckrecipient.com (63.35.35.123) by AM5EUR03FT051.mail.protection.outlook.com (10.152.16.246) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.18 via Frontend Transport; Mon, 2 Aug 2021 12:50:28 +0000 Received: ("Tessian outbound 8529ae990a93:v101"); Mon, 02 Aug 2021 12:50:28 +0000 X-CheckRecipientChecked: true X-CR-MTA-CID: 089acfd58c3a3c82 X-CR-MTA-TID: 64aa7808 Received: from ce9fedbe06e3.1 by 64aa7808-outbound-1.mta.getcheckrecipient.com id A6772A67-D17C-4DF2-9505-D06E28AEE45E.1; Mon, 02 Aug 2021 12:50:18 +0000 Received: from EUR01-DB5-obe.outbound.protection.outlook.com by 64aa7808-outbound-1.mta.getcheckrecipient.com with ESMTPS id ce9fedbe06e3.1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384); Mon, 02 Aug 2021 12:50:18 +0000 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=OtiuqPPjfiWRsx5VjP4rgyWtNFn3ikJxtkF6fQEL3XI9CcLrEvoR83f5kqmx7DncXVu8xYiQaZ+TwawJH8QOVrtPoIq6d2Grf5pcd7z8AgfugMhUn0TqosTZA8OS6PsgCTo/0wrxe0k+KbAFjA21LLcg1PXGjCqezM0Bo9M+rzWTN/Gt25etCbd7xAe3NijfMDwMxk1Ou1IVNAqteHA3WUCdeGo0Y94z80FdEs0A0AT83jEf4OIJsyalddsf8QQbla4R9g8HGFE8rqFoX4smnD2MSvl1/Bs4jv9f17WEiQ25aQeshtvZ65p75aaYyw55teI/U2+5eIPtvxjlQmS/Vw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=BpriCFZFJknTpA8hoG2AxkNZGAHPEqderyf+X+uyhlk=; b=WwVAsVsrAPub8mXlXKVsi026Ju0Bg0md9i1ZQls+kOFOZrMZE5N8uG0HuD8i1tXONFltSnQ3EmGRnbN0JNYP4qSc8tc/wPVdAi6IfJ/tgb41h8K8cOsl8EfxizykaIJQlWnJmEW78ICF1G9HVcZP1dfXHod3ruEMSjFoWPD0Tfm1fiIwWZSAnFHnnuU/sadNSrIY7W3+z2cZxoN1HOL212Q/EdMk15JSeTFwkfJYFSNjFRkzU5WMU4798KJCXleyPDJy9lW080rOH6JLdbZoygO9Fi/M+e/t/DhdWPYPYT2V/z6m+TmjPbD3ewb9EVwcJwejDXyubImalwXGSA/m0Q== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=armh.onmicrosoft.com; s=selector2-armh-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=BpriCFZFJknTpA8hoG2AxkNZGAHPEqderyf+X+uyhlk=; b=3qIP21xfxGhZnPKyGztYJVDA118VlDgykgRVaV8sDRHB6D9ISgLifE2af9yhUH8YG3uw34pjPvq+sS8/bXgHGwvC3BawiIK6GSGzAzHWNVdjdEnmhLClO5VngxPyFW672ST4JjSCjemHyXPO3CvBLKYAAhPe/rKB/z4WpNN7moI= Authentication-Results-Original: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Received: from AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) by AM6PR08MB3861.eurprd08.prod.outlook.com (2603:10a6:20b:80::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.23; Mon, 2 Aug 2021 12:50:16 +0000 Received: from AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::ca6:6171:e2f1:3d40]) by AS8PR08MB6806.eurprd08.prod.outlook.com ([fe80::ca6:6171:e2f1:3d40%4]) with mapi id 15.20.4373.026; Mon, 2 Aug 2021 12:50:16 +0000 From: "Sami Mujawar" Subject: Re: [PATCH v2 1/4] ArmPlatformPkg: Allow dynamic generation of HEST ACPI table To: Omkar Anand Kulkarni , devel@edk2.groups.io Cc: Ard Biesheuvel , nd References: <20210710161831.30433-1-omkar.kulkarni@arm.com> <20210710161831.30433-2-omkar.kulkarni@arm.com> Message-ID: <59009d52-3d49-29bd-3a47-91536d569004@arm.com> Date: Mon, 2 Aug 2021 13:50:15 +0100 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.0.1 In-Reply-To: <20210710161831.30433-2-omkar.kulkarni@arm.com> X-ClientProxiedBy: LO2P265CA0174.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a::18) To AS8PR08MB6806.eurprd08.prod.outlook.com (2603:10a6:20b:39b::12) MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from [10.1.196.43] (217.140.106.54) by LO2P265CA0174.GBRP265.PROD.OUTLOOK.COM (2603:10a6:600:a::18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4373.18 via Frontend Transport; Mon, 2 Aug 2021 12:50:15 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: cef5d067-9b53-4f70-ec23-08d955b41ae5 X-MS-TrafficTypeDiagnostic: AM6PR08MB3861:|AM0PR08MB5523: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: x-checkrecipientrouted: true NoDisclaimer: true X-MS-Oob-TLC-OOBClassifiers: OLM:10000;OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-MS-Exchange-AntiSpam-Relay: 0 X-Microsoft-Antispam-Untrusted: BCL:0; X-Microsoft-Antispam-Message-Info-Original: YWier1KJ2SspGY2weZ26NIWxb4Ray/AZzigf5452I+P5Za5ZQz3tl39dPxtK20QTy4xN/QgFfiaYVZmPRQFfBZJGwyt/mTs/3ze/sAfhsEyPwMmcmgr+NGfoa0uwdfJvySWmk4Q9nLWg5kOiVX2vUYtxYKUOsZ4qBAkoxNDKfuEQe8H/4GaWnnxjd0QaWyrevUE8lURRr5DFSfldgVeMSYPpt6yJsqdw4kB2p7YAFBeUdODBLpSqBS0quHd9uUAMErOKXGmtcu1OvMV57maTqcI/XwIF9wmfGtklFSNBpKQKaF1y6sG6KpIh9TI+bJCi4jFubNtJehM6Hg3nuYHV8MvYLDxKpONqHcc65sFoqIGqpnVLFaKbRsbVYyUCjhCZBq+5eaWLbWsY+VTF7u06YkQuxziYcfvW+Lv6ZbD3bhQmbGjcGc1mvnx7z709vxHN33+QS42Dsb/0JoUvvRfgQnijZtoBi58Z/nUJiggEvGQo7bSFp8iRn3kkt9KHMDR/w+ioiDtgNi+YsEZt5+qh1qnJdUyWoeBb8s4l6R3uI3wgovjL8qL1uHHGgkUBuWgJfeySJ3qFkwKztWIcHJi3tLHrwVnZAsH6VLIwLYKZ+9k0ypNPimzuGPyFibpkF3S+VHu3kQ9lTMPtH00+Sq/AYtxbp2boo3F01IUweHkRBpcj3loHdJMBy/rqSpWsHI3Ilt2LtV7Yk0doGLN8raPfLjOnZfa5p7YnkWvHO9wosQ5+FWRcroh+pv0qlNiBdQpWO8vGsD+zf147VkDoVDkM8w== X-Forefront-Antispam-Report-Untrusted: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:AS8PR08MB6806.eurprd08.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(366004)(44832011)(2616005)(26005)(2906002)(5660300002)(956004)(31696002)(30864003)(52116002)(19627235002)(186003)(31686004)(6486002)(53546011)(33964004)(38350700002)(66946007)(8936002)(4326008)(8676002)(83380400001)(316002)(86362001)(36756003)(508600001)(38100700002)(54906003)(66476007)(16576012)(66556008)(43740500002)(45980500001)(579004)(559001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1 X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UG1QVWNsRUtLSkdTcitiSmtXZVo1VUY0MTRISjlmck5MTUR4RzdoQkRPYlRM?= =?utf-8?B?a3VzSHdEOC9pWjBUdHVHOXJDTWJ1aHM0djZEeS96Z1ZuY2NubXJuOFl0cDE4?= =?utf-8?B?NnRjRzQvMzFjZG80cC9FN09BQWJZTEppT1IrS3RscUpaaStxVEZRZXdoZThM?= =?utf-8?B?UFdFU3gwU3FQMmJGaktrbHRTRlZ5R2tPRnc3Zmo0S29hZUg3SXk4MExNdmVp?= =?utf-8?B?bHovT2s1WUZ0b3VoUkNMWDZ3dko5dWI5OWp3M0tIdmVKeklsV05YRHVuN3d0?= =?utf-8?B?SFdBNkVwekVnR2k1ZUdUVGYwcDVTUlpUY3lqT0Ewem9WSTZlNElLQXJqcmhC?= =?utf-8?B?aWNUMmV6MTF6TlJMNEVhMklranU5WTBDRUVEQXlDODR4MkdWUUF2OGtZeTFB?= =?utf-8?B?WmlXb1dRamRWTE9sT0dHb3BiSVFXQmVPbnRKdm96VFE0S2VhVFlvUk9tYm5u?= =?utf-8?B?SC9CZU1QL0l2Z0NyN3R4QUw4c3NIK2FpbE01amRIS216YkhQakhMKzRKd2ZN?= =?utf-8?B?eCtsOHZ4VDlWbmFwTzF0NkpyZnNqMUZuL015amJVVHlydlVFRHFtclY4NXE4?= =?utf-8?B?dE9ERjNYeXN6Y1JaTi9xTjlXWlZla0FKbWUyZXlYeDBiTHpqdXdsVHJJbXVu?= =?utf-8?B?SWFuMytVNDVZcE5CQ3lLeGlDK3ZzWmsrZXFqWjczczViczNCQm5hSGFIaEQ3?= =?utf-8?B?WlVYdnFyc21mMVFDWFovbGhrSllCODNTMlhSaUR1RnBSRHl6cEVXRllSN0hq?= =?utf-8?B?dXlGY1pMV0xlT0FpNG8vQWdPcDNweWFYWWhwdjIrR0VlZWM2M2llVHZxSXY1?= =?utf-8?B?MTRyaG1QY2tZRUFwMkJOZWFkck5EM0ZYZzFhZjhiU1ozN1RLT3VQZVlKQmZU?= =?utf-8?B?ejl5TithdDhlRnNMU2NRWmo3RjNnSm5lTDlHS1I2SXdQWVE4NjlQQm1JVjk3?= =?utf-8?B?a1YwaW5odVhtUXg3YnN6ZkNVTmFSMEhhd1BkMUhmY3d0UnhLN09sUXY3Vndh?= =?utf-8?B?VHkvSGxvbUlzYktBOUNDSzhPS2UvTUQrcEw0SUtpZjZ6Z0ljalB3UkJOeDdL?= =?utf-8?B?QU43MkpLWlVXaWRRQzBGZHFzYkdTTGg2bGJ0OVF3Z2dOc1N3SjdhM053YmdG?= =?utf-8?B?ZFJmQjdFK1dmMTgzRUk3MUVjWnIzNE5hc2x6cTVhaUpaS2RaQ1M3VjcwWmJH?= =?utf-8?B?Q0YwQzZFVmV3aW4xYU85cjU4cU5uODZDNHBEY0VITVlzWHc0WkFkL2cwNDdV?= =?utf-8?B?eThFWFlCWDVvZURFeTlJMkNxc1VyUFcxTlZUcTVGRlNqekJVTTBHR0l0RzJY?= =?utf-8?B?Rm5yelBFalM1K1FWcGFOS3lqRzFjY1BiNU01NTFzSkhFUVV3RTZYdEEra2lU?= =?utf-8?B?TnduN0svc3JCRTJHMXBIL2h3V3htOVRYU1RvOGRzcHM4V0Znbm9OT0p5akJS?= =?utf-8?B?aUZ5Uk1TdllSYXJSWXZYWmJtSlVpZ1N6cWw2UHAyUTZPZDlML0Z4T0FncGpE?= =?utf-8?B?d0R1RWRWNWp4UE50NlhGNUwxc2lZODBNUjJSdkgrbkZ3eFhyRjFra2VNbW83?= =?utf-8?B?VU9nRUtJN0xWcU84cUhidng1emRBOTZySXh4V01pRWd4QzZQbStob01zaHR2?= =?utf-8?B?dzBDcDNlWk1Id1ZWUDE2T1FqSE5wa3BSb09RTkg1TGY5ZFRsd2crdXQxSUVN?= =?utf-8?B?aThwdnl6ajRzS0gyeml6S0FWbFY3VzY4bERueW1kUTVDYnFxcWxDZTFzdWxH?= =?utf-8?Q?Xf/9UelDGlCQyF1kcrmsBeohABqppgo1uOkPYGL?= X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM6PR08MB3861 Original-Authentication-Results: arm.com; dkim=none (message not signed) header.d=none;arm.com; dmarc=none action=none header.from=arm.com; Return-Path: Sami.Mujawar@arm.com X-EOPAttributedMessage: 0 X-MS-Exchange-Transport-CrossTenantHeadersStripped: AM5EUR03FT051.eop-EUR03.prod.protection.outlook.com X-MS-Office365-Filtering-Correlation-Id-Prvs: c3292f16-d916-4f88-02af-08d955b4138c X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: CO4ewpzARy6mJHUJi/s7S7SObPVLEGPwU25XbElonMt+kdEGEXVQ1aevmEbH1VRdN1+uZpJZy2etfKfSrVzKGbuRG0LW0qjh7diPWARUKHQOiVvOBEFp6vThEibXzvRxSXEQnOQ7DB79/2o2hotDG/DgenTIYsDbakZxQSr44TegTzPucWSYfosq6Zz1QP033uZqMJ747jeBPKYyUlmuDdgshDbojrgWnLWrYaCO39BtG8Sk4hQKXFWqv+93I0HWZGW7XDaMqvLZHv6RA+oUgpHpbVlBS8SlaAg4XkgN+OQ2W0Ard9M1hzft/UgpTWOSeAxCsKEhEbEd3uZnSWv3isE+dXnYJoK4ewbXyuAk+micNmz6SvlAMqZ9FwvdgPJ3pz3yzQw+Vge2a/XO5mnQNX9o0XfZ7ejHXs8MbrXvpa+H3bUbBwvMxTF3ZDfEdGq1DWmADwYPg3WbYybz8g/Ta/4ERn5AA2OTc6yRkfnK2Uqao30vs8iyLRG2Adsic+AUkj6WGHa/naIAbzXdsQLbLRn5SH7owFEZEjwgiuC1d3c1KxzTVR6/Beb68d/+RLRqbh1eVbGzJo6n1Ir/csZ694CyhRgjSu4UOetvEvlDIvKztpeM+KyqkwLGAXvTPrOTegd+bR4gmkAMT4SV2BJs3xMp/2eiZlN5RRin3EFG1pk/m0bsOHxKq9o996yXCRJ1W7mWNfEapKMsXG8D1Qub4lCGfnl5CLCKdx1q5l9Ch1w= X-Forefront-Antispam-Report: CIP:63.35.35.123;CTRY:IE;LANG:en;SCL:1;SRV:;IPV:CAL;SFV:NSPM;H:64aa7808-outbound-1.mta.getcheckrecipient.com;PTR:ec2-63-35-35-123.eu-west-1.compute.amazonaws.com;CAT:NONE;SFS:(4636009)(376002)(396003)(346002)(136003)(39860400002)(36840700001)(46966006)(478600001)(82740400003)(316002)(186003)(26005)(36860700001)(47076005)(54906003)(2906002)(31686004)(16576012)(956004)(30864003)(31696002)(2616005)(83380400001)(70586007)(70206006)(33964004)(36756003)(44832011)(53546011)(82310400003)(6486002)(86362001)(4326008)(356005)(5660300002)(336012)(8936002)(81166007)(19627235002)(8676002)(43740500002)(559001)(579004);DIR:OUT;SFP:1101; X-OriginatorOrg: arm.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Aug 2021 12:50:28.2706 (UTC) X-MS-Exchange-CrossTenant-Network-Message-Id: cef5d067-9b53-4f70-ec23-08d955b41ae5 X-MS-Exchange-CrossTenant-Id: f34e5979-57d9-4aaa-ad4d-b122a662184d X-MS-Exchange-CrossTenant-OriginalAttributedTenantConnectingIp: TenantId=f34e5979-57d9-4aaa-ad4d-b122a662184d;Ip=[63.35.35.123];Helo=[64aa7808-outbound-1.mta.getcheckrecipient.com] X-MS-Exchange-CrossTenant-AuthSource: AM5EUR03FT051.eop-EUR03.prod.protection.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Anonymous X-MS-Exchange-CrossTenant-FromEntityHeader: HybridOnPrem X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR08MB5523 Content-Type: multipart/alternative; boundary="------------4609D3691EC72F45D4062A88" Content-Language: en-GB --------------4609D3691EC72F45D4062A88 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Hi Omkar, Please find my response marked inline as [SAMI]. Regards, Sami Mujawar On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote: > Introduce the HEST table generation protocol that allows platforms to > build the table with multiple error source descriptors and install the > table. The protocol provides two interfaces. The first interface allows > for adding multiple error source descriptors into the HEST table. The > second interface can then be used to dynamically install the fully > populated HEST table. This allows multiple drivers and/or libraries to > dynamically register error source descriptors into the HEST table. > > Co-authored-by: Thomas Abraham > Signed-off-by: Omkar Anand Kulkarni > --- > ArmPlatformPkg/ArmPlatformPkg.dec | 4 + > ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf | 49 +++ > ArmPlatformPkg/Include/Protocol/HestTable.h | 71 ++++ > ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c | 354 ++++++++++++++++++++ > 4 files changed, 478 insertions(+) > > diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec > index 3a25ddcdc8ca..e4afe5da8e11 100644 > --- a/ArmPlatformPkg/ArmPlatformPkg.dec > +++ b/ArmPlatformPkg/ArmPlatformPkg.dec > @@ -127,3 +127,7 @@ > gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022 > > gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033 > + > +[Protocols.common] > + ## Arm Platform HEST table generation protocol > + gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } } > diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf > new file mode 100644 > index 000000000000..91c7385bf7ff > --- /dev/null > +++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf > @@ -0,0 +1,49 @@ > +## @file > +# Dxe driver that creates and publishes the HEST table. > +# > +# This driver creates HEST header and provides protocol service to append > +# and install the HEST table. > +# > +# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved. > +# SPDX-License-Identifier: BSD-2-Clause-Patent > +# > +## > + > +[Defines] > + INF_VERSION = 0x0001001A > + BASE_NAME = HestDxe > + FILE_GUID = 933099a2-ef71-4e00-82aa-a79b1e0a3b38 > + MODULE_TYPE = DXE_DRIVER > + VERSION_STRING = 1.0 > + ENTRY_POINT = HestInitialize > + > +[Sources.Common] > + HestDxe.c > + > +[Packages] > + ArmPkg/ArmPkg.dec > + ArmPlatformPkg/ArmPlatformPkg.dec > + MdeModulePkg/MdeModulePkg.dec > + MdePkg/MdePkg.dec > + Platform/ARM/SgiPkg/SgiPlatform.dec > + > +[LibraryClasses] > + ArmLib > + BaseMemoryLib > + DebugLib > + UefiDriverEntryPoint > + UefiLib > + > +[Protocols] > + gEfiAcpiTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED > + gHestTableProtocolGuid ## PRODUCES > + > +[FixedPcd] > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision > + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId > + > +[Depex] > + TRUE > diff --git a/ArmPlatformPkg/Include/Protocol/HestTable.h b/ArmPlatformPkg/Include/Protocol/HestTable.h > new file mode 100644 > index 000000000000..3b2e1f7d9203 > --- /dev/null > +++ b/ArmPlatformPkg/Include/Protocol/HestTable.h > @@ -0,0 +1,71 @@ > +/** @file > + Builds and installs the HEST ACPI table. > + > + Define the protocol interface that allows HEST ACPI table to be created, > + populated with error record descriptions and installation of the HEST ACPI > + table. > + > + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved. > + SPDX-License-Identifier: BSD-2-Clause-Patent > +**/ > + > +#ifndef HEST_TABLE_H_ > +#define HEST_TABLE_H_ > + > +#define HEST_TABLE_PROTOCOL_GUID \ > + { \ > + 0x705bdcd9, 0x8c47, 0x457e, \ > + { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } \ > + } > + > +/** > + Append HEST error source descriptor protocol service. > + > + Protocol service used to append newly collected error source descriptors to > + to an already created HEST table. > + > + @param[in] ErrorSourceDescriptorList List of Error Source Descriptors. > + @param[in] ErrorSourceDescriptorListSize Total Size of Error Source > + Descriptors. > + @param[in] ErrorSourceDescriptorCount Total count of error source > + descriptors. > + > + @retval EFI_SUCCESS Appending the error source descriptors > + successful. > + @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hest > + table. > + @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param or > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *APPEND_ERROR_SOURCE_DESCRIPTOR) ( > + IN CONST VOID *ErrorSourceDescriptorList, > + IN UINTN ErrorSourceDescriptorListSize, > + IN UINTN ErrorSourceDescriptorCount > + ); > + > +/** > + Install HEST table protocol service. > + > + Installs the HEST table that has been appended with the error source > + descriptors. The checksum of this table is calculated and updated in > + the table header. The HEST table is then installed. > + > + @retval EFI_SUCCESS HEST table is installed successfully. > + @retval EFI_ABORTED HEST table is NULL. > + @retval Other Install service call failed. > +**/ > +typedef > +EFI_STATUS > +(EFIAPI *INSTALL_HEST_TABLE) (VOID); > + > +// > +// HEST_TABLE_PROTOCOL enables creation and installation of HEST table > +// > +typedef struct { > + APPEND_ERROR_SOURCE_DESCRIPTOR AppendErrorSourceDescriptors; > + INSTALL_HEST_TABLE InstallHestTable; > +} HEST_TABLE_PROTOCOL; > + > +extern EFI_GUID gHestTableProtocolGuid; > +#endif // HEST_TABLE_H_ > diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c > new file mode 100644 > index 000000000000..b68e1a0c4e48 > --- /dev/null > +++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c > @@ -0,0 +1,354 @@ > +/** @file > + Builds and installs the HEST ACPI table. > + > + This driver installs a protocol that can be used to create and install a HEST > + ACPI table. The protocol allows one or more error source producers to add the > + error source descriptors into the HEST table. It also allows the resulting > + HEST table to be installed. > + > + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved. > + SPDX-License-Identifier: BSD-2-Clause-Patent > + > + @par Specification Reference: > + - ACPI 6.3, Table 18-382, Hardware Error Source Table > +**/ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +typedef struct { > + VOID *HestTable; /// Pointer to HEST table. > + UINT32 CurrentTableSize; /// Current size of HEST table. > +} HEST_DXE_DRIVER_DATA; > + > +STATIC EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol = NULL; > +STATIC HEST_DXE_DRIVER_DATA mHestDriverData; > + > +/** > + Helper function to the driver. > + > + Function that reallocates memory for every new error source descriptor info > + added. > + > + @param[in] OldTableSize Old memory pool size. > + @param[in] NewTableSize Required memory pool size. > + @param[in, out] OldBuffer Current memory buffer pointer holding the Hest > + table data. > + > + @return New pointer to reallocated memory pool. > +**/ > +STATIC > +VOID* > +ReallocateHestTableMemory ( > + IN UINT32 OldTableSize, > + IN UINT32 NewTableSize, > + IN OUT VOID *OldBuffer > + ) > +{ [SAMI] Have you considered maintaining a linked list of the error sources and serialising the list once InstallHestTable() is called? > + return ReallocateReservedPool ( > + OldTableSize, > + NewTableSize, > + OldBuffer > + ); [SAMI] Is this wrapper function required? Can ReallocateReservedPool() be used directly instead? > +} > + > +/** > + Allocate memory for the HEST table header and populate it. > + > + Helper function for this driver, populates the HEST table header. Called once > + in the beginning on first invocation of append error source descriptor > + protocol service. > + > + @retval EFI_SUCCESS On successful creation of HEST header. > + @retval EFI_OUT_OF_RESOURCES If system is out of memory recources. > +**/ > +STATIC > +EFI_STATUS > +BuildHestHeader (VOID) [SAMI] VOID and closing parenthesis should be on a separate line. > +{ > + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER HestHeader; > + UINT64 TempOemTableId; > + > + // > + // Allocate memory for the HEST table header. > + // > + mHestDriverData.HestTable = > + ReallocateHestTableMemory ( > + 0, > + sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER), > + NULL > + ); [SAMI] Is the Relocate version of the function required here, maybe the Alloc version could be used. - Should EfiReservedMemoryType be used? Please see extract from section 2.3.6 AArch64 Platforms of the UEFI 2.9 specification below: "Note:Previous EFI specifications allowed ACPI tables loaded at runtime to be in the EfiReservedMemoryType and there was no guidance provided for other EFI Configuration Tables. EfiReservedMemoryType is not intended to be used by firmware. UEFI 2.0 clarified the situation moving forward. " > + if (mHestDriverData.HestTable == NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + mHestDriverData.CurrentTableSize = > + sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER); > + > + // > + // Populate the values of the HEST table header. > + // > + HestHeader.Header.Signature = > + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_SIGNATURE; > + HestHeader.Header.Revision = > + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_REVISION; > + CopyMem ( > + &HestHeader.Header.OemId, > + FixedPcdGetPtr (PcdAcpiDefaultOemId), > + sizeof (HestHeader.Header.OemId) > + ); > + TempOemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId); > + CopyMem ( > + &HestHeader.Header.OemTableId, > + &TempOemTableId, > + sizeof (HestHeader.Header.OemTableId) > + ); [SAMI] HestHeader.Header.OemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId); ? > + HestHeader.Header.OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision); > + HestHeader.Header.CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId); > + HestHeader.Header.CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision); > + HestHeader.ErrorSourceCount = 0; > + CopyMem (mHestDriverData.HestTable, &HestHeader, sizeof (HestHeader)); [SAMI] Is it possible to use a local HEST table pointer to initalise the values instead of initialising a HEST structure and then doing memcopy? > + > + return EFI_SUCCESS; > +} > + > +/** > + Append HEST error source descriptor protocol service. > + > + Protocol service used to append newly collected error source descriptors to > + to an already created HEST table. > + > + @param[in] ErrorSourceDescriptorList List of Error Source Descriptors. > + @param[in] ErrorSourceDescriptorListSize Total Size of Error Source > + Descriptors. > + @param[in] ErrorSourceDescriptorCount Total count of error source > + descriptors. > + > + @retval EFI_SUCCESS Appending the error source descriptors > + successful. > + @retval EFI_OUT_OF_RESOURCES Buffer reallocation failed for the Hest > + table. > + @retval EFI_INVALID_PARAMETER Null ErrorSourceDescriptorList param or > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +AppendErrorSourceDescriptor ( > + IN CONST VOID *ErrorSourceDescriptorList, > + IN UINTN ErrorSourceDescriptorListSize, > + IN UINTN ErrorSourceDescriptorCount > + ) > +{ > + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeaderPtr; > + EFI_STATUS Status; > + UINT32 NewTableSize; > + VOID *ErrorDescriptorPtr; > + > + if ((ErrorSourceDescriptorList == NULL) || > + (ErrorSourceDescriptorListSize == 0)) { > + return EFI_INVALID_PARAMETER; > + } > + > + // > + // Create a HEST table header if not already created. > + // > + if (mHestDriverData.HestTable == NULL) { > + Status = BuildHestHeader (); > + if (Status == EFI_OUT_OF_RESOURCES) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: Failed to build HEST header, status: %r\n", > + __FUNCTION__, > + Status > + )); > + return Status; > + } > + } > + > + // > + // Resize the existing HEST table buffer to accommodate the incoming error > + // source descriptors. > + // > + NewTableSize = mHestDriverData.CurrentTableSize + > + ErrorSourceDescriptorListSize; > + mHestDriverData.HestTable = ReallocateHestTableMemory ( > + mHestDriverData.CurrentTableSize, > + NewTableSize, > + mHestDriverData.HestTable > + ); > + if (mHestDriverData.HestTable == NULL) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: Failed to reallocate memory for HEST table\n", > + __FUNCTION__ > + )); > + return EFI_OUT_OF_RESOURCES; > + } > + [SAMI] As mentioned earlier, would it be possible to maintain a link list instead of relocating the memory? > + // > + // Copy the incoming error source descriptors into HEST table. > + // > + ErrorDescriptorPtr = (VOID *)mHestDriverData.HestTable + > + mHestDriverData.CurrentTableSize; > + HestHeaderPtr = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *) > + mHestDriverData.HestTable; > + CopyMem ( > + ErrorDescriptorPtr, > + ErrorSourceDescriptorList, > + ErrorSourceDescriptorListSize > + ); > + mHestDriverData.CurrentTableSize = NewTableSize; > + HestHeaderPtr->Header.Length = mHestDriverData.CurrentTableSize; > + HestHeaderPtr->ErrorSourceCount += ErrorSourceDescriptorCount; > + > + DEBUG (( > + DEBUG_INFO, > + "HestDxe: %d Error source descriptor(s) added \n", > + ErrorSourceDescriptorCount > + )); > + return EFI_SUCCESS; > +} > + > +/** > + Install HEST table protocol service. > + > + Installs the HEST table that has been appended with the error source > + descriptors. The checksum of this table is calculated and updated in > + the table header. The HEST table is then installed. > + > + @retval EFI_SUCCESS HEST table is installed successfully. > + @retval EFI_ABORTED HEST table is NULL. > + @retval Other Install service call failed. > +**/ > +STATIC > +EFI_STATUS > +EFIAPI > +InstallHestAcpiTable (VOID) [SAMI] Please update according to coding standard. > +{ > + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader; > + EFI_STATUS Status; > + UINTN AcpiTableHandle; > + > + // > + // Check if we have valid HEST table data. If not, there no hardware error > + // sources supported by the platform and no HEST table to publish, return. > + // > + if (mHestDriverData.HestTable == NULL) { > + DEBUG (( > + DEBUG_INFO, > + "HestDxe: No data available to generate HEST table\n" > + )); > + return EFI_SUCCESS; [SAMI] Can a suitable error code be returned here? EFI_NOT_FOUND? > + } > + > + // > + // Valid data for HEST table found. Update the header checksum and install the > + // HEST table. > + // > + HestHeader = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *) > + mHestDriverData.HestTable; > + HestHeader->Header.Checksum = CalculateCheckSum8 ( > + (UINT8 *)HestHeader, > + HestHeader->Header.Length > + ); [SAMI] Checksum calculation is not needed as ACPITableProtocol->InstallAcpiTable() does this internally. > + > + Status = mAcpiTableProtocol->InstallAcpiTable ( > + mAcpiTableProtocol, > + HestHeader, > + HestHeader->Header.Length, > + &AcpiTableHandle > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: HEST table installation failed, status: %r\n", > + __FUNCTION__, > + Status > + )); > + return Status; > + } > + > + // > + // Free the HEST table buffer. > + // > + FreePool (mHestDriverData.HestTable); > + DEBUG (( > + DEBUG_INFO, > + "HestDxe: Installed HEST table \n" > + )); > + return EFI_SUCCESS; [SAMI] return Status; > +} > + > +// > +// HEST table generation protocol instance. > +// > +STATIC HEST_TABLE_PROTOCOL mHestProtocol = { > + AppendErrorSourceDescriptor, > + InstallHestAcpiTable > +}; [SAMI] HEST is platform and processor architecture independent. Threfore, can this implementation be paced in a common location? MdeModulePkg? > + > +/** > + The Entry Point for HEST Dxe driver. > + > + This function installs the HEST table creation and installation protocol > + services. > + > + @param[in] ImageHandle Handle to the Efi image. > + @param[in] SystemTable A pointer to the Efi System Table. > + > + @retval EFI_SUCCESS On successful installation of protocol services and > + location the ACPI table protocol. > + @retval Other On Failure to locate ACPI table protocol or install > + of HEST table generation protocol. > +**/ > +EFI_STATUS > +EFIAPI > +HestInitialize ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + EFI_HANDLE Handle = NULL; > + EFI_STATUS Status; > + > + Status = gBS->LocateProtocol ( > + &gEfiAcpiTableProtocolGuid, > + NULL, > + (VOID **)&mAcpiTableProtocol > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: Failed to locate ACPI table protocol, status: %r\n", > + __FUNCTION__, > + Status > + )); > + return Status; > + } > + > + Status = gBS->InstallProtocolInterface ( > + &Handle, > + &gHestTableProtocolGuid, > + EFI_NATIVE_INTERFACE, > + &mHestProtocol > + ); > + if (EFI_ERROR (Status)) { > + DEBUG (( > + DEBUG_ERROR, > + "%a: Failed to install HEST table generation protocol status: %r\n", > + __FUNCTION__, > + Status > + )); > + return Status; > + } > + > + return EFI_SUCCESS; [SAMI] return Status; > +} --------------4609D3691EC72F45D4062A88 Content-Type: text/html; charset=utf-8 Content-Transfer-Encoding: 8bit

Hi Omkar,

Please find my response marked inline as [SAMI].

Regards,

Sami Mujawar


On 10/07/2021 05:18 PM, Omkar Anand Kulkarni wrote:
Introduce the HEST table generation protocol that allows platforms to
build the table with multiple error source descriptors and install the
table. The protocol provides two interfaces. The first interface allows
for adding multiple error source descriptors into the HEST table. The
second interface can then be used to dynamically install the fully
populated HEST table. This allows multiple drivers and/or libraries to
dynamically register error source descriptors into the HEST table.

Co-authored-by: Thomas Abraham <thomas.abraham@arm.com>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
---
 ArmPlatformPkg/ArmPlatformPkg.dec               |   4 +
 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf |  49 +++
 ArmPlatformPkg/Include/Protocol/HestTable.h     |  71 ++++
 ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c   | 354 ++++++++++++++++++++
 4 files changed, 478 insertions(+)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 3a25ddcdc8ca..e4afe5da8e11 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -127,3 +127,7 @@
   gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022
 
   gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
+
+[Protocols.common]
+  ## Arm Platform HEST table generation protocol
+  gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
new file mode 100644
index 000000000000..91c7385bf7ff
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.inf
@@ -0,0 +1,49 @@
+## @file
+#  Dxe driver that creates and publishes the HEST table.
+#
+#  This driver creates HEST header and provides protocol service to append
+#  and install the HEST table.
+#
+#  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x0001001A
+  BASE_NAME                      = HestDxe
+  FILE_GUID                      = 933099a2-ef71-4e00-82aa-a79b1e0a3b38
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  ENTRY_POINT                    = HestInitialize
+
+[Sources.Common]
+  HestDxe.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmPlatformPkg/ArmPlatformPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  Platform/ARM/SgiPkg/SgiPlatform.dec
+
+[LibraryClasses]
+  ArmLib
+  BaseMemoryLib
+  DebugLib
+  UefiDriverEntryPoint
+  UefiLib
+
+[Protocols]
+  gEfiAcpiTableProtocolGuid         ## PROTOCOL ALWAYS_CONSUMED
+  gHestTableProtocolGuid            ## PRODUCES
+
+[FixedPcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
+  gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
+
+[Depex]
+  TRUE
diff --git a/ArmPlatformPkg/Include/Protocol/HestTable.h b/ArmPlatformPkg/Include/Protocol/HestTable.h
new file mode 100644
index 000000000000..3b2e1f7d9203
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestTable.h
@@ -0,0 +1,71 @@
+/** @file
+  Builds and installs the HEST ACPI table.
+
+  Define the protocol interface that allows HEST ACPI table to be created,
+  populated with error record descriptions and installation of the HEST ACPI
+  table.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_TABLE_H_
+#define HEST_TABLE_H_
+
+#define HEST_TABLE_PROTOCOL_GUID \
+  { \
+    0x705bdcd9, 0x8c47, 0x457e, \
+    { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } \
+  }
+
+/**
+  Append HEST error source descriptor protocol service.
+
+  Protocol service used to append newly collected error source descriptors to
+  to an already created HEST table.
+
+  @param[in]  ErrorSourceDescriptorList      List of Error Source Descriptors.
+  @param[in]  ErrorSourceDescriptorListSize  Total Size of Error Source
+                                             Descriptors.
+  @param[in]  ErrorSourceDescriptorCount     Total count of error source
+                                             descriptors.
+
+  @retval  EFI_SUCCESS            Appending the error source descriptors
+                                  successful.
+  @retval  EFI_OUT_OF_RESOURCES   Buffer reallocation failed for the Hest
+                                  table.
+  @retval  EFI_INVALID_PARAMETER  Null ErrorSourceDescriptorList param or
+**/
+typedef
+EFI_STATUS
+(EFIAPI *APPEND_ERROR_SOURCE_DESCRIPTOR) (
+  IN CONST VOID *ErrorSourceDescriptorList,
+  IN UINTN      ErrorSourceDescriptorListSize,
+  IN UINTN      ErrorSourceDescriptorCount
+  );
+
+/**
+  Install HEST table protocol service.
+
+  Installs the HEST table that has been appended with the error source
+  descriptors. The checksum of this table is calculated and updated in
+  the table header. The HEST table is then installed.
+
+  @retval  EFI_SUCCESS  HEST table is installed successfully.
+  @retval  EFI_ABORTED  HEST table is NULL.
+  @retval  Other        Install service call failed.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *INSTALL_HEST_TABLE) (VOID);
+
+//
+// HEST_TABLE_PROTOCOL enables creation and installation of HEST table
+//
+typedef struct {
+  APPEND_ERROR_SOURCE_DESCRIPTOR AppendErrorSourceDescriptors;
+  INSTALL_HEST_TABLE             InstallHestTable;
+} HEST_TABLE_PROTOCOL;
+
+extern EFI_GUID gHestTableProtocolGuid;
+#endif  // HEST_TABLE_H_
diff --git a/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
new file mode 100644
index 000000000000..b68e1a0c4e48
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/Apei/HestDxe/HestDxe.c
@@ -0,0 +1,354 @@
+/** @file
+  Builds and installs the HEST ACPI table.
+
+  This driver installs a protocol that can be used to create and install a HEST
+  ACPI table. The protocol allows one or more error source producers to add the
+  error source descriptors into the HEST table. It also allows the resulting
+  HEST table to be installed.
+
+  Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+  @par Specification Reference:
+    - ACPI 6.3, Table 18-382, Hardware Error Source Table
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/ArmLib.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/AcpiSystemDescriptionTable.h>
+#include <Protocol/AcpiTable.h>
+#include <Protocol/HestTable.h>
+
+typedef struct {
+  VOID   *HestTable;        /// Pointer to HEST table.
+  UINT32 CurrentTableSize;  /// Current size of HEST table.
+} HEST_DXE_DRIVER_DATA;
+
+STATIC EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol = NULL;
+STATIC HEST_DXE_DRIVER_DATA mHestDriverData;
+
+/**
+  Helper function to the driver.
+
+  Function that reallocates memory for every new error source descriptor info
+  added.
+
+  @param[in]       OldTableSize  Old memory pool size.
+  @param[in]       NewTableSize  Required memory pool size.
+  @param[in, out]  OldBuffer     Current memory buffer pointer holding the Hest
+                                 table data.
+
+  @return  New pointer to reallocated memory pool.
+**/
+STATIC
+VOID*
+ReallocateHestTableMemory (
+  IN     UINT32 OldTableSize,
+  IN     UINT32 NewTableSize,
+  IN OUT VOID   *OldBuffer
+  )
+{
[SAMI] Have you considered maintaining a linked list of the error sources and serialising the list once InstallHestTable() is called? 
+  return ReallocateReservedPool (
+           OldTableSize,
+           NewTableSize,
+           OldBuffer
+           );
[SAMI] Is this wrapper function required? Can ReallocateReservedPool() be used directly instead? 
+}
+
+/**
+  Allocate memory for the HEST table header and populate it.
+
+  Helper function for this driver, populates the HEST table header. Called once
+  in the beginning on first invocation of append error source descriptor
+  protocol service.
+
+  @retval  EFI_SUCCESS           On successful creation of HEST header.
+  @retval  EFI_OUT_OF_RESOURCES  If system is out of memory recources.
+**/
+STATIC
+EFI_STATUS
+BuildHestHeader (VOID)
[SAMI] VOID and closing parenthesis should be on a separate line. 
+{
+  EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER HestHeader;
+  UINT64                                          TempOemTableId;
+
+  //
+  // Allocate memory for the HEST table header.
+  //
+  mHestDriverData.HestTable =
+    ReallocateHestTableMemory (
+      0,
+      sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER),
+      NULL
+      );
[SAMI] Is the Relocate version of the function required here, maybe the Alloc version could be used.
- Should EfiReservedMemoryType be used?
    Please see extract from section 2.3.6 AArch64 Platforms of the UEFI 2.9 specification below:
    "Note:Previous EFI specifications allowed ACPI tables loaded at runtime to be in the
     EfiReservedMemoryType and there was no guidance provided for other EFI Configuration
    Tables. EfiReservedMemoryType is not intended to be used by firmware. UEFI 2.0 clarified
    the situation moving forward. "
 
+  if (mHestDriverData.HestTable == NULL) {
+    return EFI_OUT_OF_RESOURCES;
+  }
+
+  mHestDriverData.CurrentTableSize =
+    sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER);
+
+  //
+  // Populate the values of the HEST table header.
+  //
+  HestHeader.Header.Signature =
+    EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_SIGNATURE;
+  HestHeader.Header.Revision =
+    EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_REVISION;
+  CopyMem (
+    &HestHeader.Header.OemId,
+    FixedPcdGetPtr (PcdAcpiDefaultOemId),
+    sizeof (HestHeader.Header.OemId)
+    );
+  TempOemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId);
+  CopyMem (
+    &HestHeader.Header.OemTableId,
+    &TempOemTableId,
+    sizeof (HestHeader.Header.OemTableId)
+    );
[SAMI] HestHeader.Header.OemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId); ? 
+  HestHeader.Header.OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision);
+  HestHeader.Header.CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId);
+  HestHeader.Header.CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision);
+  HestHeader.ErrorSourceCount = 0;
+  CopyMem (mHestDriverData.HestTable, &HestHeader, sizeof (HestHeader));
[SAMI] Is it possible to use a local HEST table pointer to initalise the values instead of initialising a HEST structure and then doing memcopy?
+
+  return EFI_SUCCESS;
+}
+
+/**
+  Append HEST error source descriptor protocol service.
+
+  Protocol service used to append newly collected error source descriptors to
+  to an already created HEST table.
+
+  @param[in]  ErrorSourceDescriptorList      List of Error Source Descriptors.
+  @param[in]  ErrorSourceDescriptorListSize  Total Size of Error Source
+                                             Descriptors.
+  @param[in]  ErrorSourceDescriptorCount     Total count of error source
+                                             descriptors.
+
+  @retval  EFI_SUCCESS            Appending the error source descriptors
+                                  successful.
+  @retval  EFI_OUT_OF_RESOURCES   Buffer reallocation failed for the Hest
+                                  table.
+  @retval  EFI_INVALID_PARAMETER  Null ErrorSourceDescriptorList param or
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+AppendErrorSourceDescriptor (
+  IN CONST VOID *ErrorSourceDescriptorList,
+  IN UINTN      ErrorSourceDescriptorListSize,
+  IN UINTN      ErrorSourceDescriptorCount
+  )
+{
+  EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeaderPtr;
+  EFI_STATUS                                      Status;
+  UINT32                                          NewTableSize;
+  VOID                                            *ErrorDescriptorPtr;
+
+  if ((ErrorSourceDescriptorList == NULL) ||
+      (ErrorSourceDescriptorListSize == 0)) {
+     return EFI_INVALID_PARAMETER;
+  }
+
+  //
+  // Create a HEST table header if not already created.
+  //
+  if (mHestDriverData.HestTable == NULL) {
+    Status = BuildHestHeader ();
+    if (Status == EFI_OUT_OF_RESOURCES) {
+      DEBUG ((
+        DEBUG_ERROR,
+        "%a: Failed to build HEST header, status: %r\n",
+        __FUNCTION__,
+        Status
+        ));
+      return Status;
+    }
+  }
+
+  //
+  // Resize the existing HEST table buffer to accommodate the incoming error
+  // source descriptors.
+  //
+  NewTableSize = mHestDriverData.CurrentTableSize +
+                 ErrorSourceDescriptorListSize;
+  mHestDriverData.HestTable = ReallocateHestTableMemory (
+                                mHestDriverData.CurrentTableSize,
+                                NewTableSize,
+                                mHestDriverData.HestTable
+                                );
+  if (mHestDriverData.HestTable == NULL) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to reallocate memory for HEST table\n",
+      __FUNCTION__
+      ));
+    return EFI_OUT_OF_RESOURCES;
+  }
+
[SAMI] As mentioned earlier, would it be possible to maintain a link list instead of relocating the memory?
+  //
+  // Copy the incoming error source descriptors into HEST table.
+  //
+  ErrorDescriptorPtr = (VOID *)mHestDriverData.HestTable +
+                       mHestDriverData.CurrentTableSize;
+  HestHeaderPtr = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+                  mHestDriverData.HestTable;
+  CopyMem (
+    ErrorDescriptorPtr,
+    ErrorSourceDescriptorList,
+    ErrorSourceDescriptorListSize
+    );
+  mHestDriverData.CurrentTableSize = NewTableSize;
+  HestHeaderPtr->Header.Length = mHestDriverData.CurrentTableSize;
+  HestHeaderPtr->ErrorSourceCount += ErrorSourceDescriptorCount;
+
+  DEBUG ((
+    DEBUG_INFO,
+    "HestDxe: %d Error source descriptor(s) added \n",
+    ErrorSourceDescriptorCount
+    ));
+  return EFI_SUCCESS;
+}
+
+/**
+  Install HEST table protocol service.
+
+  Installs the HEST table that has been appended with the error source
+  descriptors. The checksum of this table is calculated and updated in
+  the table header. The HEST table is then installed.
+
+  @retval  EFI_SUCCESS  HEST table is installed successfully.
+  @retval  EFI_ABORTED  HEST table is NULL.
+  @retval  Other        Install service call failed.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+InstallHestAcpiTable (VOID)
[SAMI] Please update according to coding standard.
+{
+  EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
+  EFI_STATUS                                      Status;
+  UINTN                                           AcpiTableHandle;
+
+  //
+  // Check if we have valid HEST table data. If not, there no hardware error
+  // sources supported by the platform and no HEST table to publish, return.
+  //
+  if (mHestDriverData.HestTable == NULL) {
+    DEBUG ((
+      DEBUG_INFO,
+      "HestDxe: No data available to generate HEST table\n"
+      ));
+    return EFI_SUCCESS;
[SAMI] Can a suitable error code be returned here? EFI_NOT_FOUND?
+  }
+
+  //
+  // Valid data for HEST table found. Update the header checksum and install the
+  // HEST table.
+  //
+  HestHeader = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+               mHestDriverData.HestTable;
+  HestHeader->Header.Checksum = CalculateCheckSum8 (
+                                  (UINT8 *)HestHeader,
+                                  HestHeader->Header.Length
+                                  );
[SAMI] Checksum calculation is not needed as ACPITableProtocol->InstallAcpiTable() does this internally.
+
+  Status = mAcpiTableProtocol->InstallAcpiTable (
+                                 mAcpiTableProtocol,
+                                 HestHeader,
+                                 HestHeader->Header.Length,
+                                 &AcpiTableHandle
+                                 );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: HEST table installation failed, status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  //
+  // Free the HEST table buffer.
+  //
+  FreePool (mHestDriverData.HestTable);
+  DEBUG ((
+    DEBUG_INFO,
+    "HestDxe: Installed HEST table \n"
+    ));
+  return EFI_SUCCESS;
[SAMI] return Status;
+}
+
+//
+// HEST table generation protocol instance.
+//
+STATIC HEST_TABLE_PROTOCOL mHestProtocol = {
+  AppendErrorSourceDescriptor,
+  InstallHestAcpiTable
+};
[SAMI] HEST is platform and processor architecture independent. Threfore, can this implementation be paced in a common location? MdeModulePkg?
+
+/**
+  The Entry Point for HEST Dxe driver.
+
+  This function installs the HEST table creation and installation protocol
+  services.
+
+  @param[in]  ImageHandle  Handle to the Efi image.
+  @param[in]  SystemTable  A pointer to the Efi System Table.
+
+  @retval EFI_SUCCESS    On successful installation of protocol services and
+                         location the ACPI table protocol.
+  @retval Other          On Failure to locate ACPI table protocol or install
+                         of HEST table generation protocol.
+**/
+EFI_STATUS
+EFIAPI
+HestInitialize (
+  IN EFI_HANDLE       ImageHandle,
+  IN EFI_SYSTEM_TABLE *SystemTable
+  )
+{
+  EFI_HANDLE Handle = NULL;
+  EFI_STATUS Status;
+
+  Status = gBS->LocateProtocol (
+                  &gEfiAcpiTableProtocolGuid,
+                  NULL,
+                  (VOID **)&mAcpiTableProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to locate ACPI table protocol, status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  Status = gBS->InstallProtocolInterface (
+                  &Handle,
+                  &gHestTableProtocolGuid,
+                  EFI_NATIVE_INTERFACE,
+                  &mHestProtocol
+                  );
+  if (EFI_ERROR (Status)) {
+    DEBUG ((
+      DEBUG_ERROR,
+      "%a: Failed to install HEST table generation protocol status: %r\n",
+      __FUNCTION__,
+      Status
+      ));
+    return Status;
+  }
+
+  return EFI_SUCCESS;
[SAMI] return Status;
+}

--------------4609D3691EC72F45D4062A88--