On Thu, 27 Feb 2020 at 09:24, Laszlo Ersek <ler...@redhat.com> wrote:
>
> On 02/26/20 20:05, Ard Biesheuvel wrote:
> > Incorporate the PEI components and the associated library class
> > resolutions and PCD declarations to enable TPM2 support in the
> > PEI phase.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> > ---
> > ArmVirtPkg/ArmVirtQemu.dsc | 20 ++++++++++++++++++++
> > ArmVirtPkg/ArmVirtQemu.fdf | 2 ++
> > 2 files changed, 22 insertions(+)
> >
> > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> > index d1757cdba671..8950116dacab 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.dsc
> > +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> > @@ -75,11 +75,17 @@ [LibraryClasses.common]
> >
> > PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
> >
> > PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
> >
> > +!if $(TPM2_ENABLE) == TRUE
> > + Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> > +!endif
> > +
> > [LibraryClasses.common.PEIM]
> >
> > ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> >
> > !if $(TPM2_ENABLE) == TRUE
> > + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> >
> > ResetSystemLib|MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
> > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> > !endif
> >
> > [LibraryClasses.common.DXE_DRIVER]
> > @@ -248,6 +254,10 @@ [PcdsDynamicDefault.common]
> > # TPM2 support
> > #
> > gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
> > +!if $(TPM2_ENABLE) == TRUE
> > + gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00,
> > 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
> > 0x00}
> > + gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0
> > +!endif
> >
> > [PcdsDynamicHii]
> >
> > gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
> > @@ -278,6 +288,16 @@ [Components.common]
> > <LibraryClasses>
> >
> > ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
> > }
> > + OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> > + SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> > + <LibraryClasses>
> > +
> > HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> > + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> > +
> > NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> > +
> > NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> > +
> > NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> > + }
> > !endif
> >
> > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
> > diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> > index f55918d26b06..b5e2253295fe 100644
> > --- a/ArmVirtPkg/ArmVirtQemu.fdf
> > +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> > @@ -115,6 +115,8 @@ [FV.FVMAIN_COMPACT]
> >
> > !if $(TPM2_ENABLE) == TRUE
> > INF MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
> > + INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> > + INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> > !endif
> >
> > FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
> >
>
> I'm OK with this patch; I have two requests:
>
> (1) Please add the following to the commit message:
>
> -----
> This patch ports (parts of) the following OvmfPkg commits to ArmVirtQemu:
> - 6cf1880fb5b6 ("OvmfPkg: add customized Tcg2ConfigPei clone",
> 2018-03-09)
> - 4672a4892867 ("OvmfPkg: include Tcg2Pei module", 2018-03-09)
> - b9130c866dc0 ("OvmfPkg: link Sha384 and Sha512 support into Tcg2Pei
> and Tcg2Dxe", 2018-08-16)
> - 5d3ef15da7c3 ("OvmfPkg: link SM3 support into Tcg2Pei and Tcg2Dxe",
> 2019-07-19)
> -----
>
OK
> (2) I don't understand why PcdTpm2HashMask is necessary, especially that
> it diverges from the "SecurityPkg.dec" default. Please either drop the
> PCD, or (more likely) add a sentence to the commit message explaining
> why we're setting it to zero (unlike in OVMF).
>
I'm not sure about OVMF, but according to SecurityPkg.dec's
description of this PCD (line 455):
# If all the bits are clear, that means hash algorithm is
determined by current Active PCR Banks.<BR>
which seems like a reasonable default, rather than going through the
logic of SyncPcrAllocationsAndPcrMask() in Tcg2Pei, and essentially
end up doing the same.
I'll add a comment to the PCD declaration to clarify this.
> With those addressed:
>
> Reviewed-by: Laszlo Ersek <ler...@redhat.com>
>
> Thanks!
> Laszlo
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54987): https://edk2.groups.io/g/devel/message/54987
Mute This Topic: https://groups.io/mt/71571523/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
