[v1] Formalize the reset system core design

[edk2] [PATCH 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message

Posted by Ruiyu Ni 6 years, 9 months ago

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>
Cc: 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 43400e1338..4b5af76999 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_NOTIFICATION_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_NOTIFICATION_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.15.1.windows.2

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

Re: [edk2] [PATCH 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message

Posted by Zeng, Star 6 years, 8 months ago

On 2018/2/2 14:45, Ruiyu Ni 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>
> Cc: 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 43400e1338..4b5af76999 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_NOTIFICATION_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) {

Should be mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH?

Thanks,
Star

> +    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_NOTIFICATION_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:
> 

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

Re: [edk2] [PATCH 05/10] MdeModulePkg/ResetSystemRuntimeDxe: Add more debug message

Posted by Ni, Ruiyu 6 years, 8 months ago

On 2/7/2018 8:04 PM, Zeng, Star wrote:
> On 2018/2/2 14:45, Ruiyu Ni 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>
>> Cc: 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 43400e1338..4b5af76999 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_NOTIFICATION_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) {
> 
> Should be mResetNotifyDepth < MAX_RESET_NOTIFY_DEPTH?

No. The intention is the MAX_RESET_NOTIFY_DEPTH times of call is
permitted.

> 
> Thanks,
> Star
> 
>> +    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_NOTIFICATION_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:
>>
> 


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