From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=209.132.183.28; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (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 2DF1021F303F9 for ; Fri, 29 Sep 2017 09:35:40 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 8F34313A91; Fri, 29 Sep 2017 16:38:55 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 8F34313A91 Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx05.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-121-145.rdu2.redhat.com [10.10.121.145]) by smtp.corp.redhat.com (Postfix) with ESMTP id 179DA84CE0; Fri, 29 Sep 2017 16:38:53 +0000 (UTC) To: "Yao, Jiewen" 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> From: Laszlo Ersek Cc: edk2-devel-01 , Ladi Prosek Message-ID: <5931c346-67cd-e2fa-aa11-dec14f1841bc@redhat.com> Date: Fri, 29 Sep 2017 18:38:52 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: <74D8A39837DF1E4DA445A8C0B3885C503A9C9E00@shsmsx102.ccr.corp.intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.29]); Fri, 29 Sep 2017 16:38:55 +0000 (UTC) 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: Fri, 29 Sep 2017 16:35:40 -0000 Content-Type: text/plain; charset=windows-1252 Content-Language: en-US Content-Transfer-Encoding: 7bit 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 Laszlo 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 EDKII? >> >> 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 present. >> >> My thought is to let Variable driver check if MOR is present. Variable driver 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 good 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=727 > > 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 >