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

[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@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 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

[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@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>
---
 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 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@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/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

Re: [edk2-devel] [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@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> 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@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>
> ---
>  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
> 
> 
> 
> 
> 

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

[Kun Qin]

Hi Mike,

Thanks for the input.

Is it better to rename "EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE" as 
"EFI_SW_EC_INCONSISTENT_MEM_MAP"? This name should be more generic yet 
still describing what OSes may be expecting.

As per RELEASE_ASSERT, I agree that this definition is implementation 
specific and such unrecoverable errors are already covered by 
"EFI_SW_EC_ILLEGAL_SOFTWARE_STATE". So I will drop the addition of 
"RELEASE_ASSERT" in the next update.

Please let me know if you have other comments or concerns, otherwise I 
will send out v2 update shortly.

Thanks,
Kun

On 01/06/2022 18:09, Michael D Kinney wrote:
> 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@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming
>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>> 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@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>
>> ---
>>   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
>>
>>
>>
>>
>>
> 
> 
> 
> 
> 
> 

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

[Michael D Kinney]

Perhaps the term fragmented would be better than inconsistent for this condition?

EFI_SW_EC_FRAGMENTED_MEMORY_MAP

The term inconsistent could be used to describe other issues with the memory map.
If we think there may be other memory map conditions that we want to cover with
the same status code, then I am fine with INCONSISTENT.

Mike

> -----Original Message-----
> From: Kun Qin <kuqin12@gmail.com>
> Sent: Friday, January 7, 2022 12:10 PM
> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
> <zhiguang.liu@intel.com>
> Subject: Re: [edk2-devel] [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class
> 
> Hi Mike,
> 
> Thanks for the input.
> 
> Is it better to rename "EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE" as
> "EFI_SW_EC_INCONSISTENT_MEM_MAP"? This name should be more generic yet
> still describing what OSes may be expecting.
> 
> As per RELEASE_ASSERT, I agree that this definition is implementation
> specific and such unrecoverable errors are already covered by
> "EFI_SW_EC_ILLEGAL_SOFTWARE_STATE". So I will drop the addition of
> "RELEASE_ASSERT" in the next update.
> 
> Please let me know if you have other comments or concerns, otherwise I
> will send out v2 update shortly.
> 
> Thanks,
> Kun
> 
> On 01/06/2022 18:09, Michael D Kinney wrote:
> > 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@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming
> >> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
> >> 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@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>
> >> ---
> >>   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
> >>
> >>
> >>
> >>
> >>
> >
> >
> >
> > 
> >
> >

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

[Kun Qin]

Hi Mike,

Thanks for the suggestion. I am good with the new name and will send out 
the updated branch shortly.

Regards,
Kun

On 01/07/2022 12:45, Kinney, Michael D wrote:
> Perhaps the term fragmented would be better than inconsistent for this condition?
> 
> EFI_SW_EC_FRAGMENTED_MEMORY_MAP
> 
> The term inconsistent could be used to describe other issues with the memory map.
> If we think there may be other memory map conditions that we want to cover with
> the same status code, then I am fine with INCONSISTENT.
> 
> Mike
> 
>> -----Original Message-----
>> From: Kun Qin <kuqin12@gmail.com>
>> Sent: Friday, January 7, 2022 12:10 PM
>> To: devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
>> Cc: Andrew Fish <afish@apple.com>; Leif Lindholm <leif@nuviainc.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Liu, Zhiguang
>> <zhiguang.liu@intel.com>
>> Subject: Re: [edk2-devel] [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class
>>
>> Hi Mike,
>>
>> Thanks for the input.
>>
>> Is it better to rename "EFI_SW_EC_MEMORY_TYPE_INFORMATION_CHANGE" as
>> "EFI_SW_EC_INCONSISTENT_MEM_MAP"? This name should be more generic yet
>> still describing what OSes may be expecting.
>>
>> As per RELEASE_ASSERT, I agree that this definition is implementation
>> specific and such unrecoverable errors are already covered by
>> "EFI_SW_EC_ILLEGAL_SOFTWARE_STATE". So I will drop the addition of
>> "RELEASE_ASSERT" in the next update.
>>
>> Please let me know if you have other comments or concerns, otherwise I
>> will send out v2 update shortly.
>>
>> Thanks,
>> Kun
>>
>> On 01/06/2022 18:09, Michael D Kinney wrote:
>>> 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@apple.com>; Leif Lindholm <leif@nuviainc.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
>> Liming
>>>> <gaoliming@byosoft.com.cn>; Liu, Zhiguang <zhiguang.liu@intel.com>
>>>> 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@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>
>>>> ---
>>>>    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
>>>>
>>>>
>>>>
>>>>
>>>>
>>>
>>>
>>>
>>> 
>>>
>>>