From: "Michael D Kinney" <michael.d.kinney@intel.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>,
"kuqin12@gmail.com" <kuqin12@gmail.com>,
"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
Date: Fri, 7 Jan 2022 02:09:52 +0000 [thread overview]
Message-ID: <CO1PR11MB4929A67A09886B70327B9DCED24D9@CO1PR11MB4929.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20220107010306.1253-2-kuqin12@gmail.com>
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
>
>
>
>
>
next prev parent reply other threads:[~2022-01-07 2:09 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-07 1:03 [PATCH v1 0/2] EDK2 Code First: PI Specification: Update EFI_MM_COMMUNICATE_HEADER Kun Qin
2022-01-07 1:03 ` [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class Kun Qin
2022-01-07 2:09 ` Michael D Kinney [this message]
2022-01-07 20:09 ` [edk2-devel] " Kun Qin
2022-01-07 20:45 ` Michael D Kinney
2022-01-07 21:56 ` Kun Qin
2022-01-07 1:03 ` [PATCH v1 2/2] MdePkg: MmCommunication: Add new Host Software class Error Codes to MdePkg Kun Qin
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CO1PR11MB4929A67A09886B70327B9DCED24D9@CO1PR11MB4929.namprd11.prod.outlook.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox