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.web12.496.1582677629607869521 for ; Tue, 25 Feb 2020 16:40:29 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=bynF4kKE; 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=1582677628; 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=8NJFL/W3kKXz+oiT9ZkJmbTOTbwZCfddakR161YtYIo=; b=bynF4kKEqEOohw139zGrRAIsFupzuCXs7YvpBGjNPz6hLvtE1+GpXeNf1vf40RehROvbbY M8XW+XsLE/rGDwEFcAaC4OBUTpII3oGk60EkBJotRLAHU7fnlSxd1UBIrgGb/Uku+ixQTI xy/Bn4QnS615dnoWkiZl2KoH7bE7D4c= 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-300-Dst0If31N5iXXWZq7VUAJA-1; Tue, 25 Feb 2020 19:40:20 -0500 X-MC-Unique: Dst0If31N5iXXWZq7VUAJA-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 7D4E81902EC1; Wed, 26 Feb 2020 00:40:19 +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 84FC58C07A; Wed, 26 Feb 2020 00:40:09 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH v2 5/5] ArmVirtPkg/ArmVirtQemu: add optional support for TPM2 measured boot 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-6-ard.biesheuvel@linaro.org> From: "Laszlo Ersek" Message-ID: <660bb2b6-5870-68b7-4324-ec1a16b58c94@redhat.com> Date: Wed, 26 Feb 2020 01:40:08 +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-6-ard.biesheuvel@linaro.org> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=WINDOWS-1252 Content-Transfer-Encoding: quoted-printable 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. >=20 > 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= . >=20 > 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. >=20 > Signed-off-by: Ard Biesheuvel > --- > 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=E9 (which proposes enabling TPM-1.2 in OvmfPkg), I asked Marc-Andr=E9 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=E9 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 =3D FALSE > DEFINE SECURE_BOOT_ENABLE =3D FALSE > + DEFINE TPM2_ENABLE =3D FALSE > + DEFINE TPM2_CONFIG_ENABLE =3D FALSE > =20 > # > # Network definition > @@ -74,12 +76,32 @@ [LibraryClasses.common] > PciSegmentLib|MdePkg/Library/BasePciSegmentLibPci/BasePciSegmentLibPci= .inf > PciHostBridgeLib|ArmVirtPkg/Library/FdtPciHostBridgeLib/FdtPciHostBrid= geLib.inf > =20 > +!if $(TPM2_ENABLE) =3D=3D TRUE > + Tpm2CommandLib|SecurityPkg/Library/Tpm2CommandLib/Tpm2CommandLib.inf > + Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibQemu/Dx= eTcg2PhysicalPresenceLib.inf > + Tcg2PpVendorLib|SecurityPkg/Library/Tcg2PpVendorLibNull/Tcg2PpVendorLi= bNull.inf > + TpmMeasurementLib|SecurityPkg/Library/DxeTpmMeasurementLib/DxeTpmMeasu= rementLib.inf > +!else > + Tcg2PhysicalPresenceLib|OvmfPkg/Library/Tcg2PhysicalPresenceLibNull/Dx= eTcg2PhysicalPresenceLib.inf > + TpmMeasurementLib|MdeModulePkg/Library/TpmMeasurementLibNull/TpmMeasur= ementLibNull.inf > +!endif > + > [LibraryClasses.common.PEIM] > ArmVirtMemInfoLib|ArmVirtPkg/Library/QemuVirtMemInfoLib/QemuVirtMemInf= oPeiLib.inf > + ResetSystemLib|MdeModulePkg/Library/PeiResetSystemLib/PeiResetSystemLi= b.inf > + > +!if $(TPM2_ENABLE) =3D=3D TRUE > + BaseCryptLib|CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2DeviceLibDTpm.= inf > +!endif > =20 > [LibraryClasses.common.DXE_DRIVER] > ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeRep= ortStatusCodeLib.inf > =20 > +!if $(TPM2_ENABLE) =3D=3D TRUE > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.= inf > +!endif > + > [LibraryClasses.common.UEFI_DRIVER] > UefiScsiLib|MdePkg/Library/UefiScsiLib/UefiScsiLib.inf > =20 > @@ -100,6 +122,8 @@ [PcdsFeatureFlag.common] > =20 > gEfiMdeModulePkgTokenSpaceGuid.PcdTurnOffUsbLegacySupport|TRUE > =20 > + gArmVirtTokenSpaceGuid.PcdTpm2SupportEnabled|$(TPM2_ENABLE) > + > [PcdsFixedAtBuild.common] > !if $(ARCH) =3D=3D AARCH64 > gArmTokenSpaceGuid.PcdVFPEnabled|1 > @@ -237,9 +261,20 @@ [PcdsDynamicDefault.common] > gEfiMdeModulePkgTokenSpaceGuid.PcdSmbiosDocRev|0x0 > gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE > =20 > +!if $(TPM2_ENABLE) =3D=3D TRUE > + gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid|{0x00, 0x00, 0x00, 0x= 00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00} > + gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress|0x0 > + gEfiSecurityPkgTokenSpaceGuid.PcdTpm2HashMask|0 > +!endif > + > [PcdsDynamicHii] > gArmVirtTokenSpaceGuid.PcdForceNoAcpi|L"ForceNoAcpi"|gArmVirtVariableG= uid|0x0|FALSE|NV,BS > =20 > +!if $(TPM2_CONFIG_ENABLE) =3D=3D TRUE > + gEfiSecurityPkgTokenSpaceGuid.PcdTcgPhysicalPresenceInterfaceVer|L"TCG= 2_VERSION"|gTcg2ConfigFormSetGuid|0x0|"1.3"|NV,BS > + gEfiSecurityPkgTokenSpaceGuid.PcdTpm2AcpiTableRev|L"TCG2_VERSION"|gTcg= 2ConfigFormSetGuid|0x8|3|NV,BS > +!endif > + > ########################################################################= ######## > # > # Components Section - list of all EDK II Modules needed by this Platfor= m > @@ -261,6 +296,23 @@ [Components.common] > =20 > MdeModulePkg/Universal/Variable/Pei/VariablePei.inf > =20 > +!if $(TPM2_ENABLE) =3D=3D TRUE > + MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf { > + > + ResetSystemLib|ArmVirtPkg/Library/ArmVirtPsciResetSystemPeiLib/Arm= VirtPsciResetSystemPeiLib.inf > + } > + OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf > + SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf { > + > + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCry= ptoRouterPei.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.i= nf > + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha2= 56.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha3= 84.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha5= 12.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf > + } > +!endif > + > MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf { > > NULL|MdeModulePkg/Library/LzmaCustomDecompressLib/LzmaCustomDecomp= ressLib.inf > @@ -295,6 +347,9 @@ [Components.common] > MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf { > > NULL|SecurityPkg/Library/DxeImageVerificationLib/DxeImageVerificat= ionLib.inf > +!if $(TPM2_ENABLE) =3D=3D TRUE > + NULL|SecurityPkg/Library/DxeTpm2MeasureBootLib/DxeTpm2MeasureBootL= ib.inf > +!endif > } > SecurityPkg/VariableAuthenticated/SecureBootConfigDxe/SecureBootConfig= Dxe.inf > OvmfPkg/EnrollDefaultKeys/EnrollDefaultKeys.inf > @@ -430,6 +485,26 @@ [Components.common] > MdeModulePkg/Bus/Usb/UsbKbDxe/UsbKbDxe.inf > MdeModulePkg/Bus/Usb/UsbMassStorageDxe/UsbMassStorageDxe.inf > =20 > + # > + # TPM2 support > + # > +!if $(TPM2_ENABLE) =3D=3D TRUE > + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf { > + > + HashLib|SecurityPkg/Library/HashLibBaseCryptoRouter/HashLibBaseCry= ptoRouterDxe.inf > + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLi= bRouterDxe.inf > + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.i= nf > + NULL|SecurityPkg/Library/HashInstanceLibSha256/HashInstanceLibSha2= 56.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha384/HashInstanceLibSha3= 84.inf > + NULL|SecurityPkg/Library/HashInstanceLibSha512/HashInstanceLibSha5= 12.inf > + NULL|SecurityPkg/Library/HashInstanceLibSm3/HashInstanceLibSm3.inf > + } > +!if $(TPM2_CONFIG_ENABLE) =3D=3D 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 > =20 > +!if $(TPM2_ENABLE) =3D=3D TRUE > + INF MdeModulePkg/Universal/ResetSystemPei/ResetSystemPei.inf > + INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf > + INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf > +!endif > + > FILE FV_IMAGE =3D 9E21FD93-9C72-4c15-8C4B-E77F1DB2D792 { > SECTION GUIDED EE4E5898-3914-4259-9D6E-DC7BD79403CF PROCESSING_REQUI= RED =3D TRUE { > SECTION FV_IMAGE =3D FVMAIN > diff --git a/ArmVirtPkg/ArmVirtQemuFvMain.fdf.inc b/ArmVirtPkg/ArmVirtQem= uFvMain.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) =3D=3D TRUE > + INF SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf > +!if $(TPM2_CONFIG_ENABLE) =3D=3D TRUE > + INF SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf > +!endif > +!endif >=20