From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) by mx.groups.io with SMTP id smtpd.web12.11867.1641586198056250774 for ; Fri, 07 Jan 2022 12:09:58 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@gmail.com header.s=20210112 header.b=F1yGQwOR; spf=pass (domain: gmail.com, ip: 209.85.214.178, mailfrom: kuqin12@gmail.com) Received: by mail-pl1-f178.google.com with SMTP id l15so5658103pls.7 for ; Fri, 07 Jan 2022 12:09:58 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=message-id:date:mime-version:user-agent:subject:content-language:to :cc:references:from:in-reply-to:content-transfer-encoding; bh=IE6jgzEqOM/LyEyGVfeQlwmBTcbk166Uzyhd/O4mnb8=; b=F1yGQwOR9QxQE4sHSd0N6br9iNg6ubkRUKA26r2VWlzMUIECypYI+aJUNvPQTx6N7X ahcRZEf1iFDOKr4w6khsjkBuKOs+SsS/ZjAsqp/PeRbMhZ56D9OcBw0xfy95N6DKvJTm k01vzuztXM+bha0kyJEGnKxzwg7xBAx5UjgXgFATsNlwGzlRA6V4jltC/5uKF2WlMJ4O Xdr2SILDvK5AX5jPvNAlIUEEgLIgaTsZM+lX6w0Egbu5Mw3ZogQoO+5XtqtOAIgZQknM KqVebhXIdYhiHmXPHjuTlbiOJfOit8poyl+7AmuCGTCI83w3gh5wYH7bGIkguCjbPCAP CzYw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=IE6jgzEqOM/LyEyGVfeQlwmBTcbk166Uzyhd/O4mnb8=; b=ra3hzF1E2JGjzhUcy5Cxv46JO3Mc1S9ulAC7i18mC8bpiHecrDGd4FltCDcjUAWCve XnGjfX8EsRNOKNAKEvPlBjCdqLGsIbOs+slAA2dvMOxWxxOFZeXHPe0+FR206YPQJD1P 6+6ZPERuKVFdNHwNw/JzXMS7Gi32xlCL9NwSoVct7rjxkn4SGnWH80mQSQi/fSgXiLEl K+D1hm+up1rTEwPMHzexPcVsgaiKFnfXIHeCiyN/UdbrOpfTkAUDHy6hY6qO1oPZr3n5 C27s2rXgcoorATDBB8F94FQ9SciqU7oQHJixQzzreTe7OwlaHnvho0qI8DRs1d7pgabk u0LQ== X-Gm-Message-State: AOAM531PSM+ZinBsQwEB/mTmk9UotiiY6iA5p8gnfF0lA32kjssKYpMG 0TUarOWFvYUl9V2UuDRH+BeRgAP9nHo= X-Google-Smtp-Source: ABdhPJyhQB6PNUPgu5flOuJpWYKvwcin0hjN3P1t8Hes5ab9lSREPvAI61OVjOU4De+rqnOtI/X2ig== X-Received: by 2002:a17:90b:8d:: with SMTP id bb13mr17313927pjb.156.1641586197376; Fri, 07 Jan 2022 12:09:57 -0800 (PST) Return-Path: Received: from [192.168.1.18] ([50.35.74.198]) by smtp.gmail.com with ESMTPSA id h5sm6321631pfi.46.2022.01.07.12.09.56 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Fri, 07 Jan 2022 12:09:57 -0800 (PST) Message-ID: <015bc526-1f7f-ccfc-13f9-1b3996337c62@gmail.com> Date: Fri, 7 Jan 2022 12:09:56 -0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101 Thunderbird/91.4.1 Subject: Re: [edk2-devel] [PATCH v1 1/2] EDK2 Code First: PI Specification: New error codes of Host Software class To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Andrew Fish , Leif Lindholm , "Gao, Liming" , "Liu, Zhiguang" References: <20220107010306.1253-1-kuqin12@gmail.com> <20220107010306.1253-2-kuqin12@gmail.com> From: "Kun Qin" In-Reply-To: Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit 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 On Behalf Of Kun Qin >> Sent: Thursday, January 6, 2022 5:03 PM >> To: devel@edk2.groups.io >> Cc: Andrew Fish ; Leif Lindholm ; Kinney, Michael D ; Gao, Liming >> ; Liu, Zhiguang >> 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 >> Cc: Leif Lindholm >> Cc: Michael D Kinney >> Cc: Liming Gao >> Cc: Zhiguang Liu >> >> Signed-off-by: Kun Qin >> --- >> 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 >> >> >> >> >> > > > > > >