On 02/26/20 20:05, Ard Biesheuvel wrote: > Enable the TPM2 support module in the DXE phase, and the associated > libraries and PCDs that it requires. This will be wired into the > measured boot support code in a subsequent patch. > > Note that Tcg2Dxe.inf is added to ArmVirtQemuFvMain.fdf.inc, which > is shared with other platforms in ArmVirtPkg, but as those will not > set the TPM2_ENABLE define, this change does not affect them. > > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > --- > ArmVirtPkg/ArmVirtQemu.dsc | 23 ++++++++++++++++++++ > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 7 ++++++ > 2 files changed, 30 insertions(+) > > diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc > index 8950116dacab..9fe5ab37611c 100644 > --- a/ArmVirtPkg/ArmVirtQemu.dsc > +++ b/ArmVirtPkg/ArmVirtQemu.dsc > @@ -77,6 +77,8 @@ [LibraryClasses.common] > > !if $(TPM2_ENABLE) == TRUE > Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf > + > Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
This should not be needed. Or more precisely, not like this. In OvmfPkg, Marc-André added TPM2 PPI support in the following series (which was separately building upon the initial TPM2 enablement): [edk2] [PATCH v3 0/4] RFC: ovmf: Add support for TPM Physical Presence interface http://mid.mail-archive.com/20180518122304.31020-1-marcandre.lureau@redhat.com 1 fe280ecbee11 OvmfPkg: add Tcg2PhysicalPresenceLibNull when !TPM2_ENABLE 2 909ac65af113 OvmfPkg/IndustryStandard: add QemuTpm.h header 3 b9777bb42e4f OvmfPkg: add Tcg2PhysicalPresenceLibQemu 4 8d65d3b25e35 OvmfPkg/PlatformBootManagerLib: process TPM PPI request The above lib class resolution mirrors commit#3 from those. But it only really makes sense if, in the end, we intend to add a Tcg2PhysicalPresenceLibProcessRequest() call to the PlatformBootManagerLib instance (see commit#4). The present ArmVirtQemu patch generally mirrors OVMF commit 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module", 2018-03-09), from the initial TPM2 enablement work. In that commit, the Tcg2PhysicalPresenceLib class was resolved to the SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf instance only. (And then later it was udpated by commit#3, from the TPM2 PPI series.) Now... I guess it does make sense to mirror OVMF's lib class resolution here, even if we don't port the rest of the PPI stuff to ArmVirtQemu at this time; such as commit#1 and commit #4. (Note: I do agree we shouldn't port the PPI stuff now.) (1) So ultimtely my request is to add the following to the commit message: -------- This patch ports (parts of) the following OvmfPkg commits to ArmVirtQemu: - 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module", 2018-03-09) - b9777bb42e4f ("OvmfPkg: add Tcg2PhysicalPresenceLibQemu", 2018-05-22) -- only to match OVMF's current lib class resolutions -------- > + > TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf (2) I agree this is necessary, but it should come in differently. Please see OvmfPkg commit 1ec05b81e59f ("OvmfPkg: use DxeTpmMeasurementLib if and only if TPM2_ENABLE", 2019-07-04). The gist is that, going forward, the TpmMeasurementLib resolution should depend on TPM2_ENABLE only, in ArmVirtQemu as well. But currently we have a SECURE_BOOT_ENABLE-dependent resolution in "ArmVirt.dsc.inc". Therefore, (2a) Please add another patch, before this one, replacing the current TpmMeasurementLib resolution in "ArmVirt.dsc.inc" with an *unconditional* resolution to TpmMeasurementLibNull. Please mention commit 1ec05b81e59f ("OvmfPkg: use DxeTpmMeasurementLib if and only if TPM2_ENABLE", 2019-07-04) in the commit message. (2b) Please add yet another patch, to move / triplicate that (unconditional) TpmMeasurementLib Null resolution from the "ArmVirt.dsc.inc" file, to the individual DSC files, (2c) and in the current patch, please conditionalize the TpmMeasurementLib resolution, solely based on TPM2_ENABLE, (2d) also in the current patch, please also mention the commit below, in the commit message section that I request above in (1): ------ - 1ec05b81e59f ("OvmfPkg: use DxeTpmMeasurementLib if and only if TPM2_ENABLE", 2019-07-04) ------ > !endif > > [LibraryClasses.common.PEIM] > @@ -91,6 +93,10 @@ [LibraryClasses.common.PEIM] > [LibraryClasses.common.DXE_DRIVER] > > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf > > +!if $(TPM2_ENABLE) == TRUE > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf > +!endif > + > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > > @@ -469,6 +475,23 @@ [Components.common] > MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf > MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf > > + # > + # TPM2 support > + # > +!if $(TPM2_ENABLE) == TRUE > + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf { > + <LibraryClasses> > + > HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf > + > Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf > + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.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 > + > # > # ACPI Support > # (3) Looks good; please extend the commit message block (1) with the following references: - 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) ... So ultimately, block (1) should refer to *five* commits: ------------ This patch ports (parts of) the following OvmfPkg commits to ArmVirtQemu: - 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe module", 2018-03-09) - b9777bb42e4f ("OvmfPkg: add Tcg2PhysicalPresenceLibQemu", 2018-05-22) -- only to match OVMF's current lib class resolutions - 1ec05b81e59f ("OvmfPkg: use DxeTpmMeasurementLib if and only if TPM2_ENABLE", 2019-07-04) - 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) ------------ > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > index 31f615a9d0f9..63247fd10058 100644 > --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc > @@ -182,3 +182,10 @@ [FV.FvMain] > # Ramdisk support > # > INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf > + > + # > + # TPM2 support > + # > +!if $(TPM2_ENABLE) == TRUE > + INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +!endif > (4) Can you hoist this addition please, so that it follow "UsbMassStorageDxe.inf"? Because that would reflect the DSC listing order more closely. Thanks! Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#54989): https://edk2.groups.io/g/devel/message/54989 Mute This Topic: https://groups.io/mt/71571524/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
