From: "Laszlo Ersek" <lersek@redhat.com>
To: marcandre.lureau@redhat.com, devel@edk2.groups.io
Cc: simon.hardy@itdev.co.uk, stefanb@linux.ibm.com
Subject: Re: [PATCH v3 2/6] OvmfPkg: detect TPM 1.2 in Tcg2ConfigPei
Date: Wed, 26 Feb 2020 11:37:10 +0100 [thread overview]
Message-ID: <dcf1f99b-9b64-d204-a65a-5a53e8a2f784@redhat.com> (raw)
In-Reply-To: <20200226093459.1131530-3-marcandre.lureau@redhat.com>
On 02/26/20 10:34, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Complement commit 6cf1880fb5b ("OvmfPkg: add customized Tcg2ConfigPei
> clone", 2018-03-09) by detecting TPM 1.2 devices.
>
> Since Tpm12RequestUseTpm() returns success on any TPM interface,
> (including FIFO & CRB which are TPM 2.0), try to send a GetTicks TPM
> 1.2 command to probe the version. In case of failure, fallback on TPM
> 2.0 path.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
> OvmfPkg/OvmfPkgIa32.dsc | 2 +
> OvmfPkg/OvmfPkgIa32X64.dsc | 2 +
> OvmfPkg/OvmfPkgX64.dsc | 2 +
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 3 +
> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 81 +++++++++++++++++++-----
> 5 files changed, 75 insertions(+), 15 deletions(-)
>
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 38b013ad9543..293e95a2ae81 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
>
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index e075f0766935..5cfa3fc849fe 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
>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 3b1ebf123b51..78481a62e021 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
>
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> index e34cd6210611..f380b86b5d89 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> @@ -31,11 +31,14 @@
> PeimEntryPoint
> DebugLib
> PeiServicesLib
> + Tpm12CommandLib
> + Tpm12DeviceLib
> Tpm2DeviceLib
>
> [Guids]
> gEfiTpmDeviceSelectedGuid ## PRODUCES ## GUID # Used as a PPI GUID
> gEfiTpmDeviceInstanceTpm20DtpmGuid ## SOMETIMES_CONSUMES
> + gEfiTpmDeviceInstanceTpm12Guid ## SOMETIMES_CONSUMES
>
> [Ppis]
> gPeiTpmInitializationDonePpiGuid ## SOMETIMES_PRODUCES
> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> index 99d571d9fa6d..770229ed9912 100644
> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c
> @@ -18,6 +18,8 @@
> #include <Library/DebugLib.h>
> #include <Library/PeiServicesLib.h>
> #include <Library/Tpm2DeviceLib.h>
> +#include <Library/Tpm12DeviceLib.h>
> +#include <Library/Tpm12CommandLib.h>
> #include <Ppi/TpmInitialized.h>
>
> STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmSelectedPpi = {
> @@ -32,6 +34,43 @@ STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmInitializationDonePpiList = {
> NULL
> };
>
> +#pragma pack(1)
> +
> +typedef struct {
> + TPM_RSP_COMMAND_HDR Hdr;
> + TPM_CURRENT_TICKS CurrentTicks;
> +} TPM_RSP_GET_TICKS;
> +
> +#pragma pack()
(1) Please insert a space character before the opening parens.
> +
> +/**
> + Probe for the TPM for 1.2 version, by sending TPM1.2 GetTicks
> +
> + Sending a TPM1.2 command to a TPM2 should return a TPM1.2
> + header (tag = 0xc4) and error code (TPM_BADTAG = 0x1e)
> +**/
> +static EFI_STATUS
(2) Please break EFI_STATUS to a separate line.
(I'm not asking for replacing "static" with "STATIC" because of
<https://bugzilla.tianocore.org/show_bug.cgi?id=1767> -- so that's fine.)
> +TestTpm12 (
> + )
> +{
> + EFI_STATUS Status;
> + TPM_RQU_COMMAND_HDR Command;
> + TPM_RSP_GET_TICKS Response;
> + UINT32 Length;
> +
> + Command.tag = SwapBytes16 (TPM_TAG_RQU_COMMAND);
> + Command.paramSize = SwapBytes32 (sizeof (Command));
> + Command.ordinal = SwapBytes32 (TPM_ORD_GetTicks);
> +
> + Length = sizeof (Response);
> + Status = Tpm12SubmitCommand (sizeof (Command), (UINT8 *)&Command, &Length, (UINT8 *)&Response);
> + if (EFI_ERROR (Status)) {
> + return Status;
> + }
> +
> + return EFI_SUCCESS;
> +}
> +
> /**
> The entry point for Tcg2 configuration driver.
>
> @@ -50,27 +89,39 @@ Tcg2ConfigPeimEntryPoint (
>
> DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__));
>
> - Status = Tpm2RequestUseTpm ();
> - if (!EFI_ERROR (Status)) {
> - DEBUG ((DEBUG_INFO, "%a: TPM2 detected\n", __FUNCTION__));
> - Size = sizeof (gEfiTpmDeviceInstanceTpm20DtpmGuid);
> + Status = Tpm12RequestUseTpm ();
> + if (!EFI_ERROR (Status) && TestTpm12 () == EFI_SUCCESS) {
(3) Please write EFI_ERROR (TestTpm12 ()); or else break the TestTpm12
() invocation out to a separate assignment to Status, and then check
Status again.
> + DEBUG ((DEBUG_INFO, "%a: TPM1.2 detected\n", __FUNCTION__));
> + Size = sizeof (gEfiTpmDeviceInstanceTpm12Guid);
> Status = PcdSetPtrS (
> PcdTpmInstanceGuid,
> &Size,
> - &gEfiTpmDeviceInstanceTpm20DtpmGuid
> + &gEfiTpmDeviceInstanceTpm12Guid
> );
> ASSERT_EFI_ERROR (Status);
> } else {
> - DEBUG ((DEBUG_INFO, "%a: no TPM2 detected\n", __FUNCTION__));
> - //
> - // If no TPM2 was detected, we still need to install
> - // TpmInitializationDonePpi. Namely, Tcg2Pei will exit early upon seeing
> - // the default (all-bits-zero) contents of PcdTpmInstanceGuid, thus we have
> - // to install the PPI in its place, in order to unblock any dependent
> - // PEIMs.
> - //
> - Status = PeiServicesInstallPpi (&mTpmInitializationDonePpiList);
> - ASSERT_EFI_ERROR (Status);
> + Status = Tpm2RequestUseTpm ();
> + if (!EFI_ERROR (Status)) {
> + DEBUG ((DEBUG_INFO, "%a: TPM2 detected\n", __FUNCTION__));
> + Size = sizeof (gEfiTpmDeviceInstanceTpm20DtpmGuid);
> + Status = PcdSetPtrS (
> + PcdTpmInstanceGuid,
> + &Size,
> + &gEfiTpmDeviceInstanceTpm20DtpmGuid
> + );
> + ASSERT_EFI_ERROR (Status);
> + } else {
> + 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
> + // the default (all-bits-zero) contents of PcdTpmInstanceGuid, thus we have
> + // to install the PPI in its place, in order to unblock any dependent
> + // PEIMs.
> + //
> + Status = PeiServicesInstallPpi (&mTpmInitializationDonePpiList);
> + ASSERT_EFI_ERROR (Status);
> + }
> }
>
> //
>
Very happy about this version. With (1) through (3) fixed:
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Thanks!
Laszlo
next prev parent reply other threads:[~2020-02-26 10:37 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-02-26 9:34 [PATCH v3 0/6] Ovmf: enable TPM 1.2 marcandre.lureau
2020-02-26 9:34 ` [PATCH v3 1/6] OvmfPkg: rename TPM2 config prefix to TPM marcandre.lureau
2020-02-26 9:34 ` [PATCH v3 2/6] OvmfPkg: detect TPM 1.2 in Tcg2ConfigPei marcandre.lureau
2020-02-26 10:37 ` Laszlo Ersek [this message]
2020-02-26 14:02 ` Stefan Berger
2020-02-26 9:34 ` [PATCH v3 3/6] OvmfPkg: include TcgPei module marcandre.lureau
2020-02-26 10:37 ` Laszlo Ersek
2020-02-26 9:34 ` [PATCH v3 4/6] OvmfPkg: include TcgDxe module marcandre.lureau
2020-02-26 10:40 ` Laszlo Ersek
2020-02-26 9:34 ` [PATCH v3 5/6] OvmfPkg: plug DxeTpmMeasureBootLib into SecurityStubDxe marcandre.lureau
2020-02-26 10:41 ` Laszlo Ersek
2020-02-26 9:34 ` [PATCH v3 6/6] OvmfPkg: add TCG Configuration menu to the Device Manager menu marcandre.lureau
2020-02-26 10:46 ` Laszlo Ersek
2020-02-26 14:29 ` [edk2-devel] " Marc-André Lureau
2020-02-26 10:47 ` [PATCH v3 0/6] Ovmf: enable TPM 1.2 Laszlo Ersek
2020-02-26 10:55 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=dcf1f99b-9b64-d204-a65a-5a53e8a2f784@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox