From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.43; helo=mga05.intel.com; envelope-from=jiewen.yao@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 776C120945B7C for ; Fri, 29 Sep 2017 20:50:18 -0700 (PDT) Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga105.fm.intel.com with ESMTP; 29 Sep 2017 20:53:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,455,1500966000"; d="scan'208,217";a="133758044" Received: from fmsmsx107.amr.corp.intel.com ([10.18.124.205]) by orsmga004.jf.intel.com with ESMTP; 29 Sep 2017 20:53:32 -0700 Received: from fmsmsx114.amr.corp.intel.com (10.18.116.8) by fmsmsx107.amr.corp.intel.com (10.18.124.205) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 29 Sep 2017 20:53:32 -0700 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX114.amr.corp.intel.com (10.18.116.8) with Microsoft SMTP Server (TLS) id 14.3.319.2; Fri, 29 Sep 2017 20:53:31 -0700 Received: from shsmsx102.ccr.corp.intel.com ([169.254.2.175]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.98]) with mapi id 14.03.0319.002; Sat, 30 Sep 2017 11:53:29 +0800 From: "Yao, Jiewen" To: Laszlo Ersek CC: edk2-devel-01 , Ladi Prosek Thread-Topic: [edk2] multiple levels of support for MOR / MORLock Thread-Index: AQHTOFkQdWjN5VpEtUydh2Cd6XgU/KLLGEVggAAWqYCAAKqDwP//sp4AgAFCDoA= Date: Sat, 30 Sep 2017 03:53:28 +0000 Message-ID: <74D8A39837DF1E4DA445A8C0B3885C503A9CAC10@shsmsx102.ccr.corp.intel.com> References: <039cf353-80fb-9f20-6ad2-f52517ab4de7@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A9C9497@shsmsx102.ccr.corp.intel.com> <1aa05a42-a7a7-410b-c123-8face8be9f78@redhat.com> <74D8A39837DF1E4DA445A8C0B3885C503A9C9E00@shsmsx102.ccr.corp.intel.com> <5931c346-67cd-e2fa-aa11-dec14f1841bc@redhat.com> In-Reply-To: <5931c346-67cd-e2fa-aa11-dec14f1841bc@redhat.com> Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 X-Content-Filtered-By: Mailman/MimeDel 2.1.22 Subject: Re: multiple levels of support for MOR / MORLock X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 30 Sep 2017 03:50:18 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Agree. Adding a hook function is OK for me, as long as the hook function im= plementation is in TcgMorLockXXX.c Thank you Yao Jiewen From: Laszlo Ersek [mailto:lersek@redhat.com] Sent: Saturday, September 30, 2017 12:39 AM To: Yao, Jiewen Cc: edk2-devel-01 ; Ladi Prosek Subject: Re: [edk2] multiple levels of support for MOR / MORLock On 09/29/17 15:26, Yao, Jiewen wrote: > Sure. Feel free to submit patch. > > For the fix, I suggest we keep all MORL related code into TcgMorLockXXX.c= . > I do not suggest we mix the MORL stuff into VariableXXX.c. I've had more thoughts about this, since sending my last email. Currently SmmEndOfDxeCallback() [MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.c] performs the following actions: - sets "mEndOfDxe" to TRUE - calls VarCheckLibInitializeAtEndOfDxe() - calls InitializeVariableQuota() - calls ReclaimForOS() optionally If we register a separate callback for gEfiSmmEndOfDxeProtocolGuid, in order to create the MORL variable, then that callback could be invoked after SmmEndOfDxeCallback(). I think it would not be optimal to create MORL after SmmEndOfDxeCallback() has run, even if it worked, technically speaking. This suggests to hook the MORL creation to the top of SmmEndOfDxeCallback(). What do you suggest? > A reminder that, the PRC team will have the PRC national holiday in next = week. As such, we won't have too much activity in next week. > > If you submit the patch, I will try to review in next week to unblock the= Linux work. Ah, sorry, I haven't been aware of the holidays. I might send a patch soon, but please don't let me disturb you during the holidays! Especially because this patch might need more discussion and more versins. Holidays are for getting rested from work! :) We can sort it out after you return. > In the patch, if you can add the detailed test result, that will be very = helpful for us. > For example, test SMM/non-SMM version, test with MOR present/absent, etc. For the "no MOR, no MORL" case, I'll have to rely on Ladi, for test results (Ladi offered to help test that case, thanks again for that, Ladi). However, for regression-testing the "both MOR and MORL" case, I can only rely on Intel developers and quality engineers -- OVMF doesn't have any TPM / MOR solution at the moment. (If we had one, then we wouldn't have found or reported this issue in the first place. :) ) The fact that I can't regression-test the MOR+MORL case myself supports the idea that we should postpone the review and commit (if appropriate) of the patch until after the PRC national holiday. Thanks! Laszlo > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of La= szlo Ersek > Sent: Friday, September 29, 2017 7:06 PM > To: Yao, Jiewen > > Cc: edk2-devel-01 > > Subject: Re: [edk2] multiple levels of support for MOR / MORLock > > On 09/29/17 03:52, Yao, Jiewen wrote: >> Thanks Laszlo. >> >> Yes, I agree it is bug. Would you please help to file a bugzilar in EDKI= I? >> >> For the fix, I think we have a way to resolve it without PCD. (I do not = want to bother a platform developer to set a new PCD.) >> >> The only invalid case we need handle is: MOR is absent, but MORL is pres= ent. >> >> My thought is to let Variable driver check if MOR is present. Variable d= river can defer the MORL setting at EndOfDxe event based upon the presence = of MOR. If MOR driver is present, it sets MOR at entrypoint. EndOfDxe is go= od enough to know the state. >> >> Also, because EndOfDxe is PI event, the UEFI OS is not aware of that. > > Sounds great; thanks a lot! > > I've filed: > > https://bugzilla.tianocore.org/show_bug.cgi?id=3D727 > > If possible I'd like the fix to be committed sometime next week. Is it > OK if I try and submit a patch soon? My plan is the following: > > - In MorLockInit() [TcgMorLockSmm.c], call > gSmst->SmmRegisterProtocolNotify() in order to register a callback for > gEfiSmmEndOfDxeProtocolGuid > > - In the callback function, call VariableServiceGetVariable(), with size > 0, to see if MOR is present -- if EFI_BUFFER_TOO_SMALL is returned, MOR > is present; otherwise MOR is absent. > > - If MOR is present, then call SetMorLockVariable(0) from the callback > function; like MorLockInit() does now. Otherwise, do nothing. > > - It looks like there are no circumstances under which I should > de-register the callback. (I.e. call SmmRegisterProtocolNotify() with a > NULL Function argument.) > > > Now, I can see that VariableSmm.c already installs such a callback -- > SmmEndOfDxeCallback(). Should I hook into that callback function through > a new BOOLEAN variable, such as "mDelayedMorLockInit" (and then > MorLockInit() would only set this variable to TRUE), or should I install > a separate callback? > > Either way, I don't think that I should do the MOR/MORL stuff in the > current SmmEndOfDxeCallback() function *unconditionally*, because that > callback is set up when the *read* half of the variable services is > initialized, but MORL only becomes relevant when the *write* half of the > variable services is initialized (which occurs in the > SmmFtwNotificationEvent() callback, i.e. when the FaultTolerantWrite SMM > protocol becomes available). Hence I think we need either a separate > callback registration, or a new boolean for the existent callback. > > Thanks! > Laszlo > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org> > https://lists.01.org/mailman/listinfo/edk2-devel >