> On May 27, 2018, at 1:44 PM, Andrew Fish <[email protected]> wrote: > > > >> On May 27, 2018, at 9:47 AM, Marvin H?user <[email protected]> >> 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 <[email protected]> On Behalf Of >>> Abhishek Singh >>> Sent: Saturday, May 26, 2018 5:05 PM >>> To: [email protected] >>> 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 >>> [email protected] >>> https://lists.01.org/mailman/listinfo/edk2-devel >> _______________________________________________ >> edk2-devel mailing list >> [email protected] >> https://lists.01.org/mailman/listinfo/edk2-devel > > _______________________________________________ > edk2-devel mailing list > [email protected] <mailto:[email protected]> > https://lists.01.org/mailman/listinfo/edk2-devel > <https://lists.01.org/mailman/listinfo/edk2-devel> _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
