1. Patchew
  2. EDK2
  3. [edk2] [Patch 0/2] Remove redundant definition for PcdCpuProcTraceMemSize/PcdCpuProcTraceOutputScheme
  4. View patch

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

Eric Dong posted 2 patches 7 years, 4 months ago
[edk2] [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
Posted by Eric Dong 7 years, 4 months ago 
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

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Re: [edk2] [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
Posted by Kinney, Michael D 7 years, 4 months ago 
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
Re: [edk2] [Patch 1/2] UefiCpuPkg/CpuCommonFeaturesLib: Remove redundant definition.
Posted by Dong, Eric 7 years, 4 months ago 
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
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Patchew 2.1-devel-8c64711

© 2016 - 2024 Red Hat, Inc.