[edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support

marcandre.lureau@redhat.com posted 3 patches 5 years, 12 months ago
There is a newer version of this series
[edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support
Posted by marcandre.lureau@redhat.com 5 years, 12 months ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

Enable TcgPei & TcgDxe modules to initialize a TPM 1.2 device and
measure boot environment.

Tpm12RequestUseTpm() returns success on any TPM interface, including
FIFO & CRB which are TPM 2.0. Check the actual interface with
Tpm12GetPtpInterfaceType(), and only detect 1.2 if it's a TIS.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 OvmfPkg/OvmfPkgIa32.dsc                  | 15 +++++++++++++++
 OvmfPkg/OvmfPkgIa32.fdf                  |  2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc               | 15 +++++++++++++++
 OvmfPkg/OvmfPkgIa32X64.fdf               |  2 ++
 OvmfPkg/OvmfPkgX64.dsc                   | 15 +++++++++++++++
 OvmfPkg/OvmfPkgX64.fdf                   |  2 ++
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf |  3 +++
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c  | 17 ++++++++++++++++-
 8 files changed, 70 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 38b013ad9543..02300886563e 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -206,6 +206,7 @@
   XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

 

 !if $(TPM_ENABLE) == TRUE

+  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf

   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf

   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf

   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf

@@ -281,6 +282,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf

 !endif

 

@@ -361,6 +363,7 @@
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf

   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf

 !if $(TPM_ENABLE) == TRUE

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf

 !endif

 

@@ -633,6 +636,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+  SecurityPkg/Tcg/TcgPei/TcgPei.inf

   SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {

     <LibraryClasses>

       HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf

@@ -668,6 +672,7 @@
       NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf

 !endif

 !if $(TPM_ENABLE) == TRUE

+      NULL|SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf

       NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf

 !endif

   }

@@ -926,5 +931,15 @@
   }

 !if $(TPM_CONFIG_ENABLE) == TRUE

   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

+!endif

+  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {

+    <LibraryClasses>

+      Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

+  }

+!if $(TPM_CONFIG_ENABLE) == TRUE

+  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {

+    <LibraryClasses>

+      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

+  }

 !endif

 !endif

diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
index 2c7d6cccdfb0..b0ddc5a4ae73 100644
--- a/OvmfPkg/OvmfPkgIa32.fdf
+++ b/OvmfPkg/OvmfPkgIa32.fdf
@@ -161,6 +161,7 @@ INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 

 !if $(TPM_ENABLE) == TRUE

 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+INF  SecurityPkg/Tcg/TcgPei/TcgPei.inf

 INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf

 !endif

 

@@ -347,6 +348,7 @@ INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 # TPM support

 #

 !if $(TPM_ENABLE) == TRUE

+INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf

 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf

 !if $(TPM_CONFIG_ENABLE) == TRUE

 INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index e075f0766935..3adc75223d05 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -211,6 +211,7 @@
   XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

 

 !if $(TPM_ENABLE) == TRUE

+  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf

   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf

   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf

   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf

@@ -286,6 +287,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf

 !endif

 

@@ -366,6 +368,7 @@
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf

   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf

 !if $(TPM_ENABLE) == TRUE

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf

 !endif

 

@@ -645,6 +648,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+  SecurityPkg/Tcg/TcgPei/TcgPei.inf

   SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {

     <LibraryClasses>

       HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf

@@ -681,6 +685,7 @@
       NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf

 !endif

 !if $(TPM_ENABLE) == TRUE

+      NULL|SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf

       NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf

 !endif

   }

@@ -940,5 +945,15 @@
   }

 !if $(TPM_CONFIG_ENABLE) == TRUE

   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

+!endif

+  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {

+    <LibraryClasses>

+      Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

+  }

+!if $(TPM_CONFIG_ENABLE) == TRUE

+  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {

+    <LibraryClasses>

+      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

+  }

 !endif

 !endif

diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
index 6a4c9089ab58..dffbfaa5fc4f 100644
--- a/OvmfPkg/OvmfPkgIa32X64.fdf
+++ b/OvmfPkg/OvmfPkgIa32X64.fdf
@@ -161,6 +161,7 @@ INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 

 !if $(TPM_ENABLE) == TRUE

 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+INF  SecurityPkg/Tcg/TcgPei/TcgPei.inf

 INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf

 !endif

 

@@ -354,6 +355,7 @@ INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 # TPM support

 #

 !if $(TPM_ENABLE) == TRUE

+INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf

 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf

 !if $(TPM_CONFIG_ENABLE) == TRUE

 INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index 3b1ebf123b51..5f3740ae890a 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -211,6 +211,7 @@
   XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf

 

 !if $(TPM_ENABLE) == TRUE

+  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf

   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf

   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf

   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf

@@ -286,6 +287,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf

 !endif

 

@@ -366,6 +368,7 @@
   MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf

   QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf

 !if $(TPM_ENABLE) == TRUE

+  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf

   Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf

 !endif

 

@@ -644,6 +647,7 @@
 

 !if $(TPM_ENABLE) == TRUE

   OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+  SecurityPkg/Tcg/TcgPei/TcgPei.inf

   SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {

     <LibraryClasses>

       HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf

@@ -679,6 +683,7 @@
       NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf

 !endif

 !if $(TPM_ENABLE) == TRUE

+      NULL|SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf

       NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf

 !endif

   }

@@ -938,5 +943,15 @@
   }

 !if $(TPM_CONFIG_ENABLE) == TRUE

   SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

+!endif

+  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {

+    <LibraryClasses>

+      Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf

+  }

+!if $(TPM_CONFIG_ENABLE) == TRUE

+  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {

+    <LibraryClasses>

+      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf

+  }

 !endif

 !endif

diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
index 6a4c9089ab58..dffbfaa5fc4f 100644
--- a/OvmfPkg/OvmfPkgX64.fdf
+++ b/OvmfPkg/OvmfPkgX64.fdf
@@ -161,6 +161,7 @@ INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
 

 !if $(TPM_ENABLE) == TRUE

 INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf

+INF  SecurityPkg/Tcg/TcgPei/TcgPei.inf

 INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf

 !endif

 

@@ -354,6 +355,7 @@ INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
 # TPM support

 #

 !if $(TPM_ENABLE) == TRUE

+INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf

 INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf

 !if $(TPM_CONFIG_ENABLE) == TRUE

 INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf

diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
index e34cd6210611..15f9b7cda099 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -31,17 +31,20 @@
   PeimEntryPoint

   DebugLib

   PeiServicesLib

+  Tpm12DeviceLib

   Tpm2DeviceLib

 

 [Guids]

   gEfiTpmDeviceSelectedGuid           ## PRODUCES ## GUID # Used as a PPI GUID

   gEfiTpmDeviceInstanceTpm20DtpmGuid  ## SOMETIMES_CONSUMES

+  gEfiTpmDeviceInstanceTpm12Guid      ## SOMETIMES_CONSUMES

 

 [Ppis]

   gPeiTpmInitializationDonePpiGuid    ## SOMETIMES_PRODUCES

 

 [Pcd]

   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid                 ## PRODUCES

+  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType          ## SOMETIMES_CONSUMES

 

 [Depex]

   TRUE

diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
index 99d571d9fa6d..ae3d4fc2c380 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
@@ -18,6 +18,7 @@
 #include <Library/DebugLib.h>

 #include <Library/PeiServicesLib.h>

 #include <Library/Tpm2DeviceLib.h>

+#include <Library/Tpm12DeviceLib.h>

 #include <Ppi/TpmInitialized.h>

 

 STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {

@@ -50,6 +51,19 @@ Tcg2ConfigPeimEntryPoint (
 

   DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));

 

+  Status = Tpm12RequestUseTpm ();

+  if (!EFI_ERROR (Status) && Tpm12GetPtpInterfaceType () == PtpInterfaceTis) {

+      DEBUG ((DEBUG_INFO, "%a: TPM1.2 detected\n", __FUNCTION__));

+      Size = sizeof (gEfiTpmDeviceInstanceTpm12Guid);

+      Status = PcdSetPtrS (

+          PcdTpmInstanceGuid,

+          &Size,

+          &gEfiTpmDeviceInstanceTpm12Guid

+          );

+      ASSERT_EFI_ERROR (Status);

+      goto done;

+  }

+

   Status = Tpm2RequestUseTpm ();

   if (!EFI_ERROR (Status)) {

     DEBUG ((DEBUG_INFO, "%a: TPM2 detected\n", __FUNCTION__));

@@ -61,7 +75,7 @@ Tcg2ConfigPeimEntryPoint (
                );

     ASSERT_EFI_ERROR (Status);

   } else {

-    DEBUG ((DEBUG_INFO, "%a: no TPM2 detected\n", __FUNCTION__));

+    DEBUG ((DEBUG_INFO, "%a: no TPM detected\n", __FUNCTION__));

     //

     // If no TPM2 was detected, we still need to install

     // TpmInitializationDonePpi. Namely, Tcg2Pei will exit early upon seeing

@@ -73,6 +87,7 @@ Tcg2ConfigPeimEntryPoint (
     ASSERT_EFI_ERROR (Status);

   }

 

+done:

   //

   // Selection done

   //

-- 
2.25.0.rc2.1.g09a9a1a997


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54360): https://edk2.groups.io/g/devel/message/54360
Mute This Topic: https://groups.io/mt/71240875/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support
Posted by Laszlo Ersek 5 years, 12 months ago
Hi Marc-André,

On 02/13/20 14:12, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Enable TcgPei & TcgDxe modules to initialize a TPM 1.2 device and
> measure boot environment.
> 
> Tpm12RequestUseTpm() returns success on any TPM interface, including
> FIFO & CRB which are TPM 2.0. Check the actual interface with
> Tpm12GetPtpInterfaceType(), and only detect 1.2 if it's a TIS.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  OvmfPkg/OvmfPkgIa32.dsc                  | 15 +++++++++++++++
>  OvmfPkg/OvmfPkgIa32.fdf                  |  2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc               | 15 +++++++++++++++
>  OvmfPkg/OvmfPkgIa32X64.fdf               |  2 ++
>  OvmfPkg/OvmfPkgX64.dsc                   | 15 +++++++++++++++
>  OvmfPkg/OvmfPkgX64.fdf                   |  2 ++
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf |  3 +++
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c  | 17 ++++++++++++++++-
>  8 files changed, 70 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 38b013ad9543..02300886563e 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -206,6 +206,7 @@
>    XenPlatformLib|OvmfPkg/Library/XenPlatformLib/XenPlatformLib.inf
> 
>  
> 
>  !if $(TPM_ENABLE) == TRUE
> 
> +  Tpm12CommandLib|SecurityPkg/Library/Tpm12CommandLib/Tpm12CommandLib.inf
> 
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> 
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
> 
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> 
> @@ -281,6 +282,7 @@
>  
> 
>  !if $(TPM_ENABLE) == TRUE
> 
>    BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> 
> +  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> 
>    Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> 
>  !endif
> 
>  
> 

OK, these reflect commit [1] 6cf1880fb5b6 ("OvmfPkg: add customized
Tcg2ConfigPei clone", 2018-03-09).


> @@ -361,6 +363,7 @@
>    MpInitLib|UefiCpuPkg/Library/MpInitLib/DxeMpInitLib.inf
> 
>    QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf
> 
>  !if $(TPM_ENABLE) == TRUE
> 
> +  Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibTcg/Tpm12DeviceLibTcg.inf
> 
>    Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
> 
>  !endif
> 
>  
> 

This reflects commit [3] 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe
module", 2018-03-09).


> @@ -633,6 +636,7 @@
>  
> 
>  !if $(TPM_ENABLE) == TRUE
> 
>    OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> 
> +  SecurityPkg/Tcg/TcgPei/TcgPei.inf
> 
>    SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> 
>      <LibraryClasses>
> 
>        HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> 

Mirrors commit [2] 4672a4892867 ("OvmfPkg: include Tcg2Pei module",
2018-03-09).

> @@ -668,6 +672,7 @@
>        NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> 
>  !endif
> 
>  !if $(TPM_ENABLE) == TRUE
> 
> +      NULL|SecurityPkg/Library/DxeTpmMeasureBootLib/DxeTpmMeasureBootLib.inf
> 
>        NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> 
>  !endif
> 
>    }
> 

Mirrors commit [4] d5a002aba0aa ("OvmfPkg: plug DxeTpm2MeasureBootLib
into SecurityStubDxe", 2018-03-09)


> @@ -926,5 +931,15 @@
>    }
> 
>  !if $(TPM_CONFIG_ENABLE) == TRUE
> 
>    SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> 
> +!endif
> 
> +  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf {
> 
> +    <LibraryClasses>
> 
> +      Tpm12DeviceLib|SecurityPkg/Library/Tpm12DeviceLibDTpm/Tpm12DeviceLibDTpm.inf
> 
> +  }

Again reflects commit [3] 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe
module", 2018-03-09).

> 
> +!if $(TPM_CONFIG_ENABLE) == TRUE
> 
> +  SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf {
> 
> +    <LibraryClasses>
> 
> +      PcdLib|MdePkg/Library/DxePcdLib/DxePcdLib.inf
> 
> +  }
> 
>  !endif
> 
>  !endif
> 

This matches commit [5] 3103389043bd ("OvmfPkg: Add TCG2 Configuration
menu to the Device Manager menu", 2019-02-11).

... Which was later cleaned up by commit cf3ad972a210 ("OvmfPkg:
reorganize TPM2 support in DSC/FDF files", 2020-01-09).

> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 2c7d6cccdfb0..b0ddc5a4ae73 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -161,6 +161,7 @@ INF  UefiCpuPkg/CpuMpPei/CpuMpPei.inf
>  
> 
>  !if $(TPM_ENABLE) == TRUE
> 
>  INF  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> 
> +INF  SecurityPkg/Tcg/TcgPei/TcgPei.inf
> 
>  INF  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> 
>  !endif
> 
>  
> 

Mirrors commit [2] 4672a4892867 ("OvmfPkg: include Tcg2Pei module",
2018-03-09).

> @@ -347,6 +348,7 @@ INF  MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
>  # TPM support
> 
>  #
> 
>  !if $(TPM_ENABLE) == TRUE
> 
> +INF  SecurityPkg/Tcg/TcgDxe/TcgDxe.inf
> 
>  INF  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> 
>  !if $(TPM_CONFIG_ENABLE) == TRUE
> 
>  INF  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> 

Again reflects commit [3] 0c0a50d6b3ff ("OvmfPkg: include Tcg2Dxe
module", 2018-03-09).


So, my requests thus far:

(1) Please split this part of the patch into five separate patches, in
parallel to commits [1] through [5].

The messages on the new patches need not be very long, they should
basically repeat the original subject lines, customized for TPM-1.2, and
refer to the TPM-2 commit that they mirror.


(2) Where you add TcgDxe and TcgConfigDxe to the DSC file, I'd prefer if
we didn't duplicate the TPM_CONFIG_ENABLE condition. Can you please add
TcgDxe just above Tcg2Dxe, and TcgConfigDxe just above Tcg2ConfigDxe?

Because, this would be consistent with the rest of the DSC file updates,
as you (nicely) add the TPM-1.2 artifacts just above the TPM-2.0 ones.


(3) In the FDF file, you forgot to add
"SecurityPkg/Tcg/TcgConfigDxe/TcgConfigDxe.inf" (paralleling commit [5]
3103389043bd ("OvmfPkg: Add TCG2 Configuration menu to the Device
Manager menu", 2019-02-11)).

[...]

> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> index e34cd6210611..15f9b7cda099 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -31,17 +31,20 @@
>    PeimEntryPoint
> 
>    DebugLib
> 
>    PeiServicesLib
> 
> +  Tpm12DeviceLib
> 
>    Tpm2DeviceLib
> 
>  
> 
>  [Guids]
> 
>    gEfiTpmDeviceSelectedGuid           ## PRODUCES ## GUID # Used as a PPI GUID
> 
>    gEfiTpmDeviceInstanceTpm20DtpmGuid  ## SOMETIMES_CONSUMES
> 
> +  gEfiTpmDeviceInstanceTpm12Guid      ## SOMETIMES_CONSUMES
> 
>  
> 
>  [Ppis]
> 
>    gPeiTpmInitializationDonePpiGuid    ## SOMETIMES_PRODUCES
> 
>  
> 
>  [Pcd]
> 
>    gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid                 ## PRODUCES
> 
> +  gEfiSecurityPkgTokenSpaceGuid.PcdActiveTpmInterfaceType          ## SOMETIMES_CONSUMES

(4) This shouldn't be necessary. The PCD is not referenced in this patch
anywhere else.


> 
>  
> 
>  [Depex]
> 
>    TRUE
> 
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> index 99d571d9fa6d..ae3d4fc2c380 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> @@ -18,6 +18,7 @@
>  #include <Library/DebugLib.h>
> 
>  #include <Library/PeiServicesLib.h>
> 
>  #include <Library/Tpm2DeviceLib.h>
> 
> +#include <Library/Tpm12DeviceLib.h>
> 
>  #include <Ppi/TpmInitialized.h>
> 
>  
> 
>  STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {
> 
> @@ -50,6 +51,19 @@ Tcg2ConfigPeimEntryPoint (
>  
> 
>    DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
> 
>  
> 
> +  Status = Tpm12RequestUseTpm ();
> 
> +  if (!EFI_ERROR (Status) && Tpm12GetPtpInterfaceType () == PtpInterfaceTis) {
> 
> +      DEBUG ((DEBUG_INFO, "%a: TPM1.2 detected\n", __FUNCTION__));
> 
> +      Size = sizeof (gEfiTpmDeviceInstanceTpm12Guid);
> 
> +      Status = PcdSetPtrS (
> 
> +          PcdTpmInstanceGuid,
> 
> +          &Size,
> 
> +          &gEfiTpmDeviceInstanceTpm12Guid
> 
> +          );

(5) The indentation is not correct; it should be two spaces to the right
of the start of the word "PcdSetPtrS".

(6) IIUC, we shouldn't use the Tpm12GetPtpInterfaceType() function here,
per Jiewen's comment. (Sorry, I can't comment on patch#2.)

> 
> +      ASSERT_EFI_ERROR (Status);
> 
> +      goto done;

(7) Use of "goto" is generally restricted to error handling; please use
"else" here. (Independently, the label should start with a capital letter.)

> 
> +  }
> 
> +
> 
>    Status = Tpm2RequestUseTpm ();
> 
>    if (!EFI_ERROR (Status)) {
> 
>      DEBUG ((DEBUG_INFO, "%a: TPM2 detected\n", __FUNCTION__));
> 
> @@ -61,7 +75,7 @@ Tcg2ConfigPeimEntryPoint (
>                 );
> 
>      ASSERT_EFI_ERROR (Status);
> 
>    } else {
> 
> -    DEBUG ((DEBUG_INFO, "%a: no TPM2 detected\n", __FUNCTION__));
> 
> +    DEBUG ((DEBUG_INFO, "%a: no TPM detected\n", __FUNCTION__));
> 
>      //
> 
>      // If no TPM2 was detected, we still need to install
> 
>      // TpmInitializationDonePpi. Namely, Tcg2Pei will exit early upon seeing
> 
> @@ -73,6 +87,7 @@ Tcg2ConfigPeimEntryPoint (
>      ASSERT_EFI_ERROR (Status);
> 
>    }
> 
>  
> 
> +done:
> 
>    //
> 
>    // Selection done
> 
>    //
> 

Thanks!
Laszlo


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#54473): https://edk2.groups.io/g/devel/message/54473
Mute This Topic: https://groups.io/mt/71240875/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-