[PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[william2.wang]

From: William2 Wang <william2.wang@intel.com>

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

Update CPUID Leaf 06H to follow latest SDM.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
---
 MdePkg/Include/Register/Intel/Cpuid.h | 56 ++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/Register/Intel/Cpuid.h b/MdePkg/Include/Register/Intel/Cpuid.h
index 350bf60252..46cdb827e2 100644
--- a/MdePkg/Include/Register/Intel/Cpuid.h
+++ b/MdePkg/Include/Register/Intel/Cpuid.h
@@ -1195,12 +1195,24 @@ typedef union {
     /// [Bit 18] Fast access mode for the IA32_HWP_REQUEST MSR is supported if set.
     ///
     UINT32    FastAccessMode                         : 1;
-    UINT32    Reserved4                              : 1;
+    ///
+    /// [Bit 19] IA32_HW_FEEDBACK_PTR MSR, IA32_HW_FEEDBACK_CONFIG MSR,
+    /// IA32_PACKAGE_THERM_STATUS MSR bit 26, and IA32_PACKAGE_THERM_INTERRUPT MSR bit 25 are supported if set.
+    UINT32    HW_FEEDBACK                            : 1;
     ///
     /// [Bit 20] Ignoring Idle Logical Processor HWP request is supported if set.
     ///
     UINT32    IgnoringIdleLogicalProcessorHWPRequest : 1;
-    UINT32    Reserved5                              : 11;
+    UINT32    Reserved4                              : 2;
+    ///
+    /// [Bit 23] Intel Thread Director supported if set. IA32_HW_FEEDBACK_CHAR and
+    /// IA32_HW_FEEDBACK_THREAD_CONFIG MSRs are supported if set.
+    UINT32    ThreadDirector                         : 1;
+    ///
+    /// [Bit 24] IA32_THERM_INTERRUPT MSR bit 25 is supported if set.
+    ///
+    UINT32    IA32_THERM_INTERRUPT                   : 1;
+    UINT32    Reserved5                              : 7;
   } Bits;
   ///
   /// All bit fields as a 32-bit value
@@ -1252,7 +1264,13 @@ typedef union {
     /// (1B0H).
     ///
     UINT32    PerformanceEnergyBias        : 1;
-    UINT32    Reserved2                    : 28;
+    UINT32    Reserved2                    : 4;
+    ///
+    /// {Bit 15:8] Number of Intel Thread Director classes supported by the processor. Information for that
+    /// many classes is written into the Intel Thread Director Table by the hardware.
+    ///
+    UINT32    ThreadDirectorClasses        : 8;
+    UINT32    Reserved3                    : 16;
   } Bits;
   ///
   /// All bit fields as a 32-bit value
@@ -1260,6 +1278,38 @@ typedef union {
   UINT32    Uint32;
 } CPUID_THERMAL_POWER_MANAGEMENT_ECX;
 
+/**
+  CPUID Thermal and Power Management Information returned in EDX for CPUID leaf
+  #CPUID_THERMAL_POWER_MANAGEMENT.
+**/
+typedef union {
+  ///
+  /// Individual bit fields
+  ///
+  struct {
+    ///
+    /// {Bits 7:0] Bitmap of supported hardware feedback interface capabilities.
+    ///
+    UINT32    InterfaceCapability      : 8;
+    ///
+    /// {Bits 11:8] Enumerates the size of the hardware feedback interface structure in number of 4 KB pages;
+    /// add one to the return value to get the result.
+    ///
+    UINT32    InterfaceStructureSize   : 4;
+    UINT32    Reserved                 : 4;
+    ///
+    /// {Bits 31:16] : Index (starting at 0) of this logical processor's row in the hardware feedback interface structure.
+    /// Note that on some parts the index may be same for multiple logical processors. On some parts the
+    /// indices may not be contiguous, i.e., there may be unused rows in the hardware feedback interface structure.
+    ///
+    UINT32    LogicalProcessorRowIndex : 16;
+  } Bits;
+  ///
+  /// All bit fields as a 32-bit value
+  ///
+  UINT32    Uint32;
+} CPUID_THERMAL_POWER_MANAGEMENT_EDX;
+
 /**
   CPUID Structured Extended Feature Flags Enumeration
 
-- 
2.34.1.windows.1

Re: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[Donald Kuo]

Hi William

Overall looks good to me.

1. For EAX: Can we change Reserved4 & 5 to Reserved3 & 4, since only 4 reserved.

2. For EDX: Bits [7:0], how about to add more detail description for BIT0 and BIT1, and BIT [7:2] = Reserved to align with SDM


+    /// {Bits 7:0] Bitmap of supported hardware feedback interface capabilities.

+    ///

+    UINT32    InterfaceCapability      : 8;

Thanks,
	




-----Original Message-----
From: Wang, William2 <william2.wang@intel.com> 
Sent: Friday, January 13, 2023 4:26 PM
To: devel@edk2.groups.io
Cc: Wang, William2 <william2.wang@intel.com>; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Ni, Ray <ray.ni@intel.com>; Kuo, Donald <donald.kuo@intel.com>; Kumar, Chandana C <chandana.c.kumar@intel.com>
Subject: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

From: William2 Wang <william2.wang@intel.com>

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

Update CPUID Leaf 06H to follow latest SDM.

Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <gaoliming@byosoft.com.cn>
Cc: Ray Ni <ray.ni@intel.com>
Cc: Donald Kuo <Donald.Kuo@intel.com>
Cc: Chandana C Kumar <chandana.c.kumar@intel.com>
---
 MdePkg/Include/Register/Intel/Cpuid.h | 56 ++++++++++++++++++--
 1 file changed, 53 insertions(+), 3 deletions(-)

diff --git a/MdePkg/Include/Register/Intel/Cpuid.h b/MdePkg/Include/Register/Intel/Cpuid.h
index 350bf60252..46cdb827e2 100644
--- a/MdePkg/Include/Register/Intel/Cpuid.h
+++ b/MdePkg/Include/Register/Intel/Cpuid.h
@@ -1195,12 +1195,24 @@ typedef union {
     /// [Bit 18] Fast access mode for the IA32_HWP_REQUEST MSR is supported if set.

     ///

     UINT32    FastAccessMode                         : 1;

-    UINT32    Reserved4                              : 1;

+    ///

+    /// [Bit 19] IA32_HW_FEEDBACK_PTR MSR, IA32_HW_FEEDBACK_CONFIG MSR,

+    /// IA32_PACKAGE_THERM_STATUS MSR bit 26, and IA32_PACKAGE_THERM_INTERRUPT MSR bit 25 are supported if set.

+    UINT32    HW_FEEDBACK                            : 1;

     ///

     /// [Bit 20] Ignoring Idle Logical Processor HWP request is supported if set.

     ///

     UINT32    IgnoringIdleLogicalProcessorHWPRequest : 1;

-    UINT32    Reserved5                              : 11;

+    UINT32    Reserved4                              : 2;

+    ///

+    /// [Bit 23] Intel Thread Director supported if set. IA32_HW_FEEDBACK_CHAR and

+    /// IA32_HW_FEEDBACK_THREAD_CONFIG MSRs are supported if set.

+    UINT32    ThreadDirector                         : 1;

+    ///

+    /// [Bit 24] IA32_THERM_INTERRUPT MSR bit 25 is supported if set.

+    ///

+    UINT32    IA32_THERM_INTERRUPT                   : 1;

+    UINT32    Reserved5                              : 7;

   } Bits;

   ///

   /// All bit fields as a 32-bit value

@@ -1252,7 +1264,13 @@ typedef union {
     /// (1B0H).

     ///

     UINT32    PerformanceEnergyBias        : 1;

-    UINT32    Reserved2                    : 28;

+    UINT32    Reserved2                    : 4;

+    ///

+    /// {Bit 15:8] Number of Intel Thread Director classes supported by the processor. Information for that

+    /// many classes is written into the Intel Thread Director Table by the hardware.

+    ///

+    UINT32    ThreadDirectorClasses        : 8;

+    UINT32    Reserved3                    : 16;

   } Bits;

   ///

   /// All bit fields as a 32-bit value

@@ -1260,6 +1278,38 @@ typedef union {
   UINT32    Uint32;

 } CPUID_THERMAL_POWER_MANAGEMENT_ECX;

 

+/**

+  CPUID Thermal and Power Management Information returned in EDX for CPUID leaf

+  #CPUID_THERMAL_POWER_MANAGEMENT.

+**/

+typedef union {

+  ///

+  /// Individual bit fields

+  ///

+  struct {

+    ///

+    /// {Bits 7:0] Bitmap of supported hardware feedback interface capabilities.

+    ///

+    UINT32    InterfaceCapability      : 8;

+    ///

+    /// {Bits 11:8] Enumerates the size of the hardware feedback interface structure in number of 4 KB pages;

+    /// add one to the return value to get the result.

+    ///

+    UINT32    InterfaceStructureSize   : 4;

+    UINT32    Reserved                 : 4;

+    ///

+    /// {Bits 31:16] : Index (starting at 0) of this logical processor's row in the hardware feedback interface structure.

+    /// Note that on some parts the index may be same for multiple logical processors. On some parts the

+    /// indices may not be contiguous, i.e., there may be unused rows in the hardware feedback interface structure.

+    ///

+    UINT32    LogicalProcessorRowIndex : 16;

+  } Bits;

+  ///

+  /// All bit fields as a 32-bit value

+  ///

+  UINT32    Uint32;

+} CPUID_THERMAL_POWER_MANAGEMENT_EDX;

+

 /**

   CPUID Structured Extended Feature Flags Enumeration

 

-- 
2.34.1.windows.1

Re: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[Ni, Ray]

Donald,
Old code might reference existing Reserved field in the structure definition.
So, to avoid old code assigning value to different bits if using same field name but for different bits position,
edk2 recommends to change the field name from ReservedX to ReservedY when the bits covered by ReservedX
are changed. Y is the next unused index.

So comments as below:

> 
> -    UINT32    Reserved5                              : 11;
> 
> +    UINT32    Reserved4                              : 2;
We should rename Reserved5 to Reserved6.


> 
> +    UINT32    IA32_THERM_INTERRUPT                   : 1;
> 
> +    UINT32    Reserved5                              : 7;
Reserved7.

Re: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[Donald Kuo]

Thanks Ray to clarify.

@Wang, William2 let's follow the EDK2 recommendation.   

Thanks,

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, January 16, 2023 5:19 PM
To: Kuo, Donald <donald.kuo@intel.com>; Wang, William2 <william2.wang@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kumar, Chandana C <chandana.c.kumar@intel.com>
Subject: RE: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

Donald,
Old code might reference existing Reserved field in the structure definition.
So, to avoid old code assigning value to different bits if using same field name but for different bits position,
edk2 recommends to change the field name from ReservedX to ReservedY when the bits covered by ReservedX are changed. Y is the next unused index.

So comments as below:

> 
> -    UINT32    Reserved5                              : 11;
> 
> +    UINT32    Reserved4                              : 2;
We should rename Reserved5 to Reserved6.


> 
> +    UINT32    IA32_THERM_INTERRUPT                   : 1;
> 
> +    UINT32    Reserved5                              : 7;
Reserved7.

Re: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

[william2.wang]

OK, I've updated the reserved bit.

Thanks,
William Wang

-----Original Message-----
From: Kuo, Donald <donald.kuo@intel.com> 
Sent: Monday, January 16, 2023 5:23 PM
To: Ni, Ray <ray.ni@intel.com>; Wang, William2 <william2.wang@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kumar, Chandana C <chandana.c.kumar@intel.com>
Subject: RE: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

Thanks Ray to clarify.

@Wang, William2 let's follow the EDK2 recommendation.   

Thanks,

-----Original Message-----
From: Ni, Ray <ray.ni@intel.com> 
Sent: Monday, January 16, 2023 5:19 PM
To: Kuo, Donald <donald.kuo@intel.com>; Wang, William2 <william2.wang@intel.com>; devel@edk2.groups.io
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>; Kumar, Chandana C <chandana.c.kumar@intel.com>
Subject: RE: [PATCH v2] Update CPUID Leaf 06H to follow latest SDM.

Donald,
Old code might reference existing Reserved field in the structure definition.
So, to avoid old code assigning value to different bits if using same field name but for different bits position,
edk2 recommends to change the field name from ReservedX to ReservedY when the bits covered by ReservedX are changed. Y is the next unused index.

So comments as below:

> 
> -    UINT32    Reserved5                              : 11;
> 
> +    UINT32    Reserved4                              : 2;
We should rename Reserved5 to Reserved6.


> 
> +    UINT32    IA32_THERM_INTERRUPT                   : 1;
> 
> +    UINT32    Reserved5                              : 7;
Reserved7.