Re: [edk2-devel] [PATCH 3/4] MdePkg: Add EFI_PROPERTIES_TABLE

["Zhiguang Liu" <zhiguang.liu@intel.com>]

Hi Oleksiy,
I have added my comment inline.
And as I mentioned before, please use ECC check tool or PatchCheck.py to make sure your patch is align with edk2 coding style. Thanks
You can also refer to https://github.com/tianocore/tianocore.github.io/wiki/Code-Style-C to check edk2 C code style.

Thanks
Zhiguang

> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Oleksiy
> Yakovlev
> Sent: Saturday, April 11, 2020 3:58 AM
> To: devel@edk2.groups.io
> Cc: Gao, Liming <liming.gao@intel.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Felixp@ami.com; oleksiyy@ami.com
> Subject: [edk2-devel] [PATCH 3/4] MdePkg: Add EFI_PROPERTIES_TABLE
> 
> Define Guid & data structure for EFI_RT _PROPERTIES_TABLE, designed to be

The space in "EFI_RT _PROPERTIES_TABLE" is unnecessary.

> published by a
> platform if it no longer  supports all EFI runtime services once
> ExitBootServices()
> has been called by the OS.
> (UEFI 2.8 errata a, mantis 2049)
> 
> Signed-off-by: Oleksiy Yakovlev <oleksiyy@ami.com>
> ---
>  MdePkg/Include/Guid/RtPropertiesTable.h | 71
> +++++++++++++++++++++++++++++++++
>  MdePkg/MdePkg.dec                       |  6 +++
>  2 files changed, 77 insertions(+)
>  create mode 100644 MdePkg/Include/Guid/RtPropertiesTable.h
> 
> diff --git a/MdePkg/Include/Guid/RtPropertiesTable.h
> b/MdePkg/Include/Guid/RtPropertiesTable.h
> new file mode 100644
> index 0000000..a5b6c14
> --- /dev/null
> +++ b/MdePkg/Include/Guid/RtPropertiesTable.h
> @@ -0,0 +1,71 @@
> +/** @file
> +Guid & data structure for EFI_RT _PROPERTIES_TABLE, designed to be

The space in "EFI_RT _PROPERTIES_TABLE" is unnecessary.

> published by a
> +platform if it no longer  supports all EFI runtime services once
> ExitBootServices()
> +has been called by the OS.
> +
> +
> +Copyright (c) 2020, American Megatrends International LLC. All rights
> reserved.<BR>
> +SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#ifndef _RT _PROPERTIES_TABLE_GUID_H__
> +#define _RT _PROPERTIES_TABLE_GUID_H__
> +
> +//
> +// Table, defined here, should be published by a platform if it no longer
> supports all EFI runtime
> +// services once ExitBootServices() has been called by the OS. Note that this
> is merely a hint
> +// to the OS, which it is free to ignore, and so the platform is still required to
> provide callable
> +// implementations of unsupported runtime services that simply return
> EFI_UNSUPPORTED.
> +//
> +#define EFI_RT_PROPERTIES_TABLE_GUID \
> +    { 0xeb66918a, 0x7eef, 0x402a, \
> +    { 0x84, 0x2e, 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9 }}
> +
> +
> +
> +
> +#pragma pack(1)
> +
> +typedef struct {
> +  ///
> +  /// Version of the structure, must be 0x1.
> +  ///
> +  UINT16 Version;
> +
> +  ///
> +  /// Size in bytes of the entire EFI_RT_PROPERTIES_TABLE, must be 8.
> +  ///
> +  UINT16 Length;
> +
> +  ///
> +  /// The length of the JSON payload immediately following this header, in
> bytes.
> +  ///

The upper comments is incorrect.

> +  UINT32 RuntimeServicesSupported;
> +
> +  ///
> +  /// Bitmask of which calls are or are not supported, where a bit set to 1
> indicates
> +  /// that the call is supported, and 0 indicates that it is not.
> +  ///

Move the comment up to describe RuntimeServicesSupported

> +} EFI_RT_PROPERTIES_TABLE;
> +
> +#pragma pack()
> +

I think " #define EFI_RT_PROPERTIES_TABLE_VERSION 0x1" should be added

> +#define EFI_RT_SUPPORTED_GET_TIME                       0x0001
> +#define EFI_RT_SUPPORTED_SET_TIME                       0x0002
> +#define EFI_RT_SUPPORTED_GET_WAKEUP_TIME                0x0004
> +#define EFI_RT_SUPPORTED_SET_WAKEUP_TIME                0x0008
> +#define EFI_RT_SUPPORTED_GET_VARIABLE                   0x0010
> +#define EFI_RT_SUPPORTED_GET_NEXT_VARIABLE_NAME         0x0020
> +#define EFI_RT_SUPPORTED_SET_VARIABLE                   0x0040
> +#define EFI_RT_SUPPORTED_SET_VIRTUAL_ADDRESS_MAP        0x0080
> +#define EFI_RT_SUPPORTED_CONVERT_POINTER                0x0100
> +#define EFI_RT_SUPPORTED_GET_NEXT_HIGH_MONOTONIC_COUNT
> 0x0200
> +#define EFI_RT_SUPPORTED_RESET_SYSTEM                   0x0400
> +#define EFI_RT_SUPPORTED_UPDATE_CAPSULE                 0x0800
> +#define EFI_RT_SUPPORTED_QUERY_CAPSULE_CAPABILITIES     0x1000
> +#define EFI_RT_SUPPORTED_QUERY_VARIABLE_INFO            0x2000
> +
> +extern EFI_GUID gEfiRtPropertiesTableGuid;
> +
> +#endif
> diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec
> index 99e75f7..d5340c5 100644
> --- a/MdePkg/MdePkg.dec
> +++ b/MdePkg/MdePkg.dec
> @@ -655,6 +655,12 @@
>    gEfiBttAbstractionGuid         = { 0x18633bfc, 0x1735, 0x4217, { 0x8a, 0xc9,
> 0x17, 0x23, 0x92, 0x82, 0xd3, 0xf8 }}
> 
>    #
> +  # GUIDs defined in UEFI2.8a
> +  #
> +  ## Include/Guid/RtPropertiesTable.h
> +  gEfiRtPropertiesTableGuid      = { 0xeb66918a, 0x7eef, 0x402a, { 0x84, 0x2e,
> 0x93, 0x1d, 0x21, 0xc3, 0x8a, 0xe9 }}
> +
> +  #
>    # GUID defined in PI1.0
>    #
>    ## Include/Guid/AprioriFileName.h
> --
> 2.9.0.windows.1
> 
> 
> Please consider the environment before printing this email.
> 
> The information contained in this message may be confidential and
> proprietary to American Megatrends (AMI).  This communication is intended
> to be read only by the individual or entity to whom it is addressed or by their
> designee. If the reader of this message is not the intended recipient, you are
> on notice that any distribution of this message, in any form, is strictly
> prohibited.  Please promptly notify the sender by reply e-mail or by
> telephone at 770-246-8600, and then delete or destroy all copies of the
> transmission.
> 
>