public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [Patch 0/2] Remove redundant definition for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme
@ 2017-08-23  2:31 Eric Dong
  2017-08-23  2:31 ` [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
  2017-08-23  2:31 ` [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
  0 siblings, 2 replies; 6+ messages in thread
From: Eric Dong @ 2017-08-23  2:31 UTC (permalink / raw)
  To: edk2-devel

These two PCD have redundant definition which can be handle by code. This patch series update the
code and default value for these two PCDs.

Eric Dong (2):
  UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  UefiCpuPkg: Update default for
    PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.

 UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c | 14 ++++----------
 UefiCpuPkg/UefiCpuPkg.dec                           | 14 ++++++--------
 UefiCpuPkg/UefiCpuPkg.uni                           |  8 +++-----
 3 files changed, 13 insertions(+), 23 deletions(-)

-- 
2.7.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  2017-08-23  2:31 [Patch 0/2] Remove redundant definition for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
@ 2017-08-23  2:31 ` Eric Dong
  2017-08-23 23:02   ` Kinney, Michael D
  2017-08-23  2:31 ` [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Dong @ 2017-08-23  2:31 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni

The EnumProcTraceMemDisable/OutputSchemeInvalid are redundant
definitions. These definitions can be handled by other code,
so remove them.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
index a90dd4e..6524882 100644
--- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
+++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
@@ -35,8 +35,7 @@ typedef enum {
   Enum16M,
   Enum32M,
   Enum64M,
-  Enum128M,
-  EnumProcTraceMemDisable
+  Enum128M
 } PROC_TRACE_MEM_SIZE;
 
 ///
@@ -44,8 +43,7 @@ typedef enum {
 ///
 typedef enum {
   OutputSchemeSingleRange = 0,
-  OutputSchemeToPA,
-  OutputSchemeInvalid
+  OutputSchemeToPA
 } PROC_TRACE_OUTPUT_SCHEME;
 
 typedef struct  {
@@ -134,10 +132,6 @@ ProcTraceSupport (
   // Check if ProcTraceMemorySize option is enabled (0xFF means disable by user)
   //
   ProcTraceData = (PROC_TRACE_DATA *) ConfigData;
-  if ((ProcTraceData->ProcTraceMemSize >= EnumProcTraceMemDisable) ||
-      (ProcTraceData->ProcTraceOutputScheme >= OutputSchemeInvalid)) {
-    return FALSE;
-  }
 
   //
   // Check if Processor Trace is supported
@@ -151,8 +145,8 @@ ProcTraceSupport (
   AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE, CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, NULL, &Ecx.Uint32, NULL);
   ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported = (BOOLEAN) (Ecx.Bits.RTIT == 1);
   ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported = (BOOLEAN) (Ecx.Bits.SingleRangeOutput == 1);
-  if (ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported || 
-      ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported) {
+  if ((ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported && (ProcTraceData->ProcTraceOutputScheme == OutputSchemeToPA)) ||
+      (ProcTraceData->ProcessorData[ProcessorNumber].SingleRangeSupported && (ProcTraceData->ProcTraceOutputScheme == OutputSchemeSingleRange))) {
     return TRUE;
   }
 
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.
  2017-08-23  2:31 [Patch 0/2] Remove redundant definition for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
  2017-08-23  2:31 ` [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
@ 2017-08-23  2:31 ` Eric Dong
  2017-08-23 22:00   ` Kinney, Michael D
  1 sibling, 1 reply; 6+ messages in thread
From: Eric Dong @ 2017-08-23  2:31 UTC (permalink / raw)
  To: edk2-devel; +Cc: Michael Kinney, Ruiyu Ni

These two definitions have redundant definition which can be handle by code.
This patch update them to follow new code definitions.

Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Eric Dong <eric.dong@intel.com>
---
 UefiCpuPkg/UefiCpuPkg.dec | 14 ++++++--------
 UefiCpuPkg/UefiCpuPkg.uni |  8 +++-----
 2 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
index b4e099d..51ae0e2 100644
--- a/UefiCpuPkg/UefiCpuPkg.dec
+++ b/UefiCpuPkg/UefiCpuPkg.dec
@@ -286,7 +286,7 @@
   gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000019
 
   ## Contains the size of memory required when CPU processor trace is enabled.<BR><BR>
-  #  Default value is 0x10 which disables the processor trace.<BR>
+  #  Default value is 0x0 which use 4K memory size.<BR>
   #  0x0  -  4K.<BR>
   #  0x1  -  8K.<BR>
   #  0x2  -  16K.<BR>
@@ -303,19 +303,17 @@
   #  0xD  -  32M.<BR>
   #  0xE  -  64M.<BR>
   #  0xF  -  128M.<BR>
-  #  0x10 -  ProcTraceMemDisable.<BR>
   # @Prompt The memory size used for processor trace.
-  # @ValidRange  0x80000001 | 0 - 0x10
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x10|UINT32|0x60000012
+  # @ValidRange  0x80000001 | 0 - 0xF
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x0|UINT32|0x60000012
 
   ## Contains the processor trace output scheme when CPU processor trace is enabled.<BR><BR>
-  #  Default value is 2 which disables the processor trace.<BR>
+  #  Default value is 1 which use single range output scheme.<BR>
   #  0 - Single Range output scheme.<BR>
   #  1 - ToPA(Table of physical address) scheme.<BR>
-  #  2 - Invalid scheme.<BR>
   # @Prompt The processor trace output scheme.
-  # @ValidRange  0x80000001 | 0 - 2
-  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x2|UINT8|0x60000015
+  # @ValidRange  0x80000001 | 0 - 1
+  gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x0|UINT8|0x60000015
 
 [UserExtensions.TianoCore."ExtraFiles"]
   UefiCpuPkgExtra.uni
diff --git a/UefiCpuPkg/UefiCpuPkg.uni b/UefiCpuPkg/UefiCpuPkg.uni
index 858e4a7..f3be041 100644
--- a/UefiCpuPkg/UefiCpuPkg.uni
+++ b/UefiCpuPkg/UefiCpuPkg.uni
@@ -198,7 +198,7 @@
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_PROMPT  #language en-US "Memory size used by Processor Trace feature."
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_HELP  #language en-US "User input the memory size can be used by processor trace feature.<BR><BR>\n"
-                                                                                   "Default value is 0x10 which disables the processor memory trace.<BR>\n"
+                                                                                   "Default value is 0x0 which use 4K memory size.<BR>\n"
                                                                                    "0x0  -  4K.<BR>\n"
                                                                                    "0x1  -  8K.<BR>\n"
                                                                                    "0x2  -  16K.<BR>\n"
@@ -215,12 +215,10 @@
                                                                                    "0xD  -  32M.<BR>\n"
                                                                                    "0xE  -  64M.<BR>\n"
                                                                                    "0xF  -  128M.<BR>\n"
-                                                                                   "0x10 -  ProcTraceMemDisable.<BR>\n"
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_PROMPT  #language en-US "Processor Trace output scheme type."
 
 #string STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_HELP  #language en-US "User input the processor trace output scheme type.<BR><BR>\n"
-                                                                                        "Default value is 2 which disables the processor memory trace.<BR>\n"

+                                                                                        "Default value is 0 which use single range output scheme.<BR>\n"

                                                                                         "0 - Single Range output scheme.<BR>\n"

-                                                                                        "1 - ToPA(Table of physical address) scheme.<BR>\n"

-                                                                                        "2 - Invalid scheme.<BR>\n"
\ No newline at end of file
+                                                                                        "1 - ToPA(Table of physical address) scheme.<BR>\n"
\ No newline at end of file
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.
  2017-08-23  2:31 ` [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
@ 2017-08-23 22:00   ` Kinney, Michael D
  0 siblings, 0 replies; 6+ messages in thread
From: Kinney, Michael D @ 2017-08-23 22:00 UTC (permalink / raw)
  To: Dong, Eric, edk2-devel@lists.01.org, Kinney, Michael D; +Cc: Ni, Ruiyu

Eric,

Some Comments embedded below.  

Please make sure the UNI file is updated to match the DEC file.

For the enum PROC_TRACE_MEM_SIZE, I would prefer the enum values
have more than just "Enum" in the beginning.  It would be better
to use names like "ProcTraceMemSize" so the first enum value would
be "ProcTraceMemSize4KB".

Same for PROC_TRACE_OUTPUT_SCHEME.  Use "ProcTraceOutputScheme".

Thanks,

Mike

> -----Original Message-----
> From: Dong, Eric
> Sent: Tuesday, August 22, 2017 7:31 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>
> Subject: [Patch 2/2] UefiCpuPkg: Update default for
> PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme.
> 
> These two definitions have redundant definition which can be
> handle by code.
> This patch update them to follow new code definitions.
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/UefiCpuPkg.dec | 14 ++++++--------
>  UefiCpuPkg/UefiCpuPkg.uni |  8 +++-----
>  2 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec
> b/UefiCpuPkg/UefiCpuPkg.dec
> index b4e099d..51ae0e2 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -286,7 +286,7 @@
>    gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000019
> 
>    ## Contains the size of memory required when CPU processor
> trace is enabled.<BR><BR>

Add a reference to the PCD and bit that enables/disables CPU 
Processor trace and describe that this PCD is ignored if
CPU processor trace is disabled.


> -  #  Default value is 0x10 which disables the processor
> trace.<BR>
> +  #  Default value is 0x0 which use 4K memory size.<BR>

"Default value is 0x00 which means 4KB of memory is allocated if CPU processor trace is enabled."


>    #  0x0  -  4K.<BR>
>    #  0x1  -  8K.<BR>
>    #  0x2  -  16K.<BR>
> @@ -303,19 +303,17 @@
>    #  0xD  -  32M.<BR>
>    #  0xE  -  64M.<BR>
>    #  0xF  -  128M.<BR>
> -  #  0x10 -  ProcTraceMemDisable.<BR>
>    # @Prompt The memory size used for processor trace.
> -  # @ValidRange  0x80000001 | 0 - 0x10
> -
> gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x10|UINT32|0x
> 60000012
> +  # @ValidRange  0x80000001 | 0 - 0xF
> +
> gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceMemSize|0x0|UINT32|0x6
> 0000012
> 
>    ## Contains the processor trace output scheme when CPU
> processor trace is enabled.<BR><BR>

Add a reference to the PCD and bit that enables/disable CPU 
Processor trace and describe that this PCD is ignored if
CPU processor trace is disabled.

> -  #  Default value is 2 which disables the processor
> trace.<BR>
> +  #  Default value is 1 which use single range output
> scheme.<BR>

The default value in the statement below is 0, not 1.
The UNI file looks correct.

>    #  0 - Single Range output scheme.<BR>
>    #  1 - ToPA(Table of physical address) scheme.<BR>
> -  #  2 - Invalid scheme.<BR>
>    # @Prompt The processor trace output scheme.
> -  # @ValidRange  0x80000001 | 0 - 2
> -
> gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x2|UINT8
> |0x60000015
> +  # @ValidRange  0x80000001 | 0 - 1
> +
> gUefiCpuPkgTokenSpaceGuid.PcdCpuProcTraceOutputScheme|0x0|UINT8
> |0x60000015
> 
>  [UserExtensions.TianoCore."ExtraFiles"]
>    UefiCpuPkgExtra.uni
> diff --git a/UefiCpuPkg/UefiCpuPkg.uni
> b/UefiCpuPkg/UefiCpuPkg.uni
> index 858e4a7..f3be041 100644
> --- a/UefiCpuPkg/UefiCpuPkg.uni
> +++ b/UefiCpuPkg/UefiCpuPkg.uni
> @@ -198,7 +198,7 @@
>  #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_PROMPT
> #language en-US "Memory size used by Processor Trace feature."
> 
>  #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceMemSize_HELP
> #language en-US "User input the memory size can be used by
> processor trace feature.<BR><BR>\n"
> -
> "Default value is 0x10 which disables the processor memory
> trace.<BR>\n"
> +
> "Default value is 0x0 which use 4K memory size.<BR>\n"
> 
> "0x0  -  4K.<BR>\n"
> 
> "0x1  -  8K.<BR>\n"
> 
> "0x2  -  16K.<BR>\n"
> @@ -215,12 +215,10 @@
> 
> "0xD  -  32M.<BR>\n"
> 
> "0xE  -  64M.<BR>\n"
> 
> "0xF  -  128M.<BR>\n"
> -
> "0x10 -  ProcTraceMemDisable.<BR>\n"
> 
>  #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_PROMP
> T  #language en-US "Processor Trace output scheme type."
> 
>  #string
> STR_gUefiCpuPkgTokenSpaceGuid_PcdCpuProcTraceOutputScheme_HELP
> #language en-US "User input the processor trace output scheme
> type.<BR><BR>\n"
> -
> "Default value is 2 which disables the processor memory
> trace.<BR>\n"
> 
> +
> "Default value is 0 which use single range output
> scheme.<BR>\n"
> 
> 
> "0 - Single Range output scheme.<BR>\n"
> 
> -
> "1 - ToPA(Table of physical address) scheme.<BR>\n"
> 
> -
> "2 - Invalid scheme.<BR>\n"
> \ No newline at end of file
> +
> "1 - ToPA(Table of physical address) scheme.<BR>\n"
> \ No newline at end of file
> --
> 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  2017-08-23  2:31 ` [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
@ 2017-08-23 23:02   ` Kinney, Michael D
  2017-08-24  3:08     ` Dong, Eric
  0 siblings, 1 reply; 6+ messages in thread
From: Kinney, Michael D @ 2017-08-23 23:02 UTC (permalink / raw)
  To: Dong, Eric, edk2-devel@lists.01.org, Kinney, Michael D; +Cc: Ni, Ruiyu

Eric,

Comment below.

Mike

> -----Original Message-----
> From: Dong, Eric
> Sent: Tuesday, August 22, 2017 7:31 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu
> <ruiyu.ni@intel.com>
> Subject: [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove
> redundant definition.
> 
> The EnumProcTraceMemDisable/OutputSchemeInvalid are redundant
> definitions. These definitions can be handled by other code,
> so remove them.
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c | 14 ++++----
> ------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> index a90dd4e..6524882 100644
> --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> @@ -35,8 +35,7 @@ typedef enum {
>    Enum16M,
>    Enum32M,
>    Enum64M,
> -  Enum128M,
> -  EnumProcTraceMemDisable
> +  Enum128M
>  } PROC_TRACE_MEM_SIZE;
> 
>  ///
> @@ -44,8 +43,7 @@ typedef enum {
>  ///
>  typedef enum {
>    OutputSchemeSingleRange = 0,
> -  OutputSchemeToPA,
> -  OutputSchemeInvalid
> +  OutputSchemeToPA
>  } PROC_TRACE_OUTPUT_SCHEME;
> 
>  typedef struct  {
> @@ -134,10 +132,6 @@ ProcTraceSupport (
>    // Check if ProcTraceMemorySize option is enabled (0xFF means
> disable by user)
>    //
>    ProcTraceData = (PROC_TRACE_DATA *) ConfigData;
> -  if ((ProcTraceData->ProcTraceMemSize >=
> EnumProcTraceMemDisable) ||
> -      (ProcTraceData->ProcTraceOutputScheme >=
> OutputSchemeInvalid)) {
> -    return FALSE;
> -  }

I see the ProcTraceOutputScheme values are checked below.
Do we need to keep the check for a valid ProcTraceMemSize value?

> 
>    //
>    // Check if Processor Trace is supported
> @@ -151,8 +145,8 @@ ProcTraceSupport (
>    AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE,
> CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, NULL, &Ecx.Uint32,
> NULL);
>    ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported =
> (BOOLEAN) (Ecx.Bits.RTIT == 1);
>    ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported = (BOOLEAN)
> (Ecx.Bits.SingleRangeOutput == 1);
> -  if (ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported
> ||
> -      ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported) {
> +  if ((ProcTraceData-
> >ProcessorData[ProcessorNumber].TopaSupported && (ProcTraceData-
> >ProcTraceOutputScheme == OutputSchemeToPA)) ||
> +      (ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported &&
> (ProcTraceData->ProcTraceOutputScheme ==
> OutputSchemeSingleRange))) {
>      return TRUE;
>    }
> 
> --
> 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
  2017-08-23 23:02   ` Kinney, Michael D
@ 2017-08-24  3:08     ` Dong, Eric
  0 siblings, 0 replies; 6+ messages in thread
From: Dong, Eric @ 2017-08-24  3:08 UTC (permalink / raw)
  To: Kinney, Michael D, edk2-devel@lists.01.org, Kinney, Michael D; +Cc: Ni, Ruiyu

Mike,

The new check for ProcTraceOutputScheme is for the functionality which is missed before. the user selection and hardware capability may not consistent. So I add this new check.

I agree to keep the validate check. Please check the new patch.

Thanks,
Eric
-----Original Message-----
From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Kinney, Michael D
Sent: Thursday, August 24, 2017 7:03 AM
To: Dong, Eric <eric.dong@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
Cc: Ni, Ruiyu <ruiyu.ni@intel.com>
Subject: Re: [edk2] [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.

Eric,

Comment below.

Mike

> -----Original Message-----
> From: Dong, Eric
> Sent: Tuesday, August 22, 2017 7:31 PM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Ni, Ruiyu 
> <ruiyu.ni@intel.com>
> Subject: [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant 
> definition.
> 
> The EnumProcTraceMemDisable/OutputSchemeInvalid are redundant 
> definitions. These definitions can be handled by other code, so remove 
> them.
> 
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.dong@intel.com>
> ---
>  UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c | 14 ++++----
> ------
>  1 file changed, 4 insertions(+), 10 deletions(-)
> 
> diff --git a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> index a90dd4e..6524882 100644
> --- a/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> +++ b/UefiCpuPkg/Library/CpuCommonFeaturesLib/ProcTrace.c
> @@ -35,8 +35,7 @@ typedef enum {
>    Enum16M,
>    Enum32M,
>    Enum64M,
> -  Enum128M,
> -  EnumProcTraceMemDisable
> +  Enum128M
>  } PROC_TRACE_MEM_SIZE;
> 
>  ///
> @@ -44,8 +43,7 @@ typedef enum {
>  ///
>  typedef enum {
>    OutputSchemeSingleRange = 0,
> -  OutputSchemeToPA,
> -  OutputSchemeInvalid
> +  OutputSchemeToPA
>  } PROC_TRACE_OUTPUT_SCHEME;
> 
>  typedef struct  {
> @@ -134,10 +132,6 @@ ProcTraceSupport (
>    // Check if ProcTraceMemorySize option is enabled (0xFF means 
> disable by user)
>    //
>    ProcTraceData = (PROC_TRACE_DATA *) ConfigData;
> -  if ((ProcTraceData->ProcTraceMemSize >=
> EnumProcTraceMemDisable) ||
> -      (ProcTraceData->ProcTraceOutputScheme >=
> OutputSchemeInvalid)) {
> -    return FALSE;
> -  }

I see the ProcTraceOutputScheme values are checked below.
Do we need to keep the check for a valid ProcTraceMemSize value?

> 
>    //
>    // Check if Processor Trace is supported @@ -151,8 +145,8 @@ 
> ProcTraceSupport (
>    AsmCpuidEx (CPUID_INTEL_PROCESSOR_TRACE, 
> CPUID_INTEL_PROCESSOR_TRACE_MAIN_LEAF, NULL, NULL, &Ecx.Uint32, NULL);
>    ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported =
> (BOOLEAN) (Ecx.Bits.RTIT == 1);
>    ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported = (BOOLEAN)
> (Ecx.Bits.SingleRangeOutput == 1);
> -  if (ProcTraceData->ProcessorData[ProcessorNumber].TopaSupported
> ||
> -      ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported) {
> +  if ((ProcTraceData-
> >ProcessorData[ProcessorNumber].TopaSupported && (ProcTraceData- 
> >ProcTraceOutputScheme == OutputSchemeToPA)) ||
> +      (ProcTraceData-
> >ProcessorData[ProcessorNumber].SingleRangeSupported &&
> (ProcTraceData->ProcTraceOutputScheme ==
> OutputSchemeSingleRange))) {
>      return TRUE;
>    }
> 
> --
> 2.7.0.windows.1

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-08-24  3:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-23  2:31 [Patch 0/2] Remove redundant definition for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
2017-08-23  2:31 ` [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition Eric Dong
2017-08-23 23:02   ` Kinney, Michael D
2017-08-24  3:08     ` Dong, Eric
2017-08-23  2:31 ` [Patch 2/2] UefiCpuPkg: Update default for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme Eric Dong
2017-08-23 22:00   ` Kinney, Michael D

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox