[edk2-devel] [PATCH v2 3/4] ArmPlatformPkg: retreive error source descriptors from MM
Sami Mujawar
sami.mujawar at arm.com
Tue Aug 3 14:46:22 UTC 2021
Hi Omkar,
Please find my response inline marked [SAMI].
Regards,
Sami Mujawar
On 10/07/2021 05:18 PM, 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 at arm.com>
> Signed-off-by: Omkar Anand Kulkarni<omkar.kulkarni at arm.com>
> ---
> ArmPlatformPkg/ArmPlatformPkg.dec | 7 +
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf | 44 +++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.inf | 51 ++++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestMmErrorSourceCommon.h | 37 +++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c | 308 +++++++++++++++++++
> ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceStandaloneMm.c | 312 ++++++++++++++++++++
[SAMI] Should this patch be split into 2?
> 6 files changed, 759 insertions(+)
>
> diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
> index 4f062292663b..846b3e863aa9 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,6 +130,11 @@
>
> gArmPlatformTokenSpaceGuid.PcdWatchdogCount|0x0|UINT32|0x00000033
>
> +[PcdsFixedAtBuild, PcdsPatchableInModule]
> + ## ACPI CPER memory space
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferBase|0x00000000|UINT64|0x00000046
> + gArmPlatformTokenSpaceGuid.PcdGhesGenericErrorDataMmBufferSize|0x00000000|UINT64|0x00000047
> +
> [Protocols.common]
> ## Arm Platform HEST table generation protocol
> gHestTableProtocolGuid = { 0x705bdcd9, 0x8c47, 0x457e, { 0xad, 0x0d, 0xf7, 0x86, 0xf3, 0x4a, 0x0d, 0x63 } }
> diff --git a/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
> new file mode 100644
> index 000000000000..5227dea91630
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.inf
> @@ -0,0 +1,44 @@
> +## @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
> + 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))
[SAMI] I feel there can be a simple way to do this, see the comments below.
> +
> +//
> +// 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.
> + //
[SAMI] Does the Total size also include the size of ErrSourceDescCount
and ErrSourceDescSize?
> + UINTN ErrSourceDescSize;
[SAMI] Can the first 2 fields of this structure be moved to a structure
called HEST_ERROR_SOURCE_DESC_HEADER? I think it may simplify computing
of the size of HEST_ERROR_SOURCE_DESC_INFO.
[/SAMI]
> + //
> + // 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/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
> new file mode 100644
> index 000000000000..acfb0fc9e838
> --- /dev/null
> +++ b/ArmPlatformPkg/Drivers/HestMmErrorSources/HestErrorSourceDxe.c
> @@ -0,0 +1,308 @@
> +/** @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"
> +
> +#define MM_COMMUNICATE_HEADER_SIZE (OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data))
[SAMI] Can this definition be moved to
MdePkg\Include\Protocol\MmCommunication.h, please ?
> +
> +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,
> + (VOID *)*CommBuffer,
> + (VOID *)*CommBuffer,
[SAMI] Can you check if the third parameter to Communicate() is correct,
please?
> + 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)
[SAMI] VOID and ) should be on a separate line. Can you check the other
patches in this series as well, please?
> +{
> + 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 = AllocatePool (CommBufferSize);
[SAMI] Would it be better to use AllocateZeroPool() ?
> + 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_BAD_BUFFER_SIZE)) {
> + 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_SUCCESS;
[SAMI] 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 = AllocatePool (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_SUCCESS;
[SAMI] Can EFI_NOT_FOUND be returned here?
> + }
> +
> + 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..c7b2304fc494
> --- /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 OUT 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 = AllocatePool (HandleBufferSize);
[SAMI] AllocateZeroPool() ?
> + 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))
[SAMI] Is check for HandleBuffer == NULL right here?
> + {
> + 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) {
[SAMI] I think the error handling in this function and the error return
implementation in GetHestErrorSourceDescriptors() could be improved.
e.g. GetHestErrorSourceDescriptors() could first check for the
SourceLength & SourceCount and if it is less than what is required, it
returns EFI_BUFFER_TOO_SMALL.
The next check would be to check ErrorSourcePtr and return
EFI_INVALID_PARAMETER if it is NULL.
[/SAMI]
> + 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_BAD_BUFFER_SIZE;
[SAMI] Should the return code be EFI_BUFFER_TOO_SMALL? The difference
being, the caller can attempt to call again with a larger buffer if
EFI_BUFFER_TOO_SMALL is returned.
CommBufferSize is declared as an OUT paramter, was the intent to return
the required buffer size?
[/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) {
> + ErrorSourcePtr += SourceLength;
> + }
> + }
> +
> + // Free the buffer holding all the protocol handles.
> + FreePool (HandleBuffer);
> +
> + return EFI_SUCCESS;
[SAMI] return Status of last operation.
> +}
> +
> +/**
> + 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 EFI_SUCCESS;
[SAMI] return Status of last operation.
> +}
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#78599): https://edk2.groups.io/g/devel/message/78599
Mute This Topic: https://groups.io/mt/84115238/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/edk2-devel-archive/attachments/20210803/e6ef0519/attachment.htm>
More information about the edk2-devel-archive
mailing list