[edk2-devel] [edk2-platforms][PATCH v3 5/5] Platform/Sgi: Add platform error handling driver
Sami Mujawar
sami.mujawar at arm.com
Mon Oct 4 12:46:54 UTC 2021
Hi Omkar,
Please find my feedback inline marked [SAMI].
Regards,
Sami Mujawar
On 24/08/2021 07:00 AM, Omkar Anand Kulkarni wrote:
> Enables firmware first error handling on the given platform. Installs
> and publishes the SDEI and HEST ACPI tables required for firmware first
> error handling.
>
> Signed-off-by: Omkar Anand Kulkarni <omkar.kulkarni at arm.com>
> ---
> Platform/ARM/SgiPkg/SgiPlatform.dsc.inc | 10 ++
> Platform/ARM/SgiPkg/SgiPlatform.fdf | 7 +
> Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf | 51 ++++++
> Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.c | 171 ++++++++++++++++++++
> 4 files changed, 239 insertions(+)
>
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> index 102d7926bde1..20f003b96cdb 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.dsc.inc
> @@ -24,6 +24,9 @@
> # To allow firmware first error handling, set this to TRUE.
> DEFINE ENABLE_GHES_MM = FALSE
>
> + # To allow firmware first error handling, set this to TRUE.
> + DEFINE ENABLE_FIRWARE_FIRST = FALSE
> +
> [BuildOptions]
> *_*_*_CC_FLAGS = -D DISABLE_NEW_DEPRECATED_INTERFACES
>
> @@ -326,6 +329,13 @@
> #
> Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>
> + #
> + # platform error handler driver
> + #
> +!if $(ENABLE_FIRMWARE_FIRST) == TRUE
> + Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf
> +!endif
> +
> #
> # FAT filesystem + GPT/MBR partitioning
> #
> diff --git a/Platform/ARM/SgiPkg/SgiPlatform.fdf b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> index d6e942e19b81..b1d088610c4c 100644
> --- a/Platform/ARM/SgiPkg/SgiPlatform.fdf
> +++ b/Platform/ARM/SgiPkg/SgiPlatform.fdf
> @@ -190,6 +190,13 @@ READ_LOCK_STATUS = TRUE
> #
> INF Platform/ARM/SgiPkg/Drivers/PlatformDxe/PlatformDxe.inf
>
> + #
> + # platform error handler driver
> + #
> +!if $(ENABLE_FIRMWARE_FIRST) == TRUE
> + INF Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf
> +!endif
> +
> #
> # Bds
> #
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf b/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf
> new file mode 100644
> index 000000000000..fe9ed4175b0b
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.inf
> @@ -0,0 +1,51 @@
> +## @file
> +# Dxe driver to handle platform errors.
> +#
> +# This driver installs SDEI and HEST ACPI tables required for firmware first
> +# error handling.
> +#
> +# Copyright (c) 2020 - 2021, ARM Limited. All rights reserved.
> +# SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +##
> +
> +[Defines]
> + INF_VERSION = 0x0001001A
[SAMI] Update INF revision to use the latest version.
> + BASE_NAME = PlatformErrorHandlerDxe
> + FILE_GUID = a3187ea4-feb4-415f-b11e-2312623ffa6f
> + MODULE_TYPE = DXE_DRIVER
> + VERSION_STRING = 1.0
> + ENTRY_POINT = PlatformErrorHandlerEntryPoint
> +
> +[Sources.common]
> + PlatformErrorHandlerDxe.c
> +
> +[Packages]
> + ArmPlatformPkg/ArmPlatformPkg.dec
> + EmbeddedPkg/EmbeddedPkg.dec
> + MdeModulePkg/MdeModulePkg.dec
> + MdePkg/MdePkg.dec
> + Platform/ARM/SgiPkg/SgiPlatform.dec
> +
> +[LibraryClasses]
> + AcpiLib
> + BaseLib
> + DebugLib
> + UefiDriverEntryPoint
> +
> +[Guids]
> + gArmSgiAcpiTablesGuid
> +
> +[Protocols]
> + gEfiAcpiTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
> + gHestTableProtocolGuid ## PROTOCOL ALWAYS_CONSUMED
> +
> +[FixedPcd]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorId
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemId
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemRevision
> + gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultOemTableId
> +
> +[Depex]
> + AFTER gArmPlatformHestErrorSourcesGuid
> diff --git a/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.c b/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.c
> new file mode 100644
> index 000000000000..25b29152f1bb
> --- /dev/null
> +++ b/Platform/ARM/SgiPkg/Drivers/PlatformErrorHandlerDxe/PlatformErrorHandlerDxe.c
> @@ -0,0 +1,171 @@
> +/** @file
> + Driver to handle and support all platform errors.
> +
> + Installs the SDEI and HEST ACPI tables for firmware first error handling.
> +
> + 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
> + - SDEI Platform Design Document, revision b, 10 Appendix C, ACPI table
> + definitions for SDEI
> +**/
> +
> +#include <IndustryStandard/Acpi.h>
> +
> +#include <Library/AcpiLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +
> +#include <Protocol/AcpiTable.h>
> +#include <Protocol/HestTable.h>
> +
> +
> +/**
> + Build and install the SDEI ACPI table.
> +
> + For platforms that allow firmware-first platform error handling, SDEI is used
> + as the notification mechanism for those errors.
> +
> + @retval EFI_SUCCESS SDEI table installed successfully.
> + @retval Other For any error during installation.
> +**/
> +STATIC
> +EFI_STATUS
> +InstallSdeiTable (VOID)
[SAMI] Fix coding style, please. VOID and closing brackets should each
be on a new line.
> +{
> + EFI_ACPI_TABLE_PROTOCOL *mAcpiTableProtocol = NULL;
> + EFI_ACPI_DESCRIPTION_HEADER Header;
> + EFI_STATUS Status;
> + UINTN AcpiTableHandle;
> +
> + Header =
> + (EFI_ACPI_DESCRIPTION_HEADER) {
> + EFI_ACPI_6_3_SOFTWARE_DELEGATED_EXCEPTIONS_INTERFACE_TABLE_SIGNATURE,
> + sizeof (EFI_ACPI_DESCRIPTION_HEADER), // Length
> + 0x01, // Revision
> + 0x00, // Checksum
> + {'A', 'R', 'M', 'L', 'T', 'D'}, // OemId
> + 0x4152464e49464552, // OemTableId:"REFINFRA"
> + 0x20201027, // OemRevision
> + 0x204d5241, // CreatorId:"ARM "
> + 0x00000001, // CreatorRevision
> + };
> +
[SAMI] I would prefer a static structure locally initialised in this
file and used in this function. Can you fix this, please?
> + Header.Checksum = CalculateCheckSum8 ((UINT8 *)&Header, Header.Length);
[SAMI] I dont think this is needed as the checksum is calculated in
mAcpiTableProtocol->InstallAcpiTable().
See
https://github.com/tianocore/edk2/blob/master/MdeModulePkg/Universal/Acpi/AcpiTableDxe/AcpiTableProtocol.c#L241
[/SAMI]
> + 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 = mAcpiTableProtocol->InstallAcpiTable (
> + mAcpiTableProtocol,
> + &Header,
> + Header.Length,
> + &AcpiTableHandle
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to install SDEI ACPI table, status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + }
> +
> + return Status;
> +}
> +
> +/**
> + Install the HEST ACPI table.
> +
> + HEST ACPI table is used to list the platform errors for which the error
> + handling has been supported. Use the HEST table generation protocol to
> + install the HEST table.
> +
> + @retval EFI_SUCCESS HEST table installed successfully.
> + @retval Other For any error during installation.
> +**/
> +STATIC
> +EFI_STATUS
> +InstallHestTable (VOID)
[SAMI] Fix coding style, please. VOID and closing brackets should each
be on a new line.
> +{
> + HEST_TABLE_PROTOCOL *HestProtocol;
> + EFI_STATUS Status;
> +
> + Status = gBS->LocateProtocol (
> + &gHestTableProtocolGuid,
> + NULL,
> + (VOID **)&HestProtocol
> + );
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to locate HEST DXE Protocol, status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + return Status;
[SAMI] Fix alignment.
> + }
> +
> + Status = HestProtocol->InstallHestTable ();
> + if (EFI_ERROR (Status)) {
> + DEBUG ((
> + DEBUG_ERROR,
> + "%a: Failed to install HEST table, status: %r\n",
> + __FUNCTION__,
> + Status
> + ));
> + }
> +
> + return Status;
> +}
> +
> +/**
> + Entry point for the DXE driver.
> +
> + This function installs the HEST ACPI table, using the HEST table generation
> + protocol. Also creates and installs the SDEI ACPI table required for firmware
> + first error handling.
> +
> + @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 ACPI tables
> + @retval Other On Failure
> +**/
> +EFI_STATUS
> +EFIAPI
> +PlatformErrorHandlerEntryPoint (
> + IN EFI_HANDLE ImageHandle,
> + IN EFI_SYSTEM_TABLE *SystemTable
> + )
> +{
> + EFI_STATUS Status;
> +
> + // Build and install SDEI table.
> + Status = InstallSdeiTable ();
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + // Install the created HEST table.
> + Status = InstallHestTable ();
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + return EFI_SUCCESS;
> +}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#81447): https://edk2.groups.io/g/devel/message/81447
Mute This Topic: https://groups.io/mt/85104852/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list