[U-Boot] [PATCH 07/11] efi_loader: fix events

Rob Clark robdclark at gmail.com
Tue Oct 10 12:23:03 UTC 2017


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;
-- 
2.13.6



More information about the U-Boot mailing list