Re: [Patch] UefiCpuPkg: Add missing components to UefiCpuPkg.dsc

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hi Eric,

I will enter a new BZ to remove binary.

Can you please review the current patch?

Thanks,

Mike

> -----Original Message-----
> From: Dong, Eric <eric.dong@intel.com>
> Sent: Tuesday, October 22, 2019 6:29 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Ni, Ray
> <ray.ni@intel.com>; Wei, David Y
> <david.y.wei@intel.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: RE: [Patch] UefiCpuPkg: Add missing components
> to UefiCpuPkg.dsc
> 
> Hi Mike and Laszlo,
> 
> I agree with Mike's proposal. We can submit new bugz to
> remove the binary.
> 
> Thanks,
> Eric
> 
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, October 23, 2019 7:39 AM
> To: Laszlo Ersek <lersek@redhat.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Sean Brogan <sean.brogan@microsoft.com>; Dong, Eric
> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>; Wei,
> David Y <david.y.wei@intel.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> Subject: RE: [Patch] UefiCpuPkg: Add missing components
> to UefiCpuPkg.dsc
> 
> Laszlo,
> 
> Can we move forward with this patch and enter a new BZ
> to remove the binary module?
> 
> Mike
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Tuesday, October 22, 2019 4:27 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Sean Brogan <sean.brogan@microsoft.com>; Dong,
> Eric
> > <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>;
> Wei, David Y
> > <david.y.wei@intel.com>; Agyeman, Prince
> <prince.agyeman@intel.com>
> > Subject: Re: [Patch] UefiCpuPkg: Add missing
> components to
> > UefiCpuPkg.dsc
> >
> > (+David, +Prince)
> >
> > On 10/23/19 01:13, Kinney, Michael D wrote:
> > > Laszlo,
> > >
> > > I see 2 usages of this module in edk2-platforms:
> > >
> > > edk2-
> >
> platforms\Silicon\Intel\SimicsX58SktPkg\SktPkgPei.dsc(13
> > ):
> > > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
> > >
> > > edk2-
> >
> platforms\Silicon\Intel\SimicsX58SktPkg\SktSecInclude.fd
> > f(16):INF
> > > RuleOverride=RESET_VECTOR USE = IA32
> > > UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
> >
> > Can we update those platforms to build the reset
> vector from source?
> >
> > Embedding a binary blob seems wrong in its own
> right...
> >
> > Anyway, if Ray and/or Eric are OK with the present
> patch, I won't try
> > to block it.
> >
> > Thanks,
> > Laszlo
> >
> > >> -----Original Message-----
> > >> From: Laszlo Ersek <lersek@redhat.com>
> > >> Sent: Tuesday, October 22, 2019 3:57 PM
> > >> To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > >> devel@edk2.groups.io
> > >> Cc: Sean Brogan <sean.brogan@microsoft.com>; Dong,
> > Eric
> > >> <eric.dong@intel.com>; Ni, Ray <ray.ni@intel.com>
> > >> Subject: Re: [Patch] UefiCpuPkg: Add missing
> > components to
> > >> UefiCpuPkg.dsc
> > >>
> > >> On 10/22/19 21:06, Michael D Kinney wrote:
> > >>> From: Sean Brogan <sean.brogan@microsoft.com>
> > >>>
> > >>>
> https://bugzilla.tianocore.org/show_bug.cgi?id=2255
> > >>>
> > >>> Update UefiCpuPkg.dsc to guarantee all libraries
> and
> > >> modules are
> > >>> always built.  Add the following components.
> > >>>
> > >>> * UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
> > >>> *
> > >>
> >
> UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeature
> > >> sLib.inf
> > >>>
> > >>> ResetVector.inf is a binary INF, so no source
> builds
> > >> are triggered
> > >>> from adding this line.  However, a build with this
> > >> component does
> > >>> verify the contents of the INF file.
> > >>>
> > >>> Cc: Eric Dong <eric.dong@intel.com>
> > >>> Cc: Ray Ni <ray.ni@intel.com>
> > >>> Cc: Laszlo Ersek <lersek@redhat.com>
> > >>> Signed-off-by: Michael D Kinney
> > >> <michael.d.kinney@intel.com>
> > >>> ---
> > >>>  UefiCpuPkg/UefiCpuPkg.dsc | 2 ++
> > >>>  1 file changed, 2 insertions(+)
> > >>>
> > >>> diff --git a/UefiCpuPkg/UefiCpuPkg.dsc
> > >> b/UefiCpuPkg/UefiCpuPkg.dsc
> > >>> index e7dfe30eda..d28cb5cccb 100644
> > >>> --- a/UefiCpuPkg/UefiCpuPkg.dsc
> > >>> +++ b/UefiCpuPkg/UefiCpuPkg.dsc
> > >>> @@ -121,6 +121,7 @@ [Components.IA32,
> > Components.X64]
> > >>>
> > >>
> UefiCpuPkg/Library/BaseUefiCpuLib/BaseUefiCpuLib.inf
> > >>>
> UefiCpuPkg/Library/BaseXApicLib/BaseXApicLib.inf
> > >>>
> > >>
> >
> UefiCpuPkg/Library/BaseXApicX2ApicLib/BaseXApicX2ApicLib
> > >> .inf
> > >>> +
> > >>
> >
> UefiCpuPkg/Library/CpuCommonFeaturesLib/CpuCommonFeature
> > >> sLib.inf
> > >>>
> > >>
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/DxeCpuExceptio
> > >> nHandlerLib.inf
> > >>>
> > >>
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SecPeiCpuExcep
> > >> tionHandlerLib.inf
> > >>>
> > >>>
> > >>
> >
> UefiCpuPkg/Library/CpuExceptionHandlerLib/SmmCpuExceptio
> > >> nHandlerLib.in
> > >>> f @@ -146,6 +147,7 @@ [Components.IA32,
> > >> Components.X64]
> > >>>
> > >>
> >
> SmmCpuFeaturesLib|UefiCpuPkg/Library/SmmCpuFeaturesLib/S
> > >> mmCpuFeaturesLibStm.inf
> > >>>    }
> > >>>
> > >>
> >
> UefiCpuPkg/Universal/Acpi/S3Resume2Pei/S3Resume2Pei.inf
> > >>> +  UefiCpuPkg/ResetVector/Vtf0/Bin/ResetVector.inf
> > >>>
> > >>>  [BuildOptions]
> > >>>    *_*_*_CC_FLAGS = -D
> > >> DISABLE_NEW_DEPRECATED_INTERFACES
> > >>>
> > >>
> > >> Can we retire
> > >>
> > >>   UefiCpuPkg/ResetVector/Vtf0/Bin/
> > >>
> > >> instead (in a separate patch)?
> > >>
> > >> Because, according to
> > >>
> > >>   git log --stat master --
> > >> UefiCpuPkg/ResetVector/Vtf0/Bin/
> > >>
> > >> the most recent commit in which we updated the
> *.raw
> > files has been
> > >> f7bb98019a71 ("UefiCpuPkg VTF0: Fix support for
> > finding SEC image of
> > >> type TE.", 2011-09-28).
> > >>
> > >> I doubt anyone consumes this (binary) module, since
> > Jordan's commits
> > >> - 5a1f324d946c ("UefiCpuPkg: Support building VTF0
> > ResetVector during
> > >> the EDK II build", 2014-08-18)
> > >> - eee1d2ca9078 ("UefiCpuPkg VTF0 X64: Build page
> > tables in NASM
> > >> code", 2014-08-18).
> > >>
> > >> We've by now dropped all top-level *BinPkg package
> > directories too.
> > >>
> > >> If we remove the Bin subdir, then we should
> probably
> > >> remove:
> > >> - Build.py
> > >> - Tools/
> > >> too.
> > >>
> > >> For the other addition (CpuCommonFeaturesLib): if
> > that gets its own
> > >> patch (after the removal of
> > UefiCpuPkg/ResetVector/Vtf0/Bin/), then
> > >> please add
> > >>
> > >> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> > >>
> > >> to it.
> > >>
> > >> Thanks
> > >> Laszlo
> > >