On 10.10.17 14:23, Rob Clark wrote: > An event can be created with type==0, Shell.efi does this for an event > that is set when Ctrl-C is typed. So our current approach of having a > fixed set of timer slots, and determining which slots are unused by > type==0 doesn't work so well. But we don't have any particularly good > reason to have a fixed table of events, so just dynamically allocate > them and keep a list. > > Also fixes an incorrect implementation of CheckEvent() which was (a) > incorrectly returning an error if type==0, and (b) didn't handle the > case of an unsignaled event with a notify callback. > > With these fixes (plus implementation of SIMPLE_TEXT_INPUT_EX protocol), > Ctrl-C works in Shell.efi. > > Signed-off-by: Rob Clark <robdcl...@gmail.com> > --- > include/efi_loader.h | 1 + > lib/efi_loader/efi_boottime.c | 217 > +++++++++++++++++++++--------------------- > 2 files changed, 111 insertions(+), 107 deletions(-) > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index e6e55d2cb4..2232caca44 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -154,6 +154,7 @@ struct efi_event { > enum efi_timer_delay trigger_type; > bool is_queued; > bool is_signaled; > + struct list_head link; > }; > > > diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > index 39dcc72648..19fafe546c 100644 > --- a/lib/efi_loader/efi_boottime.c > +++ b/lib/efi_loader/efi_boottime.c > @@ -350,11 +350,26 @@ static efi_status_t efi_create_handle(void **handle) > return r; > } > > +static LIST_HEAD(efi_events); > + > /* > - * Our event capabilities are very limited. Only a small limited > - * number of events is allowed to coexist. > + * Check if a pointer is a valid event. > + * > + * It might be nice at some point to extend this to a more general > + * mechanism to check if pointers passed from the EFI world are > + * valid objects of a particular type. > */ > -static struct efi_event efi_events[16]; > +static bool efi_is_event(const void *obj) > +{ > + struct efi_event *evt; > + > + list_for_each_entry(evt, &efi_events, link) { > + if (evt == obj) > + return true; > + } > + > + return false; > +} > > /* > * Create an event. > @@ -377,7 +392,7 @@ efi_status_t efi_create_event(uint32_t type, UINTN > notify_tpl, > void *context), > void *notify_context, struct efi_event **event) > { > - int i; > + struct efi_event *evt; > > if (event == NULL) > return EFI_INVALID_PARAMETER; > @@ -389,21 +404,24 @@ efi_status_t efi_create_event(uint32_t type, UINTN > notify_tpl, > notify_function == NULL) > return EFI_INVALID_PARAMETER; > > - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { > - if (efi_events[i].type) > - continue; > - efi_events[i].type = type; > - efi_events[i].notify_tpl = notify_tpl; > - efi_events[i].notify_function = notify_function; > - efi_events[i].notify_context = notify_context; > - /* Disable timers on bootup */ > - efi_events[i].trigger_next = -1ULL; > - efi_events[i].is_queued = false; > - efi_events[i].is_signaled = false; > - *event = &efi_events[i]; > - return EFI_SUCCESS; > - } > - return EFI_OUT_OF_RESOURCES; > + evt = calloc(1, sizeof(*evt)); > + if (!evt) > + return EFI_OUT_OF_RESOURCES; > + > + evt->type = type; > + evt->notify_tpl = notify_tpl; > + evt->notify_function = notify_function; > + evt->notify_context = notify_context; > + /* Disable timers on bootup */ > + evt->trigger_next = -1ULL; > + evt->is_queued = false; > + evt->is_signaled = false; > + > + list_add_tail(&evt->link, &efi_events); > + > + *event = evt; > + > + return EFI_SUCCESS; > } > > /* > @@ -443,30 +461,31 @@ static efi_status_t EFIAPI efi_create_event_ext( > */ > void efi_timer_check(void) > { > - int i; > + struct efi_event *evt; > u64 now = timer_get_us(); > > - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { > - if (!efi_events[i].type) > - continue; > - if (efi_events[i].is_queued) > - efi_signal_event(&efi_events[i]); > - if (!(efi_events[i].type & EVT_TIMER) || > - now < efi_events[i].trigger_next) > + /* > + * TODO perhaps optimize a bit and track the time of next > + * timer to expire? > + */ > + list_for_each_entry(evt, &efi_events, link) { > + if (evt->is_queued) > + efi_signal_event(evt); > + if (!(evt->type & EVT_TIMER) || > + now < evt->trigger_next) > continue; > - switch (efi_events[i].trigger_type) { > + switch (evt->trigger_type) { > case EFI_TIMER_RELATIVE: > - efi_events[i].trigger_type = EFI_TIMER_STOP; > + evt->trigger_type = EFI_TIMER_STOP; > break; > case EFI_TIMER_PERIODIC: > - efi_events[i].trigger_next += > - efi_events[i].trigger_time; > + evt->trigger_next += evt->trigger_time; > break; > default: > continue; > } > - efi_events[i].is_signaled = true; > - efi_signal_event(&efi_events[i]); > + evt->is_signaled = true; > + efi_signal_event(evt); > } > WATCHDOG_RESET(); > } > @@ -485,7 +504,8 @@ void efi_timer_check(void) > efi_status_t efi_set_timer(struct efi_event *event, enum efi_timer_delay > type, > uint64_t trigger_time) > { > - int i; > + if (!efi_is_event(event)) > + return EFI_INVALID_PARAMETER; > > /* > * The parameter defines a multiple of 100ns. > @@ -493,30 +513,25 @@ efi_status_t efi_set_timer(struct efi_event *event, > enum efi_timer_delay type, > */ > do_div(trigger_time, 10); > > - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { > - if (event != &efi_events[i]) > - continue; > + if (!(event->type & EVT_TIMER)) > + return EFI_INVALID_PARAMETER; > > - if (!(event->type & EVT_TIMER)) > - break; > - switch (type) { > - case EFI_TIMER_STOP: > - event->trigger_next = -1ULL; > - break; > - case EFI_TIMER_PERIODIC: > - case EFI_TIMER_RELATIVE: > - event->trigger_next = > - timer_get_us() + trigger_time; > - break; > - default: > - return EFI_INVALID_PARAMETER; > - } > - event->trigger_type = type; > - event->trigger_time = trigger_time; > - event->is_signaled = false; > - return EFI_SUCCESS; > + switch (type) { > + case EFI_TIMER_STOP: > + event->trigger_next = -1ULL; > + break; > + case EFI_TIMER_PERIODIC: > + case EFI_TIMER_RELATIVE: > + event->trigger_next = timer_get_us() + trigger_time; > + break; > + default: > + return EFI_INVALID_PARAMETER; > } > - return EFI_INVALID_PARAMETER; > + event->trigger_type = type; > + event->trigger_time = trigger_time; > + event->is_signaled = false; > + > + return EFI_SUCCESS; > } > > /* > @@ -555,7 +570,7 @@ static efi_status_t EFIAPI efi_wait_for_event(unsigned > long num_events, > struct efi_event **event, > size_t *index) > { > - int i, j; > + int i; > > EFI_ENTRY("%ld, %p, %p", num_events, event, index); > > @@ -566,12 +581,8 @@ static efi_status_t EFIAPI efi_wait_for_event(unsigned > long num_events, > if (efi_tpl != TPL_APPLICATION) > return EFI_EXIT(EFI_UNSUPPORTED); > for (i = 0; i < num_events; ++i) { > - for (j = 0; j < ARRAY_SIZE(efi_events); ++j) { > - if (event[i] == &efi_events[j]) > - goto known_event; > - } > - return EFI_EXIT(EFI_INVALID_PARAMETER); > -known_event: > + if (!efi_is_event(event[i])) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > if (!event[i]->type || event[i]->type & EVT_NOTIFY_SIGNAL) > return EFI_EXIT(EFI_INVALID_PARAMETER); > if (!event[i]->is_signaled) > @@ -614,19 +625,12 @@ out: > */ > static efi_status_t EFIAPI efi_signal_event_ext(struct efi_event *event) > { > - int i; > - > EFI_ENTRY("%p", event); > - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { > - if (event != &efi_events[i]) > - continue; > - if (event->is_signaled) > - break; > - event->is_signaled = true; > - if (event->type & EVT_NOTIFY_SIGNAL) > - efi_signal_event(event); > - break; > - } > + if (!efi_is_event(event)) > + return EFI_EXIT(EFI_INVALID_PARAMETER); > + event->is_signaled = true; > + if (event->type & EVT_NOTIFY_SIGNAL) > + efi_signal_event(event); > return EFI_EXIT(EFI_SUCCESS); > } > > @@ -642,19 +646,10 @@ static efi_status_t EFIAPI efi_signal_event_ext(struct > efi_event *event) > */ > static efi_status_t EFIAPI efi_close_event(struct efi_event *event) > { > - int i; > - > EFI_ENTRY("%p", event); > - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) { > - if (event == &efi_events[i]) { > - event->type = 0; > - event->trigger_next = -1ULL; > - event->is_queued = false; > - event->is_signaled = false; > - return EFI_EXIT(EFI_SUCCESS); > - } > - } > - return EFI_EXIT(EFI_INVALID_PARAMETER); > + list_del(&event->link); > + free(event); > + return EFI_EXIT(EFI_SUCCESS); > } > > /* > @@ -664,29 +659,37 @@ static efi_status_t EFIAPI efi_close_event(struct > efi_event *event) > * See the Unified Extensible Firmware Interface (UEFI) specification > * for details. > * > - * If an event is not signaled yet the notification function is queued. > + * - If Event is in the signaled state, it is cleared and EFI_SUCCESS > + * is returned. > + * > + * - If Event is not in the signaled state and has no notification > + * function, EFI_NOT_READY is returned. > + * > + * - If Event is not in the signaled state but does have a notification > + * function, the notification function is queued at the event’s > + * notification task priority level. If the execution of the > + * notification function causes Event to be signaled, then the signaled > + * state is cleared and EFI_SUCCESS is returned; if the Event is not > + * signaled, then EFI_NOT_READY is returned. > * > * @event event to check > * @return status code > */ > -static efi_status_t EFIAPI efi_check_event(struct efi_event *event) > +/* > + */
I assume this is one comment block too much? > +static efi_status_t EFIAPI efi_check_event(struct efi_event *evt) > { Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot