* [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table
@ 2021-08-24 5:33 Omkar Anand Kulkarni
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:33 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
Changes since v2:
- Addressed the comments given by Sami.
- Added Readme file with all cover letter information.
- Rebased to the latest upstream code.
Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
(SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
patch series introduces a framework to build and install the HEST ACPI table
dynamically.
The following figure illustrates the possible usage of the dyanamic
generation of HEST ACPI table.
NS | S
+--------------------------------------+--------------------------------------+
| | |
|+-------------------------------------+---------------------+ |
|| +---------------------+--------------------+| |
|| | | || |
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
|| | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
|| |GHESv2 | || |
|| +---------------------+--------------------+| |
|| +--------------------+ | | |
|| |PlatformErrorHandler| | | |
|| | DXE | | | |
|| +--------------------+ | | |
||FF FWK | | |
|+-------------------------------------+---------------------+ |
| | |
+--------------------------------------+--------------------------------------+
|
Figure: Firmware First Error Handling approach.
All the hardware error sources are added to HEST table as GHESv2[3] error source
descriptors. The framework comprises of following DXE and MM drivers:
- HestTableDxe:
Builds HEST table header and allows appending error source descriptors to the
HEST table. Also provides protocol interface to install the built HEST table.
- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
These two drivers together retrieve all possible error source descriptors of
type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
protocol. Once all the descriptors are collected HestErrorSourceDxe appends
it to HEST table using HestTableDxe driver.
Link to github branch with the patches in this series -
https://github.com/omkkul01/edk2/tree/ras_firware_first_edk2-platforms_v3
Omkar Anand Kulkarni (5):
MdeModulePkg: Allow dynamic generation of HEST ACPI table
ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
ArmPlatformPkg: retreive error source descriptors from MM
EmbeddedPkg: Add helpers for HEST table generation
ArmPlatformPkg: Add Readme file
ArmPlatformPkg/ArmPlatformPkg.dec | 10 +
MdeModulePkg/MdeModulePkg.dec | 3 +
.../HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
.../HestErrorSourceStandaloneMm.inf | 51 +++
.../Universal/Apei/HestDxe/HestDxe.inf | 49 +++
.../HestMmErrorSourceCommon.h | 37 ++
.../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++
MdeModulePkg/Include/Protocol/HestTable.h | 71 ++++
MdePkg/Include/Protocol/MmCommunication.h | 2 +
.../HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++
.../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++++
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++
.../Drivers/HestMmErrorSources/Readme.md | 66 ++++
14 files changed, 1357 insertions(+)
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
create mode 100644 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
create mode 100644 MdeModulePkg/Include/Protocol/HestTable.h
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
--
2.17.1
^ permalink raw reply [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of HEST ACPI table
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
@ 2021-08-24 5:33 ` Omkar Anand Kulkarni
2021-10-04 17:46 ` Sami Mujawar
2024-04-09 4:57 ` [edk2-devel] " Dhaval Sharma
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL Omkar Anand Kulkarni
` (4 subsequent siblings)
5 siblings, 2 replies; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:33 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
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>
---
MdeModulePkg/MdeModulePkg.dec | 3 +
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf | 49 +++
MdeModulePkg/Include/Protocol/HestTable.h | 71 +++++
MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++++
4 files changed, 441 insertions(+)
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index ad84421cf3f7..2cb4ba69d6bf 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -663,6 +663,9 @@
## Include/Protocol/VariablePolicy.h
gEdkiiVariablePolicyProtocolGuid = { 0x81D1675C, 0x86F6, 0x48DF, { 0xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } }
+ ## Arm Platform HEST table generation protocol
+ gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
+
[PcdsFeatureFlag]
## Indicates if the platform can support update capsule across a system reset.<BR><BR>
# TRUE - Supports update capsule across a system reset.<BR>
diff --git a/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
new file mode 100644
index 000000000000..91c7385bf7ff
--- /dev/null
+++ b/MdeModulePkg/Universal/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/MdeModulePkg/Include/Protocol/HestTable.h b/MdeModulePkg/Include/Protocol/HestTable.h
new file mode 100644
index 000000000000..3b2e1f7d9203
--- /dev/null
+++ b/MdeModulePkg/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/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
new file mode 100644
index 000000000000..87f21acf61f4
--- /dev/null
+++ b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
@@ -0,0 +1,318 @@
+/** @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;
+
+/**
+ 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
+ )
+{
+ EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
+
+ //
+ // Allocate memory for the HEST table header.
+ //
+ mHestDriverData.HestTable =
+ AllocateZeroPool (sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER));
+ if (mHestDriverData.HestTable == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ mHestDriverData.CurrentTableSize =
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER);
+
+ HestHeader = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
+ mHestDriverData.HestTable;
+
+ //
+ // 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)
+ );
+ HestHeader->Header.OemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId);
+ HestHeader->Header.OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision);
+ HestHeader->Header.CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId);
+ HestHeader->Header.CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision);
+ HestHeader->ErrorSourceCount = 0;
+
+ 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 = ReallocatePool (
+ 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;
+ }
+
+ //
+ // 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
+ )
+{
+ 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_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;
+
+ 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 Status;
+}
+
+//
+// HEST table generation protocol instance.
+//
+STATIC HEST_TABLE_PROTOCOL mHestProtocol = {
+ AppendErrorSourceDescriptor,
+ InstallHestAcpiTable
+};
+
+/**
+ 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 Status;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
@ 2021-08-24 5:34 ` Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM Omkar Anand Kulkarni
` (3 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:34 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
Add the protocol definition of the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
protocol. This protocol can be implemented by MM drivers to publish
error source descriptors that have to be populated into HEST table.
Co-authored-by: Thomas Abraham <thomas.abraham@arm.com>
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
---
ArmPlatformPkg/ArmPlatformPkg.dec | 3 +
ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h | 64 ++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 3a25ddcdc8ca..446d93b880f9 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -127,3 +127,6 @@
gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022
gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
+
+[Protocols.common]
+ gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
new file mode 100644
index 000000000000..86662ea7af57
--- /dev/null
+++ b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
@@ -0,0 +1,64 @@
+/** @file
+ MM protocol to get the secure error source descriptor information.
+
+ MM Drivers must implement this protocol in order to publish secure side
+ error source descriptor information to OSPM through the HEST ACPI table.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef MM_HEST_ERROR_SOURCE_DESC_
+#define MM_HEST_ERROR_SOURCE_DESC_
+
+#define MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_GUID \
+ { \
+ 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } \
+ }
+
+typedef struct MmHestErrorSourceDescProtocol
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL;
+
+/**
+ Get HEST Secure Error Source Descriptors.
+
+ The MM drivers implementing this protocol must convey the total count and
+ total length of the error sources the driver has along with the actual error
+ source descriptor(s).
+
+ Passing NULL as Buffer parameter shall return EFI_INVALID_PARAMETR with the
+ total length and count of the error source descriptor(s) it supports.
+
+ @param[in] This MM_HEST_ERROR_SOURCE_DESC_PROTOCOL instance.
+ @param[out] Buffer Buffer to be appended with the error
+ source descriptors information.
+ @param[out] ErrorSourcesLength Total length of all the error source
+ descriptors.
+ @param[out] ErrorSourceCount Count of total error source descriptors
+ supported by the driver.
+
+ retval EFI_SUCCESS If the Buffer is valid and is filled with valid
+ Error Source descriptor data.
+ retval EFI_INVALID_PARAMTER Buffer is NULL.
+ retval Other If no error source descriptor information is
+ available.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS) (
+ IN MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *This,
+ OUT VOID **Buffer,
+ OUT UINTN *ErrorSourcesLength,
+ OUT UINTN *ErrorSourcesCount
+ );
+
+//
+// Protocol declaration
+//
+struct MmHestErrorSourceDescProtocol {
+ MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS GetHestErrorSourceDescriptors;
+};
+
+extern EFI_GUID gMmHestErrorSourceDescProtocolGuid;
+
+#endif // MM_HEST_ERROR_SOURCE_DESC_
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL Omkar Anand Kulkarni
@ 2021-08-24 5:34 ` Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation Omkar Anand Kulkarni
` (2 subsequent siblings)
5 siblings, 1 reply; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:34 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
Add a driver that retreives error source descriptors from MM and
populates those into the HEST ACPI table. The error source descriptors
that are available from the MM side are retreived using MM Communicate 2
protocol.
The first call into the MM returns the size of MM Communicate buffer
required to hold all error source descriptor info. The communication
buffer of that size is then allocated and the second call into MM
returns the error source descriptors in the communication buffer.
The retreived error source descriptors are then appended to 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 | 7 +
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf | 51 ++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h | 37 +++
MdePkg/Include/Protocol/MmCommunication.h | 2 +
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++++
ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c | 312 ++++++++++++++++++++
7 files changed, 763 insertions(+)
diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 446d93b880f9..a400f175726e 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -52,6 +52,8 @@
[Guids.common]
gArmPlatformTokenSpaceGuid = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
+ gArmPlatformHestErrorSourcesGuid = { 0x76b8ab43, 0x822d, 0x4b00, { 0x9f, 0xd0, 0xf4, 0xa5, 0x35, 0x82, 0x47, 0x0a } }
+ gMmHestGetErrorSourceInfoGuid = { 0x7d602951, 0x678e, 0x4cc4, { 0x98, 0xd9, 0xe3, 0x76, 0x04, 0xf6, 0x93, 0x0d } }
[PcdsFeatureFlag.common]
gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOOLEAN|0x00000004
@@ -128,5 +130,10 @@
gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
+[PcdsFixedAtBuild, PcdsPatchableInModule]
+ ## ACPI CPER memory space
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000000|UINT64|0x00000046
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000000|UINT64|0x00000047
+
[Protocols.common]
gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
new file mode 100644
index 000000000000..b16ff2353aa5
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
@@ -0,0 +1,45 @@
+## @file
+# DXE driver to get secure error sources.
+#
+# DXE driver to retrieve the error source descriptors from Standalone MM and
+# append those to 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 = HestMmErrorSourceDxe
+ FILE_GUID = 76b8ab43-822d-4b00-9fd0-f4a53582470a
+ MODULE_TYPE = DXE_DRIVER
+ VERSION_STRING = 1.0
+ ENTRY_POINT = HestErrorSourceInitialize
+
+[Sources.common]
+ HestErrorSourceDxe.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ BaseMemoryLib
+ DebugLib
+ DxeServicesTableLib
+ UefiDriverEntryPoint
+ UefiLib
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ## PRODUCES
+
+[Protocols]
+ gHestTableProtocolGuid ## CONSUMES
+ gEfiMmCommunication2ProtocolGuid
+
+[Depex]
+ gHestTableProtocolGuid AND gEfiMmCommunication2ProtocolGuid
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
new file mode 100644
index 000000000000..9d566de9bec3
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
@@ -0,0 +1,51 @@
+## @file
+# HEST error source gateway Standalone MM driver.
+#
+# Collects HEST error source descriptors,by communicating with all the MM
+# drivers implementing the HEST error source descriptor protocol.
+#
+# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+# SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+##
+
+[Defines]
+ INF_VERSION = 0x0001001A
+ BASE_NAME = HestErrorSourceStandaloneMm
+ FILE_GUID = 3ddbebcc-9841-4ef8-87fa-305843c1922d
+ MODULE_TYPE = MM_STANDALONE
+ VERSION_STRING = 1.0
+ PI_SPECIFICATION_VERSION = 0x00010032
+ ENTRY_POINT = StandaloneMmHestErrorSourceInitialize
+
+[Sources]
+ HestErrorSourceStandaloneMm.c
+
+[Packages]
+ ArmPkg/ArmPkg.dec
+ ArmPlatformPkg/ArmPlatformPkg.dec
+ MdeModulePkg/MdeModulePkg.dec
+ MdePkg/MdePkg.dec
+ StandaloneMmPkg/StandaloneMmPkg.dec
+
+[LibraryClasses]
+ ArmLib
+ ArmSvcLib
+ BaseMemoryLib
+ DebugLib
+ MemoryAllocationLib
+ StandaloneMmDriverEntryPoint
+
+[Protocols]
+ gMmHestErrorSourceDescProtocolGuid
+
+[Guids]
+ gMmHestGetErrorSourceInfoGuid ##PRODUCES
+ gEfiStandaloneMmNonSecureBufferGuid
+
+[FixedPcd]
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase
+ gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize
+
+[Depex]
+ TRUE
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
new file mode 100644
index 000000000000..6ddc6bd21922
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
@@ -0,0 +1,37 @@
+/** @file
+ Data structures for error source descriptor information.
+
+ This data structure forms the CommBuffer part of the MM Communication
+ protocol used for communicating the Hardware Error sources form MM to
+ Non-MM environment.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#ifndef HEST_ERROR_SOURCE_DESCRIPTOR_H_
+#define HEST_ERROR_SOURCE_DESCRIPTOR_H_
+
+#define HEST_ERROR_SOURCE_DESC_INFO_SIZE \
+ (OFFSET_OF (HEST_ERROR_SOURCE_DESC_INFO, ErrSourceDescList))
+
+//
+// Data Structure to communicate the error source descriptor information from
+// Standalone MM.
+//
+typedef struct {
+ //
+ // Total count of error source descriptors.
+ //
+ UINTN ErrSourceDescCount;
+ //
+ // Total size of all the error source descriptors.
+ //
+ UINTN ErrSourceDescSize;
+ //
+ // Array of error source descriptors that is ErrSourceDescSize in size.
+ //
+ UINT8 ErrSourceDescList[1];
+} HEST_ERROR_SOURCE_DESC_INFO;
+
+#endif // HEST_ERROR_SOURCE_DESCRIPTOR_H_
diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h
index 34c3e2b5a9e3..acde54971fd9 100644
--- a/MdePkg/Include/Protocol/MmCommunication.h
+++ b/MdePkg/Include/Protocol/MmCommunication.h
@@ -12,6 +12,8 @@
#ifndef _MM_COMMUNICATION_H_
#define _MM_COMMUNICATION_H_
+#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data))
+
#pragma pack(1)
///
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
new file mode 100644
index 000000000000..28de8daf2f5e
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
@@ -0,0 +1,309 @@
+/** @file
+ Collects and appends the HEST error source descriptors from the MM drivers.
+
+ The drivers entry point locates the MM Communication protocol and calls into
+ Standalone MM to get the HEST error sources length and count. It also
+ retrieves descriptor information. The information is then used to build the
+ HEST table using the HEST table generation protocol.
+
+ This driver collects the secure error source descriptor information from the
+ MM drviers that implement HEST error source protocol. Instead of directly
+ communicating with the individual MM drivers, it calls into
+ HestErrorSourceStandaloneMM driver which is a gatway MM driver. This MM driver
+ in-turn communicates with individual MM drivers collecting the error source
+ descriptor information.
+
+ Once all the error source descriptor information is retrieved the driver
+ appends the descriptors to HEST table using the HestDxe driver.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <IndustryStandard/Acpi.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/DxeServicesTableLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Protocol/MmCommunication2.h>
+#include <Protocol/HestTable.h>
+#include "HestMmErrorSourceCommon.h"
+
+STATIC HEST_TABLE_PROTOCOL *mHestProtocol;
+STATIC EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication2;
+
+/**
+ Retrieve the error source descriptors from Standalone MM.
+
+ Initialize the MM comminication buffer by assigning the MM service to
+ invoke as gMmHestGetErrorSourceInfoGuid. Use the MM communication
+ protocol to retrieve the error source descriptors.
+
+ @param[in] CommBuffSize Size of communicate buffer.
+ @param[in, out] CommBuffer The communicate buffer.
+
+ @retval EFI_SUCCESS MM Communicate protocol call successful.
+ @retval Other MM Communicate protocol call failed.
+**/
+STATIC
+EFI_STATUS
+GetErrorSourceDescriptors (
+ IN UINTN CommBuffSize,
+ IN OUT EFI_MM_COMMUNICATE_HEADER **CommBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ //
+ // Initialize the CommBuffer with MM Communicate metadata.
+ //
+ CopyGuid (&(*CommBuffer)->HeaderGuid, &gMmHestGetErrorSourceInfoGuid);
+ (*CommBuffer)->MessageLength =
+ CommBuffSize -
+ sizeof ((*CommBuffer)->HeaderGuid) -
+ sizeof ((*CommBuffer)->MessageLength);
+
+ //
+ // Call into the Standalone MM using the MM Communicate protocol.
+ //
+ Status = mMmCommunication2->Communicate (
+ mMmCommunication2,
+ NULL,
+ (VOID *)*CommBuffer,
+ NULL
+ );
+
+ return Status;
+}
+
+/**
+ Collect HEST error source descriptors from all Standalone MM drivers and
+ append them to the HEST table.
+
+ Use MM Communication Protocol to communicate and collect the error source
+ descriptor information from Standalone MM. Check for the required buffer size
+ returned by the MM driver. Allocate buffer of adequate size and call again
+ into MM.
+
+ @retval EFI_SUCCESS Successful to collect and append the error
+ source.
+ descriptors to HEST table.
+ @retval EFI_OUT_OF_RESOURCES Memory allocation failure.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+AppendMmErrorSources (
+ VOID
+ )
+{
+ EFI_MM_COMMUNICATE_HEADER *CommunicationHeader = NULL;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceDescInfo;
+ EFI_STATUS Status;
+ UINTN CommBufferSize;
+
+ //
+ // Retrieve the count, length and the actual eror source descriptors from
+ // the MM drivers. Do this by performing two MM Communicate calls, in the
+ // first call pass CommBuffer which is atleast of the size of error source
+ // descriptor info structure. Followed by another communicate call with
+ // CommBuffer allocated to required buffer size to hold all descriptors.
+ //
+ // Allocate CommBuffer atleast the size of error source descriptor info
+ // structure.
+ CommBufferSize =
+ MM_COMMUNICATE_HEADER_SIZE + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+ CommunicationHeader = AllocateZeroPool (CommBufferSize);
+ if (CommunicationHeader == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make the first MM Communicate call to HestErrorSourceStandaloneMM gateway
+ // driver, which returns the required buffer size adequate to hold all the
+ // desctriptor information.
+ //
+ Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+ if ((EFI_ERROR (Status)) &&
+ (Status != EFI_BUFFER_TOO_SMALL)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol call failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ // Check for the length of Error Source descriptors.
+ ErrorSourceDescInfo =
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if ((ErrorSourceDescInfo->ErrSourceDescSize == 0) ||
+ (ErrorSourceDescInfo->ErrSourceDescCount == 0)) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HesErrorSourceDxe: HEST error source(s) not found\n"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_NOT_FOUND;
+ }
+
+ //
+ // Allocate CommBuffer of required size to accomodate all the error source
+ // descriptors. Required size of communication buffer =
+ // MM communicate metadata. + (error source desc info struct + error source
+ // descriptor size).
+ //
+
+ CommBufferSize =
+ MM_COMMUNICATE_HEADER_SIZE +
+ HEST_ERROR_SOURCE_DESC_INFO_SIZE +
+ ErrorSourceDescInfo->ErrSourceDescSize;
+
+ // Free old MM Communicate buffer and allocate a new buffer of required size.
+ FreePool (CommunicationHeader);
+ CommunicationHeader = AllocateZeroPool (CommBufferSize);
+ if (CommunicationHeader == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for CommunicationHeader\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ //
+ // Make second MM Communicate call to HestErrorSourceStandaloneMM driver to
+ // get the error source descriptors from the MM drivers.
+ //
+ Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: MM Communicate protocol failed, status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ FreePool (CommunicationHeader);
+ return Status;
+ }
+
+ //
+ // Retrieve the HEST error source descriptor information. Ensure that there
+ // is a valid list of error source descriptors.
+ //
+ ErrorSourceDescInfo =
+ (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
+ if (ErrorSourceDescInfo->ErrSourceDescList == NULL) {
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: Error source descriptor list is empty"
+ ));
+ FreePool (CommunicationHeader);
+ return EFI_NOT_FOUND;
+ }
+
+ DEBUG ((
+ DEBUG_INFO,
+ "HestErrorSourceDxe: ErrorSources: TotalCount = %d TotalLength = %d \n",
+ ErrorSourceDescInfo->ErrSourceDescCount,
+ ErrorSourceDescInfo->ErrSourceDescSize
+ ));
+
+ //
+ // Append the error source descriptors to HEST table using the HEST table
+ // generation protocol.
+ //
+ Status = mHestProtocol->AppendErrorSourceDescriptors (
+ ErrorSourceDescInfo->ErrSourceDescList,
+ ErrorSourceDescInfo->ErrSourceDescSize,
+ ErrorSourceDescInfo->ErrSourceDescCount
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to append error source(s), status: %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ }
+
+ FreePool (CommunicationHeader);
+ return Status;
+}
+
+/**
+ The Entry Point for HEST Error Source Dxe driver.
+
+ Locates the HEST Table generation and MM Communication2 protocols. Using the
+ MM Communication2, the driver collects the Error Source Descriptor(s) from
+ Standalone MM. It then appends those Error Source Descriptor(s) to the Hest
+ table using the HEST Table generation protocol.
+
+ @param[in] ImageHandle The firmware allocated handle for the Efi image.
+ @param[in] SystemTable A pointer to the Efi System Table.
+
+ @retval EFI_SUCCESS The entry point is executed successfully.
+ @retval Other Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+HestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_STATUS Status;
+
+ Status = gBS->LocateProtocol (
+ &gHestTableProtocolGuid,
+ NULL,
+ (VOID **)&mHestProtocol
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate HEST table generation protocol, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ Status = gBS->LocateProtocol (
+ &gEfiMmCommunication2ProtocolGuid,
+ NULL,
+ (VOID **)&mMmCommunication2
+ );
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to locate MMCommunication2 driver protocol, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ //
+ // Append HEST error sources retrieved from StandaloneMM, if any, into the
+ // HEST ACPI table.
+ //
+ Status = AppendMmErrorSources ();
+ if (EFI_ERROR (Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed appending error source desc to HEST table, status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ }
+ return EFI_SUCCESS;
+}
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
new file mode 100644
index 000000000000..75dc31438180
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
@@ -0,0 +1,312 @@
+/** @file
+ MM HEST error source gateway driver.
+
+ This MM driver installs a handler which can be used to retrieve the error
+ source descriptors from the all MM drivers implementing the HEST error source
+ descriptor protocol.
+
+ The MM driver acts as a single point of contact to collect secure hardware
+ error sources from the MM drivers. It loops over all the MM drivers that
+ implement HEST error source descriptor protocol and collects error source
+ descriptor information along with the error source count and length.
+
+ Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+**/
+
+#include <Base.h>
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Protocol/HestErrorSourceInfo.h>
+
+#include "HestMmErrorSourceCommon.h"
+
+STATIC EFI_MM_SYSTEM_TABLE *mMmst = NULL;
+
+/**
+ Returns an array of handles that implement the HEST error source descriptor
+ protocol.
+
+ Passing HandleBuffer as NULL will return the actual size of the buffer
+ required to hold the array of handles implementing the protocol.
+
+ @param[in, out] HandleBufferSize The size of the HandleBuffer.
+ @param[out] HandleBuffer A pointer to the buffer containing the list
+ of handles.
+
+ @retval EFI_SUCCESS The array of handles returned in HandleBuffer.
+ @retval EFI_NOT_FOUND No implementation present for the protocol.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+GetHestErrorSourceProtocolHandles (
+ IN OUT UINTN *HandleBufferSize,
+ OUT EFI_HANDLE **HandleBuffer
+ )
+{
+ EFI_STATUS Status;
+
+ Status = mMmst->MmLocateHandle (
+ ByProtocol,
+ &gMmHestErrorSourceDescProtocolGuid,
+ NULL,
+ HandleBufferSize,
+ *HandleBuffer
+ );
+ if ((EFI_ERROR (Status)) &&
+ (Status != EFI_BUFFER_TOO_SMALL))
+ {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: No implementation of MmHestErrorSourceDescProtocol found, \
+ Status:%r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return EFI_NOT_FOUND;
+ }
+
+ return Status;
+}
+
+/**
+ Mmi handler to retrieve HEST error source descriptor information.
+
+ Handler for Mmi service that returns the supported HEST error source
+ descriptors in MM. This handler populates the CommBuffer with the
+ list of all error source descriptors, prepended with the length and
+ the number of descriptors populated into CommBuffer.
+
+ @param[in] DispatchHandle The unique handle assigned to this handler by
+ MmiHandlerRegister().
+ @param[in] Context Points to an optional handler context that
+ is specified when the handler was registered.
+ @param[in, out] CommBuffer Buffer used for communication of HEST error
+ source descriptors.
+ @param[in, out] CommBufferSize The size of the CommBuffer.
+
+ @retval EFI_SUCCESS CommBuffer has valid data.
+ @retval EFI_BAD_BUFFER_SIZE CommBufferSize not adequate.
+ @retval EFI_OUT_OF_RESOURCES System out of memory resources.
+ @retval EFI_INVALID_PARAMETER Invalid CommBufferSize recieved.
+ @retval Other For any other error.
+**/
+STATIC
+EFI_STATUS
+EFIAPI
+HestErrorSourcesInfoMmiHandler (
+ IN EFI_HANDLE DispatchHandle,
+ IN CONST VOID *Context, OPTIONAL
+ IN OUT VOID *CommBuffer, OPTIONAL
+ IN UINTN *CommBufferSize OPTIONAL
+ )
+{
+ MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *HestErrSourceDescProtocolHandle;
+ HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceInfoList;
+ EFI_HANDLE *HandleBuffer;
+ EFI_STATUS Status;
+ UINTN HandleCount;
+ UINTN HandleBufferSize;
+ UINTN Index;
+ UINTN SourceCount = 0;
+ UINTN SourceLength = 0;
+ VOID *ErrorSourcePtr;
+ UINTN TotalSourceLength = 0;
+ UINTN TotalSourceCount = 0;
+
+ if (*CommBufferSize < HEST_ERROR_SOURCE_DESC_INFO_SIZE) {
+ //
+ // Ensures that the communication buffer has enough space to atleast hold
+ // the ErrSourceDescCount and ErrSourceDescSize elements of the
+ // HEST_ERROR_SOURCE_DESC_INFO structure.
+ //
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Get all handles that implement the HEST error source descriptor protocol.
+ // Get the buffer size required to store list of handles for the protocol.
+ //
+ HandleBuffer = NULL;
+ HandleBufferSize = 0;
+ Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+ if ((Status == EFI_NOT_FOUND) ||
+ (HandleBufferSize == 0))
+ {
+ return Status;
+ }
+
+ // Allocate memory for HandleBuffer of size HandleBufferSize.
+ HandleBuffer = AllocateZeroPool (HandleBufferSize);
+ if (HandleBuffer == NULL) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Failed to allocate memory for HandleBuffer\n",
+ __FUNCTION__
+ ));
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ // Get the list of handles.
+ Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
+ if ((EFI_ERROR (Status)) ||
+ (HandleBuffer == NULL))
+ {
+ FreePool (HandleBuffer);
+ return Status;
+ }
+
+ // Count of handles for the protocol.
+ HandleCount = HandleBufferSize / sizeof (EFI_HANDLE);
+
+ //
+ // Loop to get the count and length of the error source descriptors.
+ //
+ // This loop collects and adds the length of error source descriptors and
+ // its count from all the the MM drivers implementing HEST error source.
+ // descriptor protocol. The total length and count values retrieved help
+ // to determine weather the CommBuffer is big enough to hold the descriptor
+ // information.
+ // As mentioned in the HEST error source descriptor protocol definition,
+ // Buffer parameter set to NULL ensures only length and the count values
+ // are returned from the driver and no error source information is copied to
+ // Buffer.
+ //
+ for (Index = 0; Index < HandleCount; ++Index) {
+ Status = mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ //
+ // Protocol called with Buffer parameter passed as NULL, must return
+ // error source length and error count for that driver.
+ //
+ Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+ HestErrSourceDescProtocolHandle,
+ NULL,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status == EFI_INVALID_PARAMETER) {
+ TotalSourceLength += SourceLength;
+ TotalSourceCount += SourceCount;
+ }
+ }
+
+ // Set the count and length in the error source descriptor.
+ ErrorSourceInfoList = (HEST_ERROR_SOURCE_DESC_INFO *)(CommBuffer);
+ ErrorSourceInfoList->ErrSourceDescCount = TotalSourceCount;
+ ErrorSourceInfoList->ErrSourceDescSize = TotalSourceLength;
+
+ //
+ // Check the size of CommBuffer, it should atleast be of size
+ // TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE.
+ //
+ TotalSourceLength = TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
+ if ((*CommBufferSize) < TotalSourceLength) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Invalid CommBufferSize parameter\n",
+ __FUNCTION__
+ ));
+ FreePool (HandleBuffer);
+ return EFI_BUFFER_TOO_SMALL;
+ }
+
+ //
+ // CommBuffer size is adequate to return all the error source descriptors.
+ // So go ahead and populate it with the error source descriptor information.
+ //
+
+ // Buffer pointer to append the Error Descriptors data.
+ ErrorSourcePtr = ErrorSourceInfoList->ErrSourceDescList;
+
+ //
+ // Loop to retrieve error source descriptors information.
+ //
+ // Calls into each MM driver that implement the HEST error source descriptor
+ // protocol. Here the Buffer parameter passed to the protocol service is
+ // valid. So the MM driver when called copies the descriptor information.
+ //
+ for (Index = 0; Index < HandleCount; ++Index) {
+ Status = mMmst->MmHandleProtocol (
+ HandleBuffer[Index],
+ &gMmHestErrorSourceDescProtocolGuid,
+ (VOID **)&HestErrSourceDescProtocolHandle
+ );
+ if (EFI_ERROR (Status)) {
+ continue;
+ }
+
+ Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
+ HestErrSourceDescProtocolHandle,
+ (VOID **)&ErrorSourcePtr,
+ &SourceLength,
+ &SourceCount
+ );
+ if (Status == EFI_SUCCESS) {
+ ErrorSourcePtr += SourceLength;
+ }
+ }
+
+ // Free the buffer holding all the protocol handles.
+ FreePool (HandleBuffer);
+
+ return Status;
+}
+
+/**
+ Entry point for this Stanalone MM driver.
+
+ Registers an Mmi handler that retrieves the error source descriptors from all
+ the MM drivers implementing the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL.
+
+ @param[in] ImageHandle The firmware allocated handle for the EFI image.
+ @param[in] SystemTable A pointer to the EFI System Table.
+
+ @retval EFI_SUCCESS The entry point registered handler successfully.
+ @retval Other Some error occurred when executing this entry point.
+**/
+EFI_STATUS
+EFIAPI
+StandaloneMmHestErrorSourceInitialize (
+ IN EFI_HANDLE ImageHandle,
+ IN EFI_MM_SYSTEM_TABLE *SystemTable
+ )
+{
+ EFI_HANDLE DispatchHandle;
+ EFI_STATUS Status;
+
+ ASSERT (SystemTable != NULL);
+ mMmst = SystemTable;
+
+ Status = mMmst->MmiHandlerRegister (
+ HestErrorSourcesInfoMmiHandler,
+ &gMmHestGetErrorSourceInfoGuid,
+ &DispatchHandle
+ );
+ if (EFI_ERROR(Status)) {
+ DEBUG ((
+ DEBUG_ERROR,
+ "%a: Mmi handler registration failed with status : %r\n",
+ __FUNCTION__,
+ Status
+ ));
+ return Status;
+ }
+
+ return Status;
+}
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
` (2 preceding siblings ...)
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM Omkar Anand Kulkarni
@ 2021-08-24 5:34 ` Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file Omkar Anand Kulkarni
2021-10-04 17:46 ` [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Sami Mujawar
5 siblings, 1 reply; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:34 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
Add helper macros for the generation of the HEST ACPI table. Macros to
initialize the HEST GHESv2 Notification Structure and Error Status
Structure are introduced.
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
---
EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h b/EmbeddedPkg/Include/Library/AcpiLib.h
index c142446d9d59..6de067823011 100644
--- a/EmbeddedPkg/Include/Library/AcpiLib.h
+++ b/EmbeddedPkg/Include/Library/AcpiLib.h
@@ -22,6 +22,7 @@
#define ARM_GAS16(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 16, 0, EFI_ACPI_5_0_WORD, Address }
#define ARM_GAS32(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 32, 0, EFI_ACPI_5_0_DWORD, Address }
#define ARM_GASN(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 0, 0, EFI_ACPI_5_0_DWORD, Address }
+#define ARM_GAS64(Address) { EFI_ACPI_6_3_SYSTEM_MEMORY, 64, 0, EFI_ACPI_6_3_QWORD, Address }
//
// Macros for the Multiple APIC Description Table (MADT)
@@ -89,6 +90,25 @@
WatchdogTimerGSIV, WatchdogTimerFlags \
}
+//
+// HEST table GHESv2 type related structure.
+// Helper Macro to initialize the HEST GHESv2 Notification Structure.
+// Refer Table 18-394 in ACPI Specification, Version 6.3.
+//
+#define EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT(Type, \
+ PollInterval, EventId) \
+ { \
+ Type, \
+ sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE), \
+ {0, 0, 0, 0, 0, 0, 0}, /* ConfigurationWriteEnable */ \
+ PollInterval, \
+ EventId, \
+ 0, /* Poll Interval Threshold Value */ \
+ 0, /* Poll Interval Threshold Window */ \
+ 0, /* Error Threshold Value */ \
+ 0 /* Error Threshold Window */ \
+ }
+
typedef
BOOLEAN
(EFIAPI *EFI_LOCATE_ACPI_CHECK) (
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
` (3 preceding siblings ...)
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation Omkar Anand Kulkarni
@ 2021-08-24 5:34 ` Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-10-04 17:46 ` [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Sami Mujawar
5 siblings, 1 reply; 13+ messages in thread
From: Omkar Anand Kulkarni @ 2021-08-24 5:34 UTC (permalink / raw)
To: devel; +Cc: Ard Biesheuvel, Sami Mujawar
Added a readme file that explains the software framework for dynamic
generation of HEST table.
Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
---
ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md | 66 ++++++++++++++++++++
1 file changed, 66 insertions(+)
diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md b/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
new file mode 100644
index 000000000000..1b6f0713cb9a
--- /dev/null
+++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
@@ -0,0 +1,66 @@
+Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
+(SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
+patch series introduces a framework to build and install the HEST ACPI table
+dynamically.
+
+The following figure illustrates the possible usage of the dyanamic
+generation of HEST ACPI table.
+
+ NS | S
++--------------------------------------+--------------------------------------+
+| | |
+|+-------------------------------------+---------------------+ |
+|| +---------------------+--------------------+| |
+|| | | || |
+|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
+|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
+|| | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
+|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
+|| |GHESv2 | || |
+|| +---------------------+--------------------+| |
+|| +--------------------+ | | |
+|| |PlatformErrorHandler| | | |
+|| | DXE | | | |
+|| +--------------------+ | | |
+||FF FWK | | |
+|+-------------------------------------+---------------------+ |
+| | |
++--------------------------------------+--------------------------------------+
+ |
+ Figure: Dynamic Hest Table Generation.
+
+All the hardware error sources are added to HEST table as GHESv2[3] error source
+descriptors. The framework comprises of following DXE and MM drivers:
+
+- HestTableDxe:
+ Builds HEST table header and allows appending error source descriptors to the
+ HEST table. Also provides protocol interface to install the built HEST table.
+
+- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
+ These two drivers together retrieve all possible error source descriptors of
+ type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
+ protocol. Once all the descriptors are collected HestErrorSourceDxe appends
+ it to HEST table using HestTableDxe driver.
+
+- PlatformErrorHandlerDxe:
+ Builds and installs SDEI ACPI table. This driver does not initialize(load)
+ until HestErrorSourceDxe driver has finished appending all possible GHESv2
+ error source descriptors to the HEST table. Once that is complete using the
+ HestTableDxe driver it installs the HEST table.
+
+This patch series provides reference implementation for DMC-620 Dynamic Memory
+Controller[4] that has RAS feature enabled. This is platform code
+implemented as Standalone MM driver in edk2-platforms.
+
+References:
+[1] : ACPI 6.3, Table 18-382, Hardware Error Source Table
+[2] : SDEI Platform Design Document, revision b, 10 Appendix C, ACPI table
+ definitions for SDEI
+[3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure
+[4] : DMC620 Dynamic Memory Controller, revision r1p0
+[5] : UEFI Reference Specification 2.8, Appendix N - Common Platform Error
+ Record
+[6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error Section
+
+Link to github branch with the patches in this series -
+https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2
--
2.17.1
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
` (4 preceding siblings ...)
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file Omkar Anand Kulkarni
@ 2021-10-04 17:46 ` Sami Mujawar
5 siblings, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:46 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
Hi Omkar,
The subject for this patch series should be fixed. These patches are not
for edk2-platforms.
Regards,
Sami Mujawar
On 24/08/2021 06:33 AM, Omkar Anand Kulkarni wrote:
> Changes since v2:
> - Addressed the comments given by Sami.
> - Added Readme file with all cover letter information.
> - Rebased to the latest upstream code.
>
> Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
> (SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
> patch series introduces a framework to build and install the HEST ACPI table
> dynamically.
>
> The following figure illustrates the possible usage of the dyanamic
> generation of HEST ACPI table.
>
> NS | S
> +--------------------------------------+--------------------------------------+
> | | |
> |+-------------------------------------+---------------------+ |
> || +---------------------+--------------------+| |
> || | | || |
> || +-----------+ |+------------------+ | +-----------------+|| +-------------+|
> || |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
> || | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
> || +-----------+ |+------------------+ | +-----------------+|| +-------------+|
> || |GHESv2 | || |
> || +---------------------+--------------------+| |
> || +--------------------+ | | |
> || |PlatformErrorHandler| | | |
> || | DXE | | | |
> || +--------------------+ | | |
> ||FF FWK | | |
> |+-------------------------------------+---------------------+ |
> | | |
> +--------------------------------------+--------------------------------------+
> |
> Figure: Firmware First Error Handling approach.
>
> All the hardware error sources are added to HEST table as GHESv2[3] error source
> descriptors. The framework comprises of following DXE and MM drivers:
>
> - HestTableDxe:
> Builds HEST table header and allows appending error source descriptors to the
> HEST table. Also provides protocol interface to install the built HEST table.
>
> - HestErrorSourceDxe & HestErrorSourceStandaloneMM:
> These two drivers together retrieve all possible error source descriptors of
> type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
> protocol. Once all the descriptors are collected HestErrorSourceDxe appends
> it to HEST table using HestTableDxe driver.
>
> Link to github branch with the patches in this series -
> https://github.com/omkkul01/edk2/tree/ras_firware_first_edk2-platforms_v3
>
> Omkar Anand Kulkarni (5):
> MdeModulePkg: Allow dynamic generation of HEST ACPI table
> ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
> ArmPlatformPkg: retreive error source descriptors from MM
> EmbeddedPkg: Add helpers for HEST table generation
> ArmPlatformPkg: Add Readme file
>
> ArmPlatformPkg/ArmPlatformPkg.dec | 10 +
> MdeModulePkg/MdeModulePkg.dec | 3 +
> .../HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
> .../HestErrorSourceStandaloneMm.inf | 51 +++
> .../Universal/Apei/HestDxe/HestDxe.inf | 49 +++
> .../HestMmErrorSourceCommon.h | 37 ++
> .../Include/Protocol/HestErrorSourceInfo.h | 64 ++++
> EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++
> MdeModulePkg/Include/Protocol/HestTable.h | 71 ++++
> MdePkg/Include/Protocol/MmCommunication.h | 2 +
> .../HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++
> .../HestErrorSourceStandaloneMm.c | 312 +++++++++++++++++
> MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++
> .../Drivers/HestMmErrorSources/Readme.md | 66 ++++
> 14 files changed, 1357 insertions(+)
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
> create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
> create mode 100644 ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
> create mode 100644 MdeModulePkg/Include/Protocol/HestTable.h
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
> create mode 100644 MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
> create mode 100644 ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
>
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of HEST ACPI table
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
@ 2021-10-04 17:46 ` Sami Mujawar
2024-04-09 4:57 ` [edk2-devel] " Dhaval Sharma
1 sibling, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:46 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
[-- Attachment #1: Type: text/plain, Size: 18324 bytes --]
Hi Omkar,
Please find my feedback inline marked [SAMI].
Regards,
Sami Mujawar
On 24/08/2021 06:33 AM, 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>
> ---
> MdeModulePkg/MdeModulePkg.dec | 3 +
> MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf | 49 +++
> MdeModulePkg/Include/Protocol/HestTable.h | 71 +++++
> MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c | 318 ++++++++++++++++++++
> 4 files changed, 441 insertions(+)
>
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index ad84421cf3f7..2cb4ba69d6bf 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -663,6 +663,9 @@
> ## Include/Protocol/VariablePolicy.h
> gEdkiiVariablePolicyProtocolGuid = { 0x81D1675C, 0x86F6, 0x48DF, { 0xBD, 0x95, 0x9A, 0x6E, 0x4F, 0x09, 0x25, 0xC3 } }
>
> + ## Arm Platform HEST table generation protocol
> + gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
> +
> [PcdsFeatureFlag]
> ## Indicates if the platform can support update capsule across a system reset.<BR><BR>
> # TRUE - Supports update capsule across a system reset.<BR>
> diff --git a/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.inf
> new file mode 100644
> index 000000000000..91c7385bf7ff
> --- /dev/null
> +++ b/MdeModulePkg/Universal/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
[SAMI] Use latest INF version.
> + 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
[SAMI] Is SgiPlatform.dec, ArmPkg.dec and ArmPlatformPkg.dec needed here?
> +
> +[LibraryClasses]
> + ArmLib
[SAMI] Is ArmLib needed here?
> + 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/MdeModulePkg/Include/Protocol/HestTable.h b/MdeModulePkg/Include/Protocol/HestTable.h
> new file mode 100644
> index 000000000000..3b2e1f7d9203
> --- /dev/null
> +++ b/MdeModulePkg/Include/Protocol/HestTable.h
[SAMI] Should this file be renamed to HestTableProtocol.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;
[SAMI] I think HEST_TABLE_PROTOCOL should be renamed
toEDKII_HEST_TABLE_PROTOCOL.
Similarly, the function pointers to the interfaces defined by the
protocol should also be prefixed with EDKII_, e.g. EDKII_INSTALL_HEST_TABLE.
[/SAMI]
> +
> +extern EFI_GUID gHestTableProtocolGuid;
> +#endif // HEST_TABLE_H_
> diff --git a/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
> new file mode 100644
> index 000000000000..87f21acf61f4
> --- /dev/null
> +++ b/MdeModulePkg/Universal/Apei/HestDxe/HestDxe.c
> @@ -0,0 +1,318 @@
> +/** @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>
[SAMI] Do we need ArmLib.h here?
> +#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;
> +
> +/**
> + 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
> + )
> +{
> + EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *HestHeader;
> +
> + //
> + // Allocate memory for the HEST table header.
> + //
> + mHestDriverData.HestTable =
> + AllocateZeroPool (sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER));
> + if (mHestDriverData.HestTable == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + mHestDriverData.CurrentTableSize =
> + sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER);
> +
> + HestHeader = (EFI_ACPI_6_3_HARDWARE_ERROR_SOURCE_TABLE_HEADER *)
> + mHestDriverData.HestTable;
> +
> + //
> + // 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)
> + );
> + HestHeader->Header.OemTableId = FixedPcdGet64 (PcdAcpiDefaultOemTableId);
> + HestHeader->Header.OemRevision = PcdGet32 (PcdAcpiDefaultOemRevision);
> + HestHeader->Header.CreatorId = PcdGet32 (PcdAcpiDefaultCreatorId);
> + HestHeader->Header.CreatorRevision = PcdGet32 (PcdAcpiDefaultCreatorRevision);
> + HestHeader->ErrorSourceCount = 0;
> +
> + 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 = ReallocatePool (
> + mHestDriverData.CurrentTableSize,
> + NewTableSize,
> + mHestDriverData.HestTable
> + );
[SAMI] I think a Linked list based implementation should be used here,
instead of using ReallocatePool. This should not be too complicated as
the HEST_TABLE_PROTOCOL design already makes this easy. The
AppendErrorSourceDescriptor() interface should probably be renamed to
AddErrorSourceDescriptor() and can be used to collect the error source
information and add it to a LinkedList. The InstallHestTable() can
iterate the linked list to collect the error sources and publist the
ACPI table. Also, there is a LinkedList implementation in
edk2\MdePkg\BaseLib\LinkedList.c that can be used.
[/SAMI]
> + if (mHestDriverData.HestTable == NULL) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to reallocate memory for HEST table\n",
> + __FUNCTION__
> + ));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // 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
> + )
> +{
> + 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_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;
> +
> + 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;
[SAMI] There would be a memory leak here as mHestDriverData.HestTableis
not freed. So just remove the return statement from here.
> + }
> +
> + //
> + // Free the HEST table buffer.
> + //
> + FreePool (mHestDriverData.HestTable);
> + DEBUG ((
> + DEBUG_INFO,
> + "HestDxe: Installed HEST table \n"
> + ));
> + return Status;
> +}
> +
> +//
> +// HEST table generation protocol instance.
> +//
> +STATIC HEST_TABLE_PROTOCOL mHestProtocol = {
> + AppendErrorSourceDescriptor,
> + InstallHestAcpiTable
> +};
> +
> +/**
> + 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;
[SAMI] This return statement can be removed as there is already one at
the end of the function.
> + }
> +
> + return Status;
> +}
[-- Attachment #2: Type: text/html, Size: 19495 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL Omkar Anand Kulkarni
@ 2021-10-04 17:47 ` Sami Mujawar
0 siblings, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:47 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
[-- Attachment #1: Type: text/plain, Size: 4949 bytes --]
Hi Omkar,
Please find my feedback inline marked [SAMI].
Regards,
Sami Mujawar
On 24/08/2021 06:34 AM, Omkar Anand Kulkarni wrote:
> Add the protocol definition of the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL
> protocol. This protocol can be implemented by MM drivers to publish
> error source descriptors that have to be populated into HEST table.
>
> Co-authored-by: Thomas Abraham <thomas.abraham@arm.com>
> Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
> ---
> ArmPlatformPkg/ArmPlatformPkg.dec | 3 +
> ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h | 64 ++++++++++++++++++++
> 2 files changed, 67 insertions(+)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 3a25ddcdc8ca..446d93b880f9 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -127,3 +127,6 @@
> gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy|300000000|UINT32|0x00000022
>
> gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
> +
> +[Protocols.common]
> + gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
> diff --git a/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
> new file mode 100644
> index 000000000000..86662ea7af57
> --- /dev/null
> +++ b/ArmPlatformPkg/Include/Protocol/HestErrorSourceInfo.h
[SAMI] Can this file be rename to MmHestErrorSourceProtocol.h?
> @@ -0,0 +1,64 @@
> +/** @file
> + MM protocol to get the secure error source descriptor information.
> +
> + MM Drivers must implement this protocol in order to publish secure side
> + error source descriptor information to OSPM through the HEST ACPI table.
> +
> + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef MM_HEST_ERROR_SOURCE_DESC_
> +#define MM_HEST_ERROR_SOURCE_DESC_
> +
> +#define MM_HEST_ERROR_SOURCE_DESC_PROTOCOL_GUID \
> + { \
> + 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } \
> + }
> +
> +typedef struct MmHestErrorSourceDescProtocol
> + MM_HEST_ERROR_SOURCE_DESC_PROTOCOL;
[SAMI] Can this be renamed to EDKII_MM_HEST_ERROR_SOURCE_PROTOCOL ?
> +
> +/**
> + Get HEST Secure Error Source Descriptors.
> +
> + The MM drivers implementing this protocol must convey the total count and
> + total length of the error sources the driver has along with the actual error
> + source descriptor(s).
> +
> + Passing NULL as Buffer parameter shall return EFI_INVALID_PARAMETR with the
> + total length and count of the error source descriptor(s) it supports.
[SAMI]I think the error code EFI_BUFFER_TOO_SMALL should be returned
when the Buffer is NULL and *ErrorSourcesLength is 0.
i.e. To determine the size of required buffer, the caller shall call
this interface with Buffer = NULL and *ErrorSourcesLength = 0. In
response this interface shall return an error code EFI_BUFFER_TOO_SMALL
with the required buffer length in ErrorSourcesLength and count in
ErrorSourcesCount.
The EFI_INVALID_PARAMETR should be returned when a parameter is invalid.
e.g. ErrorSourcesLength should be an IN OUT parameter and cannot be NULL.
The ErrorSourcesCount could be an optional parameter and can be NULL.
Can you revisit the design of this interface, please?
[/SAMI]
> +
> + @param[in] This MM_HEST_ERROR_SOURCE_DESC_PROTOCOL instance.
> + @param[out] Buffer Buffer to be appended with the error
> + source descriptors information.
> + @param[out] ErrorSourcesLength Total length of all the error source
> + descriptors.
> + @param[out] ErrorSourceCount Count of total error source descriptors
> + supported by the driver.
> +
> + retval EFI_SUCCESS If the Buffer is valid and is filled with valid
> + Error Source descriptor data.
> + retval EFI_INVALID_PARAMTER Buffer is NULL.
> + retval Other If no error source descriptor information is
> + available.
> +**/
> +typedef
> +EFI_STATUS
> +(EFIAPI *MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS) (
[SAMI] Can this be renamed to EDKII_MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS?
> + IN MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *This,
> + OUT VOID **Buffer,
> + OUT UINTN *ErrorSourcesLength,
> + OUT UINTN *ErrorSourcesCount
> + );
> +
> +//
> +// Protocol declaration
> +//
> +struct MmHestErrorSourceDescProtocol {
> + MM_HEST_GET_ERROR_SOURCE_DESCRIPTORS GetHestErrorSourceDescriptors;
> +};
> +
> +extern EFI_GUID gMmHestErrorSourceDescProtocolGuid;
> +
> +#endif // MM_HEST_ERROR_SOURCE_DESC_
[-- Attachment #2: Type: text/html, Size: 5998 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM Omkar Anand Kulkarni
@ 2021-10-04 17:47 ` Sami Mujawar
0 siblings, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:47 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
[-- Attachment #1: Type: text/plain, Size: 32614 bytes --]
Hi Omkar,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 24/08/2021 06:34 AM, Omkar Anand Kulkarni wrote:
> Add a driver that retreives error source descriptors from MM and
> populates those into the HEST ACPI table. The error source descriptors
> that are available from the MM side are retreived using MM Communicate 2
> protocol.
>
> The first call into the MM returns the size of MM Communicate buffer
> required to hold all error source descriptor info. The communication
> buffer of that size is then allocated and the second call into MM
> returns the error source descriptors in the communication buffer.
> The retreived error source descriptors are then appended to 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 | 7 +
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf | 45 +++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf | 51 ++++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h | 37 +++
> MdePkg/Include/Protocol/MmCommunication.h | 2 +
[SAMI] I think this patch needs to be split as MmCommunication.h is from
MdePkg while all other files are in ArmPlatformPkg.
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c | 309 +++++++++++++++++++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c | 312 ++++++++++++++++++++
> 7 files changed, 763 insertions(+)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 446d93b880f9..a400f175726e 100644
> --- a/ArmPlatformPkg/ArmPlatformPkg.dec
> +++ b/ArmPlatformPkg/ArmPlatformPkg.dec
> @@ -52,6 +52,8 @@
>
> [Guids.common]
> gArmPlatformTokenSpaceGuid = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
> + gArmPlatformHestErrorSourcesGuid = { 0x76b8ab43, 0x822d, 0x4b00, { 0x9f, 0xd0, 0xf4, 0xa5, 0x35, 0x82, 0x47, 0x0a } }
> + gMmHestGetErrorSourceInfoGuid = { 0x7d602951, 0x678e, 0x4cc4, { 0x98, 0xd9, 0xe3, 0x76, 0x04, 0xf6, 0x93, 0x0d } }
>
> [PcdsFeatureFlag.common]
> gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOOLEAN|0x00000004
> @@ -128,5 +130,10 @@
>
> gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
>
> +[PcdsFixedAtBuild, PcdsPatchableInModule]
> + ## ACPI CPER memory space
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000000|UINT64|0x00000046
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000000|UINT64|0x00000047
> +
> [Protocols.common]
> gMmHestErrorSourceDescProtocolGuid = { 0x560bf236, 0xa4a8, 0x4d69, { 0xbc, 0xf6, 0xc2, 0x97, 0x24, 0x10, 0x9d, 0x91 } }
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
> new file mode 100644
> index 000000000000..b16ff2353aa5
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
> @@ -0,0 +1,45 @@
> +## @file
> +# DXE driver to get secure error sources.
> +#
> +# DXE driver to retrieve the error source descriptors from Standalone MM and
> +# append those to the HEST table.
> +#
> +# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
[SAMI] Use latest INF version.
> + BASE_NAME = HestMmErrorSourceDxe
> + FILE_GUID = 76b8ab43-822d-4b00-9fd0-f4a53582470a
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = HestErrorSourceInitialize
> +
> +[Sources.common]
> + HestErrorSourceDxe.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + ArmPlatformPkg/ArmPlatformPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + StandaloneMmPkg/StandaloneMmPkg.dec
> +
> +[LibraryClasses]
> + BaseMemoryLib
> + DebugLib
> + DxeServicesTableLib
> + UefiDriverEntryPoint
> + UefiLib
> +
> +[Guids]
> + gMmHestGetErrorSourceInfoGuid ## PRODUCES
> +
> +[Protocols]
> + gHestTableProtocolGuid ## CONSUMES
> + gEfiMmCommunication2ProtocolGuid
> +
> +[Depex]
> + gHestTableProtocolGuid AND gEfiMmCommunication2ProtocolGuid
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
> new file mode 100644
> index 000000000000..9d566de9bec3
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf
> @@ -0,0 +1,51 @@
> +## @file
> +# HEST error source gateway Standalone MM driver.
> +#
> +# Collects HEST error source descriptors,by communicating with all the MM
> +# drivers implementing the HEST error source descriptor protocol.
> +#
> +# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
[SAMI] Use latest INF version.
> + BASE_NAME = HestErrorSourceStandaloneMm
> + FILE_GUID = 3ddbebcc-9841-4ef8-87fa-305843c1922d
> + MODULE_TYPE = MM_STANDALONE
> + VERSION_STRING = 1.0
> + PI_SPECIFICATION_VERSION = 0x00010032
> + ENTRY_POINT = StandaloneMmHestErrorSourceInitialize
> +
> +[Sources]
> + HestErrorSourceStandaloneMm.c
> +
> +[Packages]
> + ArmPkg/ArmPkg.dec
> + ArmPlatformPkg/ArmPlatformPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + StandaloneMmPkg/StandaloneMmPkg.dec
> +
> +[LibraryClasses]
> + ArmLib
> + ArmSvcLib
> + BaseMemoryLib
> + DebugLib
> + MemoryAllocationLib
> + StandaloneMmDriverEntryPoint
> +
> +[Protocols]
> + gMmHestErrorSourceDescProtocolGuid
> +
> +[Guids]
> + gMmHestGetErrorSourceInfoGuid ##PRODUCES
> + gEfiStandaloneMmNonSecureBufferGuid
> +
> +[FixedPcd]
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize
> +
> +[Depex]
> + TRUE
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
> new file mode 100644
> index 000000000000..6ddc6bd21922
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h
> @@ -0,0 +1,37 @@
> +/** @file
> + Data structures for error source descriptor information.
> +
> + This data structure forms the CommBuffer part of the MM Communication
> + protocol used for communicating the Hardware Error sources form MM to
> + Non-MM environment.
> +
> + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#ifndef HEST_ERROR_SOURCE_DESCRIPTOR_H_
> +#define HEST_ERROR_SOURCE_DESCRIPTOR_H_
> +
> +#define HEST_ERROR_SOURCE_DESC_INFO_SIZE \
> + (OFFSET_OF (HEST_ERROR_SOURCE_DESC_INFO, ErrSourceDescList))
> +
> +//
> +// Data Structure to communicate the error source descriptor information from
> +// Standalone MM.
> +//
> +typedef struct {
> + //
> + // Total count of error source descriptors.
> + //
> + UINTN ErrSourceDescCount;
> + //
> + // Total size of all the error source descriptors.
> + //
> + UINTN ErrSourceDescSize;
> + //
> + // Array of error source descriptors that is ErrSourceDescSize in size.
> + //
> + UINT8 ErrSourceDescList[1];
> +} HEST_ERROR_SOURCE_DESC_INFO;
> +
> +#endif // HEST_ERROR_SOURCE_DESCRIPTOR_H_
> diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h
> index 34c3e2b5a9e3..acde54971fd9 100644
> --- a/MdePkg/Include/Protocol/MmCommunication.h
> +++ b/MdePkg/Include/Protocol/MmCommunication.h
> @@ -12,6 +12,8 @@
> #ifndef _MM_COMMUNICATION_H_
> #define _MM_COMMUNICATION_H_
>
> +#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data))
> +
> #pragma pack(1)
>
> ///
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
> new file mode 100644
> index 000000000000..28de8daf2f5e
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
> @@ -0,0 +1,309 @@
> +/** @file
> + Collects and appends the HEST error source descriptors from the MM drivers.
> +
> + The drivers entry point locates the MM Communication protocol and calls into
> + Standalone MM to get the HEST error sources length and count. It also
> + retrieves descriptor information. The information is then used to build the
> + HEST table using the HEST table generation protocol.
> +
> + This driver collects the secure error source descriptor information from the
> + MM drviers that implement HEST error source protocol. Instead of directly
> + communicating with the individual MM drivers, it calls into
> + HestErrorSourceStandaloneMM driver which is a gatway MM driver. This MM driver
> + in-turn communicates with individual MM drivers collecting the error source
> + descriptor information.
> +
> + Once all the error source descriptor information is retrieved the driver
> + appends the descriptors to HEST table using the HestDxe driver.
> +
> + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/DxeServicesTableLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Protocol/MmCommunication2.h>
> +#include <Protocol/HestTable.h>
> +#include "HestMmErrorSourceCommon.h"
> +
> +STATIC HEST_TABLE_PROTOCOL *mHestProtocol;
> +STATIC EFI_MM_COMMUNICATION2_PROTOCOL *mMmCommunication2;
> +
> +/**
> + Retrieve the error source descriptors from Standalone MM.
> +
> + Initialize the MM comminication buffer by assigning the MM service to
> + invoke as gMmHestGetErrorSourceInfoGuid. Use the MM communication
> + protocol to retrieve the error source descriptors.
> +
> + @param[in] CommBuffSize Size of communicate buffer.
> + @param[in, out] CommBuffer The communicate buffer.
> +
> + @retval EFI_SUCCESS MM Communicate protocol call successful.
> + @retval Other MM Communicate protocol call failed.
> +**/
> +STATIC
> +EFI_STATUS
> +GetErrorSourceDescriptors (
> + IN UINTN CommBuffSize,
> + IN OUT EFI_MM_COMMUNICATE_HEADER **CommBuffer
> + )
> +{
> + EFI_STATUS Status;
> +
[SAMI] It may be good to add a check to see if CommBuffSize is at least
the minimum required size.
> + //
> + // Initialize the CommBuffer with MM Communicate metadata.
> + //
> + CopyGuid (&(*CommBuffer)->HeaderGuid, &gMmHestGetErrorSourceInfoGuid);
> + (*CommBuffer)->MessageLength =
> + CommBuffSize -
> + sizeof ((*CommBuffer)->HeaderGuid) -
> + sizeof ((*CommBuffer)->MessageLength);
> +
> + //
> + // Call into the Standalone MM using the MM Communicate protocol.
> + //
> + Status = mMmCommunication2->Communicate (
> + mMmCommunication2,
> + NULL,
> + (VOID *)*CommBuffer,
> + NULL
> + );
> +
[SAMI] Unless you intend to assert or log the status code you could
directly return the value returned by mMmCommunication2->Communicate().
> + return Status;
> +}
> +
> +/**
> + Collect HEST error source descriptors from all Standalone MM drivers and
> + append them to the HEST table.
> +
> + Use MM Communication Protocol to communicate and collect the error source
> + descriptor information from Standalone MM. Check for the required buffer size
> + returned by the MM driver. Allocate buffer of adequate size and call again
> + into MM.
> +
> + @retval EFI_SUCCESS Successful to collect and append the error
> + source.
> + descriptors to HEST table.
> + @retval EFI_OUT_OF_RESOURCES Memory allocation failure.
> + @retval Other For any other error.
> +**/
> +STATIC
> +EFI_STATUS
> +AppendMmErrorSources (
> + VOID
> + )
> +{
> + EFI_MM_COMMUNICATE_HEADER *CommunicationHeader = NULL;
> + HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceDescInfo;
> + EFI_STATUS Status;
> + UINTN CommBufferSize;
> +
> + //
> + // Retrieve the count, length and the actual eror source descriptors from
> + // the MM drivers. Do this by performing two MM Communicate calls, in the
> + // first call pass CommBuffer which is atleast of the size of error source
> + // descriptor info structure. Followed by another communicate call with
> + // CommBuffer allocated to required buffer size to hold all descriptors.
> + //
> + // Allocate CommBuffer atleast the size of error source descriptor info
> + // structure.
> + CommBufferSize =
> + MM_COMMUNICATE_HEADER_SIZE + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
> + CommunicationHeader = AllocateZeroPool (CommBufferSize);
> + if (CommunicationHeader == NULL) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to allocate memory for CommunicationHeader\n",
> + __FUNCTION__
> + ));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Make the first MM Communicate call to HestErrorSourceStandaloneMM gateway
> + // driver, which returns the required buffer size adequate to hold all the
> + // desctriptor information.
> + //
> + Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
> + if ((EFI_ERROR (Status)) &&
> + (Status != EFI_BUFFER_TOO_SMALL)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: MM Communicate protocol call failed, status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + FreePool (CommunicationHeader);
> + return Status;
> + }
> +
> + // Check for the length of Error Source descriptors.
> + ErrorSourceDescInfo =
> + (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
> + if ((ErrorSourceDescInfo->ErrSourceDescSize == 0) ||
> + (ErrorSourceDescInfo->ErrSourceDescCount == 0)) {
> + DEBUG ((
> + DEBUG_INFO,
> + "HesErrorSourceDxe: HEST error source(s) not found\n"
> + ));
> + FreePool (CommunicationHeader);
> + return EFI_NOT_FOUND;
> + }
> +
> + //
> + // Allocate CommBuffer of required size to accomodate all the error source
> + // descriptors. Required size of communication buffer =
> + // MM communicate metadata. + (error source desc info struct + error source
> + // descriptor size).
> + //
> +
> + CommBufferSize =
> + MM_COMMUNICATE_HEADER_SIZE +
> + HEST_ERROR_SOURCE_DESC_INFO_SIZE +
> + ErrorSourceDescInfo->ErrSourceDescSize;
> +
> + // Free old MM Communicate buffer and allocate a new buffer of required size.
> + FreePool (CommunicationHeader);
> + CommunicationHeader = AllocateZeroPool (CommBufferSize);
> + if (CommunicationHeader == NULL) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to allocate memory for CommunicationHeader\n",
> + __FUNCTION__
> + ));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + //
> + // Make second MM Communicate call to HestErrorSourceStandaloneMM driver to
> + // get the error source descriptors from the MM drivers.
> + //
> + Status = GetErrorSourceDescriptors (CommBufferSize, &CommunicationHeader);
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: MM Communicate protocol failed, status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + FreePool (CommunicationHeader);
> + return Status;
> + }
> +
> + //
> + // Retrieve the HEST error source descriptor information. Ensure that there
> + // is a valid list of error source descriptors.
> + //
> + ErrorSourceDescInfo =
> + (HEST_ERROR_SOURCE_DESC_INFO *)(CommunicationHeader->Data);
> + if (ErrorSourceDescInfo->ErrSourceDescList == NULL) {
> + DEBUG ((
> + DEBUG_INFO,
> + "HestErrorSourceDxe: Error source descriptor list is empty"
> + ));
> + FreePool (CommunicationHeader);
> + return EFI_NOT_FOUND;
> + }
> +
> + DEBUG ((
> + DEBUG_INFO,
> + "HestErrorSourceDxe: ErrorSources: TotalCount = %d TotalLength = %d \n",
> + ErrorSourceDescInfo->ErrSourceDescCount,
> + ErrorSourceDescInfo->ErrSourceDescSize
> + ));
> +
> + //
> + // Append the error source descriptors to HEST table using the HEST table
> + // generation protocol.
> + //
> + Status = mHestProtocol->AppendErrorSourceDescriptors (
> + ErrorSourceDescInfo->ErrSourceDescList,
> + ErrorSourceDescInfo->ErrSourceDescSize,
> + ErrorSourceDescInfo->ErrSourceDescCount
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to append error source(s), status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + }
> +
> + FreePool (CommunicationHeader);
> + return Status;
> +}
> +
> +/**
> + The Entry Point for HEST Error Source Dxe driver.
> +
> + Locates the HEST Table generation and MM Communication2 protocols. Using the
> + MM Communication2, the driver collects the Error Source Descriptor(s) from
> + Standalone MM. It then appends those Error Source Descriptor(s) to the Hest
> + table using the HEST Table generation protocol.
> +
> + @param[in] ImageHandle The firmware allocated handle for the Efi image.
> + @param[in] SystemTable A pointer to the Efi System Table.
> +
> + @retval EFI_SUCCESS The entry point is executed successfully.
> + @retval Other Some error occurred when executing this entry point.
> +**/
> +EFI_STATUS
> +EFIAPI
> +HestErrorSourceInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = gBS->LocateProtocol (
> + &gHestTableProtocolGuid,
> + NULL,
> + (VOID **)&mHestProtocol
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to locate HEST table generation protocol, status:%r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> +
> + Status = gBS->LocateProtocol (
> + &gEfiMmCommunication2ProtocolGuid,
> + NULL,
> + (VOID **)&mMmCommunication2
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to locate MMCommunication2 driver protocol, status:%r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
> + }
> +
> + //
> + // Append HEST error sources retrieved from StandaloneMM, if any, into the
> + // HEST ACPI table.
> + //
> + Status = AppendMmErrorSources ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed appending error source desc to HEST table, status:%r\n",
> + __FUNCTION__,
> + Status
> + ));
> + }
> + return EFI_SUCCESS;
> +}
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
> new file mode 100644
> index 000000000000..75dc31438180
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c
> @@ -0,0 +1,312 @@
> +/** @file
> + MM HEST error source gateway driver.
> +
> + This MM driver installs a handler which can be used to retrieve the error
> + source descriptors from the all MM drivers implementing the HEST error source
> + descriptor protocol.
> +
> + The MM driver acts as a single point of contact to collect secure hardware
> + error sources from the MM drivers. It loops over all the MM drivers that
> + implement HEST error source descriptor protocol and collects error source
> + descriptor information along with the error source count and length.
> +
> + Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> + SPDX-License-Identifier: BSD-2-Clause-Patent
> +**/
> +
> +#include <Base.h>
> +#include <Library/BaseLib.h>
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Protocol/HestErrorSourceInfo.h>
> +
> +#include "HestMmErrorSourceCommon.h"
> +
> +STATIC EFI_MM_SYSTEM_TABLE *mMmst = NULL;
> +
> +/**
> + Returns an array of handles that implement the HEST error source descriptor
> + protocol.
> +
> + Passing HandleBuffer as NULL will return the actual size of the buffer
> + required to hold the array of handles implementing the protocol.
> +
> + @param[in, out] HandleBufferSize The size of the HandleBuffer.
> + @param[out] HandleBuffer A pointer to the buffer containing the list
> + of handles.
> +
> + @retval EFI_SUCCESS The array of handles returned in HandleBuffer.
> + @retval EFI_NOT_FOUND No implementation present for the protocol.
> + @retval Other For any other error.
> +**/
> +STATIC
> +EFI_STATUS
> +GetHestErrorSourceProtocolHandles (
> + IN OUT UINTN *HandleBufferSize,
> + OUT EFI_HANDLE **HandleBuffer
> + )
> +{
> + EFI_STATUS Status;
> +
> + Status = mMmst->MmLocateHandle (
> + ByProtocol,
> + &gMmHestErrorSourceDescProtocolGuid,
> + NULL,
> + HandleBufferSize,
> + *HandleBuffer
> + );
> + if ((EFI_ERROR (Status)) &&
> + (Status != EFI_BUFFER_TOO_SMALL))
> + {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: No implementation of MmHestErrorSourceDescProtocol found, \
> + Status:%r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return EFI_NOT_FOUND;
> + }
> +
> + return Status;
> +}
> +
> +/**
> + Mmi handler to retrieve HEST error source descriptor information.
> +
> + Handler for Mmi service that returns the supported HEST error source
> + descriptors in MM. This handler populates the CommBuffer with the
> + list of all error source descriptors, prepended with the length and
> + the number of descriptors populated into CommBuffer.
> +
> + @param[in] DispatchHandle The unique handle assigned to this handler by
> + MmiHandlerRegister().
> + @param[in] Context Points to an optional handler context that
> + is specified when the handler was registered.
> + @param[in, out] CommBuffer Buffer used for communication of HEST error
> + source descriptors.
> + @param[in, out] CommBufferSize The size of the CommBuffer.
> +
> + @retval EFI_SUCCESS CommBuffer has valid data.
> + @retval EFI_BAD_BUFFER_SIZE CommBufferSize not adequate.
[SAMI] I think this should be EFI_BUFFER_TOO_SMALL, right?
> + @retval EFI_OUT_OF_RESOURCES System out of memory resources.
> + @retval EFI_INVALID_PARAMETER Invalid CommBufferSize recieved.
> + @retval Other For any other error.
> +**/
> +STATIC
> +EFI_STATUS
> +EFIAPI
> +HestErrorSourcesInfoMmiHandler (
> + IN EFI_HANDLE DispatchHandle,
> + IN CONST VOID *Context, OPTIONAL
> + IN OUT VOID *CommBuffer, OPTIONAL
> + IN UINTN *CommBufferSize OPTIONAL
> + )
> +{
> + MM_HEST_ERROR_SOURCE_DESC_PROTOCOL *HestErrSourceDescProtocolHandle;
> + HEST_ERROR_SOURCE_DESC_INFO *ErrorSourceInfoList;
> + EFI_HANDLE *HandleBuffer;
> + EFI_STATUS Status;
> + UINTN HandleCount;
> + UINTN HandleBufferSize;
> + UINTN Index;
> + UINTN SourceCount = 0;
> + UINTN SourceLength = 0;
> + VOID *ErrorSourcePtr;
> + UINTN TotalSourceLength = 0;
> + UINTN TotalSourceCount = 0;
> +
> + if (*CommBufferSize < HEST_ERROR_SOURCE_DESC_INFO_SIZE) {
> + //
> + // Ensures that the communication buffer has enough space to atleast hold
> + // the ErrSourceDescCount and ErrSourceDescSize elements of the
> + // HEST_ERROR_SOURCE_DESC_INFO structure.
> + //
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Invalid CommBufferSize parameter\n",
> + __FUNCTION__
> + ));
> + return EFI_INVALID_PARAMETER;
[SAMI] I don't understand why this check is needed. Can you explain, please?
> + }
> +
> + //
> + // Get all handles that implement the HEST error source descriptor protocol.
> + // Get the buffer size required to store list of handles for the protocol.
> + //
> + HandleBuffer = NULL;
> + HandleBufferSize = 0;
> + Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
> + if ((Status == EFI_NOT_FOUND) ||
> + (HandleBufferSize == 0))
> + {
> + return Status;
> + }
> +
> + // Allocate memory for HandleBuffer of size HandleBufferSize.
> + HandleBuffer = AllocateZeroPool (HandleBufferSize);
> + if (HandleBuffer == NULL) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to allocate memory for HandleBuffer\n",
> + __FUNCTION__
> + ));
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + // Get the list of handles.
> + Status = GetHestErrorSourceProtocolHandles (&HandleBufferSize, &HandleBuffer);
> + if ((EFI_ERROR (Status)) ||
> + (HandleBuffer == NULL))
> + {
> + FreePool (HandleBuffer);
> + return Status;
> + }
> +
> + // Count of handles for the protocol.
> + HandleCount = HandleBufferSize / sizeof (EFI_HANDLE);
> +
> + //
> + // Loop to get the count and length of the error source descriptors.
> + //
> + // This loop collects and adds the length of error source descriptors and
> + // its count from all the the MM drivers implementing HEST error source.
> + // descriptor protocol. The total length and count values retrieved help
> + // to determine weather the CommBuffer is big enough to hold the descriptor
> + // information.
> + // As mentioned in the HEST error source descriptor protocol definition,
> + // Buffer parameter set to NULL ensures only length and the count values
> + // are returned from the driver and no error source information is copied to
> + // Buffer.
> + //
> + for (Index = 0; Index < HandleCount; ++Index) {
> + Status = mMmst->MmHandleProtocol (
> + HandleBuffer[Index],
> + &gMmHestErrorSourceDescProtocolGuid,
> + (VOID **)&HestErrSourceDescProtocolHandle
> + );
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
> +
> + //
> + // Protocol called with Buffer parameter passed as NULL, must return
> + // error source length and error count for that driver.
> + //
[SAMI] The SourceLength and SourceCount variables must be initialised to
0 here.
> + Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
> + HestErrSourceDescProtocolHandle,
> + NULL,
> + &SourceLength,
> + &SourceCount
> + );
> + if (Status == EFI_INVALID_PARAMETER) {
[SAMI] Please see my earlier comment. EFI_INVALID_PARAMETER is not the
correct value to be returned here. It should beEFI_BUFFER_TOO_SMALL.
> + TotalSourceLength += SourceLength;
> + TotalSourceCount += SourceCount;
> + }
> + }
> +
> + // Set the count and length in the error source descriptor.
> + ErrorSourceInfoList = (HEST_ERROR_SOURCE_DESC_INFO *)(CommBuffer);
> + ErrorSourceInfoList->ErrSourceDescCount = TotalSourceCount;
> + ErrorSourceInfoList->ErrSourceDescSize = TotalSourceLength;
> +
> + //
> + // Check the size of CommBuffer, it should atleast be of size
> + // TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE.
> + //
> + TotalSourceLength = TotalSourceLength + HEST_ERROR_SOURCE_DESC_INFO_SIZE;
> + if ((*CommBufferSize) < TotalSourceLength) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Invalid CommBufferSize parameter\n",
> + __FUNCTION__
> + ));
> + FreePool (HandleBuffer);
> + return EFI_BUFFER_TOO_SMALL;
[SAMI] If EFI_BUFFER_TOO_SMALL is returned it is expected that the
caller will be provided with information on the size of the buffer
required. However, that does not seem to be the case here as
CommBufferSize is not an OUT parameter.
If this is an unrecoverable error, or if the caller is not expected to
probe the size then the return code should be EFI_BAD_BUFFER_SIZE and my
comment in the function documentation header can be ignored.
Can you check what is should happen here, please?
[/SAMI]
> + }
> +
> + //
> + // CommBuffer size is adequate to return all the error source descriptors.
> + // So go ahead and populate it with the error source descriptor information.
> + //
> +
> + // Buffer pointer to append the Error Descriptors data.
> + ErrorSourcePtr = ErrorSourceInfoList->ErrSourceDescList;
> +
> + //
> + // Loop to retrieve error source descriptors information.
> + //
> + // Calls into each MM driver that implement the HEST error source descriptor
> + // protocol. Here the Buffer parameter passed to the protocol service is
> + // valid. So the MM driver when called copies the descriptor information.
> + //
> + for (Index = 0; Index < HandleCount; ++Index) {
> + Status = mMmst->MmHandleProtocol (
> + HandleBuffer[Index],
> + &gMmHestErrorSourceDescProtocolGuid,
> + (VOID **)&HestErrSourceDescProtocolHandle
> + );
> + if (EFI_ERROR (Status)) {
> + continue;
> + }
> +
> + Status = HestErrSourceDescProtocolHandle->GetHestErrorSourceDescriptors (
> + HestErrSourceDescProtocolHandle,
> + (VOID **)&ErrorSourcePtr,
> + &SourceLength,
> + &SourceCount
> + );
> + if (Status == EFI_SUCCESS) {
[SAMI] if (!EFI_ERROR (Status)) ?
> + ErrorSourcePtr += SourceLength;
> + }
> + }
> +
> + // Free the buffer holding all the protocol handles.
> + FreePool (HandleBuffer);
> +
> + return Status;
> +}
> +
> +/**
> + Entry point for this Stanalone MM driver.
> +
> + Registers an Mmi handler that retrieves the error source descriptors from all
> + the MM drivers implementing the MM_HEST_ERROR_SOURCE_DESC_PROTOCOL.
> +
> + @param[in] ImageHandle The firmware allocated handle for the EFI image.
> + @param[in] SystemTable A pointer to the EFI System Table.
> +
> + @retval EFI_SUCCESS The entry point registered handler successfully.
> + @retval Other Some error occurred when executing this entry point.
> +**/
> +EFI_STATUS
> +EFIAPI
> +StandaloneMmHestErrorSourceInitialize (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_MM_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_HANDLE DispatchHandle;
> + EFI_STATUS Status;
> +
> + ASSERT (SystemTable != NULL);
> + mMmst = SystemTable;
> +
> + Status = mMmst->MmiHandlerRegister (
> + HestErrorSourcesInfoMmiHandler,
> + &gMmHestGetErrorSourceInfoGuid,
> + &DispatchHandle
> + );
> + if (EFI_ERROR(Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Mmi handler registration failed with status : %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
[SAMI] This return statment can be removed.
> + }
> +
> + return Status;
> +}
[-- Attachment #2: Type: text/html, Size: 33731 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation Omkar Anand Kulkarni
@ 2021-10-04 17:47 ` Sami Mujawar
0 siblings, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:47 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
Hi Omkar,
This patch looks good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 24/08/2021 06:34 AM, Omkar Anand Kulkarni wrote:
> Add helper macros for the generation of the HEST ACPI table. Macros to
> initialize the HEST GHESv2 Notification Structure and Error Status
> Structure are introduced.
>
> Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
> ---
> EmbeddedPkg/Include/Library/AcpiLib.h | 20 ++++++++++++++++++++
> 1 file changed, 20 insertions(+)
>
> diff --git a/EmbeddedPkg/Include/Library/AcpiLib.h b/EmbeddedPkg/Include/Library/AcpiLib.h
> index c142446d9d59..6de067823011 100644
> --- a/EmbeddedPkg/Include/Library/AcpiLib.h
> +++ b/EmbeddedPkg/Include/Library/AcpiLib.h
> @@ -22,6 +22,7 @@
> #define ARM_GAS16(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 16, 0, EFI_ACPI_5_0_WORD, Address }
> #define ARM_GAS32(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 32, 0, EFI_ACPI_5_0_DWORD, Address }
> #define ARM_GASN(Address) { EFI_ACPI_5_0_SYSTEM_MEMORY, 0, 0, EFI_ACPI_5_0_DWORD, Address }
> +#define ARM_GAS64(Address) { EFI_ACPI_6_3_SYSTEM_MEMORY, 64, 0, EFI_ACPI_6_3_QWORD, Address }
>
> //
> // Macros for the Multiple APIC Description Table (MADT)
> @@ -89,6 +90,25 @@
> WatchdogTimerGSIV, WatchdogTimerFlags \
> }
>
> +//
> +// HEST table GHESv2 type related structure.
> +// Helper Macro to initialize the HEST GHESv2 Notification Structure.
> +// Refer Table 18-394 in ACPI Specification, Version 6.3.
> +//
> +#define EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE_INIT(Type, \
> + PollInterval, EventId) \
> + { \
> + Type, \
> + sizeof (EFI_ACPI_6_3_HARDWARE_ERROR_NOTIFICATION_STRUCTURE), \
> + {0, 0, 0, 0, 0, 0, 0}, /* ConfigurationWriteEnable */ \
> + PollInterval, \
> + EventId, \
> + 0, /* Poll Interval Threshold Value */ \
> + 0, /* Poll Interval Threshold Window */ \
> + 0, /* Error Threshold Value */ \
> + 0 /* Error Threshold Window */ \
> + }
> +
> typedef
> BOOLEAN
> (EFIAPI *EFI_LOCATE_ACPI_CHECK) (
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file Omkar Anand Kulkarni
@ 2021-10-04 17:47 ` Sami Mujawar
0 siblings, 0 replies; 13+ messages in thread
From: Sami Mujawar @ 2021-10-04 17:47 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel; +Cc: Ard Biesheuvel, nd
Hi Omkar,
Thank you for adding this description. This is really helpful to
understand the framework.
I have a few minor suggestions marked inline as [SAMI].
With those changed,
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
On 24/08/2021 06:34 AM, Omkar Anand Kulkarni wrote:
> Added a readme file that explains the software framework for dynamic
> generation of HEST table.
>
> Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni@arm.com>
> ---
> ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md | 66 ++++++++++++++++++++
> 1 file changed, 66 insertions(+)
>
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md b/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
> new file mode 100644
> index 000000000000..1b6f0713cb9a
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/Readme.md
> @@ -0,0 +1,66 @@
> +Hardware Error Source Table (HEST)[1] and Software Delegated Exception Interface
> +(SDEI)[2] ACPI tables are used to acomplish firmware first error handling.This
> +patch series introduces a framework to build and install the HEST ACPI table
> +dynamically.
[SAMI] I think the reference to the patch series can be dropped.
> +
> +The following figure illustrates the possible usage of the dyanamic
> +generation of HEST ACPI table.
> +
> + NS | S
> ++--------------------------------------+--------------------------------------+
> +| | |
> +|+-------------------------------------+---------------------+ |
> +|| +---------------------+--------------------+| |
> +|| | | || |
> +|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
> +|| |HestTable | || HestErrorSource | | | HestErrorSource ||| | DMC-620 ||
> +|| | DXE | || DXE | | | StandaloneMM ||| |Standalone MM||
> +|| +-----------+ |+------------------+ | +-----------------+|| +-------------+|
> +|| |GHESv2 | || |
> +|| +---------------------+--------------------+| |
> +|| +--------------------+ | | |
> +|| |PlatformErrorHandler| | | |
> +|| | DXE | | | |
> +|| +--------------------+ | | |
> +||FF FWK | | |
> +|+-------------------------------------+---------------------+ |
> +| | |
> ++--------------------------------------+--------------------------------------+
> + |
> + Figure: Dynamic Hest Table Generation.
> +
> +All the hardware error sources are added to HEST table as GHESv2[3] error source
> +descriptors. The framework comprises of following DXE and MM drivers:
> +
> +- HestTableDxe:
> + Builds HEST table header and allows appending error source descriptors to the
> + HEST table. Also provides protocol interface to install the built HEST table.
> +
> +- HestErrorSourceDxe & HestErrorSourceStandaloneMM:
> + These two drivers together retrieve all possible error source descriptors of
> + type GHESv2 from the MM drivers implementing HEST Error Source Descriptor
> + protocol. Once all the descriptors are collected HestErrorSourceDxe appends
> + it to HEST table using HestTableDxe driver.
> +
> +- PlatformErrorHandlerDxe:
> + Builds and installs SDEI ACPI table. This driver does not initialize(load)
> + until HestErrorSourceDxe driver has finished appending all possible GHESv2
> + error source descriptors to the HEST table. Once that is complete using the
> + HestTableDxe driver it installs the HEST table.
> +
> +This patch series provides reference implementation for DMC-620 Dynamic Memory
> +Controller[4] that has RAS feature enabled. This is platform code
> +implemented as Standalone MM driver in edk2-platforms.
[SAMI] You may want to consider rewording this paragraph.
> +
> +References:
> +[1] : ACPI 6.3, Table 18-382, Hardware Error Source Table
> +[2] : SDEI Platform Design Document, revision b, 10 Appendix C, ACPI table
> + definitions for SDEI
> +[3] : ACPI Reference Specification 6.3, Table 18-393 GHESv2 Structure
> +[4] : DMC620 Dynamic Memory Controller, revision r1p0
> +[5] : UEFI Reference Specification 2.8, Appendix N - Common Platform Error
> + Record
> +[6] : UEFI Reference Specification 2.8, Section N.2.5 Memory Error Section
> +
> +Link to github branch with the patches in this series -
> +https://github.com/omkkul01/edk2/tree/ras_firmware_first_edk2
[SAMI] The link to the github branch can be dropped.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [edk2-devel] [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of HEST ACPI table
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
2021-10-04 17:46 ` Sami Mujawar
@ 2024-04-09 4:57 ` Dhaval Sharma
1 sibling, 0 replies; 13+ messages in thread
From: Dhaval Sharma @ 2024-04-09 4:57 UTC (permalink / raw)
To: Omkar Anand Kulkarni, devel
[-- Attachment #1: Type: text/plain, Size: 457 bytes --]
Hi,
Is there any plan to upstream these HEST/ACPI patches? They seem to be arch agnostic.
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#117526): https://edk2.groups.io/g/devel/message/117526
Mute This Topic: https://groups.io/mt/85104605/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-
[-- Attachment #2: Type: text/html, Size: 873 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-04-09 4:57 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-08-24 5:33 [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Omkar Anand Kulkarni
2021-08-24 5:33 ` [edk2-platforms][PATCH v3 1/5] MdeModulePkg: Allow dynamic generation of " Omkar Anand Kulkarni
2021-10-04 17:46 ` Sami Mujawar
2024-04-09 4:57 ` [edk2-devel] " Dhaval Sharma
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 2/5] ArmPlatformPkg: add definition for MM_HEST_ERROR_SOURCE_DESC_PROTOCOL Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 3/5] ArmPlatformPkg: retreive error source descriptors from MM Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 4/5] EmbeddedPkg: Add helpers for HEST table generation Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-08-24 5:34 ` [edk2-platforms][PATCH v3 5/5] ArmPlatformPkg: Add Readme file Omkar Anand Kulkarni
2021-10-04 17:47 ` Sami Mujawar
2021-10-04 17:46 ` [edk2-platforms][PATCH v3 0/5] Add support to generate HEST ACPI table Sami Mujawar
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox