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.biesheu...@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
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#54813): https://edk2.groups.io/g/devel/message/54813
Mute This Topic: https://groups.io/mt/71530903/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
