[Qemu-devel] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Qemu-devel] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Hi,

The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
with TPM2 (I haven't tested TPM1, for lack of interest).

PPI test runs successfully with Windows 10 WHLK, despite the limited
number of supported funcions (tpm2_ppi_funcs table, in particular, no
function allows to manipulate Tcg2PhysicalPresenceFlags)

The way it works is relatively simple: a memory region is allocated by
QEMU to save PPI related variables. An ACPI interface is exposed by
QEMU to let the guest manipulate those. At boot, ovmf processes and
updates the PPI qemu region and request variables.

I build edk2 with:

$ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE

I test with qemu & swtpm/libtpms (tpm2 branches, swtpm_setup.sh --tpm2 --tpm-state tpmstatedir)

$ swtpm socket --tpmstate tpmstatedir --ctrl type=unixio,path=tpmsock  --tpm2 &
$ qemu .. -chardev socket,id=chrtpm,path=tpmsock -tpmdev emulator,id=tpm0,chardev=chrtpm -device tpm-crb,tpmdev=tpm0

Github trees:
https://github.com/elmarco/edk2/tree/tpm-ppi
https://github.com/elmarco/qemu/tree/tpm-ppi

Thanks

Marc-André Lureau (4):
  ovmf: add and link with Tcg2PhysicalPresenceLibNull when !TPM2_ENABLE
  ovmf: add QemuTpm.h header
  ovmf: replace SecurityPkg with OvfmPkg Tcg2PhysicalPresenceLibQemu
  ovmf: process TPM PPI request in AfterConsole()

 OvmfPkg/Include/IndustryStandard/QemuTpm.h    |  67 ++
 .../PlatformBootManagerLib/BdsPlatform.c      |   8 +
 .../PlatformBootManagerLib.inf                |   2 +
 .../DxeTcg2PhysicalPresenceLib.c              |  26 +
 .../DxeTcg2PhysicalPresenceLib.inf            |  34 +
 .../DxeTcg2PhysicalPresenceLib.c              | 881 ++++++++++++++++++
 .../DxeTcg2PhysicalPresenceLib.inf            |  67 ++
 .../DxeTcg2PhysicalPresenceLib.uni            |  26 +
 .../PhysicalPresenceStrings.uni               |  49 +
 OvmfPkg/OvmfPkgIa32.dsc                       |   4 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                    |   4 +-
 OvmfPkg/OvmfPkgX64.dsc                        |   4 +-
 12 files changed, 1169 insertions(+), 3 deletions(-)
 create mode 100644 OvmfPkg/Include/IndustryStandard/QemuTpm.h
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni

-- 
2.17.0.253.g3dd125b46d

[Qemu-devel] [PATCH 1/4] ovmf: add and link with Tcg2PhysicalPresenceLibNull when !TPM2_ENABLE

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

This NULL library will let us call
Tcg2PhysicalPresenceLibProcessRequest() unconditionally from
BdsPlatform when building without TPM2_ENABLE.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 .../DxeTcg2PhysicalPresenceLib.c              | 26 ++++++++++++++
 .../DxeTcg2PhysicalPresenceLib.inf            | 34 +++++++++++++++++++
 OvmfPkg/OvmfPkgIa32.dsc                       |  2 ++
 OvmfPkg/OvmfPkgIa32X64.dsc                    |  2 ++
 OvmfPkg/OvmfPkgX64.dsc                        |  2 ++
 5 files changed, 66 insertions(+)
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf

diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
new file mode 100644
index 000000000000..0b8b98410315
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
@@ -0,0 +1,26 @@
+/** @file
+  NULL Tcg2PhysicalPresenceLib library instance
+
+  Copyright (c) 2018, Red Hat, Inc.
+  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+  This program and the accompanying materials
+  are licensed and made available under the terms and conditions of the BSD License
+  which accompanies this distribution.  The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include "PiDxe.h"
+#include <Library/Tcg2PhysicalPresenceLib.h>
+
+VOID
+EFIAPI
+Tcg2PhysicalPresenceLibProcessRequest (
+  IN      TPM2B_AUTH                     *PlatformAuth  OPTIONAL
+  )
+{
+    return;
+}
diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
new file mode 100644
index 000000000000..e6f6239e1e00
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
@@ -0,0 +1,34 @@
+## @file
+#  NULL Tcg2PhysicalPresenceLib library instance
+#
+#  In SecurityPkg, this library will check and execute TPM 1.2 request
+#  from OS or BIOS. The request may ask for user confirmation before
+#  execution. This Library will also lock TPM physical presence at
+#  last.
+#
+# Copyright (C) 2018, Red Hat, Inc.
+# Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeTcg2PhysicalPresenceLibNull
+  FILE_GUID                      = 2A6BA243-DC22-42D8-9C3D-AE3728DC7AFA
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = Tcg2PhysicalPresenceLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER UEFI_APPLICATION UEFI_DRIVER
+
+[Sources]
+  DxeTcg2PhysicalPresenceLib.c
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  SecurityPkg/SecurityPkg.dec
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 2d6c4c4615b6..6c361b73cd55 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -208,6 +208,8 @@ [LibraryClasses]
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
   Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
+!else
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
 !endif
 
 [LibraryClasses.common]
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 43158c5f0627..62a6075a671d 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -213,6 +213,8 @@ [LibraryClasses]
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
   Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
+!else
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
 !endif
 
 [LibraryClasses.common]
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index d1fdf7c307c2..cbab1aa328c6 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -213,6 +213,8 @@ [LibraryClasses]
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
   Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
+!else
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
 !endif
 
 [LibraryClasses.common]
-- 
2.17.0.253.g3dd125b46d

[Qemu-devel] [PATCH 2/4] ovmf: add QemuTpm.h header

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Add some common macros and type definitions corresponding to the QEMU
TPM interface.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 OvmfPkg/Include/IndustryStandard/QemuTpm.h | 67 ++++++++++++++++++++++
 1 file changed, 67 insertions(+)
 create mode 100644 OvmfPkg/Include/IndustryStandard/QemuTpm.h

diff --git a/OvmfPkg/Include/IndustryStandard/QemuTpm.h b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
new file mode 100644
index 000000000000..054cf79374b5
--- /dev/null
+++ b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
@@ -0,0 +1,67 @@
+/** @file
+  Macro and type definitions corresponding to the QEMU TPM interface.
+
+  Refer to "docs/specs/tpm.txt" in the QEMU source directory.
+
+  Copyright (C) 2018, Red Hat, Inc.
+  Copyright (c) 2018, IBM Corporation. All rights reserved.<BR>
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#ifndef __QEMU_TPM_H__
+#define __QEMU_TPM_H__
+
+#include <Base.h>
+
+/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
+#define QEMU_TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
+#define QEMU_TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
+#define QEMU_TPM_PPI_FUNC_BLOCKED             (2 << 0)
+#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
+#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
+#define QEMU_TPM_PPI_FUNC_MASK                (7 << 0)
+
+//
+// The following structure is shared between firmware and ACPI.
+//
+#pragma pack (1)
+typedef struct {
+  UINT8 Func[256];           /* func */
+  UINT8 In;                  /* ppin */
+  UINT32 Ip;                 /* ppip */
+  UINT32 Response;           /* pprp */
+  UINT32 Request;            /* pprq */
+  UINT32 RequestParameter;   /* pprm */
+  UINT32 LastRequest;        /* lppr */
+  UINT32 FRet;               /* fret */
+  UINT8 Res1[0x40];          /* res1 */
+  UINT8 NextStep;            /* next_step */
+} QEMU_TPM_PPI;
+#pragma pack ()
+
+//
+// The following structure is for the fw_cfg etc/tpm/config file.
+//
+#pragma pack (1)
+typedef struct {
+  UINT32 PpiAddress;
+  UINT8 TpmVersion;
+  UINT8 PpiVersion;
+} QEMU_FWCFG_TPM_CONFIG;
+#pragma pack ()
+
+#define QEMU_TPM_VERSION_UNSPEC    0
+#define QEMU_TPM_VERSION_1_2       1
+#define QEMU_TPM_VERSION_2         2
+
+#define QEMU_TPM_PPI_VERSION_NONE  0
+#define QEMU_TPM_PPI_VERSION_1_30  1
+
+#endif
-- 
2.17.0.253.g3dd125b46d

[Qemu-devel] [PATCH 3/4] ovmf: replace SecurityPkg with OvfmPkg Tcg2PhysicalPresenceLibQemu

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Cloned "SecurityPkg/Library/DxeTcg2PhysicalPresenceLib" and:

- removed all the functions that are unreachable from
  Tcg2PhysicalPresenceLibProcessRequest()

- replaced everything that's related to the
  TCG2_PHYSICAL_PRESENCE*_VARIABLE variables, with direct access to
  the QEMU structures.

This commit is based on initial experimental work from Stefan Berger.
In particular, he wrote most of QEMU PPI support, and designed the
qemu/firmware interaction. Initially, Stefan tried to reuse the
existing SecurityPkg code, but we eventually decided to get rid of the
variables and simplify the ovmf/qemu version.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 .../DxeTcg2PhysicalPresenceLib.c              | 881 ++++++++++++++++++
 .../DxeTcg2PhysicalPresenceLib.inf            |  67 ++
 .../DxeTcg2PhysicalPresenceLib.uni            |  26 +
 .../PhysicalPresenceStrings.uni               |  49 +
 OvmfPkg/OvmfPkgIa32.dsc                       |   2 +-
 OvmfPkg/OvmfPkgIa32X64.dsc                    |   2 +-
 OvmfPkg/OvmfPkgX64.dsc                        |   2 +-
 7 files changed, 1026 insertions(+), 3 deletions(-)
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
 create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni

diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
new file mode 100644
index 000000000000..da45f990369a
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
@@ -0,0 +1,881 @@
+/** @file
+  Execute pending TPM2 requests from OS or BIOS.
+
+  Caution: This module requires additional review when modified.
+  This driver will have external input - variable.
+  This external input must be validated carefully to avoid security issue.
+
+  Tcg2ExecutePendingTpmRequest() will receive untrusted input and do validation.
+
+Copyright (C) 2018, Red Hat, Inc.
+Copyright (c) 2018, IBM Corporation. All rights reserved.<BR>
+Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#include <PiDxe.h>
+
+#include <Guid/Tcg2PhysicalPresenceData.h>
+#include <IndustryStandard/QemuTpm.h>
+#include <Protocol/Tcg2Protocol.h>
+
+#include <Library/BaseMemoryLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HiiLib.h>
+#include <Library/HobLib.h>
+#include <Library/MemoryAllocationLib.h>
+#include <Library/PrintLib.h>
+#include <Library/QemuFwCfgLib.h>
+#include <Library/Tpm2CommandLib.h>
+#include <Library/UefiBootServicesTableLib.h>
+#include <Library/UefiLib.h>
+#include <Library/UefiRuntimeServicesTableLib.h>
+
+#include <Library/Tcg2PhysicalPresenceLib.h>
+
+#define CONFIRM_BUFFER_SIZE         4096
+
+EFI_HII_HANDLE mTcg2PpStringPackHandle;
+
+#define TPM_PPI_FLAGS (QEMU_TPM_PPI_FUNC_ALLOWED_USR_REQ)
+
+STATIC CONST UINT8 mTpm2PPIFuncs[] = {
+  [TCG2_PHYSICAL_PRESENCE_NO_ACTION] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_CLEAR] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_CHANGE_EPS] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID] = TPM_PPI_FLAGS,
+  [TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID] = TPM_PPI_FLAGS,
+};
+
+STATIC QEMU_TPM_PPI *mPpi;
+
+
+/**
+  Reads QEMU PPI config from fw_cfg.
+**/
+EFI_STATUS
+QemuTpmReadConfig (
+  IN QEMU_FWCFG_TPM_CONFIG *Config
+  )
+{
+  EFI_STATUS           Status;
+  FIRMWARE_CONFIG_ITEM FwCfgItem;
+  UINTN                FwCfgSize;
+
+  Status = QemuFwCfgFindFile ("etc/tpm/config", &FwCfgItem, &FwCfgSize);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  if (FwCfgSize != sizeof (*Config)) {
+      return EFI_PROTOCOL_ERROR;
+  }
+
+  QemuFwCfgSelectItem (FwCfgItem);
+  QemuFwCfgReadBytes (sizeof (*Config), Config);
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Initializes QEMU PPI memory region.
+**/
+EFI_STATUS
+QemuTpmInitPPI (
+  VOID
+  )
+{
+  EFI_STATUS Status;
+  QEMU_FWCFG_TPM_CONFIG Config;
+
+  if (mPpi) {
+      return EFI_SUCCESS;
+  }
+
+  Status = QemuTpmReadConfig (&Config);
+  if (EFI_ERROR (Status)) {
+    return Status;
+  }
+
+  mPpi = (QEMU_TPM_PPI *)(unsigned long)Config.PpiAddress;
+  if (!mPpi) {
+      return EFI_INVALID_PARAMETER;
+  }
+
+  DEBUG ((EFI_D_INFO, "[TPM2PP] mPpi=%x version=%d\n", mPpi, Config.TpmVersion));
+  ZeroMem (&mPpi->Func, sizeof (mPpi->Func));
+  switch (Config.TpmVersion) {
+  case QEMU_TPM_VERSION_2:
+    CopyMem (&mPpi->Func, mTpm2PPIFuncs, sizeof (mTpm2PPIFuncs));
+    break;
+  }
+
+  if (!mPpi->In) {
+    mPpi->In = 1;
+    mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
+    mPpi->LastRequest = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
+    mPpi->NextStep = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
+  }
+
+  return EFI_SUCCESS;
+}
+
+
+/**
+  Get string by string id from HII Interface.
+
+  @param[in] Id          String ID.
+
+  @retval    CHAR16 *    String from ID.
+  @retval    NULL        If error occurs.
+
+**/
+CHAR16 *
+Tcg2PhysicalPresenceGetStringById (
+  IN  EFI_STRING_ID   Id
+  )
+{
+  return HiiGetString (mTcg2PpStringPackHandle, Id, NULL);
+}
+
+
+/**
+  Send ClearControl and Clear command to TPM.
+
+  @param[in]  PlatformAuth      platform auth value. NULL means no platform auth change.
+
+  @retval EFI_SUCCESS           Operation completed successfully.
+  @retval EFI_TIMEOUT           The register can't run into the expected status in time.
+  @retval EFI_BUFFER_TOO_SMALL  Response data buffer is too small.
+  @retval EFI_DEVICE_ERROR      Unexpected device behavior.
+
+**/
+EFI_STATUS
+EFIAPI
+Tpm2CommandClear (
+  IN TPM2B_AUTH                *PlatformAuth  OPTIONAL
+  )
+{
+  EFI_STATUS                Status;
+  TPMS_AUTH_COMMAND         *AuthSession;
+  TPMS_AUTH_COMMAND         LocalAuthSession;
+
+  if (PlatformAuth == NULL) {
+    AuthSession = NULL;
+  } else {
+    AuthSession = &LocalAuthSession;
+    ZeroMem (&LocalAuthSession, sizeof (LocalAuthSession));
+    LocalAuthSession.sessionHandle = TPM_RS_PW;
+    LocalAuthSession.hmac.size = PlatformAuth->size;
+    CopyMem (LocalAuthSession.hmac.buffer, PlatformAuth->buffer, PlatformAuth->size);
+  }
+
+  DEBUG ((EFI_D_INFO, "Tpm2ClearControl ... \n"));
+  Status = Tpm2ClearControl (TPM_RH_PLATFORM, AuthSession, NO);
+  DEBUG ((EFI_D_INFO, "Tpm2ClearControl - %r\n", Status));
+  if (EFI_ERROR (Status)) {
+    goto Done;
+  }
+  DEBUG ((EFI_D_INFO, "Tpm2Clear ... \n"));
+  Status = Tpm2Clear (TPM_RH_PLATFORM, AuthSession);
+  DEBUG ((EFI_D_INFO, "Tpm2Clear - %r\n", Status));
+
+Done:
+  ZeroMem (&LocalAuthSession.hmac, sizeof (LocalAuthSession.hmac));
+  return Status;
+}
+
+
+/**
+  Change EPS.
+
+  @param[in]  PlatformAuth      platform auth value. NULL means no platform auth change.
+
+  @retval EFI_SUCCESS Operation completed successfully.
+**/
+EFI_STATUS
+Tpm2CommandChangeEps (
+  IN TPM2B_AUTH                *PlatformAuth  OPTIONAL
+  )
+{
+  EFI_STATUS                Status;
+  TPMS_AUTH_COMMAND         *AuthSession;
+  TPMS_AUTH_COMMAND         LocalAuthSession;
+
+  if (PlatformAuth == NULL) {
+    AuthSession = NULL;
+  } else {
+    AuthSession = &LocalAuthSession;
+    ZeroMem (&LocalAuthSession, sizeof (LocalAuthSession));
+    LocalAuthSession.sessionHandle = TPM_RS_PW;
+    LocalAuthSession.hmac.size = PlatformAuth->size;
+    CopyMem (LocalAuthSession.hmac.buffer, PlatformAuth->buffer, PlatformAuth->size);
+  }
+
+  Status = Tpm2ChangeEPS (TPM_RH_PLATFORM, AuthSession);
+  DEBUG ((EFI_D_INFO, "Tpm2ChangeEPS - %r\n", Status));
+
+  ZeroMem (&LocalAuthSession.hmac, sizeof(LocalAuthSession.hmac));
+  return Status;
+}
+
+
+/**
+  Execute physical presence operation requested by the OS.
+
+  @param[in]      PlatformAuth        platform auth value. NULL means no platform auth change.
+  @param[in]      CommandCode         Physical presence operation value.
+  @param[in]      CommandParameter    Physical presence operation parameter.
+
+  @retval TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE   Unknown physical presence operation.
+  @retval TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE   Error occurred during sending command to TPM or
+                                                   receiving response from TPM.
+  @retval Others                                   Return code from the TPM device after command execution.
+**/
+UINT32
+Tcg2ExecutePhysicalPresence (
+  IN      TPM2B_AUTH                       *PlatformAuth,  OPTIONAL
+  IN      UINT32                           CommandCode,
+  IN      UINT32                           CommandParameter
+  )
+{
+  EFI_STATUS                        Status;
+  EFI_TCG2_EVENT_ALGORITHM_BITMAP   TpmHashAlgorithmBitmap;
+  UINT32                            ActivePcrBanks;
+
+  switch (CommandCode) {
+    case TCG2_PHYSICAL_PRESENCE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
+      Status = Tpm2CommandClear (PlatformAuth);
+      if (EFI_ERROR (Status)) {
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      } else {
+        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
+      }
+
+    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
+      Status = Tpm2GetCapabilitySupportedAndActivePcrs (&TpmHashAlgorithmBitmap, &ActivePcrBanks);
+      ASSERT_EFI_ERROR (Status);
+
+      //
+      // PP spec requirements:
+      //    Firmware should check that all requested (set) hashing algorithms are supported with respective PCR banks.
+      //    Firmware has to ensure that at least one PCR banks is active.
+      // If not, an error is returned and no action is taken.
+      //
+      if (CommandParameter == 0 || (CommandParameter & (~TpmHashAlgorithmBitmap)) != 0) {
+        DEBUG((DEBUG_ERROR, "PCR banks %x to allocate are not supported by TPM. Skip operation\n", CommandParameter));
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      }
+
+      Status = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap, CommandParameter);
+      if (EFI_ERROR (Status)) {
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      } else {
+        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
+      }
+
+    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
+      Status = Tpm2CommandChangeEps (PlatformAuth);
+      if (EFI_ERROR (Status)) {
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      } else {
+        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
+      }
+
+    case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
+      Status = Tpm2GetCapabilitySupportedAndActivePcrs (&TpmHashAlgorithmBitmap, &ActivePcrBanks);
+      ASSERT_EFI_ERROR (Status);
+      Status = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap, TpmHashAlgorithmBitmap);
+      if (EFI_ERROR (Status)) {
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      } else {
+        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
+      }
+
+    default:
+      if (CommandCode <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
+        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
+      } else {
+        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+      }
+  }
+}
+
+
+/**
+  Read the specified key for user confirmation.
+
+  @param[in]  CautionKey  If true,  F12 is used as confirm key;
+                          If false, F10 is used as confirm key.
+
+  @retval     TRUE        User confirmed the changes by input.
+  @retval     FALSE       User discarded the changes.
+**/
+BOOLEAN
+Tcg2ReadUserKey (
+  IN     BOOLEAN                    CautionKey
+  )
+{
+  EFI_STATUS                        Status;
+  EFI_INPUT_KEY                     Key;
+  UINT16                            InputKey;
+
+  InputKey = 0;
+  do {
+    Status = gBS->CheckEvent (gST->ConIn->WaitForKey);
+    if (!EFI_ERROR (Status)) {
+      Status = gST->ConIn->ReadKeyStroke (gST->ConIn, &Key);
+      if (Key.ScanCode == SCAN_ESC) {
+        InputKey = Key.ScanCode;
+      }
+      if ((Key.ScanCode == SCAN_F10) && !CautionKey) {
+        InputKey = Key.ScanCode;
+      }
+      if ((Key.ScanCode == SCAN_F12) && CautionKey) {
+        InputKey = Key.ScanCode;
+      }
+    }
+  } while (InputKey == 0);
+
+  if (InputKey != SCAN_ESC) {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+
+/**
+  Fill Buffer With BootHashAlg.
+
+  @param[in] Buffer               Buffer to be filled.
+  @param[in] BufferSize           Size of buffer.
+  @param[in] BootHashAlg          BootHashAlg.
+
+**/
+VOID
+Tcg2FillBufferWithBootHashAlg (
+  IN UINT16  *Buffer,
+  IN UINTN   BufferSize,
+  IN UINT32  BootHashAlg
+  )
+{
+  Buffer[0] = 0;
+  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA1) != 0) {
+    if (Buffer[0] != 0) {
+      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+    }
+    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA1", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+  }
+  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA256) != 0) {
+    if (Buffer[0] != 0) {
+      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+    }
+    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA256", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+  }
+  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA384) != 0) {
+    if (Buffer[0] != 0) {
+      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+    }
+    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA384", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+  }
+  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA512) != 0) {
+    if (Buffer[0] != 0) {
+      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+    }
+    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA512", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+  }
+  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SM3_256) != 0) {
+    if (Buffer[0] != 0) {
+      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+    }
+    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SM3_256", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
+  }
+}
+
+
+/**
+  Display the confirm text and get user confirmation.
+
+  @param[in] TpmPpCommand             The requested TPM physical presence command.
+  @param[in] TpmPpCommandParameter    The requested TPM physical presence command parameter.
+
+  @retval    TRUE          The user has confirmed the changes.
+  @retval    FALSE         The user doesn't confirm the changes.
+**/
+BOOLEAN
+Tcg2UserConfirm (
+  IN      UINT32                    TpmPpCommand,
+  IN      UINT32                    TpmPpCommandParameter
+  )
+{
+  CHAR16                            *ConfirmText;
+  CHAR16                            *TmpStr1;
+  CHAR16                            *TmpStr2;
+  UINTN                             BufSize;
+  BOOLEAN                           CautionKey;
+  BOOLEAN                           NoPpiInfo;
+  UINT16                            Index;
+  CHAR16                            DstStr[81];
+  CHAR16                            TempBuffer[1024];
+  CHAR16                            TempBuffer2[1024];
+  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
+  EFI_TCG2_BOOT_SERVICE_CAPABILITY  ProtocolCapability;
+  UINT32                            CurrentPCRBanks;
+  EFI_STATUS                        Status;
+
+  TmpStr2     = NULL;
+  CautionKey  = FALSE;
+  NoPpiInfo   = FALSE;
+  BufSize     = CONFIRM_BUFFER_SIZE;
+  ConfirmText = AllocateZeroPool (BufSize);
+  ASSERT (ConfirmText != NULL);
+
+  mTcg2PpStringPackHandle = HiiAddPackages (&gEfiTcg2PhysicalPresenceGuid, gImageHandle, DxeTcg2PhysicalPresenceLibStrings, NULL);
+  ASSERT (mTcg2PpStringPackHandle != NULL);
+
+  switch (TpmPpCommand) {
+
+    case TCG2_PHYSICAL_PRESENCE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
+      CautionKey = TRUE;
+      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CLEAR));
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
+      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
+      FreePool (TmpStr1);
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CLEAR));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), L" \n\n", (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      break;
+
+    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
+      Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
+      ASSERT_EFI_ERROR (Status);
+
+      ProtocolCapability.Size = sizeof(ProtocolCapability);
+      Status = Tcg2Protocol->GetCapability (
+                               Tcg2Protocol,
+                               &ProtocolCapability
+                               );
+      ASSERT_EFI_ERROR (Status);
+
+      Status = Tcg2Protocol->GetActivePcrBanks (
+                               Tcg2Protocol,
+                               &CurrentPCRBanks
+                               );
+      ASSERT_EFI_ERROR (Status);
+
+      CautionKey = TRUE;
+      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_SET_PCR_BANKS));
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
+      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
+      FreePool (TmpStr1);
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_SET_PCR_BANKS_1));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_SET_PCR_BANKS_2));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      Tcg2FillBufferWithBootHashAlg (TempBuffer, sizeof(TempBuffer), TpmPpCommandParameter);
+      Tcg2FillBufferWithBootHashAlg (TempBuffer2, sizeof(TempBuffer2), CurrentPCRBanks);
+
+      TmpStr1 = AllocateZeroPool (BufSize);
+      ASSERT (TmpStr1 != NULL);
+      UnicodeSPrint (TmpStr1, BufSize, L"Current PCRBanks is 0x%x. (%s)\nNew PCRBanks is 0x%x. (%s)\n", CurrentPCRBanks, TempBuffer2, TpmPpCommandParameter, TempBuffer);
+
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), L" \n", (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      break;
+
+    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
+      CautionKey = TRUE;
+      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CHANGE_EPS));
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
+      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
+      FreePool (TmpStr1);
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CHANGE_EPS_1));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CHANGE_EPS_2));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+
+      break;
+
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
+      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_ENABLE_BLOCK_SID));
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_HEAD_STR));
+      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
+      FreePool (TmpStr1);
+      break;
+
+    case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
+      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_DISABLE_BLOCK_SID));
+
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_HEAD_STR));
+      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
+      FreePool (TmpStr1);
+      break;
+
+    default:
+      ;
+  }
+
+  if (TmpStr2 == NULL) {
+    FreePool (ConfirmText);
+    return FALSE;
+  }
+
+  if (TpmPpCommand < TCG2_PHYSICAL_PRESENCE_STORAGE_MANAGEMENT_BEGIN) {
+    if (CautionKey) {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CAUTION_KEY));
+    } else {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_ACCEPT_KEY));
+    }
+    StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+    FreePool (TmpStr1);
+
+    if (NoPpiInfo) {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_NO_PPI_INFO));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+    }
+
+    TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_REJECT_KEY));
+  } else {
+    if (CautionKey) {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_CAUTION_KEY));
+    } else {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_ACCEPT_KEY));
+    }
+    StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+    FreePool (TmpStr1);
+
+    if (NoPpiInfo) {
+      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_NO_PPI_INFO));
+      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
+      FreePool (TmpStr1);
+    }
+
+    TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_REJECT_KEY));
+  }
+  BufSize -= StrSize (ConfirmText);
+  UnicodeSPrint (ConfirmText + StrLen (ConfirmText), BufSize, TmpStr1, TmpStr2);
+
+  DstStr[80] = L'\0';
+  for (Index = 0; Index < StrLen (ConfirmText); Index += 80) {
+    StrnCpyS (DstStr, sizeof (DstStr) / sizeof (CHAR16), ConfirmText + Index, sizeof (DstStr) / sizeof (CHAR16) - 1);
+    Print (DstStr);
+  }
+
+  FreePool (TmpStr1);
+  FreePool (TmpStr2);
+  FreePool (ConfirmText);
+  HiiRemovePackages (mTcg2PpStringPackHandle);
+
+  if (Tcg2ReadUserKey (CautionKey)) {
+    return TRUE;
+  }
+
+  return FALSE;
+}
+
+
+/**
+  Check if there is a valid physical presence command request. Also updates parameter value
+  to whether the requested physical presence command already confirmed by user
+
+   @param[out] RequestConfirmed          If the physical presence operation command required user confirm from UI.
+                                           True, it indicates the command doesn't require user confirm, or already confirmed
+                                                 in last boot cycle by user.
+                                           False, it indicates the command need user confirm from UI.
+
+   @retval  TRUE        Physical Presence operation command is valid.
+   @retval  FALSE       Physical Presence operation command is invalid.
+
+**/
+BOOLEAN
+Tcg2HaveValidTpmRequest  (
+  OUT     BOOLEAN                          *RequestConfirmed
+  )
+{
+  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
+  EFI_STATUS                        Status;
+
+  *RequestConfirmed = FALSE;
+
+  if (mPpi->Request <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
+    //
+    // Need TCG2 protocol.
+    //
+    Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
+    if (EFI_ERROR (Status)) {
+      return FALSE;
+    }
+  }
+
+  switch (mPpi->Request) {
+    case TCG2_PHYSICAL_PRESENCE_NO_ACTION:
+    case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
+      *RequestConfirmed = TRUE;
+      return TRUE;
+
+    case TCG2_PHYSICAL_PRESENCE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
+    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
+    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
+    case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
+    case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
+      break;
+
+    default:
+      //
+      // Wrong Physical Presence command
+      //
+      return FALSE;
+  }
+
+  //
+  // Physical Presence command is correct
+  //
+  return TRUE;
+}
+
+
+/**
+  Check and execute the requested physical presence command.
+
+  @param[in]      PlatformAuth      platform auth value. NULL means no platform auth change.
+**/
+VOID
+Tcg2ExecutePendingTpmRequest (
+  IN      TPM2B_AUTH                       *PlatformAuth OPTIONAL
+  )
+{
+  BOOLEAN                           RequestConfirmed;
+
+  if (mPpi->Request == TCG2_PHYSICAL_PRESENCE_NO_ACTION) {
+    //
+    // No operation request
+    //
+    return;
+  }
+
+  if (!Tcg2HaveValidTpmRequest (&RequestConfirmed)) {
+    //
+    // Invalid operation request.
+    //
+    if (mPpi->Request <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
+      mPpi->Response = TCG_PP_OPERATION_RESPONSE_SUCCESS;
+    } else {
+      mPpi->Response = TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
+    }
+    mPpi->LastRequest = mPpi->Request;
+    mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
+    mPpi->RequestParameter = 0;
+    return;
+  }
+
+  if (!RequestConfirmed) {
+    //
+    // Print confirm text and wait for approval.
+    //
+    RequestConfirmed = Tcg2UserConfirm (mPpi->Request, mPpi->RequestParameter);
+  }
+
+  //
+  // Execute requested physical presence command
+  //
+  mPpi->Response = TCG_PP_OPERATION_RESPONSE_USER_ABORT;
+  if (RequestConfirmed) {
+    mPpi->Response = Tcg2ExecutePhysicalPresence (
+                                                  PlatformAuth,
+                                                  mPpi->Request,
+                                                  mPpi->RequestParameter
+                                                  );
+  }
+
+  //
+  // Clear request
+  //
+  mPpi->LastRequest = mPpi->Request;
+  mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
+  mPpi->RequestParameter = 0;
+
+  if (mPpi->Response == TCG_PP_OPERATION_RESPONSE_USER_ABORT) {
+    return;
+  }
+
+  //
+  // Reset system to make new TPM settings in effect
+  //
+  switch (mPpi->LastRequest) {
+  case TCG2_PHYSICAL_PRESENCE_CLEAR:
+  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
+  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
+  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
+  case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
+  case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
+  case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
+    break;
+
+  case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
+  case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
+    break;
+
+  default:
+    if (mPpi->Request != TCG2_PHYSICAL_PRESENCE_NO_ACTION) {
+      break;
+    }
+    return;
+  }
+
+  Print (L"Rebooting system to make TPM2 settings in effect\n");
+  gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
+  ASSERT (FALSE);
+}
+
+
+/**
+   Check and execute the pending TPM request.
+
+   The TPM request may come from OS or BIOS. This API will display request information and wait
+   for user confirmation if TPM request exists. The TPM request will be sent to TPM device after
+   the TPM request is confirmed, and one or more reset may be required to make TPM request to
+   take effect.
+
+   This API should be invoked after console in and console out are all ready as they are required
+   to display request information and get user input to confirm the request.
+
+   @param[in]  PlatformAuth                   platform auth value. NULL means no platform auth change.
+**/
+VOID
+EFIAPI
+Tcg2PhysicalPresenceLibProcessRequest (
+  IN      TPM2B_AUTH                     *PlatformAuth  OPTIONAL
+  )
+{
+  EFI_STATUS Status;
+
+  Status = QemuTpmInitPPI ();
+  if (EFI_ERROR (Status)) {
+    DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
+    return ;
+  }
+
+  //
+  // Check S4 resume
+  //
+  if (GetBootModeHob () == BOOT_ON_S4_RESUME) {
+    DEBUG ((EFI_D_INFO, "S4 Resume, Skip TPM PP process!\n"));
+    return ;
+  }
+
+  DEBUG ((EFI_D_INFO, "[TPM2PP] PPRequest=%x (PPRequestParameter=%x)\n", mPpi->Request, mPpi->RequestParameter));
+  Tcg2ExecutePendingTpmRequest (PlatformAuth);
+}
+
+
+/**
+  The handler for TPM physical presence function:
+  Return TPM Operation Response to OS Environment.
+
+  @param[out]     MostRecentRequest Most recent operation request.
+  @param[out]     Response          Response to the most recent operation request.
+
+  @return Return Code for Return TPM Operation Response to OS Environment.
+**/
+UINT32
+EFIAPI
+Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction (
+  OUT UINT32                *MostRecentRequest,
+  OUT UINT32                *Response
+  )
+{
+  EFI_STATUS Status;
+
+  DEBUG ((EFI_D_INFO, "[TPM2PP] ReturnOperationResponseToOsFunction\n"));
+
+  Status = QemuTpmInitPPI ();
+  if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
+      *MostRecentRequest = 0;
+      *Response          = 0;
+      return TCG_PP_RETURN_TPM_OPERATION_RESPONSE_FAILURE;
+  }
+
+  *MostRecentRequest = mPpi->LastRequest;
+  *Response          = mPpi->Response;
+
+  return TCG_PP_RETURN_TPM_OPERATION_RESPONSE_SUCCESS;
+}
+
+
+/**
+  The handler for TPM physical presence function:
+  Submit TPM Operation Request to Pre-OS Environment and
+  Submit TPM Operation Request to Pre-OS Environment 2.
+
+  Caution: This function may receive untrusted input.
+
+  @param[in]      OperationRequest TPM physical presence operation request.
+  @param[in]      RequestParameter TPM physical presence operation request parameter.
+
+  @return Return Code for Submit TPM Operation Request to Pre-OS Environment and
+          Submit TPM Operation Request to Pre-OS Environment 2.
+**/
+UINT32
+EFIAPI
+Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
+  IN UINT32                 OperationRequest,
+  IN UINT32                 RequestParameter
+  )
+{
+  EFI_STATUS Status;
+
+  DEBUG ((EFI_D_INFO, "[TPM2PP] SubmitRequestToPreOSFunction, Request = %x, %x\n", OperationRequest, RequestParameter));
+
+  Status = QemuTpmInitPPI ();
+  if (EFI_ERROR (Status)) {
+      DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
+      return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
+  }
+
+  mPpi->Request = OperationRequest;
+  mPpi->RequestParameter = RequestParameter;
+
+  return TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS;
+}
diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
new file mode 100644
index 000000000000..6b2d70c711fe
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
@@ -0,0 +1,67 @@
+## @file
+#  Executes TPM 2.0 requests from OS or BIOS
+#
+#  This library will check and execute TPM 2.0 request from OS or BIOS. The request may
+#  ask for user confirmation before execution.
+#
+#  Caution: This module requires additional review when modified.
+#  This driver will have external input - variable.
+#  This external input must be validated carefully to avoid security issue.
+#
+# Copyright (C) 2018, Red Hat, Inc.
+# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
+# This program and the accompanying materials
+# are licensed and made available under the terms and conditions of the BSD License
+# which accompanies this distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+#
+##
+
+[Defines]
+  INF_VERSION                    = 0x00010005
+  BASE_NAME                      = DxeTcg2PhysicalPresenceLib
+  MODULE_UNI_FILE                = DxeTcg2PhysicalPresenceLib.uni
+  FILE_GUID                      = 41D3E698-9EEC-41FF-9CBB-5FE79A0CF326
+  MODULE_TYPE                    = DXE_DRIVER
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = Tcg2PhysicalPresenceLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER UEFI_APPLICATION UEFI_DRIVER
+
+#
+# The following information is for reference only and not required by the build tools.
+#
+#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
+#
+
+[Sources]
+  DxeTcg2PhysicalPresenceLib.c
+  PhysicalPresenceStrings.uni
+
+[Packages]
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  SecurityPkg/SecurityPkg.dec
+
+[LibraryClasses]
+  BaseMemoryLib
+  DebugLib
+  HiiLib
+  HobLib
+  MemoryAllocationLib
+  PrintLib
+  QemuFwCfgLib
+  Tpm2CommandLib
+  UefiBootServicesTableLib
+  UefiLib
+  UefiRuntimeServicesTableLib
+
+[Protocols]
+  gEfiTcg2ProtocolGuid                 ## SOMETIMES_CONSUMES
+
+[Guids]
+  ## SOMETIMES_CONSUMES ## HII
+  ## SOMETIMES_PRODUCES ## Variable:L"Tcg2PhysicalPresence"
+  ## SOMETIMES_CONSUMES ## Variable:L"Tcg2PhysicalPresence"
+  gEfiTcg2PhysicalPresenceGuid
diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
new file mode 100644
index 000000000000..aaae8f5014e7
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
@@ -0,0 +1,26 @@
+// /** @file
+// Executes TPM 2.0 requests from OS or BIOS
+//
+// This library will check and execute TPM 2.0 request from OS or BIOS. The request may
+// ask for user confirmation before execution.
+//
+// Caution: This module requires additional review when modified.
+// This driver will have external input - variable.
+// This external input must be validated carefully to avoid security issue.
+//
+// Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+//
+// This program and the accompanying materials
+// are licensed and made available under the terms and conditions of the BSD License
+// which accompanies this distribution. The full text of the license may be found at
+// http://opensource.org/licenses/bsd-license.php
+// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+//
+// **/
+
+
+#string STR_MODULE_ABSTRACT             #language en-US "Executes TPM 2.0 requests from OS or BIOS"
+
+#string STR_MODULE_DESCRIPTION          #language en-US "This library will check and execute TPM 2.0 request from OS or BIOS. The request may ask for user confirmation before execution.\n"
+                                                        "Caution: This module requires additional review when modified. This driver will have external input - variable. This external input must be validated carefully to avoid security issue."
diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni
new file mode 100644
index 000000000000..1470286b4c3b
--- /dev/null
+++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni
@@ -0,0 +1,49 @@
+/** @file
+  String definitions for TPM 2.0 physical presence confirm text.
+
+Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
+This program and the accompanying materials
+are licensed and made available under the terms and conditions of the BSD License
+which accompanies this distribution.  The full text of the license may be found at
+http://opensource.org/licenses/bsd-license.php
+
+THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
+WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+
+**/
+
+#langdef en-US "English"
+
+#string TPM_HEAD_STR                  #language en-US    "A configuration change was requested to %s this computer's TPM (Trusted Platform Module)\n\n"
+
+#string TPM_ACCEPT_KEY                #language en-US    "Press F10 "
+#string TPM_CAUTION_KEY               #language en-US    "Press F12 "
+#string TPM_REJECT_KEY                #language en-US    "to %s the TPM \nPress ESC to reject this change request and continue\n"
+
+#string TPM_ENABLE                    #language en-US    "enable"
+#string TPM_DISABLE                   #language en-US    "disable"
+#string TPM_CLEAR                     #language en-US    "clear"
+#string TPM_SET_PCR_BANKS                       #language en-US    "change the boot measurements to use PCR bank(s) of"
+#string TPM_CHANGE_EPS                          #language en-US    "clear and change identity of"
+
+#string TPM_NO_PPI_MAINTAIN           #language en-US    "maintain"
+#string TPM_NO_PPI_TURN_ON            #language en-US    "turn on"
+#string TPM_NO_PPI_TURN_OFF           #language en-US    "turn off"
+#string TPM_NO_PPI_INFO               #language en-US    "to approve future Operating System requests "
+
+#string TPM_WARNING_CLEAR             #language en-US    "WARNING: Clearing erases information stored on the TPM. You will lose all created keys and access to data encrypted by these keys. "
+#string TPM_WARNING_SET_PCR_BANKS_1                     #language en-US    "WARNING: Changing the PCR bank(s) of the boot measurements may prevent the Operating System from properly processing the measurements. Please check if your Operating System supports the new PCR bank(s).\n\n"
+#string TPM_WARNING_SET_PCR_BANKS_2                     #language en-US    "WARNING: Secrets in the TPM that are bound to the boot state of your machine may become unusable.\n\n"
+#string TPM_WARNING_CHANGE_EPS_1                        #language en-US    "WARNING: Clearing erases information stored on the TPM. You will lose all created keys and access to data encrypted with these keys.\n\n"
+#string TPM_WARNING_CHANGE_EPS_2                        #language en-US    "WARNING: Changing the identity of the TPM may require additional steps to establish trust into the new identity.\n\n"
+
+#string TCG_STORAGE_HEAD_STR                  #language en-US    "A configuration change was requested to %s on subsequent boots\n\n"
+
+#string TCG_STORAGE_ACCEPT_KEY                #language en-US    "Press F10 "
+#string TCG_STORAGE_CAUTION_KEY               #language en-US    "Press F12 "
+#string TCG_STORAGE_REJECT_KEY                #language en-US    "to %s\nPress ESC to reject this change request and continue\n"
+
+#string TCG_STORAGE_NO_PPI_INFO               #language en-US    "to approve future Operating System requests "
+
+#string TCG_STORAGE_ENABLE_BLOCK_SID          #language en-US    "issue a Block SID authentication command"
+#string TCG_STORAGE_DISABLE_BLOCK_SID         #language en-US    "disable issuing a Block SID authentication command"
diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
index 6c361b73cd55..251434a9ff7c 100644
--- a/OvmfPkg/OvmfPkgIa32.dsc
+++ b/OvmfPkg/OvmfPkgIa32.dsc
@@ -206,7 +206,7 @@ [LibraryClasses]
 
 !if $(TPM2_ENABLE) == TRUE
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
-  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
 !else
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
index 62a6075a671d..ce247a59d61a 100644
--- a/OvmfPkg/OvmfPkgIa32X64.dsc
+++ b/OvmfPkg/OvmfPkgIa32X64.dsc
@@ -211,7 +211,7 @@ [LibraryClasses]
 
 !if $(TPM2_ENABLE) == TRUE
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
-  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
 !else
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
index cbab1aa328c6..67f7e155ee3e 100644
--- a/OvmfPkg/OvmfPkgX64.dsc
+++ b/OvmfPkg/OvmfPkgX64.dsc
@@ -211,7 +211,7 @@ [LibraryClasses]
 
 !if $(TPM2_ENABLE) == TRUE
   Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
-  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
   Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
 !else
   Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
-- 
2.17.0.253.g3dd125b46d

[Qemu-devel] [PATCH 4/4] ovmf: process TPM PPI request in AfterConsole()

[marcandre.lureau]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Call Tcg2PhysicalPresenceLibProcessRequest() to process pending PPI
requests from PlatformBootManagerAfterConsole().

Laszlo understanding of edk2 is that the PPI operation processing was
meant to occur *entirely* before End-Of-Dxe, so that 3rd party UEFI
drivers couldn't interfere with PPI opcode processing *at all*.

He suggested that we should *not* call
Tcg2PhysicalPresenceLibProcessRequest() from BeforeConsole(). Because,
an "auth" console, i.e. one that does not depend on a 3rd party
driver, is *in general* impossible to guarantee. Instead we could opt
to trust 3rd party drivers, and use the "normal" console(s) in
AfterConsole(), in order to let the user confirm the PPI requests. It
will depend on the user to enable Secure Boot, so that the
trustworthiness of those 3rd party drivers is ensured. If an attacker
roots the guest OS from within, queues some TPM2 PPI requests, and
also modifies drivers on the EFI system partition and/or in GPU option
ROMs (?), then those drivers will not load after guest reboot, and
thus the dependent console(s) won't be used for confirming the PPI
requests.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c      | 8 ++++++++
 .../PlatformBootManagerLib/PlatformBootManagerLib.inf     | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
index 004b753f4d26..8b1beaa3e207 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
+++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
@@ -16,6 +16,7 @@
 #include <Guid/XenInfo.h>
 #include <Guid/RootBridgesConnectedEventGroup.h>
 #include <Protocol/FirmwareVolume2.h>
+#include <Library/Tcg2PhysicalPresenceLib.h>
 
 
 //
@@ -1410,6 +1411,13 @@ PlatformBootManagerAfterConsole (
   //
   PciAcpiInitialization ();
 
+
+  //
+  // Process TPM PPI request
+  //
+  Tcg2PhysicalPresenceLibProcessRequest (NULL);
+
+
   //
   // Process QEMU's -kernel command line option
   //
diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
index 27789b7377bc..4b72c44bcf0a 100644
--- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
+++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
@@ -38,6 +38,7 @@ [Packages]
   IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
   SourceLevelDebugPkg/SourceLevelDebugPkg.dec
   OvmfPkg/OvmfPkg.dec
+  SecurityPkg/SecurityPkg.dec
 
 [LibraryClasses]
   BaseLib
@@ -56,6 +57,7 @@ [LibraryClasses]
   LoadLinuxLib
   QemuBootOrderLib
   UefiLib
+  Tcg2PhysicalPresenceLib
 
 [Pcd]
   gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent
-- 
2.17.0.253.g3dd125b46d

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Laszlo Ersek]

Hi Marc-André,

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Hi,
>
> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
> with TPM2 (I haven't tested TPM1, for lack of interest).

I got the review of this patch series added to my TODO list, but I'll
have to ask for your patience. :(

>From an extremely superficial skim:

* please use the

    TopDirPkg/ModuleName: blah blah blah

  subject format, or more generally, if a module cannot be identified,

    TopDirPkg: blah blah blah

* the subject line and the commit message shouldn't be wider than 74
  chars;

* edk2 uses two spaces for general indentation, and I'm noticing some
  inconsistency there (4 spaces like in QEMU).

* Please consider formatting the patches with "--find-copies-harder"
  (although I can look at them with the same option after fetching the
  series from your repo). This option is usually helpful for reviewers
  when cloning and modifying modules cross-package.

* Please consider adopting the git settings at
  <https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers>,
  in particular:

  - "--stat=1000 --stat-graph-width=20", so that pathnames are not
    truncated in the diffstats,

  - the "xfuncname"-related settings, so that git diff hunk headers @@
    are useful for DSC and INF files too,

  - the diff order file, so that files are listed in patches in logical
    order, going from abstract / descriptive (.inf, .h) to concrete /
    imperative (.c).

Not much of a review, I know; this is all I can offer right now. If you
have the time to respin just with these superficial changes, that might
make my life easier. If you prefer to delay them, that's 100% fine too.

Thanks
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Laszlo Ersek]

On 05/16/18 11:29, Laszlo Ersek wrote:
> Hi Marc-André,
> 
> On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
>> with TPM2 (I haven't tested TPM1, for lack of interest).

Here's another general request: please make sure that all code you add
(new lines in existent files, and especially brand new files) use CRLF
line terminators.

Thanks!
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Laszlo Ersek]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Hi,
> 
> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
> with TPM2 (I haven't tested TPM1, for lack of interest).
> 
> PPI test runs successfully with Windows 10 WHLK, despite the limited
> number of supported funcions (tpm2_ppi_funcs table, in particular, no
> function allows to manipulate Tcg2PhysicalPresenceFlags)
> 
> The way it works is relatively simple: a memory region is allocated by
> QEMU to save PPI related variables. An ACPI interface is exposed by
> QEMU to let the guest manipulate those. At boot, ovmf processes and
> updates the PPI qemu region and request variables.
> 
> I build edk2 with:
> 
> $ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE

Is -DSECURE_BOOT_ENABLE necessary for *building* with -DTPM2_ENABLE? If
that's the case, we should update the DSC files; users building OVMF
from source shouldn't have to care about "-D" inter-dependencies, if we
can manage that somehow.

If -DSECURE_BOOT_ENABLE is only there because otherwise a guest OS
doesn't really make use of -DTPM2_ENABLE either, that's different. In
that case, it's fine to allow building OVMF with TPM2 support but
without SB support.

Thanks!
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 1/4] ovmf: add and link with Tcg2PhysicalPresenceLibNull when !TPM2_ENABLE

[Laszlo Ersek]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> This NULL library will let us call
> Tcg2PhysicalPresenceLibProcessRequest() unconditionally from
> BdsPlatform when building without TPM2_ENABLE.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  .../DxeTcg2PhysicalPresenceLib.c              | 26 ++++++++++++++
>  .../DxeTcg2PhysicalPresenceLib.inf            | 34 +++++++++++++++++++
>  OvmfPkg/OvmfPkgIa32.dsc                       |  2 ++
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |  2 ++
>  OvmfPkg/OvmfPkgX64.dsc                        |  2 ++
>  5 files changed, 66 insertions(+)
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> 
> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
> new file mode 100644
> index 000000000000..0b8b98410315
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.c
> @@ -0,0 +1,26 @@
> +/** @file
> +  NULL Tcg2PhysicalPresenceLib library instance
> +
> +  Copyright (c) 2018, Red Hat, Inc.
> +  Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +  This program and the accompanying materials
> +  are licensed and made available under the terms and conditions of the BSD License
> +  which accompanies this distribution.  The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +  WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include "PiDxe.h"

(1) Can you drop this #include?

> +#include <Library/Tcg2PhysicalPresenceLib.h>
> +
> +VOID
> +EFIAPI
> +Tcg2PhysicalPresenceLibProcessRequest (
> +  IN      TPM2B_AUTH                     *PlatformAuth  OPTIONAL
> +  )
> +{
> +    return;
> +}

(2) Indentation.

Better yet: please replace the "return" statement with a comment:

  //
  // do nothing
  //

> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> new file mode 100644
> index 000000000000..e6f6239e1e00
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> @@ -0,0 +1,34 @@
> +## @file
> +#  NULL Tcg2PhysicalPresenceLib library instance
> +#
> +#  In SecurityPkg, this library will check and execute TPM 1.2 request
> +#  from OS or BIOS. The request may ask for user confirmation before
> +#  execution. This Library will also lock TPM physical presence at
> +#  last.

(3) The approach on this comment is generally OK, but the specific text
originates from
"SecurityPkg/Library/DxeTcgPhysicalPresenceLib/DxeTcgPhysicalPresenceLib.inf".
I think we should update the comment from the TPM2 variant, namely
"SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf".

Thus, I suggest the following comment:

"Under SecurityPkg, the corresponding library instance will check and
execute TPM 2.0 request from OS or BIOS; the request may ask for user
confirmation before execution. This Null instance implements a no-op
Tcg2PhysicalPresenceLibProcessRequest(), without user interaction."

> +#
> +# Copyright (C) 2018, Red Hat, Inc.
> +# Copyright (c) 2009 - 2015, Intel Corporation. All rights reserved.<BR>

(4) Same comment applies to the Intel copyright notice: from the TCG2
variant, this should come as

"Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>"

> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions of the BSD License
> +# which accompanies this distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = DxeTcg2PhysicalPresenceLibNull
> +  FILE_GUID                      = 2A6BA243-DC22-42D8-9C3D-AE3728DC7AFA
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = Tcg2PhysicalPresenceLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +
> +[Sources]
> +  DxeTcg2PhysicalPresenceLib.c
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec

(5) I think you can drop "MdeModulePkg/MdeModulePkg.dec". (MdePkg.dec is
needed by all modules, and SecurityPkg.dec below is needed for the lib
class header; so those are OK).

> +  SecurityPkg/SecurityPkg.dec
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 2d6c4c4615b6..6c361b73cd55 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -208,6 +208,8 @@ [LibraryClasses]
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
>    Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> +!else
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>  !endif
>  
>  [LibraryClasses.common]
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 43158c5f0627..62a6075a671d 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -213,6 +213,8 @@ [LibraryClasses]
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
>    Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> +!else
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>  !endif
>  
>  [LibraryClasses.common]
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index d1fdf7c307c2..cbab1aa328c6 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -213,6 +213,8 @@ [LibraryClasses]
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
>    Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> +!else
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
>  !endif
>  
>  [LibraryClasses.common]
> 

Thanks!
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 2/4] ovmf: add QemuTpm.h header

[Laszlo Ersek]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Add some common macros and type definitions corresponding to the QEMU
> TPM interface.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  OvmfPkg/Include/IndustryStandard/QemuTpm.h | 67 ++++++++++++++++++++++
>  1 file changed, 67 insertions(+)
>  create mode 100644 OvmfPkg/Include/IndustryStandard/QemuTpm.h
> 
> diff --git a/OvmfPkg/Include/IndustryStandard/QemuTpm.h b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
> new file mode 100644
> index 000000000000..054cf79374b5
> --- /dev/null
> +++ b/OvmfPkg/Include/IndustryStandard/QemuTpm.h
> @@ -0,0 +1,67 @@
> +/** @file
> +  Macro and type definitions corresponding to the QEMU TPM interface.
> +
> +  Refer to "docs/specs/tpm.txt" in the QEMU source directory.
> +
> +  Copyright (C) 2018, Red Hat, Inc.
> +  Copyright (c) 2018, IBM Corporation. All rights reserved.<BR>
> +
> +  This program and the accompanying materials are licensed and made available
> +  under the terms and conditions of the BSD License which accompanies this
> +  distribution. The full text of the license may be found at
> +  http://opensource.org/licenses/bsd-license.php
> +
> +  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
> +  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +**/
> +
> +#ifndef __QEMU_TPM_H__
> +#define __QEMU_TPM_H__
> +
> +#include <Base.h>
> +
> +/* whether function is blocked by BIOS settings; bits 0, 1, 2 */
> +#define QEMU_TPM_PPI_FUNC_NOT_IMPLEMENTED     (0 << 0)
> +#define QEMU_TPM_PPI_FUNC_BIOS_ONLY           (1 << 0)
> +#define QEMU_TPM_PPI_FUNC_BLOCKED             (2 << 0)
> +#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_REQ     (3 << 0)
> +#define QEMU_TPM_PPI_FUNC_ALLOWED_USR_NOT_REQ (4 << 0)
> +#define QEMU_TPM_PPI_FUNC_MASK                (7 << 0)
> +
> +//
> +// The following structure is shared between firmware and ACPI.
> +//
> +#pragma pack (1)
> +typedef struct {
> +  UINT8 Func[256];           /* func */
> +  UINT8 In;                  /* ppin */
> +  UINT32 Ip;                 /* ppip */
> +  UINT32 Response;           /* pprp */
> +  UINT32 Request;            /* pprq */
> +  UINT32 RequestParameter;   /* pprm */
> +  UINT32 LastRequest;        /* lppr */
> +  UINT32 FRet;               /* fret */
> +  UINT8 Res1[0x40];          /* res1 */
> +  UINT8 NextStep;            /* next_step */
> +} QEMU_TPM_PPI;
> +#pragma pack ()
> +
> +//
> +// The following structure is for the fw_cfg etc/tpm/config file.
> +//
> +#pragma pack (1)
> +typedef struct {
> +  UINT32 PpiAddress;
> +  UINT8 TpmVersion;
> +  UINT8 PpiVersion;
> +} QEMU_FWCFG_TPM_CONFIG;
> +#pragma pack ()
> +
> +#define QEMU_TPM_VERSION_UNSPEC    0
> +#define QEMU_TPM_VERSION_1_2       1
> +#define QEMU_TPM_VERSION_2         2
> +
> +#define QEMU_TPM_PPI_VERSION_NONE  0
> +#define QEMU_TPM_PPI_VERSION_1_30  1
> +
> +#endif
> 

(1) Please update the subject line as discussed earlier; for example:

OvmfPkg/IndustryStandard: add QemuTpm.h header

(2) Please convert the file to CRLF.

(3) Please use the "// ..." comment style near the fields of QEMU_TPM_PPI.

(4) Please align the member identifiers in each of QEMU_TPM_PPI and
QEMU_FWCFG_TPM_CONFIG -- in practice this means inserting another space
char after each "UINT8" type name.

With those changes:

Acked-by: Laszlo Ersek <lersek@redhat.com>

Thanks!
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Laszlo Ersek]

On 05/17/18 09:54, Laszlo Ersek wrote:
> On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
>> with TPM2 (I haven't tested TPM1, for lack of interest).
>>
>> PPI test runs successfully with Windows 10 WHLK, despite the limited
>> number of supported funcions (tpm2_ppi_funcs table, in particular, no
>> function allows to manipulate Tcg2PhysicalPresenceFlags)
>>
>> The way it works is relatively simple: a memory region is allocated by
>> QEMU to save PPI related variables. An ACPI interface is exposed by
>> QEMU to let the guest manipulate those. At boot, ovmf processes and
>> updates the PPI qemu region and request variables.
>>
>> I build edk2 with:
>>
>> $ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE
> 
> Is -DSECURE_BOOT_ENABLE necessary for *building* with -DTPM2_ENABLE? If
> that's the case, we should update the DSC files; users building OVMF
> from source shouldn't have to care about "-D" inter-dependencies, if we
> can manage that somehow.
> 
> If -DSECURE_BOOT_ENABLE is only there because otherwise a guest OS
> doesn't really make use of -DTPM2_ENABLE either, that's different. In
> that case, it's fine to allow building OVMF with TPM2 support but
> without SB support.

Oops, almost missed another important omission: in every commit message,
please insert the following line just above your S-o-b:

Contributed-under: TianoCore Contribution Agreement 1.1

We cannot take patches without that line. You can read about it in the
"Contributions.txt" file, in the project root directory.

Thanks!
Laszlo

Re: [Qemu-devel] [edk2] [PATCH 3/4] ovmf: replace SecurityPkg with OvfmPkg Tcg2PhysicalPresenceLibQemu

[Laszlo Ersek]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Cloned "SecurityPkg/Library/DxeTcg2PhysicalPresenceLib" and:
> 
> - removed all the functions that are unreachable from
>   Tcg2PhysicalPresenceLibProcessRequest()
> 
> - replaced everything that's related to the
>   TCG2_PHYSICAL_PRESENCE*_VARIABLE variables, with direct access to
>   the QEMU structures.
> 
> This commit is based on initial experimental work from Stefan Berger.
> In particular, he wrote most of QEMU PPI support, and designed the
> qemu/firmware interaction. Initially, Stefan tried to reuse the
> existing SecurityPkg code, but we eventually decided to get rid of the
> variables and simplify the ovmf/qemu version.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  .../DxeTcg2PhysicalPresenceLib.c              | 881 ++++++++++++++++++
>  .../DxeTcg2PhysicalPresenceLib.inf            |  67 ++
>  .../DxeTcg2PhysicalPresenceLib.uni            |  26 +

(1) Please drop the "DxeTcg2PhysicalPresenceLib.uni" file (also the
reference in the INF file).

We generally don't do MODULE_UNI_FILEs in OvmfPkg because OvmfPkg is not
distributed with UPT (UEFI Packaging Tool).

>  .../PhysicalPresenceStrings.uni               |  49 +
>  OvmfPkg/OvmfPkgIa32.dsc                       |   2 +-
>  OvmfPkg/OvmfPkgIa32X64.dsc                    |   2 +-
>  OvmfPkg/OvmfPkgX64.dsc                        |   2 +-
>  7 files changed, 1026 insertions(+), 3 deletions(-)
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
>  create mode 100644 OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni
> 
> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
> new file mode 100644
> index 000000000000..da45f990369a
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.c
> @@ -0,0 +1,881 @@
> +/** @file
> +  Execute pending TPM2 requests from OS or BIOS.
> +
> +  Caution: This module requires additional review when modified.
> +  This driver will have external input - variable.
> +  This external input must be validated carefully to avoid security issue.
> +
> +  Tcg2ExecutePendingTpmRequest() will receive untrusted input and do validation.
> +
> +Copyright (C) 2018, Red Hat, Inc.
> +Copyright (c) 2018, IBM Corporation. All rights reserved.<BR>
> +Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution.  The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#include <PiDxe.h>
> +
> +#include <Guid/Tcg2PhysicalPresenceData.h>
> +#include <IndustryStandard/QemuTpm.h>
> +#include <Protocol/Tcg2Protocol.h>
> +
> +#include <Library/BaseMemoryLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HiiLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/MemoryAllocationLib.h>
> +#include <Library/PrintLib.h>
> +#include <Library/QemuFwCfgLib.h>
> +#include <Library/Tpm2CommandLib.h>
> +#include <Library/UefiBootServicesTableLib.h>
> +#include <Library/UefiLib.h>
> +#include <Library/UefiRuntimeServicesTableLib.h>
> +
> +#include <Library/Tcg2PhysicalPresenceLib.h>
> +
> +#define CONFIRM_BUFFER_SIZE         4096
> +
> +EFI_HII_HANDLE mTcg2PpStringPackHandle;
> +
> +#define TPM_PPI_FLAGS (QEMU_TPM_PPI_FUNC_ALLOWED_USR_REQ)
> +
> +STATIC CONST UINT8 mTpm2PPIFuncs[] = {
> +  [TCG2_PHYSICAL_PRESENCE_NO_ACTION] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_CLEAR] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_CHANGE_EPS] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID] = TPM_PPI_FLAGS,
> +  [TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID] = TPM_PPI_FLAGS,
> +};

(2) Unfortunately, designated initializers cannot be used in edk2.
You'll have to

- either spell out the entire initial slice of the array (and use "//
TCG2_PHYSICAL_PRESENCE_NO_ACTION" style comments to help readers),

- or else drop the "CONST", introduce a CONSTRUCTOR function to the
library instance, and perform the assignments manually in the
constructor function, such as

  mTpm2PPIFuncs[TCG2_PHYSICAL_PRESENCE_NO_ACTION] = TPM_PPI_FLAGS;

> +
> +STATIC QEMU_TPM_PPI *mPpi;
> +
> +
> +/**
> +  Reads QEMU PPI config from fw_cfg.
> +**/
> +EFI_STATUS
> +QemuTpmReadConfig (

(3) Please make this STATIC.

Please make *all* functions STATIC that can be.

> +  IN QEMU_FWCFG_TPM_CONFIG *Config

(4) Should be decorated as OUT, not IN.

> +  )
> +{
> +  EFI_STATUS           Status;
> +  FIRMWARE_CONFIG_ITEM FwCfgItem;
> +  UINTN                FwCfgSize;
> +
> +  Status = QemuFwCfgFindFile ("etc/tpm/config", &FwCfgItem, &FwCfgSize);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  if (FwCfgSize != sizeof (*Config)) {
> +      return EFI_PROTOCOL_ERROR;
> +  }

(5) indentation (please re-check everything for that)

> +
> +  QemuFwCfgSelectItem (FwCfgItem);
> +  QemuFwCfgReadBytes (sizeof (*Config), Config);
> +  return EFI_SUCCESS;
> +}
> +
> +
> +/**
> +  Initializes QEMU PPI memory region.
> +**/
> +EFI_STATUS
> +QemuTpmInitPPI (
> +  VOID
> +  )
> +{
> +  EFI_STATUS Status;
> +  QEMU_FWCFG_TPM_CONFIG Config;

(6) we tend to align the identifiers.

> +
> +  if (mPpi) {

(7) Please spell out

  if (mPpi != NULL) {

> +      return EFI_SUCCESS;
> +  }
> +
> +  Status = QemuTpmReadConfig (&Config);
> +  if (EFI_ERROR (Status)) {
> +    return Status;
> +  }
> +
> +  mPpi = (QEMU_TPM_PPI *)(unsigned long)Config.PpiAddress;

(8) The idiomatic way to write this in edk2 is:

  mPpi = (QEMU_TPM_PPI *)(UINTN)Config.PpiAddress;

> +  if (!mPpi) {

(9) Please spell out (mPpi == NULL)

> +      return EFI_INVALID_PARAMETER;

(10) EFI_PROTOCOL_ERROR would be more idiomatic, for fw_cfg errors that
shouldn't happen.

> +  }
> +
> +  DEBUG ((EFI_D_INFO, "[TPM2PP] mPpi=%x version=%d\n", mPpi, Config.TpmVersion));

(11) We no longer use EFI_D_* macros in new code; please use DEBUG_*
instead.

(12) mPpi is a pointer, please format it with the %p conversion
specifier. Or, you can keep %x and pass Config.PpiAddress instead (which
has type UINT32).

(13) I'd like to request a safety check here, before we dereference mPpi
(into MMIO):

(13a) Please verify that QEMU_FWCFG_TPM_CONFIG does not cross a page
boundary:

  EFI_PHYSICAL_ADDRESS PpiAddress64;

  PpiAddress64 = (UINTN)mPpi;
  if ((PpiAddress64 & ~(UINT64)EFI_PAGE_MASK) !=
      ((PpiAddress64 + sizeof *mPpi - 1) & ~(UINT64)EFI_PAGE_MASK)) {
    //
    // log DEBUG_ERROR, and return an error
    //
  }

(13b) If check (a) passes, please call gDS->GetMemorySpaceDescriptor()
on PpiAddress64. (See the interface under
EFI_GET_MEMORY_SPACE_DESCRIPTOR in "MdePkg/Include/Pi/PiDxeCis.h").
Verify that one of the following holds:

- the function returns EFI_NOT_FOUND,

- or Descriptor.GcdMemoryType is either EfiGcdMemoryTypeNonExistent or
EfiGcdMemoryTypeMemoryMappedIo.

(For bells and whistles, we should even add and allocate a GCD range
with EfiGcdMemoryTypeMemoryMappedIo, but I'll spare you that.)

> +  ZeroMem (&mPpi->Func, sizeof (mPpi->Func));
> +  switch (Config.TpmVersion) {
> +  case QEMU_TPM_VERSION_2:
> +    CopyMem (&mPpi->Func, mTpm2PPIFuncs, sizeof (mTpm2PPIFuncs));
> +    break;
> +  }

(14) Can you use an "if" here please?

(15) Also, jumping back to my comment (2) here; it seems that placing
the individual assignments here would be superior to a CONSTRUCTOR
function (in case the CONSTRUCTOR function were your choice for (2)).

> +
> +  if (!mPpi->In) {
> +    mPpi->In = 1;
> +    mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
> +    mPpi->LastRequest = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
> +    mPpi->NextStep = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
> +  }

(16) A more idiomatic way for accessing all entries in *mPpi would be
calling MmioWrite8() / MmioWrite32().

The *really* idiomatic way would be to locate the EFI_CPU_IO2_PROTOCOL
instance, and use its Mem.Read() and Mem.Write() functions.

However, given that this library instance is entirely platform- and
architecture-specific, it's not lost on me that such an update would
mostly be "busywork".

Namely, for IA32/X64, EFI_CPU_IO2_PROTOCOL is implemented in
"UefiCpuPkg/CpuIo2Dxe/CpuIo2Dxe.c", which turns the protocol member
function calls into those same MmioRead*() / MmioWrite*() calls. In
turn, in OVMF, MmioRead*() / MmioWrite*() are just naked de-references
of volatile-qualified pointers, guarded by MemoryFence() calls on both
sides.

Given that
- "volatile" will prevent the compiler from reordering the accesses, and
- every single such access will trap to QEMU because *mPpi is not backed
  by guest DRAM (hence the CPU can't reorder accesses either),

I think we can safely forego the MemoryFence() calls as well, and simply
modify the definition of mPpi as follows:

STATIC volatile QEMU_TPM_PPI *mPpi;

Now, this will (rightfully) trigger a number of compilation errors, for
example because ZeroMem() and CopyMem() would require you to cast away
"volatile". I suggest replacing ZeroMem() with an open-coded loop that
zeroes the area through a (volatile UINT32 *) pointer, and the CopyMem()
should be replaced by the individual assignments anyway, such as:

  mPpi->Func[TCG2_PHYSICAL_PRESENCE_NO_ACTION] = TPM_PPI_FLAGS;

> +
> +  return EFI_SUCCESS;
> +}
> +
> +
> +/**
> +  Get string by string id from HII Interface.
> +
> +  @param[in] Id          String ID.
> +
> +  @retval    CHAR16 *    String from ID.
> +  @retval    NULL        If error occurs.
> +
> +**/
> +CHAR16 *
> +Tcg2PhysicalPresenceGetStringById (
> +  IN  EFI_STRING_ID   Id
> +  )
> +{
> +  return HiiGetString (mTcg2PpStringPackHandle, Id, NULL);
> +}
> +
> +
> +/**
> +  Send ClearControl and Clear command to TPM.
> +
> +  @param[in]  PlatformAuth      platform auth value. NULL means no platform auth change.
> +
> +  @retval EFI_SUCCESS           Operation completed successfully.
> +  @retval EFI_TIMEOUT           The register can't run into the expected status in time.
> +  @retval EFI_BUFFER_TOO_SMALL  Response data buffer is too small.
> +  @retval EFI_DEVICE_ERROR      Unexpected device behavior.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +Tpm2CommandClear (
> +  IN TPM2B_AUTH                *PlatformAuth  OPTIONAL
> +  )
> +{
> +  EFI_STATUS                Status;
> +  TPMS_AUTH_COMMAND         *AuthSession;
> +  TPMS_AUTH_COMMAND         LocalAuthSession;
> +
> +  if (PlatformAuth == NULL) {
> +    AuthSession = NULL;
> +  } else {
> +    AuthSession = &LocalAuthSession;
> +    ZeroMem (&LocalAuthSession, sizeof (LocalAuthSession));
> +    LocalAuthSession.sessionHandle = TPM_RS_PW;
> +    LocalAuthSession.hmac.size = PlatformAuth->size;
> +    CopyMem (LocalAuthSession.hmac.buffer, PlatformAuth->buffer, PlatformAuth->size);
> +  }
> +
> +  DEBUG ((EFI_D_INFO, "Tpm2ClearControl ... \n"));
> +  Status = Tpm2ClearControl (TPM_RH_PLATFORM, AuthSession, NO);
> +  DEBUG ((EFI_D_INFO, "Tpm2ClearControl - %r\n", Status));
> +  if (EFI_ERROR (Status)) {
> +    goto Done;
> +  }
> +  DEBUG ((EFI_D_INFO, "Tpm2Clear ... \n"));
> +  Status = Tpm2Clear (TPM_RH_PLATFORM, AuthSession);
> +  DEBUG ((EFI_D_INFO, "Tpm2Clear - %r\n", Status));
> +
> +Done:
> +  ZeroMem (&LocalAuthSession.hmac, sizeof (LocalAuthSession.hmac));
> +  return Status;
> +}
> +
> +
> +/**
> +  Change EPS.
> +
> +  @param[in]  PlatformAuth      platform auth value. NULL means no platform auth change.
> +
> +  @retval EFI_SUCCESS Operation completed successfully.
> +**/
> +EFI_STATUS
> +Tpm2CommandChangeEps (
> +  IN TPM2B_AUTH                *PlatformAuth  OPTIONAL
> +  )
> +{
> +  EFI_STATUS                Status;
> +  TPMS_AUTH_COMMAND         *AuthSession;
> +  TPMS_AUTH_COMMAND         LocalAuthSession;
> +
> +  if (PlatformAuth == NULL) {
> +    AuthSession = NULL;
> +  } else {
> +    AuthSession = &LocalAuthSession;
> +    ZeroMem (&LocalAuthSession, sizeof (LocalAuthSession));
> +    LocalAuthSession.sessionHandle = TPM_RS_PW;
> +    LocalAuthSession.hmac.size = PlatformAuth->size;
> +    CopyMem (LocalAuthSession.hmac.buffer, PlatformAuth->buffer, PlatformAuth->size);
> +  }
> +
> +  Status = Tpm2ChangeEPS (TPM_RH_PLATFORM, AuthSession);
> +  DEBUG ((EFI_D_INFO, "Tpm2ChangeEPS - %r\n", Status));
> +
> +  ZeroMem (&LocalAuthSession.hmac, sizeof(LocalAuthSession.hmac));
> +  return Status;
> +}
> +
> +
> +/**
> +  Execute physical presence operation requested by the OS.
> +
> +  @param[in]      PlatformAuth        platform auth value. NULL means no platform auth change.
> +  @param[in]      CommandCode         Physical presence operation value.
> +  @param[in]      CommandParameter    Physical presence operation parameter.
> +
> +  @retval TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE   Unknown physical presence operation.
> +  @retval TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE   Error occurred during sending command to TPM or
> +                                                   receiving response from TPM.
> +  @retval Others                                   Return code from the TPM device after command execution.
> +**/
> +UINT32
> +Tcg2ExecutePhysicalPresence (
> +  IN      TPM2B_AUTH                       *PlatformAuth,  OPTIONAL
> +  IN      UINT32                           CommandCode,
> +  IN      UINT32                           CommandParameter
> +  )
> +{
> +  EFI_STATUS                        Status;
> +  EFI_TCG2_EVENT_ALGORITHM_BITMAP   TpmHashAlgorithmBitmap;
> +  UINT32                            ActivePcrBanks;
> +
> +  switch (CommandCode) {
> +    case TCG2_PHYSICAL_PRESENCE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
> +      Status = Tpm2CommandClear (PlatformAuth);
> +      if (EFI_ERROR (Status)) {
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      } else {
> +        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +      }
> +
> +    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
> +      Status = Tpm2GetCapabilitySupportedAndActivePcrs (&TpmHashAlgorithmBitmap, &ActivePcrBanks);
> +      ASSERT_EFI_ERROR (Status);
> +
> +      //
> +      // PP spec requirements:
> +      //    Firmware should check that all requested (set) hashing algorithms are supported with respective PCR banks.
> +      //    Firmware has to ensure that at least one PCR banks is active.
> +      // If not, an error is returned and no action is taken.
> +      //
> +      if (CommandParameter == 0 || (CommandParameter & (~TpmHashAlgorithmBitmap)) != 0) {
> +        DEBUG((DEBUG_ERROR, "PCR banks %x to allocate are not supported by TPM. Skip operation\n", CommandParameter));
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      }
> +
> +      Status = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap, CommandParameter);
> +      if (EFI_ERROR (Status)) {
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      } else {
> +        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +      }
> +
> +    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
> +      Status = Tpm2CommandChangeEps (PlatformAuth);
> +      if (EFI_ERROR (Status)) {
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      } else {
> +        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +      }
> +
> +    case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
> +      Status = Tpm2GetCapabilitySupportedAndActivePcrs (&TpmHashAlgorithmBitmap, &ActivePcrBanks);
> +      ASSERT_EFI_ERROR (Status);
> +      Status = Tpm2PcrAllocateBanks (PlatformAuth, TpmHashAlgorithmBitmap, TpmHashAlgorithmBitmap);
> +      if (EFI_ERROR (Status)) {
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      } else {
> +        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +      }
> +
> +    default:
> +      if (CommandCode <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
> +        return TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +      } else {
> +        return TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +      }
> +  }
> +}
> +
> +
> +/**
> +  Read the specified key for user confirmation.
> +
> +  @param[in]  CautionKey  If true,  F12 is used as confirm key;
> +                          If false, F10 is used as confirm key.
> +
> +  @retval     TRUE        User confirmed the changes by input.
> +  @retval     FALSE       User discarded the changes.
> +**/
> +BOOLEAN
> +Tcg2ReadUserKey (
> +  IN     BOOLEAN                    CautionKey
> +  )
> +{
> +  EFI_STATUS                        Status;
> +  EFI_INPUT_KEY                     Key;
> +  UINT16                            InputKey;
> +
> +  InputKey = 0;
> +  do {
> +    Status = gBS->CheckEvent (gST->ConIn->WaitForKey);
> +    if (!EFI_ERROR (Status)) {
> +      Status = gST->ConIn->ReadKeyStroke (gST->ConIn, &Key);
> +      if (Key.ScanCode == SCAN_ESC) {
> +        InputKey = Key.ScanCode;
> +      }
> +      if ((Key.ScanCode == SCAN_F10) && !CautionKey) {
> +        InputKey = Key.ScanCode;
> +      }
> +      if ((Key.ScanCode == SCAN_F12) && CautionKey) {
> +        InputKey = Key.ScanCode;
> +      }
> +    }
> +  } while (InputKey == 0);
> +
> +  if (InputKey != SCAN_ESC) {
> +    return TRUE;
> +  }
> +
> +  return FALSE;
> +}
> +
> +
> +/**
> +  Fill Buffer With BootHashAlg.
> +
> +  @param[in] Buffer               Buffer to be filled.
> +  @param[in] BufferSize           Size of buffer.
> +  @param[in] BootHashAlg          BootHashAlg.
> +
> +**/
> +VOID
> +Tcg2FillBufferWithBootHashAlg (
> +  IN UINT16  *Buffer,
> +  IN UINTN   BufferSize,
> +  IN UINT32  BootHashAlg
> +  )
> +{
> +  Buffer[0] = 0;
> +  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA1) != 0) {
> +    if (Buffer[0] != 0) {
> +      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +    }
> +    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA1", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +  }
> +  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA256) != 0) {
> +    if (Buffer[0] != 0) {
> +      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +    }
> +    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA256", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +  }
> +  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA384) != 0) {
> +    if (Buffer[0] != 0) {
> +      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +    }
> +    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA384", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +  }
> +  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SHA512) != 0) {
> +    if (Buffer[0] != 0) {
> +      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +    }
> +    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SHA512", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +  }
> +  if ((BootHashAlg & EFI_TCG2_BOOT_HASH_ALG_SM3_256) != 0) {
> +    if (Buffer[0] != 0) {
> +      StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L", ", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +    }
> +    StrnCatS (Buffer, BufferSize / sizeof (CHAR16), L"SM3_256", (BufferSize / sizeof (CHAR16)) - StrLen (Buffer) - 1);
> +  }
> +}
> +
> +
> +/**
> +  Display the confirm text and get user confirmation.
> +
> +  @param[in] TpmPpCommand             The requested TPM physical presence command.
> +  @param[in] TpmPpCommandParameter    The requested TPM physical presence command parameter.
> +
> +  @retval    TRUE          The user has confirmed the changes.
> +  @retval    FALSE         The user doesn't confirm the changes.
> +**/
> +BOOLEAN
> +Tcg2UserConfirm (
> +  IN      UINT32                    TpmPpCommand,
> +  IN      UINT32                    TpmPpCommandParameter
> +  )
> +{
> +  CHAR16                            *ConfirmText;
> +  CHAR16                            *TmpStr1;
> +  CHAR16                            *TmpStr2;
> +  UINTN                             BufSize;
> +  BOOLEAN                           CautionKey;
> +  BOOLEAN                           NoPpiInfo;
> +  UINT16                            Index;
> +  CHAR16                            DstStr[81];
> +  CHAR16                            TempBuffer[1024];
> +  CHAR16                            TempBuffer2[1024];
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> +  EFI_TCG2_BOOT_SERVICE_CAPABILITY  ProtocolCapability;
> +  UINT32                            CurrentPCRBanks;
> +  EFI_STATUS                        Status;
> +
> +  TmpStr2     = NULL;
> +  CautionKey  = FALSE;
> +  NoPpiInfo   = FALSE;
> +  BufSize     = CONFIRM_BUFFER_SIZE;
> +  ConfirmText = AllocateZeroPool (BufSize);
> +  ASSERT (ConfirmText != NULL);
> +
> +  mTcg2PpStringPackHandle = HiiAddPackages (&gEfiTcg2PhysicalPresenceGuid, gImageHandle, DxeTcg2PhysicalPresenceLibStrings, NULL);
> +  ASSERT (mTcg2PpStringPackHandle != NULL);
> +
> +  switch (TpmPpCommand) {
> +
> +    case TCG2_PHYSICAL_PRESENCE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
> +      CautionKey = TRUE;
> +      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CLEAR));
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
> +      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
> +      FreePool (TmpStr1);
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CLEAR));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), L" \n\n", (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      break;
> +
> +    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
> +      Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +      ASSERT_EFI_ERROR (Status);
> +
> +      ProtocolCapability.Size = sizeof(ProtocolCapability);
> +      Status = Tcg2Protocol->GetCapability (
> +                               Tcg2Protocol,
> +                               &ProtocolCapability
> +                               );
> +      ASSERT_EFI_ERROR (Status);
> +
> +      Status = Tcg2Protocol->GetActivePcrBanks (
> +                               Tcg2Protocol,
> +                               &CurrentPCRBanks
> +                               );
> +      ASSERT_EFI_ERROR (Status);
> +
> +      CautionKey = TRUE;
> +      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_SET_PCR_BANKS));
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
> +      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
> +      FreePool (TmpStr1);
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_SET_PCR_BANKS_1));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_SET_PCR_BANKS_2));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      Tcg2FillBufferWithBootHashAlg (TempBuffer, sizeof(TempBuffer), TpmPpCommandParameter);
> +      Tcg2FillBufferWithBootHashAlg (TempBuffer2, sizeof(TempBuffer2), CurrentPCRBanks);
> +
> +      TmpStr1 = AllocateZeroPool (BufSize);
> +      ASSERT (TmpStr1 != NULL);
> +      UnicodeSPrint (TmpStr1, BufSize, L"Current PCRBanks is 0x%x. (%s)\nNew PCRBanks is 0x%x. (%s)\n", CurrentPCRBanks, TempBuffer2, TpmPpCommandParameter, TempBuffer);
> +
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), L" \n", (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      break;
> +
> +    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
> +      CautionKey = TRUE;
> +      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CHANGE_EPS));
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_HEAD_STR));
> +      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
> +      FreePool (TmpStr1);
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CHANGE_EPS_1));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_WARNING_CHANGE_EPS_2));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +
> +      break;
> +
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
> +      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_ENABLE_BLOCK_SID));
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_HEAD_STR));
> +      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
> +      FreePool (TmpStr1);
> +      break;
> +
> +    case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
> +      TmpStr2 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_DISABLE_BLOCK_SID));
> +
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_HEAD_STR));
> +      UnicodeSPrint (ConfirmText, BufSize, TmpStr1, TmpStr2);
> +      FreePool (TmpStr1);
> +      break;
> +
> +    default:
> +      ;
> +  }
> +
> +  if (TmpStr2 == NULL) {
> +    FreePool (ConfirmText);
> +    return FALSE;
> +  }
> +
> +  if (TpmPpCommand < TCG2_PHYSICAL_PRESENCE_STORAGE_MANAGEMENT_BEGIN) {
> +    if (CautionKey) {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_CAUTION_KEY));
> +    } else {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_ACCEPT_KEY));
> +    }
> +    StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +    FreePool (TmpStr1);
> +
> +    if (NoPpiInfo) {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_NO_PPI_INFO));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +    }
> +
> +    TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TPM_REJECT_KEY));
> +  } else {
> +    if (CautionKey) {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_CAUTION_KEY));
> +    } else {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_ACCEPT_KEY));
> +    }
> +    StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +    FreePool (TmpStr1);
> +
> +    if (NoPpiInfo) {
> +      TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_NO_PPI_INFO));
> +      StrnCatS (ConfirmText, BufSize / sizeof (CHAR16), TmpStr1, (BufSize / sizeof (CHAR16)) - StrLen (ConfirmText) - 1);
> +      FreePool (TmpStr1);
> +    }
> +
> +    TmpStr1 = Tcg2PhysicalPresenceGetStringById (STRING_TOKEN (TCG_STORAGE_REJECT_KEY));
> +  }
> +  BufSize -= StrSize (ConfirmText);
> +  UnicodeSPrint (ConfirmText + StrLen (ConfirmText), BufSize, TmpStr1, TmpStr2);
> +
> +  DstStr[80] = L'\0';
> +  for (Index = 0; Index < StrLen (ConfirmText); Index += 80) {
> +    StrnCpyS (DstStr, sizeof (DstStr) / sizeof (CHAR16), ConfirmText + Index, sizeof (DstStr) / sizeof (CHAR16) - 1);
> +    Print (DstStr);
> +  }
> +
> +  FreePool (TmpStr1);
> +  FreePool (TmpStr2);
> +  FreePool (ConfirmText);
> +  HiiRemovePackages (mTcg2PpStringPackHandle);
> +
> +  if (Tcg2ReadUserKey (CautionKey)) {
> +    return TRUE;
> +  }
> +
> +  return FALSE;
> +}
> +
> +
> +/**
> +  Check if there is a valid physical presence command request. Also updates parameter value
> +  to whether the requested physical presence command already confirmed by user
> +
> +   @param[out] RequestConfirmed          If the physical presence operation command required user confirm from UI.
> +                                           True, it indicates the command doesn't require user confirm, or already confirmed
> +                                                 in last boot cycle by user.
> +                                           False, it indicates the command need user confirm from UI.
> +
> +   @retval  TRUE        Physical Presence operation command is valid.
> +   @retval  FALSE       Physical Presence operation command is invalid.
> +
> +**/
> +BOOLEAN
> +Tcg2HaveValidTpmRequest  (
> +  OUT     BOOLEAN                          *RequestConfirmed
> +  )
> +{
> +  EFI_TCG2_PROTOCOL                 *Tcg2Protocol;
> +  EFI_STATUS                        Status;
> +
> +  *RequestConfirmed = FALSE;
> +
> +  if (mPpi->Request <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
> +    //
> +    // Need TCG2 protocol.
> +    //
> +    Status = gBS->LocateProtocol (&gEfiTcg2ProtocolGuid, NULL, (VOID **) &Tcg2Protocol);
> +    if (EFI_ERROR (Status)) {
> +      return FALSE;
> +    }
> +  }
> +
> +  switch (mPpi->Request) {
> +    case TCG2_PHYSICAL_PRESENCE_NO_ACTION:
> +    case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
> +      *RequestConfirmed = TRUE;
> +      return TRUE;
> +
> +    case TCG2_PHYSICAL_PRESENCE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
> +    case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
> +    case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
> +    case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
> +    case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
> +      break;
> +
> +    default:
> +      //
> +      // Wrong Physical Presence command
> +      //
> +      return FALSE;
> +  }
> +
> +  //
> +  // Physical Presence command is correct
> +  //
> +  return TRUE;
> +}
> +
> +
> +/**
> +  Check and execute the requested physical presence command.
> +
> +  @param[in]      PlatformAuth      platform auth value. NULL means no platform auth change.
> +**/
> +VOID
> +Tcg2ExecutePendingTpmRequest (
> +  IN      TPM2B_AUTH                       *PlatformAuth OPTIONAL
> +  )
> +{
> +  BOOLEAN                           RequestConfirmed;
> +
> +  if (mPpi->Request == TCG2_PHYSICAL_PRESENCE_NO_ACTION) {
> +    //
> +    // No operation request
> +    //
> +    return;
> +  }
> +
> +  if (!Tcg2HaveValidTpmRequest (&RequestConfirmed)) {
> +    //
> +    // Invalid operation request.
> +    //
> +    if (mPpi->Request <= TCG2_PHYSICAL_PRESENCE_NO_ACTION_MAX) {
> +      mPpi->Response = TCG_PP_OPERATION_RESPONSE_SUCCESS;
> +    } else {
> +      mPpi->Response = TCG_PP_OPERATION_RESPONSE_BIOS_FAILURE;
> +    }
> +    mPpi->LastRequest = mPpi->Request;
> +    mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
> +    mPpi->RequestParameter = 0;
> +    return;
> +  }
> +
> +  if (!RequestConfirmed) {
> +    //
> +    // Print confirm text and wait for approval.
> +    //
> +    RequestConfirmed = Tcg2UserConfirm (mPpi->Request, mPpi->RequestParameter);
> +  }
> +
> +  //
> +  // Execute requested physical presence command
> +  //
> +  mPpi->Response = TCG_PP_OPERATION_RESPONSE_USER_ABORT;
> +  if (RequestConfirmed) {
> +    mPpi->Response = Tcg2ExecutePhysicalPresence (
> +                                                  PlatformAuth,
> +                                                  mPpi->Request,
> +                                                  mPpi->RequestParameter
> +                                                  );
> +  }
> +
> +  //
> +  // Clear request
> +  //
> +  mPpi->LastRequest = mPpi->Request;
> +  mPpi->Request = TCG2_PHYSICAL_PRESENCE_NO_ACTION;
> +  mPpi->RequestParameter = 0;
> +
> +  if (mPpi->Response == TCG_PP_OPERATION_RESPONSE_USER_ABORT) {
> +    return;
> +  }
> +
> +  //
> +  // Reset system to make new TPM settings in effect
> +  //
> +  switch (mPpi->LastRequest) {
> +  case TCG2_PHYSICAL_PRESENCE_CLEAR:
> +  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR:
> +  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_2:
> +  case TCG2_PHYSICAL_PRESENCE_ENABLE_CLEAR_3:
> +  case TCG2_PHYSICAL_PRESENCE_SET_PCR_BANKS:
> +  case TCG2_PHYSICAL_PRESENCE_CHANGE_EPS:
> +  case TCG2_PHYSICAL_PRESENCE_LOG_ALL_DIGESTS:
> +    break;
> +
> +  case TCG2_PHYSICAL_PRESENCE_ENABLE_BLOCK_SID:
> +  case TCG2_PHYSICAL_PRESENCE_DISABLE_BLOCK_SID:
> +    break;
> +
> +  default:
> +    if (mPpi->Request != TCG2_PHYSICAL_PRESENCE_NO_ACTION) {
> +      break;
> +    }
> +    return;
> +  }
> +
> +  Print (L"Rebooting system to make TPM2 settings in effect\n");
> +  gRT->ResetSystem (EfiResetCold, EFI_SUCCESS, 0, NULL);
> +  ASSERT (FALSE);
> +}
> +
> +
> +/**
> +   Check and execute the pending TPM request.
> +
> +   The TPM request may come from OS or BIOS. This API will display request information and wait
> +   for user confirmation if TPM request exists. The TPM request will be sent to TPM device after
> +   the TPM request is confirmed, and one or more reset may be required to make TPM request to
> +   take effect.
> +
> +   This API should be invoked after console in and console out are all ready as they are required
> +   to display request information and get user input to confirm the request.
> +
> +   @param[in]  PlatformAuth                   platform auth value. NULL means no platform auth change.
> +**/
> +VOID
> +EFIAPI
> +Tcg2PhysicalPresenceLibProcessRequest (
> +  IN      TPM2B_AUTH                     *PlatformAuth  OPTIONAL
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  Status = QemuTpmInitPPI ();
> +  if (EFI_ERROR (Status)) {
> +    DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
> +    return ;
> +  }
> +
> +  //
> +  // Check S4 resume
> +  //
> +  if (GetBootModeHob () == BOOT_ON_S4_RESUME) {
> +    DEBUG ((EFI_D_INFO, "S4 Resume, Skip TPM PP process!\n"));
> +    return ;
> +  }
> +
> +  DEBUG ((EFI_D_INFO, "[TPM2PP] PPRequest=%x (PPRequestParameter=%x)\n", mPpi->Request, mPpi->RequestParameter));
> +  Tcg2ExecutePendingTpmRequest (PlatformAuth);
> +}
> +
> +
> +/**
> +  The handler for TPM physical presence function:
> +  Return TPM Operation Response to OS Environment.
> +
> +  @param[out]     MostRecentRequest Most recent operation request.
> +  @param[out]     Response          Response to the most recent operation request.
> +
> +  @return Return Code for Return TPM Operation Response to OS Environment.
> +**/
> +UINT32
> +EFIAPI
> +Tcg2PhysicalPresenceLibReturnOperationResponseToOsFunction (
> +  OUT UINT32                *MostRecentRequest,
> +  OUT UINT32                *Response
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  DEBUG ((EFI_D_INFO, "[TPM2PP] ReturnOperationResponseToOsFunction\n"));
> +
> +  Status = QemuTpmInitPPI ();
> +  if (EFI_ERROR (Status)) {
> +      DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
> +      *MostRecentRequest = 0;
> +      *Response          = 0;
> +      return TCG_PP_RETURN_TPM_OPERATION_RESPONSE_FAILURE;
> +  }
> +
> +  *MostRecentRequest = mPpi->LastRequest;
> +  *Response          = mPpi->Response;
> +
> +  return TCG_PP_RETURN_TPM_OPERATION_RESPONSE_SUCCESS;
> +}
> +
> +
> +/**
> +  The handler for TPM physical presence function:
> +  Submit TPM Operation Request to Pre-OS Environment and
> +  Submit TPM Operation Request to Pre-OS Environment 2.
> +
> +  Caution: This function may receive untrusted input.
> +
> +  @param[in]      OperationRequest TPM physical presence operation request.
> +  @param[in]      RequestParameter TPM physical presence operation request parameter.
> +
> +  @return Return Code for Submit TPM Operation Request to Pre-OS Environment and
> +          Submit TPM Operation Request to Pre-OS Environment 2.
> +**/
> +UINT32
> +EFIAPI
> +Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunction (
> +  IN UINT32                 OperationRequest,
> +  IN UINT32                 RequestParameter
> +  )
> +{
> +  EFI_STATUS Status;
> +
> +  DEBUG ((EFI_D_INFO, "[TPM2PP] SubmitRequestToPreOSFunction, Request = %x, %x\n", OperationRequest, RequestParameter));
> +
> +  Status = QemuTpmInitPPI ();
> +  if (EFI_ERROR (Status)) {
> +      DEBUG ((EFI_D_INFO, "[TPM2PP] no TPM\n"));
> +      return TCG_PP_SUBMIT_REQUEST_TO_PREOS_GENERAL_FAILURE;
> +  }
> +
> +  mPpi->Request = OperationRequest;
> +  mPpi->RequestParameter = RequestParameter;
> +
> +  return TCG_PP_SUBMIT_REQUEST_TO_PREOS_SUCCESS;
> +}

Right; the Tcg2PhysicalPresenceLib class is a bit tricky because it
provides two sets of interfaces.

The common trait between both sets is that all of these functions have
to call QemuTpmInitPPI() in this lib instance.

And the distinguishing factor is that
- the first set, namely ProcessRequest() alone, is provided to platform BDS,
- while the second set, consisting of SubmitRequestToPreOSFunction() and
 ReturnOperationResponseToOsFunction(), is required by the TCG2 protocol
implementation.

(17) Thus, my request here is that you please update the first bullet
point in the commit message:

  - remove all the functions that are unreachable from ProcessRequest()
    [called from platform BDS], or SubmitRequestToPreOSFunction() and
    ReturnOperationResponseToOsFunction() [called from Tcg2Dxe].

> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
> new file mode 100644
> index 000000000000..6b2d70c711fe
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
> @@ -0,0 +1,67 @@
> +## @file
> +#  Executes TPM 2.0 requests from OS or BIOS
> +#
> +#  This library will check and execute TPM 2.0 request from OS or BIOS. The request may
> +#  ask for user confirmation before execution.

(18) Please append the two main bullet points from the commit message to
this comment as well (as updated according to (17)).

> +#
> +#  Caution: This module requires additional review when modified.
> +#  This driver will have external input - variable.
> +#  This external input must be validated carefully to avoid security issue.
> +#
> +# Copyright (C) 2018, Red Hat, Inc.
> +# Copyright (c) 2013 - 2016, Intel Corporation. All rights reserved.<BR>
> +# This program and the accompanying materials
> +# are licensed and made available under the terms and conditions of the BSD License
> +# which accompanies this distribution. The full text of the license may be found at
> +# http://opensource.org/licenses/bsd-license.php
> +# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +# WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +#
> +##
> +
> +[Defines]
> +  INF_VERSION                    = 0x00010005
> +  BASE_NAME                      = DxeTcg2PhysicalPresenceLib

(19) This should say "Tcg2PhysicalPresenceLibQemu".

> +  MODULE_UNI_FILE                = DxeTcg2PhysicalPresenceLib.uni
> +  FILE_GUID                      = 41D3E698-9EEC-41FF-9CBB-5FE79A0CF326
> +  MODULE_TYPE                    = DXE_DRIVER
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = Tcg2PhysicalPresenceLib|DXE_DRIVER DXE_RUNTIME_DRIVER DXE_SAL_DRIVER UEFI_APPLICATION UEFI_DRIVER
> +
> +#
> +# The following information is for reference only and not required by the build tools.
> +#
> +#  VALID_ARCHITECTURES           = IA32 X64 IPF EBC
> +#
> +
> +[Sources]
> +  DxeTcg2PhysicalPresenceLib.c
> +  PhysicalPresenceStrings.uni
> +
> +[Packages]
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  SecurityPkg/SecurityPkg.dec
> +
> +[LibraryClasses]
> +  BaseMemoryLib
> +  DebugLib
> +  HiiLib
> +  HobLib
> +  MemoryAllocationLib
> +  PrintLib
> +  QemuFwCfgLib
> +  Tpm2CommandLib
> +  UefiBootServicesTableLib
> +  UefiLib
> +  UefiRuntimeServicesTableLib
> +
> +[Protocols]
> +  gEfiTcg2ProtocolGuid                 ## SOMETIMES_CONSUMES
> +
> +[Guids]
> +  ## SOMETIMES_CONSUMES ## HII
> +  ## SOMETIMES_PRODUCES ## Variable:L"Tcg2PhysicalPresence"
> +  ## SOMETIMES_CONSUMES ## Variable:L"Tcg2PhysicalPresence"

(20) Please drop the above two comment lines
('Variable:L"Tcg2PhysicalPresence"' is TCG2_PHYSICAL_PRESENCE_VARIABLE.)

> +  gEfiTcg2PhysicalPresenceGuid
> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
> new file mode 100644
> index 000000000000..aaae8f5014e7
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.uni
> @@ -0,0 +1,26 @@
> +// /** @file
> +// Executes TPM 2.0 requests from OS or BIOS
> +//
> +// This library will check and execute TPM 2.0 request from OS or BIOS. The request may
> +// ask for user confirmation before execution.
> +//
> +// Caution: This module requires additional review when modified.
> +// This driver will have external input - variable.
> +// This external input must be validated carefully to avoid security issue.
> +//
> +// Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> +//
> +// This program and the accompanying materials
> +// are licensed and made available under the terms and conditions of the BSD License
> +// which accompanies this distribution. The full text of the license may be found at
> +// http://opensource.org/licenses/bsd-license.php
> +// THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +// WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +//
> +// **/
> +
> +
> +#string STR_MODULE_ABSTRACT             #language en-US "Executes TPM 2.0 requests from OS or BIOS"
> +
> +#string STR_MODULE_DESCRIPTION          #language en-US "This library will check and execute TPM 2.0 request from OS or BIOS. The request may ask for user confirmation before execution.\n"
> +                                                        "Caution: This module requires additional review when modified. This driver will have external input - variable. This external input must be validated carefully to avoid security issue."
> diff --git a/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni
> new file mode 100644
> index 000000000000..1470286b4c3b
> --- /dev/null
> +++ b/OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/PhysicalPresenceStrings.uni
> @@ -0,0 +1,49 @@
> +/** @file
> +  String definitions for TPM 2.0 physical presence confirm text.
> +
> +Copyright (c) 2013 - 2014, Intel Corporation. All rights reserved.<BR>
> +This program and the accompanying materials
> +are licensed and made available under the terms and conditions of the BSD License
> +which accompanies this distribution.  The full text of the license may be found at
> +http://opensource.org/licenses/bsd-license.php
> +
> +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS,
> +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
> +
> +**/
> +
> +#langdef en-US "English"
> +
> +#string TPM_HEAD_STR                  #language en-US    "A configuration change was requested to %s this computer's TPM (Trusted Platform Module)\n\n"
> +
> +#string TPM_ACCEPT_KEY                #language en-US    "Press F10 "
> +#string TPM_CAUTION_KEY               #language en-US    "Press F12 "
> +#string TPM_REJECT_KEY                #language en-US    "to %s the TPM \nPress ESC to reject this change request and continue\n"
> +
> +#string TPM_ENABLE                    #language en-US    "enable"
> +#string TPM_DISABLE                   #language en-US    "disable"

(21) I think you can remove these two

> +#string TPM_CLEAR                     #language en-US    "clear"
> +#string TPM_SET_PCR_BANKS                       #language en-US    "change the boot measurements to use PCR bank(s) of"
> +#string TPM_CHANGE_EPS                          #language en-US    "clear and change identity of"
> +
> +#string TPM_NO_PPI_MAINTAIN           #language en-US    "maintain"
> +#string TPM_NO_PPI_TURN_ON            #language en-US    "turn on"
> +#string TPM_NO_PPI_TURN_OFF           #language en-US    "turn off"

(22) these three appear unused as well

The rest looks good. Thanks!
Laszlo

> +#string TPM_NO_PPI_INFO               #language en-US    "to approve future Operating System requests "
> +
> +#string TPM_WARNING_CLEAR             #language en-US    "WARNING: Clearing erases information stored on the TPM. You will lose all created keys and access to data encrypted by these keys. "
> +#string TPM_WARNING_SET_PCR_BANKS_1                     #language en-US    "WARNING: Changing the PCR bank(s) of the boot measurements may prevent the Operating System from properly processing the measurements. Please check if your Operating System supports the new PCR bank(s).\n\n"
> +#string TPM_WARNING_SET_PCR_BANKS_2                     #language en-US    "WARNING: Secrets in the TPM that are bound to the boot state of your machine may become unusable.\n\n"
> +#string TPM_WARNING_CHANGE_EPS_1                        #language en-US    "WARNING: Clearing erases information stored on the TPM. You will lose all created keys and access to data encrypted with these keys.\n\n"
> +#string TPM_WARNING_CHANGE_EPS_2                        #language en-US    "WARNING: Changing the identity of the TPM may require additional steps to establish trust into the new identity.\n\n"
> +
> +#string TCG_STORAGE_HEAD_STR                  #language en-US    "A configuration change was requested to %s on subsequent boots\n\n"
> +
> +#string TCG_STORAGE_ACCEPT_KEY                #language en-US    "Press F10 "
> +#string TCG_STORAGE_CAUTION_KEY               #language en-US    "Press F12 "
> +#string TCG_STORAGE_REJECT_KEY                #language en-US    "to %s\nPress ESC to reject this change request and continue\n"
> +
> +#string TCG_STORAGE_NO_PPI_INFO               #language en-US    "to approve future Operating System requests "
> +
> +#string TCG_STORAGE_ENABLE_BLOCK_SID          #language en-US    "issue a Block SID authentication command"
> +#string TCG_STORAGE_DISABLE_BLOCK_SID         #language en-US    "disable issuing a Block SID authentication command"
> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc
> index 6c361b73cd55..251434a9ff7c 100644
> --- a/OvmfPkg/OvmfPkgIa32.dsc
> +++ b/OvmfPkg/OvmfPkgIa32.dsc
> @@ -206,7 +206,7 @@ [LibraryClasses]
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> -  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
>  !else
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc
> index 62a6075a671d..ce247a59d61a 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.dsc
> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc
> @@ -211,7 +211,7 @@ [LibraryClasses]
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> -  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
>  !else
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index cbab1aa328c6..67f7e155ee3e 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -211,7 +211,7 @@ [LibraryClasses]
>  
>  !if $(TPM2_ENABLE) == TRUE
>    Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> -  Tcg2PhysicalPresenceLib|SecurityPkg/Library/DxeTcg2PhysicalPresenceLib/DxeTcg2PhysicalPresenceLib.inf
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
>    Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
>  !else
>    Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> 

Re: [Qemu-devel] [edk2] [PATCH 4/4] ovmf: process TPM PPI request in AfterConsole()

[Laszlo Ersek]

On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Call Tcg2PhysicalPresenceLibProcessRequest() to process pending PPI
> requests from PlatformBootManagerAfterConsole().
> 
> Laszlo understanding of edk2 is that the PPI operation processing was
> meant to occur *entirely* before End-Of-Dxe, so that 3rd party UEFI
> drivers couldn't interfere with PPI opcode processing *at all*.
> 
> He suggested that we should *not* call
> Tcg2PhysicalPresenceLibProcessRequest() from BeforeConsole(). Because,
> an "auth" console, i.e. one that does not depend on a 3rd party
> driver, is *in general* impossible to guarantee. Instead we could opt
> to trust 3rd party drivers, and use the "normal" console(s) in
> AfterConsole(), in order to let the user confirm the PPI requests. It
> will depend on the user to enable Secure Boot, so that the
> trustworthiness of those 3rd party drivers is ensured. If an attacker
> roots the guest OS from within, queues some TPM2 PPI requests, and
> also modifies drivers on the EFI system partition and/or in GPU option
> ROMs (?), then those drivers will not load after guest reboot, and
> thus the dependent console(s) won't be used for confirming the PPI
> requests.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c      | 8 ++++++++
>  .../PlatformBootManagerLib/PlatformBootManagerLib.inf     | 2 ++
>  2 files changed, 10 insertions(+)
> 
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> index 004b753f4d26..8b1beaa3e207 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/BdsPlatform.c
> @@ -16,6 +16,7 @@
>  #include <Guid/XenInfo.h>
>  #include <Guid/RootBridgesConnectedEventGroup.h>
>  #include <Protocol/FirmwareVolume2.h>
> +#include <Library/Tcg2PhysicalPresenceLib.h>
>  
>  
>  //
> @@ -1410,6 +1411,13 @@ PlatformBootManagerAfterConsole (
>    //
>    PciAcpiInitialization ();
>  
> +
> +  //
> +  // Process TPM PPI request
> +  //
> +  Tcg2PhysicalPresenceLibProcessRequest (NULL);
> +
> +

Please just keep one empty line before and after the new code. With that
cleanup, for this patch:

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

This series is a very nice work IMO, thank you both Stefan and
Marc-André. I hope v2 can be merged!

Thanks!
Laszlo

>    //
>    // Process QEMU's -kernel command line option
>    //
> diff --git a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> index 27789b7377bc..4b72c44bcf0a 100644
> --- a/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> +++ b/OvmfPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf
> @@ -38,6 +38,7 @@ [Packages]
>    IntelFrameworkModulePkg/IntelFrameworkModulePkg.dec
>    SourceLevelDebugPkg/SourceLevelDebugPkg.dec
>    OvmfPkg/OvmfPkg.dec
> +  SecurityPkg/SecurityPkg.dec
>  
>  [LibraryClasses]
>    BaseLib
> @@ -56,6 +57,7 @@ [LibraryClasses]
>    LoadLinuxLib
>    QemuBootOrderLib
>    UefiLib
> +  Tcg2PhysicalPresenceLib
>  
>  [Pcd]
>    gUefiOvmfPkgTokenSpaceGuid.PcdEmuVariableEvent
> 

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Marc-André Lureau]

Hi

On Wed, May 16, 2018 at 11:29 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> Hi Marc-André,
>
> On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
>> with TPM2 (I haven't tested TPM1, for lack of interest).
>
> I got the review of this patch series added to my TODO list, but I'll
> have to ask for your patience. :(
>
> From an extremely superficial skim:
>
> * please use the
>
>     TopDirPkg/ModuleName: blah blah blah
>
>   subject format, or more generally, if a module cannot be identified,
>
>     TopDirPkg: blah blah blah
>

done

> * the subject line and the commit message shouldn't be wider than 74
>   chars;
>

that should be ok

> * edk2 uses two spaces for general indentation, and I'm noticing some
>   inconsistency there (4 spaces like in QEMU).

yes, I tried to respect that, but sometime fail (emacs c-basic-offset
2 isn't great with comments)

>
> * Please consider formatting the patches with "--find-copies-harder"
>   (although I can look at them with the same option after fetching the
>   series from your repo). This option is usually helpful for reviewers
>   when cloning and modifying modules cross-package.

Hmm, I didn't know that option, ok

>
> * Please consider adopting the git settings at
>   <https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers>,
>   in particular:
>
>   - "--stat=1000 --stat-graph-width=20", so that pathnames are not
>     truncated in the diffstats,
>

I use git-publish very often. I had to modify it to pass those options
(https://github.com/stefanha/git-publish/pull/48)

>   - the "xfuncname"-related settings, so that git diff hunk headers @@
>     are useful for DSC and INF files too,
>

This is already in my .git/config, I hope it takes it by default in
format-patch?

>   - the diff order file, so that files are listed in patches in logical
>     order, going from abstract / descriptive (.inf, .h) to concrete /
>     imperative (.c).
>

ok

> Not much of a review, I know; this is all I can offer right now. If you
> have the time to respin just with these superficial changes, that might
> make my life easier. If you prefer to delay them, that's 100% fine too.
>

I am going to resend with the style fixes.

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Marc-André Lureau]

Hi

On Thu, May 17, 2018 at 9:54 AM, Laszlo Ersek <lersek@redhat.com> wrote:
> On 05/15/18 14:30, marcandre.lureau@redhat.com wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> Hi,
>>
>> The following series adds basic TPM PPI 1.3 support for OVMF-on-QEMU
>> with TPM2 (I haven't tested TPM1, for lack of interest).
>>
>> PPI test runs successfully with Windows 10 WHLK, despite the limited
>> number of supported funcions (tpm2_ppi_funcs table, in particular, no
>> function allows to manipulate Tcg2PhysicalPresenceFlags)
>>
>> The way it works is relatively simple: a memory region is allocated by
>> QEMU to save PPI related variables. An ACPI interface is exposed by
>> QEMU to let the guest manipulate those. At boot, ovmf processes and
>> updates the PPI qemu region and request variables.
>>
>> I build edk2 with:
>>
>> $ build -DTPM2_ENABLE -DSECURE_BOOT_ENABLE
>
> Is -DSECURE_BOOT_ENABLE necessary for *building* with -DTPM2_ENABLE? If
> that's the case, we should update the DSC files; users building OVMF
> from source shouldn't have to care about "-D" inter-dependencies, if we
> can manage that somehow.

No, that's only my build setup, because it is likely both will be used
together. TPM usage/tests seem to be fine without it.

>
> If -DSECURE_BOOT_ENABLE is only there because otherwise a guest OS
> doesn't really make use of -DTPM2_ENABLE either, that's different. In
> that case, it's fine to allow building OVMF with TPM2 support but
> without SB support.
>
> Thanks!
> Laszlo

Re: [Qemu-devel] [edk2] [PATCH 0/4] RFC: ovmf: Add support for TPM Physical Presence interface

[Laszlo Ersek]

On 05/17/18 16:43, Marc-André Lureau wrote:
> On Wed, May 16, 2018 at 11:29 AM, Laszlo Ersek <lersek@redhat.com> wrote:

>>   - the "xfuncname"-related settings, so that git diff hunk headers @@
>>     are useful for DSC and INF files too,
>>
> 
> This is already in my .git/config, I hope it takes it by default in
> format-patch?

You also need to classify files appropriately so that the xfuncname
setting apply to them:

https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-09

>> Not much of a review, I know; this is all I can offer right now. If you
>> have the time to respin just with these superficial changes, that might
>> make my life easier. If you prefer to delay them, that's 100% fine too.
>>
> 
> I am going to resend with the style fixes.

I managed to review your v1 in full earlier today, so I'd prefer to
review your v3, with my more in-depth comments addressed as well.

Thanks!
Laszlo