[U-Boot] [PATCH 2/3] efi_loader: implement WaitForKey

[Alexander Graf]

On 05.07.17 19:47, Heinrich Schuchardt wrote:
> The EFI_SIMPLE_TEXT_INPUT_PROTOCOL requires an event WaitForKey.
> 
> We can easily signal the event in the efi_timer_check function.
> 
> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> ---
>   include/efi_loader.h          |  5 ++++-
>   lib/efi_loader/efi_boottime.c | 17 ++++++++++++++---
>   lib/efi_loader/efi_console.c  |  2 +-
>   3 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index a35b971f7e..d7344d54e4 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -92,7 +92,10 @@ struct efi_event {
>   	enum efi_event_type trigger_type;
>   	int signaled;
>   };
> -
> +extern struct efi_event efi_events[];
> +#define WAIT_FOR_KEY_EVENT (&efi_events[0])
> +/* Events below this number cannot be closed */
> +#define FIRST_EDITABLE_EVENT 1
>   
>   /* 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 ef3e7d9d52..f509d457a7 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -165,7 +165,13 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer)
>    * Our event capabilities are very limited. Only a small limited
>    * number of events is allowed to coexist.
>    */
> -static struct efi_event efi_events[16];
> +struct efi_event efi_events[16] = {
> +	{
> +		/* WaitForKey */
> +		.type = EVT_NOTIFY_WAIT,
> +		.trigger_next = -1ULL,
> +	}
> +};

This moves quite a bit of data from bss to rodata. Can you initialize it 
with a function call instead?

>   
>   static efi_status_t EFIAPI efi_create_event(
>   			enum efi_event_type type, ulong notify_tpl,
> @@ -187,7 +193,8 @@ static efi_status_t EFIAPI efi_create_event(
>   	    notify_function == NULL)
>   		return EFI_EXIT(EFI_INVALID_PARAMETER);
>   
> -	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
> +	/* Use first empty slot */
> +	for (i = FIRST_EDITABLE_EVENT; i < ARRAY_SIZE(efi_events); ++i) {

Do we really need to have FIRST_EDITABLE_EVENT here? We ignore events 
with a type already set anyway. Sure, we waste a few cycles, but the 
code would be more consistent if we always looped through all events.

>   		if (efi_events[i].type)
>   			continue;
>   		efi_events[i].type = type;
> @@ -212,6 +219,10 @@ void efi_timer_check(void)
>   	int i;
>   	u64 now = timer_get_us();
>   
> +	/* Signal keystroke */
> +	if (tstc())
> +		efi_events[0].signaled = 1;

Can we somehow contain all callers of tstc() into efi_console? Maybe 
register the event from efi_console as a periodic event that always 
triggers?


Alex

> +
>   	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
>   		if (!(efi_events[i].type & EVT_TIMER) ||
>   		    efi_events[i].trigger_type == EFI_TIMER_STOP ||
> @@ -315,7 +326,7 @@ static efi_status_t EFIAPI efi_close_event(void *event)
>   	int i;
>   
>   	EFI_ENTRY("%p", event);
> -	for (i = 0; i < ARRAY_SIZE(efi_events); ++i) {
> +	for (i = FIRST_EDITABLE_EVENT; i < ARRAY_SIZE(efi_events); ++i) {
>   		if (event != &efi_events[i])
>   			continue;
>   		efi_events[i].type = 0;
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 8ef7326fef..b2d9ff5497 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -424,5 +424,5 @@ static efi_status_t EFIAPI efi_cin_read_key_stroke(
>   const struct efi_simple_input_interface efi_con_in = {
>   	.reset = efi_cin_reset,
>   	.read_key_stroke = efi_cin_read_key_stroke,
> -	.wait_for_key = NULL,
> +	.wait_for_key = WAIT_FOR_KEY_EVENT,
>   };
>