public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* smm lock query
@ 2018-05-26 15:04 Abhishek Singh
  2018-05-27 16:47 ` Marvin H?user
  0 siblings, 1 reply; 12+ messages in thread
From: Abhishek Singh @ 2018-05-26 15:04 UTC (permalink / raw)
  To: edk2-devel

Hi,

This is the first time I am mailing to this list. If this is not the right
place for the kind of questions I am asking please let me know where to
direct my queries.

I have been looking at the SMM IPL code and a portion of the code is a
little confusing to me. In the function SmmIplReadyToLockEventNotify, smram
is locked (mSmmAccess->Lock) before the ready to lock notifications are
sent through
SmmIplGuidedEventNotify. Shouldn't the lock be placed after the ready to
lock notifications?

Best regards,
Abhishek


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-26 15:04 smm lock query Abhishek Singh
@ 2018-05-27 16:47 ` Marvin H?user
  2018-05-27 20:44   ` Andrew Fish
  0 siblings, 1 reply; 12+ messages in thread
From: Marvin H?user @ 2018-05-27 16:47 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Abhishek Singh
  Cc: star.zeng@intel.com, eric.dong@intel.com, ruiyu.ni@intel.com,
	Laszlo Ersek

Good day Abhishek,

I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.

Strictly speaking you are, right, because of the description for the MM protocol:
"Indicates that MM resources and services that should not be used by the third party code are about[Marvin: (!)] to be locked."
Practically however, I don't see any issue with the current implementation. Code inside MMRAM is not affected directly by the lock, it is just notified.
However, either the code or the specification should be slightly updated to be in sync. A code update might require review of the caller assumptions, just to be sure.

I have a different concern though and hope I'm actually overlooking something.
If I understand the code correctly, it is the Platform BDS that exposes the (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and lock SMM resources based on the event.
Because of latter being an event however, I don't think it is, or can be, guaranteed to be the last event group member executing.
When it is not the last, the "about to be locked" part is not true for any subsequent callbacks, that could actually be a risky break of the specification - if it is.
If it is a break of the specification, I can only think of letting Platform BDS expose an "internal" event group, which is only caught by PiSmmIpl, which then drives the actual SmmReadyToLock flow.
This would require updates to all platform trees and hence I would propose a temporary backwards-compatibility.

Any comments? Did I overlook something (I hope)?

Thanks and regards,
Marvin

> -----Original Message-----
> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
> Abhishek Singh
> Sent: Saturday, May 26, 2018 5:05 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] smm lock query
> 
> Hi,
> 
> This is the first time I am mailing to this list. If this is not the right place for the
> kind of questions I am asking please let me know where to direct my queries.
> 
> I have been looking at the SMM IPL code and a portion of the code is a little
> confusing to me. In the function SmmIplReadyToLockEventNotify, smram is
> locked (mSmmAccess->Lock) before the ready to lock notifications are sent
> through SmmIplGuidedEventNotify. Shouldn't the lock be placed after the
> ready to lock notifications?
> 
> Best regards,
> Abhishek
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-27 16:47 ` Marvin H?user
@ 2018-05-27 20:44   ` Andrew Fish
  2018-05-27 20:45     ` Andrew Fish
  2018-05-28 10:57     ` Laszlo Ersek
  0 siblings, 2 replies; 12+ messages in thread
From: Andrew Fish @ 2018-05-27 20:44 UTC (permalink / raw)
  To: Marvin H?user
  Cc: edk2-devel@lists.01.org, Abhishek Singh, ruiyu.ni@intel.com,
	Laszlo Ersek, eric.dong@intel.com, star.zeng@intel.com



> On May 27, 2018, at 9:47 AM, Marvin H?user <Marvin.Haeuser@outlook.com> wrote:
> 
> Good day Abhishek,
> 
> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> 
> Strictly speaking you are, right, because of the description for the MM protocol:
> "Indicates that MM resources and services that should not be used by the third party code are about[Marvin: (!)] to be locked."
> Practically however, I don't see any issue with the current implementation. Code inside MMRAM is not affected directly by the lock, it is just notified.
> However, either the code or the specification should be slightly updated to be in sync. A code update might require review of the caller assumptions, just to be sure.
> 
> I have a different concern though and hope I'm actually overlooking something.
> If I understand the code correctly, it is the Platform BDS that exposes the (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and lock SMM resources based on the event.
> Because of latter being an event however, I don't think it is, or can be, guaranteed to be the last event group member executing.
> When it is not the last, the "about to be locked" part is not true for any subsequent callbacks, that could actually be a risky break of the specification - if it is.
> If it is a break of the specification, I can only think of letting Platform BDS expose an "internal" event group, which is only caught by PiSmmIpl, which then drives the actual SmmReadyToLock flow.
> This would require updates to all platform trees and hence I would propose a temporary backwards-compatibility.
> 
> Any comments? Did I overlook something (I hope)?
> 

Mavvin,

You are correct there is no guarantee of order in events. Thanks for cc'ing the right folks, as I don't remember all the low level details...

In general the idea behind the MM code is it only comes from the platform, then by definition that code should be aware when the platform was going to lock MM. In a practical sense any MM module that had a depex evaluate to true would have dispatched in DXE prior to BDS being launched. In general BDS is the code that enumerates PCI and connects devices, thus there is no chance for 3rd party software to run before that point in the boot. So in an abstract sense that lock represents the end of DXE dispatch.

We probably need to reread the PI spec and make sure the spec is following the letter of the law, but I'd guess locking earlier is likely OK. 

Thanks,

Andrew Fish

> Thanks and regards,
> Marvin
> 
>> -----Original Message-----
>> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
>> Abhishek Singh
>> Sent: Saturday, May 26, 2018 5:05 PM
>> To: edk2-devel@lists.01.org
>> Subject: [edk2] smm lock query
>> 
>> Hi,
>> 
>> This is the first time I am mailing to this list. If this is not the right place for the
>> kind of questions I am asking please let me know where to direct my queries.
>> 
>> I have been looking at the SMM IPL code and a portion of the code is a little
>> confusing to me. In the function SmmIplReadyToLockEventNotify, smram is
>> locked (mSmmAccess->Lock) before the ready to lock notifications are sent
>> through SmmIplGuidedEventNotify. Shouldn't the lock be placed after the
>> ready to lock notifications?
>> 
>> Best regards,
>> Abhishek
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-27 20:44   ` Andrew Fish
@ 2018-05-27 20:45     ` Andrew Fish
  2018-05-28 10:57     ` Laszlo Ersek
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Fish @ 2018-05-27 20:45 UTC (permalink / raw)
  To: Marvin H?user
  Cc: ruiyu.ni@intel.com, eric.dong@intel.com, edk2-devel@lists.01.org,
	Laszlo Ersek, star.zeng@intel.com



> On May 27, 2018, at 1:44 PM, Andrew Fish <afish@apple.com> wrote:
> 
> 
> 
>> On May 27, 2018, at 9:47 AM, Marvin H?user <Marvin.Haeuser@outlook.com> wrote:
>> 
>> Good day Abhishek,
>> 
>> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
>> 
>> Strictly speaking you are, right, because of the description for the MM protocol:
>> "Indicates that MM resources and services that should not be used by the third party code are about[Marvin: (!)] to be locked."
>> Practically however, I don't see any issue with the current implementation. Code inside MMRAM is not affected directly by the lock, it is just notified.
>> However, either the code or the specification should be slightly updated to be in sync. A code update might require review of the caller assumptions, just to be sure.
>> 
>> I have a different concern though and hope I'm actually overlooking something.
>> If I understand the code correctly, it is the Platform BDS that exposes the (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and lock SMM resources based on the event.
>> Because of latter being an event however, I don't think it is, or can be, guaranteed to be the last event group member executing.
>> When it is not the last, the "about to be locked" part is not true for any subsequent callbacks, that could actually be a risky break of the specification - if it is.
>> If it is a break of the specification, I can only think of letting Platform BDS expose an "internal" event group, which is only caught by PiSmmIpl, which then drives the actual SmmReadyToLock flow.
>> This would require updates to all platform trees and hence I would propose a temporary backwards-compatibility.
>> 
>> Any comments? Did I overlook something (I hope)?
>> 
> 
> Mavvin,
> 
> You are correct there is no guarantee of order in events. Thanks for cc'ing the right folks, as I don't remember all the low level details...
> 
> In general the idea behind the MM code is it only comes from the platform, then by definition that code should be aware when the platform was going to lock MM. In a practical sense any MM module that had a depex evaluate to true would have dispatched in DXE prior to BDS being launched. In general BDS is the code that enumerates PCI and connects devices, thus there is no chance for 3rd party software to run before that point in the boot. So in an abstract sense that lock represents the end of DXE dispatch.
> 
> We probably need to reread the PI spec and make sure the spec is following the letter of the law, but I'd guess locking earlier is likely OK. 
> 

Sorry I meant edk2, not spec, following the letter of the law.

Thanks,

Andrew Fish

> Thanks,
> 
> Andrew Fish
> 
>> Thanks and regards,
>> Marvin
>> 
>>> -----Original Message-----
>>> From: edk2-devel <edk2-devel-bounces@lists.01.org> On Behalf Of
>>> Abhishek Singh
>>> Sent: Saturday, May 26, 2018 5:05 PM
>>> To: edk2-devel@lists.01.org
>>> Subject: [edk2] smm lock query
>>> 
>>> Hi,
>>> 
>>> This is the first time I am mailing to this list. If this is not the right place for the
>>> kind of questions I am asking please let me know where to direct my queries.
>>> 
>>> I have been looking at the SMM IPL code and a portion of the code is a little
>>> confusing to me. In the function SmmIplReadyToLockEventNotify, smram is
>>> locked (mSmmAccess->Lock) before the ready to lock notifications are sent
>>> through SmmIplGuidedEventNotify. Shouldn't the lock be placed after the
>>> ready to lock notifications?
>>> 
>>> Best regards,
>>> Abhishek
>>> _______________________________________________
>>> edk2-devel mailing list
>>> edk2-devel@lists.01.org
>>> https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>
> https://lists.01.org/mailman/listinfo/edk2-devel <https://lists.01.org/mailman/listinfo/edk2-devel>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-27 20:44   ` Andrew Fish
  2018-05-27 20:45     ` Andrew Fish
@ 2018-05-28 10:57     ` Laszlo Ersek
  2018-05-28 14:03       ` Marvin Häuser
  1 sibling, 1 reply; 12+ messages in thread
From: Laszlo Ersek @ 2018-05-28 10:57 UTC (permalink / raw)
  To: Andrew Fish, Marvin H?user
  Cc: edk2-devel@lists.01.org, Abhishek Singh, ruiyu.ni@intel.com,
	eric.dong@intel.com, star.zeng@intel.com

On 05/27/18 22:44, Andrew Fish wrote:
> 
> 
>> On May 27, 2018, at 9:47 AM, Marvin H?user <Marvin.Haeuser@outlook.com> wrote:
>>
>> Good day Abhishek,
>>
>> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
>>
>> Strictly speaking you are, right, because of the description for the MM protocol:
>> "Indicates that MM resources and services that should not be used by the third party code are about[Marvin: (!)] to be locked."
>> Practically however, I don't see any issue with the current implementation. Code inside MMRAM is not affected directly by the lock, it is just notified.
>> However, either the code or the specification should be slightly updated to be in sync. A code update might require review of the caller assumptions, just to be sure.
>>
>> I have a different concern though and hope I'm actually overlooking something.
>> If I understand the code correctly, it is the Platform BDS that exposes the (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and lock SMM resources based on the event.
>> Because of latter being an event however, I don't think it is, or can be, guaranteed to be the last event group member executing.
>> When it is not the last, the "about to be locked" part is not true for any subsequent callbacks, that could actually be a risky break of the specification - if it is.
>> If it is a break of the specification, I can only think of letting Platform BDS expose an "internal" event group, which is only caught by PiSmmIpl, which then drives the actual SmmReadyToLock flow.
>> This would require updates to all platform trees and hence I would propose a temporary backwards-compatibility.
>>
>> Any comments? Did I overlook something (I hope)?
>>
> 
> Mavvin,
> 
> You are correct there is no guarantee of order in events. Thanks for cc'ing the right folks, as I don't remember all the low level details...
> 
> In general the idea behind the MM code is it only comes from the platform, then by definition that code should be aware when the platform was going to lock MM. In a practical sense any MM module that had a depex evaluate to true would have dispatched in DXE prior to BDS being launched. In general BDS is the code that enumerates PCI and connects devices, thus there is no chance for 3rd party software to run before that point in the boot. So in an abstract sense that lock represents the end of DXE dispatch.

This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
installed by Plaform BDS before any non-platform binaries get a chance
to run. In terms of the current PlatformBootManagerLib interfaces, that
means the protocol should be installed from
PlatformBootManagerBeforeConsole() (as noted on the API declaration itself).

Thanks
Laszlo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-28 10:57     ` Laszlo Ersek
@ 2018-05-28 14:03       ` Marvin Häuser
  2018-05-28 18:02         ` Abhishek Singh
  0 siblings, 1 reply; 12+ messages in thread
From: Marvin Häuser @ 2018-05-28 14:03 UTC (permalink / raw)
  To: edk2-devel@lists.01.org, Laszlo Ersek, afish@apple.com
  Cc: abh@cs.unc.edu, ruiyu.ni@intel.com, eric.dong@intel.com,
	star.zeng@intel.com

Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock event and therefor is aware.
However, they might rely on the protocol's description that the resources are about(!) to be locked and code accordingly, not considering the event characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against edk2's actions, or additional code might be supplied by third parties that do not have tree code access (which, by integration, would be "platform binaries" by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this discrepancy from the specification, such as the internal "dummy event" I proposed.

Thanks,
Marvin.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com>
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish <afish@apple.com>; Marvin H?user
> <Marvin.Haeuser@outlook.com>
> Cc: edk2-devel@lists.01.org; Abhishek Singh <abh@cs.unc.edu>;
> ruiyu.ni@intel.com; eric.dong@intel.com; star.zeng@intel.com
> Subject: Re: [edk2] smm lock query
> 
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
> <Marvin.Haeuser@outlook.com> wrote:
> >>
> >> Good day Abhishek,
> >>
> >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> >>
> >> Strictly speaking you are, right, because of the description for the MM
> protocol:
> >> "Indicates that MM resources and services that should not be used by the
> third party code are about[Marvin: (!)] to be locked."
> >> Practically however, I don't see any issue with the current
> implementation. Code inside MMRAM is not affected directly by the lock, it is
> just notified.
> >> However, either the code or the specification should be slightly updated
> to be in sync. A code update might require review of the caller assumptions,
> just to be sure.
> >>
> >> I have a different concern though and hope I'm actually overlooking
> something.
> >> If I understand the code correctly, it is the Platform BDS that exposes the
> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> lock SMM resources based on the event.
> >> Because of latter being an event however, I don't think it is, or can be,
> guaranteed to be the last event group member executing.
> >> When it is not the last, the "about to be locked" part is not true for any
> subsequent callbacks, that could actually be a risky break of the specification
> - if it is.
> >> If it is a break of the specification, I can only think of letting Platform BDS
> expose an "internal" event group, which is only caught by PiSmmIpl, which
> then drives the actual SmmReadyToLock flow.
> >> This would require updates to all platform trees and hence I would
> propose a temporary backwards-compatibility.
> >>
> >> Any comments? Did I overlook something (I hope)?
> >>
> >
> > Mavvin,
> >
> > You are correct there is no guarantee of order in events. Thanks for cc'ing
> the right folks, as I don't remember all the low level details...
> >
> > In general the idea behind the MM code is it only comes from the platform,
> then by definition that code should be aware when the platform was going
> to lock MM. In a practical sense any MM module that had a depex evaluate
> to true would have dispatched in DXE prior to BDS being launched. In general
> BDS is the code that enumerates PCI and connects devices, thus there is no
> chance for 3rd party software to run before that point in the boot. So in an
> abstract sense that lock represents the end of DXE dispatch.
> 
> This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> installed by Plaform BDS before any non-platform binaries get a chance to
> run. In terms of the current PlatformBootManagerLib interfaces, that means
> the protocol should be installed from
> PlatformBootManagerBeforeConsole() (as noted on the API declaration
> itself).
> 
> Thanks
> Laszlo

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-28 14:03       ` Marvin Häuser
@ 2018-05-28 18:02         ` Abhishek Singh
  2018-05-29  1:15           ` Zeng, Star
  0 siblings, 1 reply; 12+ messages in thread
From: Abhishek Singh @ 2018-05-28 18:02 UTC (permalink / raw)
  To: Marvin Häuser
  Cc: edk2-devel@lists.01.org, Laszlo Ersek, afish@apple.com,
	ruiyu.ni@intel.com, eric.dong@intel.com, star.zeng@intel.com

Thank you everyone for your inputs and clarifications. They are helping me
to better understand the uefi code, to which I am very new. I do not mean
to hijack the thread: so please continue your discussions about whether the
implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my
research, I wish to dump the contents of SMRAM when it has reached steady
state, i.e., all the drivers have made changes to smram and it has been
locked. The current implementation (smm ipl) locks smram when it receives
the SmmReadyToLock event and then propagates the event to the smm drivers
that make further changes to smram. Unfortunately, I cannot take a snapshot
of smram after it has been locked! Thus, I have solved this issue by
propagating the event to the smm drivers first (using
SmmIplGuidedEventNotify), then opening access to and dumping the contents
of SMRAM, and finally closing access to and locking smram. Would it be fair
to say that this would give me the fully initialized contents of smram?



My second observation is that despite opening access to smram, I am unable
to access its contents, which is a violation of the
EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This
function “opens” MMRAM so that it is visible while not inside of MM." Note
that I am working with minnowboard firmware release 0.97. So some of the
binaries like SmmAccess.efi are provided by Intel, while others have been
built from the edk source tree: thus, this may not be an EDK issue. Please
suggest further steps and/or workarounds. Should I contact edk2-platforms
maintainers, or start a new thread here for this issue?

-Abhishek


On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <Marvin.Haeuser@outlook.com>
wrote:

> Hello Andrew and Laszlo,
>
> Thanks for your comments!
> Of course I'm with you that it is the platform that signals the
> SmmReadyToLock event and therefor is aware.
> However, they might rely on the protocol's description that the resources
> are about(!) to be locked and code accordingly, not considering the event
> characteristic of the handler in PiSmmIpl.
> The code might be written by different people, not especially reviewed
> against edk2's actions, or additional code might be supplied by third
> parties that do not have tree code access (which, by integration, would be
> "platform binaries" by the definition applying here).
>
> Therefor I would still ask everyone to consider figuring out a solution to
> this discrepancy from the specification, such as the internal "dummy event"
> I proposed.
>
> Thanks,
> Marvin.
>
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Monday, May 28, 2018 12:58 PM
> > To: Andrew Fish <afish@apple.com>; Marvin H?user
> > <Marvin.Haeuser@outlook.com>
> > Cc: edk2-devel@lists.01.org; Abhishek Singh <abh@cs.unc.edu>;
> > ruiyu.ni@intel.com; eric.dong@intel.com; star.zeng@intel.com
> > Subject: Re: [edk2] smm lock query
> >
> > On 05/27/18 22:44, Andrew Fish wrote:
> > >
> > >
> > >> On May 27, 2018, at 9:47 AM, Marvin H?user
> > <Marvin.Haeuser@outlook.com> wrote:
> > >>
> > >> Good day Abhishek,
> > >>
> > >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> > (exposes the ReadyToLock protocol) and Laszlo for his high-quality
> answers.
> > >>
> > >> Strictly speaking you are, right, because of the description for the
> MM
> > protocol:
> > >> "Indicates that MM resources and services that should not be used by
> the
> > third party code are about[Marvin: (!)] to be locked."
> > >> Practically however, I don't see any issue with the current
> > implementation. Code inside MMRAM is not affected directly by the lock,
> it is
> > just notified.
> > >> However, either the code or the specification should be slightly
> updated
> > to be in sync. A code update might require review of the caller
> assumptions,
> > just to be sure.
> > >>
> > >> I have a different concern though and hope I'm actually overlooking
> > something.
> > >> If I understand the code correctly, it is the Platform BDS that
> exposes the
> > (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> > lock SMM resources based on the event.
> > >> Because of latter being an event however, I don't think it is, or can
> be,
> > guaranteed to be the last event group member executing.
> > >> When it is not the last, the "about to be locked" part is not true
> for any
> > subsequent callbacks, that could actually be a risky break of the
> specification
> > - if it is.
> > >> If it is a break of the specification, I can only think of letting
> Platform BDS
> > expose an "internal" event group, which is only caught by PiSmmIpl, which
> > then drives the actual SmmReadyToLock flow.
> > >> This would require updates to all platform trees and hence I would
> > propose a temporary backwards-compatibility.
> > >>
> > >> Any comments? Did I overlook something (I hope)?
> > >>
> > >
> > > Mavvin,
> > >
> > > You are correct there is no guarantee of order in events. Thanks for
> cc'ing
> > the right folks, as I don't remember all the low level details...
> > >
> > > In general the idea behind the MM code is it only comes from the
> platform,
> > then by definition that code should be aware when the platform was going
> > to lock MM. In a practical sense any MM module that had a depex evaluate
> > to true would have dispatched in DXE prior to BDS being launched. In
> general
> > BDS is the code that enumerates PCI and connects devices, thus there is
> no
> > chance for 3rd party software to run before that point in the boot. So
> in an
> > abstract sense that lock represents the end of DXE dispatch.
> >
> > This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> > installed by Plaform BDS before any non-platform binaries get a chance to
> > run. In terms of the current PlatformBootManagerLib interfaces, that
> means
> > the protocol should be installed from
> > PlatformBootManagerBeforeConsole() (as noted on the API declaration
> > itself).
> >
> > Thanks
> > Laszlo
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-28 18:02         ` Abhishek Singh
@ 2018-05-29  1:15           ` Zeng, Star
  2018-05-29  2:21             ` Yao, Jiewen
  0 siblings, 1 reply; 12+ messages in thread
From: Zeng, Star @ 2018-05-29  1:15 UTC (permalink / raw)
  To: Abhishek Singh, Marvin H?user
  Cc: edk2-devel@lists.01.org, Laszlo Ersek, afish@apple.com, Ni, Ruiyu,
	Dong, Eric, Zeng, Star, Yao, Jiewen

I do not see issue according to the spec.
Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
DxeSmmReadyToLock event is to notify DXE handlers.
Modules are responsible to lock or protect their resource and effect the appropriate protections in their notification handlers.
SmmIplGuidedEventNotify is used to inform SMM environment to signal SmmReadyToLock.
SmmReadyToLock event is to notify SMM handlers.

DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).

“This protocol in tandem with the End of DXE Event facilitates transition of the platform from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party extensible modules such as UEFI drivers and UEFI applications are executed.

The protocol is published immediately after signaling of the End of DXE Event.
PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers. For example, PI platform code may choose to use notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() function.”


SMM environment is a *runtime* environment. SMRAM will be even changed after SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.


Thanks,
Star
From: Abhishek Singh [mailto:abh@cs.unc.edu]
Sent: Tuesday, May 29, 2018 2:02 AM
To: Marvin Häuser <Marvin.Haeuser@outlook.com>
Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [edk2] smm lock query

Thank you everyone for your inputs and clarifications. They are helping me to better understand the uefi code, to which I am very new. I do not mean to hijack the thread: so please continue your discussions about whether the implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my research, I wish to dump the contents of SMRAM when it has reached steady state, i.e., all the drivers have made changes to smram and it has been locked. The current implementation (smm ipl) locks smram when it receives the SmmReadyToLock event and then propagates the event to the smm drivers that make further changes to smram. Unfortunately, I cannot take a snapshot of smram after it has been locked! Thus, I have solved this issue by propagating the event to the smm drivers first (using SmmIplGuidedEventNotify), then opening access to and dumping the contents of SMRAM, and finally closing access to and locking smram. Would it be fair to say that this would give me the fully initialized contents of smram?



My second observation is that despite opening access to smram, I am unable to access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This function “opens” MMRAM so that it is visible while not inside of MM." Note that I am working with minnowboard firmware release 0.97. So some of the binaries like SmmAccess.efi are provided by Intel, while others have been built from the edk source tree: thus, this may not be an EDK issue. Please suggest further steps and/or workarounds. Should I contact edk2-platforms maintainers, or start a new thread here for this issue?

-Abhishek


On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock event and therefor is aware.
However, they might rely on the protocol's description that the resources are about(!) to be locked and code accordingly, not considering the event characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against edk2's actions, or additional code might be supplied by third parties that do not have tree code access (which, by integration, would be "platform binaries" by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this discrepancy from the specification, such as the internal "dummy event" I proposed.

Thanks,
Marvin.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Abhishek Singh <abh@cs.unc.edu<mailto:abh@cs.unc.edu>>;
> ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>; eric.dong@intel.com<mailto:eric.dong@intel.com>; star.zeng@intel.com<mailto:star.zeng@intel.com>
> Subject: Re: [edk2] smm lock query
>
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
> >>
> >> Good day Abhishek,
> >>
> >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> >>
> >> Strictly speaking you are, right, because of the description for the MM
> protocol:
> >> "Indicates that MM resources and services that should not be used by the
> third party code are about[Marvin: (!)] to be locked."
> >> Practically however, I don't see any issue with the current
> implementation. Code inside MMRAM is not affected directly by the lock, it is
> just notified.
> >> However, either the code or the specification should be slightly updated
> to be in sync. A code update might require review of the caller assumptions,
> just to be sure.
> >>
> >> I have a different concern though and hope I'm actually overlooking
> something.
> >> If I understand the code correctly, it is the Platform BDS that exposes the
> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> lock SMM resources based on the event.
> >> Because of latter being an event however, I don't think it is, or can be,
> guaranteed to be the last event group member executing.
> >> When it is not the last, the "about to be locked" part is not true for any
> subsequent callbacks, that could actually be a risky break of the specification
> - if it is.
> >> If it is a break of the specification, I can only think of letting Platform BDS
> expose an "internal" event group, which is only caught by PiSmmIpl, which
> then drives the actual SmmReadyToLock flow.
> >> This would require updates to all platform trees and hence I would
> propose a temporary backwards-compatibility.
> >>
> >> Any comments? Did I overlook something (I hope)?
> >>
> >
> > Mavvin,
> >
> > You are correct there is no guarantee of order in events. Thanks for cc'ing
> the right folks, as I don't remember all the low level details...
> >
> > In general the idea behind the MM code is it only comes from the platform,
> then by definition that code should be aware when the platform was going
> to lock MM. In a practical sense any MM module that had a depex evaluate
> to true would have dispatched in DXE prior to BDS being launched. In general
> BDS is the code that enumerates PCI and connects devices, thus there is no
> chance for 3rd party software to run before that point in the boot. So in an
> abstract sense that lock represents the end of DXE dispatch.
>
> This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> installed by Plaform BDS before any non-platform binaries get a chance to
> run. In terms of the current PlatformBootManagerLib interfaces, that means
> the protocol should be installed from
> PlatformBootManagerBeforeConsole() (as noted on the API declaration
> itself).
>
> Thanks
> Laszlo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-29  1:15           ` Zeng, Star
@ 2018-05-29  2:21             ` Yao, Jiewen
  2018-05-29  5:17               ` Abhishek Singh
  0 siblings, 1 reply; 12+ messages in thread
From: Yao, Jiewen @ 2018-05-29  2:21 UTC (permalink / raw)
  To: Zeng, Star, Abhishek Singh, Marvin H?user
  Cc: edk2-devel@lists.01.org, Laszlo Ersek, afish@apple.com, Ni, Ruiyu,
	Dong, Eric

Let me share my thought.

1)      From interface point of view, ReadyToLock means it is the last time to lock. But it does not mean it must be open before.
As implementation choice, a platform MAY lock it earlier.

Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN set.


2)      Dumping SMRAM exposes the secret inside of SMRAM, I would treat it as debug feature only, not a production.

If you want to debug, you can add a debug SMM driver and expose an interface to copy SMRAM content out.

Thank you
Yao Jiewen

From: Zeng, Star
Sent: Monday, May 28, 2018 6:16 PM
To: Abhishek Singh <abh@cs.unc.edu>; Marvin H?user <Marvin.Haeuser@outlook.com>
Cc: edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [edk2] smm lock query

I do not see issue according to the spec.
Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
DxeSmmReadyToLock event is to notify DXE handlers.
Modules are responsible to lock or protect their resource and effect the appropriate protections in their notification handlers.
SmmIplGuidedEventNotify is used to inform SMM environment to signal SmmReadyToLock.
SmmReadyToLock event is to notify SMM handlers.

DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).

“This protocol in tandem with the End of DXE Event facilitates transition of the platform from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party extensible modules such as UEFI drivers and UEFI applications are executed.

The protocol is published immediately after signaling of the End of DXE Event.
PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers. For example, PI platform code may choose to use notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() function.”


SMM environment is a *runtime* environment. SMRAM will be even changed after SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.


Thanks,
Star
From: Abhishek Singh [mailto:abh@cs.unc.edu]
Sent: Tuesday, May 29, 2018 2:02 AM
To: Marvin Häuser <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: Re: [edk2] smm lock query

Thank you everyone for your inputs and clarifications. They are helping me to better understand the uefi code, to which I am very new. I do not mean to hijack the thread: so please continue your discussions about whether the implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my research, I wish to dump the contents of SMRAM when it has reached steady state, i.e., all the drivers have made changes to smram and it has been locked. The current implementation (smm ipl) locks smram when it receives the SmmReadyToLock event and then propagates the event to the smm drivers that make further changes to smram. Unfortunately, I cannot take a snapshot of smram after it has been locked! Thus, I have solved this issue by propagating the event to the smm drivers first (using SmmIplGuidedEventNotify), then opening access to and dumping the contents of SMRAM, and finally closing access to and locking smram. Would it be fair to say that this would give me the fully initialized contents of smram?



My second observation is that despite opening access to smram, I am unable to access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This function “opens” MMRAM so that it is visible while not inside of MM." Note that I am working with minnowboard firmware release 0.97. So some of the binaries like SmmAccess.efi are provided by Intel, while others have been built from the edk source tree: thus, this may not be an EDK issue. Please suggest further steps and/or workarounds. Should I contact edk2-platforms maintainers, or start a new thread here for this issue?

-Abhishek


On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock event and therefor is aware.
However, they might rely on the protocol's description that the resources are about(!) to be locked and code accordingly, not considering the event characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against edk2's actions, or additional code might be supplied by third parties that do not have tree code access (which, by integration, would be "platform binaries" by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this discrepancy from the specification, such as the internal "dummy event" I proposed.

Thanks,
Marvin.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Abhishek Singh <abh@cs.unc.edu<mailto:abh@cs.unc.edu>>;
> ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>; eric.dong@intel.com<mailto:eric.dong@intel.com>; star.zeng@intel.com<mailto:star.zeng@intel.com>
> Subject: Re: [edk2] smm lock query
>
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
> >>
> >> Good day Abhishek,
> >>
> >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> >>
> >> Strictly speaking you are, right, because of the description for the MM
> protocol:
> >> "Indicates that MM resources and services that should not be used by the
> third party code are about[Marvin: (!)] to be locked."
> >> Practically however, I don't see any issue with the current
> implementation. Code inside MMRAM is not affected directly by the lock, it is
> just notified.
> >> However, either the code or the specification should be slightly updated
> to be in sync. A code update might require review of the caller assumptions,
> just to be sure.
> >>
> >> I have a different concern though and hope I'm actually overlooking
> something.
> >> If I understand the code correctly, it is the Platform BDS that exposes the
> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> lock SMM resources based on the event.
> >> Because of latter being an event however, I don't think it is, or can be,
> guaranteed to be the last event group member executing.
> >> When it is not the last, the "about to be locked" part is not true for any
> subsequent callbacks, that could actually be a risky break of the specification
> - if it is.
> >> If it is a break of the specification, I can only think of letting Platform BDS
> expose an "internal" event group, which is only caught by PiSmmIpl, which
> then drives the actual SmmReadyToLock flow.
> >> This would require updates to all platform trees and hence I would
> propose a temporary backwards-compatibility.
> >>
> >> Any comments? Did I overlook something (I hope)?
> >>
> >
> > Mavvin,
> >
> > You are correct there is no guarantee of order in events. Thanks for cc'ing
> the right folks, as I don't remember all the low level details...
> >
> > In general the idea behind the MM code is it only comes from the platform,
> then by definition that code should be aware when the platform was going
> to lock MM. In a practical sense any MM module that had a depex evaluate
> to true would have dispatched in DXE prior to BDS being launched. In general
> BDS is the code that enumerates PCI and connects devices, thus there is no
> chance for 3rd party software to run before that point in the boot. So in an
> abstract sense that lock represents the end of DXE dispatch.
>
> This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> installed by Plaform BDS before any non-platform binaries get a chance to
> run. In terms of the current PlatformBootManagerLib interfaces, that means
> the protocol should be installed from
> PlatformBootManagerBeforeConsole() (as noted on the API declaration
> itself).
>
> Thanks
> Laszlo


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-29  2:21             ` Yao, Jiewen
@ 2018-05-29  5:17               ` Abhishek Singh
  2018-05-29 14:54                 ` Andrew Fish
  2018-05-29 14:56                 ` Yao, Jiewen
  0 siblings, 2 replies; 12+ messages in thread
From: Abhishek Singh @ 2018-05-29  5:17 UTC (permalink / raw)
  To: Yao, Jiewen
  Cc: Zeng, Star, Marvin H?user, edk2-devel@lists.01.org, Laszlo Ersek,
	afish@apple.com, Ni, Ruiyu, Dong, Eric

Thanks, Jiewen and Star. I was able to figure out that smrr was preventing
me from accessing the contents of smram. It seems to me that this violates
the EFI_MM_ACCESS_PROTOCOL  protocol. I have talked to the edk2-platform
maintainers about this on a private thread, but, of course, I may be
mistaken.

I agree that smm is a runtime environment, but you must also agree that
there is an initialization or loading phase in which all the smi handlers
are loaded into smram. Theoretically, new handlers may keep getting
installed as smi's are received but I doubt that this is the case. There
must be a point at which the code (not necessarily the data) is supposed to
be fixed in smram. My guess is that that point is after the SMI handlers
have responded to the SmmReadyToLock event, but I would like to know if you
disagree.

I am definitely not seeking to add the smram dumping code as a production
feature. I am merely interested in using it for my research.

I may have to resort to writing an smm driver, as you suggest Jiewen, but
currently I am just trying to dump smram contents from smm ipl, having
disabled smrr in SmmCpuFeaturesLib. The problem, in both cases, is that the
smram range is quite large (around 8 MB) and cannot fit in a single UEFI
variable. Do you have any suggestions on how to actually dump out this
large range?

Thanks again,
Abhishek


On Mon, May 28, 2018 at 10:21 PM, Yao, Jiewen <jiewen.yao@intel.com> wrote:

> Let me share my thought.
>
> 1)      From interface point of view, ReadyToLock means it is the last
> time to lock. But it does not mean it must be open before.
>
> As implementation choice, a platform MAY lock it earlier.
>
>
>
> Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN
> set.
>
>
>
> 2)      Dumping SMRAM exposes the secret inside of SMRAM, I would treat
> it as debug feature only, not a production.
>
>
>
> If you want to debug, you can add a debug SMM driver and expose an
> interface to copy SMRAM content out.
>
>
>
> Thank you
>
> Yao Jiewen
>
>
>
> *From:* Zeng, Star
> *Sent:* Monday, May 28, 2018 6:16 PM
> *To:* Abhishek Singh <abh@cs.unc.edu>; Marvin H?user <
> Marvin.Haeuser@outlook.com>
> *Cc:* edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>;
> afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <
> eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <
> jiewen.yao@intel.com>
> *Subject:* RE: [edk2] smm lock query
>
>
>
> I do not see issue according to the spec.
>
> Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
>
> DxeSmmReadyToLock event is to notify DXE handlers.
>
> Modules are responsible to lock or protect their resource and effect the
> appropriate protections in their notification handlers.
>
> SmmIplGuidedEventNotify is used to inform SMM environment to signal
> SmmReadyToLock.
>
> SmmReadyToLock event is to notify SMM handlers.
>
>
>
> DXE handlers could not touch SMRAM (after SMRAM is locked or even after
> SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).
>
>
>
> “This protocol in tandem with the *End of DXE Even*t facilitates
> transition of the platform from the environment where all of the components
> are under the authority of the platform manufacturer to the environment
> where third party extensible modules such as UEFI drivers and UEFI
> applications are executed.
>
> The protocol is published immediately after signaling of the *End of DXE
> Event*.
>
> PI modules that need to lock or protect their resources in anticipation of
> the invocation of 3rd party extensible modules should register for
> notification on installation of this protocol and effect the appropriate
> protections in their notification handlers. For example, PI platform code
> may choose to use notification handler to lock MM by invoking *EFI_MM_ACCESS_PROTOCOL.Lock()
> *function.”
>
>
>
>
>
> SMM environment is a *runtime* environment. SMRAM will be even changed
> after SmmReadyToLock, for example, by SMM handler for SMM communication
> from DXE.
>
>
>
>
>
> Thanks,
>
> Star
>
> *From:* Abhishek Singh [mailto:abh@cs.unc.edu <abh@cs.unc.edu>]
> *Sent:* Tuesday, May 29, 2018 2:02 AM
> *To:* Marvin Häuser <Marvin.Haeuser@outlook.com>
> *Cc:* edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>;
> afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <
> eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> *Subject:* Re: [edk2] smm lock query
>
>
>
> Thank you everyone for your inputs and clarifications. They are helping me
> to better understand the uefi code, to which I am very new. I do not mean
> to hijack the thread: so please continue your discussions about whether the
> implementation matches the spec.
>
>
>
> However, I want to state why I am interested in the IPL code. For my
> research, I wish to dump the contents of SMRAM when it has reached steady
> state, i.e., all the drivers have made changes to smram and it has been
> locked. The current implementation (smm ipl) locks smram when it receives
> the SmmReadyToLock event and then propagates the event to the smm drivers
> that make further changes to smram. Unfortunately, I cannot take a snapshot
> of smram after it has been locked! Thus, I have solved this issue by
> propagating the event to the smm drivers first (using
> SmmIplGuidedEventNotify), then opening access to and dumping the contents
> of SMRAM, and finally closing access to and locking smram. Would it be fair
> to say that this would give me the fully initialized contents of smram?
>
>
>
>
>
>
>
> My second observation is that despite opening access to smram, I am unable
> to access its contents, which is a violation of the
> EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This
> function “opens” MMRAM so that it is visible while not inside of MM." Note
> that I am working with minnowboard firmware release 0.97. So some of the
> binaries like SmmAccess.efi are provided by Intel, while others have been
> built from the edk source tree: thus, this may not be an EDK issue. Please
> suggest further steps and/or workarounds. Should I contact edk2-platforms
> maintainers, or start a new thread here for this issue?
>
>
>
> -Abhishek
>
>
>
>
>
> On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <
> Marvin.Haeuser@outlook.com> wrote:
>
> Hello Andrew and Laszlo,
>
> Thanks for your comments!
> Of course I'm with you that it is the platform that signals the
> SmmReadyToLock event and therefor is aware.
> However, they might rely on the protocol's description that the resources
> are about(!) to be locked and code accordingly, not considering the event
> characteristic of the handler in PiSmmIpl.
> The code might be written by different people, not especially reviewed
> against edk2's actions, or additional code might be supplied by third
> parties that do not have tree code access (which, by integration, would be
> "platform binaries" by the definition applying here).
>
> Therefor I would still ask everyone to consider figuring out a solution to
> this discrepancy from the specification, such as the internal "dummy event"
> I proposed.
>
> Thanks,
> Marvin.
>
>
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com>
> > Sent: Monday, May 28, 2018 12:58 PM
> > To: Andrew Fish <afish@apple.com>; Marvin H?user
> > <Marvin.Haeuser@outlook.com>
> > Cc: edk2-devel@lists.01.org; Abhishek Singh <abh@cs.unc.edu>;
> > ruiyu.ni@intel.com; eric.dong@intel.com; star.zeng@intel.com
> > Subject: Re: [edk2] smm lock query
> >
> > On 05/27/18 22:44, Andrew Fish wrote:
> > >
> > >
> > >> On May 27, 2018, at 9:47 AM, Marvin H?user
> > <Marvin.Haeuser@outlook.com> wrote:
> > >>
> > >> Good day Abhishek,
> > >>
> > >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> > (exposes the ReadyToLock protocol) and Laszlo for his high-quality
> answers.
> > >>
> > >> Strictly speaking you are, right, because of the description for the
> MM
> > protocol:
> > >> "Indicates that MM resources and services that should not be used by
> the
> > third party code are about[Marvin: (!)] to be locked."
> > >> Practically however, I don't see any issue with the current
> > implementation. Code inside MMRAM is not affected directly by the lock,
> it is
> > just notified.
> > >> However, either the code or the specification should be slightly
> updated
> > to be in sync. A code update might require review of the caller
> assumptions,
> > just to be sure.
> > >>
> > >> I have a different concern though and hope I'm actually overlooking
> > something.
> > >> If I understand the code correctly, it is the Platform BDS that
> exposes the
> > (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> > lock SMM resources based on the event.
> > >> Because of latter being an event however, I don't think it is, or can
> be,
> > guaranteed to be the last event group member executing.
> > >> When it is not the last, the "about to be locked" part is not true
> for any
> > subsequent callbacks, that could actually be a risky break of the
> specification
> > - if it is.
> > >> If it is a break of the specification, I can only think of letting
> Platform BDS
> > expose an "internal" event group, which is only caught by PiSmmIpl, which
> > then drives the actual SmmReadyToLock flow.
> > >> This would require updates to all platform trees and hence I would
> > propose a temporary backwards-compatibility.
> > >>
> > >> Any comments? Did I overlook something (I hope)?
> > >>
> > >
> > > Mavvin,
> > >
> > > You are correct there is no guarantee of order in events. Thanks for
> cc'ing
> > the right folks, as I don't remember all the low level details...
> > >
> > > In general the idea behind the MM code is it only comes from the
> platform,
> > then by definition that code should be aware when the platform was going
> > to lock MM. In a practical sense any MM module that had a depex evaluate
> > to true would have dispatched in DXE prior to BDS being launched. In
> general
> > BDS is the code that enumerates PCI and connects devices, thus there is
> no
> > chance for 3rd party software to run before that point in the boot. So
> in an
> > abstract sense that lock represents the end of DXE dispatch.
> >
> > This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> > installed by Plaform BDS before any non-platform binaries get a chance to
> > run. In terms of the current PlatformBootManagerLib interfaces, that
> means
> > the protocol should be installed from
> > PlatformBootManagerBeforeConsole() (as noted on the API declaration
> > itself).
> >
> > Thanks
> > Laszlo
>
>
>


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-29  5:17               ` Abhishek Singh
@ 2018-05-29 14:54                 ` Andrew Fish
  2018-05-29 14:56                 ` Yao, Jiewen
  1 sibling, 0 replies; 12+ messages in thread
From: Andrew Fish @ 2018-05-29 14:54 UTC (permalink / raw)
  To: Abhishek Singh
  Cc: Yao, Jiewen, Zeng, Star, Marvin H?user, edk2-devel@lists.01.org,
	Laszlo Ersek, Ni, Ruiyu, Dong, Eric



> On May 28, 2018, at 10:17 PM, Abhishek Singh <abh@cs.unc.edu> wrote:
> 
> Thanks, Jiewen and Star. I was able to figure out that smrr was preventing me from accessing the contents of smram. It seems to me that this violates the EFI_MM_ACCESS_PROTOCOL   protocol. I have talked to the edk2-platform maintainers about this on a private thread, but, of course, I may be mistaken.
> 
> I agree that smm is a runtime environment, but you must also agree that there is an initialization or loading phase in which all the smi handlers are loaded into smram. Theoretically, new handlers may keep getting installed as smi's are received but I doubt that this is the case. There must be a point at which the code (not necessarily the data) is supposed to be fixed in smram. My guess is that that point is after the SMI handlers have responded to the SmmReadyToLock event, but I would like to know if you disagree.
> 
> I am definitely not seeking to add the smram dumping code as a production feature. I am merely interested in using it for my research.
> 
> I may have to resort to writing an smm driver, as you suggest Jiewen, but currently I am just trying to dump smram contents from smm ipl, having disabled smrr in SmmCpuFeaturesLib. The problem, in both cases, is that the smram range is quite large (around 8 MB) and cannot fit in a single UEFI variable. Do you have any suggestions on how to actually dump out this large range?
> 

Abhishek,

If you can write a variable you can write to memory. You can allocate EfiACPIMemoryNVS memory, and that memory will not be used by the OS. Then the variable can point to that memory region. 

Thanks,

Andrew Fish

> Thanks again,
> Abhishek
> 
> 
> On Mon, May 28, 2018 at 10:21 PM, Yao, Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>> wrote:
> Let me share my thought.
> 
> 1)      From interface point of view, ReadyToLock means it is the last time to lock. But it does not mean it must be open before.
> 
> As implementation choice, a platform MAY lock it earlier.
> 
>  
> 
> Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN set.
> 
>  
> 
> 2)      Dumping SMRAM exposes the secret inside of SMRAM, I would treat it as debug feature only, not a production.
> 
>  
> 
> If you want to debug, you can add a debug SMM driver and expose an interface to copy SMRAM content out.
> 
>   <>
> Thank you
> 
> Yao Jiewen
> 
>  
> 
>  <>From: Zeng, Star 
> Sent: Monday, May 28, 2018 6:16 PM
> To: Abhishek Singh <abh@cs.unc.edu <mailto:abh@cs.unc.edu>>; Marvin H?user <Marvin.Haeuser@outlook.com <mailto:Marvin.Haeuser@outlook.com>>
> Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; afish@apple.com <mailto:afish@apple.com>; Ni, Ruiyu <ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>>; Dong, Eric <eric.dong@intel.com <mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com <mailto:star.zeng@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com <mailto:jiewen.yao@intel.com>>
> Subject: RE: [edk2] smm lock query
> 
>  
> 
> I do not see issue according to the spec.
> 
> Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
> 
> DxeSmmReadyToLock event is to notify DXE handlers.
> 
> Modules are responsible to lock or protect their resource and effect the appropriate protections in their notification handlers.
> 
> SmmIplGuidedEventNotify is used to inform SMM environment to signal SmmReadyToLock.
> 
> SmmReadyToLock event is to notify SMM handlers.
> 
>  
> 
> DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).
> 
>  
> 
> “This protocol in tandem with the End of DXE Event facilitates transition of the platform from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party extensible modules such as UEFI drivers and UEFI applications are executed.
> 
> The protocol is published immediately after signaling of the End of DXE Event.
> 
> PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers. For example, PI platform code may choose to use notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() function.”
> 
>  
> 
>  
> 
> SMM environment is a *runtime* environment. SMRAM will be even changed after SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.
> 
>  
> 
>  
> 
> Thanks,
> 
> Star
> 
> From: Abhishek Singh [mailto:abh@cs.unc.edu <mailto:abh@cs.unc.edu>] 
> Sent: Tuesday, May 29, 2018 2:02 AM
> To: Marvin Häuser <Marvin.Haeuser@outlook.com <mailto:Marvin.Haeuser@outlook.com>>
> Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>; afish@apple.com <mailto:afish@apple.com>; Ni, Ruiyu <ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>>; Dong, Eric <eric.dong@intel.com <mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com <mailto:star.zeng@intel.com>>
> Subject: Re: [edk2] smm lock query
> 
>  
> 
> Thank you everyone for your inputs and clarifications. They are helping me to better understand the uefi code, to which I am very new. I do not mean to hijack the thread: so please continue your discussions about whether the implementation matches the spec.
> 
>  
> 
> However, I want to state why I am interested in the IPL code. For my research, I wish to dump the contents of SMRAM when it has reached steady state, i.e., all the drivers have made changes to smram and it has been locked. The current implementation (smm ipl) locks smram when it receives the SmmReadyToLock event and then propagates the event to the smm drivers that make further changes to smram. Unfortunately, I cannot take a snapshot of smram after it has been locked! Thus, I have solved this issue by propagating the event to the smm drivers first (using SmmIplGuidedEventNotify), then opening access to and dumping the contents of SMRAM, and finally closing access to and locking smram. Would it be fair to say that this would give me the fully initialized contents of smram?
> 
>  
> 
>  
> 
>  
> 
> My second observation is that despite opening access to smram, I am unable to access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This function “opens” MMRAM so that it is visible while not inside of MM." Note that I am working with minnowboard firmware release 0.97. So some of the binaries like SmmAccess.efi are provided by Intel, while others have been built from the edk source tree: thus, this may not be an EDK issue. Please suggest further steps and/or workarounds. Should I contact edk2-platforms maintainers, or start a new thread here for this issue?
> 
>  
> 
> -Abhishek
> 
>  
> 
>  
> 
> On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <Marvin.Haeuser@outlook.com <mailto:Marvin.Haeuser@outlook.com>> wrote:
> 
> Hello Andrew and Laszlo,
> 
> Thanks for your comments!
> Of course I'm with you that it is the platform that signals the SmmReadyToLock event and therefor is aware.
> However, they might rely on the protocol's description that the resources are about(!) to be locked and code accordingly, not considering the event characteristic of the handler in PiSmmIpl.
> The code might be written by different people, not especially reviewed against edk2's actions, or additional code might be supplied by third parties that do not have tree code access (which, by integration, would be "platform binaries" by the definition applying here).
> 
> Therefor I would still ask everyone to consider figuring out a solution to this discrepancy from the specification, such as the internal "dummy event" I proposed.
> 
> Thanks,
> Marvin.
> 
> 
> > -----Original Message-----
> > From: Laszlo Ersek <lersek@redhat.com <mailto:lersek@redhat.com>>
> > Sent: Monday, May 28, 2018 12:58 PM
> > To: Andrew Fish <afish@apple.com <mailto:afish@apple.com>>; Marvin H?user
> > <Marvin.Haeuser@outlook.com <mailto:Marvin.Haeuser@outlook.com>>
> > Cc: edk2-devel@lists.01.org <mailto:edk2-devel@lists.01.org>; Abhishek Singh <abh@cs.unc.edu <mailto:abh@cs.unc.edu>>;
> > ruiyu.ni@intel.com <mailto:ruiyu.ni@intel.com>; eric.dong@intel.com <mailto:eric.dong@intel.com>; star.zeng@intel.com <mailto:star.zeng@intel.com>
> > Subject: Re: [edk2] smm lock query
> > 
> > On 05/27/18 22:44, Andrew Fish wrote:
> > >
> > >
> > >> On May 27, 2018, at 9:47 AM, Marvin H?user
> > <Marvin.Haeuser@outlook.com <mailto:Marvin.Haeuser@outlook.com>> wrote:
> > >>
> > >> Good day Abhishek,
> > >>
> > >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> > (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> > >>
> > >> Strictly speaking you are, right, because of the description for the MM
> > protocol:
> > >> "Indicates that MM resources and services that should not be used by the
> > third party code are about[Marvin: (!)] to be locked."
> > >> Practically however, I don't see any issue with the current
> > implementation. Code inside MMRAM is not affected directly by the lock, it is
> > just notified.
> > >> However, either the code or the specification should be slightly updated
> > to be in sync. A code update might require review of the caller assumptions,
> > just to be sure.
> > >>
> > >> I have a different concern though and hope I'm actually overlooking
> > something.
> > >> If I understand the code correctly, it is the Platform BDS that exposes the
> > (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> > lock SMM resources based on the event.
> > >> Because of latter being an event however, I don't think it is, or can be,
> > guaranteed to be the last event group member executing.
> > >> When it is not the last, the "about to be locked" part is not true for any
> > subsequent callbacks, that could actually be a risky break of the specification
> > - if it is.
> > >> If it is a break of the specification, I can only think of letting Platform BDS
> > expose an "internal" event group, which is only caught by PiSmmIpl, which
> > then drives the actual SmmReadyToLock flow.
> > >> This would require updates to all platform trees and hence I would
> > propose a temporary backwards-compatibility.
> > >>
> > >> Any comments? Did I overlook something (I hope)?
> > >>
> > >
> > > Mavvin,
> > >
> > > You are correct there is no guarantee of order in events. Thanks for cc'ing
> > the right folks, as I don't remember all the low level details...
> > >
> > > In general the idea behind the MM code is it only comes from the platform,
> > then by definition that code should be aware when the platform was going
> > to lock MM. In a practical sense any MM module that had a depex evaluate
> > to true would have dispatched in DXE prior to BDS being launched. In general
> > BDS is the code that enumerates PCI and connects devices, thus there is no
> > chance for 3rd party software to run before that point in the boot. So in an
> > abstract sense that lock represents the end of DXE dispatch.
> > 
> > This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> > installed by Plaform BDS before any non-platform binaries get a chance to
> > run. In terms of the current PlatformBootManagerLib interfaces, that means
> > the protocol should be installed from
> > PlatformBootManagerBeforeConsole() (as noted on the API declaration
> > itself).
> > 
> > Thanks
> > Laszlo
> 
>  
> 
> 



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: smm lock query
  2018-05-29  5:17               ` Abhishek Singh
  2018-05-29 14:54                 ` Andrew Fish
@ 2018-05-29 14:56                 ` Yao, Jiewen
  1 sibling, 0 replies; 12+ messages in thread
From: Yao, Jiewen @ 2018-05-29 14:56 UTC (permalink / raw)
  To: Abhishek Singh
  Cc: Zeng, Star, Marvin H?user, edk2-devel@lists.01.org, Laszlo Ersek,
	afish@apple.com, Ni, Ruiyu, Dong, Eric, Yao, Jiewen

I am not sure the purpose of using Variable to store SMRAM.

But if you just want to know the content, you can allocate a normal DRAM buffer, and just save the buffer address and length in the UEFI variable.

That will save much variable size.

Later, you can write a UEFI Shell app to read the variable, get the memory address, and save the content in a file on the file system.

Thank you
Yao Jiewen

From: Abhishek Singh [mailto:abh@cs.unc.edu]
Sent: Monday, May 28, 2018 10:18 PM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: Zeng, Star <star.zeng@intel.com>; Marvin H?user <Marvin.Haeuser@outlook.com>; edk2-devel@lists.01.org; Laszlo Ersek <lersek@redhat.com>; afish@apple.com; Ni, Ruiyu <ruiyu.ni@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: Re: [edk2] smm lock query

Thanks, Jiewen and Star. I was able to figure out that smrr was preventing me from accessing the contents of smram. It seems to me that this violates the EFI_MM_ACCESS_PROTOCOL  protocol. I have talked to the edk2-platform maintainers about this on a private thread, but, of course, I may be mistaken.

I agree that smm is a runtime environment, but you must also agree that there is an initialization or loading phase in which all the smi handlers are loaded into smram. Theoretically, new handlers may keep getting installed as smi's are received but I doubt that this is the case. There must be a point at which the code (not necessarily the data) is supposed to be fixed in smram. My guess is that that point is after the SMI handlers have responded to the SmmReadyToLock event, but I would like to know if you disagree.

I am definitely not seeking to add the smram dumping code as a production feature. I am merely interested in using it for my research.

I may have to resort to writing an smm driver, as you suggest Jiewen, but currently I am just trying to dump smram contents from smm ipl, having disabled smrr in SmmCpuFeaturesLib. The problem, in both cases, is that the smram range is quite large (around 8 MB) and cannot fit in a single UEFI variable. Do you have any suggestions on how to actually dump out this large range?

Thanks again,
Abhishek


On Mon, May 28, 2018 at 10:21 PM, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
Let me share my thought.

1)      From interface point of view, ReadyToLock means it is the last time to lock. But it does not mean it must be open before.
As implementation choice, a platform MAY lock it earlier.

Also SMRR may force the SMRAM invisible to outside SMRAM, even with D_OPEN set.


2)      Dumping SMRAM exposes the secret inside of SMRAM, I would treat it as debug feature only, not a production.

If you want to debug, you can add a debug SMM driver and expose an interface to copy SMRAM content out.

Thank you
Yao Jiewen

From: Zeng, Star
Sent: Monday, May 28, 2018 6:16 PM
To: Abhishek Singh <abh@cs.unc.edu<mailto:abh@cs.unc.edu>>; Marvin H?user <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>; Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>>
Subject: RE: [edk2] smm lock query

I do not see issue according to the spec.
Platform should know when to signal DxeSmmReadyToLock (after EndOfDxe).
DxeSmmReadyToLock event is to notify DXE handlers.
Modules are responsible to lock or protect their resource and effect the appropriate protections in their notification handlers.
SmmIplGuidedEventNotify is used to inform SMM environment to signal SmmReadyToLock.
SmmReadyToLock event is to notify SMM handlers.

DXE handlers could not touch SMRAM (after SMRAM is locked or even after SMRR is configured in PiSmmCpuDxeSmm if I know it is correct).

“This protocol in tandem with the End of DXE Event facilitates transition of the platform from the environment where all of the components are under the authority of the platform manufacturer to the environment where third party extensible modules such as UEFI drivers and UEFI applications are executed.

The protocol is published immediately after signaling of the End of DXE Event.
PI modules that need to lock or protect their resources in anticipation of the invocation of 3rd party extensible modules should register for notification on installation of this protocol and effect the appropriate protections in their notification handlers. For example, PI platform code may choose to use notification handler to lock MM by invoking EFI_MM_ACCESS_PROTOCOL.Lock() function.”


SMM environment is a *runtime* environment. SMRAM will be even changed after SmmReadyToLock, for example, by SMM handler for SMM communication from DXE.


Thanks,
Star
From: Abhishek Singh [mailto:abh@cs.unc.edu]
Sent: Tuesday, May 29, 2018 2:02 AM
To: Marvin Häuser <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>; afish@apple.com<mailto:afish@apple.com>; Ni, Ruiyu <ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>>; Dong, Eric <eric.dong@intel.com<mailto:eric.dong@intel.com>>; Zeng, Star <star.zeng@intel.com<mailto:star.zeng@intel.com>>
Subject: Re: [edk2] smm lock query

Thank you everyone for your inputs and clarifications. They are helping me to better understand the uefi code, to which I am very new. I do not mean to hijack the thread: so please continue your discussions about whether the implementation matches the spec.

However, I want to state why I am interested in the IPL code. For my research, I wish to dump the contents of SMRAM when it has reached steady state, i.e., all the drivers have made changes to smram and it has been locked. The current implementation (smm ipl) locks smram when it receives the SmmReadyToLock event and then propagates the event to the smm drivers that make further changes to smram. Unfortunately, I cannot take a snapshot of smram after it has been locked! Thus, I have solved this issue by propagating the event to the smm drivers first (using SmmIplGuidedEventNotify), then opening access to and dumping the contents of SMRAM, and finally closing access to and locking smram. Would it be fair to say that this would give me the fully initialized contents of smram?



My second observation is that despite opening access to smram, I am unable to access its contents, which is a violation of the EFI_MM_ACCESS_PROTOCOL.Open() description in the spec, which says: "This function “opens” MMRAM so that it is visible while not inside of MM." Note that I am working with minnowboard firmware release 0.97. So some of the binaries like SmmAccess.efi are provided by Intel, while others have been built from the edk source tree: thus, this may not be an EDK issue. Please suggest further steps and/or workarounds. Should I contact edk2-platforms maintainers, or start a new thread here for this issue?

-Abhishek


On Mon, May 28, 2018 at 10:03 AM, Marvin Häuser <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
Hello Andrew and Laszlo,

Thanks for your comments!
Of course I'm with you that it is the platform that signals the SmmReadyToLock event and therefor is aware.
However, they might rely on the protocol's description that the resources are about(!) to be locked and code accordingly, not considering the event characteristic of the handler in PiSmmIpl.
The code might be written by different people, not especially reviewed against edk2's actions, or additional code might be supplied by third parties that do not have tree code access (which, by integration, would be "platform binaries" by the definition applying here).

Therefor I would still ask everyone to consider figuring out a solution to this discrepancy from the specification, such as the internal "dummy event" I proposed.

Thanks,
Marvin.

> -----Original Message-----
> From: Laszlo Ersek <lersek@redhat.com<mailto:lersek@redhat.com>>
> Sent: Monday, May 28, 2018 12:58 PM
> To: Andrew Fish <afish@apple.com<mailto:afish@apple.com>>; Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>>
> Cc: edk2-devel@lists.01.org<mailto:edk2-devel@lists.01.org>; Abhishek Singh <abh@cs.unc.edu<mailto:abh@cs.unc.edu>>;
> ruiyu.ni@intel.com<mailto:ruiyu.ni@intel.com>; eric.dong@intel.com<mailto:eric.dong@intel.com>; star.zeng@intel.com<mailto:star.zeng@intel.com>
> Subject: Re: [edk2] smm lock query
>
> On 05/27/18 22:44, Andrew Fish wrote:
> >
> >
> >> On May 27, 2018, at 9:47 AM, Marvin H?user
> <Marvin.Haeuser@outlook.com<mailto:Marvin.Haeuser@outlook.com>> wrote:
> >>
> >> Good day Abhishek,
> >>
> >> I CC'd the MdeModulePkg maintainers, Ruiyu for the Platform BDS aspect
> (exposes the ReadyToLock protocol) and Laszlo for his high-quality answers.
> >>
> >> Strictly speaking you are, right, because of the description for the MM
> protocol:
> >> "Indicates that MM resources and services that should not be used by the
> third party code are about[Marvin: (!)] to be locked."
> >> Practically however, I don't see any issue with the current
> implementation. Code inside MMRAM is not affected directly by the lock, it is
> just notified.
> >> However, either the code or the specification should be slightly updated
> to be in sync. A code update might require review of the caller assumptions,
> just to be sure.
> >>
> >> I have a different concern though and hope I'm actually overlooking
> something.
> >> If I understand the code correctly, it is the Platform BDS that exposes the
> (S)MmReadyToLock protocol. PiSmmIpl seems to consume that event and
> lock SMM resources based on the event.
> >> Because of latter being an event however, I don't think it is, or can be,
> guaranteed to be the last event group member executing.
> >> When it is not the last, the "about to be locked" part is not true for any
> subsequent callbacks, that could actually be a risky break of the specification
> - if it is.
> >> If it is a break of the specification, I can only think of letting Platform BDS
> expose an "internal" event group, which is only caught by PiSmmIpl, which
> then drives the actual SmmReadyToLock flow.
> >> This would require updates to all platform trees and hence I would
> propose a temporary backwards-compatibility.
> >>
> >> Any comments? Did I overlook something (I hope)?
> >>
> >
> > Mavvin,
> >
> > You are correct there is no guarantee of order in events. Thanks for cc'ing
> the right folks, as I don't remember all the low level details...
> >
> > In general the idea behind the MM code is it only comes from the platform,
> then by definition that code should be aware when the platform was going
> to lock MM. In a practical sense any MM module that had a depex evaluate
> to true would have dispatched in DXE prior to BDS being launched. In general
> BDS is the code that enumerates PCI and connects devices, thus there is no
> chance for 3rd party software to run before that point in the boot. So in an
> abstract sense that lock represents the end of DXE dispatch.
>
> This is my understanding as well. gEfiDxeSmmReadyToLockProtocolGuid is
> installed by Plaform BDS before any non-platform binaries get a chance to
> run. In terms of the current PlatformBootManagerLib interfaces, that means
> the protocol should be installed from
> PlatformBootManagerBeforeConsole() (as noted on the API declaration
> itself).
>
> Thanks
> Laszlo



^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-05-29 14:56 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-05-26 15:04 smm lock query Abhishek Singh
2018-05-27 16:47 ` Marvin H?user
2018-05-27 20:44   ` Andrew Fish
2018-05-27 20:45     ` Andrew Fish
2018-05-28 10:57     ` Laszlo Ersek
2018-05-28 14:03       ` Marvin Häuser
2018-05-28 18:02         ` Abhishek Singh
2018-05-29  1:15           ` Zeng, Star
2018-05-29  2:21             ` Yao, Jiewen
2018-05-29  5:17               ` Abhishek Singh
2018-05-29 14:54                 ` Andrew Fish
2018-05-29 14:56                 ` Yao, Jiewen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox