diff mbox

[U-Boot,v2,2/7] efi_loader: implement multiple event support

Message ID 20170718181723.1780-3-xypron.glpk@gmx.de
State Accepted
Commit c68415922b340c6b26f94326a6899977a67d61c9
Delegated to: Alexander Graf
Headers show

Commit Message

Heinrich Schuchardt July 18, 2017, 6:17 p.m. UTC 
Up to now the boot time supported only a single event.
This patch now allows four events.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	add TPL constants
	consider multiple events in efi_wait_for_event
	move notification to new function efi_signal_event
---
 include/efi_api.h             |  13 ++-
 include/efi_loader.h          |  24 ++++++
 lib/efi_loader/efi_boottime.c | 195 ++++++++++++++++++++++++++++--------------
 3 files changed, 168 insertions(+), 64 deletions(-)

Comments

Alexander Graf July 19, 2017, 12:30 p.m. UTC | #1 
On 07/18/2017 08:17 PM, Heinrich Schuchardt wrote:
> Up to now the boot time supported only a single event.
> This patch now allows four events.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
> 	add TPL constants
> 	consider multiple events in efi_wait_for_event
> 	move notification to new function efi_signal_event
> ---
>   include/efi_api.h             |  13 ++-
>   include/efi_loader.h          |  24 ++++++
>   lib/efi_loader/efi_boottime.c | 195 ++++++++++++++++++++++++++++--------------
>   3 files changed, 168 insertions(+), 64 deletions(-)
>
> diff --git a/include/efi_api.h b/include/efi_api.h
> index a1f8221111..a3b8e04576 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -28,8 +28,17 @@ enum efi_event_type {
>   	EFI_TIMER_RELATIVE = 2
>   };
>   
> -#define EVT_NOTIFY_WAIT		0x00000100
> -#define EVT_NOTIFY_SIGNAL	0x00000200
> +#define EVT_TIMER				0x80000000
> +#define EVT_RUNTIME				0x40000000
> +#define EVT_NOTIFY_WAIT				0x00000100
> +#define EVT_NOTIFY_SIGNAL			0x00000200
> +#define EVT_SIGNAL_EXIT_BOOT_SERVICES		0x00000201
> +#define EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE	0x60000202
> +
> +#define TPL_APPLICATION		0x04
> +#define TPL_CALLBACK		0x08
> +#define TPL_NOTIFY		0x10
> +#define TPL_HIGH_LEVEL		0x1F
>   
>   struct efi_event;
>   
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index d7847d23e5..3d18bfbd2e 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -63,6 +63,30 @@ struct efi_object {
>   	void *handle;
>   };
>   
> +/**
> + * struct efi_event
> + *
> + * @type:		Type of event, see efi_create_event
> + * @notify_tpl:		Task priority level of notifications
> + * @trigger_time:	Period of the timer
> + * @trigger_next:	Next time to trigger the timer
> + * @nofify_function:	Function to call when the event is triggered
> + * @notify_context:	Data to be passed to the notify function
> + * @trigger_type:	Type of timer, see efi_set_timer
> + * @signaled:		The notify function was already called
> + */
> +struct efi_event {
> +	u32 type;
> +	unsigned long notify_tpl;
> +	void (EFIAPI *notify_function)(struct efi_event *event, void *context);
> +	void *notify_context;
> +	u64 trigger_next;
> +	u64 trigger_time;
> +	enum efi_event_type trigger_type;
> +	int signaled;
> +};
> +
> +
>   /* This list contains all UEFI objects we know of */
>   extern struct list_head efi_obj_list;
>   
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 0eda465359..a49acc8693 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -82,6 +82,18 @@ efi_status_t efi_exit_func(efi_status_t ret)
>   	return ret;
>   }
>   
> +static void efi_signal_event(struct efi_event *event)
> +{
> +	if (event->signaled)
> +		return;
> +	event->signaled = 1;
> +	if (event->type & EVT_NOTIFY_SIGNAL) {
> +		EFI_EXIT(EFI_SUCCESS);
> +		event->notify_function(event, event->notify_context);
> +		EFI_ENTRY("returning from notification function");
> +	}
> +}
> +
>   static efi_status_t efi_unsupported(const char *funcname)
>   {
>   	debug("EFI: App called into unimplemented function %s\n", funcname);
> @@ -163,22 +175,10 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer)
>   }
>   
>   /*
> - * Our event capabilities are very limited. Only support a single
> - * event to exist, so we don't need to maintain lists.
> + * Our event capabilities are very limited. Only a small limited
> + * number of events is allowed to coexist.
>    */
> -static struct efi_event {
> -	enum efi_event_type type;
> -	u32 trigger_type;
> -	u32 trigger_time;
> -	u64 trigger_next;
> -	unsigned long notify_tpl;
> -	void (EFIAPI *notify_function) (struct efi_event *event,
> -					void *context);
> -	void *notify_context;
> -} efi_event = {
> -	/* Disable timers on bootup */
> -	.trigger_next = -1ULL,
> -};
> +static struct efi_event efi_events[16];
>   
>   static efi_status_t EFIAPI efi_create_event(
>   			enum efi_event_type type, ulong notify_tpl,

The argument type of "type" is (incorrectly) enum efi_event_type. Can 
you send a follow-up patch that changes it to u32 (or its own enum) to 
avoid confusion?


Alex
Simon Glass July 28, 2017, 4:19 a.m. UTC | #2 
Hi,

On 18 July 2017 at 12:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Up to now the boot time supported only a single event.
> This patch now allows four events.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         add TPL constants
>         consider multiple events in efi_wait_for_event
>         move notification to new function efi_signal_event
> ---
>  include/efi_api.h             |  13 ++-
>  include/efi_loader.h          |  24 ++++++
>  lib/efi_loader/efi_boottime.c | 195 ++++++++++++++++++++++++++++--------------
>  3 files changed, 168 insertions(+), 64 deletions(-)

Could this use driver model for the events? There is a notify method
which could be a device operation.

Regards,
Simon
Heinrich Schuchardt July 28, 2017, 10:45 a.m. UTC | #3 
On 07/28/2017 06:19 AM, Simon Glass wrote:
> Hi,
> 
> On 18 July 2017 at 12:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> Up to now the boot time supported only a single event.
>> This patch now allows four events.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>         add TPL constants
>>         consider multiple events in efi_wait_for_event
>>         move notification to new function efi_signal_event
>> ---
>>  include/efi_api.h             |  13 ++-
>>  include/efi_loader.h          |  24 ++++++
>>  lib/efi_loader/efi_boottime.c | 195 ++++++++++++++++++++++++++++--------------
>>  3 files changed, 168 insertions(+), 64 deletions(-)
> 
> Could this use driver model for the events? There is a notify method
> which could be a device operation.
> 
> Regards,
> Simon
> 
UEFI events can be signaled between different parts of an UEFI
application. This does not necessarily involve any drivers.

So I think this is not the right place to apply the driver model.

If you would like to move more UEFI coding to the driver model you could
think about the UEFI device drivers (network, graphical output, console,
file system). Having all UEFI device drivers in one uclass might be an
interesting direction. Can you provide a design suggestion or patches?

Best regards

Heinrich
Simon Glass Aug. 6, 2017, 5:16 a.m. UTC | #4 
Hi,

On 28 July 2017 at 04:45, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 07/28/2017 06:19 AM, Simon Glass wrote:
>> Hi,
>>
>> On 18 July 2017 at 12:17, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> Up to now the boot time supported only a single event.
>>> This patch now allows four events.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> v2
>>>         add TPL constants
>>>         consider multiple events in efi_wait_for_event
>>>         move notification to new function efi_signal_event
>>> ---
>>>  include/efi_api.h             |  13 ++-
>>>  include/efi_loader.h          |  24 ++++++
>>>  lib/efi_loader/efi_boottime.c | 195 ++++++++++++++++++++++++++++--------------
>>>  3 files changed, 168 insertions(+), 64 deletions(-)
>>
>> Could this use driver model for the events? There is a notify method
>> which could be a device operation.
>>
>> Regards,
>> Simon
>>
> UEFI events can be signaled between different parts of an UEFI
> application. This does not necessarily involve any drivers.
>
> So I think this is not the right place to apply the driver model.
>
> If you would like to move more UEFI coding to the driver model you could
> think about the UEFI device drivers (network, graphical output, console,
> file system). Having all UEFI device drivers in one uclass might be an
> interesting direction. Can you provide a design suggestion or patches?

Perhaps we could start with just the console since it is simple.

One idea is to have a UEFI device as a child of the serial device, in
a single UCLASS_EFI as you suggest. Then you can iterate through
devices in that uclass to find those which are children of a serial
port device (UCLASS_SERIAL).

I suspect that would allow the ad-hoc UEFI data structures to not be
needed. Instead, information could be returned 'on the fly' by looking
up DM data structures.

I have proposed this a few times. I worry that the direction the code
is taking is leading to an unnecessary fork between UEFI support and
the rest of U-Boot, which is moving to driver model. If this can be
resolved, then it will be easier to adjust things now than later.

Regards,
Simon
diff mbox

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index a1f8221111..a3b8e04576 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -28,8 +28,17 @@  enum efi_event_type {
 	EFI_TIMER_RELATIVE = 2
 };
 
-#define EVT_NOTIFY_WAIT		0x00000100
-#define EVT_NOTIFY_SIGNAL	0x00000200
+#define EVT_TIMER				0x80000000
+#define EVT_RUNTIME				0x40000000
+#define EVT_NOTIFY_WAIT				0x00000100
+#define EVT_NOTIFY_SIGNAL			0x00000200
+#define EVT_SIGNAL_EXIT_BOOT_SERVICES		0x00000201
+#define EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE	0x60000202
+
+#define TPL_APPLICATION		0x04
+#define TPL_CALLBACK		0x08
+#define TPL_NOTIFY		0x10
+#define TPL_HIGH_LEVEL		0x1F
 
 struct efi_event;
 
diff --git a/include/efi_loader.h b/include/efi_loader.h
index d7847d23e5..3d18bfbd2e 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -63,6 +63,30 @@  struct efi_object {
 	void *handle;
 };
 
+/**
+ * struct efi_event
+ *
+ * @type:		Type of event, see efi_create_event
+ * @notify_tpl:		Task priority level of notifications
+ * @trigger_time:	Period of the timer
+ * @trigger_next:	Next time to trigger the timer
+ * @nofify_function:	Function to call when the event is triggered
+ * @notify_context:	Data to be passed to the notify function
+ * @trigger_type:	Type of timer, see efi_set_timer
+ * @signaled:		The notify function was already called
+ */
+struct efi_event {
+	u32 type;
+	unsigned long notify_tpl;
+	void (EFIAPI *notify_function)(struct efi_event *event, void *context);
+	void *notify_context;
+	u64 trigger_next;
+	u64 trigger_time;
+	enum efi_event_type trigger_type;
+	int signaled;
+};
+
+
 /* This list contains all UEFI objects we know of */
 extern struct list_head efi_obj_list;
 
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 0eda465359..a49acc8693 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -82,6 +82,18 @@  efi_status_t efi_exit_func(efi_status_t ret)
 	return ret;
 }
 
+static void efi_signal_event(struct efi_event *event)
+{
+	if (event->signaled)
+		return;
+	event->signaled = 1;
+	if (event->type & EVT_NOTIFY_SIGNAL) {
+		EFI_EXIT(EFI_SUCCESS);
+		event->notify_function(event, event->notify_context);
+		EFI_ENTRY("returning from notification function");
+	}
+}
+
 static efi_status_t efi_unsupported(const char *funcname)
 {
 	debug("EFI: App called into unimplemented function %s\n", funcname);
@@ -163,22 +175,10 @@  static efi_status_t EFIAPI efi_free_pool_ext(void *buffer)
 }
 
 /*
- * Our event capabilities are very limited. Only support a single
- * event to exist, so we don't need to maintain lists.
+ * Our event capabilities are very limited. Only a small limited
+ * number of events is allowed to coexist.
  */
-static struct efi_event {
-	enum efi_event_type type;
-	u32 trigger_type;
-	u32 trigger_time;
-	u64 trigger_next;
-	unsigned long notify_tpl;
-	void (EFIAPI *notify_function) (struct efi_event *event,
-					void *context);
-	void *notify_context;
-} efi_event = {
-	/* Disable timers on bootup */
-	.trigger_next = -1ULL,
-};
+static struct efi_event efi_events[16];
 
 static efi_status_t EFIAPI efi_create_event(
 			enum efi_event_type type, ulong notify_tpl,
@@ -187,13 +187,10 @@  static efi_status_t EFIAPI efi_create_event(
 					void *context),
 			void *notify_context, struct efi_event **event)
 {
+	int i;
+
 	EFI_ENTRY("%d, 0x%lx, %p, %p", type, notify_tpl, notify_function,
 		  notify_context);
-	if (efi_event.notify_function) {
-		/* We only support one event at a time */
-		return EFI_EXIT(EFI_OUT_OF_RESOURCES);
-	}
-
 	if (event == NULL)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
@@ -204,13 +201,20 @@  static efi_status_t EFIAPI efi_create_event(
 	    notify_function == NULL)
 		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-	efi_event.type = type;
-	efi_event.notify_tpl = notify_tpl;
-	efi_event.notify_function = notify_function;
-	efi_event.notify_context = notify_context;
-	*event = &efi_event;
-
-	return EFI_EXIT(EFI_SUCCESS);
+	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].signaled = 0;
+		*event = &efi_events[i];
+		return EFI_EXIT(EFI_SUCCESS);
+	}
+	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
 }
 
 /*
@@ -219,17 +223,22 @@  static efi_status_t EFIAPI efi_create_event(
  */
 void efi_timer_check(void)
 {
+	int i;
 	u64 now = timer_get_us();
 
-	if (now >= efi_event.trigger_next) {
-		/* Triggering! */
-		if (efi_event.trigger_type == EFI_TIMER_PERIODIC)
-			efi_event.trigger_next += efi_event.trigger_time / 10;
-		if (efi_event.type & (EVT_NOTIFY_WAIT | EVT_NOTIFY_SIGNAL))
-			efi_event.notify_function(&efi_event,
-			                          efi_event.notify_context);
+	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
+		if (!efi_events[i].type ||
+		    !(efi_events[i].type & EVT_TIMER) ||
+		    efi_events[i].trigger_type == EFI_TIMER_STOP ||
+		    now < efi_events[i].trigger_next)
+			continue;
+		if (efi_events[i].trigger_type == EFI_TIMER_PERIODIC) {
+			efi_events[i].trigger_next +=
+				efi_events[i].trigger_time / 10;
+			efi_events[i].signaled = 0;
+		}
+		efi_signal_event(&efi_events[i]);
 	}
-
 	WATCHDOG_RESET();
 }
 
@@ -239,6 +248,7 @@  static efi_status_t EFIAPI efi_set_timer(struct efi_event *event, int type,
 	/* We don't have 64bit division available everywhere, so limit timer
 	 * distances to 32bit bits. */
 	u32 trigger32 = trigger_time;
+	int i;
 
 	EFI_ENTRY("%p, %d, %"PRIx64, event, type, trigger_time);
 
@@ -247,60 +257,121 @@  static efi_status_t EFIAPI efi_set_timer(struct efi_event *event, int type,
 		       trigger_time, trigger32);
 	}
 
-	if (event != &efi_event) {
-		/* We only support one event at a time */
-		return EFI_EXIT(EFI_INVALID_PARAMETER);
-	}
+	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
+		if (event != &efi_events[i])
+			continue;
 
-	switch (type) {
-	case EFI_TIMER_STOP:
-		efi_event.trigger_next = -1ULL;
-		break;
-	case EFI_TIMER_PERIODIC:
-	case EFI_TIMER_RELATIVE:
-		efi_event.trigger_next = timer_get_us() + (trigger32 / 10);
-		break;
-	default:
-		return EFI_EXIT(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() + (trigger32 / 10);
+			break;
+		default:
+			return EFI_EXIT(EFI_INVALID_PARAMETER);
+		}
+		event->trigger_type = type;
+		event->trigger_time = trigger_time;
+		return EFI_EXIT(EFI_SUCCESS);
 	}
-	efi_event.trigger_type = type;
-	efi_event.trigger_time = trigger_time;
-
-	return EFI_EXIT(EFI_SUCCESS);
+	return EFI_EXIT(EFI_INVALID_PARAMETER);
 }
 
 static efi_status_t EFIAPI efi_wait_for_event(unsigned long num_events,
 					      struct efi_event **event,
 					      unsigned long *index)
 {
-	u64 now;
+	int i, j;
 
 	EFI_ENTRY("%ld, %p, %p", num_events, event, index);
 
-	now = timer_get_us();
-	while (now < efi_event.trigger_next) { }
-	efi_timer_check();
+	/* Check parameters */
+	if (!num_events || !event)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
+	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 (!event[i]->type || event[i]->type & EVT_NOTIFY_SIGNAL)
+			return EFI_EXIT(EFI_INVALID_PARAMETER);
+	}
+
+	/* Wait for signal */
+	for (;;) {
+		for (i = 0; i < num_events; ++i) {
+			if (event[i]->signaled)
+				goto out;
+		}
+		/* Allow events to occur. */
+		efi_timer_check();
+	}
+
+out:
+	/*
+	 * Reset the signal which is passed to the caller to allow periodic
+	 * events to occur.
+	 */
+	event[i]->signaled = 0;
+	if (index)
+		*index = i;
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
-static efi_status_t EFIAPI efi_signal_event(struct efi_event *event)
+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;
+		efi_signal_event(event);
+		break;
+	}
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
 static efi_status_t EFIAPI efi_close_event(struct efi_event *event)
 {
+	int i;
+
 	EFI_ENTRY("%p", event);
-	efi_event.trigger_next = -1ULL;
-	return EFI_EXIT(EFI_SUCCESS);
+	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
+		if (event == &efi_events[i]) {
+			event->type = 0;
+			event->trigger_next = -1ULL;
+			event->signaled = 0;
+			return EFI_EXIT(EFI_SUCCESS);
+		}
+	}
+	return EFI_EXIT(EFI_INVALID_PARAMETER);
 }
 
 static efi_status_t EFIAPI efi_check_event(struct efi_event *event)
 {
+	int i;
+
 	EFI_ENTRY("%p", event);
-	return EFI_EXIT(EFI_NOT_READY);
+	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->signaled)
+			return EFI_EXIT(EFI_SUCCESS);
+		return EFI_EXIT(EFI_NOT_READY);
+	}
+	return EFI_EXIT(EFI_INVALID_PARAMETER);
 }
 
 static efi_status_t EFIAPI efi_install_protocol_interface(void **handle,
@@ -1032,7 +1103,7 @@  static const struct efi_boot_services efi_boot_services = {
 	.create_event = efi_create_event,
 	.set_timer = efi_set_timer,
 	.wait_for_event = efi_wait_for_event,
-	.signal_event = efi_signal_event,
+	.signal_event = efi_signal_event_ext,
 	.close_event = efi_close_event,
 	.check_event = efi_check_event,
 	.install_protocol_interface = efi_install_protocol_interface_ext,