REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 In PI Spec v1.7 Errata A, Vol.4, Sec 5.7 MM Communication Protocol, the MessageLength field of EFI_MM_COMMUNICATE_HEADER (also derived as EFI_SMM_COMMUNICATE_HEADER) is currently 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 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 being used. The suggested change is to make the MessageLength field defined with definitive size as below: ``` typedef struct { EFI_GUID HeaderGuid; UINT64 MessageLength; UINT8 Data[ANYSIZE_ARRAY]; } EFI_MM_COMMUNICATE_HEADER; ``` Patch v1 branch: https://github.com/kuqin12/edk2/tree/BZ3398-MmCommunicate-Length Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Leif Lindholm <leif@nuviainc.com> Kun Qin (5): EDK2 Code First: PI Specification: EFI_MM_COMMUNICATE_HEADER Update MdeModulePkg: PiSmmIpl: Update MessageLength calculation for MmCommunicate MdeModulePkg: MemoryProfileInfo: Updated MessageLength calculation MdeModulePkg: SmiHandlerProfileInfo: Updated MessageLength calculation MdePkg: MmCommunication: Extend MessageLength field size to UINT64 MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 20 +++-- MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 8 +- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++- BZ3430-SpecChange.md | 88 ++++++++++++++++++++ MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 + MdePkg/Include/Protocol/MmCommunication.h | 3 +- 6 files changed, 124 insertions(+), 9 deletions(-) create mode 100644 BZ3430-SpecChange.md -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76303): https://edk2.groups.io/g/devel/message/76303 Mute This Topic: https://groups.io/mt/83435921/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- 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@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Cc: Andrew Fish <afish@apple.com> Cc: Laszlo Ersek <lersek@redhat.com> Cc: Leif Lindholm <leif@nuviainc.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- BZ3430-SpecChange.md | 88 ++++++++++++++++++++ 1 file changed, 88 insertions(+) diff --git a/BZ3430-SpecChange.md b/BZ3430-SpecChange.md new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/BZ3430-SpecChange.md @@ -XXX,XX +XXX,XX @@ +# Title: Change MessageLength Field of EFI_MM_COMMUNICATE_HEADER to UINT64 + +## 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 + +Change the `MessageLength` Field of `EFI_MM_COMMUNICATE_HEADER` from UINTN to UINT64 to remove architecture dependency: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINT64 MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +## 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 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. + +## Impact of the change + +This change will impact the known structure consumers including: + +```bash +MdeModulePkg/Core/PiSmmCore/PiSmmIpl +MdeModulePkg/Application/SmiHandlerProfileInfo +MdeModulePkg/Application/MemoryProfileInfo +``` + +For consumers that are not using `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)`, but performing explicit addition such as the existing MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c, one will need to change code implementation to match new structure definition. Otherwise, the code compiled on IA32 architecture will experience structure field dereference error. + +User who currently uses UINTN local variables as place holder of MessageLength will need to use caution to make cast from UINTN to UINT64 and vice versa. It is recommended to use `SafeUint64ToUintn` for such operations when the value is indeterministic. + +Note: MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib is also consuming this structure, but it handled this size discrepancy internally. If this potential spec change is not applied, all applicable PEI MM communicate callers will need to use the same routine as that of SmmLockBoxPeiLib to invoke a properly populated EFI_MM_COMMUNICATE_HEADER to be used in X64 MM foundation. + +## Detailed description of the change [normative updates] + +### Specification Changes + +1. In PI Specification v1.7 Errata A: Vol. 4 Page-91, the definition of `EFI_MM_COMMUNICATE_HEADER` should be changed from current: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINTN MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +to: + +```c +typedef struct { + EFI_GUID HeaderGuid; + UINT64 MessageLength; + UINT8 Data[ANYSIZE_ARRAY]; +} EFI_MM_COMMUNICATE_HEADER; +``` + +### Code Changes + +1. Remove the explicit calculation of the offset of `Data` in `EFI_MM_COMMUNICATE_HEADER`. Thus applicable calculations of `sizeof(EFI_GUID) + sizeof(UINTN)` should be replaced with `OFFSET_OF(EFI_MM_COMMUNICATE_HEADER, Data)` or similar alternatives. These calculations are identified in: + +```bash +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +``` + +1. Resolve potentially mismatched type between `UINTN` and `UINT64`. This would occur when `MessageLength` or its derivitive are used for local calculation with existing `UINTN` typed variables. Code change regarding this perspective is per case evaluation: if the variables involved are all deterministic values, and there is no overflow or underflow risk, a cast operation (from `UINTN` to `UINT64`) can be safely used. Otherwise, the calculation will be performed in `UINT64` bitwidth and then convert to `UINTN` using `SafeUint64*` and `SafeUint64ToUintn`, respectively. These operations are identified in: + +```bash +MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +``` + +1. After all above changes applied and specification updated, `MdePkg/Include/Protocol/MmCommunication.h` will need to be updated to match new definition that includes the field type update. -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76304): https://edk2.groups.io/g/devel/message/76304 Mute This Topic: https://groups.io/mt/83435922/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 This change updated calculation routine for MM communication in PiSmmIpl. It removes ambiguity brought in by UINTN variables from this routine and paves way for updating definition of field MessageLength in EFI_MM_COMMUNICATE_HEADER to definitive size. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 13 ++++++++++++- MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c index XXXXXXX..XXXXXXX 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c @@ -XXX,XX +XXX,XX @@ #include <Library/UefiRuntimeLib.h> #include <Library/PcdLib.h> #include <Library/ReportStatusCodeLib.h> +#include <Library/SafeIntLib.h> // BZ3398 #include "PiSmmCorePrivateData.h" @@ -XXX,XX +XXX,XX @@ SmmCommunicationCommunicate ( EFI_STATUS Status; EFI_SMM_COMMUNICATE_HEADER *CommunicateHeader; BOOLEAN OldInSmm; + UINT64 BZ3398_LongCommSize; UINTN TempCommSize; // @@ -XXX,XX +XXX,XX @@ SmmCommunicationCommunicate ( CommunicateHeader = (EFI_SMM_COMMUNICATE_HEADER *) CommBuffer; if (CommSize == NULL) { - TempCommSize = OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data) + CommunicateHeader->MessageLength; + // BZ3398 Starts: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + Status = SafeUint64Add (OFFSET_OF (EFI_SMM_COMMUNICATE_HEADER, Data), CommunicateHeader->MessageLength, &BZ3398_LongCommSize); + if (EFI_ERROR (Status)) { + return EFI_INVALID_PARAMETER; + } + Status = SafeUint64ToUintn (BZ3398_LongCommSize, &TempCommSize); + if (EFI_ERROR (Status)) { + return EFI_INVALID_PARAMETER; + } + // BZ3398 Ends } else { TempCommSize = *CommSize; // diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf index XXXXXXX..XXXXXXX 100644 --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.inf @@ -XXX,XX +XXX,XX @@ [LibraryClasses] DxeServicesLib PcdLib ReportStatusCodeLib + SafeIntLib #BZ3398 [Protocols] gEfiSmmBase2ProtocolGuid ## PRODUCES -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76305): https://edk2.groups.io/g/devel/message/76305 Mute This Topic: https://groups.io/mt/83435923/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 This change replaced the calculation of communication buffer size from explicitly adding the size of each member with the OFFSET macro function. This will make the structure field defition change transparent to consumers. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c | 20 +++++++++++++++----- 1 file changed, 15 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c index XXXXXXX..XXXXXXX 100644 --- a/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c +++ b/MdeModulePkg/Application/MemoryProfileInfo/MemoryProfileInfo.c @@ -XXX,XX +XXX,XX @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); if (EFI_ERROR (Status)) { DEBUG ((EFI_D_ERROR, "SmramProfile: SmmCommunication - %r\n", Status)); @@ -XXX,XX +XXX,XX @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_DISABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); } @@ -XXX,XX +XXX,XX @@ GetSmramProfileData ( CommGetProfileInfo->Header.ReturnStatus = (UINT64)-1; CommGetProfileInfo->ProfileSize = 0; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); ASSERT_EFI_ERROR (Status); @@ -XXX,XX +XXX,XX @@ GetSmramProfileData ( CommGetProfileData->Header.DataLength = sizeof (*CommGetProfileData); CommGetProfileData->Header.ReturnStatus = (UINT64)-1; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Buffer = (UINT8 *) CommHeader + CommSize; Size -= CommSize; @@ -XXX,XX +XXX,XX @@ GetSmramProfileData ( CommRecordingState->Header.ReturnStatus = (UINT64)-1; CommRecordingState->RecordingState = MEMORY_PROFILE_RECORDING_ENABLE; - CommSize = sizeof (EFI_GUID) + sizeof (UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; SmmCommunication->Communicate (SmmCommunication, CommBuffer, &CommSize); } -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76306): https://edk2.groups.io/g/devel/message/76306 Mute This Topic: https://groups.io/mt/83435924/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 This change replaced the calculation of communication buffer size from explicitly adding the size of each member with the OFFSET macro function. This will make the structure field defition change transparent to consumers. Cc: Jian J Wang <jian.j.wang@intel.com> Cc: Hao A Wu <hao.a.wu@intel.com> Cc: Eric Dong <eric.dong@intel.com> Cc: Ray Ni <ray.ni@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c index XXXXXXX..XXXXXXX 100644 --- a/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c +++ b/MdeModulePkg/Application/SmiHandlerProfileInfo/SmiHandlerProfileInfo.c @@ -XXX,XX +XXX,XX @@ GetSmiHandlerProfileDatabase( CommGetInfo->Header.ReturnStatus = (UINT64)-1; CommGetInfo->DataSize = 0; - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Status = SmmCommunication->Communicate(SmmCommunication, CommBuffer, &CommSize); if (EFI_ERROR(Status)) { Print(L"SmiHandlerProfile: SmmCommunication - %r\n", Status); @@ -XXX,XX +XXX,XX @@ GetSmiHandlerProfileDatabase( CommGetData->Header.DataLength = sizeof(*CommGetData); CommGetData->Header.ReturnStatus = (UINT64)-1; - CommSize = sizeof(EFI_GUID) + sizeof(UINTN) + CommHeader->MessageLength; + // BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + // The CommHeader->MessageLength contains a definitive value, thus UINTN cast is safe here. + CommSize = OFFSET_OF(EFI_SMM_COMMUNICATE_HEADER, Data) + (UINTN)CommHeader->MessageLength; Buffer = (UINT8 *)CommHeader + CommSize; Size -= CommSize; -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76307): https://edk2.groups.io/g/devel/message/76307 Mute This Topic: https://groups.io/mt/83435925/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3398 REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3430 The MessageLength field of EFI_MM_COMMUNICATE_HEADER, as a generic definition, could be 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 change removes the architecture dependent field by extending this field definition as UINT64. Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- MdePkg/Include/Protocol/MmCommunication.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MdePkg/Include/Protocol/MmCommunication.h b/MdePkg/Include/Protocol/MmCommunication.h index XXXXXXX..XXXXXXX 100644 --- a/MdePkg/Include/Protocol/MmCommunication.h +++ b/MdePkg/Include/Protocol/MmCommunication.h @@ -XXX,XX +XXX,XX @@ typedef struct { /// /// Describes the size of Data (in bytes) and does not include the size of the header. /// - UINTN MessageLength; + /// BZ3398: Make MessageLength the same size in EFI_MM_COMMUNICATE_HEADER for both IA32 and X64. + UINT64 MessageLength; /// /// Designates an array of bytes that is MessageLength in size. /// -- 2.31.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#76308): https://edk2.groups.io/g/devel/message/76308 Mute This Topic: https://groups.io/mt/83435926/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794 This patch series is a rebase of previous submission: https://edk2.groups.io/g/devel/message/85302 In current Status Codes definitions of PI spec v1.7 errata, there are a few instances where the software could trigger system reboots while the corresponding case were not covered by the already defined status codes. One scenario that OEMs would be interested is that fragmented memory map from boot to boot would fail to meet certain OS ACPI requirements (i.e. S4 resume boot requires consistent memory maps) and trigger system reboots. Yet the corresponding case was not covered by the already defined status codes. 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. Compared to v1 series, v2 patch changes include: a. Removed "RELEASE_ASSERT" definition; b. Removed reference to memory type information; Patch v2 branch: https://github.com/kuqin12/edk2/tree/BZ3794-expand_status_codes_v2 Cc: Andrew Fish <afish@apple.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Kun Qin (2): EDK2 Code First: PI Specification: New error codes of Host Software class MdePkg: MmCommunication: Add new Host Software class Error Code to MdePkg CodeFirst/BZ3794-SpecChange.md | 55 ++++++++++++++++++++ MdePkg/Include/Pi/PiStatusCode.h | 1 + 2 files changed, 56 insertions(+) create mode 100644 CodeFirst/BZ3794-SpecChange.md -- 2.34.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85335): https://edk2.groups.io/g/devel/message/85335 Mute This Topic: https://groups.io/mt/88276262/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- 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@apple.com> Cc: Leif Lindholm <leif@nuviainc.com> Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- Notes: v2: - Removed "RELEASE_ASSERT" definition - Removed reference to EDK2 based memory type info [Mike] CodeFirst/BZ3794-SpecChange.md | 55 ++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/CodeFirst/BZ3794-SpecChange.md b/CodeFirst/BZ3794-SpecChange.md new file mode 100644 index XXXXXXX..XXXXXXX --- /dev/null +++ b/CodeFirst/BZ3794-SpecChange.md @@ -XXX,XX +XXX,XX @@ +# Title: Introduction of `EFI_SW_EC_FRAGMENTED_MEMORY_MAP` Status Code + +## 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 + +Add `EFI_SW_EC_FRAGMENTED_MEMORY_MAP` 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, fragmented memory map from boot to boot would fail to meet certain OS ACPI requirements (i.e. S4 resume boot requires consistent memory maps) and trigger system reboots. Yet the corresponding case was not covered by the already defined status codes. + +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 a 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 one new rows below `EFI_SW_EC_FV_CORRUPTED` definition: + + | Operation | Description | Extended Data | + | --- | --- | --- | + | EFI_SW_EC_FRAGMENTED_MEMORY_MAP | System will reboot due to fragmented memory maps | 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 | + | --- | --- | --- | + | 0x0015-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 one new definitions below `EFI_SW_EC_FV_CORRUPTED` definition: + + ```c + #define EFI_SW_EC_FRAGMENTED_MEMORY_MAP 0x00000014 + ``` + +### Code Changes + +1. Add macro definitions in `MdePkg/Include/Pi/PiStatusCode.h` to match new specification. -- 2.34.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85336): https://edk2.groups.io/g/devel/message/85336 Mute This Topic: https://groups.io/mt/88276263/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=- REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3794 This change introduces a new error code definitions under Host Software class. The new error code definition will cover system reboot events under the conditions of fragmented memory map from one boot to another. These error codes could provide helpful datapoints to OEMs to investigate and prevent system failures in general. Cc: Michael D Kinney <michael.d.kinney@intel.com> Cc: Liming Gao <gaoliming@byosoft.com.cn> Cc: Zhiguang Liu <zhiguang.liu@intel.com> Signed-off-by: Kun Qin <kuqin12@gmail.com> --- Notes: v2: - Removed "RELEASE_ASSERT" definition - Removed reference to EDK2 based memory type info [Mike] MdePkg/Include/Pi/PiStatusCode.h | 1 + 1 file changed, 1 insertion(+) diff --git a/MdePkg/Include/Pi/PiStatusCode.h b/MdePkg/Include/Pi/PiStatusCode.h index XXXXXXX..XXXXXXX 100644 --- a/MdePkg/Include/Pi/PiStatusCode.h +++ b/MdePkg/Include/Pi/PiStatusCode.h @@ -XXX,XX +XXX,XX @@ typedef struct { #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_FRAGMENTED_MEMORY_MAP 0x00000014 ///@} // -- 2.34.1.windows.1 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#85337): https://edk2.groups.io/g/devel/message/85337 Mute This Topic: https://groups.io/mt/88276264/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-