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

["Laszlo Ersek" <lersek@redhat.com>]

(+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.fdf(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
>