From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by mx.groups.io with SMTP id smtpd.web11.68956.1674719422689637732 for ; Wed, 25 Jan 2023 23:50:22 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=exc77lTv; spf=pass (domain: redhat.com, ip: 170.10.133.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1674719421; 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: in-reply-to:in-reply-to:references:references; bh=GkYzlkOAND2Zv7X4UzUjqGpedJCCg90dVFNEfSK1E4Y=; b=exc77lTv2vA0+k76fko6IS2UZGoGo/TCkPQsaVLFsZ44R88MmEluG9IFpWUdpwo/YNxoJl cueo57zY7H1N2AgygV5gAfQWGH7IHXRi+0QeGjevPC8dp51b6VAIg/RP3YOvKsdbEO5MGB +DjeMIA+LHVBCz/2taSRsgIyNIUaCTA= Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-536-3qmMzsxlNhOEawnwTTibZg-1; Thu, 26 Jan 2023 02:50:18 -0500 X-MC-Unique: 3qmMzsxlNhOEawnwTTibZg-1 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.rdu2.redhat.com [10.11.54.1]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 253C3185A78B; Thu, 26 Jan 2023 07:50:18 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.46]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C9FB040C2064; Thu, 26 Jan 2023 07:50:17 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 657AA180091C; Thu, 26 Jan 2023 08:50:16 +0100 (CET) Date: Thu, 26 Jan 2023 08:50:16 +0100 From: "Gerd Hoffmann" To: "Xu, Min M" Cc: "devel@edk2.groups.io" , "Aktas, Erdem" , James Bottomley , "Yao, Jiewen" , Tom Lendacky , Michael Roth Subject: Re: [PATCH V3 5/9] OvmfPkg/TdxHelperLib: Implement TdxHelperBuildGuidHobForTdxMeasurement Message-ID: <20230126075016.56fu6o64mwmdtll5@sirius.home.kraxel.org> References: <20230125022359.1645-1-min.m.xu@intel.com> <20230125022359.1645-6-min.m.xu@intel.com> <20230125122223.xsc2kwwpcajn53bi@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 3.1 on 10.11.54.1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Thu, Jan 26, 2023 at 07:09:55AM +0000, Xu, Min M wrote: > On January 25, 2023 8:22 PM, Gerd Hoffmann wrote: > > > +#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX- > > XXXXXXXXXXXX)" > > > +typedef PLATFORM_FIRMWARE_BLOB2_STRUCT > > CFV_HANDOFF_TABLE_POINTERS2; > > > > > -#define FV_HANDOFF_TABLE_DESC "Fv(XXXXXXXX-XXXX-XXXX-XXXX- > > XXXXXXXXXXXX)" > > > -typedef struct { > > > - UINT8 BlobDescriptionSize; > > > - UINT8 BlobDescription[sizeof (FV_HANDOFF_TABLE_DESC)]; > > > - EFI_PHYSICAL_ADDRESS BlobBase; > > > - UINT64 BlobLength; > > > -} FV_HANDOFF_TABLE_POINTERS2; > > > > That update makes sense, but can you please split this (and possibly > > other) code changes to a separate patch so this patch does nothing but > > moving code and the absolute necessary changes needed to make it work > > (update library references, function renames). > > > Some of the data struct and functions in TdxMeasurementHob.c are moved from PeilessStartupLib/IntelTdx.c > - TDX_HANDOFF_TABLE_POINTERS2 > - CFV_HANDOFF_TABLE_POINTERS2 > - GetFvName > > BuildTdxMeasurementGuidHob is partially copied from TpmMeasureAndLogData@SecTpmMeasurementLibTdx.c. > InternalBuildGuidHobForTdxMeasurement is newly added. > > That is because in the previous PeilessStartupLib/IntelTdx.c the > measurement uses the TpmMeasureAndLogData which is exported by > SecTpmMeasurementLibTdx.c. It does not only measurement/extending, but > also builds GuidHob of the measurement. > > Now in this patch-set, the measurement/extending and the building of > GuidHob are split. Firstly HobList and CFV image are measured and > extended (at the same time the measurement values are stored in > WorkArea because at that time the Hob service is not ready). Then > after hob service is ready, GuidHob of the measurements are built. > > So it's hard to separate the patch into the one that does nothing but > moving code. I think you can first do the code changes (split the functions so measurement and GuidHob building can be done separately, but keep things in IntelTdx.c), then move the code without functional changes to the new places. take care, Gerd