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

["Agyeman, Prince" <prince.agyeman@intel.com>]

Thanks Mike !

Prince

-----Original Message-----
From: Kinney, Michael D <michael.d.kinney@intel.com> 
Sent: Wednesday, October 23, 2019 9:07 AM
To: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek <lersek@redhat.com>; devel@edk2.groups.io; Kinney, Michael D <michael.d.kinney@intel.com>
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

I have enter 2 bugs.  One for SimicsX58SktPkg and one for UefiCpuPkg.

https://bugzilla.tianocore.org/show_bug.cgi?id=2307

https://bugzilla.tianocore.org/show_bug.cgi?id=2306


Mike

> -----Original Message-----
> From: Kinney, Michael D <michael.d.kinney@intel.com>
> Sent: Wednesday, October 23, 2019 8:15 AM
> To: Dong, Eric <eric.dong@intel.com>; Laszlo Ersek 
> <lersek@redhat.com>; devel@edk2.groups.io; Kinney, Michael D 
> <michael.d.kinney@intel.com>
> 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 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
> > > >