public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
From: Rob Clark <robdclark@gmail.com>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH 07/11] efi_loader: fix events
Date: Tue, 10 Oct 2017 08:23:03 -0400	[thread overview]
Message-ID: <20171010122309.25313-8-robdclark@gmail.com> (raw)
In-Reply-To: <20171010122309.25313-1-robdclark@gmail.com>

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@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

  parent reply	other threads:[~2017-10-10 12:23 UTC|newest]

Thread overview: 75+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-10 12:22 [U-Boot] [PATCH 00/11] efi_loader: patches for Shell.efi Rob Clark
2017-10-10 12:22 ` [U-Boot] [PATCH 01/11] efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL Rob Clark
2017-10-11  0:03   ` Heinrich Schuchardt
2017-10-11 14:07   ` Alexander Graf
2017-10-11 20:32     ` Rob Clark
2017-10-12  6:51       ` Heinrich Schuchardt
2017-10-10 12:22 ` [U-Boot] [PATCH 02/11] efi_loader: Initial HII protocols Rob Clark
2017-10-11 14:30   ` Alexander Graf
2017-10-11 22:02     ` Rob Clark
2017-10-12  7:13       ` Alexander Graf
2017-10-12  9:55         ` Rob Clark
2017-10-12  9:59           ` Alexander Graf
2018-09-22 10:34   ` Heinrich Schuchardt
2018-09-23 10:11     ` Alexander Graf
2018-10-03  7:39       ` AKASHI, Takahiro
2018-10-05  8:52         ` AKASHI, Takahiro
2018-10-05  9:49           ` Leif Lindholm
2018-10-05 13:06             ` Alexander Graf
2018-10-09  7:24               ` AKASHI, Takahiro
2018-10-09 17:19                 ` Heinrich Schuchardt
2018-10-10  0:54                   ` AKASHI, Takahiro
2018-10-05 16:24             ` Heinrich Schuchardt
2017-10-10 12:22 ` [U-Boot] [PATCH 03/11] efi_loader: Initial EFI_UNICODE_COLLATION_PROTOCOL Rob Clark
2017-10-11 14:36   ` Alexander Graf
2017-10-11 20:30     ` Rob Clark
2017-10-11 20:47       ` Alexander Graf
2017-10-12 11:54         ` Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 04/11] efi_loader: SIMPLE_TEXT_INPUT_EX plus wire up objects properly Rob Clark
2017-10-11 14:39   ` Alexander Graf
2018-09-04 14:07   ` [U-Boot] [U-Boot, " Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 05/11] efi_loader: console support for color attributes Rob Clark
2017-10-10 23:41   ` Heinrich Schuchardt
2017-10-11 14:41   ` Alexander Graf
2017-10-12 15:24   ` [U-Boot] [U-Boot, " Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 06/11] efi_loader: Decouple EFI input/output from stdin/stdout Rob Clark
2017-10-11 14:45   ` Alexander Graf
2017-10-11 22:07     ` Rob Clark
2017-10-12  7:15       ` Alexander Graf
2017-10-12 12:48         ` Rob Clark
2017-10-12 13:05           ` Heinrich Schuchardt
2017-10-12 13:40             ` Rob Clark
2017-10-12 13:50               ` Alexander Graf
2017-10-12 14:28                 ` Rob Clark
2017-10-12 14:31                   ` Alexander Graf
2017-10-12 16:00                   ` Mark Kettenis
2017-10-12 16:25                     ` Alexander Graf
2017-10-12 22:38                   ` Heinrich Schuchardt
2017-10-12 21:26                     ` Rob Clark
2017-10-12 23:48                       ` Heinrich Schuchardt
2017-10-13  0:41                         ` Rob Clark
2017-10-12 13:11           ` Alexander Graf
2017-10-12 13:42             ` Rob Clark
2017-10-12 13:44           ` Mark Kettenis
2017-10-12 14:24             ` Rob Clark
2017-10-10 12:23 ` Rob Clark [this message]
2017-10-10 22:40   ` [U-Boot] [PATCH 07/11] efi_loader: fix events Heinrich Schuchardt
2017-10-11 14:49   ` Alexander Graf
2017-10-11 22:09     ` Rob Clark
2017-10-13  5:24   ` Heinrich Schuchardt
2017-10-13 14:08     ` Rob Clark
2017-10-10 12:23 ` [U-Boot] [PATCH 08/11] efi_loader: implement SetWatchdogTimer Rob Clark
2017-10-11 14:55   ` Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 09/11] efi_loader: Fix disk dp's for pre-DM/legacy devices Rob Clark
2017-10-11 14:56   ` Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 10/11] efi_loader: Add mem-mapped for fallback Rob Clark
2017-10-10 22:31   ` Heinrich Schuchardt
2017-10-11 14:59   ` Alexander Graf
2017-10-11 22:14     ` Rob Clark
2017-10-12 15:24   ` [U-Boot] [U-Boot, " Alexander Graf
2017-10-10 12:23 ` [U-Boot] [PATCH 11/11] efi_loader: exclude openrd devices Rob Clark
2017-10-10 22:28   ` Heinrich Schuchardt
2017-10-10 22:50     ` Rob Clark
2017-10-11  7:07       ` Stefan Roese
2017-10-11  7:22         ` Alexander Graf
2017-10-11  0:24 ` [U-Boot] [PATCH 00/11] efi_loader: patches for Shell.efi Heinrich Schuchardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171010122309.25313-8-robdclark@gmail.com \
    --to=robdclark@gmail.com \
    --cc=u-boot@lists.denx.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox