Re: [edk2-devel] [edk2platforms][PATCH v2 1/1] IntelSiliconPkg: Add IntelDieInfoProtocol

2021-11-02 Thread Chaganty, Rangasai V
Thanks for incorporating the feedback. 
Few more minor comments based on the new patch:
1. The description of "DieIndex" perhaps should be modified to " Index of the 
die in the package" to make it more apt. 
2. The description of "DieId" perhaps should be modified to " Unique ID 
specific to the die and the associated generation" to make it more apt.

Regards,
Sai
-Original Message-
From: Czajkowski, Maciej  
Sent: Monday, October 18, 2021 6:57 AM
To: devel@edk2.groups.io
Cc: Ni, Ray ; Chaganty, Rangasai V 

Subject: [edk2platforms][PATCH v2 1/1] IntelSiliconPkg: Add IntelDieInfoProtocol

Added IntelDieInfoProtocol 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.

Change-Id: Iedc414d435c27f37e6f12e7affd046a0a9e7e19d
Cc: Ray Ni 
Cc: Rangasai V Chaganty 
Signed-off-by: Maciej Czajkowski 
---

Notes:
v2:
- added interface description
- added die specific GUIDs into .dec file

 Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfoProtocol.h | 117 

 Silicon/Intel/IntelSiliconPkg/IntelSiliconPkg.dec |   9 ++
 2 files changed, 126 insertions(+)

diff --git 
a/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfoProtocol.h 
b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfoProtocol.h
new file mode 100644
index ..954c9ee10c22
--- /dev/null
+++ b/Silicon/Intel/IntelSiliconPkg/Include/Protocol/IntelDieInfoProtoco
+++ l.h
@@ -0,0 +1,117 @@
+/** @file+  IntelDieInfoProtocol definition++  Copyright (c) 2021, Intel 
Corporation. All rights reserved.+  SPDX-License-Identifier: 
BSD-2-Clause-Patent++**/+#ifndef _DIE_INFO_PROTOCOL_H_+#define 
_DIE_INFO_PROTOCOL_H_++typedef struct _INTEL_DIE_INFO_PROTOCOL  
INTEL_DIE_INFO_PROTOCOL;+typedef INTEL_DIE_INFO_PROTOCOL  
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 
INTEL_DIE_INFO_PPI_GUID \+{ 0xAED8A0A1, 0xFDE6, 0x4CF2, { 0xA3, 0x85, 0x08, 
0xF1, 0x25, 0xF2, 0x40, 0x37 }}++#define INTEL_DIE_INFO_PROTOCOL_GUID \+{ 
0xAED8A0A1, 0xFDE6, 0x4CF2, { 0xA3, 0x85, 0x08, 0xF1, 0x25, 0xF2, 0x40, 0x37 
}}++#define DIE_INFO_PROTOCOL_REVISION 1+#define DIE_INFO_PROTOCOL_SIGNATURE  
SIGNATURE_32 ('I', 'D', 'I', 'P')++#define DIE_INFO_PCH_GUID \+{ 0x62CB6D68, 
0x4771, 0x4569, { 0x81, 0xFA, 0x1E, 0x99, 0x6E, 0xA9, 0x91, 0xC5 }}++#define 
DIE_INFO_SOC_GUID \+{ 0x63287105, 0x578E, 0x4799, { 0xBE, 0x55, 0x5D, 0xDA, 
0xCA, 0x03, 0x74, 0xD0 }}++#define DIE_INFO_IO_GUID \+{ 0x23DA4C74, 0x54A0, 
0x4E01, { 0x83, 0xB1, 0x8C, 0xA7, 0x43, 0x43, 0x1F, 0xF0 }}++#define 
DIE_INFO_CPU_GUID \+{ 0x6E5AF2E3, 0x5D84, 0x48F2, { 0x84, 0x28, 0x99, 0xE4, 
0x93, 0x4F, 0x51, 0xE4 }}++#define DIE_INFO_GFX_GUID \+{ 0x1D3D2599, 0x7A1C, 
0x4B1E, { 0x8C, 0xC5, 0x0F, 0x88, 0x27, 0xA0, 0x2E, 0xEC }}++/**+  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 
*INTEL_DIE_INFO_GET_DIE_NAME_STR) (+  IN 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 
*INTEL_DIE_INFO_GET_STEPPING_STR) (+  IN 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 
*INTEL_DIE_INFO_GET_SKU_STR) (+  IN 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.++  Revision 1:+   - Initial version.+**/+struct 
_INTEL_DIE_INFO_PROTOCOL {+  UINT32   Signature; ///< 
Protocol signature+  UINT32   Revision; ///< Current 
protocol revision+  /**+Type of the die that particular instance is 
reffering to. See DIE_INFO_*_GUID+  **/+  EFI_GUID 
Type;+  /**+Index of the die in the system.+  **/+  UINT32  
 DieIndex;+  /**+Generation and die specific ID number.+  **/+  
UINT64   DieId;+  /**+Generation and die specific 
stepping ID.+  **/+  UINT32   SteppingId;++  
INTEL_DIE_INFO_GET_DIE_NAME_STR  GetNameStr;+  

Re: [edk2-devel] [edk2platforms][PATCH v2 1/1] IntelSiliconPkg: Add IntelDieInfoProtocol

2021-10-26 Thread Maciej Czajkowski
Hi Ray, Sai,

Could you take a look on this patch?

Pasting below questions and answers from previous patch for a reference.

What's the difference between SOC and CPU?
SOC die differs from typical CPU die in Intel's chiplet design.
 
What the purpose of "DieIndex"?
We want to have an indication to which exactly die given protocol instance is 
referring to (in example in multi socket/multi PCH designs).
 
Have you considered to use data fields such as "CONST CHAR8 * DieName, 
Stepping, Sku" instead of the methods?
Yes, but there might be a configuration where chipset id is going to be changed 
during the boot. By using methods, we ensure that returned value is correct in 
every call.

Can you add some details on how a producer of this interface would know the 
values to be assigned here? Does this contain any sort of encoding of 
generation and Die specific ID in a single UINT64?
Yes, in order to have a generic field regardless of the die type and 
per-generation changes it will be packed into a single UINT64. Protocol will be 
installed by silicon code, and consumer will need to be aware of the encoding 
and decode it. Same applies for stepping id.
 
Any reason the die specific GUIDs are not declared in the .dec file?
I can't see any problems with that, they will be added in v2 changes.
-
Intel Technology Poland sp. z o.o.
ul. Slowackiego 173 | 80-298 Gdansk | Sad Rejonowy Gdansk Polnoc | VII Wydzial 
Gospodarczy Krajowego Rejestru Sadowego - KRS 101882 | NIP 957-07-52-316 | 
Kapital zakladowy 200.000 PLN.
Ta wiadomosc wraz z zalacznikami jest przeznaczona dla okreslonego adresata i 
moze zawierac informacje poufne. W razie przypadkowego otrzymania tej 
wiadomosci, prosimy o powiadomienie nadawcy oraz trwale jej usuniecie; 
jakiekolwiek przegladanie lub rozpowszechnianie jest zabronione.
This e-mail and any attachments may contain confidential material for the sole 
use of the intended recipient(s). If you are not the intended recipient, please 
contact the sender and delete all copies; any review or distribution by others 
is strictly prohibited.



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#82709): https://edk2.groups.io/g/devel/message/82709
Mute This Topic: https://groups.io/mt/86608843/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-