[edk2-devel] [PATCH 1/2] Features/Intel/AdvancedFeaturePkg: Remove temporary build workaround
Ni, Ray
ray.ni at intel.com
Fri Apr 24 05:02:39 UTC 2020
Thanks for the explanation. Make sense to me.
> -----Original Message-----
> From: Luo, Heng <heng.luo at intel.com>
> Sent: Friday, April 24, 2020 9:06 AM
> To: Ni, Ray <ray.ni at intel.com>; devel at edk2.groups.io
> Cc: Michael Kubacki <michael.a.kubacki at intel.com>; Gao, Liming <liming.gao at intel.com>; Dong, Eric
> <eric.dong at intel.com>
> Subject: RE: [PATCH 1/2] Features/Intel/AdvancedFeaturePkg: Remove temporary build workaround
>
> Hi Ray,
> Option 1, In current implement, the usage of AdvancedFeaturePkg in board DSC is:
>
> !include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc # invoke section "[Packages]", define default Feature PCD
> #option: [PcdsFeatureFlag], board can override default Feature PCD here, enable specific feature.
> !include Include/AdvancedFeatures.dsc
>
> Option 2, if there is no AdvancedFeaturesPcd.dsc, we can move section "[Packages]" to AdvancedFeatures.dsc, there is 2
> concerns here:
> 1. there is no default Feature PCD now, we need to add [PcdsFeatureFlag] below in every OpenBoardPkg.dsc. if we add a
> new feature, we have to change all OpenBoardPkg.dsc files, but in option 1, we just need to change
> AdvancedFeaturesPcd.dsc
> [PcdsFeatureFlag]
> gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable |FALSE
> gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable |FALSE
> .....
>
> 2. we define FeaturePCDs before section "[Packages]", it does work, but it looks strange.
>
> So I would like option 1, but I also agree option2 if you think it is better.
>
> Thanks,
> Heng
> > -----Original Message-----
> > From: Ni, Ray <ray.ni at intel.com>
> > Sent: Thursday, April 23, 2020 7:43 PM
> > To: Luo, Heng <heng.luo at intel.com>; devel at edk2.groups.io
> > Cc: Michael Kubacki <michael.a.kubacki at intel.com>; Gao, Liming
> > <liming.gao at intel.com>; Dong, Eric <eric.dong at intel.com>
> > Subject: RE: [PATCH 1/2] Features/Intel/AdvancedFeaturePkg: Remove
> > temporary build workaround
> >
> > Heng,
> > Any reason to create a separate Pcd.dsc file?
> >
> > > -----Original Message-----
> > > From: Luo, Heng <heng.luo at intel.com>
> > > Sent: Thursday, April 23, 2020 4:43 PM
> > > To: devel at edk2.groups.io
> > > Cc: Michael Kubacki <michael.a.kubacki at intel.com>; Gao, Liming
> > > <liming.gao at intel.com>; Dong, Eric <eric.dong at intel.com>; Ni, Ray
> > > <ray.ni at intel.com>
> > > Subject: [PATCH 1/2] Features/Intel/AdvancedFeaturePkg: Remove
> > > temporary build workaround
> > >
> > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2688
> > >
> > > Remove Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround,
> > > Add
> > Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> > > to support Feature PCDs.
> > >
> > > Cc: Michael Kubacki <michael.a.kubacki at intel.com>
> > > Cc: Liming Gao <liming.gao at intel.com>
> > > Cc: Eric Dong <eric.dong at intel.com>
> > > Cc: Ray Ni <ray.ni at intel.com>
> > > Signed-off-by: Heng Luo <heng.luo at intel.com>
> > > ---
> > > Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> > | 2 +-
> > > Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> > | 41
> > > +++++++++++++++++++++++++++++++++++++++++
> > >
> > Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Temporar
> > yBuildWorkaround.c | 31 ---------------------------
> > > ----
> > >
> > >
> > Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Temporar
> > yBu
> > > ildWorkaround.dsc | 76 --------------------------
> > > --------------------------------------------------
> > >
> > >
> > Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Temporar
> > yBu
> > > ildWorkaround.inf | 60 ---------------------------
> > > ---------------------------------
> > > 5 files changed, 42 insertions(+), 168 deletions(-)
> > >
> > > diff --git a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> > > b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> > > index ea879680ba..e509ef3e1b 100644
> > > --- a/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> > > +++ b/Features/Intel/AdvancedFeaturePkg/AdvancedFeaturePkg.dsc
> > > @@ -30,7 +30,7 @@
> > > PEI_ARCH = IA32
> > >
> > > DXE_ARCH = X64
> > >
> > >
> > >
> > > -!include
> > >
> > AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaro
> > und.d
> > > sc
> > >
> > > +!include AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> > >
> > >
> > >
> > >
> > >
> > ################################################################
> > ######
> > > ##########
> > >
> > > #
> > >
> > > diff --git
> > > a/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> > > b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.dsc
> > > new file mode 100644
> > > index 0000000000..2eacec18a0
> > > --- /dev/null
> > > +++
> > b/Features/Intel/AdvancedFeaturePkg/Include/AdvancedFeaturesPcd.ds
> > > +++ c
> > > @@ -0,0 +1,41 @@
> > > +## @file
> > >
> > > +# DSC file for defining Pcd of advanced features.
> > >
> > > +#
> > >
> > > +# This file is intended to be included into another package so
> > > +advanced features
> > >
> > > +# can be conditionally built by enabling the respective feature via its
> > FeaturePCD.
> > >
> > > +#
> > >
> > > +# Copyright (c) 2020, Intel Corporation. All rights reserved.<BR>
> > >
> > > +#
> > >
> > > +# SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > +#
> > >
> > > +##
> > >
> > > +
> > >
> > > +#
> > >
> > > +# The section references the package DEC files,
> > >
> > > +# it allow a FeaturePCD to be used in a conditional statement
> > >
> > > +#
> > >
> > > +[Packages]
> > >
> > > + MdePkg/MdePkg.dec
> > >
> > > + AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
> > >
> > > + Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec
> > >
> > > + NetworkFeaturePkg/NetworkFeaturePkg.dec
> > >
> > > + IpmiFeaturePkg/IpmiFeaturePkg.dec
> > >
> > > + S3FeaturePkg/S3FeaturePkg.dec
> > >
> > > + SmbiosFeaturePkg/SmbiosFeaturePkg.dec
> > >
> > > + UserAuthFeaturePkg/UserAuthFeaturePkg.dec
> > >
> > > + LogoFeaturePkg/LogoFeaturePkg.dec
> > >
> > > +
> > >
> > > +#
> > >
> > > +# The section below sets all PCDs to FALSE in this DSC file so the feature is
> > not enabled by default.
> > >
> > > +# Board can set PCDs to TRUE in its DSC file to enable a subset of
> > > +advanced features
> > >
> > > +#
> > >
> > > +[PcdsFeatureFlag]
> > >
> > > + gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable
> > |FALSE
> > >
> > > + gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
> > |FALSE
> > >
> > > + gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable
> > |FALSE
> > >
> > > + gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable |FALSE
> > >
> > > + gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> > |FALSE
> > >
> > > + gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> > |FALSE
> > >
> > > +
> > gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable
> > |FALSE
> > >
> > > + gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable
> > |FALSE
> > >
> > > + gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable |FALSE
> > >
> > > diff --git
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.c
> > >
> > b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.c
> > > deleted file mode 100644
> > > index 2cd91b06f0..0000000000
> > > ---
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.c
> > > +++ /dev/null
> > > @@ -1,31 +0,0 @@
> > > -/** @file
> > >
> > > - Source code file for a temporary build workaround.
> > >
> > > -
> > >
> > > - The purpose of this workaround is described in the module INF file.
> > >
> > > -
> > >
> > > -Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> > >
> > > -SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > -
> > >
> > > -**/
> > >
> > > -
> > >
> > > -#include <Base.h>
> > >
> > > -#include <Library/BaseLib.h>
> > >
> > > -
> > >
> > > -/**
> > >
> > > - An empty entry point function.
> > >
> > > -
> > >
> > > - @param FileHandle Handle of the file being invoked.
> > >
> > > - @param PeiServices Describes the list of possible PEI Services.
> > >
> > > -
> > >
> > > - @retval EFI_SUCCESS This function always returns EFI_SUCCESS.
> > >
> > > -
> > >
> > > -**/
> > >
> > > -EFI_STATUS
> > >
> > > -EFIAPI
> > >
> > > -TemporaryBuildWorkaroundEntry (
> > >
> > > - IN EFI_PEI_FILE_HANDLE FileHandle,
> > >
> > > - IN CONST EFI_PEI_SERVICES **PeiServices
> > >
> > > - )
> > >
> > > -{
> > >
> > > - return EFI_SUCCESS;
> > >
> > > -}
> > >
> > > diff --git
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.dsc
> > >
> > b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.dsc
> > > deleted file mode 100644
> > > index c62f9ecc6e..0000000000
> > > ---
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.dsc
> > > +++ /dev/null
> > > @@ -1,76 +0,0 @@
> > > -## @file
> > >
> > > -# Build description file for a temporary build workaround.
> > >
> > > -#
> > >
> > > -# The feature enable PCD for advanced features must be referenced in
> > > an INF
> > >
> > > -# to be referenced in DSC/FDF files. This DSC only exists in the
> > > build to
> > >
> > > -# allow the PCDs to be referenced. This workaround does not affect
> > > the final
> > >
> > > -# flash image or boot in any way.
> > >
> > > -#
> > >
> > > -# The request to update BaseTools to allow a PCD to be referenced in
> > > DSC/FDF
> > >
> > > -# files without requiring the PCD to be referenced in an INF file is
> > > tracked
> > >
> > > -# here: https://bugzilla.tianocore.org/show_bug.cgi?id=2270
> > >
> > > -#
> > >
> > > -# When the BaseTools update is complete, this file can entirely be
> > > removed
> > >
> > > -# from this package.
> > >
> > > -#
> > >
> > > -# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > -#
> > >
> > > -# SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > -#
> > >
> > > -##
> > >
> > > -
> > >
> > > -#
> > >
> > > -# BEGIN:Temporary Build Workaround (resolution:
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2270)
> > >
> > > -#
> > >
> > > -
> > >
> > > -# THIS FILE IS TEMPORARY. PLEASE TRY TO LOOK PAST THE "HACKS"
> > ASSOCIATED WITH IT.
> > >
> > > -#
> > >
> > > -# With the BaseTools change requested, the changes needed to move to
> > the end state are simply:
> > >
> > > -# 1. Remove the !include for this file in AdvancedFeatures.dsc
> > >
> > > -# 2. Remove this directory
> > >
> > > -
> > >
> > > -!if $(PLATFORM_NAME) != AdvancedFeaturePkg
> > >
> > > -#
> > >
> > > -# AdvancedFeaturePkg initializes all FeaturePCDs to TRUE so they can
> > conveniently be built in one package.
> > >
> > > -# Board packages will normally only enable (and therefore reference)
> > > a small subset of advanced features
> > >
> > > -# relative to the board. If an INF does not reference a package DEC
> > > file (which will be the case if the
> > >
> > > -# feature is not enabled) then the DSC must set ("define") the PCD
> > > for the conditional statements based
> > >
> > > -# on the PCD to work.
> > >
> > > -#
> > >
> > > -# AdvancedFeaturePkg has no problem as it naturally has a need to set al
> > PCDs to TRUE for build.
> > >
> > > -# The section below sets all PCDs to FALSE in the DSC file so if the
> > > feature is not enabled by a board,
> > >
> > > -# the build will still be successful.
> > >
> > > -#
> > >
> > > -[PcdsFeatureFlag]
> > >
> > > - gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable
> > |FALSE
> > >
> > > - gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
> > |FALSE
> > >
> > > - gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable
> > |FALSE
> > >
> > > - gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable |FALSE
> > >
> > > - gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> > |FALSE
> > >
> > > - gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> > |FALSE
> > >
> > > -
> > gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable
> > |FALSE
> > >
> > > - gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable
> > |FALSE
> > >
> > > - gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable |FALSE
> > >
> > > -!endif
> > >
> > > -
> > >
> > > -#
> > >
> > > -# The LibraryClasses required to build TemporaryBuildWorkaround.inf
> > >
> > > -# (mostly libraries requiring other libraries)
> > >
> > > -#
> > >
> > > -[LibraryClasses]
> > >
> > > - BaseLib|MdePkg/Library/BaseLib/BaseLib.inf
> > >
> > > -
> > >
> > BaseMemoryLib|MdePkg/Library/BaseMemoryLibRepStr/BaseMemoryLibRe
> > pStr.i
> > > nf
> > >
> > > - DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf
> > >
> > > - PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> > >
> > > - PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf
> > >
> > > - PeiServicesLib|MdePkg/Library/PeiServicesLib/PeiServicesLib.inf
> > >
> > > -
> > > PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibId
> > > t/PeiServicesTablePointerLibIdt.inf
> > >
> > > -
> > >
> > > -#
> > >
> > > -# The driver that references all feature PCDs to satsify current
> > > build limitations
> > >
> > > -#
> > >
> > > -[Components]
> > >
> > > -
> > >
> > AdvancedFeaturePkg/TemporaryBuildWorkaround/TemporaryBuildWorkaro
> > und.i
> > > nf
> > >
> > > -#
> > >
> > > -# END:Temporary Build Workaround (resolution:
> > > https://bugzilla.tianocore.org/show_bug.cgi?id=2270)
> > >
> > > -#
> > >
> > > diff --git
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.inf
> > >
> > b/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.inf
> > > deleted file mode 100644
> > > index 00818fbe0a..0000000000
> > > ---
> > >
> > a/Features/Intel/AdvancedFeaturePkg/TemporaryBuildWorkaround/Tempor
> > ary
> > > BuildWorkaround.inf
> > > +++ /dev/null
> > > @@ -1,60 +0,0 @@
> > > -### @file
> > >
> > > -# Component information file for a temporary build workaround.
> > >
> > > -#
> > >
> > > -# The feature enable PCD for this package must be referenced in an
> > > INF to be
> > >
> > > -# referenced in DSC/FDF files. This driver is only included in the
> > > build to
> > >
> > > -# allow the PCD to be referenced. This driver is not included in the
> > > flash
> > >
> > > -# image and does not affect the boot in any way.
> > >
> > > -#
> > >
> > > -# The request to update BaseTools to allow a PCD to be referenced in
> > > DSC/FDF
> > >
> > > -# files without requiring the PCD to be referenced in an INF file is
> > > tracked
> > >
> > > -# here: https://bugzilla.tianocore.org/show_bug.cgi?id=2270
> > >
> > > -#
> > >
> > > -# When the BaseTools update is complete, this file can entirely be
> > > removed
> > >
> > > -# from this package.
> > >
> > > -#
> > >
> > > -# Copyright (c) 2019 - 2020, Intel Corporation. All rights
> > > reserved.<BR>
> > >
> > > -#
> > >
> > > -# SPDX-License-Identifier: BSD-2-Clause-Patent
> > >
> > > -#
> > >
> > > -###
> > >
> > > -
> > >
> > > -[Defines]
> > >
> > > - INF_VERSION = 0x00010017
> > >
> > > - BASE_NAME = TemporaryBuildWorkaround
> > >
> > > - FILE_GUID = 8846A81E-F552-4917-81F5-80B62E4EFBAC
> > >
> > > - VERSION_STRING = 1.0
> > >
> > > - MODULE_TYPE = PEIM
> > >
> > > - ENTRY_POINT = TemporaryBuildWorkaroundEntry
> > >
> > > -
> > >
> > > -[LibraryClasses]
> > >
> > > - BaseLib
> > >
> > > - PeimEntryPoint
> > >
> > > -
> > >
> > > -[Packages]
> > >
> > > - MdePkg/MdePkg.dec
> > >
> > > - Debugging/AcpiDebugFeaturePkg/AcpiDebugFeaturePkg.dec
> > >
> > > - Debugging/Usb3DebugFeaturePkg/Usb3DebugFeaturePkg.dec
> > >
> > > - Network/NetworkFeaturePkg/NetworkFeaturePkg.dec
> > >
> > > - OutOfBandManagement/IpmiFeaturePkg/IpmiFeaturePkg.dec
> > >
> > > - PowerManagement/S3FeaturePkg/S3FeaturePkg.dec
> > >
> > > - SystemInformation/SmbiosFeaturePkg/SmbiosFeaturePkg.dec
> > >
> > > - UserInterface/UserAuthFeaturePkg/UserAuthFeaturePkg.dec
> > >
> > > - UserInterface/LogoFeaturePkg/LogoFeaturePkg.dec
> > >
> > > -
> > >
> > > -[FeaturePcd]
> > >
> > > - gAcpiDebugFeaturePkgTokenSpaceGuid.PcdAcpiDebugFeatureEnable
> > >
> > > - gIpmiFeaturePkgTokenSpaceGuid.PcdIpmiFeatureEnable
> > >
> > > - gNetworkFeaturePkgTokenSpaceGuid.PcdNetworkFeatureEnable
> > >
> > > - gS3FeaturePkgTokenSpaceGuid.PcdS3FeatureEnable
> > >
> > > - gSmbiosFeaturePkgTokenSpaceGuid.PcdSmbiosFeatureEnable
> > >
> > > - gUsb3DebugFeaturePkgTokenSpaceGuid.PcdUsb3DebugFeatureEnable
> > >
> > > -
> > >
> > gUserAuthFeaturePkgTokenSpaceGuid.PcdUserAuthenticationFeatureEnable
> > >
> > > - gLogoFeaturePkgTokenSpaceGuid.PcdLogoFeatureEnable
> > >
> > > - gLogoFeaturePkgTokenSpaceGuid.PcdJpgEnable
> > >
> > > -
> > >
> > > -[Sources]
> > >
> > > - TemporaryBuildWorkaround.c
> > >
> > > -
> > >
> > > -[Depex]
> > >
> > > - TRUE
> > >
> > > --
> > > 2.24.0.windows.2
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#58014): https://edk2.groups.io/g/devel/message/58014
Mute This Topic: https://groups.io/mt/73214615/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list