Re: [edk2] [Patch v2 2/6] UefiCpuPkg/RegisterCpuFeaturesLib.h: Add new dependence types.

2018-10-17 Thread Ni, Ruiyu

On 10/17/2018 10:16 AM, Eric Dong wrote:

Add new core/package dependence types which consumed by different MSRs.

Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
  .../Include/Library/RegisterCpuFeaturesLib.h   | 25 ++
  1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index 9331e49d13..e6f0ebe4bc 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -73,10 +73,17 @@
  #define CPU_FEATURE_PPIN(32+11)
  #define CPU_FEATURE_PROC_TRACE  (32+12)
  
-#define CPU_FEATURE_BEFORE_ALL  BIT27

-#define CPU_FEATURE_AFTER_ALL   BIT28
-#define CPU_FEATURE_BEFORE  BIT29
-#define CPU_FEATURE_AFTER   BIT30
+#define CPU_FEATURE_BEFORE_ALL  BIT23
+#define CPU_FEATURE_AFTER_ALL   BIT24



We could add comments to emphasize that CPU_FEATURE_BEFORE and 
CPU_FEATURE_AFTER only mean Thread scope before and Thread scope after.


And after the whole patch serials are checked in, I prefer to mark the 
below two macros as deprecated and avoid using them in core code any more.



+#define CPU_FEATURE_BEFORE  BIT25
+#define CPU_FEATURE_AFTER   BIT26
+




+#define CPU_FEATURE_THREAD_BEFORE   CPU_FEATURE_BEFORE
+#define CPU_FEATURE_THREAD_AFTERCPU_FEATURE_AFTER
+#define CPU_FEATURE_CORE_BEFORE BIT27
+#define CPU_FEATURE_CORE_AFTER  BIT28
+#define CPU_FEATURE_PACKAGE_BEFORE  BIT29
+#define CPU_FEATURE_PACKAGE_AFTER   BIT30
  #define CPU_FEATURE_END MAX_UINT32
  /// @}
  
@@ -116,6 +123,16 @@ typedef struct {

CPUID_VERSION_INFO_EDX   CpuIdVersionInfoEdx;
  } REGISTER_CPU_FEATURE_INFORMATION;
  
+//

+// Describe the dependency type for different features.


Can you add comments to say like below?
"The value set to CPU_REGISTER_TABLE_ENTRY.Value when the REGISTER_TYPE 
is Semaphore."


And maybe move the enum definition to AcpiCpuData.h because the 
definition of CPU_REGISTER_TABLE_ENTRY and REGISTER_TYPE are all defined 
there.



+//
+typedef enum {
+  NoneDepType,
+  ThreadDepType,
+  CoreDepType,
+  PackageDepType
+} CPU_FEATURE_DEPENDENCE_TYPE; > +
  /**
Determines if a CPU feature is enabled in PcdCpuFeaturesSupport bit mask.
If a CPU feature is disabled in PcdCpuFeaturesSupport then all the code/data




--
Thanks,
Ray
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


[edk2] [Patch v2 2/6] UefiCpuPkg/RegisterCpuFeaturesLib.h: Add new dependence types.

2018-10-16 Thread Eric Dong
Add new core/package dependence types which consumed by different MSRs.

Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong 
---
 .../Include/Library/RegisterCpuFeaturesLib.h   | 25 ++
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h 
b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
index 9331e49d13..e6f0ebe4bc 100644
--- a/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
+++ b/UefiCpuPkg/Include/Library/RegisterCpuFeaturesLib.h
@@ -73,10 +73,17 @@
 #define CPU_FEATURE_PPIN(32+11)
 #define CPU_FEATURE_PROC_TRACE  (32+12)
 
-#define CPU_FEATURE_BEFORE_ALL  BIT27
-#define CPU_FEATURE_AFTER_ALL   BIT28
-#define CPU_FEATURE_BEFORE  BIT29
-#define CPU_FEATURE_AFTER   BIT30
+#define CPU_FEATURE_BEFORE_ALL  BIT23
+#define CPU_FEATURE_AFTER_ALL   BIT24
+#define CPU_FEATURE_BEFORE  BIT25
+#define CPU_FEATURE_AFTER   BIT26
+
+#define CPU_FEATURE_THREAD_BEFORE   CPU_FEATURE_BEFORE
+#define CPU_FEATURE_THREAD_AFTERCPU_FEATURE_AFTER
+#define CPU_FEATURE_CORE_BEFORE BIT27
+#define CPU_FEATURE_CORE_AFTER  BIT28
+#define CPU_FEATURE_PACKAGE_BEFORE  BIT29
+#define CPU_FEATURE_PACKAGE_AFTER   BIT30
 #define CPU_FEATURE_END MAX_UINT32
 /// @}
 
@@ -116,6 +123,16 @@ typedef struct {
   CPUID_VERSION_INFO_EDX   CpuIdVersionInfoEdx;
 } REGISTER_CPU_FEATURE_INFORMATION;
 
+//
+// Describe the dependency type for different features.
+//
+typedef enum {
+  NoneDepType,
+  ThreadDepType,
+  CoreDepType,
+  PackageDepType
+} CPU_FEATURE_DEPENDENCE_TYPE;
+
 /**
   Determines if a CPU feature is enabled in PcdCpuFeaturesSupport bit mask.
   If a CPU feature is disabled in PcdCpuFeaturesSupport then all the code/data
-- 
2.15.0.windows.1

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel