Re: [edk2-devel][edk2-platforms][PATCH V1 2/2] MinPlatformPkg/Build: Reduce duplication

["Chiu, Chasel" <chasel.chiu@intel.com>]

Hi Isaac,

Please see my question below inline.

Thanks,
Chasel


> -----Original Message-----
> From: Oram, Isaac W <isaac.w.oram@intel.com>
> Sent: Wednesday, June 29, 2022 3:12 PM
> To: devel@edk2.groups.io
> Cc: Oram, Isaac W <isaac.w.oram@intel.com>; Chiu, Chasel
> <chasel.chiu@intel.com>; Desimone, Nathaniel L
> <nathaniel.l.desimone@intel.com>; Gao, Liming <gaoliming@byosoft.com.cn>;
> Dong, Eric <eric.dong@intel.com>
> Subject: [edk2-devel][edk2-platforms][PATCH V1 2/2] MinPlatformPkg/Build:
> Reduce duplication
> 
> Removed needless duplication between sections.
> Addes spaces after commas.
> Remove commented out code.
> 
> Cc: Chasel Chiu <chasel.chiu@intel.com>
> Cc: Nate DeSimone <nathaniel.l.desimone@intel.com>
> Cc: Liming Gao <gaoliming@byosoft.com.cn>
> Cc: Eric Dong <eric.dong@intel.com>
> Signed-off-by: Isaac Oram <isaac.w.oram@intel.com>
> ---
>  .../MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc | 55 +++++--------------
>   .../MinPlatformPkg/Include/Dsc/CorePeiLib.dsc | 29 +++-------
>  2 files changed, 20 insertions(+), 64 deletions(-)
> 
> diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> index 9b3095d662..6a4d586ddf 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CoreDxeLib.dsc
> @@ -8,14 +8,14 @@
>  ##
> 
> 
> -  #
> -  # Generic EDKII Lib
> -  #
> +#
> +# Generic EDKII Lib
> +#
> 
> -  #
> -  # DXE phase common
> -  #
> -
> [LibraryClasses.common.DXE_CORE,LibraryClasses.common.DXE_SMM_DRIVER
> ,LibraryClasses.common.SMM_CORE,LibraryClasses.common.DXE_DRIVER,Libra
> ryClasses.common.DXE_RUNTIME_DRIVER,LibraryClasses.common.UEFI_DRIVE
> R,LibraryClasses.common.UEFI_APPLICATION]
> +#
> +# DXE phase common
> +#
> +[LibraryClasses.common.DXE_CORE,
> LibraryClasses.common.DXE_SMM_DRIVER,
> +LibraryClasses.common.SMM_CORE, LibraryClasses.common.DXE_DRIVER,
> +LibraryClasses.common.DXE_RUNTIME_DRIVER,
> +LibraryClasses.common.UEFI_DRIVER,
> +LibraryClasses.common.UEFI_APPLICATION]
>    HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
>    PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> 
> MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryA
> llocationLib.inf
> @@ -32,14 +32,8 @@
> 
> FrameBufferBltLib|MdeModulePkg/Library/FrameBufferBltLib/FrameBufferBltLi
> b.inf
>    TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> 
> -!if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> -
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceL
> ib.inf
> -!endif
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> 
> TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMe
> asurementLib.inf
> -
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf
> -
> 
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRout
> erDxe.inf
> 
> HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRout
> erDxe.inf
> 
> Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeT
> cg2PhysicalPresenceLib.inf
> @@ -50,6 +44,9 @@
> 
> VariableReadLib|MinPlatformPkg/Library/DxeRuntimeVariableReadLib/DxeRunti
> meVariableReadLib.inf
> 
> VariableWriteLib|MinPlatformPkg/Library/DxeRuntimeVariableWriteLib/DxeRun
> timeVariableWriteLib.inf
> 
> +[LibraryClasses.common.DXE_DRIVER,
> +LibraryClasses.common.DXE_RUNTIME_DRIVER,
> +LibraryClasses.common.UEFI_DRIVER,
> +LibraryClasses.common.UEFI_APPLICATION]
> +
> +PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformance
> Lib
> +.inf
> +


Why we always include DxePerformanceLib without checking enabling /disabling PCD?
I think platforms might want to exclude DxePerformanceLib in some types of builds. (e.g., Debug build)



>  [LibraryClasses.common.DXE_CORE, LibraryClasses.common.SMM_CORE]  !if
> $(TARGET) != RELEASE
> 
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> @@ -62,21 +59,15 @@
> 
>  !if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> 
> PerformanceLib|MdeModulePkg/Library/DxeCorePerformanceLib/DxeCorePerf
> ormanceLib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -!endif
>  !endif
> 
> +[LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.UEFI_DRIVER]
> +
> +Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.i
> +nf
> +
>  [LibraryClasses.common.DXE_DRIVER]
> -
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.in
> f
> 
> TpmPlatformHierarchyLib|MinPlatformPkg/Tcg/Library/PeiDxeTpmPlatformHier
> archyLib/PeiDxeTpmPlatformHierarchyLib.inf
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -
> -[LibraryClasses.common.UEFI_DRIVER]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -#  PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> 
>  [LibraryClasses.common.DXE_SMM_DRIVER]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> 
> SmmServicesTableLib|MdePkg/Library/SmmServicesTableLib/SmmServicesTable
> Lib.inf
> 
> MmServicesTableLib|MdePkg/Library/MmServicesTableLib/MmServicesTableLib
> .inf
> 
> ReportStatusCodeLib|MdeModulePkg/Library/SmmReportStatusCodeLib/SmmR
> eportStatusCodeLib.inf
> @@ -87,7 +78,6 @@
> 
>  !if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> 
> PerformanceLib|MdeModulePkg/Library/SmmPerformanceLib/SmmPerformanc
> eLib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
>  !endif
> 
> 
> SmmCpuPlatformHookLib|UefiCpuPkg/Library/SmmCpuPlatformHookLibNull/S
> mmCpuPlatformHookLibNull.inf
> @@ -100,8 +90,6 @@
> 
> VariableWriteLib|MinPlatformPkg/Library/SmmVariableWriteLib/TraditionalMm
> VariableWriteLib.inf
> 
>  [LibraryClasses.common.SMM_CORE]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -  HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf
> 
> MemoryAllocationLib|MdeModulePkg/Library/PiSmmCoreMemoryAllocationLib
> /PiSmmCoreMemoryAllocationLib.inf
> 
> SmmServicesTableLib|MdeModulePkg/Library/PiSmmCoreSmmServicesTableLib
> /PiSmmCoreSmmServicesTableLib.inf
> 
> ReportStatusCodeLib|MdeModulePkg/Library/SmmReportStatusCodeLib/SmmR
> eportStatusCodeLib.inf
> @@ -109,31 +97,14 @@
> 
>  !if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> 
> PerformanceLib|MdeModulePkg/Library/SmmCorePerformanceLib/SmmCorePe
> rformanceLib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -!endif
>  !endif
> 
>  [LibraryClasses.common.DXE_RUNTIME_DRIVER]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> 
> ReportStatusCodeLib|MdeModulePkg/Library/RuntimeDxeReportStatusCodeLib
> /RuntimeDxeReportStatusCodeLib.inf
> -!if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> -
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceL
> ib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -!endif
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf
> 
> VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRu
> ntimeDxe.inf
> 
> -[LibraryClasses.common.UEFI_DRIVER]
> -
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.in
> f
> -
>  [LibraryClasses.common.UEFI_APPLICATION]
> -  PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> -  #PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
>    ShellLib|ShellPkg/Library/UefiShellLib/UefiShellLib.inf
>    FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf
>    SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf
> -
> -!if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> -
> PerformanceLib|MdeModulePkg/Library/DxePerformanceLib/DxePerformanceL
> ib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/DxeAcpiTimerLib.inf
> -!endif
> diff --git a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
> b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
> index 1bf8338f95..8e91dba6ac 100644
> --- a/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
> +++ b/Platform/Intel/MinPlatformPkg/Include/Dsc/CorePeiLib.dsc
> @@ -15,7 +15,12 @@
>    # PEI phase common
>    #
> 
> -
> [LibraryClasses.common.SEC,LibraryClasses.common.PEI_CORE,LibraryClasses.c
> ommon.PEIM]
> +[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE] !if
> +$(TARGET) != RELEASE
> +
> +DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.i
> +nf
> +!endif
> +
> +[LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE,
> +LibraryClasses.common.PEIM]
> 
> S3BootScriptLib|MdePkg/Library/BaseS3BootScriptLibNull/BaseS3BootScriptLib
> Null.inf
>    PcdLib|MdePkg/Library/PeiPcdLib/PeiPcdLib.inf
>    HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf
> @@ -25,39 +30,22 @@
>    LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf
> 
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCp
> uExceptionHandlerLib.inf
> 
> -
>  !if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> 
> PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib
> .inf
>  !endif


Here we checking PCD for including PeiPerformanceLib.inf, should we apply the same check for DxePerformanceLib.inf?



> 
>    TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/BaseAcpiTimerLib.inf
> -
>    VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf
> 
>  [LibraryClasses.common.SEC]
> 
> ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseRepo
> rtStatusCodeLibNull.inf
> -
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCp
> uExceptionHandlerLib.inf
> -!if $(TARGET) != RELEASE
> -
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!endif
>    PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> 
> VariableReadLib|MinPlatformPkg/Library/BaseVariableReadLibNull/BaseVariabl
> eReadLibNull.inf
> 
> -[LibraryClasses.common.PEI_CORE]
> +[LibraryClasses.common.PEI_CORE, LibraryClasses.common.PEIM]
>    TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf
> -
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCp
> uExceptionHandlerLib.inf
> -!if $(TARGET) != RELEASE
> -
> DebugLib|MdePkg/Library/BaseDebugLibSerialPort/BaseDebugLibSerialPort.inf
> -!endif
> -
> -!if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> -
> PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib
> .inf
> -!endif
> 
>  [LibraryClasses.common.PEIM]
> -
> CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCp
> uExceptionHandlerLib.inf
> -  TimerLib|PcAtChipsetPkg/Library/AcpiTimerLib/PeiAcpiTimerLib.inf
> -
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> 
> 
> Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRout
> erPei.inf
> @@ -69,7 +57,4 @@
> 
> TcgEventLogRecordLib|SecurityPkg/Library/TcgEventLogRecordLib/TcgEventLo
> gRecordLib.inf
> 
> TpmMeasurementLib|SecurityPkg/Library/PeiTpmMeasurementLib/PeiTpmMea
> surementLib.inf
> 
> -!if gMinPlatformPkgTokenSpaceGuid.PcdPerformanceEnable == TRUE
> -
> PerformanceLib|MdeModulePkg/Library/PeiPerformanceLib/PeiPerformanceLib
> .inf
> -!endif
> 
> VariableReadLib|MinPlatformPkg/Library/PeiVariableReadLib/PeiVariableReadLi
> b.inf
> --
> 2.36.1.windows.1