Andrew--

It accounts for the strange usage I've seen. When the code wants to signal the 
event group, it also has to create an event in the event group. With the 
current codebase, you can see it creating dummy notify functions for this 
event, even though the intention is just to signal the other events in the 
group. For example, see IntelFrameworkModulePkg/Universal/BdsDxe/BdsEntry.c, 
line 144 where it has the usefully-titles reference to 
"BdsEmptyCallbackFunction" or ConSpliter.c (line 590) with 
"ConSplitterEmptyCallbackFunction". Even DXE Core's CoreEmptyCallbackFunction 
in Event.c (line 142)

I could not find anything in CreateEventEx that tries to enforce this, which is 
good.

Tim

-----Original Message-----
From: Andrew Fish [mailto:af...@apple.com] 
Sent: Friday, May 30, 2014 1:15 PM
To: edk2-devel@lists.sourceforge.net
Subject: Re: [edk2] Signalling an event group


On May 30, 2014, at 12:41 PM, justin_johns...@dell.com wrote:

> Andrew,
> Your suggested code makes sense to me, I will give it a try. Besides that 
> sample code snippet, I don't think the spec isn't really explicit; but does 
> imply that the entire group is signaled, regardless of whether the event 
> itself is of type EVT_NOTIFY_SIGNAL.
>  

The reason CreateEventEx() got created was to allow the signaling of a group of 
functions so they could run their notify function. But the example clearly 
shows it is legal to create and Ex event of any type. The spec does say the 
event is signaled and their notification functions are schedule. 

Good point, if all the events got signaled I think CoreNotifySignalList() would 
look more like:

VOID
CoreNotifySignalList (
 IN EFI_GUID     *EventGroup
 )
{
 LIST_ENTRY              *Link;
 LIST_ENTRY              *Head;
 IEVENT                  *Event;

 CoreAcquireEventLock ();

 Head = &gEventSignalQueue;
 for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) {
   Event = CR (Link, IEVENT, SignalLink, EVENT_SIGNATURE);
   if (CompareGuid (&Event->EventGroup, EventGroup)) {
      if ((Event->Type & EVT_NOTIFY_SIGNAL) != 0) {
       CoreNotifyEvent (Event);
     } else if (Event->SignalCount == 0x00000000) {
      //
      // The check for 0 prevents double counting the signaled event. 
      //
      Event->SignalCount++;
     }
   }
 }

 CoreReleaseEventLock ();
}

Probably a good idea to change the name to CoreSignalEventGroup(). 

Thanks,

Andrew Fish

> -- Justin
>  
>  
> -----Original Message-----
> From: Andrew Fish [mailto:af...@apple.com]
> Sent: Friday, May 30, 2014 2:19 PM
> To: edk2-devel@lists.sourceforge.net
> Subject: Re: [edk2] Signalling an event group
> 
> 
> On May 30, 2014, at 11:51 AM, Andrew Fish wrote:
> 
> > 
> > On May 30, 2014, at 11:00 AM, justin_johns...@dell.com wrote:
> > 
> >> Hello all,
> >> I'm working with the DXE core event services and am not sure that the code 
> >> in MdeModulePkg agrees with the UEFI spec.
> >> The situation is this: in several modules I have created an event using 
> >> the CreateEventEx() method, with an EventGroup GUID. Later, I want to 
> >> signal the event group, but the module which does the signaling does not 
> >> have any work to do in response to the event. It would seem that I can 
> >> signal the event by doing as indicated in the UEFI spec:
> >> gBS->CreateEventEx (
> >> 0,
> >> 0,
> >> NULL,
> >> NULL,
> >> &gMyEventGroupGuid,
> >> &Event);
> >> gBS->SignalEvent(Event);
> >> 
> >> However, this does not work with EDKII. I see in Event.c :
> >> if ((Event->Type & EVT_NOTIFY_SIGNAL) != 0) { if (Event->ExFlag) { 
> >> // // The CreateEventEx() style requires all members of the Event 
> >> Group // to be signaled.
> >> //
> >> CoreReleaseEventLock ();
> >> CoreNotifySignalList (&Event->EventGroup); CoreAcquireEventLock ();
> >> 
> >> Which seems to indicate that the event group is only signaled if the event 
> >> being signaled is of type EVT_NOTIFY_SIGNAL. In order to get my event 
> >> group signaled, I must create an event with a dummy callback and give it 
> >> type EVT_NOTIFY_SIGNAL, even though I do not have any work to do in 
> >> response to the event.
> >> 
> >> Is this an error in MdeModulePkg's implementation? Or is the spec 
> >> incomplete?
> >> 
> 
> Justin,
> 
> I think the logic in the DXE Core should look like this:
> 
> if (Event->SignalCount == 0x00000000) {
> Event->SignalCount++;
> 
> if (Event->ExFlag) {
> //
> // The CreateEventEx() style requires all members of the Event Group 
> // to be signaled.
> //
> CoreReleaseEventLock ();
> CoreNotifySignalList (&Event->EventGroup); CoreAcquireEventLock (); } 
> else if ((Event->Type & EVT_NOTIFY_SIGNAL) != 0) { // // If signalling 
> type is a notify function, queue it // CoreNotifyEvent (Event); } }
> 
> So always walk the List for an Ex event, but only notify the event if 
> it is the right type. And the check should be in 
> CoreNotifySignalList()
> 
> VOID
> CoreNotifySignalList (
> IN EFI_GUID *EventGroup
> )
> {
> LIST_ENTRY *Link;
> LIST_ENTRY *Head;
> IEVENT *Event;
> 
> CoreAcquireEventLock ();
> 
> Head = &gEventSignalQueue;
> for (Link = Head->ForwardLink; Link != Head; Link = Link->ForwardLink) 
> { Event = CR (Link, IEVENT, SignalLink, EVENT_SIGNATURE); if 
> (CompareGuid (&Event->EventGroup, EventGroup)) { if ((Event->Type & 
> EVT_NOTIFY_SIGNAL) != 0) { CoreNotifyEvent (Event); } } }
> 
> CoreReleaseEventLock ();
> }
> 
> 
> Man, this code has been busted for a long time.... This bug predates the edk2!
> 
> The edk2 UefiLib even has a Dummy event and tries to hide the need for it 
> from the caller....
> 
> https://svn.code.sf.net/p/edk2/code/trunk/edk2/MdePkg/Library/UefiLib/
> UefiNotTiano.c
> /**
> An empty function to pass error checking of CreateEventEx ().
> 
> This empty function ensures that EVT_NOTIFY_SIGNAL_ALL is error 
> checked correctly since it is now mapped into CreateEventEx() in UEFI 2.0.
> 
> @param Event Event whose notification function is being invoked.
> @param Context The pointer to the notification function's context, 
> which is implementation-dependent.
> 
> **/
> VOID
> EFIAPI
> InternalEmptyFunction (
> IN EFI_EVENT Event,
> IN VOID *Context
> )
> {
> return;
> }
> 
> EFI_STATUS
> EFIAPI
> EfiCreateEventReadyToBootEx (
> IN EFI_TPL NotifyTpl,
> IN EFI_EVENT_NOTIFY NotifyFunction, OPTIONAL IN VOID *NotifyContext, 
> OPTIONAL OUT EFI_EVENT *ReadyToBootEvent
> )
> {
> EFI_STATUS Status;
> EFI_EVENT_NOTIFY WorkerNotifyFunction;
> 
> ASSERT (ReadyToBootEvent != NULL);
> 
> if (gST->Hdr.Revision < EFI_2_00_SYSTEM_TABLE_REVISION) { DEBUG 
> ((EFI_D_ERROR, "EFI1.1 can't support ReadyToBootEvent!")); ASSERT 
> (FALSE);
> 
> return EFI_UNSUPPORTED;
> } else {
> //
> // For UEFI 2.0 and the future use an Event Group // if 
> (NotifyFunction == NULL) { // // CreateEventEx will check 
> NotifyFunction is NULL or not and return error.
> // Use dummy routine for the case NotifyFunction is NULL.
> //
> WorkerNotifyFunction = InternalEmptyFunction; } else { 
> WorkerNotifyFunction = NotifyFunction; } Status = gBS->CreateEventEx ( 
> EVT_NOTIFY_SIGNAL, NotifyTpl, WorkerNotifyFunction, NotifyContext, 
> &gEfiEventReadyToBootGuid, ReadyToBootEvent ); }
> 
> return Status;
> }
> 
> 
> 
> Thanks,
> 
> Andrew Fish
> 
> 
> >> Thanks.
> >> 
> >> --
> >> Justin Johnson
> >> Platform Software Engineer
> >> Dell, Inc.
> >> 
> >> ---------------------------------------------------------------------
> >> --------- Time is money. Stop wasting it! Get your web API in 5 
> >> minutes.
> >> www.restlet.com/download
> >> http://p.sf.net/sfu/restlet__________________________________________
> >> _____
> >> edk2-devel mailing list
> >> edk2-devel@lists.sourceforge.net
> >> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> > 
> > ----------------------------------------------------------------------
> > -------- Time is money. Stop wasting it! Get your web API in 5 
> > minutes.
> > www.restlet.com/download
> > http://p.sf.net/sfu/restlet___________________________________________
> > ____
> > edk2-devel mailing list
> > edk2-devel@lists.sourceforge.net
> > https://lists.sourceforge.net/lists/listinfo/edk2-devel
> 
> 
> ------------------------------------------------------------------------------
> Time is money. Stop wasting it! Get your web API in 5 minutes.
> www.restlet.com/download
> http://p.sf.net/sfu/restlet
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel
> 
> ------------------------------------------------------------------------------
> Time is money. Stop wasting it! Get your web API in 5 minutes.
> www.restlet.com/download
> http://p.sf.net/sfu/restlet_______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/edk2-devel


------------------------------------------------------------------------------
Time is money. Stop wasting it! Get your web API in 5 minutes.
www.restlet.com/download
http://p.sf.net/sfu/restlet
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

------------------------------------------------------------------------------
Time is money. Stop wasting it! Get your web API in 5 minutes.
www.restlet.com/download
http://p.sf.net/sfu/restlet
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/edk2-devel

Reply via email to