Re: [PATCH 10/10] MdeModulePkg/ResetSystemPei: Add reset notifications in PEI

["Zeng, Star" <star.zeng@intel.com>]

On 2018/2/7 20:40, Zeng, Star wrote:
> On 2018/2/2 14:45, Ruiyu Ni wrote:
>> From: Bret Barkelew <brbarkel@microsoft.com>
>>
>> The Reset Notification protocol is added in UEFI spec to support
>> reset notification mechanism in the DXE phase.
>> This patch adds similar EDKII specific Reset Notification PPI to PEI
>> phase to provide the same support.
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Bret Barkelew <Bret.Barkelew@microsoft.com>
>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> ---
>>   .../Ppi/PlatformSpecificResetNotification.h        | 31 
>> ++++++++++++++++++++++
>>   MdeModulePkg/MdeModulePkg.dec                      |  3 +++
>>   .../Universal/ResetSystemPei/ResetSystem.c         | 16 +++++++++++
>>   .../Universal/ResetSystemPei/ResetSystem.h         |  1 +
>>   .../Universal/ResetSystemPei/ResetSystemPei.inf    |  7 ++---
>>   5 files changed, 55 insertions(+), 3 deletions(-)
>>   create mode 100644 
>> MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
>>
>> diff --git 
>> a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h 
>> b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
>> new file mode 100644
>> index 0000000000..ea53e24133
>> --- /dev/null
>> +++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
>> @@ -0,0 +1,31 @@
>> +/** @file
>> +  This PPI provides services to register a platform specific 
>> notification callback for
>> +  ResetSystem().  The registered handlers are processed after
>> +  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications.
> 
> How about adding "and after EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI" 
> at the end of the sentence?
> 
> And also need enhance the comments in 
> MdeModulePkg\Include\Ppi\PlatformSpecificResetFilter.h to state it will 
> be processed before EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI 
> instead of EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI.
> 
> And also need enhance the comments in 
> MdeModulePkg\Include\Ppi\PlatformSpecificResetHandler.h to state it will 
> be processed after EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI 
> instead of EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI.
> 
> 
> Thanks,
> Star

Please also update the comments for PcdMaximumPeiResetNotifies in 
MdeModulePkg.dec and MdeModulePkg.uni accordingly.


Thanks,
Star

> 
>> +
>> +  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
>> +  Copyright (c) 2017 Microsoft Corporation. All rights reserved.<BR>
>> +
>> +  This program and the accompanying materials are licensed and made 
>> available under
>> +  the terms and conditions of the BSD License that accompanies this 
>> distribution.
>> +  The full text of the license may be found at
>> +  http://opensource.org/licenses/bsd-license.php.
>> +
>> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
>> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS 
>> OR IMPLIED.
>> +
>> +**/
>> +
>> +#ifndef _PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI_H_
>> +#define _PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI_H_
>> +
>> +#include <Protocol/ResetNotification.h>
>> +
>> +#define EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI_GUID \
>> +  { 0xe09f355d, 0xdae8, 0x4910, { 0xb1, 0x4a, 0x92, 0x78, 0x0f, 0xdc, 
>> 0xf7, 0xcb } }
>> +
>> +typedef EFI_RESET_NOTIFICATION_PROTOCOL  
>> EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI;
>> +
>> +extern EFI_GUID gEdkiiPlatformSpecificResetNotificationPpiGuid;
>> +
>> +#endif
>> diff --git a/MdeModulePkg/MdeModulePkg.dec 
>> b/MdeModulePkg/MdeModulePkg.dec
>> index 1b971d599f..297b02ffa9 100644
>> --- a/MdeModulePkg/MdeModulePkg.dec
>> +++ b/MdeModulePkg/MdeModulePkg.dec
>> @@ -444,6 +444,9 @@ [Ppis]
>>     ## Include/Ppi/PlatformSpecificResetFilter.h
>>     gEdkiiPlatformSpecificResetFilterPpiGuid = { 0x8c9f4de3, 0x7b90, 
>> 0x47ef, { 0x93, 0x8, 0x28, 0x7c, 0xec, 0xd6, 0x6d, 0xe8 } }
>> +  ## Include/Ppi/PlatformSpecificResetNotification.h
>> +  gEdkiiPlatformSpecificResetNotificationPpiGuid = { 0xe09f355d, 
>> 0xdae8, 0x4910, { 0xb1, 0x4a, 0x92, 0x78, 0xf, 0xdc, 0xf7, 0xcb } }
>> +
>>     ## Include/Ppi/PlatformSpecificResetHandler.h
>>     gEdkiiPlatformSpecificResetHandlerPpiGuid = { 0x75cf14ae, 0x3441, 
>> 0x49dc, { 0xaa, 0x10, 0xbb, 0x35, 0xa7, 0xba, 0x8b, 0xab } }
>> diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c 
>> b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
>> index 720593de6a..4dfe303f77 100644
>> --- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
>> +++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
>> @@ -25,6 +25,7 @@ EFI_PEI_RESET2_PPI mPpiReset2 = {
>>   EFI_GUID                *mProcessingOrder[] = {
>>     &gEdkiiPlatformSpecificResetFilterPpiGuid,
>> +  &gEdkiiPlatformSpecificResetNotificationPpiGuid,
>>     &gEdkiiPlatformSpecificResetHandlerPpiGuid
>>   };
>> @@ -36,6 +37,14 @@ RESET_FILTER_INSTANCE   mResetFilter = {
>>     &gEdkiiPlatformSpecificResetFilterPpiGuid
>>   };
>> +RESET_FILTER_INSTANCE   mResetNotification = {
>> +  {
>> +    RegisterResetNotify,
>> +    UnregisterResetNotify
>> +  },
>> +  &gEdkiiPlatformSpecificResetNotificationPpiGuid
>> +};
>> +
>>   RESET_FILTER_INSTANCE   mResetHandler = {
>>     {
>>       RegisterResetNotify,
>> @@ -55,6 +64,11 @@ EFI_PEI_PPI_DESCRIPTOR mPpiListReset[] = {
>>       &gEdkiiPlatformSpecificResetFilterPpiGuid,
>>       &mResetFilter.ResetFilter
>>     },
>> +  {
>> +    EFI_PEI_PPI_DESCRIPTOR_PPI,
>> +    &gEdkiiPlatformSpecificResetNotificationPpiGuid,
>> +    &mResetNotification.ResetFilter
>> +  },
>>     {
>>       EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
>>       &gEdkiiPlatformSpecificResetHandlerPpiGuid,
>> @@ -101,6 +115,7 @@ RegisterResetNotify (
>>     ResetFilter = (RESET_FILTER_INSTANCE *) This;
>>     ASSERT (CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetFilterPpiGuid) ||
>> +          CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetNotificationPpiGuid) ||
>>             CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetHandlerPpiGuid)
>>             );
>> @@ -187,6 +202,7 @@ UnregisterResetNotify (
>>     ResetFilter = (RESET_FILTER_INSTANCE *)This;
>>     ASSERT (CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetFilterPpiGuid) ||
>> +    CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetNotificationPpiGuid) ||
>>       CompareGuid (ResetFilter->Guid, 
>> &gEdkiiPlatformSpecificResetHandlerPpiGuid)
>>     );
>> diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h 
>> b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
>> index 2fcc3592b6..b623a4c381 100644
>> --- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
>> +++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
>> @@ -21,6 +21,7 @@
>>   #include <Ppi/Reset2.h>
>>   #include <Ppi/PlatformSpecificResetFilter.h>
>> +#include <Ppi/PlatformSpecificResetNotification.h>
>>   #include <Ppi/PlatformSpecificResetHandler.h>
>>   #include <Library/BaseLib.h>
>> diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf 
>> b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
>> index 38fdd16ceb..a88e2018b7 100644
>> --- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
>> +++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
>> @@ -48,9 +48,10 @@ [LibraryClasses]
>>     ReportStatusCodeLib
>>   [Ppis]
>> -  gEfiPeiReset2PpiGuid                       ## PRODUCES
>> -  gEdkiiPlatformSpecificResetFilterPpiGuid   ## PRODUCES
>> -  gEdkiiPlatformSpecificResetHandlerPpiGuid  ## PRODUCES
>> +  gEfiPeiReset2PpiGuid                           ## PRODUCES
>> +  gEdkiiPlatformSpecificResetFilterPpiGuid       ## PRODUCES
>> +  gEdkiiPlatformSpecificResetHandlerPpiGuid      ## PRODUCES
>> +  gEdkiiPlatformSpecificResetNotificationPpiGuid ## PRODUCES
>>   [Pcd]
>>     gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies
>>