> -----Original Message-----
> From: Dong, Eric <eric.d...@intel.com>
> Sent: Wednesday, February 13, 2019 10:04 AM
> To: edk2-devel@lists.01.org
> Cc: Ni, Ray <ray...@intel.com>; Laszlo Ersek <ler...@redhat.com>
> Subject: [Patch 2/3] UefiCpuPkg/RegisterCpuFeaturesLib: Optimize PCD
> PcdCpuFeaturesUserConfiguration.
>
> In current implementation, PCD PcdCpuFeaturesUserConfiguration used as
> user enabled CPU features list. It is initialzied in platform driver and as an
> input for CpuFeatures driver. PCD PcdCpuFeaturesSetting used as an output
> for the final enabled CPU features list. For now,
> PcdCpuFeaturesUserConfiguration is only used as an input and
> PcdCpuFeaturesSetting only used as an output.
>
> This change merge PcdCpuFeaturesUserConfiguration into
> PcdCpuFeaturesSetting.
> Use PcdCpuFeaturesSetting as input for the user input feature setting Use
> PcdCpuFeaturesSetting as output for the final CPU feature setting
>
> BZ:https://bugzilla.tianocore.org/show_bug.cgi?id=1375
>
> Cc: Ray Ni <ray...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Eric Dong <eric.d...@intel.com>
> ---
> .../RegisterCpuFeaturesLib/CpuFeaturesInitialize.c | 37 ++++++++++++------
> ----
> .../DxeRegisterCpuFeaturesLib.inf | 1 -
> .../PeiRegisterCpuFeaturesLib.inf | 1 -
> .../RegisterCpuFeaturesLib/RegisterCpuFeatures.h | 1 -
> UefiCpuPkg/UefiCpuPkg.dec | 7 ++--
> 5 files changed, 22 insertions(+), 25 deletions(-)
>
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> index bae92b89a6..4ebd0025b4 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/CpuFeaturesInitialize.c
> @@ -54,41 +54,41 @@ SetSettingPcd (
> }
>
> /**
> - Worker function to get PcdCpuFeaturesSupport.
> + Worker function to get PcdCpuFeaturesSetting.
>
> @return The pointer to CPU feature bits mask buffer.
> **/
> UINT8 *
> -GetSupportPcd (
> +GetSettingPcd (
> VOID
> )
> {
> - UINT8 *SupportBitMask;
> + UINT8 *SettingBitMask;
>
> - SupportBitMask = AllocateCopyPool (
> - PcdGetSize (PcdCpuFeaturesSupport),
> - PcdGetPtr (PcdCpuFeaturesSupport)
> + SettingBitMask = AllocateCopyPool (
> + PcdGetSize (PcdCpuFeaturesSetting),
> + PcdGetPtr (PcdCpuFeaturesSetting)
> );
> - ASSERT (SupportBitMask != NULL);
> + ASSERT (SettingBitMask != NULL);
>
> - return SupportBitMask;
> + return SettingBitMask;
> }
>
> /**
> - Worker function to get PcdCpuFeaturesUserConfiguration.
> + Worker function to get PcdCpuFeaturesSupport.
>
> @return The pointer to CPU feature bits mask buffer.
> **/
> UINT8 *
> -GetConfigurationPcd (
> +GetSupportPcd (
> VOID
> )
> {
> UINT8 *SupportBitMask;
>
> SupportBitMask = AllocateCopyPool (
> - PcdGetSize (PcdCpuFeaturesUserConfiguration),
> - PcdGetPtr (PcdCpuFeaturesUserConfiguration)
> + PcdGetSize (PcdCpuFeaturesSupport),
> + PcdGetPtr (PcdCpuFeaturesSupport)
> );
> ASSERT (SupportBitMask != NULL);
>
> @@ -287,7 +287,6 @@ CpuInitDataInitialize (
> // Get support and configuration PCDs
> //
> CpuFeaturesData->SupportPcd = GetSupportPcd ();
> - CpuFeaturesData->ConfigurationPcd = GetConfigurationPcd (); }
>
> /**
> @@ -595,6 +594,9 @@ AnalysisProcessorFeatures (
> CPU_FEATURE_DEPENDENCE_TYPE AfterDep;
> CPU_FEATURE_DEPENDENCE_TYPE NoneNeibBeforeDep;
> CPU_FEATURE_DEPENDENCE_TYPE NoneNeibAfterDep;
> + UINT8 *ConfigurationPcd;
1. Can we use the name "SettingPcd"? To use the same term.
> +
> + ConfigurationPcd = NULL;
>
> CpuFeaturesData = GetCpuFeaturesData ();
> CpuFeaturesData->CapabilityPcd = AllocatePool (CpuFeaturesData-
> >BitMaskSize); @@ -610,10 +612,13 @@ AnalysisProcessorFeatures (
> //
> // Calculate the last setting
> //
> -
> CpuFeaturesData->SettingPcd = AllocateCopyPool (CpuFeaturesData-
> >BitMaskSize, CpuFeaturesData->CapabilityPcd);
> ASSERT (CpuFeaturesData->SettingPcd != NULL);
> - SupportedMaskAnd (CpuFeaturesData->SettingPcd, CpuFeaturesData-
> >ConfigurationPcd);
> + ConfigurationPcd = GetSettingPcd ();
2. SuportedMaskAnd() function doesn't change the parameter 2.
So how about we pass PcdGetPtr (PcdCpuFeaturesSetting) directly to
SupportedMaskAnd()?
> + SupportedMaskAnd (CpuFeaturesData->SettingPcd, ConfigurationPcd); if
> + (ConfigurationPcd != NULL) {
> + FreePool (ConfigurationPcd);
> + }
>
> //
> // Save PCDs and display CPU PCDs
> @@ -643,8 +648,6 @@ AnalysisProcessorFeatures (
> }
> DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSupport:\n"));
> DumpCpuFeatureMask (CpuFeaturesData->SupportPcd);
> - DEBUG ((DEBUG_INFO, "PcdCpuFeaturesUserConfiguration:\n"));
> - DumpCpuFeatureMask (CpuFeaturesData->ConfigurationPcd);
3. The debug message cannot be just deleted.
We still need to dump the original PcdCpuFeaturesSetting.
> DEBUG ((DEBUG_INFO, "PcdCpuFeaturesCapability:\n"));
> DumpCpuFeatureMask (CpuFeaturesData->CapabilityPcd);
> DEBUG ((DEBUG_INFO, "PcdCpuFeaturesSetting:\n")); diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.i
> nf
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.i
> nf
> index 362e0c6cd1..b7dc70808f 100644
> ---
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLib.i
> nf
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/DxeRegisterCpuFeaturesLi
> +++ b.inf
> @@ -56,7 +56,6 @@
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuS3DataAddress ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSupport ##
> CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesUserConfiguration ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesCapability ##
> PRODUCES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting ##
> PRODUCES
4. For PcdCpuFeaturesSetting, "CONSUMES" should be mentioned in the comments.
>
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.in
> f
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.in
> f
> index f3907e25d3..cd69721a2d 100644
> ---
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.in
> f
> +++
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLi
> +++ b.inf
> @@ -57,7 +57,6 @@
> [Pcd]
> gUefiCpuPkgTokenSpaceGuid.PcdCpuS3DataAddress ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSupport ##
> CONSUMES
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesUserConfiguration ##
> CONSUMES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesCapability ##
> PRODUCES
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting ##
> PRODUCES
5. Similar as #4.
>
> diff --git
> a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> index 21dd5773a6..3e0a342fd1 100644
> --- a/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> +++ b/UefiCpuPkg/Library/RegisterCpuFeaturesLib/RegisterCpuFeatures.h
> @@ -83,7 +83,6 @@ typedef struct {
> CPU_FEATURES_INIT_ORDER *InitOrder;
> UINT8 *SupportPcd;
> UINT8 *CapabilityPcd;
> - UINT8 *ConfigurationPcd;
> UINT8 *SettingPcd;
>
> UINT32 NumberOfCpus;
> diff --git a/UefiCpuPkg/UefiCpuPkg.dec b/UefiCpuPkg/UefiCpuPkg.dec
> index cb05fa2660..793490872f 100644
> --- a/UefiCpuPkg/UefiCpuPkg.dec
> +++ b/UefiCpuPkg/UefiCpuPkg.dec
> @@ -261,10 +261,6 @@
> # @Prompt SMM CPU Synchronization Method.
>
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmSyncMode|0x00|UINT8|0x60000
> 014
>
> - ## Specifies user's desired settings for enabling/disabling processor
> features.
> - # @Prompt User settings for enabling/disabling processor features.
> - gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesUserConfiguration|{0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000017
> -
> ## Specifies the On-demand clock modulation duty cycle when ACPI
> feature is enabled.
> # @Prompt The encoded values for target duty cycle modulation.
> # @ValidRange 0x80000001 | 0 - 15
> @@ -292,7 +288,8 @@
> # @ValidList 0x80000001 | 0
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesCapability|{0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000018
>
> - ## Specifies actual settings for processor features, each bit corresponding
> to a specific feature.
> + ## As input, specifies user's desired settings for enabling/disabling
> processor features.
> + ## As output, specifies actual settings for processor features, each bit
> corresponding to a specific feature.
> # @Prompt Actual processor feature settings.
> # @ValidList 0x80000001 | 0
> gUefiCpuPkgTokenSpaceGuid.PcdCpuFeaturesSetting|{0x00, 0x00, 0x00,
> 0x00, 0x00, 0x00, 0x00, 0x00}|VOID*|0x00000019
> --
> 2.15.0.windows.1
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
