From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web11.321.1582676692418892788 for ; Tue, 25 Feb 2020 16:24:52 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=NFJABUkR; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1582676691; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0e8+JnpS+vb5jE+LSf8deUTPXrvNSFZqvszg93h03ak=; b=NFJABUkR1kN4SaxPLAp1xeU1D+AzYgFihNzEHxe66pstkhe8w0jmHSuRPjfKKnlHYMIPIe ugAHfHWwfNs/Wva7ty5H/n5bwT1i7bI1bu3I61wNxHOwpx/DF2k0p0nnczQUMp6UPeyv7y uOGWMnSoDcfDY2By7eshhc/HqVQ711k= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-458-A0-8YoBNMHqPSVxE4GTD6g-1; Tue, 25 Feb 2020 19:24:47 -0500 X-MC-Unique: A0-8YoBNMHqPSVxE4GTD6g-1 Received: from smtp.corp.redhat.com (int-mx08.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A827C13E4; Wed, 26 Feb 2020 00:24:46 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-104.ams2.redhat.com [10.36.117.104]) by smtp.corp.redhat.com (Postfix) with ESMTP id CB3E9393; Wed, 26 Feb 2020 00:24:39 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 3/5] ArmVirtPkg/PlatformPeiLib: discover the TPM base address from the DT To: devel@edk2.groups.io, ard.biesheuvel@linaro.org Cc: eric.auger@redhat.com, philmd@redhat.com, marcandre.lureau@redhat.com, stefanb@linux.ibm.com, leif@nuviainc.com References: <20200225104449.22453-1-ard.biesheuvel@linaro.org> <20200225104449.22453-4-ard.biesheuvel@linaro.org> From: "Laszlo Ersek" Message-ID: <74e50cfd-148b-7085-417b-b5613f1a7f2c@redhat.com> Date: Wed, 26 Feb 2020 01:24:38 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200225104449.22453-4-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.23 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit 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 > --- > 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 > #include > #include > +#include > #include > > #include > #include > > +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 > + // 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 >