> -----Original Message----- > From: Yao, Jiewen <jiewen....@intel.com> > Sent: Friday, January 3, 2020 1:59 PM > To: devel@edk2.groups.io; Yao, Jiewen <jiewen....@intel.com>; Gao, Zhichao > <zhichao....@intel.com> > Cc: Wang, Jian J <jian.j.w...@intel.com>; Zhang, Chao B > <chao.b.zh...@intel.com>; Justen, Jordan L <jordan.l.jus...@intel.com>; > Laszlo Ersek <ler...@redhat.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>; > Marc-André Lureau <marcandre.lur...@redhat.com>; Stefan Berger > <stef...@linux.ibm.com> > Subject: RE: [edk2-devel] [PATCH 00/13] Extend and fix the TCG/TCG2 Physical > Presence Interface > > Some other comment: > > 6. I disagree to move NvData.h to securityPkg. > This is designed to be an internal variable. > Why it need to move to security pkg to share with other driver? > > I agree that duplicating code is bad solution. > And exposing internal data structure is as bad as duplicating code. > > By design, the platform can have its own TCG platform lib and its own > NvData.h. > Would you please help me understand why this is needed?
Some definitions in the NvData.h are useful for Tcg2VerndorLib, such as: #define TPM_DEVICE_NULL 0 #define TPM_DEVICE_1_2 1 #define TPM_DEVICE_2_0_DTPM 2 #define TPM_DEVICE_MIN TPM_DEVICE_1_2 #define TPM_DEVICE_MAX TPM_DEVICE_2_0_DTPM #define TPM_DEVICE_DEFAULT TPM_DEVICE_1_2 I'd better to move the common definition to a normal tcg device related header file instead of moving this file. > > 7. Removing assert has no relationship with this extend PPI interface. > If you want, please file another Bugzilla and handle it separately. > > 8. Fixing bug to follow spec is good. > But I do not see the relationship with the PPI interface extension. > Please file another Bugzilla and handle that separately. > For #7, #8 and other non-extend changes, I would add new Bugzilla to separate the patches. Thanks, Zhichao > > Thank you > Yao Jiewen > > > > > -----Original Message----- > > From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Yao, > > Jiewen > > Sent: Friday, January 3, 2020 1:30 PM > > To: Gao, Zhichao <zhichao....@intel.com>; devel@edk2.groups.io > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Zhang, Chao B > > <chao.b.zh...@intel.com>; Justen, Jordan L > > <jordan.l.jus...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Ard > > Biesheuvel <ard.biesheu...@linaro.org>; Marc-André Lureau > > <marcandre.lur...@redhat.com>; Stefan Berger <stef...@linux.ibm.com> > > Subject: Re: [edk2-devel] [PATCH 00/13] Extend and fix the TCG/TCG2 > > Physical Presence Interface > > > > What is the platform use case? > > Please give at least some examples. > > > > > > > -----Original Message----- > > > From: Gao, Zhichao <zhichao....@intel.com> > > > Sent: Friday, January 3, 2020 1:08 PM > > > To: Yao, Jiewen <jiewen....@intel.com>; devel@edk2.groups.io > > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Zhang, Chao B > > > <chao.b.zh...@intel.com>; Justen, Jordan L > > > <jordan.l.jus...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Ard > > > Biesheuvel > > <ard.biesheu...@linaro.org>; > > > Marc-André Lureau <marcandre.lur...@redhat.com>; Stefan Berger > > > <stef...@linux.ibm.com> > > > Subject: RE: [PATCH 00/13] Extend and fix the TCG/TCG2 Physical > > > Presence Interface > > > > > > See below. > > > > > > > -----Original Message----- > > > > From: Yao, Jiewen > > > > Sent: Friday, January 3, 2020 11:09 AM > > > > To: Gao, Zhichao <zhichao....@intel.com>; devel@edk2.groups.io > > > > Cc: Wang, Jian J <jian.j.w...@intel.com>; Zhang, Chao B > > > > <chao.b.zh...@intel.com>; Justen, Jordan L > > > > <jordan.l.jus...@intel.com>; Laszlo Ersek <ler...@redhat.com>; Ard > > > > Biesheuvel <ard.biesheu...@linaro.org>; Marc-André Lureau > > > > <marcandre.lur...@redhat.com>; Stefan Berger > > > > <stef...@linux.ibm.com> > > > > Subject: RE: [PATCH 00/13] Extend and fix the TCG/TCG2 Physical > > > > Presence Interface > > > > > > > > Hi > > > > I am not clear on the purpose of this extension. > > > > > > > > The Bugzilla just describes the solution. > > > > But what is the problem you are trying to resolve? > > > > > > > > I completely don’t understand. > > > > > > > > Please do consider add the background information there. > > > > Or it is hard for me to comment. > > > > > > > > > > > > Thank you > > > > Yao Jiewen > > > > > > > > > -----Original Message----- > > > > > From: Gao, Zhichao <zhichao....@intel.com> > > > > > Sent: Friday, January 3, 2020 11:04 AM > > > > > To: devel@edk2.groups.io > > > > > Cc: Yao, Jiewen <jiewen....@intel.com>; Wang, Jian J > > > > > <jian.j.w...@intel.com>; Zhang, Chao B <chao.b.zh...@intel.com>; > > > > > Justen, Jordan L <jordan.l.jus...@intel.com>; Laszlo Ersek > > > > > <ler...@redhat.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>; > > > > > Marc-André Lureau <marcandre.lur...@redhat.com>; Stefan Berger > > > > > <stef...@linux.ibm.com> > > > > > Subject: [PATCH 00/13] Extend and fix the TCG/TCG2 Physical > > > > > Presence Interface > > > > > > > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=2443 > > > > > > > > > > 1. Add two interfaces Tcg2PpVendorLibExecutePendingRequestEx and > > > > > Tcg2PpVendorLibSubmitRequestToPreOSFunctionEx to Tcg2PpVendorLib. > > It > > > > > has one more parameter PPData (type EFI_TCG2_PHYSICAL_PRESENCE) > > > > > to transfer more data. > > > > > 2. Use the Ex version instead of original one in > > > > > Tcg2PhysicalPresenceLib 3. Add a pcd > > > > > PcdPhysicalPresenceUserConfirmTimeout to control the user > > > > > confirm input key timeout. > > > > > 4. Add FunctionIndex to structure type > > > > > EFI_TCG2_PHYSICAL_PRESENCE to transfer > > > > > mTcgNvs->PhysicalPresence.Parameter data. > > > > > 5. Add parameter FunctionIndex to > > > > > Tcg2PhysicalPresenceLibSubmitRequestToPreOSFunctionEx > > > > > to initialize the PPdata. > > > > > > Background: > > > Some platforms implement their own Tcg2PpVendorLib and want to > > > operate with more parameters. > > > > > > #1-#2 and #4-#5 changes aim to add extend the interface with PPdata. > > > PPdata pointer can transfer most of the required parameter to do the > operation. > > > All the extend changes is for the PPdata and transfer to the > > > platform implemented interfaces. This would affect the platforms > > > which implement > > their > > > own Tcg2PpVendorLib. > > > But for open source platform, I didn't see any implementation of it. > > > > > > #3 aims to add a pcd to let the customers to decide whether to wait > > > for the input forever or with a timeout count. > > > > > > > > 6. Move Tcg2ConfigNvData.h from SecurityPkg/Tcg/Tcg2Config to > > > > > SecurityPkg/Include. > > > > > It is useful for platform code to implement their own > > > Tcg2PhysicalPresenceLib. > > > > > > #6 is a movement to decrease the duplicated code at platform side if > > > the platform code implement its own TCG library or driver. > > > > > > > > 7. Replace the ASSERT with error code return in > > > > > TpmPhysicalPresenceLib > > > > > > #7 aims to remove the ASSERT because it is not critical. ASSERT when > > > fail to > > call > > > TpmPhysicalPresence and GetTpmCapability is not a good behavior. > > > > > > > > 8. Fix one operation > > > > > (PHYSICAL_PRESENCE_DEACTIVATE_DISABLE_OWNER_FALSE) flow of > > > > > TcgPhysicalPresenceLib (refer to Physical Presence Interface Spec Page > 37). > > > > > > #8 is a bug fix to follow the spec. > > > > > > Thanks, > > > Zhichao > > > > > > > > > > > > > Cc: Jiewen Yao <jiewen....@intel.com> > > > > > Cc: Jian J Wang <jian.j.w...@intel.com> > > > > > Cc: Chao Zhang <chao.b.zh...@intel.com> > > > > > Cc: Jordan Justen <jordan.l.jus...@intel.com> > > > > > Cc: Laszlo Ersek <ler...@redhat.com> > > > > > Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > > > > > Cc: Marc-André Lureau <marcandre.lur...@redhat.com> > > > > > Cc: Stefan Berger <stef...@linux.ibm.com> > > > > > Signed-off-by: Zhichao Gao <zhichao....@intel.com> Zhichao Gao (13): > > > > > SecurityPkg/Tcg2PpVerndorLib: Add two Ex function to handle PPdata > > > > > SecurityPkg/Tcg2PpVendorLib: Add implementation of new Ex function > > > > > SecurityPkg/Tcg2PhysicalPresenceLib: Use the new Ex function > > > > > SecurityPkg/SmmTcg2PhysicalPresenceLib: Use the new Ex function > > > > > SecurityPkg/dec: Add a pcd for user response wait time > > > > > OvmfPkg/Tcg2PhysicalPresenceLib: Use pcd for user response wait time > > > > > SecurityPkg/Tcg2PhysicalPresenceLib: Use Pcd for user resp wait time > > > > > SecurityPkg/TcgPyhsicalPresenceLib: Use Pcd for user resp wait time > > > > > SecurityPkg/Tcg2PhysicalPresenceData.h: Add FunctionIndex for PPdata > > > > > SecurityPkg/Tcg2PhysicalPresenceLib: Extend the submit preOS func > > > > > SecurityPkg: Move the Tcg2ConfigNvData.h to Include folder > > > > > SecurityPkg/TcgPhysicalPresenceLib: Replace the ASSERT with error > code > > > > > SecurityPkg/TcgPhysicalPresenceLib: Fix the operation of 11 > > > > > > > > > > .../DxeTcg2PhysicalPresenceLib.c | 63 +++++++--- > > > > > .../DxeTcg2PhysicalPresenceLib.inf | 6 +- > > > > > .../Include/Guid/Tcg2PhysicalPresenceData.h | 3 +- > > > > > .../Include/Library/Tcg2PhysicalPresenceLib.h | 4 +- > > > > > SecurityPkg/Include/Library/Tcg2PpVendorLib.h | 54 ++++++++- > > > > > .../Tcg2Config => Include}/Tcg2ConfigNvData.h | 2 +- > > > > > .../DxeTcg2PhysicalPresenceLib.c | 68 ++++++++--- > > > > > .../DxeTcg2PhysicalPresenceLib.inf | 4 +- > > > > > .../DxeTcgPhysicalPresenceLib.c | 110 > > > > > ++++++++++++------ > > > > > .../DxeTcgPhysicalPresenceLib.inf | 6 +- > > > > > .../SmmTcg2PhysicalPresenceLib.c | 15 ++- > > > > > .../Tcg2PpVendorLibNull/Tcg2PpVendorLibNull.c | 61 +++++++++- > > > > > SecurityPkg/SecurityPkg.dec | 7 +- > > > > > SecurityPkg/SecurityPkg.uni | 7 +- > > > > > SecurityPkg/Tcg/Tcg2Config/Tcg2Config.vfr | 4 +- > > > > > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigDxe.inf | 3 +- > > > > > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigImpl.h | 4 +- > > > > > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 3 +- > > > > > SecurityPkg/Tcg/Tcg2Config/Tcg2ConfigPeim.c | 4 +- > > > > > SecurityPkg/Tcg/Tcg2Config/TpmDetection.c | 4 +- > > > > > SecurityPkg/Tcg/Tcg2Smm/Tcg2Smm.c | 10 +- > > > > > 21 files changed, 347 insertions(+), 95 deletions(-) rename > > > > > SecurityPkg/{Tcg/Tcg2Config => Include}/Tcg2ConfigNvData.h (94%) > > > > > > > > > > -- > > > > > 2.21.0.windows.1 > > > > > > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#52959): https://edk2.groups.io/g/devel/message/52959 Mute This Topic: https://groups.io/mt/69392326/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
