Re: [Qemu-devel] [edk2] [PATCH v2 6/8] ovmf: link with Tcg2Pei module

[Laszlo Ersek]

On 03/08/18 19:20, Laszlo Ersek wrote:
> On 03/07/18 16:57, address@hidden wrote:
>> From: Marc-André Lureau <address@hidden>
>>
>> This module will initialize TPM device, measure reported FVs and BIOS
>> version. We keep both SHA-1 and SHA-256 for the TCG 1.2 log format
>> compatibility, but the SHA-256 measurements and TCG 2 log format are
>> now recommended.
>>
>> Cc: Laszlo Ersek <address@hidden>
>> Cc: Stefan Berger <address@hidden>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Marc-André Lureau <address@hidden>
>> ---
>>  OvmfPkg/OvmfPkgX64.dsc | 7 +++++++
>>  OvmfPkg/OvmfPkgX64.fdf | 1 +
>>  2 files changed, 8 insertions(+)
> 
> (1) Please change the subject line to:
> 
> OvmfPkg: include Tcg2Pei module
> 
>>
>> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
>> index 64bd6b6a9f08..3fa1a31f4c37 100644
>> --- a/OvmfPkg/OvmfPkgX64.dsc
>> +++ b/OvmfPkg/OvmfPkgX64.dsc
>> @@ -278,6 +278,8 @@ [LibraryClasses.common.PEIM]
>>    QemuFwCfgLib|OvmfPkg/Library/QemuFwCfgLib/QemuFwCfgPeiLib.inf
>>  
>>  !if $(TPM2_ENABLE)
>> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> 
> (2) Technically this makes sense, but given the fact that we resolve
> BaseCryptLib unconditionally for a bunch of other module types, I think
> we should do that for PEIMs as well.
> 
>> +  
>> HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf

(3) Actually, can you please move this library resolution under
"Tcg2Pei.inf"? Every single PEIM that uses this library instance will
need us to spell out the individual hash plugins for it anyway. So I
think keeping the "hash router" lib instance together with those
NULL-class instances is cleaner.

Thanks
Laszlo

>>    Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
>>  !endif
>>  
>> @@ -615,6 +617,11 @@ [Components]
>>  
>>  !if $(TPM2_ENABLE) == TRUE
>>    OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> +  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
>> +    <LibraryClasses>
>> +      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
>> +      
>> NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
>> +  }
>>  !endif
>>  
>>    #
>> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
>> index dbafada5226b..c0173e7adf5f 100644
>> --- a/OvmfPkg/OvmfPkgX64.fdf
>> +++ b/OvmfPkg/OvmfPkgX64.fdf
>> @@ -167,6 +167,7 @@ [FV.PEIFV]
>>  
>>  !if $(TPM2_ENABLE) == TRUE
>>  INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
>> +INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
>>  !endif
>>  
>>  
>> ################################################################################
>>
> 
> Looks good. (The final version should handle the other DSC / FDF files too.)
> 
> Thanks!
> Laszlo
>