Re: multiple levels of support for MOR / MORLock

["Yao, Jiewen" <jiewen.yao@intel.com>]

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.


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.

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.


Thank you
Yao Jiewen

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 <jiewen.yao@intel.com>
Cc: edk2-devel-01 <edk2-devel@lists.01.org>
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<mailto:edk2-devel@lists.01.org>
https://lists.01.org/mailman/listinfo/edk2-devel