[edk2-devel] [PATCH 1/4] MdeModulePkg: Add reset data difinition and guid

Tue May 7 10:14:33 UTC 2019

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 at edk2.groups.io [mailto:devel at edk2.groups.io]
>> On Behalf Of Laszlo Ersek
>> Sent: Monday, May 6, 2019 8:44 AM
>> To: devel at edk2.groups.io; Gao, Zhichao
>> <zhichao.gao at intel.com>
>> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>; Wang,
>> Jian J <jian.j.wang at intel.com>; Wu, Hao A
>> <hao.a.wu at intel.com>; Ni, Ray <ray.ni at intel.com>; Zeng,
>> Star <star.zeng at intel.com>; Gao, Liming
>> <liming.gao at intel.com>; Sean Brogan
>> <sean.brogan at microsoft.com>; Michael Turner
>> <Michael.Turner at 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 at 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 at intel.com>
>>> Cc: Hao Wu <hao.a.wu at intel.com>
>>> Cc: Ray Ni <ray.ni at intel.com>
>>> Cc: Star Zeng <star.zeng at intel.com>
>>> Cc: Liming Gao <liming.gao at intel.com>
>>> Cc: Sean Brogan <sean.brogan at microsoft.com>
>>> Cc: Michael Turner <Michael.Turner at microsoft.com>
>>> Cc: Bret Barkelew <Bret.Barkelew at microsoft.com>
>>> Signed-off-by: Zhichao Gao <zhichao.gao at 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 }}
>>>
>>
>>
>> 
> 

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#40103): https://edk2.groups.io/g/devel/message/40103
Mute This Topic: https://groups.io/mt/31507358/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-