Date   

Re: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update FmpDeviceCheckImageWithStatus() handling

Guomin Jiang
 

Reviewed-by: Guomin Jiang <guomin.jiang@...>

Guomin

-----Original Message-----
From: mikuback@... <mikuback@...>
Sent: Wednesday, January 5, 2022 4:38 AM
To: devel@edk2.groups.io
Cc: Gao, Liming <gaoliming@...>; Kinney, Michael D
<michael.d.kinney@...>; Jiang, Guomin <Guomin.Jiang@...>;
Xu, Wei6 <wei6.xu@...>
Subject: [PATCH v1 1/1] FmpDevicePkg/FmpDxe: Update
FmpDeviceCheckImageWithStatus() handling

From: Michael Kubacki <michael.kubacki@...>

Update the logic handling last attempt status codes from
FmpDeviceCheckImageWithStatus() implementations to account for cases
when the function return status code is EFI_SUCCESS (since the image was
checked successfully) but the ImageUpdatable value is not valid.

In addition the following sentence is removed from the LastAttemptStatus
parameter definition for
FmpDeviceCheckImageWithStatus() since it can lead to confusion.
The expected status code value range is sufficient to implement the library
API.

"This value will only be checked when this
function returns an error."

Cc: Liming Gao <gaoliming@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Guomin Jiang <guomin.jiang@...>
Cc: Wei6 Xu <wei6.xu@...>
Signed-off-by: Michael Kubacki <michael.kubacki@...>
---
FmpDevicePkg/FmpDxe/FmpDxe.c | 23 +++++++++++++++-----
FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c | 3 +--
FmpDevicePkg/Include/Library/FmpDeviceLib.h | 3 +--
3 files changed, 19 insertions(+), 10 deletions(-)

diff --git a/FmpDevicePkg/FmpDxe/FmpDxe.c
b/FmpDevicePkg/FmpDxe/FmpDxe.c index 197df28c8dd6..1e7ec4a09e16
100644
--- a/FmpDevicePkg/FmpDxe/FmpDxe.c
+++ b/FmpDevicePkg/FmpDxe/FmpDxe.c
@@ -1040,8 +1040,19 @@ CheckTheImageInternal (
//
Status = FmpDeviceCheckImageWithStatus ((((UINT8 *)Image) +
AllHeaderSize), RawSize, ImageUpdatable, LastAttemptStatus);
if (EFI_ERROR (Status)) {
+ // The image cannot be valid if an error occurred checking the image
+ if (*ImageUpdatable == IMAGE_UPDATABLE_VALID) {
+ *ImageUpdatable = IMAGE_UPDATABLE_INVALID;
+ }
+
DEBUG ((DEBUG_ERROR, "FmpDxe(%s): CheckTheImage() - FmpDeviceLib
CheckImage failed. Status = %r\n", mImageIdName, Status));
+ }

+ //
+ // Only validate the library last attempt status code if the image is not
updatable.
+ // This specifically avoids converting LAST_ATTEMPT_STATUS_SUCCESS if it
set for an updatable image.
+ //
+ if (*ImageUpdatable != IMAGE_UPDATABLE_VALID) {
//
// LastAttemptStatus returned from the device library should fall within
the designated error range
// [LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE,
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE]
@@ -1049,12 +1060,12 @@ CheckTheImageInternal (
if ((*LastAttemptStatus <
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE) ||
(*LastAttemptStatus >
LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MAX_ERROR_CODE_VALUE))
{
- DEBUG (
- (DEBUG_ERROR,
- "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
FmpDeviceCheckImageWithStatus() is invalid.\n",
- mImageIdName,
- *LastAttemptStatus)
- );
+ DEBUG ((
+ DEBUG_ERROR,
+ "FmpDxe(%s): CheckTheImage() - LastAttemptStatus %d from
FmpDeviceCheckImageWithStatus() is invalid.\n",
+ mImageIdName,
+ *LastAttemptStatus
+ ));
*LastAttemptStatus = LAST_ATTEMPT_STATUS_ERROR_UNSUCCESSFUL;
}
}
diff --git a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
index 2e5c17b2b0f9..82219e87a430 100644
--- a/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
+++ b/FmpDevicePkg/Library/FmpDeviceLibNull/FmpDeviceLib.c
@@ -434,8 +434,7 @@ FmpDeviceCheckImage (
IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
@param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
attempt
status to report back to the ESRT table in case
- of error. This value will only be checked when this
- function returns an error.
+ of error.

The return status code must fall in the range of

LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
diff --git a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
index a14406abe8b5..f82ef64503fa 100644
--- a/FmpDevicePkg/Include/Library/FmpDeviceLib.h
+++ b/FmpDevicePkg/Include/Library/FmpDeviceLib.h
@@ -421,8 +421,7 @@ FmpDeviceCheckImage (
IMAGE_UPDATABLE_VALID_WITH_VENDOR_CODE
@param[out] LastAttemptStatus A pointer to a UINT32 that holds the last
attempt
status to report back to the ESRT table in case
- of error. This value will only be checked when this
- function returns an error.
+ of error.

The return status code must fall in the range of

LAST_ATTEMPT_STATUS_DEVICE_LIBRARY_MIN_ERROR_CODE_VALUE to
--
2.28.0.windows.1


Re: [PATCH 0/3] Add support for gdb and lldb

Rebecca Cran
 

Since it's now the new year, I thought it might be worth pinging people again to try and get these patches pushed.


--

Rebecca Cran


On 12/13/21 14:59, Rebecca Cran wrote:

(cc other TianoCore stewards)


With edk2-stable202111 just tagged, now would be a good time to get the patches pushed.


--
Rebecca Cran


On 9/14/21 18:47, Andrew Fish wrote:
Sorry the patches stalled out. I need to push them….

Thanks,

Andrew Fish

On Sep 14, 2021, at 4:47 PM, Rebecca Cran <rebecca@...> wrote:

I was wondering what your plan for committing these to the repo is? It would be nice to get them committed so people can start using them.


-- 
Rebecca Cran


On 8/8/21 3:46 PM, Andrew Fish via groups.io wrote:
This patch set adds debugging support for gdb and lldb.
It also adds generic debugging classes that use a file like object to
make it easy to import into any debugger that supports Python.

Since these debugging scripts don't depend on any EFI code I was thinking
we could place them in the root of the repo to be easy to discover.

I've tested gdb on Ubuntu and lldb on macOS for IA32 and X64.

Andrew Fish (3):
  efi_debugging.py: - Add debugger agnostic debugging Python Classes
  efi_gdb.py: - Add gdb EFI commands and pretty Print
  efi_lldb.py: - Add lldb EFI commands and pretty Print

 efi_debugging.py | 2187 ++++++++++++++++++++++++++++++++++++++++++++++
 efi_gdb.py       |  918 +++++++++++++++++++
 efi_lldb.py      | 1044 ++++++++++++++++++++++
 3 files changed, 4149 insertions(+)
 create mode 100755 efi_debugging.py
 create mode 100755 efi_gdb.py
 create mode 100755 efi_lldb.py










[PATCH v4 7/7] MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This change added support of installing `EFI_MM_COMMUNICATION3_PROTOCOL`.

MmCommunicate v3 routine that calculates message length is also updated
to remove ambiguity in contrast to v1 routine.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly added v3 communicate protocol instance

v4:
- Rebased with uncrustify changes.

MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 190 ++++++++++++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 2 +
2 files changed, 192 insertions(+)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
index 4f00cebaf5ed..910f54bed5fb 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c
@@ -11,6 +11,7 @@
#include <Protocol/SmmBase2.h>
#include <Protocol/SmmCommunication.h>
#include <Protocol/MmCommunication2.h>
+#include <Protocol/MmCommunication3.h>
#include <Protocol/SmmAccess2.h>
#include <Protocol/SmmConfiguration.h>
#include <Protocol/SmmControl2.h>
@@ -34,6 +35,7 @@
#include <Library/UefiRuntimeLib.h>
#include <Library/PcdLib.h>
#include <Library/ReportStatusCodeLib.h>
+#include <Library/SafeIntLib.h>

#include "PiSmmCorePrivateData.h"

@@ -146,6 +148,41 @@ SmmCommunicationMmCommunicate2 (
IN OUT UINTN *CommSize OPTIONAL
);

+/**
+ Communicates with a registered handler.
+
+ This function provides a service to send and receive messages from a registered UEFI service.
+
+ @param[in] This The EFI_MM_COMMUNICATION3_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommSize The size of the data buffer being passed in. On exit, the size of data
+ being returned. Zero if the handler does not wish to reply with any data.
+ This parameter is optional and may be NULL.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+ @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM implementation.
+ If this error is returned, the MessageLength field
+ in the CommBuffer header or the integer pointed by
+ CommSize, are updated to reflect the maximum payload
+ size the implementation can accommodate.
+ @retval EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize parameter,
+ if not omitted, are in address range that cannot be
+ accessed by the MM environment.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationMmCommunicate3 (
+ IN CONST EFI_MM_COMMUNICATION3_PROTOCOL *This,
+ IN OUT VOID *CommBufferPhysical,
+ IN OUT VOID *CommBufferVirtual,
+ IN OUT UINTN *CommSize OPTIONAL
+ );
+
/**
Event notification that is fired every time a gEfiSmmConfigurationProtocol installs.

@@ -275,6 +312,13 @@ EFI_MM_COMMUNICATION2_PROTOCOL mMmCommunication2 = {
SmmCommunicationMmCommunicate2
};

+//
+// PI 1.7 MM Communication Protocol 3 instance
+//
+EFI_MM_COMMUNICATION3_PROTOCOL mMmCommunication3 = {
+ MmCommunicationMmCommunicate3
+};
+
//
// SMM Core Private Data structure that contains the data shared between
// the SMM IPL and the SMM Core.
@@ -651,6 +695,150 @@ SmmCommunicationMmCommunicate2 (
);
}

+/**
+ Communicates with a registered handler.
+
+ This function provides a service to send and receive messages from a registered UEFI service.
+
+ @param[in] This The EFI_MM_COMMUNICATION3_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommSize The size of the data buffer being passed in. On exit, the size of data
+ being returned. Zero if the handler does not wish to reply with any data.
+ This parameter is optional and may be NULL.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+ @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM implementation.
+ If this error is returned, the MessageLength field
+ in the CommBuffer header or the integer pointed by
+ CommSize, are updated to reflect the maximum payload
+ size the implementation can accommodate.
+ @retval EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize parameter,
+ if not omitted, are in address range that cannot be
+ accessed by the MM environment.
+
+**/
+EFI_STATUS
+EFIAPI
+MmCommunicationMmCommunicate3 (
+ IN CONST EFI_MM_COMMUNICATION3_PROTOCOL *This,
+ IN OUT VOID *CommBufferPhysical,
+ IN OUT VOID *CommBufferVirtual,
+ IN OUT UINTN *CommSize OPTIONAL
+ )
+{
+ EFI_STATUS Status;
+ EFI_MM_COMMUNICATE_HEADER_V3 *CommunicateHeader;
+ BOOLEAN OldInSmm;
+ UINTN TempCommSize;
+ UINT64 LongCommSize;
+
+ //
+ // Check parameters
+ //
+ if (CommBufferPhysical == NULL) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER_V3 *)CommBufferPhysical;
+
+ if (CommSize == NULL) {
+ Status = SafeUint64Add (sizeof (EFI_MM_COMMUNICATE_HEADER_V3), CommunicateHeader->MessageSize, &LongCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ Status = SafeUint64ToUintn (LongCommSize, &TempCommSize);
+ if (EFI_ERROR (Status)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ } else {
+ TempCommSize = *CommSize;
+ //
+ // CommSize must hold the entire EFI_MM_COMMUNICATE_HEADER_V3
+ //
+ if (TempCommSize < sizeof (EFI_MM_COMMUNICATE_HEADER_V3)) {
+ return EFI_INVALID_PARAMETER;
+ }
+ }
+
+ //
+ // If not already in SMM, then generate a Software SMI
+ //
+ if (!gSmmCorePrivate->InSmm && gSmmCorePrivate->SmmEntryPointRegistered) {
+ //
+ // Put arguments for Software SMI in gSmmCorePrivate
+ //
+ gSmmCorePrivate->CommunicationBuffer = CommBufferPhysical;
+ gSmmCorePrivate->BufferSize = TempCommSize;
+
+ //
+ // Generate Software SMI
+ //
+ Status = mSmmControl2->Trigger (mSmmControl2, NULL, NULL, FALSE, 0);
+ if (EFI_ERROR (Status)) {
+ return EFI_UNSUPPORTED;
+ }
+
+ //
+ // Return status from software SMI
+ //
+ if (CommSize != NULL) {
+ *CommSize = gSmmCorePrivate->BufferSize;
+ }
+
+ return gSmmCorePrivate->ReturnStatus;
+ }
+
+ //
+ // If we are in SMM, then the execution mode must be physical, which means that
+ // OS established virtual addresses can not be used. If SetVirtualAddressMap()
+ // has been called, then a direct invocation of the Software SMI is not allowed,
+ // so return EFI_INVALID_PARAMETER.
+ //
+ if (EfiGoneVirtual ()) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // If we are not in SMM, don't allow call SmiManage() directly when SMRAM is closed or locked.
+ //
+ if ((!gSmmCorePrivate->InSmm) && (!mSmmAccess->OpenState || mSmmAccess->LockState)) {
+ return EFI_INVALID_PARAMETER;
+ }
+
+ //
+ // Save current InSmm state and set InSmm state to TRUE
+ //
+ OldInSmm = gSmmCorePrivate->InSmm;
+ gSmmCorePrivate->InSmm = TRUE;
+
+ //
+ // Before SetVirtualAddressMap(), we are in SMM or SMRAM is open and unlocked, call SmiManage() directly.
+ //
+ TempCommSize -= sizeof (EFI_MM_COMMUNICATE_HEADER_V3);
+ Status = gSmmCorePrivate->Smst->SmiManage (
+ &CommunicateHeader->MessageGuid,
+ NULL,
+ CommunicateHeader->MessageData,
+ &TempCommSize
+ );
+ TempCommSize += sizeof (EFI_MM_COMMUNICATE_HEADER_V3);
+ if (CommSize != NULL) {
+ *CommSize = TempCommSize;
+ }
+
+ //
+ // Restore original InSmm state
+ //
+ gSmmCorePrivate->InSmm = OldInSmm;
+
+ return (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
+}
+
/**
Event notification that is fired when GUIDed Event Group is signaled.

@@ -1859,6 +2047,8 @@ SmmIplEntry (
&mSmmCommunication,
&gEfiMmCommunication2ProtocolGuid,
&mMmCommunication2,
+ &gEfiMmCommunication3ProtocolGuid,
+ &mMmCommunication3,
NULL
);
ASSERT_EFI_ERROR (Status);
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
index 6109d6b5449c..afab228cc04c 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf
@@ -46,11 +46,13 @@ [LibraryClasses]
DxeServicesLib
PcdLib
ReportStatusCodeLib
+ SafeIntLib

[Protocols]
gEfiSmmBase2ProtocolGuid ## PRODUCES
gEfiSmmCommunicationProtocolGuid ## PRODUCES
gEfiMmCommunication2ProtocolGuid ## PRODUCES
+ gEfiMmCommunication3ProtocolGuid ## PRODUCES
gEfiSmmAccess2ProtocolGuid ## CONSUMES
## NOTIFY
## CONSUMES
--
2.34.1.windows.1


[PATCH v4 6/7] StandaloneMmPkg: StandaloneMmCore: Parsing new MM communicate header

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

MM communicate protocols are expanded with EFI_MM_COMMUNICATE_HEADER_V3
structure that cooperates with updated field types and flexible array.
The PiSmmCore implementation is updated to detect and process incoming
data accordingly.

Two checks are also performed to prevent legacy communicate data or
unsupported data is fed into MM core under agreed header guid.

Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly added

v4:
- Rebased with uncrusitify changes.

StandaloneMmPkg/Core/StandaloneMmCore.c | 35 ++++++++++++++++----
StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
2 files changed, 29 insertions(+), 7 deletions(-)

diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.c b/StandaloneMmPkg/Core/StandaloneMmCore.c
index d221f1d1115d..8afb22493cb2 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.c
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.c
@@ -338,8 +338,12 @@ MmEntryPoint (
IN CONST EFI_MM_ENTRY_CONTEXT *MmEntryContext
)
{
- EFI_STATUS Status;
- EFI_MM_COMMUNICATE_HEADER *CommunicateHeader;
+ EFI_STATUS Status;
+ EFI_MM_COMMUNICATE_HEADER_V3 *CommunicateHeader;
+ EFI_MM_COMMUNICATE_HEADER *LegacyCommunicateHeader;
+ EFI_GUID *CommGuid;
+ VOID *CommData;
+ UINTN CommHeaderSize;

DEBUG ((DEBUG_INFO, "MmEntryPoint ...\n"));

@@ -377,19 +381,36 @@ MmEntryPoint (
gMmCorePrivate->CommunicationBuffer = 0;
gMmCorePrivate->ReturnStatus = EFI_INVALID_PARAMETER;
} else {
- CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
- gMmCorePrivate->BufferSize -= OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
+ CommGuid = &((EFI_MM_COMMUNICATE_HEADER_V3 *)(UINTN)gMmCorePrivate->CommunicationBuffer)->HeaderGuid;
+ //
+ // Check if the signature matches EFI_MM_COMMUNICATE_HEADER_V3 definition
+ //
+ if (CompareGuid (CommGuid, &gCommunicateHeaderV3Guid)) {
+ CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER_V3 *)(UINTN)gMmCorePrivate->CommunicationBuffer;
+ ASSERT (CommunicateHeader->Signature == EFI_MM_COMMUNICATE_HEADER_V3_SIGNATURE);
+ ASSERT (CommunicateHeader->Version <= EFI_MM_COMMUNICATE_HEADER_V3_VERSION);
+ CommGuid = &CommunicateHeader->MessageGuid;
+ CommData = CommunicateHeader->MessageData;
+ CommHeaderSize = sizeof (EFI_MM_COMMUNICATE_HEADER_V3);
+ } else {
+ LegacyCommunicateHeader = (EFI_MM_COMMUNICATE_HEADER *)(UINTN)gMmCorePrivate->CommunicationBuffer;
+ CommGuid = &LegacyCommunicateHeader->HeaderGuid;
+ CommData = LegacyCommunicateHeader->Data;
+ CommHeaderSize = OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
+ }
+
+ gMmCorePrivate->BufferSize -= CommHeaderSize;
Status = MmiManage (
- &CommunicateHeader->HeaderGuid,
+ CommGuid,
NULL,
- CommunicateHeader->Data,
+ CommData,
(UINTN *)&gMmCorePrivate->BufferSize
);
//
// Update CommunicationBuffer, BufferSize and ReturnStatus
// Communicate service finished, reset the pointer to CommBuffer to NULL
//
- gMmCorePrivate->BufferSize += OFFSET_OF (EFI_MM_COMMUNICATE_HEADER, Data);
+ gMmCorePrivate->BufferSize += CommHeaderSize;
gMmCorePrivate->CommunicationBuffer = 0;
gMmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
}
diff --git a/StandaloneMmPkg/Core/StandaloneMmCore.inf b/StandaloneMmPkg/Core/StandaloneMmCore.inf
index c44b9ff33303..e2e6cd32beee 100644
--- a/StandaloneMmPkg/Core/StandaloneMmCore.inf
+++ b/StandaloneMmPkg/Core/StandaloneMmCore.inf
@@ -75,6 +75,7 @@ [Guids]
gEfiEventLegacyBootGuid
gEfiEventExitBootServicesGuid
gEfiEventReadyToBootGuid
+ gCommunicateHeaderV3Guid ## CONSUMES ## GUID # Communicate header

#
# This configuration fails for CLANGPDB, which does not support PIE in the GCC
--
2.34.1.windows.1


[PATCH v4 5/7] MdeModulePkg: PiSmmCore: Added parser of new MM communicate header

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

MM communicate protocols are expanded with EFI_MM_COMMUNICATE_HEADER_V3
structure that cooperates with updated field types and flexible array.
The PiSmmCore implementation is updated to detect and process incoming
data accordingly.

Two checks are also performed to prevent legacy communicate data or
unsupported data is fed into MM core under agreed header guid.

Cc: Jian J Wang <jian.j.wang@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly added

v4:
- Rebased with uncrusitify changes.

MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 51 ++++++++++++++------
MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
2 files changed, 37 insertions(+), 15 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
index 9e5c6cbe33dd..8d57f71dc969 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.c
@@ -647,12 +647,16 @@ SmmEntryPoint (
IN CONST EFI_SMM_ENTRY_CONTEXT *SmmEntryContext
)
{
- EFI_STATUS Status;
- EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader;
- BOOLEAN InLegacyBoot;
- BOOLEAN IsOverlapped;
- VOID *CommunicationBuffer;
- UINTN BufferSize;
+ EFI_STATUS Status;
+ EFI_MM_COMMUNICATE_HEADER_V3 *CommunicateHeader;
+ EFI_SMM_COMMUNICATE_HEADER *LegacyCommunicateHeader;
+ BOOLEAN InLegacyBoot;
+ BOOLEAN IsOverlapped;
+ VOID *CommunicationBuffer;
+ UINTN BufferSize;
+ EFI_GUID *CommGuid;
+ VOID *CommData;
+ UINTN CommHeaderSize;

//
// Update SMST with contents of the SmmEntryContext structure
@@ -708,19 +712,36 @@ SmmEntryPoint (
gSmmCorePrivate->CommunicationBuffer = NULL;
gSmmCorePrivate->ReturnStatus = EFI_ACCESS_DENIED;
} else {
- CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;
- BufferSize -= OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
- Status = SmiManage (
- &CommunicateHeader->HeaderGuid,
- NULL,
- CommunicateHeader->Data,
- &BufferSize
- );
+ CommGuid = &((EFI_MM_COMMUNICATE_HEADER_V3 *)CommunicationBuffer)->HeaderGuid;
+ //
+ // Check if the signature matches EFI_MM_COMMUNICATE_HEADER_V3 definition
+ //
+ if (CompareGuid (CommGuid, &gCommunicateHeaderV3Guid)) {
+ CommunicateHeader = (EFI_MM_COMMUNICATE_HEADER_V3 *)CommunicationBuffer;
+ ASSERT (CommunicateHeader->Signature == EFI_MM_COMMUNICATE_HEADER_V3_SIGNATURE);
+ ASSERT (CommunicateHeader->Version <= EFI_MM_COMMUNICATE_HEADER_V3_VERSION);
+ CommGuid = &CommunicateHeader->MessageGuid;
+ CommData = CommunicateHeader->MessageData;
+ CommHeaderSize = sizeof (EFI_MM_COMMUNICATE_HEADER_V3);
+ } else {
+ LegacyCommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *)CommunicationBuffer;
+ CommGuid = &LegacyCommunicateHeader->HeaderGuid;
+ CommData = LegacyCommunicateHeader->Data;
+ CommHeaderSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
+ }
+
+ BufferSize -= CommHeaderSize;
+ Status = SmiManage (
+ CommGuid,
+ NULL,
+ CommData,
+ &BufferSize
+ );
//
// Update CommunicationBuffer, BufferSize and ReturnStatus
// Communicate service finished, reset the pointer to CommBuffer to NULL
//
- gSmmCorePrivate->BufferSize = BufferSize + OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data);
+ gSmmCorePrivate->BufferSize = BufferSize + CommHeaderSize;
gSmmCorePrivate->CommunicationBuffer = NULL;
gSmmCorePrivate->ReturnStatus = (Status == EFI_SUCCESS) ? EFI_SUCCESS : EFI_NOT_FOUND;
}
diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
index c8bfae3860fc..5a0929a45e19 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf
@@ -118,6 +118,7 @@ [Guids]
gSmiHandlerProfileGuid
gEdkiiEndOfS3ResumeGuid ## SOMETIMES_PRODUCES ## GUID # Install protocol
gEdkiiS3SmmInitDoneGuid ## SOMETIMES_PRODUCES ## GUID # Install protocol
+ gCommunicateHeaderV3Guid ## CONSUMES ## GUID # Communicate header

[UserExtensions.TianoCore."ExtraFiles"]
PiSmmCoreExtra.uni
--
2.34.1.windows.1


[PATCH v4 4/7] MdePkg: MmCommunication: Introduce EFI_PEI_MM_COMMUNICATION3_PPI to MdePkg

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This change introduces a new definition for MM communicate PPI v3.

This PPI will be installed under a new GUID in contrast to exisiting
EFI_PEI_MM_COMMUNICATION_PPI.

Data communicated to MM through EFI_PEI_MM_COMMUNICATION3_PPI should
always start with EFI_MM_COMMUNICATE_HEADER_V3 with its HeaderGuid,
Signature and Version fields properly populated.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Marvin Häuser <mhaeuser@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly introduced v3 communicate PPI

v4:
- Rebased with uncrustify changes.

MdePkg/Include/Ppi/MmCommunication3.h | 57 ++++++++++++++++++++
MdePkg/MdePkg.dec | 3 ++
2 files changed, 60 insertions(+)

diff --git a/MdePkg/Include/Ppi/MmCommunication3.h b/MdePkg/Include/Ppi/MmCommunication3.h
new file mode 100644
index 000000000000..1cc75c38c012
--- /dev/null
+++ b/MdePkg/Include/Ppi/MmCommunication3.h
@@ -0,0 +1,57 @@
+/** @file
+ EFI MM Communication v3 PPI definition.
+
+ This Ppi provides a means of communicating between PEIM and MMI
+ handlers inside of MM.
+
+Copyright (c) 2010 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) Microsoft Corporation.
+
+SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MM_COMMUNICATION3_PPI_H_
+#define MM_COMMUNICATION3_PPI_H_
+
+#include <Pi/PiMultiPhase.h>
+
+#define EFI_PEI_MM_COMMUNICATION3_PPI_GUID \
+ { \
+ 0xe70febf6, 0x13ef, 0x4a21, { 0x89, 0x9e, 0xb2, 0x36, 0xf8, 0x25, 0xff, 0xc9 } \
+ }
+
+typedef struct _EFI_PEI_MM_COMMUNICATION3_PPI EFI_PEI_MM_COMMUNICATION3_PPI;
+
+/**
+ Communicates with a registered handler.
+
+ This function provides a service to send and receive messages from a registered UEFI service.
+
+ @param[in] This The EFI_PEI_MM_COMMUNICATE3 instance.
+ @param[in, out] CommBuffer A pointer to the buffer to convey into MMRAM.
+ @param[in, out] CommSize The size of the data buffer being passed in.On exit, the size of data
+ being returned. Zero if the handler does not wish to reply with any data.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER The CommBuffer was NULL.
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_PEI_MM_COMMUNICATE3)(
+ IN CONST EFI_PEI_MM_COMMUNICATION3_PPI *This,
+ IN OUT VOID *CommBuffer,
+ IN OUT UINTN *CommSize
+ );
+
+///
+/// EFI MM Communication PPI provides runtime services for communicating
+/// between DXE drivers and a registered SMI handler.
+///
+struct _EFI_PEI_MM_COMMUNICATION3_PPI {
+ EFI_PEI_MM_COMMUNICATE3 Communicate;
+};
+
+extern EFI_GUID gEfiPeiMmCommunication3PpiGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 8c61661e4ee7..cb42078fa330 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1012,6 +1012,9 @@ [Ppis]
## Include/Ppi/DelayedDispatch.h
gEfiPeiDelayedDispatchPpiGuid = { 0x869c711d, 0x649c, 0x44fe, { 0x8b, 0x9e, 0x2c, 0xbb, 0x29, 0x11, 0xc3, 0xe6 }}

+ ## Include/Ppi/MmCommunication3.h
+ gEfiPeiMmCommunication3PpiGuid = { 0xe70febf6, 0x13ef, 0x4a21, { 0x89, 0x9e, 0xb2, 0x36, 0xf8, 0x25, 0xff, 0xc9 }}
+
[Protocols]
## Include/Protocol/Pcd.h
gPcdProtocolGuid = { 0x11B34006, 0xD85B, 0x4D0A, { 0xA2, 0x90, 0xD5, 0xA5, 0x71, 0x31, 0x0E, 0xF7 }}
--
2.34.1.windows.1


[PATCH v4 3/7] MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATION3_PROTOCOL to MdePkg

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This change introduces a new definition for MM communicate protocol v3.

This protocol will be installed under a new GUID in contrast to exisiting
EFI_MM_COMMUNICATION_PROTOCOL.

Data communicated to MM through EFI_MM_COMMUNICATION3_PROTOCOL should
always start with EFI_MM_COMMUNICATE_HEADER_V3 with its HeaderGuid,
Signature and Version fields properly populated.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Marvin Häuser <mhaeuser@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly introduced v3 of communicate protocol

v4:
- Rebased with uncrustify changes.

MdePkg/Include/Protocol/MmCommunication3.h | 70 ++++++++++++++++++++
MdePkg/MdePkg.dec | 3 +
2 files changed, 73 insertions(+)

diff --git a/MdePkg/Include/Protocol/MmCommunication3.h b/MdePkg/Include/Protocol/MmCommunication3.h
new file mode 100644
index 000000000000..0df9e5b875b7
--- /dev/null
+++ b/MdePkg/Include/Protocol/MmCommunication3.h
@@ -0,0 +1,70 @@
+/** @file
+ EFI MM Communication Protocol 2 as defined in the PI 1.7 errata A specification.
+
+ This protocol provides a means of communicating between drivers outside of MM and MMI
+ handlers inside of MM.
+
+ Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+ Copyright (c) 2019, Arm Limited. All rights reserved.<BR>
+ SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#ifndef MM_COMMUNICATION3_H_
+#define MM_COMMUNICATION3_H_
+
+#include <Pi/PiMultiPhase.h>
+
+#define EFI_MM_COMMUNICATION3_PROTOCOL_GUID \
+ { \
+ 0xf7234a14, 0xdf2, 0x46c0, { 0xad, 0x28, 0x90, 0xe6, 0xb8, 0x83, 0xa7, 0x2f } \
+ }
+
+typedef struct _EFI_MM_COMMUNICATION3_PROTOCOL EFI_MM_COMMUNICATION3_PROTOCOL;
+
+/**
+ Communicates with a registered handler.
+
+ This function provides a service to send and receive messages from a registered UEFI service.
+
+ @param[in] This The EFI_MM_COMMUNICATION3_PROTOCOL instance.
+ @param[in, out] CommBufferPhysical Physical address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommBufferVirtual Virtual address of the MM communication buffer, of which content must
+ start with EFI_MM_COMMUNICATE_HEADER_V3.
+ @param[in, out] CommSize The size of the data buffer being passed in. On exit, the size of data
+ being returned. Zero if the handler does not wish to reply with any data.
+ This parameter is optional and may be NULL.
+
+ @retval EFI_SUCCESS The message was successfully posted.
+ @retval EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+ @retval EFI_BAD_BUFFER_SIZE The buffer is too large for the MM implementation.
+ If this error is returned, the MessageLength field
+ in the CommBuffer header or the integer pointed by
+ CommSize, are updated to reflect the maximum payload
+ size the implementation can accommodate.
+ @retval EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize parameter,
+ if not omitted, are in address range that cannot be
+ accessed by the MM environment.
+
+**/
+typedef
+EFI_STATUS
+(EFIAPI *EFI_MM_COMMUNICATE3)(
+ IN CONST EFI_MM_COMMUNICATION3_PROTOCOL *This,
+ IN OUT VOID *CommBufferPhysical,
+ IN OUT VOID *CommBufferVirtual,
+ IN OUT UINTN *CommSize OPTIONAL
+ );
+
+///
+/// EFI MM Communication Protocol provides runtime services for communicating
+/// between DXE drivers and a registered MMI handler.
+///
+struct _EFI_MM_COMMUNICATION3_PROTOCOL {
+ EFI_MM_COMMUNICATE3 Communicate;
+};
+
+extern EFI_GUID gEfiMmCommunication3ProtocolGuid;
+
+#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 11c08e617511..8c61661e4ee7 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -1357,6 +1357,9 @@ [Protocols]
## Include/Protocol/MmCommunication2.h
gEfiMmCommunication2ProtocolGuid = { 0x378daedc, 0xf06b, 0x4446, { 0x83, 0x14, 0x40, 0xab, 0x93, 0x3c, 0x87, 0xa3 }}

+ ## Include/Protocol/MmCommunication3.h
+ gEfiMmCommunication3ProtocolGuid = { 0xf7234a14, 0xdf2, 0x46c0, { 0xad, 0x28, 0x90, 0xe6, 0xb8, 0x83, 0xa7, 0x2f }}
+
#
# Protocols defined in UEFI2.1/UEFI2.0/EFI1.1
#
--
2.34.1.windows.1


[PATCH v4 2/7] MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATE_HEADER_V3 to MdePkg

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This change introduces a new definition for MM communicate header
structure, intending to provide better portability between different
architectures (IA32 & X64) and adapt to flexible array supported by
modern compilers.

The original MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a
generic definition, was used for both PEI and DXE MM communication. On a
system that supports PEI MM launch, but operates PEI in 32bit mode and MM
foundation in 64bit, the current EFI_MM_COMMUNICATE_HEADER definition
will cause structure parse error due to UINTN used. This introduction
removes the architecture dependent field by defining this field as
UINT64.

The new signature could help identifying whether the data received is
compiliant with this new data structure, which will help for binary
release modules to identify usage of legacy data structure.

Version field is also added to indicate the current version of header in
case there is need for minor modification in the future.

The data field of MM communicate message is replaced with flexible array
to allow users not having to consume extra data during communicate and
author code more intrinsically.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Marvin Häuser <mhaeuser@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- Newly introduced communicate v3
- Used flexible arrays [Marvin]
- Added static assert [Michael]

v4:
- Rebased with uncrusitify changes.

MdePkg/Include/Pi/PiMultiPhase.h | 57 ++++++++++++++++++++
MdePkg/MdePkg.dec | 5 ++
2 files changed, 62 insertions(+)

diff --git a/MdePkg/Include/Pi/PiMultiPhase.h b/MdePkg/Include/Pi/PiMultiPhase.h
index a7e95820ef65..178606eacfb0 100644
--- a/MdePkg/Include/Pi/PiMultiPhase.h
+++ b/MdePkg/Include/Pi/PiMultiPhase.h
@@ -103,6 +103,17 @@ SPDX-License-Identifier: BSD-2-Clause-Patent
#define EFI_SMRAM_CLOSED EFI_MMRAM_CLOSED
#define EFI_SMRAM_LOCKED EFI_MMRAM_LOCKED

+///
+/// MM Communicate header constants
+///
+#define COMMUNICATE_HEADER_V3_GUID \
+ { \
+ 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } \
+ }
+
+#define EFI_MM_COMMUNICATE_HEADER_V3_SIGNATURE 0x4D434833 // "MCH3"
+#define EFI_MM_COMMUNICATE_HEADER_V3_VERSION 3
+
///
/// Structure describing a MMRAM region and its accessibility attributes.
///
@@ -149,6 +160,50 @@ typedef struct _EFI_MM_RESERVED_MMRAM_REGION {
UINT64 MmramReservedSize;
} EFI_MM_RESERVED_MMRAM_REGION;

+#pragma pack(1)
+
+///
+/// To avoid confusion in interpreting frames, the buffer communicating to MM core through
+/// EFI_MM_COMMUNICATE3 or later should always start with EFI_MM_COMMUNICATE_HEADER_V3.
+///
+typedef struct {
+ ///
+ /// Indicator GUID for MM core that the communication buffer is compliant with this v3 header.
+ /// Must be gCommunicateHeaderV3Guid.
+ ///
+ EFI_GUID HeaderGuid;
+ ///
+ /// Signature to indicate data is compliant with new MM communicate header structure.
+ /// Must be "MCH3".
+ ///
+ UINT32 Signature;
+ ///
+ /// MM communicate data format version, MM foundation entry point should check if incoming
+ /// data is a supported format before proceeding.
+ /// Must be 3.
+ ///
+ UINT32 Version;
+ ///
+ /// Allows for disambiguation of the message format.
+ ///
+ EFI_GUID MessageGuid;
+ ///
+ /// Describes the size of MessageData (in bytes) and does not include the size of the header.
+ ///
+ UINT64 MessageSize;
+ ///
+ /// Designates an array of bytes that is MessageSize in size.
+ ///
+ UINT8 MessageData[];
+} EFI_MM_COMMUNICATE_HEADER_V3;
+
+#pragma pack()
+
+STATIC_ASSERT (
+ (sizeof (EFI_MM_COMMUNICATE_HEADER_V3) == OFFSET_OF (EFI_MM_COMMUNICATE_HEADER_V3, MessageData)), \
+ "sizeof (EFI_MM_COMMUNICATE_HEADER_V3) does not align with the beginning of flexible array MessageData"
+ );
+
typedef enum {
EFI_PCD_TYPE_8,
EFI_PCD_TYPE_16,
@@ -208,4 +263,6 @@ EFI_STATUS
IN VOID *ProcedureArgument
);

+extern EFI_GUID gCommunicateHeaderV3Guid;
+
#endif
diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
index 59b405928bf8..11c08e617511 100644
--- a/MdePkg/MdePkg.dec
+++ b/MdePkg/MdePkg.dec
@@ -826,6 +826,11 @@ [Guids]
## Include/Protocol/CcMeasurement.h
gEfiCcFinalEventsTableGuid = { 0xdd4a4648, 0x2de7, 0x4665, { 0x96, 0x4d, 0x21, 0xd9, 0xef, 0x5f, 0xb4, 0x46 }}

+ #
+ # GUID indicates the MM communication data is compliant with v3 communication header.
+ #
+ gCommunicateHeaderV3Guid = { 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } }
+
[Guids.IA32, Guids.X64]
## Include/Guid/Cper.h
gEfiIa32X64ErrorTypeCacheCheckGuid = { 0xA55701F5, 0xE3EF, 0x43de, { 0xAC, 0x72, 0x24, 0x9B, 0x57, 0x3F, 0xAD, 0x2C }}
--
2.34.1.windows.1


[PATCH v4 1/7] EDK2 Code First: PI Specification: New communicate header and interfaces

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This change includes specification update markdown file that describes
the proposed PI Specification v1.7 Errata A in detail and potential
impact to the existing codebase.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <leif@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Marvin Häuser <mhaeuser@...>
Cc: Bret Barkelew <Bret.Barkelew@...>

Signed-off-by: Kun Qin <kuqin12@...>
---

Notes:
v3:
- switched to use communicate v3 instead of sprinkle structure updates

v4:
- No review, no change.

CodeFirst/BZ3430-SpecChange.md | 277 ++++++++++++++++++++
1 file changed, 277 insertions(+)

diff --git a/CodeFirst/BZ3430-SpecChange.md b/CodeFirst/BZ3430-SpecChange.md
new file mode 100644
index 000000000000..39a96b9a44f0
--- /dev/null
+++ b/CodeFirst/BZ3430-SpecChange.md
@@ -0,0 +1,277 @@
+# Title: Introduction of `EFI_MM_COMMUNICATE_HEADER_V3` and `MM_COMMUNICATE3_*` interface
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7 Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Introduce `EFI_PEI_MM_COMMUNICATION3_PPI` and `EFI_MM_COMMUNICATE3_PROTOCOL` that works with communication buffer starts with `EFI_MM_COMMUNICATE_HEADER_V3` to provide better portability between different architectures (IA32 & X64) and adapt to flexible array supported by modern compilers.
+
+## Benefits of the change
+
+In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the MessageLength field of `EFI_MM_COMMUNICATE_HEADER` (also defined as `EFI_SMM_COMMUNICATE_HEADER`) is defined as type UINTN.
+
+But this structure, as a generic definition, could be used for both PEI and DXE MM communication. Thus for a system that supports PEI Standalone MM launch, but operates PEI in 32bit mode and MM foundation in 64bit, the current `EFI_MM_COMMUNICATE_HEADER` definition will cause structure parse error due to UINTN used.
+
+This proposed data structure resolved it by introducing `EFI_MM_COMMUNICATE_HEADER_V3` that defines the MessageSize as UINT64 to remove data size ambiguity.
+
+The data structure still starts with a `HeaderGuid` field that is typed as `EFI_GUID`. This will be populated as `gCommunicateHeaderV3Guid` or `COMMUNICATE_HEADER_V3_GUID` as an indicator to differentiate new data format vesus legacy format.
+
+Furthermore, the addition of signature could help identifying whether the data received is compiliant with this new data structure, which will help for binary release modules to identify usage of legacy `EFI_MM_COMMUNICATE_HEADER`.
+
+Version field is also added to indicate the current version of header in case there is need for minor modification in the future.
+
+In addition, the data field of MM communicate message is replaced with flexible array to allow users not having to consume extra data during communicate and author code more intrinsically.
+
+On the non-MM environment side, the Standalone MM DXE IPL agent can add installation of `EFI_MM_COMMUNICATE3_PROTOCOL`, while the Standalone MM PEI IPL agent that launches MM foundation should publish and only publish `EFI_PEI_MM_COMMUNICATION3_PPI` for MM communication during PEI phase.
+
+For communication data that starts with `EFI_MM_COMMUNICATE_HEADER_V3`, callers always need to use V3 protocol/PPI to communicate with updated MM cores. These interface introductions instead of replacement can maintain the compatibility for existing codebase while resolving size mismatching occurred during data transfer between different architectures.
+
+## Impact of the change
+
+This change will impact the MM cores and IPLs:
+
+```bash
+MdeModulePkg/Core/PiSmmCore/PiSmmCore
+StandaloneMmPkg/Core/StandaloneMmCore
+MdeModulePkg/Core/PiSmmCore/PiSmmIpl
+```
+
+To cooporate with the newly proposed data format, existing MM cores need to be updated to parse incoming data properly to tell if the data is compliant with new or legacy format.
+
+The existing PiSmmIpl will need to be updated to publish `EFI_MM_COMMUNICATE3_PROTOCOL` for consumers that would like to invoke MMI with new data format.
+
+For potential proprietary IPLs that launches Standalone MM in PEI phase, if any, the PEIM should change to publish `EFI_PEI_MM_COMMUNICATION3_PPI` instead of `EFI_MM_COMMUNICATE_PPI`.
+
+Accordingly, all consumers in PEI phase that communicate to PEI launched Standalone MM should switch to use `EFI_PEI_MM_COMMUNICATION3_PPI` and `EFI_MM_COMMUNICATE_HEADER_V3`.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 4-1.5.1 Initializing MM Standalone Mode in PEI phase, the last bullet point of step 3 should be changed to:
+
+ ```c
+ Publishes the EFI_PEI_MM_COMMUNICATION3_PPI
+ ```
+
+1. In PI Specification v1.7 Errata A: Vol. 4, section 6.5 MM Communication PPI, add the following:
+
+ ```markdown
+ # EFI_PEI_MM_COMMUNICATION_PPI
+
+ ## Summary
+
+ This PPI provides a means of communicating between drivers outside of MM and MMI handlers inside of MM in PEI phase.
+
+ ## GUID
+
+ #define EFI_PEI_MM_COMMUNICATION3_PPI_GUID \
+ { \
+ 0xe70febf6, 0x13ef, 0x4a21, { 0x89, 0x9e, 0xb2, 0x36, 0xf8, 0x25, 0xff, 0xc9 } \
+ }
+
+ ## PPI Structure
+
+ typedef struct _EFI_PEI_MM_COMMUNICATION3_PPI {
+ EFI_PEI_MM_COMMUNICATE3 Communicate;
+ } EFI_PEI_MM_COMMUNICATION3_PPI;
+
+ ## Members
+
+ ### Communicate
+
+ Sends/receives a message for a registered handler. See the Communicate() function description.
+
+ ## Description
+
+ This PPI provides services for communicating between PEIM and a registered MMI handler.
+
+ # EFI_PEI_MM_COMMUNICATION_PPI.Communicate()
+
+ ## Summary
+ Communicates with a registered handler.
+
+ ## Prototype
+ typedef
+ EFI_STATUS
+ (EFIAPI *EFI_PEI_MM_COMMUNICATE3)(
+ IN CONST EFI_PEI_MM_COMMUNICATION3_PPI *This,
+ IN OUT VOID *CommBuffer,
+ IN OUT UINTN *CommSize
+ );
+
+ ## Parameters
+
+ ### This
+ The EFI_PEI_MM_COMMUNICATE3 instance.
+
+ ### CommBuffer
+
+ Pointer to the buffer to convey into MMRAM.
+
+ ### CommSize
+
+ The size of the data buffer being passed in. On exit, the size of data being returned. Zero if the handler does not wish to reply with any data.
+
+ ## Description
+
+ This function provides a service to send and receive messages from a registered PEI service. The EFI_PEI_MM_COMMUNICATION3_PPI driver is responsible for doing any of the copies such that the data lives in PEI-service-accessible RAM.
+
+ A given implementation of the EFI_PEI_MM_COMMUNICATION3_PPI may choose to use the EFI_MM_CONTROL_PPI for effecting the mode transition, or it may use some other method.
+
+ The agent invoking the communication interface must be physical/virtually 1:1 mapped.
+
+ To avoid confusion in interpreting frames, the CommBuffer parameter should always begin with EFI_MM_COMMUNICATE_HEADER_V3. The header data is mandatory for messages sent into the MM agent.
+
+ Once inside of MM, the MM infrastructure will call all registered handlers with the same HandlerType as the GUID specified by HeaderGuid and the CommBuffer pointing to Data.
+
+ This function is not reentrant.
+
+ ## Status Codes Returned
+ EFI_SUCCESS The message was successfully posted.
+ EFI_INVALID_PARAMETER The CommBuffer was NULL.
+ ```
+
+1. In PI Specification v1.7 Errata A: Vol. 4, section 6.5 MM Communication PPI, add the following:
+
+ ```markdown
+ # EFI_MM_COMMUNICATION3_PROTOCOL
+
+ ## Summary
+
+ This protocol provides a means of communicating between drivers outside of MM and MMI handlers inside of MM, for communication buffer that is compliant with EFI_MM_COMMUNICATE_HEADER_V3.
+
+ ## GUID
+
+ #define EFI_MM_COMMUNICATION3_PROTOCOL_GUID \
+ { \
+ 0xf7234a14, 0xdf2, 0x46c0, { 0xad, 0x28, 0x90, 0xe6, 0xb8, 0x83, 0xa7, 0x2f } \
+ }
+
+ ## Prototype
+ typedef struct _EFI_MM_COMMUNICATION3_PROTOCOL {
+ EFI_MM_COMMUNICATE3 Communicate;
+ } EFI_MM_COMMUNICATION3_PROTOCOL;
+
+ ## Members
+
+ ### Communicate
+
+ Sends/receives a message for a registered handler. See the Communicate() function description.
+
+ ## Description
+
+ This protocol provides runtime services for communicating between DXE drivers and a registered MMI handler.
+
+ # EFI_MM_COMMUNICATION3_PROTOCOL.Communicate()
+
+ ## Summary
+
+ Communicates with a registered handler.
+
+ ## Prototype
+
+ typedef
+ EFI_STATUS
+ (EFIAPI *EFI_MM_COMMUNICATE3)(
+ IN CONST EFI_MM_COMMUNICATION3_PROTOCOL *This,
+ IN OUT VOID *CommBufferPhysical,
+ IN OUT VOID *CommBufferVirtual,
+ IN OUT UINTN *CommSize OPTIONAL
+ );
+
+ ## Parameters
+
+ ### This
+
+ The EFI_MM_COMMUNICATION3_PROTOCOL instance.
+
+ ### CommBufferPhysical
+
+ Physical address of the buffer to convey into MMRAM, of which content must start with EFI_MM_COMMUNICATE_HEADER_V3.
+
+ ### CommBufferVirtual
+
+ Virtual address of the buffer to convey into MMRAM, of which content must start with EFI_MM_COMMUNICATE_HEADER_V3.
+
+ ### CommSize
+
+ The size of the data buffer being passed in. On exit, the size of data being returned. Zero if the handler does not wish to reply with any data. This parameter is optional and may be NULL.
+
+ ## Description
+
+ Usage is similar to EFI_MM_COMMUNICATION_PROTOCOL.Communicate() except for the notes below:
+
+ * Communication buffer transfer to MM core should start with EFI_MM_COMMUNICATE_HEADER_V3.
+ * Instead of passing just the physical address via the CommBuffer parameter, the caller must pass both the physical and the virtual addresses of the communication buffer.
+ * If no virtual remapping has taken place, the physical address will be equal to the virtual address, and so the caller is required to pass the same value for both parameters.
+
+ ## Related Definitions
+ typedef struct {
+ EFI_GUID HeaderGuid;
+ UINT32 Signature;
+ UINT32 Version;
+ EFI_GUID MessageGuid;
+ UINT64 MessageSize;
+ UINT8 MessageData[ANYSIZE_ARRAY];
+ } EFI_MM_COMMUNICATE_HEADER_V3;
+
+ #define COMMUNICATE_HEADER_V3_GUID \
+ { \
+ 0x68e8c853, 0x2ba9, 0x4dd7, { 0x9a, 0xc0, 0x91, 0xe1, 0x61, 0x55, 0xc9, 0x35 } \
+ }
+
+ #define EFI_MM_COMMUNICATE_HEADER_V3_SIGNATURE 0x4D434833 // "MCH3"
+ #define EFI_MM_COMMUNICATE_HEADER_V3_VERSION 3
+
+ ### HeaderGuid
+
+ Indicator GUID for MM core that the communication buffer is compliant with this v3 header. Must be COMMUNICATE_HEADER_V3_GUID.
+
+ ### Signature
+
+ Signature to indicate data is compliant with new MM communicate header structure. Must be "MCH3".
+
+ ### Version
+
+ MM communicate data format version, MM foundation entry point should check if incoming data is a supported format before proceeding. Must be 3.
+
+ ### MessageGuid
+
+ Allows for disambiguation of the message format.
+
+ ### MessageSize
+
+ Describes the size of MessageData (in bytes) and does not include the size of the header.
+
+ ### MessageData
+
+ Designates an array of bytes that is MessageSize in size.
+
+ ## Status Codes Returned
+
+ EFI_SUCCESS The message was successfully posted.
+ EFI_INVALID_PARAMETER CommBufferPhysical was NULL or CommBufferVirtual was NULL.
+ EFI_BAD_BUFFER_SIZE The buffer is too large for the MM implementation. If this error is returned, the MessageLength field in the CommBuffer header or the integer pointed by CommSize, are updated to reflect the maximum payload size the implementation can accommodate.
+ EFI_ACCESS_DENIED The CommunicateBuffer parameter or CommSize parameter, if not omitted, are in address range that cannot be accessed by the MM environment.
+ ```
+
+### Code Changes
+
+1. Add data structure and its related definitions in `MdePkg/Include/Pi/PiMultiPhase.h` to match new definition.
+
+1. Add interface definition of `MdePkg/Include/Protocol/MmCommunication3.h` and `MdePkg/Include/Protocol/MmCommunication3.h`, respectively, to match newly proposed interfaces.
+
+1. Extend PiSmmCore to inspect `HeaderGuid` of incoming communication data. If it matches `COMMUNICATE_HEADER_V3_GUID`, parse the incoming data to start with `EFI_MM_COMMUNICATE_HEADER_V3`, otherwise it will be parse as existing way.
+
+1. Extend StandaloneMmCore to inspect `HeaderGuid` of incoming communication data. If it matches `COMMUNICATE_HEADER_V3_GUID`, parse the incoming data to start with `EFI_MM_COMMUNICATE_HEADER_V3`, otherwise it will be parse as existing way.
+
+1. Extend PiSmmIpl to publish `EFI_MM_COMMUNICATION3_PROTOCOL`, the implementation of `EFI_MM_COMMUNICATION3_PROTOCOL.Communicate()` should parse the incoming data as it starts with `EFI_MM_COMMUNICATE_HEADER_V3`, when applicable.
--
2.34.1.windows.1


[PATCH v4 0/7] New MM Communicate header and interfaces

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430

This patch series is a rebase of previous submission:
https://edk2.groups.io/g/devel/message/79397

The patches introduced MM communicate interface v3 (both protocol and
PPI) to consume the corresponding new header structure. The new structure
fixed ambiguious data field size caused by UINTN, as well as integrated
flexible arrays for data fields, while maintaining the backwards
compatibility for all existing codebases. A specified GUID is used to
differentiate old MM headers from newly defined v4 header.

The specification change is also included in this patch series v4, where
the standalone MM IPL in PEI phase is specified to install new PPI v4
after setting MM foundation.

Compared to v3 series, v4 patch changes include:
a. Rebased with uncrustify changes;

Patch v4 branch: https://github.com/kuqin12/edk2/tree/BZ3398-MmCommunicate-Length-v4

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>
Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <leif@...>
Cc: Hao A Wu <hao.a.wu@...>
Cc: Marvin Häuser <mhaeuser@...>
Cc: Bret Barkelew <Bret.Barkelew@...>
Cc: Michael Kubacki <michael.kubacki@...>
Cc: Ard Biesheuvel <ardb+tianocore@...>
Cc: Sami Mujawar <sami.mujawar@...>
Cc: Jiewen Yao <jiewen.yao@...>
Cc: Supreeth Venkatesh <supreeth.venkatesh@...>
Cc: Jian J Wang <jian.j.wang@...>
Cc: Eric Dong <eric.dong@...>
Cc: Ray Ni <ray.ni@...>

Kun Qin (7):
EDK2 Code First: PI Specification: New communicate header and
interfaces
MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATE_HEADER_V3 to
MdePkg
MdePkg: MmCommunication: Introduce EFI_MM_COMMUNICATION3_PROTOCOL to
MdePkg
MdePkg: MmCommunication: Introduce EFI_PEI_MM_COMMUNICATION3_PPI to
MdePkg
MdeModulePkg: PiSmmCore: Added parser of new MM communicate header
StandaloneMmPkg: StandaloneMmCore: Parsing new MM communicate header
MdeModulePkg: PiSmmIpl: Update MessageLength calculation for
MmCommunicate

MdeModulePkg/Core/PiSmmCore/PiSmmCore.c | 51 ++--
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 190 ++++++++++++++
StandaloneMmPkg/Core/StandaloneMmCore.c | 35 ++-
CodeFirst/BZ3430-SpecChange.md | 277 ++++++++++++++++++++
MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf | 1 +
MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 2 +
MdePkg/Include/Pi/PiMultiPhase.h | 57 ++++
MdePkg/Include/Ppi/MmCommunication3.h | 57 ++++
MdePkg/Include/Protocol/MmCommunication3.h | 70 +++++
MdePkg/MdePkg.dec | 11 +
StandaloneMmPkg/Core/StandaloneMmCore.inf | 1 +
11 files changed, 730 insertions(+), 22 deletions(-)
create mode 100644 CodeFirst/BZ3430-SpecChange.md
create mode 100644 MdePkg/Include/Ppi/MmCommunication3.h
create mode 100644 MdePkg/Include/Protocol/MmCommunication3.h

--
2.34.1.windows.1


Re: [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class

Michael D Kinney
 

Hi Kun,

Memory Type Information is the name of an EDK II feature. Also, not all memory
type information changes required a reset/reboot. That is configurable by the
platform.

The system attribute that requires a reboot is if the UEFI Memory Map during a normal
boot is in a state that would be incompatible with a potential future ACPI S4
resume boot. Some OSes require the UEFI Memory ranges for RT and ACPI memory types
to be in the same location in normal boot and ACPI S4 resume boot.

I am wondering if we can choose a different name for the new PI Status code that
reflects this OS ACPI requirement for a consistent memory map instead of referring
to the EDK II Memory Type Information feature. That way, the PI Spec name would
allow implementations that do not necessarily required the EDK II specific
implementation feature.

RELEASE_ASSERT also seems to imply an implementation specific way the reset/reboot
is triggered. An ASSERT() is typically triggered for a condition for which the
code that follow the ASSERT() can not continue without unexpected or undefined
behavior. So the system is in a bad state that is not recoverable. This type of
state could be detected with a normal if/then/else logic in C code when looking
at system state or encapsulated in an ASSERT() that is enabled in release builds.
Once again, I think we need a different name that does not require the detection
logic to be in an EDK II ASSERT() macro.

Thanks,

Mike

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Kun Qin
Sent: Thursday, January 6, 2022 5:03 PM
To: devel@edk2.groups.io
Cc: Andrew Fish <afish@...>; Leif Lindholm <leif@...>; Kinney, Michael D <michael.d.kinney@...>; Gao, Liming
<gaoliming@...>; Liu, Zhiguang <zhiguang.liu@...>
Subject: [edk2-devel] [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794

This change includes specification update markdown file that describes
the proposed PI Specification v1.7 Errata A in detail and potential
impact to the existing codebase.

Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
CodeFirst/BZ3794-SpecChange.md | 60 ++++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/CodeFirst/BZ3794-SpecChange.md b/CodeFirst/BZ3794-SpecChange.md
new file mode 100644
index 000000000000..bbb526896795
--- /dev/null
+++ b/CodeFirst/BZ3794-SpecChange.md
@@ -0,0 +1,60 @@
+# Title: Introduction of `EFI_MM_COMMUNICATE_HEADER_V3` and `MM_COMMUNICATE3_*` interface
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7 Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Introduce `EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE` and `EFI_SW_EC_RELEASE_ASSERT` into Status Codes definition.
+
+## Benefits of the change
+
+Current Status Codes covered various [software class error code
definitions](https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Pi/PiStatusCode.h).
+
+However, there are a few critical instances where the software could trigger system reboots while the corresponding case was not
covered by the already defined status codes:
+
+1. Memory type information change triggered system reboot;
+2. Assert triggered reboot on systems that did not enable system halts;
+
+The unexpected system reboots above could indicate decay of system health and reporting of such generic events would provide
helpful information to OEMs to investigate/prevent system failures in general.
+
+The request of this change intends to expand definitions of `EFI_SW_EC_**` under Status Codes to cover more unexpected system
reboot events, which could improve Status Code futility and readability.
+
+## Impact of the change
+
+Occupy 2 new macro definitions of Error Codes under Software class Status Codes.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Table 3-61: Error Code Operations: Host Software Class, add 2 new rows below
`EFI_SW_EC_FV_CORRUPTED` definition:
+
+ | Operation | Description | Extended Data |
+ | --- | --- | --- |
+ | EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE | System will reboot due to memory type information changes | None |
+ | EFI_SW_EC_RELEASE_ASSERT | System software asserted | None |
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Table 3-61: Error Code Operations: Host Software Class, replace the row of
`0x0014–0x00FF` to:
+
+ | Operation | Description | Extended Data |
+ | --- | --- | --- |
+ | 0x0016–0x00FF | Reserved for future use by this specification for Host Software class error codes. | None |
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Section 6.7.4.3 Error Code Definitions: Prototype, add 2 new definitions below
`EFI_SW_EC_FV_CORRUPTED` definition:
+
+ ```c
+ #define EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE 0x00000014
+ #define EFI_SW_EC_RELEASE_ASSERT 0x00000015
+ ```
+
+### Code Changes
+
+1. Add macro definitions in `MdePkg/Include/Pi/PiStatusCode.h` to match new specification.
--
2.34.1.windows.1





Now: TianoCore Design Meeting - APAC/NAMO - 01/07/2022 #cal-notice

devel@edk2.groups.io Calendar <noreply@...>
 

TianoCore Design Meeting - APAC/NAMO

When:
01/07/2022
9:30am to 10:30am
(UTC+08:00) Asia/Shanghai

Where:
Microsoft Teams

Organizer: Ray Ni ray.ni@...

View Event

Description:

TOPIC

  1. NA

For more info, see here: https://www.tianocore.org/design-meeting/


Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 119 715 416 0

Alternate VTC dialing instructions

Learn More | Meeting options


Event: TianoCore Design Meeting - APAC/NAMO - 01/07/2022 #cal-reminder

devel@edk2.groups.io Calendar <noreply@...>
 

Reminder: TianoCore Design Meeting - APAC/NAMO

When:
01/07/2022
9:30am to 10:30am
(UTC+08:00) Asia/Shanghai

Where:
Microsoft Teams

Organizer: Ray Ni ray.ni@...

View Event

Description:

TOPIC

  1. NA

For more info, see here: https://www.tianocore.org/design-meeting/


Microsoft Teams meeting

Join on your computer or mobile app

Click here to join the meeting

Join with a video conferencing device

teams@...

Video Conference ID: 119 715 416 0

Alternate VTC dialing instructions

Learn More | Meeting options


[PATCH v1 2/2] MdePkg: MmCommunication: Add new Host Software class Error Codes to MdePkg

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794

This change introduces two new error code definitions under Host Software
class.

The new error code definitions will cover system reboot events under the
conditions of memory type information change and software asserts when
system did not enable system halts.

These error codes could provide helpful datapoints to OEMs to investigate
and prevent system failures in general.

Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
MdePkg/Include/Pi/PiStatusCode.h | 42 ++++++++++----------
1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/MdePkg/Include/Pi/PiStatusCode.h b/MdePkg/Include/Pi/PiStatusCode.h
index ef2aea7364bc..6a1c9cc30f52 100644
--- a/MdePkg/Include/Pi/PiStatusCode.h
+++ b/MdePkg/Include/Pi/PiStatusCode.h
@@ -965,26 +965,28 @@ typedef struct {
/// These are shared by all subclasses.
///
///@{
-#define EFI_SW_EC_NON_SPECIFIC 0x00000000
-#define EFI_SW_EC_LOAD_ERROR 0x00000001
-#define EFI_SW_EC_INVALID_PARAMETER 0x00000002
-#define EFI_SW_EC_UNSUPPORTED 0x00000003
-#define EFI_SW_EC_INVALID_BUFFER 0x00000004
-#define EFI_SW_EC_OUT_OF_RESOURCES 0x00000005
-#define EFI_SW_EC_ABORTED 0x00000006
-#define EFI_SW_EC_ILLEGAL_SOFTWARE_STATE 0x00000007
-#define EFI_SW_EC_ILLEGAL_HARDWARE_STATE 0x00000008
-#define EFI_SW_EC_START_ERROR 0x00000009
-#define EFI_SW_EC_BAD_DATE_TIME 0x0000000A
-#define EFI_SW_EC_CFG_INVALID 0x0000000B
-#define EFI_SW_EC_CFG_CLR_REQUEST 0x0000000C
-#define EFI_SW_EC_CFG_DEFAULT 0x0000000D
-#define EFI_SW_EC_PWD_INVALID 0x0000000E
-#define EFI_SW_EC_PWD_CLR_REQUEST 0x0000000F
-#define EFI_SW_EC_PWD_CLEARED 0x00000010
-#define EFI_SW_EC_EVENT_LOG_FULL 0x00000011
-#define EFI_SW_EC_WRITE_PROTECTED 0x00000012
-#define EFI_SW_EC_FV_CORRUPTED 0x00000013
+#define EFI_SW_EC_NON_SPECIFIC 0x00000000
+#define EFI_SW_EC_LOAD_ERROR 0x00000001
+#define EFI_SW_EC_INVALID_PARAMETER 0x00000002
+#define EFI_SW_EC_UNSUPPORTED 0x00000003
+#define EFI_SW_EC_INVALID_BUFFER 0x00000004
+#define EFI_SW_EC_OUT_OF_RESOURCES 0x00000005
+#define EFI_SW_EC_ABORTED 0x00000006
+#define EFI_SW_EC_ILLEGAL_SOFTWARE_STATE 0x00000007
+#define EFI_SW_EC_ILLEGAL_HARDWARE_STATE 0x00000008
+#define EFI_SW_EC_START_ERROR 0x00000009
+#define EFI_SW_EC_BAD_DATE_TIME 0x0000000A
+#define EFI_SW_EC_CFG_INVALID 0x0000000B
+#define EFI_SW_EC_CFG_CLR_REQUEST 0x0000000C
+#define EFI_SW_EC_CFG_DEFAULT 0x0000000D
+#define EFI_SW_EC_PWD_INVALID 0x0000000E
+#define EFI_SW_EC_PWD_CLR_REQUEST 0x0000000F
+#define EFI_SW_EC_PWD_CLEARED 0x00000010
+#define EFI_SW_EC_EVENT_LOG_FULL 0x00000011
+#define EFI_SW_EC_WRITE_PROTECTED 0x00000012
+#define EFI_SW_EC_FV_CORRUPTED 0x00000013
+#define EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE 0x00000014
+#define EFI_SW_EC_RELEASE_ASSERT 0x00000015
///@}

//
--
2.34.1.windows.1


[PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794

This change includes specification update markdown file that describes
the proposed PI Specification v1.7 Errata A in detail and potential
impact to the existing codebase.

Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Signed-off-by: Kun Qin <kuqin12@...>
---
CodeFirst/BZ3794-SpecChange.md | 60 ++++++++++++++++++++
1 file changed, 60 insertions(+)

diff --git a/CodeFirst/BZ3794-SpecChange.md b/CodeFirst/BZ3794-SpecChange.md
new file mode 100644
index 000000000000..bbb526896795
--- /dev/null
+++ b/CodeFirst/BZ3794-SpecChange.md
@@ -0,0 +1,60 @@
+# Title: Introduction of `EFI_MM_COMMUNICATE_HEADER_V3` and `MM_COMMUNICATE3_*` interface
+
+## Status: Draft
+
+## Document: UEFI Platform Initialization Specification Version 1.7 Errata A
+
+## License
+
+SPDX-License-Identifier: CC-BY-4.0
+
+## Submitter: [TianoCore Community](https://www.tianocore.org)
+
+## Summary of the change
+
+Introduce `EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE` and `EFI_SW_EC_RELEASE_ASSERT` into Status Codes definition.
+
+## Benefits of the change
+
+Current Status Codes covered various [software class error code definitions](https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Pi/PiStatusCode.h).
+
+However, there are a few critical instances where the software could trigger system reboots while the corresponding case was not covered by the already defined status codes:
+
+1. Memory type information change triggered system reboot;
+2. Assert triggered reboot on systems that did not enable system halts;
+
+The unexpected system reboots above could indicate decay of system health and reporting of such generic events would provide helpful information to OEMs to investigate/prevent system failures in general.
+
+The request of this change intends to expand definitions of `EFI_SW_EC_**` under Status Codes to cover more unexpected system reboot events, which could improve Status Code futility and readability.
+
+## Impact of the change
+
+Occupy 2 new macro definitions of Error Codes under Software class Status Codes.
+
+## Detailed description of the change [normative updates]
+
+### Specification Changes
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Table 3-61: Error Code Operations: Host Software Class, add 2 new rows below `EFI_SW_EC_FV_CORRUPTED` definition:
+
+ | Operation | Description | Extended Data |
+ | --- | --- | --- |
+ | EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE | System will reboot due to memory type information changes | None |
+ | EFI_SW_EC_RELEASE_ASSERT | System software asserted | None |
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Table 3-61: Error Code Operations: Host Software Class, replace the row of `0x0014–0x00FF` to:
+
+ | Operation | Description | Extended Data |
+ | --- | --- | --- |
+ | 0x0016–0x00FF | Reserved for future use by this specification for Host Software class error codes. | None |
+
+1. In PI Specification v1.7 Errata A: Vol. 3, Section 6.7.4.3 Error Code Definitions: Prototype, add 2 new definitions below `EFI_SW_EC_FV_CORRUPTED` definition:
+
+ ```c
+ #define EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE 0x00000014
+ #define EFI_SW_EC_RELEASE_ASSERT 0x00000015
+ ```
+
+### Code Changes
+
+1. Add macro definitions in `MdePkg/Include/Pi/PiStatusCode.h` to match new specification.
--
2.34.1.windows.1


[PATCH v1 0/2] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER

Kun Qin
 

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794

In current Status Codes definitions of PI spec v1.7 errata, there are a
few critical instances where the software could trigger system reboots
while the corresponding case were not covered by the already defined
status codes.

Two scenarios that OEMs could be interested are memory type information
change triggered system reboot and assert triggered reboot on systems
that did not enable system halts.

The unexpected system reboots above could indicate decay of system health
and reporting of such generic events would provide helpful information to
OEMs to investigate/prevent system failures in general.

The change intends to expand definitions of `EFI_SW_EC_**` under Status
Codes to cover more unexpected system reboot events, which could improve
Status Code futility and readability.

Patch v1 branch: https://github.com/kuqin12/edk2/tree/BZ3794-expand_status_codes_v1

Cc: Andrew Fish <afish@...>
Cc: Leif Lindholm <leif@...>
Cc: Michael D Kinney <michael.d.kinney@...>
Cc: Liming Gao <gaoliming@...>
Cc: Zhiguang Liu <zhiguang.liu@...>

Kun Qin (2):
EDK2 Code First: PI Specification: New error codes of Host Software
class
MdePkg: MmCommunication: Add new Host Software class Error Codes to
MdePkg

CodeFirst/BZ3794-SpecChange.md | 60 ++++++++++++++++++++
MdePkg/Include/Pi/PiStatusCode.h | 42 +++++++-------
2 files changed, 82 insertions(+), 20 deletions(-)
create mode 100644 CodeFirst/BZ3794-SpecChange.md

--
2.34.1.windows.1


Re: Guidance about CI

Boeuf, Sebastien
 

On Wed, 2022-01-05 at 17:55 +0100, kraxel@... wrote:
On Wed, Jan 05, 2022 at 01:44:01PM +0000, Boeuf, Sebastien wrote:
Ah nevermind I found out QEMU was installed from packaging.
On ubuntu.

We don't have packages for Cloud Hypervisor, but we can download
a static binary from a specific release, do you think that would be
acceptable?
As far I know the same happens for qemu on windows,
so that should be fine.
Cool!

BTW, about microvm, I saw that you're skipping QEMU, so does that mean
you're not *really* testing that OVMF works with microvm, or am I
missing something?

Thanks,
Sebastien


take care,
  Gerd
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Re: Change from "BaseTools/LzmaCompress: Fix possible uninitialized variable" patch was reverted

Denis Nikitin <denik@...>
 

Thanks for confirmation. I agree that the warning doesn't look severe.

- Denis

On Wed, Jan 5, 2022 at 4:46 PM Wu, Hao A <hao.a.wu@...> wrote:

Hello Denis,

As far as I can recall, the fix you mentioned is a change that to please the static analysis tool.
My opinion is that it not a critical fix.

Best Regards,
Hao Wu

-----Original Message-----
From: Denis Nikitin <denik@...>
Sent: Thursday, January 6, 2022 3:19 AM
To: Wu, Hao A <hao.a.wu@...>
Cc: liming.gao@...; yonghong.zhu@...; devel@edk2.groups.io;
Ramasubramanian, Karthik <kramasub@...>
Subject: Change from "BaseTools/LzmaCompress: Fix possible uninitialized
variable" patch was reverted

Hi Hao,

While updating edk2 in Chrome OS we noticed that the change in your patch
https://edk2.groups.io/g/devel/message/19270 was reverted in
commit:
5ec5a236d1 BaseTools Lzma: Update LZMA SDK version to 18.05.

If you think the fix is critical I think it should be merged in https://www.7-
zip.org/sdk.html.

Thanks,
Denis


Re: EFI shell with microvm

Boeuf, Sebastien
 

On Thu, 2022-01-06 at 13:44 +0100, kraxel@... wrote:
On Thu, Jan 06, 2022 at 11:25:37AM +0000, Boeuf, Sebastien wrote:
Hi Gerd,

I was looking at a way to add support for EFI shell interaction
with Cloud Hypervisor when
I realized you added the support for microvm with commit
55f47d22998.
I have been able to hack OvmfPkgX64 similarly to get it to work,
but here are two follow up
questions:
How do you want interact?  Serial console?  That should work just
fine
with OvmfPkgX64.
Yes I'd like to use the serial console (available on PIO 0x3f8) to
interact with the EFI shell.


qemu-system-x86_64 -nographic -bios
Build/OvmfX64/DEBUG_GCC5/FV/OVMF.fd -net none

Possibly you have to add the cloud hypervisor pci id somewhere so isa
lpc and serial line driver are initialized properly.  SioBusDxe looks
like a hot candidate.

microvm has no lpc bridge, so I had to do it in a different way ...
Cloud Hypervisor doesn't emulate any LPC bridge or ISA bus.


2. I can see the shell but I can't interact with it, do you have a
similar behavior with microvm
or is it because I'm missing the interrupt support?
Works fine for me.

qemu-system-x86_64 -nographic -machine microvm,rtc=on -bios
Build/MicrovmX64/DEBUG_GCC5/FV/MICROVM.fd
Thanks for the confirmation, something might be wrong with the
interrupt used for my serial device.
Cloud Hypervisor only has an IOAPIC, it doesn't rely on any PIC, which
is why I'm not sure what might be missing to get the EFI shell to
receive the interrupts.


HTH,
  Gerd
---------------------------------------------------------------------
Intel Corporation SAS (French simplified joint stock company)
Registered headquarters: "Les Montalets"- 2, rue de Paris,
92196 Meudon Cedex, France
Registration Number: 302 456 199 R.C.S. NANTERRE
Capital: 4,572,000 Euros

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.


Re: EFI shell with microvm

Gerd Hoffmann
 

On Thu, Jan 06, 2022 at 11:25:37AM +0000, Boeuf, Sebastien wrote:
Hi Gerd,

I was looking at a way to add support for EFI shell interaction with Cloud Hypervisor when
I realized you added the support for microvm with commit 55f47d22998.
I have been able to hack OvmfPkgX64 similarly to get it to work, but here are two follow up
questions:
How do you want interact? Serial console? That should work just fine
with OvmfPkgX64.

qemu-system-x86_64 -nographic -bios Build/OvmfX64/DEBUG_GCC5/FV/OVMF.fd -net none

Possibly you have to add the cloud hypervisor pci id somewhere so isa
lpc and serial line driver are initialized properly. SioBusDxe looks
like a hot candidate.

microvm has no lpc bridge, so I had to do it in a different way ...

2. I can see the shell but I can't interact with it, do you have a similar behavior with microvm
or is it because I'm missing the interrupt support?
Works fine for me.

qemu-system-x86_64 -nographic -machine microvm,rtc=on -bios Build/MicrovmX64/DEBUG_GCC5/FV/MICROVM.fd

HTH,
Gerd

9341 - 9360 of 94571