public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH 0/4] MdeModulePkg: Transfer reset data
@ 2019-05-05  7:50 Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid Gao, Zhichao
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-05  7:50 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

Indicate two guids gCapsuleArmedResetGuid and gCapsuleUpdateCompleteResetGuid
for capsule update. And define a struct which start with a null string and
followed by a EFI_GUID.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>

Bret Barkelew (3):
  MdeModulePkg: Add reset data difinition and guid
  MdeModulePkg/CapsuleRuntimeDxe: Transfer reset data
  MdeModulePkg/CapsuleLib: Transfer reset data

Zhichao Gao (1):
  MdeModulePkg/ResetUtilityLib: Replace the reset data difinition

 MdeModulePkg/Include/Guid/CapsuleResetData.h  | 40 +++++++++++++++++++
 .../DxeCapsuleLibFmp/DxeCapsuleLib.inf        |  3 +-
 .../DxeCapsuleLibFmp/DxeCapsuleProcessLib.c   |  6 ++-
 .../Library/ResetUtilityLib/ResetUtility.c    | 16 ++------
 MdeModulePkg/MdeModulePkg.dec                 |  5 +++
 .../CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf   |  1 +
 .../CapsuleRuntimeDxe/CapsuleService.c        |  4 +-
 .../CapsuleRuntimeDxe/CapsuleService.h        |  3 +-
 8 files changed, 60 insertions(+), 18 deletions(-)
 create mode 100644 MdeModulePkg/Include/Guid/CapsuleResetData.h

-- 
2.21.0.windows.1


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

* [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid
  2019-05-05  7:50 [PATCH 0/4] MdeModulePkg: Transfer reset data Gao, Zhichao
@ 2019-05-05  7:50 ` Gao, Zhichao
  2019-05-06 15:44   ` [edk2-devel] " Laszlo Ersek
  2019-05-05  7:50 ` [PATCH 2/4] MdeModulePkg/CapsuleRuntimeDxe: Transfer reset data Gao, Zhichao
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-05  7:50 UTC (permalink / raw)
  To: devel
  Cc: Bret Barkelew, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772

Add a useful definition of reset data for capsule function. And add
two guids gCapsuleArmedResetGuid and gCapsuleUpdateCompleteResetGuid.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Include/Guid/CapsuleResetData.h | 40 ++++++++++++++++++++
 MdeModulePkg/MdeModulePkg.dec                |  5 +++
 2 files changed, 45 insertions(+)
 create mode 100644 MdeModulePkg/Include/Guid/CapsuleResetData.h

diff --git a/MdeModulePkg/Include/Guid/CapsuleResetData.h b/MdeModulePkg/Include/Guid/CapsuleResetData.h
new file mode 100644
index 0000000000..b0666a0da2
--- /dev/null
+++ b/MdeModulePkg/Include/Guid/CapsuleResetData.h
@@ -0,0 +1,40 @@
+/** @file
+  The capsule reset data difinition and guids.
+
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+#ifndef __CAPSULE_RESET_DATA_H__
+#define __CAPSULE_RESET_DATA_H__
+
+#include <Uefi/UefiBaseType.h>
+
+//
+// reset data started with a null string and followed by a guid data
+//
+#pragma pack(1)
+typedef struct {
+  CHAR16      NullString;
+  EFI_GUID    ResetGuid;
+} RESET_DATA_WITH_NULL_STRING;
+#pragma pack()
+
+VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);
+
+#define CAPSULE_ARMED_RESET_GUID \
+  { \
+    0xc6b4eea7, 0xfce2, 0x4625, { 0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37, 0xae, 0x23 }  \
+  }
+
+#define CAPSULE_UPDATE_COMPLETE_RESET_GUID \
+  { \
+    0x5d512714, 0xa4df, 0x4e46, { 0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d, 0x59, 0xa0 }  \
+  }
+
+extern EFI_GUID gCapsuleArmedResetGuid;
+
+extern EFI_GUID gCapsuleUpdateCompleteResetGuid;
+
+#endif
+
diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
index 2ef48f2c37..c12b836c24 100644
--- a/MdeModulePkg/MdeModulePkg.dec
+++ b/MdeModulePkg/MdeModulePkg.dec
@@ -423,6 +423,11 @@
   ## Include/Guid/S3StorageDeviceInitList.h
   gS3StorageDeviceInitListGuid = { 0x310e9b8c, 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17, 0xc8, 0xef } }
 
+  ## Guid to use for gRT->ResetSystem() to indicate the type of reset that is being performed.
+  #  Include/Guid/CapsuleResetData.h
+  gCapsuleArmedResetGuid            = {0xc6b4eea7, 0xfce2, 0x4625, {0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37, 0xae, 0x23}}
+  gCapsuleUpdateCompleteResetGuid   = {0x5d512714, 0xa4df, 0x4e46, {0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d, 0x59, 0xa0}}
+
 [Ppis]
   ## Include/Ppi/AtaController.h
   gPeiAtaControllerPpiGuid       = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
-- 
2.21.0.windows.1


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

* [PATCH 2/4] MdeModulePkg/CapsuleRuntimeDxe: Transfer reset data
  2019-05-05  7:50 [PATCH 0/4] MdeModulePkg: Transfer reset data Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid Gao, Zhichao
@ 2019-05-05  7:50 ` Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 3/4] MdeModulePkg/CapsuleLib: " Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 4/4] MdeModulePkg/ResetUtilityLib: Replace the reset data difinition Gao, Zhichao
  3 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-05  7:50 UTC (permalink / raw)
  To: devel
  Cc: Bret Barkelew, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772

Transfer reset data start with a null sting and followed by
gCapsuleArmedResetGuid for capsule update.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 .../Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf         | 1 +
 MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c     | 4 ++--
 MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h     | 3 ++-
 3 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
index 338577e293..d80b5094d3 100644
--- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
+++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
@@ -72,6 +72,7 @@
   ## SOMETIMES_PRODUCES   ## Variable:L"CapsuleLongModeBuffer" # The long mode buffer used by IA32 Capsule PEIM to call X64 CapsuleCoalesce code to handle >4GB capsule blocks
   gEfiCapsuleVendorGuid
   gEfiFmpCapsuleGuid                            ## SOMETIMES_CONSUMES   ## GUID # FMP capsule GUID
+  gCapsuleArmedResetGuid                        ## SOMETIMES_CONSUMES
 
 [Protocols]
   gEfiCapsuleArchProtocolGuid                   ## PRODUCES
diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
index aaf819c4c6..109541c17f 100644
--- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
+++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.c
@@ -4,7 +4,7 @@
   It installs the Capsule Architectural Protocol defined in PI1.0a to signify
   the capsule runtime services are ready.
 
-Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
 SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -210,7 +210,7 @@ UpdateCapsule (
        // will initiate a reset of the platform which is compatible with the passed-in capsule request and will
        // not return back to the caller.
        //
-       EfiResetSystem (EfiResetWarm, EFI_SUCCESS, 0, NULL);
+       EfiResetSystem (EfiResetWarm, EFI_SUCCESS, sizeof (RESET_DATA_WITH_NULL_STRING), &gCapsuleArmedResetGuid);
      }
   }
   return Status;
diff --git a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
index 069df3c750..ab35f1c42e 100644
--- a/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
+++ b/MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleService.h
@@ -4,7 +4,7 @@
   It installs the Capsule Architectural Protocol defined in PI1.0a to signify
   the capsule runtime services are ready.
 
-  Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved.<BR>
   Copyright (c) 2018, Linaro, Ltd. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -19,6 +19,7 @@
 #include <Protocol/Capsule.h>
 #include <Guid/CapsuleVendor.h>
 #include <Guid/FmpCapsule.h>
+#include <Guid/CapsuleResetData.h>
 
 #include <Library/DebugLib.h>
 #include <Library/PcdLib.h>
-- 
2.21.0.windows.1


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

* [PATCH 3/4] MdeModulePkg/CapsuleLib: Transfer reset data
  2019-05-05  7:50 [PATCH 0/4] MdeModulePkg: Transfer reset data Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 2/4] MdeModulePkg/CapsuleRuntimeDxe: Transfer reset data Gao, Zhichao
@ 2019-05-05  7:50 ` Gao, Zhichao
  2019-05-05  7:50 ` [PATCH 4/4] MdeModulePkg/ResetUtilityLib: Replace the reset data difinition Gao, Zhichao
  3 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-05  7:50 UTC (permalink / raw)
  To: devel
  Cc: Bret Barkelew, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

From: Bret Barkelew <Bret.Barkelew@microsoft.com>

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772

Transfer reset data start with a null sting and followed by
gCapsuleUpdateCompleteResetGuid for capsule update.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf     | 3 ++-
 .../Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c         | 6 ++++--
 2 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
index 14c3d19bc3..78e4958995 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleLib.inf
@@ -3,7 +3,7 @@
 #
 #  Capsule library instance for DXE_DRIVER module types.
 #
-#  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+#  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
 ##
@@ -74,6 +74,7 @@
   gEfiCapsuleReportGuid
   gEfiCapsuleVendorGuid                   ## SOMETIMES_CONSUMES ## Variable:L"CapsuleUpdateData"
   gEfiEndOfDxeEventGroupGuid              ## CONSUMES ## Event
+  gCapsuleUpdateCompleteResetGuid         ## SOMETIMES_CONSUMES
 
 [Depex]
   gEfiVariableWriteArchProtocolGuid
diff --git a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
index 5e2d2b87a8..dbc4287bfa 100644
--- a/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
+++ b/MdeModulePkg/Library/DxeCapsuleLibFmp/DxeCapsuleProcessLib.c
@@ -9,7 +9,7 @@
   ProcessCapsules(), ProcessTheseCapsules() will receive untrusted
   input and do basic validation.
 
-  Copyright (c) 2016 - 2018, Intel Corporation. All rights reserved.<BR>
+  Copyright (c) 2016 - 2019, Intel Corporation. All rights reserved.<BR>
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
 **/
@@ -33,6 +33,8 @@
 
 #include <IndustryStandard/WindowsUxCapsule.h>
 
+#include <Guid/CapsuleResetData.h>
+
 extern EDKII_FIRMWARE_MANAGEMENT_PROGRESS_PROTOCOL  *mFmpProgress;
 
 /**
@@ -518,7 +520,7 @@ DoResetSystem (
 
   REPORT_STATUS_CODE(EFI_PROGRESS_CODE, (EFI_SOFTWARE | PcdGet32(PcdStatusCodeSubClassCapsule) | PcdGet32(PcdCapsuleStatusCodeResettingSystem)));
 
-  gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, 0, NULL);
+  gRT->ResetSystem(EfiResetCold, EFI_SUCCESS, sizeof (RESET_DATA_WITH_NULL_STRING), &gCapsuleUpdateCompleteResetGuid);
 
   CpuDeadLoop();
 }
-- 
2.21.0.windows.1


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

* [PATCH 4/4] MdeModulePkg/ResetUtilityLib: Replace the reset data difinition
  2019-05-05  7:50 [PATCH 0/4] MdeModulePkg: Transfer reset data Gao, Zhichao
                   ` (2 preceding siblings ...)
  2019-05-05  7:50 ` [PATCH 3/4] MdeModulePkg/CapsuleLib: " Gao, Zhichao
@ 2019-05-05  7:50 ` Gao, Zhichao
  3 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-05  7:50 UTC (permalink / raw)
  To: devel
  Cc: Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao, Sean Brogan,
	Michael Turner, Bret Barkelew

REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772

Replace the definition of 'RESET_UTILITY_GUID_SPECIFIC_RESET_DATA'
with 'RESET_DATA_WITH_NULL_STRING'.

Cc: Jian J Wang <jian.j.wang@intel.com>
Cc: Hao Wu <hao.a.wu@intel.com>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Sean Brogan <sean.brogan@microsoft.com>
Cc: Michael Turner <Michael.Turner@microsoft.com>
Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
---
 .../Library/ResetUtilityLib/ResetUtility.c       | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
index 2b5af4b95a..c420e0e36a 100644
--- a/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
+++ b/MdeModulePkg/Library/ResetUtilityLib/ResetUtility.c
@@ -12,15 +12,7 @@
 #include <Library/DebugLib.h>
 #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);
+#include <Guid/CapsuleResetData.h>
 
 /**
   This is a shorthand helper function to reset with reset type and a subtype
@@ -46,11 +38,11 @@ ResetSystemWithSubtype (
   IN CONST  GUID        *ResetSubtype
   )
 {
-  RESET_UTILITY_GUID_SPECIFIC_RESET_DATA  ResetData;
+  RESET_DATA_WITH_NULL_STRING   ResetData;
 
-  ResetData.NullTerminator = CHAR_NULL;
+  ResetData.NullString = CHAR_NULL;
   CopyGuid (
-    (GUID *)((UINT8 *)&ResetData + OFFSET_OF (RESET_UTILITY_GUID_SPECIFIC_RESET_DATA, ResetSubtype)),
+    (GUID *)((UINT8 *)&ResetData + OFFSET_OF (RESET_DATA_WITH_NULL_STRING, ResetGuid)),
     ResetSubtype
     );
 
-- 
2.21.0.windows.1


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

* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid
  2019-05-05  7:50 ` [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid Gao, Zhichao
@ 2019-05-06 15:44   ` Laszlo Ersek
  2019-05-06 21:02     ` Michael D Kinney
  0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2019-05-06 15:44 UTC (permalink / raw)
  To: devel, zhichao.gao
  Cc: Bret Barkelew, Jian J Wang, Hao Wu, Ray Ni, Star Zeng, Liming Gao,
	Sean Brogan, Michael Turner

On 05/05/19 09:50, Gao, Zhichao wrote:
> From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> 
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1772
> 
> Add a useful definition of reset data for capsule function. And add
> two guids gCapsuleArmedResetGuid and gCapsuleUpdateCompleteResetGuid.
> 
> Cc: Jian J Wang <jian.j.wang@intel.com>
> Cc: Hao Wu <hao.a.wu@intel.com>
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>
> Cc: Michael Turner <Michael.Turner@microsoft.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> ---
>  MdeModulePkg/Include/Guid/CapsuleResetData.h | 40 ++++++++++++++++++++
>  MdeModulePkg/MdeModulePkg.dec                |  5 +++
>  2 files changed, 45 insertions(+)
>  create mode 100644 MdeModulePkg/Include/Guid/CapsuleResetData.h
> 
> diff --git a/MdeModulePkg/Include/Guid/CapsuleResetData.h b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> new file mode 100644
> index 0000000000..b0666a0da2
> --- /dev/null
> +++ b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> @@ -0,0 +1,40 @@
> +/** @file
> +  The capsule reset data difinition and guids.
> +
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +#ifndef __CAPSULE_RESET_DATA_H__
> +#define __CAPSULE_RESET_DATA_H__
> +
> +#include <Uefi/UefiBaseType.h>
> +
> +//
> +// reset data started with a null string and followed by a guid data
> +//
> +#pragma pack(1)
> +typedef struct {
> +  CHAR16      NullString;
> +  EFI_GUID    ResetGuid;
> +} RESET_DATA_WITH_NULL_STRING;
> +#pragma pack()
> +
> +VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);

I think that exposing RESET_DATA_WITH_NULL_STRING as a public structure
is a good idea -- it indeed looks like something useful for many
platforms and for different purposes.

- However, I'd argue that it should not go into an include file called
"CapsuleResetData.h". The type looks generic and not tied to capsules.

- Additionally, I believe (but I'm not fully sure!) that new structures
under MdeModulePkg/Include/Guid should be named EDKII_*.

If the MdeModulePkg maintainers are OK with these patches in the current
form, I'm fine too; I just thought we might want to consider the above
points. (I certainly suggest waiting for their feedback before starting
work on v2, just to address the above.)

Thanks!
Laszlo

> +
> +#define CAPSULE_ARMED_RESET_GUID \
> +  { \
> +    0xc6b4eea7, 0xfce2, 0x4625, { 0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37, 0xae, 0x23 }  \
> +  }
> +
> +#define CAPSULE_UPDATE_COMPLETE_RESET_GUID \
> +  { \
> +    0x5d512714, 0xa4df, 0x4e46, { 0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d, 0x59, 0xa0 }  \
> +  }
> +
> +extern EFI_GUID gCapsuleArmedResetGuid;
> +
> +extern EFI_GUID gCapsuleUpdateCompleteResetGuid;
> +
> +#endif
> +
> diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec
> index 2ef48f2c37..c12b836c24 100644
> --- a/MdeModulePkg/MdeModulePkg.dec
> +++ b/MdeModulePkg/MdeModulePkg.dec
> @@ -423,6 +423,11 @@
>    ## Include/Guid/S3StorageDeviceInitList.h
>    gS3StorageDeviceInitListGuid = { 0x310e9b8c, 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17, 0xc8, 0xef } }
>  
> +  ## Guid to use for gRT->ResetSystem() to indicate the type of reset that is being performed.
> +  #  Include/Guid/CapsuleResetData.h
> +  gCapsuleArmedResetGuid            = {0xc6b4eea7, 0xfce2, 0x4625, {0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37, 0xae, 0x23}}
> +  gCapsuleUpdateCompleteResetGuid   = {0x5d512714, 0xa4df, 0x4e46, {0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d, 0x59, 0xa0}}
> +
>  [Ppis]
>    ## Include/Ppi/AtaController.h
>    gPeiAtaControllerPpiGuid       = { 0xa45e60d1, 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
> 


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

* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid
  2019-05-06 15:44   ` [edk2-devel] " Laszlo Ersek
@ 2019-05-06 21:02     ` Michael D Kinney
  2019-05-07 10:14       ` Laszlo Ersek
  0 siblings, 1 reply; 9+ messages in thread
From: Michael D Kinney @ 2019-05-06 21:02 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Gao, Zhichao,
	Kinney, Michael D
  Cc: Bret Barkelew, Wang, Jian J, Wu, Hao A, Ni, Ray, Zeng, Star,
	Gao, Liming, Sean Brogan, Michael Turner

Laszlo,

I agree with both your points.  The file name should not imply
that it can only be used for capsule resets.

I also agree that the structure name and GUIDs should use 
EDKII_ and gEdkii prefixes.

I also suggest, since the 2 new GUIDs are not associated with 
a structure (they are used to fill in a GUID value field in a
structure), that they can be declared in the DEC file alone and
do not require the MACRO name or extern declaration in a new .h
file.  The .h file should only contain the new data structure
EDKII_RESET_DATA_WITH_NULL_STRING.

Depending on what modules/libs use EDKII_RESET_DATA_WITH_NULL_STRING,
it may make more sense to add this structure to an existing 
library class .h file.

Best regards,

Mike

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
> On Behalf Of Laszlo Ersek
> Sent: Monday, May 6, 2019 8:44 AM
> To: devel@edk2.groups.io; Gao, Zhichao
> <zhichao.gao@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang,
> Jian J <jian.j.wang@intel.com>; Wu, Hao A
> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
> Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan
> <sean.brogan@microsoft.com>; Michael Turner
> <Michael.Turner@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add
> reset data difinition and guid
> 
> On 05/05/19 09:50, Gao, Zhichao wrote:
> > From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >
> > REF:
> https://bugzilla.tianocore.org/show_bug.cgi?id=1772
> >
> > Add a useful definition of reset data for capsule
> function. And add
> > two guids gCapsuleArmedResetGuid and
> gCapsuleUpdateCompleteResetGuid.
> >
> > Cc: Jian J Wang <jian.j.wang@intel.com>
> > Cc: Hao Wu <hao.a.wu@intel.com>
> > Cc: Ray Ni <ray.ni@intel.com>
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Sean Brogan <sean.brogan@microsoft.com>
> > Cc: Michael Turner <Michael.Turner@microsoft.com>
> > Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> > Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> > ---
> >  MdeModulePkg/Include/Guid/CapsuleResetData.h | 40
> ++++++++++++++++++++
> >  MdeModulePkg/MdeModulePkg.dec                |  5 +++
> >  2 files changed, 45 insertions(+)
> >  create mode 100644
> MdeModulePkg/Include/Guid/CapsuleResetData.h
> >
> > diff --git
> a/MdeModulePkg/Include/Guid/CapsuleResetData.h
> b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> > new file mode 100644
> > index 0000000000..b0666a0da2
> > --- /dev/null
> > +++ b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> > @@ -0,0 +1,40 @@
> > +/** @file
> > +  The capsule reset data difinition and guids.
> > +
> > +  Copyright (c) 2019, Intel Corporation. All rights
> reserved.<BR>
> > +  SPDX-License-Identifier: BSD-2-Clause-Patent
> > +
> > +**/
> > +#ifndef __CAPSULE_RESET_DATA_H__
> > +#define __CAPSULE_RESET_DATA_H__
> > +
> > +#include <Uefi/UefiBaseType.h>
> > +
> > +//
> > +// reset data started with a null string and followed
> by a guid data
> > +//
> > +#pragma pack(1)
> > +typedef struct {
> > +  CHAR16      NullString;
> > +  EFI_GUID    ResetGuid;
> > +} RESET_DATA_WITH_NULL_STRING;
> > +#pragma pack()
> > +
> > +VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);
> 
> I think that exposing RESET_DATA_WITH_NULL_STRING as a
> public structure
> is a good idea -- it indeed looks like something useful
> for many
> platforms and for different purposes.
> 
> - However, I'd argue that it should not go into an
> include file called
> "CapsuleResetData.h". The type looks generic and not
> tied to capsules.
> 
> - Additionally, I believe (but I'm not fully sure!) that
> new structures
> under MdeModulePkg/Include/Guid should be named EDKII_*.
> 
> If the MdeModulePkg maintainers are OK with these
> patches in the current
> form, I'm fine too; I just thought we might want to
> consider the above
> points. (I certainly suggest waiting for their feedback
> before starting
> work on v2, just to address the above.)
> 
> Thanks!
> Laszlo
> 
> > +
> > +#define CAPSULE_ARMED_RESET_GUID \
> > +  { \
> > +    0xc6b4eea7, 0xfce2, 0x4625, { 0x9c, 0x4f, 0xc4,
> 0xb0, 0x82, 0x37, 0xae, 0x23 }  \
> > +  }
> > +
> > +#define CAPSULE_UPDATE_COMPLETE_RESET_GUID \
> > +  { \
> > +    0x5d512714, 0xa4df, 0x4e46, { 0xb6, 0xc7, 0xbc,
> 0x9f, 0x97, 0x9d, 0x59, 0xa0 }  \
> > +  }
> > +
> > +extern EFI_GUID gCapsuleArmedResetGuid;
> > +
> > +extern EFI_GUID gCapsuleUpdateCompleteResetGuid;
> > +
> > +#endif
> > +
> > diff --git a/MdeModulePkg/MdeModulePkg.dec
> b/MdeModulePkg/MdeModulePkg.dec
> > index 2ef48f2c37..c12b836c24 100644
> > --- a/MdeModulePkg/MdeModulePkg.dec
> > +++ b/MdeModulePkg/MdeModulePkg.dec
> > @@ -423,6 +423,11 @@
> >    ## Include/Guid/S3StorageDeviceInitList.h
> >    gS3StorageDeviceInitListGuid = { 0x310e9b8c,
> 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17,
> 0xc8, 0xef } }
> >
> > +  ## Guid to use for gRT->ResetSystem() to indicate
> the type of reset that is being performed.
> > +  #  Include/Guid/CapsuleResetData.h
> > +  gCapsuleArmedResetGuid            = {0xc6b4eea7,
> 0xfce2, 0x4625, {0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37,
> 0xae, 0x23}}
> > +  gCapsuleUpdateCompleteResetGuid   = {0x5d512714,
> 0xa4df, 0x4e46, {0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d,
> 0x59, 0xa0}}
> > +
> >  [Ppis]
> >    ## Include/Ppi/AtaController.h
> >    gPeiAtaControllerPpiGuid       = { 0xa45e60d1,
> 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85,
> 0x90, 0x6d }}
> >
> 
> 
> 


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

* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid
  2019-05-06 21:02     ` Michael D Kinney
@ 2019-05-07 10:14       ` Laszlo Ersek
  2019-05-08  4:50         ` Gao, Zhichao
  0 siblings, 1 reply; 9+ messages in thread
From: Laszlo Ersek @ 2019-05-07 10:14 UTC (permalink / raw)
  To: Kinney, Michael D, devel@edk2.groups.io, Gao, Zhichao
  Cc: Bret Barkelew, Wang, Jian J, Wu, Hao A, Ni, Ray, Zeng, Star,
	Gao, Liming, Sean Brogan, Michael Turner

On 05/06/19 23:02, Kinney, Michael D wrote:
> Laszlo,
> 
> I agree with both your points.  The file name should not imply
> that it can only be used for capsule resets.
> 
> I also agree that the structure name and GUIDs should use 
> EDKII_ and gEdkii prefixes.
> 
> I also suggest, since the 2 new GUIDs are not associated with 
> a structure (they are used to fill in a GUID value field in a
> structure), that they can be declared in the DEC file alone and
> do not require the MACRO name or extern declaration in a new .h
> file.  The .h file should only contain the new data structure
> EDKII_RESET_DATA_WITH_NULL_STRING.
> 
> Depending on what modules/libs use EDKII_RESET_DATA_WITH_NULL_STRING,
> it may make more sense to add this structure to an existing 
> library class .h file.

The lib class header "MdeModulePkg/Include/Library/ResetUtilityLib.h"
definitely crossed my mind, but I didn't dare suggest it. :)

Thanks
Laszlo

> Best regards,
> 
> Mike
> 
>> -----Original Message-----
>> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io]
>> On Behalf Of Laszlo Ersek
>> Sent: Monday, May 6, 2019 8:44 AM
>> To: devel@edk2.groups.io; Gao, Zhichao
>> <zhichao.gao@intel.com>
>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang,
>> Jian J <jian.j.wang@intel.com>; Wu, Hao A
>> <hao.a.wu@intel.com>; Ni, Ray <ray.ni@intel.com>; Zeng,
>> Star <star.zeng@intel.com>; Gao, Liming
>> <liming.gao@intel.com>; Sean Brogan
>> <sean.brogan@microsoft.com>; Michael Turner
>> <Michael.Turner@microsoft.com>
>> Subject: Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add
>> reset data difinition and guid
>>
>> On 05/05/19 09:50, Gao, Zhichao wrote:
>>> From: Bret Barkelew <Bret.Barkelew@microsoft.com>
>>>
>>> REF:
>> https://bugzilla.tianocore.org/show_bug.cgi?id=1772
>>>
>>> Add a useful definition of reset data for capsule
>> function. And add
>>> two guids gCapsuleArmedResetGuid and
>> gCapsuleUpdateCompleteResetGuid.
>>>
>>> Cc: Jian J Wang <jian.j.wang@intel.com>
>>> Cc: Hao Wu <hao.a.wu@intel.com>
>>> Cc: Ray Ni <ray.ni@intel.com>
>>> Cc: Star Zeng <star.zeng@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Sean Brogan <sean.brogan@microsoft.com>
>>> Cc: Michael Turner <Michael.Turner@microsoft.com>
>>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
>>> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
>>> ---
>>>  MdeModulePkg/Include/Guid/CapsuleResetData.h | 40
>> ++++++++++++++++++++
>>>  MdeModulePkg/MdeModulePkg.dec                |  5 +++
>>>  2 files changed, 45 insertions(+)
>>>  create mode 100644
>> MdeModulePkg/Include/Guid/CapsuleResetData.h
>>>
>>> diff --git
>> a/MdeModulePkg/Include/Guid/CapsuleResetData.h
>> b/MdeModulePkg/Include/Guid/CapsuleResetData.h
>>> new file mode 100644
>>> index 0000000000..b0666a0da2
>>> --- /dev/null
>>> +++ b/MdeModulePkg/Include/Guid/CapsuleResetData.h
>>> @@ -0,0 +1,40 @@
>>> +/** @file
>>> +  The capsule reset data difinition and guids.
>>> +
>>> +  Copyright (c) 2019, Intel Corporation. All rights
>> reserved.<BR>
>>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
>>> +
>>> +**/
>>> +#ifndef __CAPSULE_RESET_DATA_H__
>>> +#define __CAPSULE_RESET_DATA_H__
>>> +
>>> +#include <Uefi/UefiBaseType.h>
>>> +
>>> +//
>>> +// reset data started with a null string and followed
>> by a guid data
>>> +//
>>> +#pragma pack(1)
>>> +typedef struct {
>>> +  CHAR16      NullString;
>>> +  EFI_GUID    ResetGuid;
>>> +} RESET_DATA_WITH_NULL_STRING;
>>> +#pragma pack()
>>> +
>>> +VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);
>>
>> I think that exposing RESET_DATA_WITH_NULL_STRING as a
>> public structure
>> is a good idea -- it indeed looks like something useful
>> for many
>> platforms and for different purposes.
>>
>> - However, I'd argue that it should not go into an
>> include file called
>> "CapsuleResetData.h". The type looks generic and not
>> tied to capsules.
>>
>> - Additionally, I believe (but I'm not fully sure!) that
>> new structures
>> under MdeModulePkg/Include/Guid should be named EDKII_*.
>>
>> If the MdeModulePkg maintainers are OK with these
>> patches in the current
>> form, I'm fine too; I just thought we might want to
>> consider the above
>> points. (I certainly suggest waiting for their feedback
>> before starting
>> work on v2, just to address the above.)
>>
>> Thanks!
>> Laszlo
>>
>>> +
>>> +#define CAPSULE_ARMED_RESET_GUID \
>>> +  { \
>>> +    0xc6b4eea7, 0xfce2, 0x4625, { 0x9c, 0x4f, 0xc4,
>> 0xb0, 0x82, 0x37, 0xae, 0x23 }  \
>>> +  }
>>> +
>>> +#define CAPSULE_UPDATE_COMPLETE_RESET_GUID \
>>> +  { \
>>> +    0x5d512714, 0xa4df, 0x4e46, { 0xb6, 0xc7, 0xbc,
>> 0x9f, 0x97, 0x9d, 0x59, 0xa0 }  \
>>> +  }
>>> +
>>> +extern EFI_GUID gCapsuleArmedResetGuid;
>>> +
>>> +extern EFI_GUID gCapsuleUpdateCompleteResetGuid;
>>> +
>>> +#endif
>>> +
>>> diff --git a/MdeModulePkg/MdeModulePkg.dec
>> b/MdeModulePkg/MdeModulePkg.dec
>>> index 2ef48f2c37..c12b836c24 100644
>>> --- a/MdeModulePkg/MdeModulePkg.dec
>>> +++ b/MdeModulePkg/MdeModulePkg.dec
>>> @@ -423,6 +423,11 @@
>>>    ## Include/Guid/S3StorageDeviceInitList.h
>>>    gS3StorageDeviceInitListGuid = { 0x310e9b8c,
>> 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17,
>> 0xc8, 0xef } }
>>>
>>> +  ## Guid to use for gRT->ResetSystem() to indicate
>> the type of reset that is being performed.
>>> +  #  Include/Guid/CapsuleResetData.h
>>> +  gCapsuleArmedResetGuid            = {0xc6b4eea7,
>> 0xfce2, 0x4625, {0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37,
>> 0xae, 0x23}}
>>> +  gCapsuleUpdateCompleteResetGuid   = {0x5d512714,
>> 0xa4df, 0x4e46, {0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d,
>> 0x59, 0xa0}}
>>> +
>>>  [Ppis]
>>>    ## Include/Ppi/AtaController.h
>>>    gPeiAtaControllerPpiGuid       = { 0xa45e60d1,
>> 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85,
>> 0x90, 0x6d }}
>>>
>>
>>
>> 
> 


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

* Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid
  2019-05-07 10:14       ` Laszlo Ersek
@ 2019-05-08  4:50         ` Gao, Zhichao
  0 siblings, 0 replies; 9+ messages in thread
From: Gao, Zhichao @ 2019-05-08  4:50 UTC (permalink / raw)
  To: devel@edk2.groups.io, lersek@redhat.com, Kinney, Michael D
  Cc: Bret Barkelew, Wang, Jian J, Wu, Hao A, Ni, Ray, Zeng, Star,
	Gao, Liming, Sean Brogan, Michael Turner

I think ResetUtilityLib.h is a good place to add the new structure definition.
But I think this lib cannot be consumed in some conditions. The new ResetUtilityLib add a new API which would consume a new API ResetSystem in ResetSystemLib.
As we know different platforms have different instances of ResetSystemLib. And I think all of them do not implement the new API yet. So if they consume the ResetUtilityLib, a link error would happen. As an alternative, I define the structure in the ResetSystemLib.h. And do not use the new API ResetSystemWithSubtype in ResetUtilityLib to transfer the ResetData.
Here are some detail conditions:
The platform normally use its own ResetSystemLib. That would lack the new API implement. If it works with the edk2 master and some driver consume the new API. Then a link error would occur.
If the platform add the new API and it works both with edk2 master and some stable tag before, then a name conflict would occur with the stable tag. Because the new API name is conflict with ResetSystemRuntimeDxe and we already change it to RuntimeServiceResetSystem while we add the new API.

All the above is why I want to define the structure in ResetSystemLib.h and not to use the ResetSystemWithSubtype to transfer the ResetData.

Thanks,
Zhichao

> -----Original Message-----
> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> Laszlo Ersek
> Sent: Tuesday, May 7, 2019 6:15 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; devel@edk2.groups.io;
> Gao, Zhichao <zhichao.gao@intel.com>
> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> Michael Turner <Michael.Turner@microsoft.com>
> Subject: Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data
> difinition and guid
> 
> On 05/06/19 23:02, Kinney, Michael D wrote:
> > Laszlo,
> >
> > I agree with both your points.  The file name should not imply that it
> > can only be used for capsule resets.
> >
> > I also agree that the structure name and GUIDs should use EDKII_ and
> > gEdkii prefixes.
> >
> > I also suggest, since the 2 new GUIDs are not associated with a
> > structure (they are used to fill in a GUID value field in a
> > structure), that they can be declared in the DEC file alone and do not
> > require the MACRO name or extern declaration in a new .h file.  The .h
> > file should only contain the new data structure
> > EDKII_RESET_DATA_WITH_NULL_STRING.
> >
> > Depending on what modules/libs use
> EDKII_RESET_DATA_WITH_NULL_STRING,
> > it may make more sense to add this structure to an existing library
> > class .h file.
> 
> The lib class header "MdeModulePkg/Include/Library/ResetUtilityLib.h"
> definitely crossed my mind, but I didn't dare suggest it. :)
> 
> Thanks
> Laszlo
> 
> > Best regards,
> >
> > Mike
> >
> >> -----Original Message-----
> >> From: devel@edk2.groups.io [mailto:devel@edk2.groups.io] On Behalf Of
> >> Laszlo Ersek
> >> Sent: Monday, May 6, 2019 8:44 AM
> >> To: devel@edk2.groups.io; Gao, Zhichao <zhichao.gao@intel.com>
> >> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>; Wang, Jian J
> >> <jian.j.wang@intel.com>; Wu, Hao A <hao.a.wu@intel.com>; Ni, Ray
> >> <ray.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Gao, Liming
> >> <liming.gao@intel.com>; Sean Brogan <sean.brogan@microsoft.com>;
> >> Michael Turner <Michael.Turner@microsoft.com>
> >> Subject: Re: [edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data
> >> difinition and guid
> >>
> >> On 05/05/19 09:50, Gao, Zhichao wrote:
> >>> From: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >>>
> >>> REF:
> >> https://bugzilla.tianocore.org/show_bug.cgi?id=1772
> >>>
> >>> Add a useful definition of reset data for capsule
> >> function. And add
> >>> two guids gCapsuleArmedResetGuid and
> >> gCapsuleUpdateCompleteResetGuid.
> >>>
> >>> Cc: Jian J Wang <jian.j.wang@intel.com>
> >>> Cc: Hao Wu <hao.a.wu@intel.com>
> >>> Cc: Ray Ni <ray.ni@intel.com>
> >>> Cc: Star Zeng <star.zeng@intel.com>
> >>> Cc: Liming Gao <liming.gao@intel.com>
> >>> Cc: Sean Brogan <sean.brogan@microsoft.com>
> >>> Cc: Michael Turner <Michael.Turner@microsoft.com>
> >>> Cc: Bret Barkelew <Bret.Barkelew@microsoft.com>
> >>> Signed-off-by: Zhichao Gao <zhichao.gao@intel.com>
> >>> ---
> >>>  MdeModulePkg/Include/Guid/CapsuleResetData.h | 40
> >> ++++++++++++++++++++
> >>>  MdeModulePkg/MdeModulePkg.dec                |  5 +++
> >>>  2 files changed, 45 insertions(+)
> >>>  create mode 100644
> >> MdeModulePkg/Include/Guid/CapsuleResetData.h
> >>>
> >>> diff --git
> >> a/MdeModulePkg/Include/Guid/CapsuleResetData.h
> >> b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> >>> new file mode 100644
> >>> index 0000000000..b0666a0da2
> >>> --- /dev/null
> >>> +++ b/MdeModulePkg/Include/Guid/CapsuleResetData.h
> >>> @@ -0,0 +1,40 @@
> >>> +/** @file
> >>> +  The capsule reset data difinition and guids.
> >>> +
> >>> +  Copyright (c) 2019, Intel Corporation. All rights
> >> reserved.<BR>
> >>> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> >>> +
> >>> +**/
> >>> +#ifndef __CAPSULE_RESET_DATA_H__
> >>> +#define __CAPSULE_RESET_DATA_H__
> >>> +
> >>> +#include <Uefi/UefiBaseType.h>
> >>> +
> >>> +//
> >>> +// reset data started with a null string and followed
> >> by a guid data
> >>> +//
> >>> +#pragma pack(1)
> >>> +typedef struct {
> >>> +  CHAR16      NullString;
> >>> +  EFI_GUID    ResetGuid;
> >>> +} RESET_DATA_WITH_NULL_STRING;
> >>> +#pragma pack()
> >>> +
> >>> +VERIFY_SIZE_OF (RESET_DATA_WITH_NULL_STRING, 18);
> >>
> >> I think that exposing RESET_DATA_WITH_NULL_STRING as a public
> >> structure is a good idea -- it indeed looks like something useful for
> >> many platforms and for different purposes.
> >>
> >> - However, I'd argue that it should not go into an include file
> >> called "CapsuleResetData.h". The type looks generic and not tied to
> >> capsules.
> >>
> >> - Additionally, I believe (but I'm not fully sure!) that new
> >> structures under MdeModulePkg/Include/Guid should be named EDKII_*.
> >>
> >> If the MdeModulePkg maintainers are OK with these patches in the
> >> current form, I'm fine too; I just thought we might want to consider
> >> the above points. (I certainly suggest waiting for their feedback
> >> before starting work on v2, just to address the above.)
> >>
> >> Thanks!
> >> Laszlo
> >>
> >>> +
> >>> +#define CAPSULE_ARMED_RESET_GUID \
> >>> +  { \
> >>> +    0xc6b4eea7, 0xfce2, 0x4625, { 0x9c, 0x4f, 0xc4,
> >> 0xb0, 0x82, 0x37, 0xae, 0x23 }  \
> >>> +  }
> >>> +
> >>> +#define CAPSULE_UPDATE_COMPLETE_RESET_GUID \
> >>> +  { \
> >>> +    0x5d512714, 0xa4df, 0x4e46, { 0xb6, 0xc7, 0xbc,
> >> 0x9f, 0x97, 0x9d, 0x59, 0xa0 }  \
> >>> +  }
> >>> +
> >>> +extern EFI_GUID gCapsuleArmedResetGuid;
> >>> +
> >>> +extern EFI_GUID gCapsuleUpdateCompleteResetGuid;
> >>> +
> >>> +#endif
> >>> +
> >>> diff --git a/MdeModulePkg/MdeModulePkg.dec
> >> b/MdeModulePkg/MdeModulePkg.dec
> >>> index 2ef48f2c37..c12b836c24 100644
> >>> --- a/MdeModulePkg/MdeModulePkg.dec
> >>> +++ b/MdeModulePkg/MdeModulePkg.dec
> >>> @@ -423,6 +423,11 @@
> >>>    ## Include/Guid/S3StorageDeviceInitList.h
> >>>    gS3StorageDeviceInitListGuid = { 0x310e9b8c,
> >> 0xcf90, 0x421e, { 0x8e, 0x9b, 0x9e, 0xef, 0xb6, 0x17, 0xc8, 0xef } }
> >>>
> >>> +  ## Guid to use for gRT->ResetSystem() to indicate
> >> the type of reset that is being performed.
> >>> +  #  Include/Guid/CapsuleResetData.h
> >>> +  gCapsuleArmedResetGuid            = {0xc6b4eea7,
> >> 0xfce2, 0x4625, {0x9c, 0x4f, 0xc4, 0xb0, 0x82, 0x37, 0xae, 0x23}}
> >>> +  gCapsuleUpdateCompleteResetGuid   = {0x5d512714,
> >> 0xa4df, 0x4e46, {0xb6, 0xc7, 0xbc, 0x9f, 0x97, 0x9d, 0x59, 0xa0}}
> >>> +
> >>>  [Ppis]
> >>>    ## Include/Ppi/AtaController.h
> >>>    gPeiAtaControllerPpiGuid       = { 0xa45e60d1,
> >> 0xc719, 0x44aa, { 0xb0, 0x7a, 0xaa, 0x77, 0x7f, 0x85, 0x90, 0x6d }}
> >>>
> >>
> >>
> >>
> >
> 
> 
> 


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

end of thread, other threads:[~2019-05-08  4:50 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2019-05-05  7:50 [PATCH 0/4] MdeModulePkg: Transfer reset data Gao, Zhichao
2019-05-05  7:50 ` [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid Gao, Zhichao
2019-05-06 15:44   ` [edk2-devel] " Laszlo Ersek
2019-05-06 21:02     ` Michael D Kinney
2019-05-07 10:14       ` Laszlo Ersek
2019-05-08  4:50         ` Gao, Zhichao
2019-05-05  7:50 ` [PATCH 2/4] MdeModulePkg/CapsuleRuntimeDxe: Transfer reset data Gao, Zhichao
2019-05-05  7:50 ` [PATCH 3/4] MdeModulePkg/CapsuleLib: " Gao, Zhichao
2019-05-05  7:50 ` [PATCH 4/4] MdeModulePkg/ResetUtilityLib: Replace the reset data difinition Gao, Zhichao

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