[PATCH v2 0/5] ArmVirtPkg: implement measured boot for ArmVirtQemu

[PATCH v2 0/5] ArmVirtPkg: implement measured boot for ArmVirtQemu

[Ard Biesheuvel]

Wire up the various existing pieces so that we can implement measured
boot on ArmVirtQemu based on the TPM support in QEMU, just like it has
been implemented for x86 in OvmfPkg.

The main difference is that on ARM, we first need to discover the TPM base
address from the device tree provided by QEMU, as well as the PSCI method
used to perform a cold reset.

Changes since v1:
- use a separate ResetSystemLib instance based on on-demand parsing of the
  DT, and expose it via the ResetSystem PPI to other client PEIMs
- add Laszlo's ack to #1
- incorporate Laszlo's review feedback across the board

Cc: lersek@redhat.com
Cc: eric.auger@redhat.com
Cc: philmd@redhat.com
Cc: marcandre.lureau@redhat.com
Cc: stefanb@linux.ibm.com
Cc: leif@nuviainc.com

Ard Biesheuvel (5):
  OvmfPkg/Tcg2ConfigPei: introduce a signalling PPI to depex on
  ArmVirtPkg/PlatformPeiLib: make PcdLib dependency explicit in .INF
  ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT
  ArmVirtPkg: implement ArmVirtPsciResetSystemPeiLib
  ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

 ArmVirtPkg/ArmVirt.dsc.inc                    |   6 +
 ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
 ArmVirtPkg/ArmVirtQemu.dsc                    |  75 ++++++
 ArmVirtPkg/ArmVirtQemu.fdf                    |   6 +
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |  10 +
 .../ArmVirtPsciResetSystemPeiLib.c            | 232 ++++++++++++++++++
 .../ArmVirtPsciResetSystemPeiLib.inf          |  39 +++
 .../Library/PlatformPeiLib/PlatformPeiLib.c   | 101 +++++++-
 .../Library/PlatformPeiLib/PlatformPeiLib.inf |  20 +-
 OvmfPkg/OvmfPkg.dec                           |   5 +
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf      |   6 +-
 11 files changed, 491 insertions(+), 15 deletions(-)
 create mode 100644 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
 create mode 100644 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf

-- 
2.17.1

[PATCH v2 1/5] OvmfPkg/Tcg2ConfigPei: introduce a signalling PPI to depex on

[Ard Biesheuvel]

On ARM systems, the TPM does not live at a fixed address, and so we
need the platform to discover it first. So introduce a PPI that signals
that the TPM address has been discovered and recorded in the appropriate
PCD, and make Tcg2ConfigPei depex on it when built for ARM or AARCH64.

Reviewed-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 OvmfPkg/OvmfPkg.dec                      | 5 +++++
 OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 6 +++++-
 2 files changed, 10 insertions(+), 1 deletion(-)

diff --git a/OvmfPkg/OvmfPkg.dec b/OvmfPkg/OvmfPkg.dec
index 4c5b6511cb97..30faecb7a5c8 100644
--- a/OvmfPkg/OvmfPkg.dec
+++ b/OvmfPkg/OvmfPkg.dec
@@ -87,6 +87,11 @@ [Guids]
   gEfiLegacyBiosGuid                  = {0x2E3044AC, 0x879F, 0x490F, {0x97, 0x60, 0xBB, 0xDF, 0xAF, 0x69, 0x5F, 0x50}}
   gEfiLegacyDevOrderVariableGuid      = {0xa56074db, 0x65fe, 0x45f7, {0xbd, 0x21, 0x2d, 0x2b, 0xdd, 0x8e, 0x96, 0x52}}
 
+[Ppis]
+  # PPI whose presence in the PPI database signals that the TPM base address
+  # has been discovered and recorded
+  gOvmfTpmDiscoveredPpiGuid           = {0xb9a61ad0, 0x2802, 0x41f3, {0xb5, 0x13, 0x96, 0x51, 0xce, 0x6b, 0xd5, 0x75}}
+
 [Protocols]
   gVirtioDeviceProtocolGuid           = {0xfa920010, 0x6785, 0x4941, {0xb6, 0xec, 0x49, 0x8c, 0x57, 0x9f, 0x16, 0x0a}}
   gXenBusProtocolGuid                 = {0x3d3ca290, 0xb9a5, 0x11e3, {0xb7, 0x5d, 0xb8, 0xac, 0x6f, 0x7d, 0x65, 0xe6}}
diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
index e34cd6210611..55684ba045b3 100644
--- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
@@ -25,6 +25,7 @@ [Sources]
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
+  OvmfPkg/OvmfPkg.dec
   SecurityPkg/SecurityPkg.dec
 
 [LibraryClasses]
@@ -43,5 +44,8 @@ [Ppis]
 [Pcd]
   gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid                 ## PRODUCES
 
-[Depex]
+[Depex.IA32, Depex.X64]
   TRUE
+
+[Depex.ARM, Depex.AARCH64]
+  gOvmfTpmDiscoveredPpiGuid
-- 
2.17.1

[PATCH v2 2/5] ArmVirtPkg/PlatformPeiLib: make PcdLib dependency explicit in .INF

[Ard Biesheuvel]

We currently include PcdLib.h in PlatformPeiLib, without declaring
this dependency in its .INF description. Since all the PCDs we use
resolve to fixed type in practice, this does not really matter at
the moment, but since we will be adding dynamic PCD references in
a subsequent patch, let's make the PcdLib dependency explicit, so
that its dispatch is guaranteed to be ordered correctly with respect
to the provider of the dynamic PCD PPI.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 46db117ac28e..5428040f121d 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -29,6 +29,7 @@ [LibraryClasses]
   DebugLib
   HobLib
   FdtLib
+  PcdLib
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFvSize
-- 
2.17.1

[PATCH v2 3/5] ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT

[Ard Biesheuvel]

Introduce a boolean PCD that tells us whether TPM support is enabled
in the build, and if it is, record the TPM base address in the existing
routine that traverses the device tree in the platform PEIM.

If a TPM is found, install the gOvmfTpmDiscoveredPpiGuid signalling PPI
that will unlock the dispatch of OvmfPkg's Tcg2ConfigPei. If TPM2
support is enabled in the build but no TPM2 device is found, install the
gPeiTpmInitializationDonePpiGuid PPI, which is normally installed by
Tcg2ConfigPei if no TPM2 is found, but in our case Tcg2ConfigPei will
never run so let's do it here instead.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirt.dsc.inc                           |   6 ++
 ArmVirtPkg/ArmVirtPkg.dec                            |   6 ++
 ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c   | 101 ++++++++++++++++++--
 ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf |  19 +++-
 4 files changed, 118 insertions(+), 14 deletions(-)

diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
index 10037c938eb8..abb253fdf76a 100644
--- a/ArmVirtPkg/ArmVirt.dsc.inc
+++ b/ArmVirtPkg/ArmVirt.dsc.inc
@@ -366,6 +366,12 @@ [PcdsFixedAtBuild.common]
   #
   gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
 
+[PcdsPatchableInModule]
+  # we need a default resolution for this PCD that supports PcdSet64(),
+  # even though any actual calls will be compiled out on builds that have
+  # gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled == FALSE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
+
 [Components.common]
   #
   # Ramdisk support
diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
index a019cc269d10..08ddd68a863e 100644
--- a/ArmVirtPkg/ArmVirtPkg.dec
+++ b/ArmVirtPkg/ArmVirtPkg.dec
@@ -36,6 +36,12 @@ [Guids.common]
 [Protocols]
   gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
 
+[PcdsFeatureFlag]
+  #
+  # Feature Flag PCD that defines whether TPM2 support is enabled
+  #
+  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|FALSE|BOOLEAN|0x00000004
+
 [PcdsFixedAtBuild, PcdsPatchableInModule]
   #
   # This is the physical address where the device tree is expected to be stored
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
index 0a1469550db0..8b5b3dd5dc1c 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
@@ -1,7 +1,7 @@
 /** @file
 *
 *  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
-*  Copyright (c) 2014, Linaro Limited. All rights reserved.
+*  Copyright (c) 2014-2020, Linaro Limited. All rights reserved.
 *
 *  SPDX-License-Identifier: BSD-2-Clause-Patent
 *
@@ -13,11 +13,24 @@
 #include <Library/DebugLib.h>
 #include <Library/HobLib.h>
 #include <Library/PcdLib.h>
+#include <Library/PeiServicesLib.h>
 #include <libfdt.h>
 
 #include <Guid/EarlyPL011BaseAddress.h>
 #include <Guid/FdtHob.h>
 
+STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2DiscoveredPpi = {
+  EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  &gOvmfTpmDiscoveredPpiGuid,
+  NULL
+};
+
+STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2InitializationDonePpi = {
+  EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
+  &gPeiTpmInitializationDonePpiGuid,
+  NULL
+};
+
 EFI_STATUS
 EFIAPI
 PlatformPeim (
@@ -31,14 +44,18 @@ PlatformPeim (
   UINT64             *FdtHobData;
   UINT64             *UartHobData;
   INT32              Node, Prev;
+  INT32              Parent, Depth;
   CONST CHAR8        *Compatible;
   CONST CHAR8        *CompItem;
   CONST CHAR8        *NodeStatus;
   INT32              Len;
+  INT32              RangesLen;
   INT32              StatusLen;
   CONST UINT64       *RegProp;
+  CONST UINT32       *RangesProp;
   UINT64             UartBase;
-
+  UINT64             TpmBase;
+  EFI_STATUS         Status;
 
   Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
   ASSERT (Base != NULL);
@@ -58,18 +75,18 @@ PlatformPeim (
   ASSERT (UartHobData != NULL);
   *UartHobData = 0;
 
-  //
-  // Look for a UART node
-  //
-  for (Prev = 0;; Prev = Node) {
-    Node = fdt_next_node (Base, Prev, NULL);
+  TpmBase = 0;
+
+  for (Prev = Depth = 0;; Prev = Node) {
+    Node = fdt_next_node (Base, Prev, &Depth);
     if (Node < 0) {
       break;
     }
 
-    //
-    // Check for UART node
-    //
+    if (Depth == 1) {
+      Parent = Node;
+    }
+
     Compatible = fdt_getprop (Base, Node, "compatible", &Len);
 
     //
@@ -93,10 +110,74 @@ PlatformPeim (
 
         *UartHobData = UartBase;
         break;
+      } else if (FeaturePcdGet (PcdTpm2SupportEnabled) &&
+                 AsciiStrCmp (CompItem, "tcg,tpm-tis-mmio") == 0) {
+
+        RegProp = fdt_getprop (Base, Node, "reg", &Len);
+        ASSERT (Len == 8 || Len == 16);
+        if (Len == 8) {
+          TpmBase = fdt32_to_cpu (RegProp[0]);
+        } else if (Len == 16) {
+          TpmBase = fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RegProp));
+        }
+
+        if (Depth > 1) {
+          //
+          // QEMU/mach-virt may put the TPM on the platform bus, in which case
+          // we have to take its 'ranges' property into account to translate the
+          // MMIO address. This consists of a <child base, parent base, size>
+          // tuple, where the child base and the size use the same number of
+          // cells as the 'reg' property above, and the parent base uses 2 cells
+          //
+          RangesProp = fdt_getprop (Base, Parent, "ranges", &RangesLen);
+          ASSERT (RangesProp != NULL);
+
+          //
+          // a plain 'ranges' attribute without a value implies a 1:1 mapping
+          //
+          if (RangesLen != 0) {
+            //
+            // assume a single translated range with 2 cells for the parent base
+            //
+            if (RangesLen != Len + 2 * sizeof (UINT32)) {
+              DEBUG ((DEBUG_WARN,
+                "%a: 'ranges' property has unexpected size %d\n",
+                __FUNCTION__, RangesLen));
+              break;
+            }
+
+            if (Len == 8) {
+              TpmBase -= fdt32_to_cpu (RangesProp[0]);
+            } else {
+              TpmBase -= fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RangesProp));
+            }
+
+            //
+            // advance RangesProp to the parent bus address
+            //
+            RangesProp = (UINT32 *)((UINT8 *)RangesProp + Len / 2);
+            TpmBase += fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RangesProp));
+          }
+        }
+        break;
       }
     }
   }
 
+  if (FeaturePcdGet (PcdTpm2SupportEnabled)) {
+    if (TpmBase != 0) {
+      DEBUG ((DEBUG_INFO, "%a: TPM @ 0x%lx\n", __FUNCTION__, TpmBase));
+
+      Status = (EFI_STATUS)PcdSet64S (PcdTpmBaseAddress, TpmBase);
+      ASSERT_EFI_ERROR (Status);
+
+      Status = PeiServicesInstallPpi (&mTpm2DiscoveredPpi);
+    } else {
+      Status = PeiServicesInstallPpi (&mTpm2InitializationDonePpi);
+    }
+    ASSERT_EFI_ERROR (Status);
+  }
+
   BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
 
   return EFI_SUCCESS;
diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
index 5428040f121d..3f97ef080520 100644
--- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
+++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
@@ -1,7 +1,7 @@
 #/** @file
 #
 #  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
-#  Copyright (c) 2014, Linaro Limited. All rights reserved.
+#  Copyright (c) 2014-2020, Linaro Limited. All rights reserved.
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -11,7 +11,7 @@ [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PlatformPeiLib
   FILE_GUID                      = 59C11815-F8DA-4F49-B4FB-EC1E41ED1F06
-  MODULE_TYPE                    = SEC
+  MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
   LIBRARY_CLASS                  = PlatformPeiLib
 
@@ -21,15 +21,21 @@ [Sources]
 [Packages]
   ArmPkg/ArmPkg.dec
   ArmVirtPkg/ArmVirtPkg.dec
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+  OvmfPkg/OvmfPkg.dec
+  SecurityPkg/SecurityPkg.dec
+
+[FeaturePcd]
+  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled
 
 [LibraryClasses]
   DebugLib
   HobLib
   FdtLib
   PcdLib
+  PeiServicesLib
 
 [FixedPcd]
   gArmTokenSpaceGuid.PcdFvSize
@@ -38,6 +44,11 @@ [FixedPcd]
 [Pcd]
   gArmTokenSpaceGuid.PcdFvBaseAddress
   gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress         ## SOMETIMES_PRODUCES
+
+[Ppis]
+  gOvmfTpmDiscoveredPpiGuid                               ## SOMETIMES_PRODUCES
+  gPeiTpmInitializationDonePpiGuid                        ## SOMETIMES_PRODUCES
 
 [Guids]
   gEarlyPL011BaseAddressGuid
-- 
2.17.1

[PATCH v2 4/5] ArmVirtPkg: implement ArmVirtPsciResetSystemPeiLib

[Ard Biesheuvel]

Implement a ArmVirtPkg specific version of the PSCI ResetSystemLib that
is usable in the PEI phase, as the existing one relies on the FDT client
protocol, making it unsuitable.

Note that accessing the device tree passed by QEMU via its initial base
address is guaranteed to be safe at any time during the PEI phase, so we
can defer discovery of the PSCI method until the time the reset library
is actually invoked (which is rarely)

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c   | 232 ++++++++++++++++++++
 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf |  39 ++++
 2 files changed, 271 insertions(+)

diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
new file mode 100644
index 000000000000..394a04e3c384
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
@@ -0,0 +1,232 @@
+/** @file
+  Reset System lib using PSCI hypervisor or secure monitor calls
+
+  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
+  Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
+  Copyright (c) 2014-2020, Linaro Ltd. All rights reserved.<BR>
+  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
+
+  SPDX-License-Identifier: BSD-2-Clause-Patent
+
+**/
+
+#include <PiPei.h>
+
+#include <libfdt.h>
+#include <Library/ArmHvcLib.h>
+#include <Library/ArmSmcLib.h>
+#include <Library/BaseLib.h>
+#include <Library/DebugLib.h>
+#include <Library/HobLib.h>
+#include <Library/ResetSystemLib.h>
+
+#include <IndustryStandard/ArmStdSmc.h>
+
+typedef enum {
+  PsciMethodUnknown,
+  PsciMethodSmc,
+  PsciMethodHvc,
+} PSCI_METHOD;
+
+STATIC
+PSCI_METHOD
+DiscoverPsciMethod (
+  VOID
+  )
+{
+  VOID                            *DeviceTreeBase;
+  INT32                           Node, Prev;
+  INT32                           Len;
+  CONST CHAR8                     *Compatible;
+  CONST CHAR8                     *CompatibleItem;
+  CONST VOID                      *Prop;
+
+  DeviceTreeBase = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
+  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
+
+  //
+  // Enumerate all FDT nodes looking for the PSCI node and capture the method
+  //
+  for (Prev = 0;; Prev = Node) {
+    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
+    if (Node < 0) {
+      break;
+    }
+
+    Compatible = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
+    if (Compatible == NULL) {
+      continue;
+    }
+
+    //
+    // Iterate over the NULL-separated items in the compatible string
+    //
+    for (CompatibleItem = Compatible; CompatibleItem < Compatible + Len;
+      CompatibleItem += 1 + AsciiStrLen (CompatibleItem)) {
+
+      if (AsciiStrCmp (CompatibleItem, "arm,psci-0.2") != 0) {
+        continue;
+      }
+
+      Prop = fdt_getprop (DeviceTreeBase, Node, "method", NULL);
+      if (!Prop) {
+        DEBUG ((DEBUG_ERROR, "%a: Missing PSCI method property\n",
+          __FUNCTION__));
+        return PsciMethodUnknown;
+      }
+
+      if (AsciiStrnCmp (Prop, "hvc", 3) == 0) {
+        return PsciMethodHvc;
+      } else if (AsciiStrnCmp (Prop, "smc", 3) == 0) {
+        return PsciMethodSmc;
+      } else {
+        DEBUG ((DEBUG_ERROR, "%a: Unknown PSCI method \"%a\"\n", __FUNCTION__,
+          Prop));
+        return PsciMethodUnknown;
+      }
+    }
+  }
+  return PsciMethodUnknown;
+}
+
+STATIC
+VOID
+PerformPsciAction (
+  IN  UINTN     Arg0
+  )
+{
+  ARM_SMC_ARGS ArmSmcArgs;
+  ARM_HVC_ARGS ArmHvcArgs;
+
+  ArmSmcArgs.Arg0 = Arg0;
+  ArmHvcArgs.Arg0 = Arg0;
+
+  switch (DiscoverPsciMethod ()) {
+  case PsciMethodHvc:
+    ArmCallHvc (&ArmHvcArgs);
+    break;
+
+  case PsciMethodSmc:
+    ArmCallSmc (&ArmSmcArgs);
+    break;
+
+  default:
+    DEBUG ((DEBUG_ERROR, "%a: no PSCI method defined\n", __FUNCTION__));
+    ASSERT (FALSE);
+  }
+}
+
+/**
+  This function causes a system-wide reset (cold reset), in which
+  all circuitry within the system returns to its initial state. This type of reset
+  is asynchronous to system operation and operates without regard to
+  cycle boundaries.
+
+  If this function returns, it means that the system does not support cold reset.
+**/
+VOID
+EFIAPI
+ResetCold (
+  VOID
+  )
+{
+  // Send a PSCI 0.2 SYSTEM_RESET command
+  PerformPsciAction (ARM_SMC_ID_PSCI_SYSTEM_RESET);
+}
+
+/**
+  This function causes a system-wide initialization (warm reset), in which all processors
+  are set to their initial state. Pending cycles are not corrupted.
+
+  If this function returns, it means that the system does not support warm reset.
+**/
+VOID
+EFIAPI
+ResetWarm (
+  VOID
+  )
+{
+  // Map a warm reset into a cold reset
+  ResetCold ();
+}
+
+/**
+  This function causes the system to enter a power state equivalent
+  to the ACPI G2/S5 or G3 states.
+
+  If this function returns, it means that the system does not support shutdown reset.
+**/
+VOID
+EFIAPI
+ResetShutdown (
+  VOID
+  )
+{
+  // Send a PSCI 0.2 SYSTEM_OFF command
+  PerformPsciAction (ARM_SMC_ID_PSCI_SYSTEM_OFF);
+}
+
+/**
+  This function causes a systemwide reset. The exact type of the reset is
+  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
+  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
+  the platform must pick a supported reset type to perform.The platform may
+  optionally log the parameters from any non-normal reset that occurs.
+
+  @param[in]  DataSize   The size, in bytes, of ResetData.
+  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
+                         followed by the EFI_GUID.
+**/
+VOID
+EFIAPI
+ResetPlatformSpecific (
+  IN UINTN   DataSize,
+  IN VOID    *ResetData
+  )
+{
+  // Map the platform specific reset as reboot
+  ResetCold ();
+}
+
+/**
+  The ResetSystem function resets the entire platform.
+
+  @param[in] ResetType      The type of reset to perform.
+  @param[in] ResetStatus    The status code for the reset.
+  @param[in] DataSize       The size, in bytes, of ResetData.
+  @param[in] ResetData      For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown
+                            the data buffer starts with a Null-terminated string, optionally
+                            followed by additional binary data. The string is a description
+                            that the caller may use to further indicate the reason for the
+                            system reset.
+**/
+VOID
+EFIAPI
+ResetSystem (
+  IN EFI_RESET_TYPE               ResetType,
+  IN EFI_STATUS                   ResetStatus,
+  IN UINTN                        DataSize,
+  IN VOID                         *ResetData OPTIONAL
+  )
+{
+  switch (ResetType) {
+  case EfiResetWarm:
+    ResetWarm ();
+    break;
+
+  case EfiResetCold:
+    ResetCold ();
+    break;
+
+  case EfiResetShutdown:
+    ResetShutdown ();
+    return;
+
+  case EfiResetPlatformSpecific:
+    ResetPlatformSpecific (DataSize, ResetData);
+    return;
+
+  default:
+    return;
+  }
+}
diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
new file mode 100644
index 000000000000..3a65706e8dc6
--- /dev/null
+++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
@@ -0,0 +1,39 @@
+#/** @file
+# Reset System lib using PSCI hypervisor or secure monitor calls
+#
+# Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
+# Copyright (c) 2014-2020, Linaro Ltd. All rights reserved.<BR>
+#
+#  SPDX-License-Identifier: BSD-2-Clause-Patent
+#
+#
+#**/
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = ArmVirtPsciResetSystemPeiLib
+  FILE_GUID                      = 551cfb98-c185-41a3-86bf-8cdb7e2a530c
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = ResetSystemLib|PEIM
+
+[Sources]
+  ArmVirtPsciResetSystemPeiLib.c
+
+[Packages]
+  ArmPkg/ArmPkg.dec
+  ArmVirtPkg/ArmVirtPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+[LibraryClasses]
+  ArmSmcLib
+  ArmHvcLib
+  BaseLib
+  DebugLib
+  FdtLib
+  HobLib
+
+[Pcd]
+  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
-- 
2.17.1

[PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

[Ard Biesheuvel]

Duplicate the TPM2_ENABLE and TPM2_CONFIG_ENABLE build time flags that
already exist in OvmfPkg, and wire them up in the .DSC and .FDF so
that setting those flags produces a ArmVirtQemu build that implements
measured boot using a TPM provided by QEMU and described in the device
tree.

Note that the TPM2 driver stack relies on a PEI phase being implemented,
so there is no point in enabling this for ArmVirtQemuKernel or ArmVirtXen.

Also note that, despite ArmVirtQemuKernel being unaffected by this patch,
ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc is being modified, for keeping the
contexts of the referring !include directives simple.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 ArmVirtPkg/ArmVirtQemu.dsc           | 75 ++++++++++++++++++++
 ArmVirtPkg/ArmVirtQemu.fdf           |  6 ++
 ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 10 +++
 3 files changed, 91 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 7ae6702ac1f0..e8ea711e1a17 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -29,6 +29,8 @@ [Defines]
   #
   DEFINE TTY_TERMINAL            = FALSE
   DEFINE SECURE_BOOT_ENABLE      = FALSE
+  DEFINE TPM2_ENABLE             = FALSE
+  DEFINE TPM2_CONFIG_ENABLE      = FALSE
 
   #
   # Network definition
@@ -74,12 +76,32 @@ [LibraryClasses.common]
   PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
   PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
+  Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
+  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
+!else
+  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
+  TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
+!endif
+
 [LibraryClasses.common.PEIM]
   ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
+  ResetSystemLib|MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
+
+!if $(TPM2_ENABLE) == TRUE
+  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
+  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
+!endif
 
 [LibraryClasses.common.DXE_DRIVER]
   ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
+!endif
+
 [LibraryClasses.common.UEFI_DRIVER]
   UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
 
@@ -100,6 +122,8 @@ [PcdsFeatureFlag.common]
 
   gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
 
+  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE)
+
 [PcdsFixedAtBuild.common]
 !if $(ARCH) == AARCH64
   gArmTokenSpaceGuid.PcdVFPEnabled|1
@@ -237,9 +261,20 @@ [PcdsDynamicDefault.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
   gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
 
+!if $(TPM2_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0
+!endif
+
 [PcdsDynamicHii]
   gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
 
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
+  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
+!endif
+
 ################################################################################
 #
 # Components Section - list of all EDK II Modules needed by this Platform
@@ -261,6 +296,23 @@ [Components.common]
 
   MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf {
+    <LibraryClasses>
+      ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
+  }
+  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
+    <LibraryClasses>
+      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
+  }
+!endif
+
   MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
@@ -295,6 +347,9 @@ [Components.common]
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
     <LibraryClasses>
       NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
+!if $(TPM2_ENABLE) == TRUE
+      NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
+!endif
   }
   SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
   OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
@@ -430,6 +485,26 @@ [Components.common]
   MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
   MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
 
+  #
+  # TPM2 support
+  #
+!if $(TPM2_ENABLE) == TRUE
+  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
+    <LibraryClasses>
+      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
+      Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
+      NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
+      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
+  }
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+!endif
+!endif
+
   #
   # ACPI Support
   #
diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
index 2c8936a1ae15..b5e2253295fe 100644
--- a/ArmVirtPkg/ArmVirtQemu.fdf
+++ b/ArmVirtPkg/ArmVirtQemu.fdf
@@ -113,6 +113,12 @@ [FV.FVMAIN_COMPACT]
   INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
   INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
 
+!if $(TPM2_ENABLE) == TRUE
+  INF MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
+  INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
+  INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
+!endif
+
   FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
     SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
       SECTION FV_IMAGE = FVMAIN
diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
index 31f615a9d0f9..d481e4b2b8fb 100644
--- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
+++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
@@ -182,3 +182,13 @@ [FV.FvMain]
   # Ramdisk support
   #
   INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
+
+  #
+  # TPM2 support
+  #
+!if $(TPM2_ENABLE) == TRUE
+  INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
+!if $(TPM2_CONFIG_ENABLE) == TRUE
+  INF SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
+!endif
+!endif
-- 
2.17.1

Re: [PATCH v2 0/5] ArmVirtPkg: implement measured boot for ArmVirtQemu

[Ard Biesheuvel]

On Tue, 25 Feb 2020 at 11:45, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>
> Wire up the various existing pieces so that we can implement measured
> boot on ArmVirtQemu based on the TPM support in QEMU, just like it has
> been implemented for x86 in OvmfPkg.
>
> The main difference is that on ARM, we first need to discover the TPM base
> address from the device tree provided by QEMU, as well as the PSCI method
> used to perform a cold reset.
>
> Changes since v1:
> - use a separate ResetSystemLib instance based on on-demand parsing of the
>   DT, and expose it via the ResetSystem PPI to other client PEIMs
> - add Laszlo's ack to #1
> - incorporate Laszlo's review feedback across the board
>

Forgot to include the link to the QEMU work being done by Eric in parallel:

https://lists.nongnu.org/archive/html/qemu-devel/2020-02/msg03830.html


> Cc: lersek@redhat.com
> Cc: eric.auger@redhat.com
> Cc: philmd@redhat.com
> Cc: marcandre.lureau@redhat.com
> Cc: stefanb@linux.ibm.com
> Cc: leif@nuviainc.com
>
> Ard Biesheuvel (5):
>   OvmfPkg/Tcg2ConfigPei: introduce a signalling PPI to depex on
>   ArmVirtPkg/PlatformPeiLib: make PcdLib dependency explicit in .INF
>   ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT
>   ArmVirtPkg: implement ArmVirtPsciResetSystemPeiLib
>   ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot
>
>  ArmVirtPkg/ArmVirt.dsc.inc                    |   6 +
>  ArmVirtPkg/ArmVirtPkg.dec                     |   6 +
>  ArmVirtPkg/ArmVirtQemu.dsc                    |  75 ++++++
>  ArmVirtPkg/ArmVirtQemu.fdf                    |   6 +
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc          |  10 +
>  .../ArmVirtPsciResetSystemPeiLib.c            | 232 ++++++++++++++++++
>  .../ArmVirtPsciResetSystemPeiLib.inf          |  39 +++
>  .../Library/PlatformPeiLib/PlatformPeiLib.c   | 101 +++++++-
>  .../Library/PlatformPeiLib/PlatformPeiLib.inf |  20 +-
>  OvmfPkg/OvmfPkg.dec                           |   5 +
>  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf      |   6 +-
>  11 files changed, 491 insertions(+), 15 deletions(-)
>  create mode 100644 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
>  create mode 100644 ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
>
> --
> 2.17.1
>

Re: [edk2-devel] [PATCH v2 2/5] ArmVirtPkg/PlatformPeiLib: make PcdLib dependency explicit in .INF

[Laszlo Ersek]

On 02/25/20 11:44, Ard Biesheuvel wrote:
> We currently include PcdLib.h in PlatformPeiLib, without declaring
> this dependency in its .INF description. Since all the PCDs we use
> resolve to fixed type in practice, this does not really matter at
> the moment, but since we will be adding dynamic PCD references in
> a subsequent patch, let's make the PcdLib dependency explicit, so
> that its dispatch is guaranteed to be ordered correctly with respect
> to the provider of the dynamic PCD PPI.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> index 46db117ac28e..5428040f121d 100644
> --- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> +++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> @@ -29,6 +29,7 @@ [LibraryClasses]
>    DebugLib
>    HobLib
>    FdtLib
> +  PcdLib
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvSize
> 

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

Re: [edk2-devel] [PATCH v2 0/5] ArmVirtPkg: implement measured boot for ArmVirtQemu

[Laszlo Ersek]

On 02/25/20 11:44, Ard Biesheuvel wrote:
> Wire up the various existing pieces so that we can implement measured
> boot on ArmVirtQemu based on the TPM support in QEMU, just like it has
> been implemented for x86 in OvmfPkg.
> 
> The main difference is that on ARM, we first need to discover the TPM base
> address from the device tree provided by QEMU, as well as the PSCI method
> used to perform a cold reset.
> 
> Changes since v1:
> - use a separate ResetSystemLib instance based on on-demand parsing of the
>   DT, and expose it via the ResetSystem PPI to other client PEIMs
> - add Laszlo's ack to #1
> - incorporate Laszlo's review feedback across the board

Can you please file a new tianocore feature request BZ for this, and hook the BZ ref into all of the commit messages?

Also, in the BZ, can you please reference both versions posted thus far:

* [edk2-devel] [PATCH 0/4]
  ArmVirtPkg: implement measured boot for ArmVirtQemu

  https://edk2.groups.io/g/devel/message/52972
  http://mid.mail-archive.com/20200107094800.4488-1-ard.biesheuvel@linaro.org

* [edk2-devel] [PATCH v2 0/5]
  ArmVirtPkg: implement measured boot for ArmVirtQemu

  https://edk2.groups.io/g/devel/message/54779
  http://mid.mail-archive.com/20200225104449.22453-1-ard.biesheuvel@linaro.org

Thanks!
Laszlo

Re: [edk2-devel] [PATCH v2 3/5] ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT

[Laszlo Ersek]

On 02/25/20 11:44, Ard Biesheuvel wrote:
> Introduce a boolean PCD that tells us whether TPM support is enabled
> in the build, and if it is, record the TPM base address in the existing
> routine that traverses the device tree in the platform PEIM.
> 
> If a TPM is found, install the gOvmfTpmDiscoveredPpiGuid signalling PPI
> that will unlock the dispatch of OvmfPkg's Tcg2ConfigPei. If TPM2
> support is enabled in the build but no TPM2 device is found, install the
> gPeiTpmInitializationDonePpiGuid PPI, which is normally installed by
> Tcg2ConfigPei if no TPM2 is found, but in our case Tcg2ConfigPei will
> never run so let's do it here instead.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirt.dsc.inc                           |   6 ++
>  ArmVirtPkg/ArmVirtPkg.dec                            |   6 ++
>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c   | 101 ++++++++++++++++++--
>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf |  19 +++-
>  4 files changed, 118 insertions(+), 14 deletions(-)
> 
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index 10037c938eb8..abb253fdf76a 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -366,6 +366,12 @@ [PcdsFixedAtBuild.common]
>    #
>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>  
> +[PcdsPatchableInModule]
> +  # we need a default resolution for this PCD that supports PcdSet64(),
> +  # even though any actual calls will be compiled out on builds that have
> +  # gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled == FALSE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
> +
>  [Components.common]
>    #
>    # Ramdisk support

I don't understand why this is patchable-in-module, and not dynamic. I
feel like it's a "textbook case" of a dynamic PCD. What am I missing?

Otherwise, I'm ready to give Acked-by.

Thanks!
Laszlo

> diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec
> index a019cc269d10..08ddd68a863e 100644
> --- a/ArmVirtPkg/ArmVirtPkg.dec
> +++ b/ArmVirtPkg/ArmVirtPkg.dec
> @@ -36,6 +36,12 @@ [Guids.common]
>  [Protocols]
>    gFdtClientProtocolGuid = { 0xE11FACA0, 0x4710, 0x4C8E, { 0xA7, 0xA2, 0x01, 0xBA, 0xA2, 0x59, 0x1B, 0x4C } }
>  
> +[PcdsFeatureFlag]
> +  #
> +  # Feature Flag PCD that defines whether TPM2 support is enabled
> +  #
> +  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|FALSE|BOOLEAN|0x00000004
> +
>  [PcdsFixedAtBuild, PcdsPatchableInModule]
>    #
>    # This is the physical address where the device tree is expected to be stored
> diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
> index 0a1469550db0..8b5b3dd5dc1c 100644
> --- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
> +++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c
> @@ -1,7 +1,7 @@
>  /** @file
>  *
>  *  Copyright (c) 2011-2014, ARM Limited. All rights reserved.
> -*  Copyright (c) 2014, Linaro Limited. All rights reserved.
> +*  Copyright (c) 2014-2020, Linaro Limited. All rights reserved.
>  *
>  *  SPDX-License-Identifier: BSD-2-Clause-Patent
>  *
> @@ -13,11 +13,24 @@
>  #include <Library/DebugLib.h>
>  #include <Library/HobLib.h>
>  #include <Library/PcdLib.h>
> +#include <Library/PeiServicesLib.h>
>  #include <libfdt.h>
>  
>  #include <Guid/EarlyPL011BaseAddress.h>
>  #include <Guid/FdtHob.h>
>  
> +STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2DiscoveredPpi = {
> +  EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> +  &gOvmfTpmDiscoveredPpiGuid,
> +  NULL
> +};
> +
> +STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpm2InitializationDonePpi = {
> +  EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST,
> +  &gPeiTpmInitializationDonePpiGuid,
> +  NULL
> +};
> +
>  EFI_STATUS
>  EFIAPI
>  PlatformPeim (
> @@ -31,14 +44,18 @@ PlatformPeim (
>    UINT64             *FdtHobData;
>    UINT64             *UartHobData;
>    INT32              Node, Prev;
> +  INT32              Parent, Depth;
>    CONST CHAR8        *Compatible;
>    CONST CHAR8        *CompItem;
>    CONST CHAR8        *NodeStatus;
>    INT32              Len;
> +  INT32              RangesLen;
>    INT32              StatusLen;
>    CONST UINT64       *RegProp;
> +  CONST UINT32       *RangesProp;
>    UINT64             UartBase;
> -
> +  UINT64             TpmBase;
> +  EFI_STATUS         Status;
>  
>    Base = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
>    ASSERT (Base != NULL);
> @@ -58,18 +75,18 @@ PlatformPeim (
>    ASSERT (UartHobData != NULL);
>    *UartHobData = 0;
>  
> -  //
> -  // Look for a UART node
> -  //
> -  for (Prev = 0;; Prev = Node) {
> -    Node = fdt_next_node (Base, Prev, NULL);
> +  TpmBase = 0;
> +
> +  for (Prev = Depth = 0;; Prev = Node) {
> +    Node = fdt_next_node (Base, Prev, &Depth);
>      if (Node < 0) {
>        break;
>      }
>  
> -    //
> -    // Check for UART node
> -    //
> +    if (Depth == 1) {
> +      Parent = Node;
> +    }
> +
>      Compatible = fdt_getprop (Base, Node, "compatible", &Len);
>  
>      //
> @@ -93,10 +110,74 @@ PlatformPeim (
>  
>          *UartHobData = UartBase;
>          break;
> +      } else if (FeaturePcdGet (PcdTpm2SupportEnabled) &&
> +                 AsciiStrCmp (CompItem, "tcg,tpm-tis-mmio") == 0) {
> +
> +        RegProp = fdt_getprop (Base, Node, "reg", &Len);
> +        ASSERT (Len == 8 || Len == 16);
> +        if (Len == 8) {
> +          TpmBase = fdt32_to_cpu (RegProp[0]);
> +        } else if (Len == 16) {
> +          TpmBase = fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RegProp));
> +        }
> +
> +        if (Depth > 1) {
> +          //
> +          // QEMU/mach-virt may put the TPM on the platform bus, in which case
> +          // we have to take its 'ranges' property into account to translate the
> +          // MMIO address. This consists of a <child base, parent base, size>
> +          // tuple, where the child base and the size use the same number of
> +          // cells as the 'reg' property above, and the parent base uses 2 cells
> +          //
> +          RangesProp = fdt_getprop (Base, Parent, "ranges", &RangesLen);
> +          ASSERT (RangesProp != NULL);
> +
> +          //
> +          // a plain 'ranges' attribute without a value implies a 1:1 mapping
> +          //
> +          if (RangesLen != 0) {
> +            //
> +            // assume a single translated range with 2 cells for the parent base
> +            //
> +            if (RangesLen != Len + 2 * sizeof (UINT32)) {
> +              DEBUG ((DEBUG_WARN,
> +                "%a: 'ranges' property has unexpected size %d\n",
> +                __FUNCTION__, RangesLen));
> +              break;
> +            }
> +
> +            if (Len == 8) {
> +              TpmBase -= fdt32_to_cpu (RangesProp[0]);
> +            } else {
> +              TpmBase -= fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RangesProp));
> +            }
> +
> +            //
> +            // advance RangesProp to the parent bus address
> +            //
> +            RangesProp = (UINT32 *)((UINT8 *)RangesProp + Len / 2);
> +            TpmBase += fdt64_to_cpu (ReadUnaligned64 ((UINT64 *)RangesProp));
> +          }
> +        }
> +        break;
>        }
>      }
>    }
>  
> +  if (FeaturePcdGet (PcdTpm2SupportEnabled)) {
> +    if (TpmBase != 0) {
> +      DEBUG ((DEBUG_INFO, "%a: TPM @ 0x%lx\n", __FUNCTION__, TpmBase));
> +
> +      Status = (EFI_STATUS)PcdSet64S (PcdTpmBaseAddress, TpmBase);
> +      ASSERT_EFI_ERROR (Status);
> +
> +      Status = PeiServicesInstallPpi (&mTpm2DiscoveredPpi);
> +    } else {
> +      Status = PeiServicesInstallPpi (&mTpm2InitializationDonePpi);
> +    }
> +    ASSERT_EFI_ERROR (Status);
> +  }
> +
>    BuildFvHob (PcdGet64 (PcdFvBaseAddress), PcdGet32 (PcdFvSize));
>  
>    return EFI_SUCCESS;
> diff --git a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> index 5428040f121d..3f97ef080520 100644
> --- a/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> +++ b/ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf
> @@ -1,7 +1,7 @@
>  #/** @file
>  #
>  #  Copyright (c) 2011-2015, ARM Limited. All rights reserved.
> -#  Copyright (c) 2014, Linaro Limited. All rights reserved.
> +#  Copyright (c) 2014-2020, Linaro Limited. All rights reserved.
>  #
>  #  SPDX-License-Identifier: BSD-2-Clause-Patent
>  #
> @@ -11,7 +11,7 @@ [Defines]
>    INF_VERSION                    = 0x00010005
>    BASE_NAME                      = PlatformPeiLib
>    FILE_GUID                      = 59C11815-F8DA-4F49-B4FB-EC1E41ED1F06
> -  MODULE_TYPE                    = SEC
> +  MODULE_TYPE                    = BASE
>    VERSION_STRING                 = 1.0
>    LIBRARY_CLASS                  = PlatformPeiLib
>  
> @@ -21,15 +21,21 @@ [Sources]
>  [Packages]
>    ArmPkg/ArmPkg.dec
>    ArmVirtPkg/ArmVirtPkg.dec
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +  OvmfPkg/OvmfPkg.dec
> +  SecurityPkg/SecurityPkg.dec
> +
> +[FeaturePcd]
> +  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled
>  
>  [LibraryClasses]
>    DebugLib
>    HobLib
>    FdtLib
>    PcdLib
> +  PeiServicesLib
>  
>  [FixedPcd]
>    gArmTokenSpaceGuid.PcdFvSize
> @@ -38,6 +44,11 @@ [FixedPcd]
>  [Pcd]
>    gArmTokenSpaceGuid.PcdFvBaseAddress
>    gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress         ## SOMETIMES_PRODUCES
> +
> +[Ppis]
> +  gOvmfTpmDiscoveredPpiGuid                               ## SOMETIMES_PRODUCES
> +  gPeiTpmInitializationDonePpiGuid                        ## SOMETIMES_PRODUCES
>  
>  [Guids]
>    gEarlyPL011BaseAddressGuid
> 

Re: [edk2-devel] [PATCH v2 4/5] ArmVirtPkg: implement ArmVirtPsciResetSystemPeiLib

[Laszlo Ersek]

On 02/25/20 11:44, Ard Biesheuvel wrote:
> Implement a ArmVirtPkg specific version of the PSCI ResetSystemLib that
> is usable in the PEI phase, as the existing one relies on the FDT client
> protocol, making it unsuitable.
> 
> Note that accessing the device tree passed by QEMU via its initial base
> address is guaranteed to be safe at any time during the PEI phase, so we
> can defer discovery of the PSCI method until the time the reset library
> is actually invoked (which is rarely)
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c   | 232 ++++++++++++++++++++
>  ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf |  39 ++++
>  2 files changed, 271 insertions(+)
> 
> diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
> new file mode 100644
> index 000000000000..394a04e3c384
> --- /dev/null
> +++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.c
> @@ -0,0 +1,232 @@
> +/** @file
> +  Reset System lib using PSCI hypervisor or secure monitor calls
> +
> +  Copyright (c) 2008 - 2009, Apple Inc. All rights reserved.<BR>
> +  Copyright (c) 2013, ARM Ltd. All rights reserved.<BR>
> +  Copyright (c) 2014-2020, Linaro Ltd. All rights reserved.<BR>
> +  Copyright (c) 2019, Intel Corporation. All rights reserved.<BR>
> +
> +  SPDX-License-Identifier: BSD-2-Clause-Patent
> +
> +**/
> +
> +#include <PiPei.h>
> +
> +#include <libfdt.h>
> +#include <Library/ArmHvcLib.h>
> +#include <Library/ArmSmcLib.h>
> +#include <Library/BaseLib.h>
> +#include <Library/DebugLib.h>
> +#include <Library/HobLib.h>
> +#include <Library/ResetSystemLib.h>
> +
> +#include <IndustryStandard/ArmStdSmc.h>
> +
> +typedef enum {
> +  PsciMethodUnknown,
> +  PsciMethodSmc,
> +  PsciMethodHvc,
> +} PSCI_METHOD;
> +
> +STATIC
> +PSCI_METHOD
> +DiscoverPsciMethod (
> +  VOID
> +  )
> +{
> +  VOID                            *DeviceTreeBase;
> +  INT32                           Node, Prev;
> +  INT32                           Len;
> +  CONST CHAR8                     *Compatible;
> +  CONST CHAR8                     *CompatibleItem;
> +  CONST VOID                      *Prop;
> +
> +  DeviceTreeBase = (VOID*)(UINTN)PcdGet64 (PcdDeviceTreeInitialBaseAddress);
> +  ASSERT (fdt_check_header (DeviceTreeBase) == 0);
> +
> +  //
> +  // Enumerate all FDT nodes looking for the PSCI node and capture the method
> +  //
> +  for (Prev = 0;; Prev = Node) {
> +    Node = fdt_next_node (DeviceTreeBase, Prev, NULL);
> +    if (Node < 0) {
> +      break;
> +    }
> +
> +    Compatible = fdt_getprop (DeviceTreeBase, Node, "compatible", &Len);
> +    if (Compatible == NULL) {
> +      continue;
> +    }
> +
> +    //
> +    // Iterate over the NULL-separated items in the compatible string
> +    //
> +    for (CompatibleItem = Compatible; CompatibleItem < Compatible + Len;
> +      CompatibleItem += 1 + AsciiStrLen (CompatibleItem)) {
> +
> +      if (AsciiStrCmp (CompatibleItem, "arm,psci-0.2") != 0) {
> +        continue;
> +      }
> +
> +      Prop = fdt_getprop (DeviceTreeBase, Node, "method", NULL);
> +      if (!Prop) {
> +        DEBUG ((DEBUG_ERROR, "%a: Missing PSCI method property\n",
> +          __FUNCTION__));
> +        return PsciMethodUnknown;
> +      }
> +
> +      if (AsciiStrnCmp (Prop, "hvc", 3) == 0) {
> +        return PsciMethodHvc;
> +      } else if (AsciiStrnCmp (Prop, "smc", 3) == 0) {
> +        return PsciMethodSmc;
> +      } else {
> +        DEBUG ((DEBUG_ERROR, "%a: Unknown PSCI method \"%a\"\n", __FUNCTION__,
> +          Prop));
> +        return PsciMethodUnknown;
> +      }
> +    }
> +  }
> +  return PsciMethodUnknown;
> +}
> +
> +STATIC
> +VOID
> +PerformPsciAction (
> +  IN  UINTN     Arg0
> +  )
> +{
> +  ARM_SMC_ARGS ArmSmcArgs;
> +  ARM_HVC_ARGS ArmHvcArgs;
> +
> +  ArmSmcArgs.Arg0 = Arg0;
> +  ArmHvcArgs.Arg0 = Arg0;
> +
> +  switch (DiscoverPsciMethod ()) {
> +  case PsciMethodHvc:
> +    ArmCallHvc (&ArmHvcArgs);
> +    break;
> +
> +  case PsciMethodSmc:
> +    ArmCallSmc (&ArmSmcArgs);
> +    break;
> +
> +  default:
> +    DEBUG ((DEBUG_ERROR, "%a: no PSCI method defined\n", __FUNCTION__));
> +    ASSERT (FALSE);
> +  }
> +}
> +
> +/**
> +  This function causes a system-wide reset (cold reset), in which
> +  all circuitry within the system returns to its initial state. This type of reset
> +  is asynchronous to system operation and operates without regard to
> +  cycle boundaries.
> +
> +  If this function returns, it means that the system does not support cold reset.
> +**/
> +VOID
> +EFIAPI
> +ResetCold (
> +  VOID
> +  )
> +{
> +  // Send a PSCI 0.2 SYSTEM_RESET command
> +  PerformPsciAction (ARM_SMC_ID_PSCI_SYSTEM_RESET);
> +}
> +
> +/**
> +  This function causes a system-wide initialization (warm reset), in which all processors
> +  are set to their initial state. Pending cycles are not corrupted.
> +
> +  If this function returns, it means that the system does not support warm reset.
> +**/
> +VOID
> +EFIAPI
> +ResetWarm (
> +  VOID
> +  )
> +{
> +  // Map a warm reset into a cold reset
> +  ResetCold ();
> +}
> +
> +/**
> +  This function causes the system to enter a power state equivalent
> +  to the ACPI G2/S5 or G3 states.
> +
> +  If this function returns, it means that the system does not support shutdown reset.
> +**/
> +VOID
> +EFIAPI
> +ResetShutdown (
> +  VOID
> +  )
> +{
> +  // Send a PSCI 0.2 SYSTEM_OFF command
> +  PerformPsciAction (ARM_SMC_ID_PSCI_SYSTEM_OFF);
> +}
> +
> +/**
> +  This function causes a systemwide reset. The exact type of the reset is
> +  defined by the EFI_GUID that follows the Null-terminated Unicode string passed
> +  into ResetData. If the platform does not recognize the EFI_GUID in ResetData
> +  the platform must pick a supported reset type to perform.The platform may
> +  optionally log the parameters from any non-normal reset that occurs.
> +
> +  @param[in]  DataSize   The size, in bytes, of ResetData.
> +  @param[in]  ResetData  The data buffer starts with a Null-terminated string,
> +                         followed by the EFI_GUID.
> +**/
> +VOID
> +EFIAPI
> +ResetPlatformSpecific (
> +  IN UINTN   DataSize,
> +  IN VOID    *ResetData
> +  )
> +{
> +  // Map the platform specific reset as reboot
> +  ResetCold ();
> +}
> +
> +/**
> +  The ResetSystem function resets the entire platform.
> +
> +  @param[in] ResetType      The type of reset to perform.
> +  @param[in] ResetStatus    The status code for the reset.
> +  @param[in] DataSize       The size, in bytes, of ResetData.
> +  @param[in] ResetData      For a ResetType of EfiResetCold, EfiResetWarm, or EfiResetShutdown
> +                            the data buffer starts with a Null-terminated string, optionally
> +                            followed by additional binary data. The string is a description
> +                            that the caller may use to further indicate the reason for the
> +                            system reset.
> +**/
> +VOID
> +EFIAPI
> +ResetSystem (
> +  IN EFI_RESET_TYPE               ResetType,
> +  IN EFI_STATUS                   ResetStatus,
> +  IN UINTN                        DataSize,
> +  IN VOID                         *ResetData OPTIONAL
> +  )
> +{
> +  switch (ResetType) {
> +  case EfiResetWarm:
> +    ResetWarm ();
> +    break;
> +
> +  case EfiResetCold:
> +    ResetCold ();
> +    break;
> +
> +  case EfiResetShutdown:
> +    ResetShutdown ();
> +    return;
> +
> +  case EfiResetPlatformSpecific:
> +    ResetPlatformSpecific (DataSize, ResetData);
> +    return;
> +
> +  default:
> +    return;
> +  }
> +}
> diff --git a/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
> new file mode 100644
> index 000000000000..3a65706e8dc6
> --- /dev/null
> +++ b/ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
> @@ -0,0 +1,39 @@
> +#/** @file
> +# Reset System lib using PSCI hypervisor or secure monitor calls
> +#
> +# Copyright (c) 2008, Apple Inc. All rights reserved.<BR>
> +# Copyright (c) 2014-2020, Linaro Ltd. All rights reserved.<BR>
> +#
> +#  SPDX-License-Identifier: BSD-2-Clause-Patent
> +#
> +#
> +#**/
> +
> +[Defines]
> +  INF_VERSION                    = 1.27
> +  BASE_NAME                      = ArmVirtPsciResetSystemPeiLib
> +  FILE_GUID                      = 551cfb98-c185-41a3-86bf-8cdb7e2a530c
> +  MODULE_TYPE                    = BASE
> +  VERSION_STRING                 = 1.0
> +  LIBRARY_CLASS                  = ResetSystemLib|PEIM
> +
> +[Sources]
> +  ArmVirtPsciResetSystemPeiLib.c
> +
> +[Packages]
> +  ArmPkg/ArmPkg.dec
> +  ArmVirtPkg/ArmVirtPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> +  MdePkg/MdePkg.dec
> +
> +[LibraryClasses]
> +  ArmSmcLib
> +  ArmHvcLib
> +  BaseLib
> +  DebugLib
> +  FdtLib
> +  HobLib
> +
> +[Pcd]
> +  gArmVirtTokenSpaceGuid.PcdDeviceTreeInitialBaseAddress
> 

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

Re: [edk2-devel] [PATCH v2 3/5] ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT

[Laszlo Ersek]

On 02/26/20 01:24, Laszlo Ersek wrote:
> On 02/25/20 11:44, Ard Biesheuvel wrote:
>> Introduce a boolean PCD that tells us whether TPM support is enabled
>> in the build, and if it is, record the TPM base address in the existing
>> routine that traverses the device tree in the platform PEIM.
>>
>> If a TPM is found, install the gOvmfTpmDiscoveredPpiGuid signalling PPI
>> that will unlock the dispatch of OvmfPkg's Tcg2ConfigPei. If TPM2
>> support is enabled in the build but no TPM2 device is found, install the
>> gPeiTpmInitializationDonePpiGuid PPI, which is normally installed by
>> Tcg2ConfigPei if no TPM2 is found, but in our case Tcg2ConfigPei will
>> never run so let's do it here instead.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirt.dsc.inc                           |   6 ++
>>  ArmVirtPkg/ArmVirtPkg.dec                            |   6 ++
>>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c   | 101 ++++++++++++++++++--
>>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf |  19 +++-
>>  4 files changed, 118 insertions(+), 14 deletions(-)
>>
>> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
>> index 10037c938eb8..abb253fdf76a 100644
>> --- a/ArmVirtPkg/ArmVirt.dsc.inc
>> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
>> @@ -366,6 +366,12 @@ [PcdsFixedAtBuild.common]
>>    #
>>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>>  
>> +[PcdsPatchableInModule]
>> +  # we need a default resolution for this PCD that supports PcdSet64(),
>> +  # even though any actual calls will be compiled out on builds that have
>> +  # gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled == FALSE
>> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
>> +
>>  [Components.common]
>>    #
>>    # Ramdisk support
> 
> I don't understand why this is patchable-in-module, and not dynamic. I
> feel like it's a "textbook case" of a dynamic PCD. What am I missing?

I've found the dynamic default in patch v2 5/5, in ArmVirtQemu.dsc, so I
guess this is for all *other* ArmVirt platforms. Is that correct?

Can we add the PatchPcd to the other DSC files then (ArmVirtQemuKernel
and ArmVirtXen)? I'm a bit uncomfortable with ArmVirtQemu.dsc describing
two access methods for the same PCD, even though (apparently) the
dynamic one will take effect (?)

Thanks
Laszlo

Re: [edk2-devel] [PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

[Laszlo Ersek]

On 02/25/20 11:44, Ard Biesheuvel wrote:
> Duplicate the TPM2_ENABLE and TPM2_CONFIG_ENABLE build time flags that
> already exist in OvmfPkg, and wire them up in the .DSC and .FDF so
> that setting those flags produces a ArmVirtQemu build that implements
> measured boot using a TPM provided by QEMU and described in the device
> tree.
> 
> Note that the TPM2 driver stack relies on a PEI phase being implemented,
> so there is no point in enabling this for ArmVirtQemuKernel or ArmVirtXen.
> 
> Also note that, despite ArmVirtQemuKernel being unaffected by this patch,
> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc is being modified, for keeping the
> contexts of the referring !include directives simple.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>  ArmVirtPkg/ArmVirtQemu.dsc           | 75 ++++++++++++++++++++
>  ArmVirtPkg/ArmVirtQemu.fdf           |  6 ++
>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 10 +++
>  3 files changed, 91 insertions(+)

Under a similar, recent patch from Marc-André (which proposes enabling
TPM-1.2 in OvmfPkg), I asked Marc-André to build up the work in small
steps, practically mirroring the gradual TPM2.0 stuff from OvmfPkg:

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

http://mid.mail-archive.com/bbf8cf87-9c90-5507-82b3-ae8534555a54@redhat.com

https://edk2.groups.io/g/devel/message/54473

I'd like to be consistent as a review (and I indeed prefer that
approach), so I'd like to ask you for the same.

Now if you and Marc-André agree that I'm being unreasonable, I guess I
could be convinced... I don't want to annoy patch authors needlessly (I
just find small gradual steps easier to understand, later).

(Extra apologies if my current request contradicts something I asked for
in the v1 review -- please do point it out, if that's the case. I'd like
to be responsive and consistent, but there's just too much to re-review,
even incrementally. I can easily see myself making process mistakes
here, due to fatigue.)

Thanks
Laszlo

> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 7ae6702ac1f0..e8ea711e1a17 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -29,6 +29,8 @@ [Defines]
>    #
>    DEFINE TTY_TERMINAL            = FALSE
>    DEFINE SECURE_BOOT_ENABLE      = FALSE
> +  DEFINE TPM2_ENABLE             = FALSE
> +  DEFINE TPM2_CONFIG_ENABLE      = FALSE
>  
>    #
>    # Network definition
> @@ -74,12 +76,32 @@ [LibraryClasses.common]
>    PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci.inf
>    PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBridgeLib.inf
>  
> +!if $(TPM2_ENABLE) == TRUE
> +  Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/DxeTcg2PhysicalPresenceLib.inf
> +  Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.inf
> +  TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasurementLib.inf
> +!else
> +  Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/DxeTcg2PhysicalPresenceLib.inf
> +  TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasurementLibNull.inf
> +!endif
> +
>  [LibraryClasses.common.PEIM]
>    ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInfoPeiLib.inf
> +  ResetSystemLib|MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLib.inf
> +
> +!if $(TPM2_ENABLE) == TRUE
> +  BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf
> +  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.inf
> +!endif
>  
>  [LibraryClasses.common.DXE_DRIVER]
>    ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf
>  
> +!if $(TPM2_ENABLE) == TRUE
> +  Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf
> +!endif
> +
>  [LibraryClasses.common.UEFI_DRIVER]
>    UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf
>  
> @@ -100,6 +122,8 @@ [PcdsFeatureFlag.common]
>  
>    gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE
>  
> +  gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE)
> +
>  [PcdsFixedAtBuild.common]
>  !if $(ARCH) == AARCH64
>    gArmTokenSpaceGuid.PcdVFPEnabled|1
> @@ -237,9 +261,20 @@ [PcdsDynamicDefault.common]
>    gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0
>    gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE
>  
> +!if $(TPM2_ENABLE) == TRUE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0
> +!endif
> +
>  [PcdsDynamicHii]
>    gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableGuid|0x0|FALSE|NV,BS
>  
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS
> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg2ConfigFormSetGuid|0x8|3|NV,BS
> +!endif
> +
>  ################################################################################
>  #
>  # Components Section - list of all EDK II Modules needed by this Platform
> @@ -261,6 +296,23 @@ [Components.common]
>  
>    MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>  
> +!if $(TPM2_ENABLE) == TRUE
> +  MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf {
> +    <LibraryClasses>
> +      ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/ArmVirtPsciResetSystemPeiLib.inf
> +  }
> +  OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +  SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf {
> +    <LibraryClasses>
> +      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterPei.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> +  }
> +!endif
> +
>    MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecompressLib.inf
> @@ -295,6 +347,9 @@ [Components.common]
>    MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
>      <LibraryClasses>
>        NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificationLib.inf
> +!if $(TPM2_ENABLE) == TRUE
> +      NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootLib.inf
> +!endif
>    }
>    SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfigDxe.inf
>    OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf
> @@ -430,6 +485,26 @@ [Components.common]
>    MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf
>    MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf
>  
> +  #
> +  # TPM2 support
> +  #
> +!if $(TPM2_ENABLE) == TRUE
> +  SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf {
> +    <LibraryClasses>
> +      HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCryptoRouterDxe.inf
> +      Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf
> +      NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha256.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha384.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha512.inf
> +      NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf
> +  }
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +!endif
> +!endif
> +
>    #
>    # ACPI Support
>    #
> diff --git a/ArmVirtPkg/ArmVirtQemu.fdf b/ArmVirtPkg/ArmVirtQemu.fdf
> index 2c8936a1ae15..b5e2253295fe 100644
> --- a/ArmVirtPkg/ArmVirtQemu.fdf
> +++ b/ArmVirtPkg/ArmVirtQemu.fdf
> @@ -113,6 +113,12 @@ [FV.FVMAIN_COMPACT]
>    INF MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
>    INF MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf
>  
> +!if $(TPM2_ENABLE) == TRUE
> +  INF MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf
> +  INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf
> +  INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf
> +!endif
> +
>    FILE FV_IMAGE = 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 {
>      SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUIRED = TRUE {
>        SECTION FV_IMAGE = FVMAIN
> diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> index 31f615a9d0f9..d481e4b2b8fb 100644
> --- a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> +++ b/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc
> @@ -182,3 +182,13 @@ [FV.FvMain]
>    # Ramdisk support
>    #
>    INF MdeModulePkg/Universal/Disk/RamDiskDxe/RamDiskDxe.inf
> +
> +  #
> +  # TPM2 support
> +  #
> +!if $(TPM2_ENABLE) == TRUE
> +  INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf
> +!if $(TPM2_CONFIG_ENABLE) == TRUE
> +  INF SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf
> +!endif
> +!endif
> 

Re: [edk2-devel] [PATCH v2 3/5] ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT

[Ard Biesheuvel]

On Wed, 26 Feb 2020 at 01:31, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/26/20 01:24, Laszlo Ersek wrote:
> > On 02/25/20 11:44, Ard Biesheuvel wrote:
> >> Introduce a boolean PCD that tells us whether TPM support is enabled
> >> in the build, and if it is, record the TPM base address in the existing
> >> routine that traverses the device tree in the platform PEIM.
> >>
> >> If a TPM is found, install the gOvmfTpmDiscoveredPpiGuid signalling PPI
> >> that will unlock the dispatch of OvmfPkg's Tcg2ConfigPei. If TPM2
> >> support is enabled in the build but no TPM2 device is found, install the
> >> gPeiTpmInitializationDonePpiGuid PPI, which is normally installed by
> >> Tcg2ConfigPei if no TPM2 is found, but in our case Tcg2ConfigPei will
> >> never run so let's do it here instead.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmVirtPkg/ArmVirt.dsc.inc                           |   6 ++
> >>  ArmVirtPkg/ArmVirtPkg.dec                            |   6 ++
> >>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.c   | 101 ++++++++++++++++++--
> >>  ArmVirtPkg/Library/PlatformPeiLib/PlatformPeiLib.inf |  19 +++-
> >>  4 files changed, 118 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> >> index 10037c938eb8..abb253fdf76a 100644
> >> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> >> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> >> @@ -366,6 +366,12 @@ [PcdsFixedAtBuild.common]
> >>    #
> >>    gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> >>
> >> +[PcdsPatchableInModule]
> >> +  # we need a default resolution for this PCD that supports PcdSet64(),
> >> +  # even though any actual calls will be compiled out on builds that have
> >> +  # gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled == FALSE
> >> +  gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0
> >> +
> >>  [Components.common]
> >>    #
> >>    # Ramdisk support
> >
> > I don't understand why this is patchable-in-module, and not dynamic. I
> > feel like it's a "textbook case" of a dynamic PCD. What am I missing?
>
> I've found the dynamic default in patch v2 5/5, in ArmVirtQemu.dsc, so I
> guess this is for all *other* ArmVirt platforms. Is that correct?
>

Yes.

> Can we add the PatchPcd to the other DSC files then (ArmVirtQemuKernel
> and ArmVirtXen)? I'm a bit uncomfortable with ArmVirtQemu.dsc describing
> two access methods for the same PCD, even though (apparently) the
> dynamic one will take effect (?)
>

Sure, that works for me.

Re: [edk2-devel] [PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

[Ard Biesheuvel]

On Wed, 26 Feb 2020 at 01:40, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/25/20 11:44, Ard Biesheuvel wrote:
> > Duplicate the TPM2_ENABLE and TPM2_CONFIG_ENABLE build time flags that
> > already exist in OvmfPkg, and wire them up in the .DSC and .FDF so
> > that setting those flags produces a ArmVirtQemu build that implements
> > measured boot using a TPM provided by QEMU and described in the device
> > tree.
> >
> > Note that the TPM2 driver stack relies on a PEI phase being implemented,
> > so there is no point in enabling this for ArmVirtQemuKernel or ArmVirtXen.
> >
> > Also note that, despite ArmVirtQemuKernel being unaffected by this patch,
> > ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc is being modified, for keeping the
> > contexts of the referring !include directives simple.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> >  ArmVirtPkg/ArmVirtQemu.dsc           | 75 ++++++++++++++++++++
> >  ArmVirtPkg/ArmVirtQemu.fdf           |  6 ++
> >  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 10 +++
> >  3 files changed, 91 insertions(+)
>
> Under a similar, recent patch from Marc-André (which proposes enabling
> TPM-1.2 in OvmfPkg), I asked Marc-André to build up the work in small
> steps, practically mirroring the gradual TPM2.0 stuff from OvmfPkg:
>
> * [edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support
>
> http://mid.mail-archive.com/bbf8cf87-9c90-5507-82b3-ae8534555a54@redhat.com
>
> https://edk2.groups.io/g/devel/message/54473
>
> I'd like to be consistent as a review (and I indeed prefer that
> approach), so I'd like to ask you for the same.
>
> Now if you and Marc-André agree that I'm being unreasonable, I guess I
> could be convinced... I don't want to annoy patch authors needlessly (I
> just find small gradual steps easier to understand, later).
>
> (Extra apologies if my current request contradicts something I asked for
> in the v1 review -- please do point it out, if that's the case. I'd like
> to be responsive and consistent, but there's just too much to re-review,
> even incrementally. I can easily see myself making process mistakes
> here, due to fatigue.)
>

I don't mind per se, but I'm not sure I understand how you want this
to be split.

- library classes first, then PCDs, then components.
- PEI modules first, then DXE modules
- TPM2_ENABLE pieces first, then TPM2_CONFIG_ENABLE.

Re: [edk2-devel] [PATCH v2 0/5] ArmVirtPkg: implement measured boot for ArmVirtQemu

[Ard Biesheuvel]

On Wed, 26 Feb 2020 at 01:17, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/25/20 11:44, Ard Biesheuvel wrote:
> > Wire up the various existing pieces so that we can implement measured
> > boot on ArmVirtQemu based on the TPM support in QEMU, just like it has
> > been implemented for x86 in OvmfPkg.
> >
> > The main difference is that on ARM, we first need to discover the TPM base
> > address from the device tree provided by QEMU, as well as the PSCI method
> > used to perform a cold reset.
> >
> > Changes since v1:
> > - use a separate ResetSystemLib instance based on on-demand parsing of the
> >   DT, and expose it via the ResetSystem PPI to other client PEIMs
> > - add Laszlo's ack to #1
> > - incorporate Laszlo's review feedback across the board
>
> Can you please file a new tianocore feature request BZ for this, and hook the BZ ref into all of the commit messages?
>
> Also, in the BZ, can you please reference both versions posted thus far:
>
> * [edk2-devel] [PATCH 0/4]
>   ArmVirtPkg: implement measured boot for ArmVirtQemu
>
>   https://edk2.groups.io/g/devel/message/52972
>   http://mid.mail-archive.com/20200107094800.4488-1-ard.biesheuvel@linaro.org
>
> * [edk2-devel] [PATCH v2 0/5]
>   ArmVirtPkg: implement measured boot for ArmVirtQemu
>
>   https://edk2.groups.io/g/devel/message/54779
>   http://mid.mail-archive.com/20200225104449.22453-1-ard.biesheuvel@linaro.org
>

Done!

https://bugzilla.tianocore.org/show_bug.cgi?id=2560

Re: [edk2-devel] [PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

[Laszlo Ersek]

On 02/26/20 01:40, Laszlo Ersek wrote:
> On 02/25/20 11:44, Ard Biesheuvel wrote:
>> Duplicate the TPM2_ENABLE and TPM2_CONFIG_ENABLE build time flags that
>> already exist in OvmfPkg, and wire them up in the .DSC and .FDF so
>> that setting those flags produces a ArmVirtQemu build that implements
>> measured boot using a TPM provided by QEMU and described in the device
>> tree.
>>
>> Note that the TPM2 driver stack relies on a PEI phase being implemented,
>> so there is no point in enabling this for ArmVirtQemuKernel or ArmVirtXen.
>>
>> Also note that, despite ArmVirtQemuKernel being unaffected by this patch,
>> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc is being modified, for keeping the
>> contexts of the referring !include directives simple.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>  ArmVirtPkg/ArmVirtQemu.dsc           | 75 ++++++++++++++++++++
>>  ArmVirtPkg/ArmVirtQemu.fdf           |  6 ++
>>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 10 +++
>>  3 files changed, 91 insertions(+)
> 
> Under a similar, recent patch from Marc-André (which proposes enabling
> TPM-1.2 in OvmfPkg), I asked Marc-André to build up the work in small
> steps, practically mirroring the gradual TPM2.0 stuff from OvmfPkg:
> 
> * [edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support
> 
> http://mid.mail-archive.com/bbf8cf87-9c90-5507-82b3-ae8534555a54@redhat.com
> 
> https://edk2.groups.io/g/devel/message/54473
> 
> I'd like to be consistent as a review (and I indeed prefer that
> approach), so I'd like to ask you for the same.

Please see the approach here:

[PATCH v3 0/6] Ovmf: enable TPM 1.2

https://edk2.groups.io/g/devel/message/54854
http://mid.mail-archive.com/20200226093459.1131530-1-marcandre.lureau@redhat.com

Thanks!
Laszlo

Re: [edk2-devel] [PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot

[Ard Biesheuvel]

On Wed, 26 Feb 2020 at 11:49, Laszlo Ersek <lersek@redhat.com> wrote:
>
> On 02/26/20 01:40, Laszlo Ersek wrote:
> > On 02/25/20 11:44, Ard Biesheuvel wrote:
> >> Duplicate the TPM2_ENABLE and TPM2_CONFIG_ENABLE build time flags that
> >> already exist in OvmfPkg, and wire them up in the .DSC and .FDF so
> >> that setting those flags produces a ArmVirtQemu build that implements
> >> measured boot using a TPM provided by QEMU and described in the device
> >> tree.
> >>
> >> Note that the TPM2 driver stack relies on a PEI phase being implemented,
> >> so there is no point in enabling this for ArmVirtQemuKernel or ArmVirtXen.
> >>
> >> Also note that, despite ArmVirtQemuKernel being unaffected by this patch,
> >> ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc is being modified, for keeping the
> >> contexts of the referring !include directives simple.
> >>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >>  ArmVirtPkg/ArmVirtQemu.dsc           | 75 ++++++++++++++++++++
> >>  ArmVirtPkg/ArmVirtQemu.fdf           |  6 ++
> >>  ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc | 10 +++
> >>  3 files changed, 91 insertions(+)
> >
> > Under a similar, recent patch from Marc-André (which proposes enabling
> > TPM-1.2 in OvmfPkg), I asked Marc-André to build up the work in small
> > steps, practically mirroring the gradual TPM2.0 stuff from OvmfPkg:
> >
> > * [edk2-devel] [PATCH v2 3/3] Ovmf: enable TPM 1.2 support
> >
> > http://mid.mail-archive.com/bbf8cf87-9c90-5507-82b3-ae8534555a54@redhat.com
> >
> > https://edk2.groups.io/g/devel/message/54473
> >
> > I'd like to be consistent as a review (and I indeed prefer that
> > approach), so I'd like to ask you for the same.
>
> Please see the approach here:
>
> [PATCH v3 0/6] Ovmf: enable TPM 1.2
>
> https://edk2.groups.io/g/devel/message/54854
> http://mid.mail-archive.com/20200226093459.1131530-1-marcandre.lureau@redhat.com
>

Ah ok. Apologies for missing the link

I think I can emulate that - no worries.