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-boun...@lists.01.org] On Behalf Of Kinney, Michael D Sent: Thursday, August 24, 2017 7:03 AM To: Dong, Eric <eric.d...@intel.com>; edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kin...@intel.com> Cc: Ni, Ruiyu <ruiyu...@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.kin...@intel.com>; Ni, Ruiyu > <ruiyu...@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.kin...@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Eric Dong <eric.d...@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 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
