Re: [edk2platforms][PATCH v3 1/1] IntelSiliconPkg: Add IntelDieInfoProtocol

["Ni, Ray" <ray.ni@intel.com>]

Is there a reason to keep "Signature" field?

Do you expect consumers check the Signature before using? It's not a common practice in EDKII today.

Thanks,
ray

> -----Original Message-----
> From: Czajkowski, Maciej <maciej.czajkowski@intel.com>
> Sent: Thursday, November 18, 2021 11:16 PM
> To: devel@edk2.groups.io
> Cc: Ni, Ray <ray.ni@intel.com>; Chaganty, Rangasai V <rangasai.v.chaganty@intel.com>
> Subject: [edk2platforms][PATCH v3 1/1] IntelSiliconPkg: Add IntelDieInfoProtocol
> 
> Added IntelDieInfo header into IntelSiliconPkg tree.
> The purpose is to have generic and unified interface for getting
> information about dies installed in the system.
> It will be implemented by silicon code.
> 
> Cc: Ray Ni <ray.ni@intel.com>
> Cc: Rangasai V Chaganty <rangasai.v.chaganty@intel.com>
> Signed-off-by: Maciej Czajkowski <maciej.czajkowski@intel.com>
> ---
> 
> Notes:
>     v3:
>     - added EDKII_ prefix to interface
>     - removed GUID definitons from .h file
>     - corrected descriptions
>     - made PPI/protocol GUIDs unique
> 
>     v2:
>     - added interface description
>     - added die specific GUIDs into .dec file
> 
>  Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfo.h | 96 ++++++++++++++++++++
>  Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec             |  9 ++
>  2 files changed, 105 insertions(+)
> 
> diff --git a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfo.h
> b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfo.h
> new file mode 100644
> index 000000000000..6fb13ab287b6
> --- /dev/null
> +++ b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfo.h
> @@ -0,0 +1,96 @@
> +/** @file
> 
> +  IntelDieInfo definition
> 
> +
> 
> +  Copyright (c) 2021, Intel Corporation. All rights reserved.<BR>
> 
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> 
> +
> 
> +**/
> 
> +#ifndef _DIE_INFO_PROTOCOL_H_
> 
> +#define _DIE_INFO_PROTOCOL_H_
> 
> +
> 
> +typedef struct _EDKII_INTEL_DIE_INFO_PROTOCOL  EDKII_INTEL_DIE_INFO_PROTOCOL;
> 
> +typedef EDKII_INTEL_DIE_INFO_PROTOCOL  EDKII_INTEL_DIE_INFO_PPI;
> 
> +
> 
> +extern EFI_GUID gIntelDieInfoProtocolGuid;
> 
> +extern EFI_GUID gIntelDieInfoPpiGuid;
> 
> +
> 
> +extern EFI_GUID gIntelDieInfoPchGuid;
> 
> +extern EFI_GUID gIntelDieInfoSocGuid;
> 
> +extern EFI_GUID gIntelDieInfoIoGuid;
> 
> +extern EFI_GUID gIntelDieInfoCpuGuid;
> 
> +extern EFI_GUID gIntelDieInfoGfxGuid;
> 
> +
> 
> +#define DIE_INFO_PROTOCOL_REVISION 1
> 
> +#define DIE_INFO_PROTOCOL_SIGNATURE  SIGNATURE_32 ('I', 'D', 'I', 'P')
> 
> +
> 
> +/**
> 
> +  Returns pointer to constant string representing die name.
> 
> +  Name is specific to die type.
> 
> +
> 
> +  @param[in] This  Pointer to the DieInfoProtocol context structure
> 
> +  @retval Pointer to the const string
> 
> +**/
> 
> +typedef
> 
> +CONST CHAR8*
> 
> +(EFIAPI *EDKII_INTEL_DIE_INFO_GET_DIE_NAME_STR) (
> 
> +  IN EDKII_INTEL_DIE_INFO_PROTOCOL  *This
> 
> +  );
> 
> +
> 
> +/**
> 
> +  Returns pointer to constant string representing stepping of the die.
> 
> +
> 
> +  @param[in] This  Pointer to the DieInfoProtocol context structure
> 
> +  @retval Pointer to the const string
> 
> +**/
> 
> +typedef
> 
> +CONST CHAR8*
> 
> +(EFIAPI *EDKII_INTEL_DIE_INFO_GET_STEPPING_STR) (
> 
> +  IN EDKII_INTEL_DIE_INFO_PROTOCOL  *This
> 
> +  );
> 
> +
> 
> +/**
> 
> +  Returns pointer to constant string representing SKU of the die.
> 
> +
> 
> +  @param[in] This  Pointer to the DieInfoProtocol context structure
> 
> +  @retval Pointer to the const string
> 
> +**/
> 
> +typedef
> 
> +CONST CHAR8*
> 
> +(EFIAPI *EDKII_INTEL_DIE_INFO_GET_SKU_STR) (
> 
> +  IN EDKII_INTEL_DIE_INFO_PROTOCOL  *This
> 
> +  );
> 
> +
> 
> +/**
> 
> +  Protocol/PPI definition.
> 
> +  The purpose of this interface is to serve die-specific informations in a unified, generic way.
> 
> +  It will be produced by silicon code per die, and can be consumed by any module that needs contained information.
> 
> +
> 
> +  <b>Revision 1</b>:
> 
> +   - Initial version.
> 
> +**/
> 
> +struct _EDKII_INTEL_DIE_INFO_PROTOCOL {
> 
> +  UINT32                                 Signature; ///< Protocol signature
> 
> +  UINT32                                 Revision; ///< Current protocol revision
> 
> +  /**
> 
> +    Type of the die that particular instance is reffering to.
> 
> +  **/
> 
> +  EFI_GUID                               Type;
> 
> +  /**
> 
> +    Index of the die in the package.
> 
> +  **/
> 
> +  UINT32                                 DieIndex;
> 
> +  /**
> 
> +    Unique ID specific to the die and the associated generation.
> 
> +  **/
> 
> +  UINT64                                 DieId;
> 
> +  /**
> 
> +    Generation and die specific stepping ID.
> 
> +  **/
> 
> +  UINT32                                 SteppingId;
> 
> +
> 
> +  EDKII_INTEL_DIE_INFO_GET_DIE_NAME_STR  GetNameStr;
> 
> +  EDKII_INTEL_DIE_INFO_GET_STEPPING_STR  GetSteppingStr;
> 
> +  EDKII_INTEL_DIE_INFO_GET_SKU_STR       GetSkuStr;
> 
> +};
> 
> +
> 
> +#endif // _DIE_INFO_PROTOCOL_H_
> 
> diff --git a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> index 1704f9e02541..ee53f5fb902c 100644
> --- a/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> +++ b/Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec
> @@ -92,6 +92,13 @@ [Guids]
>    gFlashRegionAllGuid               = { 0xbabe60dc, 0xf88d, 0x4584, {0x9e, 0x54, 0x57, 0x44, 0x4b, 0xe2, 0x6e, 0xf3 } }
> 
>    gFlashRegionMaxGuid               = { 0x74c2e3c1, 0x8faa, 0x4659, {0xa7, 0xbb, 0x87, 0x1f, 0xbb, 0x61, 0xd3, 0xb4 } }
> 
> 
> 
> +  ## Include/Protocol/IntelDieInfo.h
> 
> +  gIntelDieInfoPchGuid = { 0x62CB6D68, 0x4771, 0x4569, { 0x81, 0xFA, 0x1E, 0x99, 0x6E, 0xA9, 0x91, 0xC5 }}
> 
> +  gIntelDieInfoSocGuid = { 0x63287105, 0x578E, 0x4799, { 0xBE, 0x55, 0x5D, 0xDA, 0xCA, 0x03, 0x74, 0xD0 }}
> 
> +  gIntelDieInfoIoGuid = { 0x23DA4C74, 0x54A0, 0x4E01, { 0x83, 0xB1, 0x8C, 0xA7, 0x43, 0x43, 0x1F, 0xF0 }}
> 
> +  gIntelDieInfoCpuGuid = { 0x6E5AF2E3, 0x5D84, 0x48F2, { 0x84, 0x28, 0x99, 0xE4, 0x93, 0x4F, 0x51, 0xE4 }}
> 
> +  gIntelDieInfoGfxGuid = { 0x1D3D2599, 0x7A1C, 0x4B1E, { 0x8C, 0xC5, 0x0F, 0x88, 0x27, 0xA0, 0x2E, 0xEC }}
> 
> +
> 
>  [Ppis]
> 
>    ## Include/Ppi/Spi.h
> 
>    gPchSpiPpiGuid = { 0x104c7177, 0xc2e6, 0x44f0, { 0xae, 0xe3, 0x9d, 0x0d, 0x9a, 0x52, 0xca, 0xdf } }
> 
> @@ -101,6 +108,7 @@ [Ppis]
> 
> 
>    gEdkiiVTdInfoPpiGuid = { 0x8a59fcb3, 0xf191, 0x400c, { 0x97, 0x67, 0x67, 0xaf, 0x2b, 0x25, 0x68, 0x4a } }
> 
>    gEdkiiVTdNullRootEntryTableGuid = { 0x3de0593f, 0x6e3e, 0x4542, { 0xa1, 0xcb, 0xcb, 0xb2, 0xdb, 0xeb, 0xd8, 0xff } }
> 
> +  gIntelDieInfoPpiGuid = { 0xF9E45CBF, 0x1E21, 0x434A, { 0x90, 0x88, 0x1D, 0x10, 0x38, 0xF3, 0x68, 0xF2 }}
> 
> 
> 
>  [Protocols]
> 
>    ## Protocols that provide services for the Intel(R) PCH SPI Host Controller Compatibility Interface
> 
> @@ -114,6 +122,7 @@ [Protocols]
>    gPchSmmSpi2ProtocolGuid = { 0x2d1c0c43, 0x20d3, 0x40ae, { 0x99, 0x07, 0x2d, 0xf0, 0xe7, 0x91, 0x21, 0xa5 } }
> 
> 
> 
>    gEdkiiPlatformVTdPolicyProtocolGuid = { 0x3d17e448, 0x466, 0x4e20, { 0x99, 0x9f, 0xb2, 0xe1, 0x34, 0x88, 0xee, 0x22 }}
> 
> +  gIntelDieInfoProtocolGuid = { 0xAED8A0A1, 0xFDE6, 0x4CF2, { 0xA3, 0x85, 0x08, 0xF1, 0x25, 0xF2, 0x40, 0x37 }}
> 
> 
> 
>    ## Protocol for device security policy.
> 
>    # Include/Protocol/PlatformDeviceSecurityPolicy.h
> 
> --
> 2.27.0.windows.1