[PATCH 0/3] Bug fix and minor enhancements to ResetSystem code

[PATCH 0/3] Bug fix and minor enhancements to ResetSystem code

[Ruiyu Ni]

Ruiyu Ni (3):
  MdeModulePkg/ResetUtilityLib: Add pack(1) for reset data structure
  MdeModulePkg/[Pei|Dxe]ResetSystemLib: Add PPI/Protocol dependency
  MdeModulePkg/ResetSystemRuntimeDxe: Add more info in debug message

 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf | 3 +++
 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf | 2 ++
 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c          | 9 ++++++++-
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c   | 5 ++++-
 4 files changed, 17 insertions(+), 2 deletions(-)

-- 
2.16.1.windows.1

[PATCH 1/3] MdeModulePkg/ResetUtilityLib: Add pack(1) for reset data structure

[Ruiyu Ni]

RESET_UTILITY_GUID_SPECIFIC_RESET_DATA structure should be declared
as pack(1).
The patch adds the missing pack(1) pragma.

(GUID *)((UINT8 *)&ResetData +
OFFSET_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, ResetSubtype))
is used to replace &ResetData.ResetSubType to resolve C4366 VS
compiler warning.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
index e3de4f117a..46a9ac6648 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -19,10 +19,14 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/ResetSystemLib.h>
 
+#pragma pack(1)
 typedef struct {
   CHAR16 NullTerminator;
   GUID   ResetSubtype;
 } RESET_UTILITY_GUID_SPECIFIC_RESET_DATA;
+#pragma pack()
+
+VERIFY_SIZE_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, 18);
 
 /**
   This is a shorthand helper function to reset with a subtype so that
@@ -49,7 +53,10 @@ ResetPlatformSpecificGuid (
   RESET_UTILITY_GUID_SPECIFIC_RESET_DATA  ResetData;
 
   ResetData.NullTerminator = CHAR_NULL;
-  CopyGuid (&ResetData.ResetSubtype, ResetSubtype);
+  CopyGuid (
+    (GUID *)((UINT8 *)&ResetData + OFFSET_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, ResetSubtype)),
+    ResetSubtype
+    );
   ResetPlatformSpecific (sizeof (ResetData), &ResetData);
 }
 
-- 
2.16.1.windows.1

Re: [PATCH 1/3] MdeModulePkg/ResetUtilityLib: Add pack(1) for reset data structure

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, July 2, 2018 11:21 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 1/3] MdeModulePkg/ResetUtilityLib: Add pack(1) for reset data structure

RESET_UTILITY_GUID_SPECIFIC_RESET_DATA structure should be declared as pack(1).
The patch adds the missing pack(1) pragma.

(GUID *)((UINT8 *)&ResetData +
OFFSET_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, ResetSubtype)) is used to replace &ResetData.ResetSubType to resolve C4366 VS compiler warning.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
index e3de4f117a..46a9ac6648 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -19,10 +19,14 @@
 #include <Library/BaseMemoryLib.h>
 #include <Library/ResetSystemLib.h>
 
+#pragma pack(1)
 typedef struct {
   CHAR16 NullTerminator;
   GUID   ResetSubtype;
 } RESET_UTILITY_GUID_SPECIFIC_RESET_DATA;
+#pragma pack()
+
+VERIFY_SIZE_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, 18);
 
 /**
   This is a shorthand helper function to reset with a subtype so that @@ -49,7 +53,10 @@ ResetPlatformSpecificGuid (
   RESET_UTILITY_GUID_SPECIFIC_RESET_DATA  ResetData;
 
   ResetData.NullTerminator = CHAR_NULL;
-  CopyGuid (&ResetData.ResetSubtype, ResetSubtype);
+  CopyGuid (
+    (GUID *)((UINT8 *)&ResetData + OFFSET_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, ResetSubtype)),
+    ResetSubtype
+    );
   ResetPlatformSpecific (sizeof (ResetData), &ResetData);  }
 
--
2.16.1.windows.1

[PATCH 2/3] MdeModulePkg/[Pei|Dxe]ResetSystemLib: Add PPI/Protocol dependency

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf | 3 +++
 MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
index 5cd52d8859..e25c3e7d55 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -36,3 +36,6 @@ [Packages]
 
 [LibraryClasses]
   UefiRuntimeServicesTableLib
+
+[Depex]
+  gEfiResetArchProtocolGuid
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
index b1b9388c63..5af03f305c 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
@@ -37,3 +37,5 @@ [Packages]
 [LibraryClasses]
   PeiServicesLib
 
+[Depex]
+  gEfiPeiReset2PpiGuid
-- 
2.16.1.windows.1

Re: [PATCH 2/3] MdeModulePkg/[Pei|Dxe]ResetSystemLib: Add PPI/Protocol dependency

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, July 2, 2018 11:21 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 2/3] MdeModulePkg/[Pei|Dxe]ResetSystemLib: Add PPI/Protocol dependency

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf | 3 +++  MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf | 2 ++
 2 files changed, 5 insertions(+)

diff --git a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
index 5cd52d8859..e25c3e7d55 100644
--- a/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
+++ b/MdeModulePkg/Library/DxeResetSystemLib/DxeResetSystemLib.inf
@@ -36,3 +36,6 @@ [Packages]
 
 [LibraryClasses]
   UefiRuntimeServicesTableLib
+
+[Depex]
+  gEfiResetArchProtocolGuid
diff --git a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
index b1b9388c63..5af03f305c 100644
--- a/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
+++ b/MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
@@ -37,3 +37,5 @@ [Packages]
 [LibraryClasses]
   PeiServicesLib
 
+[Depex]
+  gEfiPeiReset2PpiGuid
--
2.16.1.windows.1

[PATCH 3/3] MdeModulePkg/ResetSystemRuntimeDxe: Add more info in debug message

[Ruiyu Ni]

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index f23b5e86f9..afc35587fc 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -266,7 +266,10 @@ ResetSystem (
   }
 
   mResetNotifyDepth++;
-  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
+  DEBUG ((
+    DEBUG_INFO, "DXE ResetSystem2: ResetType %s, Call Depth = %d.\n",
+    mResetTypeStr[ResetType], mResetNotifyDepth
+    ));
 
   if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
     if (!EfiAtRuntime ()) {
-- 
2.16.1.windows.1

Re: [PATCH 3/3] MdeModulePkg/ResetSystemRuntimeDxe: Add more info in debug message

[Zeng, Star]

Reviewed-by: Star Zeng <star.zeng@intel.com>

How about doing same update in ResetSystemPei in an updated patch or a separated patch?


Thanks,
Star
-----Original Message-----
From: Ni, Ruiyu 
Sent: Monday, July 2, 2018 11:21 AM
To: edk2-devel@lists.01.org
Cc: Zeng, Star <star.zeng@intel.com>
Subject: [PATCH 3/3] MdeModulePkg/ResetSystemRuntimeDxe: Add more info in debug message

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
index f23b5e86f9..afc35587fc 100644
--- a/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
+++ b/MdeModulePkg/Universal/ResetSystemRuntimeDxe/ResetSystem.c
@@ -266,7 +266,10 @@ ResetSystem (
   }
 
   mResetNotifyDepth++;
-  DEBUG ((DEBUG_INFO, "DXE ResetSystem2: Reset call depth = %d.\n", mResetNotifyDepth));
+  DEBUG ((
+    DEBUG_INFO, "DXE ResetSystem2: ResetType %s, Call Depth = %d.\n",
+    mResetTypeStr[ResetType], mResetNotifyDepth
+    ));
 
   if (mResetNotifyDepth <= MAX_RESET_NOTIFY_DEPTH) {
     if (!EfiAtRuntime ()) {
-- 
2.16.1.windows.1