Re: [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.

["Kinney, Michael D" <michael.d.kinney@intel.com>]

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