[U-Boot] [PATCH 07/11] efi_loader: fix events
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Oct 10 22:40:14 UTC 2017
On 10/10/2017 02:23 PM, 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 <robdclark at 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)
> +/*
> + */
> +static efi_status_t EFIAPI efi_check_event(struct efi_event *evt)
> {
> - int i;
> -
> - EFI_ENTRY("%p", event);
> + EFI_ENTRY("%p", evt);
> efi_timer_check();
> - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
> - if (event != &efi_events[i])
> - continue;
> - if (!event->type || event->type & EVT_NOTIFY_SIGNAL)
> - break;
> - if (!event->is_signaled)
> - efi_signal_event(event);
> - if (event->is_signaled)
> - return EFI_EXIT(EFI_SUCCESS);
> - return EFI_EXIT(EFI_NOT_READY);
> + if (!efi_is_event(evt) || (evt->type & EVT_NOTIFY_SIGNAL))
> + return EFI_EXIT(EFI_INVALID_PARAMETER);
> + if (!evt->is_signaled && evt->notify_function)
> + EFI_CALL_VOID(evt->notify_function(evt, evt->notify_context));
> + if (evt->is_signaled) {
> + evt->is_signaled = true;
> + return EFI_EXIT(EFI_SUCCESS);
> }
> - return EFI_EXIT(EFI_INVALID_PARAMETER);
> + return EFI_EXIT(EFI_NOT_READY);
> }
>
> /*
> @@ -1440,15 +1443,15 @@ static void efi_exit_caches(void)
> static efi_status_t EFIAPI efi_exit_boot_services(void *image_handle,
> unsigned long map_key)
> {
> - int i;
> + struct efi_event *evt;
>
> EFI_ENTRY("%p, %ld", image_handle, map_key);
>
> /* Notify that ExitBootServices is invoked. */
> - for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
> - if (efi_events[i].type != EVT_SIGNAL_EXIT_BOOT_SERVICES)
> + list_for_each_entry(evt, &efi_events, link) {
> + if (evt->type != EVT_SIGNAL_EXIT_BOOT_SERVICES)
> continue;
> - efi_signal_event(&efi_events[i]);
> + efi_signal_event(evt);
> }
> /* Make sure that notification functions are not called anymore */
> efi_tpl = TPL_HIGH_LEVEL;
>
Thanks for adding efi_is_event().
Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
Acked-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
More information about the U-Boot
mailing list