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

["Dong, Eric" <eric.dong@intel.com>]

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
> >