public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v2 00/10] Formalize the reset system core design
@ 2018-02-09  4:16 Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 01/10] MdePkg/PeiServicesLib: Add PeiServicesResetSystem2() Ruiyu Ni
                   ` (10 more replies)
  0 siblings, 11 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel

The patches add/update two core modules that perform the reset action
  ResetSystemPei and ResetSystemRuntimeDxe

With the two core modules, every time a reset action is performed in
either PEI phase or DXE phase, the accordingly registerred
reset filter/notification/handler will be triggered.

Reset filters are processed first so the final reset type and reset
data can be determined.  Reset Notifications are processed second
so all components that have registered for a Reset Notification can
perform any required clean up actions. Reset handlers are processed
third.  If there are no registered reset handlers or none of them
resets the platform, then the default reset action based on the
ResetSystemLib is performed.

The v2 changes against v2 are attached in the end of this mail. 

Bret Barkelew (1):
  MdeModulePkg/ResetSystemPei: Add reset notifications in PEI

Michael D Kinney (6):
  MdePkg/PeiServicesLib: Add PeiServicesResetSystem2()
  MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first
  MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c
  MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler
  MdeModulePkg: Add ResetSystemLib instances that call core services
  MdeModulePkg: Add ResetUtility librray class and BASE instance

Ruiyu Ni (3):
  MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  MdePkg/UefiRuntimeLib: Support more module types.
  MdeModulePkg: Add ResetSystemPei PEIM

 MdeModulePkg/Core/Pei/Reset/Reset.c                |  67 ++--
 MdeModulePkg/Include/Library/ResetUtilityLib.h     | 111 ++++++
 .../Include/Ppi/PlatformSpecificResetFilter.h      |  31 ++
 .../Include/Ppi/PlatformSpecificResetHandler.h     |  29 ++
 .../Ppi/PlatformSpecificResetNotification.h        |  32 ++
 .../Include/Protocol/PlatformSpecificResetFilter.h |  31 ++
 .../Protocol/PlatformSpecificResetHandler.h        |  29 ++
 .../Library/DxeResetSystemLib/DxeResetSystemLib.c  |  98 ++++++
 .../DxeResetSystemLib/DxeResetSystemLib.inf        |  39 +++
 .../DxeResetSystemLib/DxeResetSystemLib.uni        |  21 ++
 .../Library/PeiResetSystemLib/PeiResetSystemLib.c  |  98 ++++++
 .../PeiResetSystemLib/PeiResetSystemLib.inf        |  39 +++
 .../PeiResetSystemLib/PeiResetSystemLib.uni        |  21 ++
 .../Library/ResetUtilityLib/ResetUtility.c         | 220 ++++++++++++
 .../Library/ResetUtilityLib/ResetUtilityLib.inf    |  40 +++
 MdeModulePkg/MdeModulePkg.dec                      |  20 ++
 MdeModulePkg/MdeModulePkg.dsc                      |   7 +
 MdeModulePkg/MdeModulePkg.uni                      |   7 +-
 .../Universal/ResetSystemPei/ResetSystem.c         | 371 +++++++++++++++++++++
 .../Universal/ResetSystemPei/ResetSystem.h         | 130 ++++++++
 .../ResetSystemPei.inf}                            |  45 ++-
 .../Universal/ResetSystemPei/ResetSystemPei.uni    |  20 ++
 .../ResetSystemPei/ResetSystemPeiExtra.uni         |  20 ++
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  |  91 ++++-
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.h  |   7 +
 .../ResetSystemRuntimeDxe.inf                      |   7 +-
 MdePkg/Include/Library/PeiServicesLib.h            |  24 ++
 MdePkg/Library/PeiServicesLib/PeiServicesLib.c     |  26 ++
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf   |   4 +-
 29 files changed, 1615 insertions(+), 70 deletions(-)
 create mode 100644 MdeModulePkg/Include/Library/ResetUtilityLib.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
 copy MdeModulePkg/Universal/{ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf => ResetSystemPei/ResetSystemPei.inf} (50%)
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h

index 0f1432f5f8..1f728387b7 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
@@ -3,9 +3,9 @@
   for ResetSystem().  A reset filter evaluates the parameters passed to
   ResetSystem() and converts a ResetType of EfiResetPlatformSpecific to a
   non-platform specific reset type.  The registered filters are processed before
-  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI handlers.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI handlers.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
index d5f1350c69..8d938abb02 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
@@ -1,9 +1,9 @@
 /** @file
   This PPI provides services to register a platform specific handler for
   ResetSystem().  The registered handlers are processed after
-  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI notifications.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
index ea53e24133..cf6d1f18a6 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
@@ -1,9 +1,10 @@
 /** @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.
+  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications and before
+  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI notifications.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 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
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
index 70ee1175d5..ea452e3231 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
@@ -1,7 +1,7 @@
 /** @file
   DXE Reset System Library instance that calls gRT->ResetSystem().
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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
   which accompanies this distribution.  The full text of the license may be found at
@@ -13,9 +13,7 @@
 **/
 
 #include <PiDxe.h>
-
 #include <Library/ResetSystemLib.h>
-#include <Library/DebugLib.h>
 #include <Library/UefiRuntimeLib.h>
 
 /**
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
index f2e04cfd00..6eb2766b93 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  DXE Reset System Library instance that calls gRT->ResetSystem().
 #
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel 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
@@ -35,6 +35,5 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  DebugLib
   UefiRuntimeLib
 
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
index b7e10110b0..30225d73a5 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
@@ -1,7 +1,7 @@
 /** @file
   PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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
   which accompanies this distribution.  The full text of the license may be found at
@@ -13,9 +13,7 @@
 **/
 
 #include <PiPei.h>
-
 #include <Library/ResetSystemLib.h>
-#include <Library/DebugLib.h>
 #include <Library/PeiServicesLib.h>
 
 /**
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
index e82ec6b2b6..b1b9388c63 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
 #
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel 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
@@ -35,6 +35,5 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  DebugLib
   PeiServicesLib
 
diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
index 5bbe002be0..90de94ca9b 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -37,7 +37,6 @@ typedef struct {
         are not initialized. For DXE, you can add gEfiResetArchProtocolGuid
         to your DEPEX.
 
-  @param[in]  ResetType     Base reset type as defined in UEFI spec.
   @param[in]  ResetSubtype  GUID pointer for the reset subtype to be used.
 
 **/
@@ -90,7 +89,7 @@ GetResetPlatformSpecificGuid (
   }
 
   //
-  // Determine the number of bytes in in the Null-terminated Unicode string
+  // Determine the number of bytes in the Null-terminated Unicode string
   // at the beginning of ResetData including the Null terminator.
   //
   ResetDataStringSize = StrnSizeS (ResetData, (DataSize / sizeof (CHAR16)));
diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
index 7a403749c5..2a4e53a8a6 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
@@ -1,10 +1,7 @@
 ## @file
 # This file contains the Reset Utility functions.
 #
-#  The application pops up a menu showing all the boot options referenced by
-#  BootOrder NV variable and user can choose to boot from one of them.
-#  
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
 #  Copyright (c) 2016, 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
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 297b02ffa9..0695ce607e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -54,6 +54,9 @@ [LibraryClasses]
   ##  @libraryclass  Defines a set of methods to reset whole system.
   ResetSystemLib|Include/Library/ResetSystemLib.h
 
+  ##  @libraryclass  Defines a set of helper functions for resetting the system.
+  ResetUtilityLib|Include/Library/ResetUtilityLib.h
+
   ##  @libraryclass  Defines a set of methods related do S3 mode.
   #   This library class is no longer used and modules using this library should
   #   directly locate EFI_PEI_S3_RESUME_PPI defined in PI 1.2 specification.
@@ -1422,8 +1425,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt CapsuleMax value in capsule report variable.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax|0xFFFF|UINT16|0x00000107
 
-  ## Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase.
-  # @Prompt Maximum Number of PEI Reset Filters or Reset Handlers.
+  ## Indicates the allowable maximum number of Reset Filters, Reset Notifications or Reset Handlers in PEI phase.
+  # @Prompt Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies|0x10|UINT32|0x00000109
 
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 0e068422e4..d8cc7416c5 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -4,7 +4,7 @@
 // It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes)
 // and libraries instances, which are used for those modules.
 //
-// Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2007 - 2018, Intel 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.
@@ -1059,6 +1059,11 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleMax_HELP  #language en-US "CapsuleMax value in capsule report variable."
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_PROMPT  #language en-US "Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_HELP  #language en-US "Indicates the allowable maximum number of Reset Filters, <BR>\n"
+                                                                                            "Reset Notifications or Reset Handlers in PEI phase."
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_PROMPT  #language en-US "Recover file name in PEI phase"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_HELP  #language en-US "This is recover file name in PEI phase.\n"
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
index 4dfe303f77..c2ee592d56 100644
--- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
@@ -80,13 +80,13 @@ EFI_PEI_PPI_DESCRIPTOR mPpiListReset[] = {
   Register a notification function to be called when ResetSystem() is called.
 
   The RegisterResetNotify() function registers a notification function that is called when
-  ResetSystem()is called and prior to completing the reset of the platform.
+  ResetSystem() is called and prior to completing the reset of the platform.
   The registered functions must not perform a platform reset themselves. These
   notifications are intended only for the notification of components which may need some
   special-purpose maintenance prior to the platform resetting.
   The list of registered reset notification functions are processed if ResetSystem()is called
   before ExitBootServices(). The list of registered reset notification functions is ignored if
-  ResetSystem()is called after ExitBootServices().
+  ResetSystem() is called after ExitBootServices().
 
   @param[in]  This              A pointer to the EFI_RESET_NOTIFICATION_PROTOCOL instance.
   @param[in]  ResetFunction     Points to the function to be called when a ResetSystem() is executed.
@@ -312,7 +312,7 @@ ResetSystem2 (
     //
     // Indicate reset system runtime service is called.
     //
-    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_EFI_RUNTIME_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
+    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_PEI_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
   }
 
   //
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
index b623a4c381..8e9f872056 100644
--- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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
@@ -34,7 +34,7 @@
 
 
 //
-// The maximum recurstion depth to ResetSystem() by reset notification handlers
+// The maximum recursion depth to ResetSystem() by reset notification handlers
 //
 #define MAX_RESET_NOTIFY_DEPTH 10
 
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index 4b5af76999..2c795426f5 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -294,7 +294,7 @@ ResetSystem (
       }
       //
       // call reset notification functions registered through the 
-      // EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PROTOCOL.
+      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
       //
       for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
           ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)

-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 01/10] MdePkg/PeiServicesLib: Add PeiServicesResetSystem2()
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 02/10] MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first Ruiyu Ni
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao

From: Michael D Kinney <michael.d.kinney@intel.com>

Add the PeiServicesResetSytstem2() function to the PeiServiesLib
to call the ResetSystem2() services in the PEI Services Table.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 MdePkg/Include/Library/PeiServicesLib.h        | 24 ++++++++++++++++++++++++
 MdePkg/Library/PeiServicesLib/PeiServicesLib.c | 26 ++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/MdePkg/Include/Library/PeiServicesLib.h b/MdePkg/Include/Library/PeiServicesLib.h
index 9fc22a10c1..0be72237f2 100644
--- a/MdePkg/Include/Library/PeiServicesLib.h
+++ b/MdePkg/Include/Library/PeiServicesLib.h
@@ -540,4 +540,28 @@ PeiServicesInstallFvInfo2Ppi (
   IN       UINT32                  AuthenticationStatus
   );
 
+/**
+  Resets the entire platform.
+
+  @param[in] ResetType      The type of reset to perform.
+  @param[in] ResetStatus    The status code for the reset.
+  @param[in] DataSize       The size, in bytes, of ResetData.
+  @param[in] ResetData      For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown
+                            the data buffer starts with a Null-terminated string, optionally
+                            followed by additional binary data. The string is a description
+                            that the caller may use to further indicate the reason for the
+                            system reset. ResetData is only valid if ResetStatus is something
+                            other than EFI_SUCCESS unless the ResetType is EfiResetPlatformSpecific
+                            where a minimum amount of ResetData is always required.
+
+**/
+VOID
+EFIAPI
+PeiServicesResetSystem2 (
+  IN EFI_RESET_TYPE     ResetType,
+  IN EFI_STATUS         ResetStatus,
+  IN UINTN              DataSize,
+  IN VOID               *ResetData OPTIONAL
+  );
+
 #endif
diff --git a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
index 89166ccd38..d0838ed709 100644
--- a/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
+++ b/MdePkg/Library/PeiServicesLib/PeiServicesLib.c
@@ -789,3 +789,29 @@ PeiServicesInstallFvInfo2Ppi (
   InternalPeiServicesInstallFvInfoPpi (FALSE, FvFormat, FvInfo, FvInfoSize, ParentFvName, ParentFileName, AuthenticationStatus);
 }
 
+/**
+  Resets the entire platform.
+
+  @param[in] ResetType      The type of reset to perform.
+  @param[in] ResetStatus    The status code for the reset.
+  @param[in] DataSize       The size, in bytes, of ResetData.
+  @param[in] ResetData      For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown
+                            the data buffer starts with a Null-terminated string, optionally
+                            followed by additional binary data. The string is a description
+                            that the caller may use to further indicate the reason for the
+                            system reset. ResetData is only valid if ResetStatus is something
+                            other than EFI_SUCCESS unless the ResetType is EfiResetPlatformSpecific
+                            where a minimum amount of ResetData is always required.
+
+**/
+VOID
+EFIAPI
+PeiServicesResetSystem2 (
+  IN EFI_RESET_TYPE     ResetType,
+  IN EFI_STATUS         ResetStatus,
+  IN UINTN              DataSize,
+  IN VOID               *ResetData OPTIONAL
+  )
+{
+  (*GetPeiServicesTablePointer())->ResetSystem2 (ResetType, ResetStatus, DataSize, ResetData);
+}
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 02/10] MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 01/10] MdePkg/PeiServicesLib: Add PeiServicesResetSystem2() Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 03/10] MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c Ruiyu Ni
                   ` (8 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao, Star Zeng

From: Michael D Kinney <michael.d.kinney@intel.com>

Update PEI Service ResetSystem() to always attempt to use
the Reset2 PPI before looking for the Reset PPI.

Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Pei/Reset/Reset.c | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Reset/Reset.c b/MdeModulePkg/Core/Pei/Reset/Reset.c
index 7440eefd78..cd36c526b5 100644
--- a/MdeModulePkg/Core/Pei/Reset/Reset.c
+++ b/MdeModulePkg/Core/Pei/Reset/Reset.c
@@ -35,16 +35,21 @@ PeiResetSystem (
   EFI_STATUS        Status;
   EFI_PEI_RESET_PPI *ResetPpi;
 
-  Status = PeiServicesLocatePpi (
-             &gEfiPeiResetPpiGuid,         
-             0,                         
-             NULL,                      
-             (VOID **)&ResetPpi                  
-             );
+  //
+  // Attempt to use newer ResetSystem2().  If this returns, then ResetSystem2()
+  // is not available.
+  //
+  PeiResetSystem2 (EfiResetCold, EFI_SUCCESS, 0, NULL);
 
   //
-  // LocatePpi returns EFI_NOT_FOUND on error
+  // Look for PEI Reset System PPI
   //
+  Status = PeiServicesLocatePpi (
+             &gEfiPeiResetPpiGuid,
+             0,
+             NULL,
+             (VOID **)&ResetPpi
+             );
   if (!EFI_ERROR (Status)) {
     return ResetPpi->ResetSystem (PeiServices);
   } 
@@ -55,6 +60,10 @@ PeiResetSystem (
     EFI_ERROR_CODE | EFI_ERROR_MINOR,
     (EFI_SOFTWARE_PEI_CORE | EFI_SW_PS_EC_RESET_NOT_AVAILABLE)
     );
+
+  //
+  // No reset PPIs are available yet.
+  //
   return  EFI_NOT_AVAILABLE_YET;
 }
 
@@ -85,6 +94,9 @@ PeiResetSystem2 (
   EFI_STATUS            Status;
   EFI_PEI_RESET2_PPI    *Reset2Ppi;
 
+  //
+  // Look for PEI Reset System 2 PPI
+  //
   Status = PeiServicesLocatePpi (
              &gEfiPeiReset2PpiGuid,
              0,
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 03/10] MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 01/10] MdePkg/PeiServicesLib: Add PeiServicesResetSystem2() Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 02/10] MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 04/10] MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler Ruiyu Ni
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao, Star Zeng

From: Michael D Kinney <michael.d.kinney@intel.com>

Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/Pei/Reset/Reset.c | 41 ++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/MdeModulePkg/Core/Pei/Reset/Reset.c b/MdeModulePkg/Core/Pei/Reset/Reset.c
index cd36c526b5..e6d7899ef7 100644
--- a/MdeModulePkg/Core/Pei/Reset/Reset.c
+++ b/MdeModulePkg/Core/Pei/Reset/Reset.c
@@ -1,14 +1,14 @@
 /** @file
   Pei Core Reset System Support
-  
+
 Copyright (c) 2006 - 2017, Intel 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         
-which 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.             
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which 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.
 
 **/
 
@@ -29,11 +29,11 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
 EFI_STATUS
 EFIAPI
 PeiResetSystem (
-  IN CONST EFI_PEI_SERVICES         **PeiServices
+  IN CONST EFI_PEI_SERVICES  **PeiServices
   )
 {
-  EFI_STATUS        Status;
-  EFI_PEI_RESET_PPI *ResetPpi;
+  EFI_STATUS         Status;
+  EFI_PEI_RESET_PPI  *ResetPpi;
 
   //
   // Attempt to use newer ResetSystem2().  If this returns, then ResetSystem2()
@@ -52,9 +52,10 @@ PeiResetSystem (
              );
   if (!EFI_ERROR (Status)) {
     return ResetPpi->ResetSystem (PeiServices);
-  } 
+  }
+
   //
-  // Report Status Code that Reset PPI is not available
+  // Report Status Code that Reset PPI is not available.
   //
   REPORT_STATUS_CODE (
     EFI_ERROR_CODE | EFI_ERROR_MINOR,
@@ -85,14 +86,14 @@ PeiResetSystem (
 VOID
 EFIAPI
 PeiResetSystem2 (
-  IN EFI_RESET_TYPE     ResetType,
-  IN EFI_STATUS         ResetStatus,
-  IN UINTN              DataSize,
-  IN VOID               *ResetData OPTIONAL
+  IN EFI_RESET_TYPE  ResetType,
+  IN EFI_STATUS      ResetStatus,
+  IN UINTN           DataSize,
+  IN VOID            *ResetData OPTIONAL
   )
 {
-  EFI_STATUS            Status;
-  EFI_PEI_RESET2_PPI    *Reset2Ppi;
+  EFI_STATUS          Status;
+  EFI_PEI_RESET2_PPI  *Reset2Ppi;
 
   //
   // Look for PEI Reset System 2 PPI
@@ -103,7 +104,6 @@ PeiResetSystem2 (
              NULL,
              (VOID **)&Reset2Ppi
              );
-
   if (!EFI_ERROR (Status)) {
     Reset2Ppi->ResetSystem (ResetType, ResetStatus, DataSize, ResetData);
     return;
@@ -117,4 +117,3 @@ PeiResetSystem2 (
     (EFI_SOFTWARE_PEI_CORE | EFI_SW_PS_EC_RESET_NOT_AVAILABLE)
     );
 }
-
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 04/10] MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (2 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 03/10] MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message Ruiyu Ni
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao

From: Michael D Kinney <michael.d.kinney@intel.com>

Add support for platform specific reset filters and platform
specific reset handlers to ResetSystem().  A filter may modify
the reset type and reset data and call ResetSystem() with the
modified parameters.  A handler performs the reset action.

The support for platform specific filters and platform specific
handlers is based on the Reset Notification feature added to the
UEFI 2.7 Specification.

Platform specific reset filters are processed first so the final
reset type and reset data can be determined.  In the DXE Phase
The UEFI Reset Notifications are processed second so all UEFI
Drivers that have registered for a Reset Notification can perform
any required clean up actions.  The platform specific reset
handlers are processed third.  If there are no registered
platform specific reset handlers or none of them reset the
platform, then the default reset action based on the
ResetSystemLib is performed.

In the PEI Phase, filters and handlers are registered through
the following 2 PPIs that are based on
EFI_RESET_NOTIFICATION_PROTOCOL.
* gEdkiiPlatformSpecificResetFilterPpiGuid
* gEdkiiPlatformSpecificResetHandlerPpiGuid

In the DXE Phase, filters and handlers are registered through
the following 2 Protocols that are based on
EFI_RESET_NOTIFICATION_PROTOCOL.
* gEdkiiPlatformSpecificResetFilterProtocolGuid
* gEdkiiPlatformSpecificResetHandlerProtocolGuid

Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../Include/Ppi/PlatformSpecificResetFilter.h      | 31 +++++++++
 .../Include/Ppi/PlatformSpecificResetHandler.h     | 29 +++++++++
 .../Include/Protocol/PlatformSpecificResetFilter.h | 31 +++++++++
 .../Protocol/PlatformSpecificResetHandler.h        | 29 +++++++++
 MdeModulePkg/MdeModulePkg.dec                      | 10 +++
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 75 ++++++++++++++++++++--
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.h  |  7 ++
 .../ResetSystemRuntimeDxe.inf                      |  7 +-
 8 files changed, 212 insertions(+), 7 deletions(-)
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h

diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
new file mode 100644
index 0000000000..0f1432f5f8
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
@@ -0,0 +1,31 @@
+/** @file
+  This PPI provides services to register a platform specific reset filter
+  for ResetSystem().  A reset filter evaluates the parameters passed to
+  ResetSystem() and converts a ResetType of EfiResetPlatformSpecific to a
+  non-platform specific reset type.  The registered filters are processed before
+  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI handlers.
+
+  Copyright (c) 2017 Intel 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_FILTER_PPI_H_
+#define _PLATFORM_SPECIFIC_RESET_FILTER_PPI_H_
+
+#include <Protocol/ResetNotification.h>
+
+#define EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI_GUID \
+  { 0x8c9f4de3, 0x7b90, 0x47ef, { 0x93, 0x8, 0x28, 0x7c, 0xec, 0xd6, 0x6d, 0xe8 } }
+
+typedef EFI_RESET_NOTIFICATION_PROTOCOL  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI;
+
+extern EFI_GUID gEdkiiPlatformSpecificResetFilterPpiGuid;
+
+#endif
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
new file mode 100644
index 0000000000..d5f1350c69
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
@@ -0,0 +1,29 @@
+/** @file
+  This PPI provides services to register a platform specific handler for
+  ResetSystem().  The registered handlers are processed after
+  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications.
+
+  Copyright (c) 2017 Intel 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_HANDLER_PPI_H_
+#define _PLATFORM_SPECIFIC_RESET_HANDLER_PPI_H_
+
+#include <Protocol/ResetNotification.h>
+
+#define EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI_GUID \
+  { 0x75cf14ae, 0x3441, 0x49dc, { 0xaa, 0x10, 0xbb, 0x35, 0xa7, 0xba, 0x8b, 0xab } }
+
+typedef EFI_RESET_NOTIFICATION_PROTOCOL  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI;
+
+extern EFI_GUID gEdkiiPlatformSpecificResetHandlerPpiGuid;
+
+#endif
diff --git a/MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h b/MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h
new file mode 100644
index 0000000000..ff5ca48fdd
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h
@@ -0,0 +1,31 @@
+/** @file
+  This Protocol provides services to register a platform specific reset filter
+  for ResetSystem().  A reset filter evaluates the parameters passed to
+  ResetSystem() and converts a ResetType of EfiResetPlatformSpecific to a
+  non-platform specific reset type.  The registered filters are processed before
+  the UEFI 2.7 Reset Notifications.
+
+  Copyright (c) 2017 Intel 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_FILTER_PROTOCOL_H_
+#define _PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL_H_
+
+#include <Protocol/ResetNotification.h>
+
+#define EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL_GUID \
+  { 0x695d7835, 0x8d47, 0x4c11, { 0xab, 0x22, 0xfa, 0x8a, 0xcc, 0xe7, 0xae, 0x7a } }
+
+typedef EFI_RESET_NOTIFICATION_PROTOCOL  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL;
+
+extern EFI_GUID gEdkiiPlatformSpecificResetFilterProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h b/MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h
new file mode 100644
index 0000000000..8a44e860b2
--- /dev/null
+++ b/MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h
@@ -0,0 +1,29 @@
+/** @file
+  This protocol provides services to register a platform specific handler for
+  ResetSystem().  The registered handlers are called after the UEFI 2.7 Reset
+  Notifications are processed
+
+  Copyright (c) 2017 Intel 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_HANDLER_PROTOCOL_H_
+#define _PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL_H_
+
+#include <Protocol/ResetNotification.h>
+
+#define EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL_GUID \
+  { 0x2df6ba0b, 0x7092, 0x440d, { 0xbd, 0x4, 0xfb, 0x9, 0x1e, 0xc3, 0xf3, 0xc1 } }
+
+typedef EFI_RESET_NOTIFICATION_PROTOCOL  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL;
+
+extern EFI_GUID gEdkiiPlatformSpecificResetHandlerProtocolGuid;
+
+#endif
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 61d034fba8..1cc9bc8ea1 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -441,6 +441,12 @@ [Ppis]
   ## Include/Ppi/IoMmu.h
   gEdkiiIoMmuPpiGuid = { 0x70b0af26, 0xf847, 0x4bb6, { 0xaa, 0xb9, 0xcd, 0xe8, 0x4f, 0xc6, 0x14, 0x31 } }
 
+  ## Include/Ppi/PlatformSpecificResetFilter.h
+  gEdkiiPlatformSpecificResetFilterPpiGuid = { 0x8c9f4de3, 0x7b90, 0x47ef, { 0x93, 0x8, 0x28, 0x7c, 0xec, 0xd6, 0x6d, 0xe8 } }
+
+  ## Include/Ppi/PlatformSpecificResetHandler.h
+  gEdkiiPlatformSpecificResetHandlerPpiGuid = { 0x75cf14ae, 0x3441, 0x49dc, { 0xaa, 0x10, 0xbb, 0x35, 0xa7, 0xba, 0x8b, 0xab } }
+
 [Protocols]
   ## Load File protocol provides capability to load and unload EFI image into memory and execute it.
   #  Include/Protocol/LoadPe32Image.h
@@ -565,6 +571,10 @@ [Protocols]
   ## Include/Protocol/SdMmcOverride.h
   gEdkiiSdMmcOverrideProtocolGuid = { 0xeaf9e3c1, 0xc9cd, 0x46db, { 0xa5, 0xe5, 0x5a, 0x12, 0x4c, 0x83, 0x23, 0x23 } }
 
+  ## Include/Protocol/PlatformSpecificResetFilter.h
+  gEdkiiPlatformSpecificResetFilterProtocolGuid  = { 0x695d7835, 0x8d47, 0x4c11, { 0xab, 0x22, 0xfa, 0x8a, 0xcc, 0xe7, 0xae, 0x7a } }
+  ## Include/Protocol/PlatformSpecificResetHandler.h
+  gEdkiiPlatformSpecificResetHandlerProtocolGuid = { 0x2df6ba0b, 0x7092, 0x440d, { 0xbd, 0x4, 0xfb, 0x9, 0x1e, 0xc3, 0xf3, 0xc1 } }
 #
 # [Error.gEfiMdeModulePkgTokenSpaceGuid]
 #   0x80000001 | Invalid value provided.
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index 75cff37773..fed527fac2 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -15,6 +15,11 @@
 
 #include "ResetSystem.h"
 
+//
+// The current ResetSystem() notification recursion depth
+//
+UINTN  mResetNotifyDepth = 0;
+
 /**
   Register a notification function to be called when ResetSystem() is called.
 
@@ -130,6 +135,24 @@ RESET_NOTIFICATION_INSTANCE mResetNotification = {
   INITIALIZE_LIST_HEAD_VARIABLE (mResetNotification.ResetNotifies)
 };
 
+RESET_NOTIFICATION_INSTANCE mPlatformSpecificResetFilter = {
+  RESET_NOTIFICATION_INSTANCE_SIGNATURE,
+  {
+    RegisterResetNotify,
+    UnregisterResetNotify
+  },
+  INITIALIZE_LIST_HEAD_VARIABLE (mPlatformSpecificResetFilter.ResetNotifies)
+};
+
+RESET_NOTIFICATION_INSTANCE mPlatformSpecificResetHandler = {
+  RESET_NOTIFICATION_INSTANCE_SIGNATURE,
+  {
+    RegisterResetNotify,
+    UnregisterResetNotify
+  },
+  INITIALIZE_LIST_HEAD_VARIABLE (mPlatformSpecificResetHandler.ResetNotifies)
+};
+
 /**
   The driver's entry point.
 
@@ -170,6 +193,8 @@ InitializeResetSystem (
                   &Handle,
                   &gEfiResetArchProtocolGuid,         NULL,
                   &gEfiResetNotificationProtocolGuid, &mResetNotification.ResetNotification,
+                  &gEdkiiPlatformSpecificResetFilterProtocolGuid, &mPlatformSpecificResetFilter.ResetNotification,
+                  &gEdkiiPlatformSpecificResetHandlerProtocolGuid, &mPlatformSpecificResetHandler.ResetNotification,
                   NULL
                   );
   ASSERT_EFI_ERROR (Status);
@@ -225,13 +250,44 @@ ResetSystem (
   UINTN               CapsuleDataPtr;
   LIST_ENTRY          *Link;
   RESET_NOTIFY_ENTRY  *Entry;
-  
+
+  //
+  // Above the maximum recursion depth, so do the smallest amount of
+  // work to perform a cold reset.
+  //
+  if (mResetNotifyDepth >= MAX_RESET_NOTIFY_DEPTH) {
+    ResetCold ();
+    ASSERT (FALSE);
+    return;
+  }
+
   //
-  // Indicate reset system runtime service is called.
+  // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
   //
-  REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_EFI_RUNTIME_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
+  if (mResetNotifyDepth == 0) {
+    //
+    // Indicate reset system runtime service is called.
+    //
+    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_EFI_RUNTIME_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
+  }
 
-  if (!EfiAtRuntime ()) {
+  mResetNotifyDepth++;
+  if (!EfiAtRuntime () && mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH) {
+    //
+    // Call reset notification functions registered through the
+    // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
+    //
+    for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
+        ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
+        ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
+        ) {
+      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
+      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+    }
+    //
+    // Call reset notification functions registered through the
+    // EFI_RESET_NOTIFICATION_PROTOCOL.
+    //
     for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
         ; !IsNull (&mResetNotification.ResetNotifies, Link)
         ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
@@ -239,6 +295,17 @@ ResetSystem (
       Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
       Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
     }
+    //
+    // call reset notification functions registered through the 
+    // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
+    //
+    for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
+        ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
+        ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
+        ) {
+      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
+      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+    }
   }
 
   switch (ResetType) {
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
index 75cdd88896..ea5660274b 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.h
@@ -20,6 +20,8 @@
 
 #include <Protocol/Reset.h>
 #include <Protocol/ResetNotification.h>
+#include <Protocol/PlatformSpecificResetFilter.h>
+#include <Protocol/PlatformSpecificResetHandler.h>
 #include <Guid/CapsuleVendor.h>
 
 #include <Library/BaseLib.h>
@@ -34,6 +36,11 @@
 #include <Library/ReportStatusCodeLib.h>
 #include <Library/MemoryAllocationLib.h>
 
+//
+// The maximum recurstion depth to ResetSystem() by reset notification handlers
+//
+#define MAX_RESET_NOTIFY_DEPTH 10
+
 typedef struct {
   UINT32                   Signature;
   LIST_ENTRY               Link;
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
index 11233757c2..da9e8e118b 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
@@ -55,9 +55,10 @@ [Guids]
 
 
 [Protocols]
-  gEfiResetArchProtocolGuid                     ## PRODUCES
-  gEfiResetNotificationProtocolGuid             ## PRODUCES
-
+  gEfiResetArchProtocolGuid                       ## PRODUCES
+  gEfiResetNotificationProtocolGuid               ## PRODUCES
+  gEdkiiPlatformSpecificResetFilterProtocolGuid   ## PRODUCES
+  gEdkiiPlatformSpecificResetHandlerProtocolGuid  ## PRODUCES
 
 [Depex]
   TRUE
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (3 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 04/10] MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-19 18:59   ` Ard Biesheuvel
  2018-02-09  4:16 ` [PATCH v2 06/10] MdeModulePkg: Add ResetSystemLib instances that call core services Ruiyu Ni
                   ` (5 subsequent siblings)
  10 siblings, 1 reply; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Michael D Kinney

The patch adds more debug message in ResetSystem().
It also removes unnecessary check of mResetNotifyDepth.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
---
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 88 +++++++++++-----------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index fed527fac2..2c795426f5 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -15,6 +15,10 @@
 
 #include "ResetSystem.h"
 
+GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mResetTypeStr[] = {
+  L"Cold", L"Warm", L"Shutdown", L"PlatformSpecific"
+};
+
 //
 // The current ResetSystem() notification recursion depth
 //
@@ -251,16 +255,6 @@ ResetSystem (
   LIST_ENTRY          *Link;
   RESET_NOTIFY_ENTRY  *Entry;
 
-  //
-  // Above the maximum recursion depth, so do the smallest amount of
-  // work to perform a cold reset.
-  //
-  if (mResetNotifyDepth >= MAX_RESET_NOTIFY_DEPTH) {
-    ResetCold ();
-    ASSERT (FALSE);
-    return;
-  }
-
   //
   // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
   //
@@ -272,40 +266,47 @@ ResetSystem (
   }
 
   mResetNotifyDepth++;
-  if (!EfiAtRuntime () && mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH) {
-    //
-    // Call reset notification functions registered through the
-    // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
-    //
-    for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
-        ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
-        ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
-        ) {
-      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
-      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
-    }
-    //
-    // Call reset notification functions registered through the
-    // EFI_RESET_NOTIFICATION_PROTOCOL.
-    //
-    for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
-        ; !IsNull (&mResetNotification.ResetNotifies, Link)
-        ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
-        ) {
-      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
-      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
-    }
-    //
-    // call reset notification functions registered through the 
-    // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
-    //
-    for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
-        ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
-        ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
-        ) {
-      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
-      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
+
+  if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
+    if (!EfiAtRuntime ()) {
+      //
+      // Call reset notification functions registered through the
+      // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
+      //
+      for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
+          ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
+          ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
+          ) {
+        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
+        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+      }
+      //
+      // Call reset notification functions registered through the
+      // EFI_RESET_NOTIFICATION_PROTOCOL.
+      //
+      for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
+          ; !IsNull (&mResetNotification.ResetNotifies, Link)
+          ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
+          ) {
+        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
+        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+      }
+      //
+      // call reset notification functions registered through the 
+      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
+      //
+      for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
+          ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
+          ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
+          ) {
+        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
+        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
+      }
     }
+  } else {
+    ASSERT (ResetType < ARRAY_SIZE (mResetTypeStr));
+    DEBUG ((DEBUG_ERROR, "DXE ResetSystem2: Maximum reset call depth is met. Use the current reset type: %s!\n", mResetTypeStr[ResetType]));
   }
 
   switch (ResetType) {
@@ -331,7 +332,6 @@ ResetSystem (
     }
 
     ResetWarm ();
-
     break;
 
  case EfiResetCold:
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 06/10] MdeModulePkg: Add ResetSystemLib instances that call core services
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (4 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 07/10] MdeModulePkg: Add ResetUtility librray class and BASE instance Ruiyu Ni
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao

From: Michael D Kinney <michael.d.kinney@intel.com>

Add a PEI instance of ResetSystemLib that calls the ResetSystem2()
service in the PEI Services Table.

Add a DXE instance of ResetSystemLib that calls the ResetSystem()
service in the UEFI Runtime Services Table.

These 2 library instances should be the default ResetSystemLib
mapping for most PEIMs and DXE drivers so all reset system requests
go through the core service.

Only the implementation of the core servies should use the
platform specific instance of the ResetSystemLib that actually
performs the hardware actions to reset the platform.

Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
---
 .../Library/DxeResetSystemLib/DxeResetSystemLib.c  | 98 ++++++++++++++++++++++
 .../DxeResetSystemLib/DxeResetSystemLib.inf        | 39 +++++++++
 .../DxeResetSystemLib/DxeResetSystemLib.uni        | 21 +++++
 .../Library/PeiResetSystemLib/PeiResetSystemLib.c  | 98 ++++++++++++++++++++++
 .../PeiResetSystemLib/PeiResetSystemLib.inf        | 39 +++++++++
 .../PeiResetSystemLib/PeiResetSystemLib.uni        | 21 +++++
 6 files changed, 316 insertions(+)
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni

diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
new file mode 100644
index 0000000000..ea452e3231
--- /dev/null
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
@@ -0,0 +1,98 @@
+/** @file
+  DXE Reset System Library instance that calls gRT->ResetSystem().
+
+  Copyright (c) 2017 - 2018, Intel 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
+  which 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.
+
+**/
+
+#include <PiDxe.h>
+#include <Library/ResetSystemLib.h>
+#include <Library/UefiRuntimeLib.h>
+
+/**
+  This function causes a system-wide reset (cold reset), in which
+  all circuitry within the system returns to its initial state. This type of reset 
+  is asynchronous to system operation and operates without regard to
+  cycle boundaries.
+
+  If this function returns, it means that the system does not support cold reset. 
+**/
+VOID
+EFIAPI
+ResetCold (
+  VOID
+  )
+{
+  EfiResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes a system-wide initialization (warm reset), in which all processors 
+  are set to their initial state. Pending cycles are not corrupted.
+
+  If this function returns, it means that the system does not support warm reset.
+**/
+VOID
+EFIAPI
+ResetWarm (
+  VOID
+  )
+{
+  EfiResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes the system to enter a power state equivalent 
+  to the ACPI G2/S5 or G3 states.
+
+  If this function returns, it means that the system does not support shut down reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  EfiResetSystem (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes the system to enter S3 and then wake up immediately.
+
+  If this function returns, it means that the system does not support S3 feature.
+**/
+VOID
+EFIAPI
+EnterS3WithImmediateWake (
+  VOID
+  )
+{
+}
+
+/**
+  This function causes a systemwide reset. The exact type of the reset is
+  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
+  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
+  the platform must pick a supported reset type to perform.The platform may
+  optionally log the parameters from any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
+                         followed by the EFI_GUID.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID    *ResetData
+  )
+{
+  EfiResetSystem (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize, ResetData);
+}
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
new file mode 100644
index 0000000000..6eb2766b93
--- /dev/null
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  DXE Reset System Library instance that calls gRT->ResetSystem().
+#
+#  Copyright (c) 2017 - 2018, Intel 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
+#  which 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeResetSystemLib
+  MODULE_UNI_FILE                = DxeResetSystemLib.uni
+  FILE_GUID                      = C2BDE4F6-65EE-440B-87B5-83ABF10EF45B
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ResetSystemLib|DXE_CORE DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_SMM_DRIVER UEFI_APPLICATION UEFI_DRIVER
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#
+
+[Sources]
+  DxeResetSystemLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  UefiRuntimeLib
+
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni
new file mode 100644
index 0000000000..7c51ce0713
--- /dev/null
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni
@@ -0,0 +1,21 @@
+// /** @file
+// DXE Reset System Library instance that calls gRT->ResetSystem().
+//
+// DXE Reset System Library instance that calls gRT->ResetSystem().
+//
+// Copyright (c) 2017, Intel 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
+// which 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.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "DXE Reset System Library instance that calls gRT->ResetSystem()"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "DXE Reset System Library instance that calls gRT->ResetSystem()."
+
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
new file mode 100644
index 0000000000..30225d73a5
--- /dev/null
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
@@ -0,0 +1,98 @@
+/** @file
+  PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
+
+  Copyright (c) 2017 - 2018, Intel 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
+  which 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.
+
+**/
+
+#include <PiPei.h>
+#include <Library/ResetSystemLib.h>
+#include <Library/PeiServicesLib.h>
+
+/**
+  This function causes a system-wide reset (cold reset), in which
+  all circuitry within the system returns to its initial state. This type of reset 
+  is asynchronous to system operation and operates without regard to
+  cycle boundaries.
+
+  If this function returns, it means that the system does not support cold reset. 
+**/
+VOID
+EFIAPI
+ResetCold (
+  VOID
+  )
+{
+  PeiServicesResetSystem2 (EfiResetCold, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes a system-wide initialization (warm reset), in which all processors 
+  are set to their initial state. Pending cycles are not corrupted.
+
+  If this function returns, it means that the system does not support warm reset.
+**/
+VOID
+EFIAPI
+ResetWarm (
+  VOID
+  )
+{
+  PeiServicesResetSystem2 (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes the system to enter a power state equivalent 
+  to the ACPI G2/S5 or G3 states.
+
+  If this function returns, it means that the system does not support shut down reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  PeiServicesResetSystem2 (EfiResetShutdown, EFI_SUCCESS, 0, NULL);
+}
+
+/**
+  This function causes the system to enter S3 and then wake up immediately.
+
+  If this function returns, it means that the system does not support S3 feature.
+**/
+VOID
+EFIAPI
+EnterS3WithImmediateWake (
+  VOID
+  )
+{
+}
+
+/**
+  This function causes a systemwide reset. The exact type of the reset is
+  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
+  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
+  the platform must pick a supported reset type to perform.The platform may
+  optionally log the parameters from any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
+                         followed by the EFI_GUID.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID    *ResetData
+  )
+{
+  PeiServicesResetSystem2 (EfiResetPlatformSpecific, EFI_SUCCESS, DataSize, ResetData);
+}
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
new file mode 100644
index 0000000000..b1b9388c63
--- /dev/null
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
@@ -0,0 +1,39 @@
+## @file
+#  PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
+#
+#  Copyright (c) 2017 - 2018, Intel 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
+#  which 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = PeiResetSystemLib
+  MODULE_UNI_FILE                = PeiResetSystemLib.uni
+  FILE_GUID                      = 3198FF36-FC72-42E7-B98A-A080D823AFBF
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ResetSystemLib|PEI_CORE PEIM
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#
+
+[Sources]
+  PeiResetSystemLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  PeiServicesLib
+
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni
new file mode 100644
index 0000000000..ac996b3cc8
--- /dev/null
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni
@@ -0,0 +1,21 @@
+// /** @file
+// PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
+//
+// PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
+//
+// Copyright (c) 2017, Intel 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
+// which 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.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "PEI Reset System Library instance that calls the ResetSystem2() PEI Service"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "PEI Reset System Library instance that calls the ResetSystem2() PEI Service."
+
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 07/10] MdeModulePkg: Add ResetUtility librray class and BASE instance
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (5 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 06/10] MdeModulePkg: Add ResetSystemLib instances that call core services Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 08/10] MdePkg/UefiRuntimeLib: Support more module types Ruiyu Ni
                   ` (3 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael D Kinney, Liming Gao

From: Michael D Kinney <michael.d.kinney@intel.com>

The library class that provides services to generate a GUID specific
reset, parse the GUID from a GUID specific reset, and build the
ResetData buffer for any type of reset that requires extra data.

Cc: Liming Gao <liming.gao@intel.com>
Reviewed-by: Ruiyu Ni <ruiyu.ni@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Michael D Kinney <michael.d.kinney@intel.com>
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
---
 MdeModulePkg/Include/Library/ResetUtilityLib.h     | 111 +++++++++++
 .../Library/ResetUtilityLib/ResetUtility.c         | 220 +++++++++++++++++++++
 .../Library/ResetUtilityLib/ResetUtilityLib.inf    |  40 ++++
 MdeModulePkg/MdeModulePkg.dec                      |   3 +
 MdeModulePkg/MdeModulePkg.dsc                      |   3 +
 5 files changed, 377 insertions(+)
 create mode 100644 MdeModulePkg/Include/Library/ResetUtilityLib.h
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf

diff --git a/MdeModulePkg/Include/Library/ResetUtilityLib.h b/MdeModulePkg/Include/Library/ResetUtilityLib.h
new file mode 100644
index 0000000000..94828785e2
--- /dev/null
+++ b/MdeModulePkg/Include/Library/ResetUtilityLib.h
@@ -0,0 +1,111 @@
+/** @file
+  This header describes various helper functions for resetting the system.
+
+  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 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 _RESET_UTILITY_LIB_H_
+#define _RESET_UTILITY_LIB_H_
+
+/**
+  This is a shorthand helper function to reset with a subtype so that
+  the caller doesn't have to bother with a function that has half a dozen
+  parameters.
+
+  This will generate a reset with status EFI_SUCCESS, a NULL string, and
+  no custom data. The subtype will be formatted in such a way that it can be
+  picked up by notification registrations and custom handlers.
+
+  NOTE: This call will fail if the architectural ResetSystem underpinnings
+        are not initialized. For DXE, you can add gEfiResetArchProtocolGuid
+        to your DEPEX.
+
+  @param[in]  ResetType     Base reset type as defined in UEFI spec.
+  @param[in]  ResetSubtype  GUID pointer for the reset subtype to be used.
+
+**/
+VOID
+EFIAPI
+ResetPlatformSpecificGuid (
+  IN CONST  GUID        *ResetSubtype
+  );
+
+/**
+  This function examines the DataSize and ResetData parameters passed to
+  to ResetSystem() and detemrines if the ResetData contains a Null-terminated
+  Unicode string followed by a GUID specific subtype.  If the GUID specific 
+  subtype is present, then a pointer to the GUID value in ResetData is returned.
+
+  @param[in]  DataSize    The size, in bytes, of ResetData.
+  @param[in]  ResetData   Pointer to the data buffer passed into ResetSystem().
+
+  @retval     Pointer     Pointer to the GUID value in ResetData.
+  @retval     NULL        ResetData is NULL.
+  @retval     NULL        ResetData does not start with a Null-terminated
+                          Unicode string.
+  @retval     NULL        A Null-terminated Unicode string is present, but there
+                          are less than sizeof (GUID) bytes after the string.
+  @retval     NULL        No subtype is found.
+
+**/
+GUID *
+EFIAPI
+GetResetPlatformSpecificGuid (
+  IN UINTN       DataSize,
+  IN CONST VOID  *ResetData
+  );
+
+/**
+  This is a helper function that creates the reset data buffer that can be 
+  passed into ResetSystem().
+
+  The reset data buffer is returned in ResetData and contains ResetString
+  followed by the ResetSubtype GUID followed by the ExtraData.
+
+  NOTE: Strings are internally limited by MAX_UINT16.
+
+  @param[in, out] ResetDataSize  On input, the size of the ResetData buffer. On
+                                 output, either the total number of bytes
+                                 copied, or the required buffer size.
+  @param[in, out] ResetData      A pointer to the buffer in which to place the
+                                 final structure.
+  @param[in]      ResetSubtype   Pointer to the GUID specific subtype.  This
+                                 parameter is optional and may be NULL.
+  @param[in]      ResetString    Pointer to a Null-terminated Unicode string
+                                 that describes the reset.  This parameter is
+                                 optional and may be NULL.
+  @param[in]      ExtraDataSize  The size, in bytes, of ExtraData buffer.
+  @param[in]      ExtraData      Pointer to a buffer of extra data.  This
+                                 parameter is optional and may be NULL.
+
+  @retval     RETURN_SUCCESS             ResetDataSize and ResetData are updated.
+  @retval     RETURN_INVALID_PARAMETER   ResetDataSize is NULL.
+  @retval     RETURN_INVALID_PARAMETER   ResetData is NULL.
+  @retval     RETURN_INVALID_PARAMETER   ExtraData was provided without a
+                                         ResetSubtype. This is not supported by the
+                                         UEFI spec.
+  @retval     RETURN_BUFFER_TOO_SMALL    An insufficient buffer was provided.
+                                         ResetDataSize is updated with minimum size
+                                         required.
+**/
+RETURN_STATUS
+EFIAPI
+BuildResetData (
+  IN OUT   UINTN     *ResetDataSize,
+  IN OUT   VOID      *ResetData,
+  IN CONST GUID      *ResetSubtype  OPTIONAL,
+  IN CONST CHAR16    *ResetString   OPTIONAL,
+  IN       UINTN     ExtraDataSize  OPTIONAL,
+  IN CONST VOID      *ExtraData     OPTIONAL
+  );
+
+#endif // _RESET_UTILITY_LIB_H_
diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
new file mode 100644
index 0000000000..90de94ca9b
--- /dev/null
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -0,0 +1,220 @@
+/** @file
+  This contains the business logic for the module-specific Reset Helper functions.
+
+  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 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.
+
+**/
+#include <Uefi.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/ResetSystemLib.h>
+
+typedef struct {
+  CHAR16 NullTerminator;
+  GUID   ResetSubtype;
+} RESET_UTILITY_GUID_SPECIFIC_RESET_DATA;
+
+/**
+  This is a shorthand helper function to reset with a subtype so that
+  the caller doesn't have to bother with a function that has half a dozen
+  parameters.
+
+  This will generate a reset with status EFI_SUCCESS, a NULL string, and
+  no custom data. The subtype will be formatted in such a way that it can be
+  picked up by notification registrations and custom handlers.
+
+  NOTE: This call will fail if the architectural ResetSystem underpinnings
+        are not initialized. For DXE, you can add gEfiResetArchProtocolGuid
+        to your DEPEX.
+
+  @param[in]  ResetSubtype  GUID pointer for the reset subtype to be used.
+
+**/
+VOID
+EFIAPI
+ResetPlatformSpecificGuid (
+  IN CONST  GUID        *ResetSubtype
+  )
+{
+  RESET_UTILITY_GUID_SPECIFIC_RESET_DATA  ResetData;
+
+  ResetData.NullTerminator = CHAR_NULL;
+  CopyGuid (&ResetData.ResetSubtype, ResetSubtype);
+  ResetPlatformSpecific (sizeof (ResetData), &ResetData);
+}
+
+/**
+  This function examines the DataSize and ResetData parameters passed to
+  to ResetSystem() and detemrines if the ResetData contains a Null-terminated
+  Unicode string followed by a GUID specific subtype.  If the GUID specific 
+  subtype is present, then a pointer to the GUID value in ResetData is returned.
+
+  @param[in]  DataSize    The size, in bytes, of ResetData.
+  @param[in]  ResetData   Pointer to the data buffer passed into ResetSystem().
+
+  @retval     Pointer     Pointer to the GUID value in ResetData.
+  @retval     NULL        ResetData is NULL.
+  @retval     NULL        ResetData does not start with a Null-terminated
+                          Unicode string.
+  @retval     NULL        A Null-terminated Unicode string is present, but there
+                          are less than sizeof (GUID) bytes after the string.
+  @retval     NULL        No subtype is found.
+
+**/
+GUID *
+EFIAPI
+GetResetPlatformSpecificGuid (
+  IN UINTN       DataSize,
+  IN CONST VOID  *ResetData
+  )
+{
+  UINTN          ResetDataStringSize;
+  GUID           *ResetSubtypeGuid;
+
+  //
+  // Make sure parameters are valid
+  //
+  if ((ResetData == NULL) || (DataSize < sizeof (GUID))) {
+    return NULL;
+  }
+
+  //
+  // Determine the number of bytes in the Null-terminated Unicode string
+  // at the beginning of ResetData including the Null terminator.
+  //
+  ResetDataStringSize = StrnSizeS (ResetData, (DataSize / sizeof (CHAR16)));
+
+  //
+  // Now, assuming that we have enough data for a GUID after the string, the
+  // GUID should be immediately after the string itself.
+  //
+  if ((ResetDataStringSize < DataSize) && (DataSize - ResetDataStringSize) >= sizeof (GUID)) {
+    ResetSubtypeGuid = (GUID *)((UINT8 *)ResetData + ResetDataStringSize);
+    DEBUG ((DEBUG_VERBOSE, __FUNCTION__" - Detected reset subtype %g...\n", ResetSubtypeGuid));
+    return ResetSubtypeGuid;
+  }
+  return NULL;
+}
+
+/**
+  This is a helper function that creates the reset data buffer that can be 
+  passed into ResetSystem().
+
+  The reset data buffer is returned in ResetData and contains ResetString
+  followed by the ResetSubtype GUID followed by the ExtraData.
+
+  NOTE: Strings are internally limited by MAX_UINT16.
+
+  @param[in, out] ResetDataSize  On input, the size of the ResetData buffer. On
+                                 output, either the total number of bytes
+                                 copied, or the required buffer size.
+  @param[in, out] ResetData      A pointer to the buffer in which to place the
+                                 final structure.
+  @param[in]      ResetSubtype   Pointer to the GUID specific subtype.  This
+                                 parameter is optional and may be NULL.
+  @param[in]      ResetString    Pointer to a Null-terminated Unicode string
+                                 that describes the reset.  This parameter is
+                                 optional and may be NULL.
+  @param[in]      ExtraDataSize  The size, in bytes, of ExtraData buffer.
+  @param[in]      ExtraData      Pointer to a buffer of extra data.  This
+                                 parameter is optional and may be NULL.
+
+  @retval     RETURN_SUCCESS             ResetDataSize and ResetData are updated.
+  @retval     RETURN_INVALID_PARAMETER   ResetDataSize is NULL.
+  @retval     RETURN_INVALID_PARAMETER   ResetData is NULL.
+  @retval     RETURN_INVALID_PARAMETER   ExtraData was provided without a
+                                         ResetSubtype. This is not supported by the
+                                         UEFI spec.
+  @retval     RETURN_BUFFER_TOO_SMALL    An insufficient buffer was provided.
+                                         ResetDataSize is updated with minimum size
+                                         required.
+**/
+RETURN_STATUS
+EFIAPI
+BuildResetData (
+  IN OUT   UINTN     *ResetDataSize,
+  IN OUT   VOID      *ResetData,
+  IN CONST GUID      *ResetSubtype  OPTIONAL,
+  IN CONST CHAR16    *ResetString   OPTIONAL,
+  IN       UINTN     ExtraDataSize  OPTIONAL,
+  IN CONST VOID      *ExtraData     OPTIONAL
+  )
+{
+  UINTN  ResetStringSize;
+  UINTN  ResetDataBufferSize;
+  UINT8  *Data;
+
+  //
+  // If the size return pointer is NULL.
+  //
+  if (ResetDataSize == NULL) {
+    return RETURN_INVALID_PARAMETER;
+  }
+  //
+  // If extra data is indicated, but pointer is NULL.
+  //
+  if (ExtraDataSize > 0 && ExtraData == NULL) {
+    return RETURN_INVALID_PARAMETER;
+  }
+  //
+  // If extra data is indicated, but no subtype GUID is supplied.
+  //
+  if (ResetSubtype == NULL && ExtraDataSize > 0) {
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Determine the final string.
+  //
+  if (ResetString == NULL) {
+    ResetString = L"";     // Use an empty string.
+  }
+  
+  //
+  // Calculate the total buffer required for ResetData.
+  //
+  ResetStringSize     = StrnSizeS (ResetString, MAX_UINT16);
+  ResetDataBufferSize = ResetStringSize + ExtraDataSize;
+  if (ResetSubtype != NULL) {
+    ResetDataBufferSize += sizeof (GUID);
+  }
+
+  //
+  // At this point, if the buffer isn't large enough (or if
+  // the buffer is NULL) we cannot proceed.
+  //
+  if (*ResetDataSize < ResetDataBufferSize) {
+    *ResetDataSize = ResetDataBufferSize;
+    return RETURN_BUFFER_TOO_SMALL;
+  }
+  *ResetDataSize = ResetDataBufferSize;
+  if (ResetData == NULL) {
+    return RETURN_INVALID_PARAMETER;
+  }
+
+  //
+  // Fill in ResetData with ResetString, the ResetSubtype GUID, and extra data
+  //
+  Data = (UINT8 *)ResetData;
+  CopyMem (Data, ResetString, ResetStringSize);
+  Data += ResetStringSize;
+  if (ResetSubtype != NULL) {
+    CopyMem (Data, ResetSubtype, sizeof (GUID));
+    Data += sizeof (GUID);
+  }
+  if (ExtraDataSize > 0) {
+    CopyMem (Data, ExtraData, ExtraDataSize);
+  }
+  
+  return RETURN_SUCCESS;
+}
diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
new file mode 100644
index 0000000000..2a4e53a8a6
--- /dev/null
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
@@ -0,0 +1,40 @@
+## @file
+# This file contains the Reset Utility functions.
+#
+#  Copyright (c) 2017 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016, 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
+#  which 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.
+#  
+##
+[Defines]
+  INF_VERSION         = 0x00010017
+  BASE_NAME           = ResetUtilityLib
+  FILE_GUID           = CAFC3CA1-3E32-449F-9B0E-40BA3CB73A12
+  VERSION_STRING      = 1.0
+  MODULE_TYPE         = BASE
+  LIBRARY_CLASS       = ResetUtilityLib
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  ResetUtility.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  DebugLib
+  BaseMemoryLib
+  ResetSystemLib
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 1cc9bc8ea1..b444e38f1a 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -54,6 +54,9 @@ [LibraryClasses]
   ##  @libraryclass  Defines a set of methods to reset whole system.
   ResetSystemLib|Include/Library/ResetSystemLib.h
 
+  ##  @libraryclass  Defines a set of helper functions for resetting the system.
+  ResetUtilityLib|Include/Library/ResetUtilityLib.h
+
   ##  @libraryclass  Defines a set of methods related do S3 mode.
   #   This library class is no longer used and modules using this library should
   #   directly locate EFI_PEI_S3_RESUME_PPI defined in PI 1.2 specification.
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index dd7e9d5988..abac00e08f 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -285,13 +285,16 @@ [Components]
   MdeModulePkg/Library/DxeIpIoLib/DxeIpIoLib.inf
   MdeModulePkg/Library/DxeNetLib/DxeNetLib.inf
   MdeModulePkg/Library/DxePerformanceLib/DxePerformanceLib.inf
+  MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
   MdeModulePkg/Library/DxeUdpIoLib/DxeUdpIoLib.inf
   MdeModulePkg/Library/DxePrintLibPrint2Protocol/DxePrintLibPrint2Protocol.inf
   MdeModulePkg/Library/PeiCrc32GuidedSectionExtractLib/PeiCrc32GuidedSectionExtractLib.inf
   MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib.inf
   MdeModulePkg/Library/PeiRecoveryLibNull/PeiRecoveryLibNull.inf
+  MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
   MdeModulePkg/Library/PeiS3LibNull/PeiS3LibNull.inf
   MdeModulePkg/Library/UefiHiiLib/UefiHiiLib.inf
+  MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
   MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf
   MdeModulePkg/Library/DxeSecurityManagementLib/DxeSecurityManagementLib.inf
   MdeModulePkg/Library/OemHookStatusCodeLibNull/OemHookStatusCodeLibNull.inf
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 08/10] MdePkg/UefiRuntimeLib: Support more module types.
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (6 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 07/10] MdeModulePkg: Add ResetUtility librray class and BASE instance Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 09/10] MdeModulePkg: Add ResetSystemPei PEIM Ruiyu Ni
                   ` (2 subsequent siblings)
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Michael D Kinney

Because DxeResetSystemLib links to this library to provide
reset system services, change UefiRuntimeLib to support
the same set of module types as what DxeResetSystemLib does.

Cc: Liming Gao <liming.gao@intel.com>
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Reviewed-by: Star Zeng <star.zeng@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
---
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf b/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
index 8f46495fc5..d053da545a 100644
--- a/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
+++ b/MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf
@@ -5,7 +5,7 @@
 #  EVT_SIGNAL_EXIT_BOOT_SERVICES event, to provide runtime services.
 # This instance also supports SAL drivers for better performance.
 #
-# Copyright (c) 2006 - 2014, Intel Corporation. All rights reserved.<BR>
+# Copyright (c) 2006 - 2017, Intel 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
@@ -24,7 +24,7 @@ [Defines]
   FILE_GUID                      = b1ee6c28-54aa-4d17-b705-3e28ccb27b2e
   MODULE_TYPE                    = DXE_RUNTIME_DRIVER
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = UefiRuntimeLib|DXE_RUNTIME_DRIVER DXE_SAL_DRIVER
+  LIBRARY_CLASS                  = UefiRuntimeLib|DXE_RUNTIME_DRIVER DXE_SAL_DRIVER DXE_CORE DXE_DRIVER DXE_SMM_DRIVER 
 
   CONSTRUCTOR                    = RuntimeDriverLibConstruct
   DESTRUCTOR                     = RuntimeDriverLibDeconstruct
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 09/10] MdeModulePkg: Add ResetSystemPei PEIM
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (7 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 08/10] MdePkg/UefiRuntimeLib: Support more module types Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:16 ` [PATCH v2 10/10] MdeModulePkg/ResetSystemPei: Add reset notifications in PEI Ruiyu Ni
  2018-02-09  4:55 ` [PATCH v2 00/10] Formalize the reset system core design Zeng, Star
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel; +Cc: Liming Gao, Michael D Kinney, Star Zeng

This driver implements Reset2, ResetFilter and ResetHandler PPIs.

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: Ruiyu Ni <ruiyu.ni@intel.com>
---
 MdeModulePkg/MdeModulePkg.dec                      |   4 +
 MdeModulePkg/MdeModulePkg.dsc                      |   4 +
 MdeModulePkg/MdeModulePkg.uni                      |   6 +-
 .../Universal/ResetSystemPei/ResetSystem.c         | 355 +++++++++++++++++++++
 .../Universal/ResetSystemPei/ResetSystem.h         | 129 ++++++++
 .../Universal/ResetSystemPei/ResetSystemPei.inf    |  62 ++++
 .../Universal/ResetSystemPei/ResetSystemPei.uni    |  20 ++
 .../ResetSystemPei/ResetSystemPeiExtra.uni         |  20 ++
 8 files changed, 599 insertions(+), 1 deletion(-)
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni

diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index b444e38f1a..484ad3fb4d 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -1422,6 +1422,10 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt CapsuleMax value in capsule report variable.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax|0xFFFF|UINT16|0x00000107
 
+  ## Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase.
+  # @Prompt Maximum Number of PEI Reset Filters or Reset Handlers.
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies|0x10|UINT32|0x00000109
+
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
   ## This PCD defines the Console output row. The default value is 25 according to UEFI spec.
   #  This PCD could be set to 0 then console output would be at max column and max row.
diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc
index abac00e08f..d96bff90b0 100644
--- a/MdeModulePkg/MdeModulePkg.dsc
+++ b/MdeModulePkg/MdeModulePkg.dsc
@@ -361,6 +361,10 @@ [Components]
   MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
   MdeModulePkg/Universal/Metronome/Metronome.inf
   MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
+  MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf {
+    <LibraryClasses>
+      ResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf
+  }
   MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf
   MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.inf
   MdeModulePkg/Universal/SmbiosMeasurementDxe/SmbiosMeasurementDxe.inf
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index 0e068422e4..a63d4764a3 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -4,7 +4,7 @@
 // It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes)
 // and libraries instances, which are used for those modules.
 //
-// Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2007 - 2018, Intel 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.
@@ -1059,6 +1059,10 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleMax_HELP  #language en-US "CapsuleMax value in capsule report variable."
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_PROMPT  #language en-US "Maximum Number of PEI Reset Filters or Reset Handlers."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_HELP  #language en-US "Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase."
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_PROMPT  #language en-US "Recover file name in PEI phase"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_HELP  #language en-US "This is recover file name in PEI phase.\n"
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
new file mode 100644
index 0000000000..8d9bd4884a
--- /dev/null
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
@@ -0,0 +1,355 @@
+/** @file
+  Implementation of Reset2, ResetFilter and ResetHandler PPIs.
+
+  Copyright (c) 2017, Intel 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
+  which 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.
+
+**/
+
+#include "ResetSystem.h"
+
+GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mResetTypeStr[] = {
+  L"Cold", L"Warm", L"Shutdown", L"PlatformSpecific"
+};
+
+EFI_PEI_RESET2_PPI mPpiReset2 = {
+  ResetSystem2
+};
+
+EFI_GUID                *mProcessingOrder[] = {
+  &gEdkiiPlatformSpecificResetFilterPpiGuid,
+  &gEdkiiPlatformSpecificResetHandlerPpiGuid
+};
+
+RESET_FILTER_INSTANCE   mResetFilter = {
+  {
+    RegisterResetNotify,
+    UnregisterResetNotify
+  },
+  &gEdkiiPlatformSpecificResetFilterPpiGuid
+};
+
+RESET_FILTER_INSTANCE   mResetHandler = {
+  {
+    RegisterResetNotify,
+    UnregisterResetNotify
+  },
+  &gEdkiiPlatformSpecificResetHandlerPpiGuid
+};
+
+EFI_PEI_PPI_DESCRIPTOR mPpiListReset[] = {
+  {
+    EFI_PEI_PPI_DESCRIPTOR_PPI,
+    &gEfiPeiReset2PpiGuid,
+    &mPpiReset2
+  },
+  {
+    EFI_PEI_PPI_DESCRIPTOR_PPI,
+    &gEdkiiPlatformSpecificResetFilterPpiGuid,
+    &mResetFilter.ResetFilter
+  },
+  {
+    EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+    &gEdkiiPlatformSpecificResetHandlerPpiGuid,
+    &mResetHandler.ResetFilter
+  }
+};
+
+/**
+  Register a notification function to be called when ResetSystem() is called.
+
+  The RegisterResetNotify() function registers a notification function that is called when
+  ResetSystem() is called and prior to completing the reset of the platform.
+  The registered functions must not perform a platform reset themselves. These
+  notifications are intended only for the notification of components which may need some
+  special-purpose maintenance prior to the platform resetting.
+  The list of registered reset notification functions are processed if ResetSystem()is called
+  before ExitBootServices(). The list of registered reset notification functions is ignored if
+  ResetSystem() is called after ExitBootServices().
+
+  @param[in]  This              A pointer to the EFI_RESET_NOTIFICATION_PROTOCOL instance.
+  @param[in]  ResetFunction     Points to the function to be called when a ResetSystem() is executed.
+
+  @retval EFI_SUCCESS           The reset notification function was successfully registered.
+  @retval EFI_INVALID_PARAMETER ResetFunction is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There are not enough resources available to register the reset notification function.
+  @retval EFI_ALREADY_STARTED   The reset notification function specified by ResetFunction has already been registered.
+
+**/
+EFI_STATUS
+EFIAPI
+RegisterResetNotify (
+  IN EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI *This,
+  IN EFI_RESET_SYSTEM                         ResetFunction
+  )
+{
+  RESET_FILTER_INSTANCE                       *ResetFilter;
+  RESET_FILTER_LIST                           *List;
+  VOID                                        *Hob;
+  UINTN                                       Index;
+
+  if (ResetFunction == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ResetFilter = (RESET_FILTER_INSTANCE *) This;
+  ASSERT (CompareGuid (ResetFilter->Guid, &gEdkiiPlatformSpecificResetFilterPpiGuid) ||
+          CompareGuid (ResetFilter->Guid, &gEdkiiPlatformSpecificResetHandlerPpiGuid)
+          );
+
+  Hob = GetFirstGuidHob (ResetFilter->Guid);
+  if (Hob == NULL) {
+    //
+    // When the GUIDed HOB doesn't exist, create it.
+    //
+    List = (RESET_FILTER_LIST *)BuildGuidHob (
+                                  ResetFilter->Guid,
+                                  sizeof (RESET_FILTER_LIST) + sizeof (EFI_RESET_SYSTEM) * PcdGet32 (PcdMaximumPeiResetNotifies)
+                                  );
+    if (List == NULL) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+    List->Signature = RESET_FILTER_LIST_SIGNATURE;
+    List->Count     = PcdGet32 (PcdMaximumPeiResetNotifies);
+    ZeroMem (List->ResetFilters, sizeof (EFI_RESET_SYSTEM) * List->Count);
+    List->ResetFilters[0] = ResetFunction;
+    return EFI_SUCCESS;
+  } else {
+    List = (RESET_FILTER_LIST *)GET_GUID_HOB_DATA (Hob);
+    ASSERT (List->Signature == RESET_FILTER_LIST_SIGNATURE);
+    //
+    // Firstly check whether the ResetFunction is already registerred.
+    //
+    for (Index = 0; Index < List->Count; Index++) {
+      if (List->ResetFilters[Index] == ResetFunction) {
+        break;
+      }
+    }
+    if (Index != List->Count) {
+      return EFI_ALREADY_STARTED;
+    }
+
+    //
+    // Secondly find the first free slot.
+    //
+    for (Index = 0; Index < List->Count; Index++) {
+      if (List->ResetFilters[Index] == NULL) {
+        break;
+      }
+    }
+
+    if (Index == List->Count) {
+      return EFI_OUT_OF_RESOURCES;
+    }
+    List->ResetFilters[Index] = ResetFunction;
+    return EFI_SUCCESS;
+  }
+}
+
+/**
+  Unregister a notification function.
+
+  The UnregisterResetNotify() function removes the previously registered
+  notification using RegisterResetNotify().
+
+  @param[in]  This              A pointer to the EFI_RESET_NOTIFICATION_PROTOCOL instance.
+  @param[in]  ResetFunction     The pointer to the ResetFunction being unregistered.
+
+  @retval EFI_SUCCESS           The reset notification function was unregistered.
+  @retval EFI_INVALID_PARAMETER ResetFunction is NULL.
+  @retval EFI_INVALID_PARAMETER The reset notification function specified by ResetFunction was not previously
+                                registered using RegisterResetNotify().
+
+**/
+EFI_STATUS
+EFIAPI
+UnregisterResetNotify (
+  IN EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI *This,
+  IN EFI_RESET_SYSTEM                         ResetFunction
+  )
+{
+
+  RESET_FILTER_INSTANCE                       *ResetFilter;
+  RESET_FILTER_LIST                           *List;
+  VOID                                        *Hob;
+  UINTN                                       Index;
+
+  if (ResetFunction == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  ResetFilter = (RESET_FILTER_INSTANCE *)This;
+  ASSERT (CompareGuid (ResetFilter->Guid, &gEdkiiPlatformSpecificResetFilterPpiGuid) ||
+    CompareGuid (ResetFilter->Guid, &gEdkiiPlatformSpecificResetHandlerPpiGuid)
+  );
+
+  Hob = GetFirstGuidHob (ResetFilter->Guid);
+  if (Hob == NULL) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  List = (RESET_FILTER_LIST *)GET_GUID_HOB_DATA (Hob);
+  ASSERT (List->Signature == RESET_FILTER_LIST_SIGNATURE);
+  for (Index = 0; Index < List->Count; Index++) {
+    if (List->ResetFilters[Index] == ResetFunction) {
+      break;
+    }
+  }
+
+  if (Index == List->Count) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  List->ResetFilters[Index] = NULL;
+  return EFI_SUCCESS;
+}
+
+
+/**
+  The PEIM's entry point.
+
+  It initializes the Reset2, ResetFilter and ResetHandler PPIs.
+
+  @param[in] FileHandle  Handle of the file being invoked.
+  @param[in] PeiServices Describes the list of possible PEI Services.
+  
+  @retval EFI_SUCCESS         The entry point is executed successfully.
+  @retval EFI_ALREADY_STARTED The Reset2 PPI was already installed.
+  @retval others              Status code returned from PeiServicesInstallPpi().
+
+**/
+EFI_STATUS
+EFIAPI
+InitializeResetSystem (
+  IN EFI_PEI_FILE_HANDLE     FileHandle,
+  IN CONST EFI_PEI_SERVICES  **PeiServices
+  )
+{
+  EFI_STATUS  Status;
+  VOID        *Ppi;
+
+  Status = PeiServicesLocatePpi (&gEfiPeiReset2PpiGuid, 0, NULL, (VOID **)&Ppi);
+  if (Status != EFI_NOT_FOUND) {
+    return EFI_ALREADY_STARTED;
+  }
+
+  PeiServicesInstallPpi (mPpiListReset);
+
+  return Status;
+}
+
+/**
+  Resets the entire platform.
+
+  @param[in] ResetType          The type of reset to perform.
+  @param[in] ResetStatus        The status code for the reset.
+  @param[in] DataSize           The size, in bytes, of ResetData.
+  @param[in] ResetData          For a ResetType of EfiResetCold, EfiResetWarm, or
+                                EfiResetShutdown the data buffer starts with a Null-terminated
+                                string, optionally followed by additional binary data.
+                                The string is a description that the caller may use to further
+                                indicate the reason for the system reset. ResetData is only
+                                valid if ResetStatus is something other than EFI_SUCCESS
+                                unless the ResetType is EfiResetPlatformSpecific
+                                where a minimum amount of ResetData is always required.
+                                For a ResetType of EfiResetPlatformSpecific the data buffer
+                                also starts with a Null-terminated string that is followed
+                                by an EFI_GUID that describes the specific type of reset to perform.
+**/
+VOID
+EFIAPI
+ResetSystem2 (
+  IN EFI_RESET_TYPE   ResetType,
+  IN EFI_STATUS       ResetStatus,
+  IN UINTN            DataSize,
+  IN VOID             *ResetData OPTIONAL
+  )
+{
+  VOID                *Hob;
+  UINTN               Index;
+  RESET_FILTER_LIST   *List;
+  UINTN               OrderIndex;
+  UINT8               RecursionDepth;
+  UINT8               *RecursionDepthPointer;
+
+  //
+  // The recursion depth is stored in GUIDed HOB using gEfiCallerIdGuid.
+  //
+  Hob = GetFirstGuidHob (&gEfiCallerIdGuid);
+  if (Hob == NULL) {
+    RecursionDepth = 0;
+    RecursionDepthPointer = BuildGuidDataHob (&gEfiCallerIdGuid, &RecursionDepth, sizeof (RecursionDepth));
+  } else {
+    RecursionDepthPointer = (UINT8 *)GET_GUID_HOB_DATA (Hob);
+  }
+  //
+  // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
+  //
+  if (*RecursionDepthPointer == 0) {
+    //
+    // Indicate reset system runtime service is called.
+    //
+    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_PEI_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
+  }
+
+  //
+  // Increase the call depth
+  //
+  (*RecursionDepthPointer)++;
+  DEBUG ((DEBUG_INFO, "PEI ResetSystem2: Reset call depth = %d.\n", *RecursionDepthPointer));
+
+  if (*RecursionDepthPointer <= MAX_RESET_NOTIFY_DEPTH) {
+    //
+    // Iteratively call Reset Filters and Reset Handlers.
+    //
+    for (OrderIndex = 0; OrderIndex < ARRAY_SIZE (mProcessingOrder); OrderIndex++) {
+      Hob = GetFirstGuidHob (mProcessingOrder[OrderIndex]);
+      if (Hob != NULL) {
+        List = (RESET_FILTER_LIST *)GET_GUID_HOB_DATA (Hob);
+        ASSERT (List->Signature == RESET_FILTER_LIST_SIGNATURE);
+
+        for (Index = 0; Index < List->Count; Index++) {
+          if (List->ResetFilters[Index] != NULL) {
+            List->ResetFilters[Index] (ResetType, ResetStatus, DataSize, ResetData);
+          }
+        }
+      }
+    }
+  } else {
+    ASSERT (ResetType < ARRAY_SIZE (mResetTypeStr));
+    DEBUG ((DEBUG_ERROR, "PEI ResetSystem2: Maximum reset call depth is met. Use the current reset type: %s!\n", mResetTypeStr[ResetType]));
+  }
+
+  switch (ResetType) {
+  case EfiResetWarm:
+    ResetWarm ();
+    break;
+
+ case EfiResetCold:
+    ResetCold ();
+    break;
+
+  case EfiResetShutdown:
+    ResetShutdown ();
+    return ;
+
+  case EfiResetPlatformSpecific:
+    ResetPlatformSpecific (DataSize, ResetData);
+    return;
+
+  default:
+    return ;
+  }
+
+  //
+  // Given we should have reset getting here would be bad
+  //
+  ASSERT (FALSE);
+}
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
new file mode 100644
index 0000000000..f06d39bef5
--- /dev/null
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
@@ -0,0 +1,129 @@
+/** @file
+
+  Copyright (c) 2017 - 2018, Intel 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
+  which 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 _RESET_SYSTEM2_H_
+#define _RESET_SYSTEM2_H_
+
+
+#include <Uefi.h>
+#include <PiPei.h>
+
+#include <Ppi/Reset2.h>
+#include <Ppi/PlatformSpecificResetFilter.h>
+#include <Ppi/PlatformSpecificResetHandler.h>
+
+#include <Library/BaseLib.h>
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/PeiServicesLib.h>
+#include <Library/HobLib.h>
+#include <Library/ResetSystemLib.h>
+#include <Library/ReportStatusCodeLib.h>
+
+
+//
+// The maximum recursion depth to ResetSystem() by reset notification handlers
+//
+#define MAX_RESET_NOTIFY_DEPTH 10
+
+//
+// Data to put in GUIDed HOB
+//
+typedef struct {
+  UINT32                          Signature;
+  UINT32                          Count;
+  EFI_RESET_SYSTEM                ResetFilters[0]; // ResetFilters[PcdGet32 (PcdMaximumResetNotifies)]
+} RESET_FILTER_LIST;
+#define RESET_FILTER_LIST_SIGNATURE    SIGNATURE_32('r', 's', 't', 'l')
+
+
+typedef struct {
+  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI ResetFilter;
+  EFI_GUID                                 *Guid;
+} RESET_FILTER_INSTANCE;
+
+/**
+  Resets the entire platform.
+
+  @param[in] ResetType          The type of reset to perform.
+  @param[in] ResetStatus        The status code for the reset.
+  @param[in] DataSize           The size, in bytes, of ResetData.
+  @param[in] ResetData          For a ResetType of EfiResetCold, EfiResetWarm, or
+                                EfiResetShutdown the data buffer starts with a Null-terminated
+                                string, optionally followed by additional binary data.
+                                The string is a description that the caller may use to further
+                                indicate the reason for the system reset. ResetData is only
+                                valid if ResetStatus is something other than EFI_SUCCESS
+                                unless the ResetType is EfiResetPlatformSpecific
+                                where a minimum amount of ResetData is always required.
+                                For a ResetType of EfiResetPlatformSpecific the data buffer
+                                also starts with a Null-terminated string that is followed
+                                by an EFI_GUID that describes the specific type of reset to perform.
+
+**/
+VOID
+EFIAPI
+ResetSystem2 (
+  IN EFI_RESET_TYPE   ResetType,
+  IN EFI_STATUS       ResetStatus,
+  IN UINTN            DataSize,
+  IN VOID             *ResetData OPTIONAL
+  );
+/**
+  Register a notification function to be called when ResetSystem() is called.
+
+  The RegisterResetNotify() function registers a notification function that is called when
+  ResetSystem()is called and prior to completing the reset of the platform.
+  The registered functions must not perform a platform reset themselves. These
+  notifications are intended only for the notification of components which may need some
+  special-purpose maintenance prior to the platform resetting.
+
+  @param[in]  This              A pointer to the EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI instance.
+  @param[in]  ResetFunction     Points to the function to be called when a ResetSystem() is executed.
+
+  @retval EFI_SUCCESS           The reset notification function was successfully registered.
+  @retval EFI_INVALID_PARAMETER ResetFunction is NULL.
+  @retval EFI_OUT_OF_RESOURCES  There are not enough resources available to register the reset notification function.
+  @retval EFI_ALREADY_STARTED   The reset notification function specified by ResetFunction has already been registered.
+
+**/
+EFI_STATUS
+EFIAPI
+RegisterResetNotify (
+  IN EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI *This,
+  IN EFI_RESET_SYSTEM                         ResetFunction
+  );
+
+/**
+  Unregister a notification function.
+
+  The UnregisterResetNotify() function removes the previously registered
+  notification using RegisterResetNotify().
+
+  @param[in]  This              A pointer to the EFI_RESET_NOTIFICATION_PROTOCOL instance.
+  @param[in]  ResetFunction     The pointer to the ResetFunction being unregistered.
+
+  @retval EFI_SUCCESS           The reset notification function was unregistered.
+  @retval EFI_INVALID_PARAMETER ResetFunction is NULL.
+  @retval EFI_INVALID_PARAMETER The reset notification function specified by ResetFunction was not previously
+                                registered using RegisterResetNotify().
+
+**/
+EFI_STATUS
+EFIAPI
+UnregisterResetNotify (
+  IN EFI_RESET_NOTIFICATION_PROTOCOL *This,
+  IN EFI_RESET_SYSTEM                ResetFunction
+  );
+#endif
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
new file mode 100644
index 0000000000..38fdd16ceb
--- /dev/null
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
@@ -0,0 +1,62 @@
+## @file
+# This driver implements Reset2, ResetFilter and ResetHandler PPIs.
+#
+# Copyright (c) 2017, Intel 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
+# which 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.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = ResetSystemPei
+  MODULE_UNI_FILE                = ResetSystemPei.uni
+  FILE_GUID                      = 6141E486-7543-4F1A-A579-FF532ED78E75
+  MODULE_TYPE                    = PEIM
+  VERSION_STRING                 = 1.0
+
+  ENTRY_POINT                    = InitializeResetSystem
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64
+#
+
+[Sources]
+  ResetSystem.h
+  ResetSystem.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+
+[LibraryClasses]
+  BaseLib
+  BaseMemoryLib
+  DebugLib
+  PeiServicesLib
+  HobLib
+  PeimEntryPoint
+  ResetSystemLib
+  ReportStatusCodeLib
+
+[Ppis]
+  gEfiPeiReset2PpiGuid                       ## PRODUCES
+  gEdkiiPlatformSpecificResetFilterPpiGuid   ## PRODUCES
+  gEdkiiPlatformSpecificResetHandlerPpiGuid  ## PRODUCES
+
+[Pcd]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies
+
+[Depex]
+  TRUE
+
+[UserExtensions.TianoCore."ExtraFiles"]
+  ResetSystemPeiExtra.uni
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni
new file mode 100644
index 0000000000..6d2d650c37
--- /dev/null
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni
@@ -0,0 +1,20 @@
+// /** @file
+// This driver implements Reset2, ResetFilter and ResetHandler PPIs.
+//
+// Copyright (c) 2017, Intel 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
+// which 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.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Implements Reset2, ResetFilter and ResetHandler PPIs"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "This driver implements Reset2, ResetFilter and ResetHandler PPIs."
+
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni
new file mode 100644
index 0000000000..2681afc707
--- /dev/null
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni
@@ -0,0 +1,20 @@
+// /** @file
+// ResetSystemPei Localized Strings and Content
+//
+// Copyright (c) 2017, Intel 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
+// which 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.
+//
+// **/
+
+#string STR_PROPERTIES_MODULE_NAME 
+#language en-US 
+"Reset System PEIM"
+
+
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 10/10] MdeModulePkg/ResetSystemPei: Add reset notifications in PEI
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (8 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 09/10] MdeModulePkg: Add ResetSystemPei PEIM Ruiyu Ni
@ 2018-02-09  4:16 ` Ruiyu Ni
  2018-02-09  4:55 ` [PATCH v2 00/10] Formalize the reset system core design Zeng, Star
  10 siblings, 0 replies; 16+ messages in thread
From: Ruiyu Ni @ 2018-02-09  4:16 UTC (permalink / raw)
  To: edk2-devel
  Cc: Bret Barkelew, Liming Gao, Michael D Kinney, Star Zeng,
	Bret Barkelew

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>
---
 .../Include/Ppi/PlatformSpecificResetFilter.h      |  4 +--
 .../Include/Ppi/PlatformSpecificResetHandler.h     |  4 +--
 .../Ppi/PlatformSpecificResetNotification.h        | 32 ++++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                      |  7 +++--
 MdeModulePkg/MdeModulePkg.uni                      |  5 ++--
 .../Universal/ResetSystemPei/ResetSystem.c         | 16 +++++++++++
 .../Universal/ResetSystemPei/ResetSystem.h         |  1 +
 .../Universal/ResetSystemPei/ResetSystemPei.inf    |  7 +++--
 8 files changed, 65 insertions(+), 11 deletions(-)
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h

diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
index 0f1432f5f8..1f728387b7 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
@@ -3,9 +3,9 @@
   for ResetSystem().  A reset filter evaluates the parameters passed to
   ResetSystem() and converts a ResetType of EfiResetPlatformSpecific to a
   non-platform specific reset type.  The registered filters are processed before
-  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI handlers.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI handlers.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
index d5f1350c69..8d938abb02 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
@@ -1,9 +1,9 @@
 /** @file
   This PPI provides services to register a platform specific handler for
   ResetSystem().  The registered handlers are processed after
-  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI notifications.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
new file mode 100644
index 0000000000..cf6d1f18a6
--- /dev/null
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
@@ -0,0 +1,32 @@
+/** @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 and before
+  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI notifications.
+
+  Copyright (c) 2017 - 2018 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 484ad3fb4d..0695ce607e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -447,6 +447,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 } }
 
@@ -1422,8 +1425,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt CapsuleMax value in capsule report variable.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax|0xFFFF|UINT16|0x00000107
 
-  ## Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase.
-  # @Prompt Maximum Number of PEI Reset Filters or Reset Handlers.
+  ## Indicates the allowable maximum number of Reset Filters, Reset Notifications or Reset Handlers in PEI phase.
+  # @Prompt Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies|0x10|UINT32|0x00000109
 
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx]
diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni
index a63d4764a3..d8cc7416c5 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -1059,9 +1059,10 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleMax_HELP  #language en-US "CapsuleMax value in capsule report variable."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_PROMPT  #language en-US "Maximum Number of PEI Reset Filters or Reset Handlers."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_PROMPT  #language en-US "Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers."
 
-#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_HELP  #language en-US "Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase."
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_HELP  #language en-US "Indicates the allowable maximum number of Reset Filters, <BR>\n"
+                                                                                            "Reset Notifications or Reset Handlers in PEI phase."
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_PROMPT  #language en-US "Recover file name in PEI phase"
 
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
index 8d9bd4884a..c2ee592d56 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 f06d39bef5..8e9f872056 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
-- 
2.16.1.windows.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 00/10] Formalize the reset system core design
  2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
                   ` (9 preceding siblings ...)
  2018-02-09  4:16 ` [PATCH v2 10/10] MdeModulePkg/ResetSystemPei: Add reset notifications in PEI Ruiyu Ni
@ 2018-02-09  4:55 ` Zeng, Star
  10 siblings, 0 replies; 16+ messages in thread
From: Zeng, Star @ 2018-02-09  4:55 UTC (permalink / raw)
  To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Zeng, Star

Thanks for the new version patch series.

With minor comments (I provided to V1) handled, Reviewed-by: Star Zeng <star.zeng@intel.com> to the patch series. :)

For patch 007: "@param[in]  ResetType     Base reset type as defined in UEFI spec." needs to be removed in ResetUtilityLib.h

For patch 009: It changed EFI_SOFTWARE_RUNTIME_SERVICE to EFI_SOFTWARE_PEI_SERVICE, but forgets to change 'runtime' to 'PEI' in comments and change EFI_SW_RS_PC_RESET_SYSTEM to EFI_SW_PS_PC_RESET_SYSTEM.


Thanks,
Star
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ruiyu Ni
Sent: Friday, February 9, 2018 12:16 PM
To: edk2-devel@lists.01.org
Subject: [edk2] [PATCH v2 00/10] Formalize the reset system core design

The patches add/update two core modules that perform the reset action
  ResetSystemPei and ResetSystemRuntimeDxe

With the two core modules, every time a reset action is performed in either PEI phase or DXE phase, the accordingly registerred reset filter/notification/handler will be triggered.

Reset filters are processed first so the final reset type and reset data can be determined.  Reset Notifications are processed second so all components that have registered for a Reset Notification can perform any required clean up actions. Reset handlers are processed third.  If there are no registered reset handlers or none of them resets the platform, then the default reset action based on the ResetSystemLib is performed.

The v2 changes against v2 are attached in the end of this mail. 

Bret Barkelew (1):
  MdeModulePkg/ResetSystemPei: Add reset notifications in PEI

Michael D Kinney (6):
  MdePkg/PeiServicesLib: Add PeiServicesResetSystem2()
  MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first
  MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c
  MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler
  MdeModulePkg: Add ResetSystemLib instances that call core services
  MdeModulePkg: Add ResetUtility librray class and BASE instance

Ruiyu Ni (3):
  MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  MdePkg/UefiRuntimeLib: Support more module types.
  MdeModulePkg: Add ResetSystemPei PEIM

 MdeModulePkg/Core/Pei/Reset/Reset.c                |  67 ++--
 MdeModulePkg/Include/Library/ResetUtilityLib.h     | 111 ++++++
 .../Include/Ppi/PlatformSpecificResetFilter.h      |  31 ++
 .../Include/Ppi/PlatformSpecificResetHandler.h     |  29 ++
 .../Ppi/PlatformSpecificResetNotification.h        |  32 ++
 .../Include/Protocol/PlatformSpecificResetFilter.h |  31 ++
 .../Protocol/PlatformSpecificResetHandler.h        |  29 ++
 .../Library/DxeResetSystemLib/DxeResetSystemLib.c  |  98 ++++++
 .../DxeResetSystemLib/DxeResetSystemLib.inf        |  39 +++
 .../DxeResetSystemLib/DxeResetSystemLib.uni        |  21 ++
 .../Library/PeiResetSystemLib/PeiResetSystemLib.c  |  98 ++++++
 .../PeiResetSystemLib/PeiResetSystemLib.inf        |  39 +++
 .../PeiResetSystemLib/PeiResetSystemLib.uni        |  21 ++
 .../Library/ResetUtilityLib/ResetUtility.c         | 220 ++++++++++++
 .../Library/ResetUtilityLib/ResetUtilityLib.inf    |  40 +++
 MdeModulePkg/MdeModulePkg.dec                      |  20 ++
 MdeModulePkg/MdeModulePkg.dsc                      |   7 +
 MdeModulePkg/MdeModulePkg.uni                      |   7 +-
 .../Universal/ResetSystemPei/ResetSystem.c         | 371 +++++++++++++++++++++
 .../Universal/ResetSystemPei/ResetSystem.h         | 130 ++++++++
 .../ResetSystemPei.inf}                            |  45 ++-
 .../Universal/ResetSystemPei/ResetSystemPei.uni    |  20 ++
 .../ResetSystemPei/ResetSystemPeiExtra.uni         |  20 ++
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  |  91 ++++-
 .../Universal/ResetSystemRuntimeDxe/ResetSystem.h  |   7 +
 .../ResetSystemRuntimeDxe.inf                      |   7 +-
 MdePkg/Include/Library/PeiServicesLib.h            |  24 ++
 MdePkg/Library/PeiServicesLib/PeiServicesLib.c     |  26 ++
 MdePkg/Library/UefiRuntimeLib/UefiRuntimeLib.inf   |   4 +-
 29 files changed, 1615 insertions(+), 70 deletions(-)  create mode 100644 MdeModulePkg/Include/Library/ResetUtilityLib.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
 create mode 100644 MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetFilter.h
 create mode 100644 MdeModulePkg/Include/Protocol/PlatformSpecificResetHandler.h
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.uni
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
 create mode 100644 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.uni
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
 create mode 100644 MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
 copy MdeModulePkg/Universal/{ResetSystemRuntimeDxe/ResetSystemRuntimeDxe.inf => ResetSystemPei/ResetSystemPei.inf} (50%)  create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.uni
 create mode 100644 MdeModulePkg/Universal/ResetSystemPei/ResetSystemPeiExtra.uni
diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h

index 0f1432f5f8..1f728387b7 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetFilter.h
@@ -3,9 +3,9 @@
   for ResetSystem().  A reset filter evaluates the parameters passed to
   ResetSystem() and converts a ResetType of EfiResetPlatformSpecific to a
   non-platform specific reset type.  The registered filters are processed before
-  EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI handlers.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI handlers.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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 diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
index d5f1350c69..8d938abb02 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetHandler.h
@@ -1,9 +1,9 @@
 /** @file
   This PPI provides services to register a platform specific handler for
   ResetSystem().  The registered handlers are processed after
-  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications.
+  EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PPI notifications.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 Intel 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 diff --git a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
index ea53e24133..cf6d1f18a6 100644
--- a/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
+++ b/MdeModulePkg/Include/Ppi/PlatformSpecificResetNotification.h
@@ -1,9 +1,10 @@
 /** @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.
+  EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PPI notifications and before  
+ EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PPI notifications.
 
-  Copyright (c) 2017 Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018 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 diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
index 70ee1175d5..ea452e3231 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.c
@@ -1,7 +1,7 @@
 /** @file
   DXE Reset System Library instance that calls gRT->ResetSystem().
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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
   which accompanies this distribution.  The full text of the license may be found at @@ -13,9 +13,7 @@  **/
 
 #include <PiDxe.h>
-
 #include <Library/ResetSystemLib.h>
-#include <Library/DebugLib.h>
 #include <Library/UefiRuntimeLib.h>
 
 /**
diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
index f2e04cfd00..6eb2766b93 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  DXE Reset System Library instance that calls gRT->ResetSystem().
 #
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel 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 @@ -35,6 +35,5 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  DebugLib
   UefiRuntimeLib
 
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
index b7e10110b0..30225d73a5 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.c
@@ -1,7 +1,7 @@
 /** @file
   PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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
   which accompanies this distribution.  The full text of the license may be found at @@ -13,9 +13,7 @@  **/
 
 #include <PiPei.h>
-
 #include <Library/ResetSystemLib.h>
-#include <Library/DebugLib.h>
 #include <Library/PeiServicesLib.h>
 
 /**
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
index e82ec6b2b6..b1b9388c63 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
@@ -1,7 +1,7 @@
 ## @file
 #  PEI Reset System Library instance that calls the ResetSystem2() PEI Service.
 #
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel 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 @@ -35,6 +35,5 @@ [Packages]
   MdeModulePkg/MdeModulePkg.dec
 
 [LibraryClasses]
-  DebugLib
   PeiServicesLib
 
diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
index 5bbe002be0..90de94ca9b 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -37,7 +37,6 @@ typedef struct {
         are not initialized. For DXE, you can add gEfiResetArchProtocolGuid
         to your DEPEX.
 
-  @param[in]  ResetType     Base reset type as defined in UEFI spec.
   @param[in]  ResetSubtype  GUID pointer for the reset subtype to be used.
 
 **/
@@ -90,7 +89,7 @@ GetResetPlatformSpecificGuid (
   }
 
   //
-  // Determine the number of bytes in in the Null-terminated Unicode string
+  // Determine the number of bytes in the Null-terminated Unicode 
+ string
   // at the beginning of ResetData including the Null terminator.
   //
   ResetDataStringSize = StrnSizeS (ResetData, (DataSize / sizeof (CHAR16))); diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
index 7a403749c5..2a4e53a8a6 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtilityLib.inf
@@ -1,10 +1,7 @@
 ## @file
 # This file contains the Reset Utility functions.
 #
-#  The application pops up a menu showing all the boot options referenced by -#  BootOrder NV variable and user can choose to boot from one of them.
-#
-#  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2017 - 2018, Intel Corporation. All rights 
+reserved.<BR>
 #  Copyright (c) 2016, 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 diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 297b02ffa9..0695ce607e 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -54,6 +54,9 @@ [LibraryClasses]
   ##  @libraryclass  Defines a set of methods to reset whole system.
   ResetSystemLib|Include/Library/ResetSystemLib.h
 
+  ##  @libraryclass  Defines a set of helper functions for resetting the system.
+  ResetUtilityLib|Include/Library/ResetUtilityLib.h
+
   ##  @libraryclass  Defines a set of methods related do S3 mode.
   #   This library class is no longer used and modules using this library should
   #   directly locate EFI_PEI_S3_RESUME_PPI defined in PI 1.2 specification.
@@ -1422,8 +1425,8 @@ [PcdsFixedAtBuild, PcdsPatchableInModule]
   # @Prompt CapsuleMax value in capsule report variable.
   gEfiMdeModulePkgTokenSpaceGuid.PcdCapsuleMax|0xFFFF|UINT16|0x00000107
 
-  ## Indicates the allowable maximum number of Reset Filters or Reset Handlers in PEI phase.
-  # @Prompt Maximum Number of PEI Reset Filters or Reset Handlers.
+  ## Indicates the allowable maximum number of Reset Filters, Reset Notifications or Reset Handlers in PEI phase.
+  # @Prompt Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers.
   gEfiMdeModulePkgTokenSpaceGuid.PcdMaximumPeiResetNotifies|0x10|UINT32|0x00000109
 
 [PcdsPatchableInModule, PcdsDynamic, PcdsDynamicEx] diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 0e068422e4..d8cc7416c5 100644
--- a/MdeModulePkg/MdeModulePkg.uni
+++ b/MdeModulePkg/MdeModulePkg.uni
@@ -4,7 +4,7 @@
 // It also provides the definitions(including PPIs/PROTOCOLs/GUIDs and library classes)  // and libraries instances, which are used for those modules.
 //
-// Copyright (c) 2007 - 2016, Intel Corporation. All rights reserved.<BR>
+// Copyright (c) 2007 - 2018, Intel 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.
@@ -1059,6 +1059,11 @@
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdCapsuleMax_HELP  #language en-US "CapsuleMax value in capsule report variable."
 
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_PROMPT  #language en-US "Maximum Number of PEI Reset Filters, Reset Notifications or Reset Handlers."
+
+#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdMaximumPeiResetNotifies_HELP  #language en-US "Indicates the allowable maximum number of Reset Filters, <BR>\n"
+                                                                                            "Reset Notifications or Reset Handlers in PEI phase."
+
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_PROMPT  #language en-US "Recover file name in PEI phase"
 
 #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdRecoveryFileName_HELP  #language en-US "This is recover file name in PEI phase.\n"
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
index 4dfe303f77..c2ee592d56 100644
--- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.c
@@ -80,13 +80,13 @@ EFI_PEI_PPI_DESCRIPTOR mPpiListReset[] = {
   Register a notification function to be called when ResetSystem() is called.
 
   The RegisterResetNotify() function registers a notification function that is called when
-  ResetSystem()is called and prior to completing the reset of the platform.
+  ResetSystem() is called and prior to completing the reset of the platform.
   The registered functions must not perform a platform reset themselves. These
   notifications are intended only for the notification of components which may need some
   special-purpose maintenance prior to the platform resetting.
   The list of registered reset notification functions are processed if ResetSystem()is called
   before ExitBootServices(). The list of registered reset notification functions is ignored if
-  ResetSystem()is called after ExitBootServices().
+  ResetSystem() is called after ExitBootServices().
 
   @param[in]  This              A pointer to the EFI_RESET_NOTIFICATION_PROTOCOL instance.
   @param[in]  ResetFunction     Points to the function to be called when a ResetSystem() is executed.
@@ -312,7 +312,7 @@ ResetSystem2 (
     //
     // Indicate reset system runtime service is called.
     //
-    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_EFI_RUNTIME_SERVICE | EFI_SW_RS_PC_RESET_SYSTEM));
+    REPORT_STATUS_CODE (EFI_PROGRESS_CODE, (EFI_SOFTWARE_PEI_SERVICE | 
+ EFI_SW_RS_PC_RESET_SYSTEM));
   }
 
   //
diff --git a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
index b623a4c381..8e9f872056 100644
--- a/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
+++ b/MdeModulePkg/Universal/ResetSystemPei/ResetSystem.h
@@ -1,6 +1,6 @@
 /** @file
 
-  Copyright (c) 2017, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2017 - 2018, Intel 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 @@ -34,7 +34,7 @@
 
 
 //
-// The maximum recurstion depth to ResetSystem() by reset notification handlers
+// The maximum recursion depth to ResetSystem() by reset notification 
+handlers
 //
 #define MAX_RESET_NOTIFY_DEPTH 10
 
diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index 4b5af76999..2c795426f5 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -294,7 +294,7 @@ ResetSystem (
       }
       //
       // call reset notification functions registered through the 
-      // EDKII_PLATFORM_SPECIFIC_RESET_NOTIFICATION_PROTOCOL.
+      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
       //
       for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
           ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)

--
2.16.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  2018-02-09  4:16 ` [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message Ruiyu Ni
@ 2018-02-19 18:59   ` Ard Biesheuvel
  2018-02-19 19:23     ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Ard Biesheuvel @ 2018-02-19 18:59 UTC (permalink / raw)
  To: Ruiyu Ni; +Cc: edk2-devel@lists.01.org, Michael D Kinney, Liming Gao

On 9 February 2018 at 04:16, Ruiyu Ni <ruiyu.ni@intel.com> wrote:
> The patch adds more debug message in ResetSystem().
> It also removes unnecessary check of mResetNotifyDepth.
>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Reviewed-by: Star Zeng <star.zeng@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> ---
>  .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 88 +++++++++++-----------
>  1 file changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
> index fed527fac2..2c795426f5 100644
> --- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
> +++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
> @@ -15,6 +15,10 @@
>
>  #include "ResetSystem.h"
>
> +GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mResetTypeStr[] = {
> +  L"Cold", L"Warm", L"Shutdown", L"PlatformSpecific"
> +};
> +
>  //
>  // The current ResetSystem() notification recursion depth
>  //
> @@ -251,16 +255,6 @@ ResetSystem (
>    LIST_ENTRY          *Link;
>    RESET_NOTIFY_ENTRY  *Entry;
>
> -  //
> -  // Above the maximum recursion depth, so do the smallest amount of
> -  // work to perform a cold reset.
> -  //
> -  if (mResetNotifyDepth >= MAX_RESET_NOTIFY_DEPTH) {
> -    ResetCold ();
> -    ASSERT (FALSE);
> -    return;
> -  }
> -
>    //
>    // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
>    //
> @@ -272,40 +266,47 @@ ResetSystem (
>    }
>
>    mResetNotifyDepth++;
> -  if (!EfiAtRuntime () && mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH) {
> -    //
> -    // Call reset notification functions registered through the
> -    // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
> -    //
> -    for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
> -        ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
> -        ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
> -        ) {
> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> -    }
> -    //
> -    // Call reset notification functions registered through the
> -    // EFI_RESET_NOTIFICATION_PROTOCOL.
> -    //
> -    for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
> -        ; !IsNull (&mResetNotification.ResetNotifies, Link)
> -        ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
> -        ) {
> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> -    }
> -    //
> -    // call reset notification functions registered through the
> -    // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
> -    //
> -    for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
> -        ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
> -        ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
> -        ) {
> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> +  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
> +

This DEBUG() print is breaking system reset from the Linux OS at
runtime in DEBUG builds.

[    4.223704] reboot: Restarting system
[    4.224733] Unable to handle kernel paging request at virtual
address 09000018

This is the boottime MMIO address of the UART on the QEMU mach-virt
model, and no runtime mapping exists for it, resulting in the crash.

Please ensure that DEBUG () is used with care in DXE_RUNTIME_DRIVER modules.

Thanks,
Ard.

(remainder of the crash log below)


[    4.226725] Mem abort info:
[    4.227564]   Exception class = DABT (current EL), IL = 32 bits
[    4.229329]   SET = 0, FnV = 0
[    4.230433]   EA = 0, S1PTW = 0
[    4.232230] Data abort info:
[    4.233125]   ISV = 0, ISS = 0x00000006
[    4.234235]   CM = 0, WnR = 0
[    4.235178] user pgtable: 4k pages, 48-bit VAs, pgd = ffff80003aa97000
[    4.237687] [0000000009000018] *pgd=0000000000000000
[    4.239807] Internal error: Oops: 96000006 [#1] PREEMPT SMP
[    4.242203] Modules linked in:
[    4.243557] CPU: 0 PID: 2016 Comm: reboot Not tainted
4.14.20-rc1-01878-gd73f37b7c835-dirty #2100
[    4.246771] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
[    4.249075] task: ffff80003a9cd400 task.stack: ffff00000cfe8000
[    4.251350] PC is at 0x242701fc
[    4.252903] LR is at 0x242701d4
[    4.254054] pc : [<00000000242701fc>] lr : [<00000000242701d4>]
pstate: 400001c5
[    4.256417] sp : ffff00000cfeba60
[    4.257444] x29: ffff00000cfeba60 x28: ffff80003a9cd400
[    4.259094] x27: ffff0000089e1000 x26: 000000000000008e
[    4.260729] x25: 0000000000000124 x24: 0000000000000000
[    4.262379] x23: 0000000000000000 x22: 0000000009000018
[    4.264017] x21: 0000000009000000 x20: ffff00000cfebb39
[    4.265661] x19: 0000000009000018 x18: 0000000000000010
[    4.267288] x17: 0000ffff8bebbe00 x16: ffff000008246818
[    4.268916] x15: 00000000000000ff x14: 0000000000000001
[    4.270560] x13: 0000000000000002 x12: 0000000000000002
[    4.272271] x11: 0000000000000000 x10: 0000000000000002
[    4.274010] x9 : 0000000000000000 x8 : 0000000000000002
[    4.275640] x7 : 0000000024272850 x6 : 0000000000000000
[    4.277297] x5 : 0000000000000001 x4 : 0000000000000001
[    4.280108] x3 : 0000000000000000 x2 : 0000000000000001
[    4.282505] x1 : ffff00000cfebb10 x0 : 0000000000000001



> +  if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
> +    if (!EfiAtRuntime ()) {
> +      //
> +      // Call reset notification functions registered through the
> +      // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
> +      //
> +      for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
> +          ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
> +          ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
> +          ) {
> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> +      }
> +      //
> +      // Call reset notification functions registered through the
> +      // EFI_RESET_NOTIFICATION_PROTOCOL.
> +      //
> +      for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
> +          ; !IsNull (&mResetNotification.ResetNotifies, Link)
> +          ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
> +          ) {
> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> +      }
> +      //
> +      // call reset notification functions registered through the
> +      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
> +      //
> +      for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
> +          ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
> +          ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
> +          ) {
> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
> +      }
>      }
> +  } else {
> +    ASSERT (ResetType < ARRAY_SIZE (mResetTypeStr));
> +    DEBUG ((DEBUG_ERROR, "DXE ResetSystem2: Maximum reset call depth is met. Use the current reset type: %s!\n", mResetTypeStr[ResetType]));
>    }
>
>    switch (ResetType) {
> @@ -331,7 +332,6 @@ ResetSystem (
>      }
>
>      ResetWarm ();
> -
>      break;
>
>   case EfiResetCold:
> --
> 2.16.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  2018-02-19 18:59   ` Ard Biesheuvel
@ 2018-02-19 19:23     ` Laszlo Ersek
  2018-02-19 23:30       ` Andrew Fish
  0 siblings, 1 reply; 16+ messages in thread
From: Laszlo Ersek @ 2018-02-19 19:23 UTC (permalink / raw)
  To: Ard Biesheuvel, Ruiyu Ni
  Cc: Michael D Kinney, edk2-devel@lists.01.org, Liming Gao

On 02/19/18 19:59, Ard Biesheuvel wrote:
> On 9 February 2018 at 04:16, Ruiyu Ni <ruiyu.ni@intel.com> wrote:
>> The patch adds more debug message in ResetSystem().
>> It also removes unnecessary check of mResetNotifyDepth.
>>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Reviewed-by: Star Zeng <star.zeng@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> ---
>>  .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 88 +++++++++++-----------
>>  1 file changed, 44 insertions(+), 44 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>> index fed527fac2..2c795426f5 100644
>> --- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>> +++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>> @@ -15,6 +15,10 @@
>>
>>  #include "ResetSystem.h"
>>
>> +GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mResetTypeStr[] = {
>> +  L"Cold", L"Warm", L"Shutdown", L"PlatformSpecific"
>> +};
>> +
>>  //
>>  // The current ResetSystem() notification recursion depth
>>  //
>> @@ -251,16 +255,6 @@ ResetSystem (
>>    LIST_ENTRY          *Link;
>>    RESET_NOTIFY_ENTRY  *Entry;
>>
>> -  //
>> -  // Above the maximum recursion depth, so do the smallest amount of
>> -  // work to perform a cold reset.
>> -  //
>> -  if (mResetNotifyDepth >= MAX_RESET_NOTIFY_DEPTH) {
>> -    ResetCold ();
>> -    ASSERT (FALSE);
>> -    return;
>> -  }
>> -
>>    //
>>    // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
>>    //
>> @@ -272,40 +266,47 @@ ResetSystem (
>>    }
>>
>>    mResetNotifyDepth++;
>> -  if (!EfiAtRuntime () && mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH) {
>> -    //
>> -    // Call reset notification functions registered through the
>> -    // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
>> -    //
>> -    for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
>> -        ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>> -        ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>> -        ) {
>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> -    }
>> -    //
>> -    // Call reset notification functions registered through the
>> -    // EFI_RESET_NOTIFICATION_PROTOCOL.
>> -    //
>> -    for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
>> -        ; !IsNull (&mResetNotification.ResetNotifies, Link)
>> -        ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
>> -        ) {
>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> -    }
>> -    //
>> -    // call reset notification functions registered through the
>> -    // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
>> -    //
>> -    for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
>> -        ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>> -        ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>> -        ) {
>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> +  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
>> +
> 
> This DEBUG() print is breaking system reset from the Linux OS at
> runtime in DEBUG builds.
> 
> [    4.223704] reboot: Restarting system
> [    4.224733] Unable to handle kernel paging request at virtual
> address 09000018
> 
> This is the boottime MMIO address of the UART on the QEMU mach-virt
> model, and no runtime mapping exists for it, resulting in the crash.
> 
> Please ensure that DEBUG () is used with care in DXE_RUNTIME_DRIVER modules.

Not disagreeing, just asking: should we perhaps take care of this in a
new DebugLib instance, specifically for DXE runtime drivers?

"MdePkg/Library/UefiRuntimeLib" provides functions like EfiAtRuntime()
and EfiGoneVirtual(). We couldn't use UefiRuntimeLib in DebugLib,
because UefiRuntimeLib already depends on DebugLib (we can't introduce a
circular dependency). But, we could reimplement EfiAtRuntime() manually,
in order to silence all debug messages after ExitBootServices().

This would make sense also because after ExitBootServices(), the serial
port is considered "owned" by the boot loader or the OS, and the
firmware should likely not mess up whatever IO occurs there.

I guess the two possible places to implement such runtime logic would be:

- in a RuntimeDxe clone of BaseDebugLibSerialPort (i.e., commonly for
all edk2 platforms),

- in a RuntimeDxe clone of
"ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf"
(i.e., move the checking to the serial port lib level).

(This is different from OVMF / x86, because (a) there the debug data are
written to IO port 0x402, and the IO address space does not depend on
paging, (b) largely, no boot loader or OS ever are aware of the QEMU
debug port, it can be considered as owned by the firmware, always.)

Just thinking out loud.

Thanks!
Laszlo


> (remainder of the crash log below)
> 
> 
> [    4.226725] Mem abort info:
> [    4.227564]   Exception class = DABT (current EL), IL = 32 bits
> [    4.229329]   SET = 0, FnV = 0
> [    4.230433]   EA = 0, S1PTW = 0
> [    4.232230] Data abort info:
> [    4.233125]   ISV = 0, ISS = 0x00000006
> [    4.234235]   CM = 0, WnR = 0
> [    4.235178] user pgtable: 4k pages, 48-bit VAs, pgd = ffff80003aa97000
> [    4.237687] [0000000009000018] *pgd=0000000000000000
> [    4.239807] Internal error: Oops: 96000006 [#1] PREEMPT SMP
> [    4.242203] Modules linked in:
> [    4.243557] CPU: 0 PID: 2016 Comm: reboot Not tainted
> 4.14.20-rc1-01878-gd73f37b7c835-dirty #2100
> [    4.246771] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
> [    4.249075] task: ffff80003a9cd400 task.stack: ffff00000cfe8000
> [    4.251350] PC is at 0x242701fc
> [    4.252903] LR is at 0x242701d4
> [    4.254054] pc : [<00000000242701fc>] lr : [<00000000242701d4>]
> pstate: 400001c5
> [    4.256417] sp : ffff00000cfeba60
> [    4.257444] x29: ffff00000cfeba60 x28: ffff80003a9cd400
> [    4.259094] x27: ffff0000089e1000 x26: 000000000000008e
> [    4.260729] x25: 0000000000000124 x24: 0000000000000000
> [    4.262379] x23: 0000000000000000 x22: 0000000009000018
> [    4.264017] x21: 0000000009000000 x20: ffff00000cfebb39
> [    4.265661] x19: 0000000009000018 x18: 0000000000000010
> [    4.267288] x17: 0000ffff8bebbe00 x16: ffff000008246818
> [    4.268916] x15: 00000000000000ff x14: 0000000000000001
> [    4.270560] x13: 0000000000000002 x12: 0000000000000002
> [    4.272271] x11: 0000000000000000 x10: 0000000000000002
> [    4.274010] x9 : 0000000000000000 x8 : 0000000000000002
> [    4.275640] x7 : 0000000024272850 x6 : 0000000000000000
> [    4.277297] x5 : 0000000000000001 x4 : 0000000000000001
> [    4.280108] x3 : 0000000000000000 x2 : 0000000000000001
> [    4.282505] x1 : ffff00000cfebb10 x0 : 0000000000000001
> 
> 
> 
>> +  if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
>> +    if (!EfiAtRuntime ()) {
>> +      //
>> +      // Call reset notification functions registered through the
>> +      // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
>> +      //
>> +      for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
>> +          ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>> +          ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>> +          ) {
>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> +      }
>> +      //
>> +      // Call reset notification functions registered through the
>> +      // EFI_RESET_NOTIFICATION_PROTOCOL.
>> +      //
>> +      for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
>> +          ; !IsNull (&mResetNotification.ResetNotifies, Link)
>> +          ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
>> +          ) {
>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> +      }
>> +      //
>> +      // call reset notification functions registered through the
>> +      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
>> +      //
>> +      for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
>> +          ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>> +          ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>> +          ) {
>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>> +      }
>>      }
>> +  } else {
>> +    ASSERT (ResetType < ARRAY_SIZE (mResetTypeStr));
>> +    DEBUG ((DEBUG_ERROR, "DXE ResetSystem2: Maximum reset call depth is met. Use the current reset type: %s!\n", mResetTypeStr[ResetType]));
>>    }
>>
>>    switch (ResetType) {
>> @@ -331,7 +332,6 @@ ResetSystem (
>>      }
>>
>>      ResetWarm ();
>> -
>>      break;
>>
>>   case EfiResetCold:
>> --
>> 2.16.1.windows.1
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
> 



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  2018-02-19 19:23     ` Laszlo Ersek
@ 2018-02-19 23:30       ` Andrew Fish
  2018-02-20  8:53         ` Laszlo Ersek
  0 siblings, 1 reply; 16+ messages in thread
From: Andrew Fish @ 2018-02-19 23:30 UTC (permalink / raw)
  To: Laszlo Ersek
  Cc: Ard Biesheuvel, Ruiyu Ni, Mike Kinney, edk2-devel@lists.01.org,
	Liming Gao



> On Feb 19, 2018, at 11:23 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> 
> On 02/19/18 19:59, Ard Biesheuvel wrote:
>> On 9 February 2018 at 04:16, Ruiyu Ni <ruiyu.ni@intel.com> wrote:
>>> The patch adds more debug message in ResetSystem().
>>> It also removes unnecessary check of mResetNotifyDepth.
>>> 
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Reviewed-by: Star Zeng <star.zeng@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
>>> ---
>>> .../Universal/ResetSystemRuntimeDxe/ResetSystem.c  | 88 +++++++++++-----------
>>> 1 file changed, 44 insertions(+), 44 deletions(-)
>>> 
>>> diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>>> index fed527fac2..2c795426f5 100644
>>> --- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>>> +++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
>>> @@ -15,6 +15,10 @@
>>> 
>>> #include "ResetSystem.h"
>>> 
>>> +GLOBAL_REMOVE_IF_UNREFERENCED CHAR16 *mResetTypeStr[] = {
>>> +  L"Cold", L"Warm", L"Shutdown", L"PlatformSpecific"
>>> +};
>>> +
>>> //
>>> // The current ResetSystem() notification recursion depth
>>> //
>>> @@ -251,16 +255,6 @@ ResetSystem (
>>>   LIST_ENTRY          *Link;
>>>   RESET_NOTIFY_ENTRY  *Entry;
>>> 
>>> -  //
>>> -  // Above the maximum recursion depth, so do the smallest amount of
>>> -  // work to perform a cold reset.
>>> -  //
>>> -  if (mResetNotifyDepth >= MAX_RESET_NOTIFY_DEPTH) {
>>> -    ResetCold ();
>>> -    ASSERT (FALSE);
>>> -    return;
>>> -  }
>>> -
>>>   //
>>>   // Only do REPORT_STATUS_CODE() on first call to ResetSystem()
>>>   //
>>> @@ -272,40 +266,47 @@ ResetSystem (
>>>   }
>>> 
>>>   mResetNotifyDepth++;
>>> -  if (!EfiAtRuntime () && mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH) {
>>> -    //
>>> -    // Call reset notification functions registered through the
>>> -    // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
>>> -    //
>>> -    for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
>>> -        ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>>> -        ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>>> -        ) {
>>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> -    }
>>> -    //
>>> -    // Call reset notification functions registered through the
>>> -    // EFI_RESET_NOTIFICATION_PROTOCOL.
>>> -    //
>>> -    for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
>>> -        ; !IsNull (&mResetNotification.ResetNotifies, Link)
>>> -        ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
>>> -        ) {
>>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> -    }
>>> -    //
>>> -    // call reset notification functions registered through the
>>> -    // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
>>> -    //
>>> -    for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
>>> -        ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>>> -        ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>>> -        ) {
>>> -      Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> -      Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> +  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
>>> +
>> 
>> This DEBUG() print is breaking system reset from the Linux OS at
>> runtime in DEBUG builds.
>> 
>> [    4.223704] reboot: Restarting system
>> [    4.224733] Unable to handle kernel paging request at virtual
>> address 09000018
>> 
>> This is the boottime MMIO address of the UART on the QEMU mach-virt
>> model, and no runtime mapping exists for it, resulting in the crash.
>> 
>> Please ensure that DEBUG () is used with care in DXE_RUNTIME_DRIVER modules.
> 
> Not disagreeing, just asking: should we perhaps take care of this in a
> new DebugLib instance, specifically for DXE runtime drivers?
> 
> "MdePkg/Library/UefiRuntimeLib" provides functions like EfiAtRuntime()
> and EfiGoneVirtual(). We couldn't use UefiRuntimeLib in DebugLib,
> because UefiRuntimeLib already depends on DebugLib (we can't introduce a
> circular dependency). But, we could reimplement EfiAtRuntime() manually,
> in order to silence all debug messages after ExitBootServices().
> 
> This would make sense also because after ExitBootServices(), the serial
> port is considered "owned" by the boot loader or the OS, and the
> firmware should likely not mess up whatever IO occurs there.
> 
> I guess the two possible places to implement such runtime logic would be:
> 
> - in a RuntimeDxe clone of BaseDebugLibSerialPort (i.e., commonly for
> all edk2 platforms),
> 
> - in a RuntimeDxe clone of
> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf"
> (i.e., move the checking to the serial port lib level).
> 
> (This is different from OVMF / x86, because (a) there the debug data are
> written to IO port 0x402, and the IO address space does not depend on
> paging, (b) largely, no boot loader or OS ever are aware of the QEMU
> debug port, it can be considered as owned by the firmware, always.)
> 
> Just thinking out loud.
> 

Laszlo,

>From a Pedantic point of view an EFI Runtime Service can only use hardware not exposed to OS as there is no clean way to share. There are some scary wiggle words about the RTC that date all the way back to EFI 1.1, and that is the only conformant exception. So that is probably why we don't have a generic solution as it is kind of dangerous. For example what happens if the OS has a kernel debugger running on that serial port and EFI Spews DEBUG prints, that would probably not come out well. 

For things I've written I usually end up writing a macro that does something like:

if (!EfiAtRuntime ()) {
  DEBUG ((DEBUG_ERROR, "Hello World!"));
}

and

if (!EfiAtRuntime ()) {
  ASSERT (FALSE);
}

Maybe it would possible to add a RUNTIME_DEBUG(), RUNTIME_ASSERT(), etc. macros to the UefiRuntimeLib? 


Makes me remember the story from back in the 1990's about and update to the Windows Plug-N-Play subsystem to auto magically detect modems. It worked great, and made it easier to get folks online (even if it was very slow), but seems a software update managed to destroy a very very expensive custom milling machine. It seems this milling machine was connected to the serial port of the PC, and it was a very dumb device as it interpreted data across the serial port as coordinates and commands, and all the error checking was done on the PC. So these random data on the serial line told the milling machine to attack its self. 

Thanks,

Andrew Fish

> Thanks!
> Laszlo
> 
> 
>> (remainder of the crash log below)
>> 
>> 
>> [    4.226725] Mem abort info:
>> [    4.227564]   Exception class = DABT (current EL), IL = 32 bits
>> [    4.229329]   SET = 0, FnV = 0
>> [    4.230433]   EA = 0, S1PTW = 0
>> [    4.232230] Data abort info:
>> [    4.233125]   ISV = 0, ISS = 0x00000006
>> [    4.234235]   CM = 0, WnR = 0
>> [    4.235178] user pgtable: 4k pages, 48-bit VAs, pgd = ffff80003aa97000
>> [    4.237687] [0000000009000018] *pgd=0000000000000000
>> [    4.239807] Internal error: Oops: 96000006 [#1] PREEMPT SMP
>> [    4.242203] Modules linked in:
>> [    4.243557] CPU: 0 PID: 2016 Comm: reboot Not tainted
>> 4.14.20-rc1-01878-gd73f37b7c835-dirty #2100
>> [    4.246771] Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
>> [    4.249075] task: ffff80003a9cd400 task.stack: ffff00000cfe8000
>> [    4.251350] PC is at 0x242701fc
>> [    4.252903] LR is at 0x242701d4
>> [    4.254054] pc : [<00000000242701fc>] lr : [<00000000242701d4>]
>> pstate: 400001c5
>> [    4.256417] sp : ffff00000cfeba60
>> [    4.257444] x29: ffff00000cfeba60 x28: ffff80003a9cd400
>> [    4.259094] x27: ffff0000089e1000 x26: 000000000000008e
>> [    4.260729] x25: 0000000000000124 x24: 0000000000000000
>> [    4.262379] x23: 0000000000000000 x22: 0000000009000018
>> [    4.264017] x21: 0000000009000000 x20: ffff00000cfebb39
>> [    4.265661] x19: 0000000009000018 x18: 0000000000000010
>> [    4.267288] x17: 0000ffff8bebbe00 x16: ffff000008246818
>> [    4.268916] x15: 00000000000000ff x14: 0000000000000001
>> [    4.270560] x13: 0000000000000002 x12: 0000000000000002
>> [    4.272271] x11: 0000000000000000 x10: 0000000000000002
>> [    4.274010] x9 : 0000000000000000 x8 : 0000000000000002
>> [    4.275640] x7 : 0000000024272850 x6 : 0000000000000000
>> [    4.277297] x5 : 0000000000000001 x4 : 0000000000000001
>> [    4.280108] x3 : 0000000000000000 x2 : 0000000000000001
>> [    4.282505] x1 : ffff00000cfebb10 x0 : 0000000000000001
>> 
>> 
>> 
>>> +  if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
>>> +    if (!EfiAtRuntime ()) {
>>> +      //
>>> +      // Call reset notification functions registered through the
>>> +      // EDKII_PLATFORM_SPECIFIC_RESET_FILTER_PROTOCOL.
>>> +      //
>>> +      for ( Link = GetFirstNode (&mPlatformSpecificResetFilter.ResetNotifies)
>>> +          ; !IsNull (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>>> +          ; Link = GetNextNode (&mPlatformSpecificResetFilter.ResetNotifies, Link)
>>> +          ) {
>>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> +      }
>>> +      //
>>> +      // Call reset notification functions registered through the
>>> +      // EFI_RESET_NOTIFICATION_PROTOCOL.
>>> +      //
>>> +      for ( Link = GetFirstNode (&mResetNotification.ResetNotifies)
>>> +          ; !IsNull (&mResetNotification.ResetNotifies, Link)
>>> +          ; Link = GetNextNode (&mResetNotification.ResetNotifies, Link)
>>> +          ) {
>>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> +      }
>>> +      //
>>> +      // call reset notification functions registered through the
>>> +      // EDKII_PLATFORM_SPECIFIC_RESET_HANDLER_PROTOCOL.
>>> +      //
>>> +      for ( Link = GetFirstNode (&mPlatformSpecificResetHandler.ResetNotifies)
>>> +          ; !IsNull (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>>> +          ; Link = GetNextNode (&mPlatformSpecificResetHandler.ResetNotifies, Link)
>>> +          ) {
>>> +        Entry = RESET_NOTIFY_ENTRY_FROM_LINK (Link);
>>> +        Entry->ResetNotify (ResetType, ResetStatus, DataSize, ResetData);
>>> +      }
>>>     }
>>> +  } else {
>>> +    ASSERT (ResetType < ARRAY_SIZE (mResetTypeStr));
>>> +    DEBUG ((DEBUG_ERROR, "DXE ResetSystem2: Maximum reset call depth is met. Use the current reset type: %s!\n", mResetTypeStr[ResetType]));
>>>   }
>>> 
>>>   switch (ResetType) {
>>> @@ -331,7 +332,6 @@ ResetSystem (
>>>     }
>>> 
>>>     ResetWarm ();
>>> -
>>>     break;
>>> 
>>>  case EfiResetCold:
>>> --
>>> 2.16.1.windows.1
>>> 
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>> 
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message
  2018-02-19 23:30       ` Andrew Fish
@ 2018-02-20  8:53         ` Laszlo Ersek
  0 siblings, 0 replies; 16+ messages in thread
From: Laszlo Ersek @ 2018-02-20  8:53 UTC (permalink / raw)
  To: Andrew Fish
  Cc: Ard Biesheuvel, Ruiyu Ni, Mike Kinney, edk2-devel@lists.01.org,
	Liming Gao

On 02/20/18 00:30, Andrew Fish wrote:
>
>
>> On Feb 19, 2018, at 11:23 AM, Laszlo Ersek <lersek@redhat.com> wrote:
>>
>> On 02/19/18 19:59, Ard Biesheuvel wrote:
>>> On 9 February 2018 at 04:16, Ruiyu Ni <ruiyu.ni@intel.com> wrote:

>>>> +  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
>>>> +
>>>
>>> This DEBUG() print is breaking system reset from the Linux OS at
>>> runtime in DEBUG builds.
>>>
>>> [    4.223704] reboot: Restarting system
>>> [    4.224733] Unable to handle kernel paging request at virtual
>>> address 09000018
>>>
>>> This is the boottime MMIO address of the UART on the QEMU mach-virt
>>> model, and no runtime mapping exists for it, resulting in the crash.
>>>
>>> Please ensure that DEBUG () is used with care in DXE_RUNTIME_DRIVER
>>> modules.
>>
>> Not disagreeing, just asking: should we perhaps take care of this in
>> a new DebugLib instance, specifically for DXE runtime drivers?
>>
>> "MdePkg/Library/UefiRuntimeLib" provides functions like
>> EfiAtRuntime() and EfiGoneVirtual(). We couldn't use UefiRuntimeLib
>> in DebugLib, because UefiRuntimeLib already depends on DebugLib (we
>> can't introduce a circular dependency). But, we could reimplement
>> EfiAtRuntime() manually, in order to silence all debug messages after
>> ExitBootServices().
>>
>> This would make sense also because after ExitBootServices(), the
>> serial port is considered "owned" by the boot loader or the OS, and
>> the firmware should likely not mess up whatever IO occurs there.
>>
>> I guess the two possible places to implement such runtime logic would
>> be:
>>
>> - in a RuntimeDxe clone of BaseDebugLibSerialPort (i.e., commonly for
>> all edk2 platforms),
>>
>> - in a RuntimeDxe clone of
>> "ArmVirtPkg/Library/FdtPL011SerialPortLib/FdtPL011SerialPortLib.inf"
>> (i.e., move the checking to the serial port lib level).
>>
>> (This is different from OVMF / x86, because (a) there the debug data
>> are written to IO port 0x402, and the IO address space does not
>> depend on paging, (b) largely, no boot loader or OS ever are aware of
>> the QEMU debug port, it can be considered as owned by the firmware,
>> always.)
>>
>> Just thinking out loud.
>>
>
> Laszlo,
>
> From a Pedantic point of view an EFI Runtime Service can only use
> hardware not exposed to OS as there is no clean way to share. There
> are some scary wiggle words about the RTC that date all the way back
> to EFI 1.1, and that is the only conformant exception. So that is
> probably why we don't have a generic solution as it is kind of
> dangerous.

I think a DebugLib instance located at

  MdePkg/Library/DxeRuntimeDebugLibSerialPort

could be general enough, since it would not share hardware with the OS
-- it would stop runtime DXE drivers from making SerialPortLib calls.

> For example what happens if the OS has a kernel debugger running on
> that serial port and EFI Spews DEBUG prints, that would probably not
> come out well.
>
> For things I've written I usually end up writing a macro that does
> something like:
>
> if (!EfiAtRuntime ()) {
>   DEBUG ((DEBUG_ERROR, "Hello World!"));
> }

Right, so this supports Ard's original idea, namely that we should
disable the DEBUGs in the client code, one way or another...

> and
>
> if (!EfiAtRuntime ()) {
>   ASSERT (FALSE);
> }

... On the other hand, I think only the debug message should be
suppressed for ASSERTs; the exception or deadloop (whatever the assert
disposition) should not be suppressed at runtime. If the firmware
encounters a fatal unexpected error, it's better to hang the system
(with the deadloop) or crash it (raise an exception and make the kernel
panic) than silently corrupt more state and pretend everything's fine.
So wrapping "ASSERT (Predicate)" with "if (!EfiAtRuntime ())" does not
seem like the best solution to me.

> Maybe it would possible to add a RUNTIME_DEBUG(), RUNTIME_ASSERT(),
> etc. macros to the UefiRuntimeLib?

> Makes me remember the story from back in the 1990's about and update
> to the Windows Plug-N-Play subsystem to auto magically detect modems.
> It worked great, and made it easier to get folks online (even if it
> was very slow), but seems a software update managed to destroy a very
> very expensive custom milling machine. It seems this milling machine
> was connected to the serial port of the PC, and it was a very dumb
> device as it interpreted data across the serial port as coordinates
> and commands, and all the error checking was done on the PC. So these
> random data on the serial line told the milling machine to attack its
> self.

A "winmodem" on steroids :)

Thanks!
Laszlo


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2018-02-20  8:48 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-02-09  4:16 [PATCH v2 00/10] Formalize the reset system core design Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 01/10] MdePkg/PeiServicesLib: Add PeiServicesResetSystem2() Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 02/10] MdeModulePkg/PeiMain: Always attempt to use Reset2 PPI first Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 03/10] MdeModulePkg/PeiMain: Cleanup whitespace in Reset.c Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 04/10] MdeModulePkg/ResetSystemRuntimeDxe: Add platform filter and handler Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message Ruiyu Ni
2018-02-19 18:59   ` Ard Biesheuvel
2018-02-19 19:23     ` Laszlo Ersek
2018-02-19 23:30       ` Andrew Fish
2018-02-20  8:53         ` Laszlo Ersek
2018-02-09  4:16 ` [PATCH v2 06/10] MdeModulePkg: Add ResetSystemLib instances that call core services Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 07/10] MdeModulePkg: Add ResetUtility librray class and BASE instance Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 08/10] MdePkg/UefiRuntimeLib: Support more module types Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 09/10] MdeModulePkg: Add ResetSystemPei PEIM Ruiyu Ni
2018-02-09  4:16 ` [PATCH v2 10/10] MdeModulePkg/ResetSystemPei: Add reset notifications in PEI Ruiyu Ni
2018-02-09  4:55 ` [PATCH v2 00/10] Formalize the reset system core design Zeng, Star

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox