[U-Boot] [PATCH 08/10] efi_selftest: test task priority levels

Heinrich Schuchardt xypron.glpk at gmx.de
Mon Sep 25 06:18:35 UTC 2017


On 09/25/2017 04:12 AM, Simon Glass wrote:
> On 15 September 2017 at 02:06, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>> Run a 10 ms periodic timer and check that it is called 10 times
>> while waiting for 100 ms single shot timer.
>>
>> Raise the TPL level to the level of the 10 ms timer and observe
>> that the notification function is not called again.
>>
>> Lower the TPL level and check that the queued notification
>> function is called.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>> ---
>>  lib/efi_selftest/Makefile           |   5 +-
>>  lib/efi_selftest/efi_selftest_tpl.c | 214 ++++++++++++++++++++++++++++++++++++
>>  2 files changed, 218 insertions(+), 1 deletion(-)
>>  create mode 100644 lib/efi_selftest/efi_selftest_tpl.c
>>
>> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
>> index a71c8bf937..ddf304e1fa 100644
>> --- a/lib/efi_selftest/Makefile
>> +++ b/lib/efi_selftest/Makefile
>> @@ -13,8 +13,11 @@ CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)
>>  CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)
>>  CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
>>  CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
>> +CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI)
>> +CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI)
>>
>>  obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \
>>  efi_selftest.o \
>>  efi_selftest_console.o \
>> -efi_selftest_events.o
>> +efi_selftest_events.o \
>> +efi_selftest_tpl.o
>> diff --git a/lib/efi_selftest/efi_selftest_tpl.c b/lib/efi_selftest/efi_selftest_tpl.c
>> new file mode 100644
>> index 0000000000..90ace0f51e
>> --- /dev/null
>> +++ b/lib/efi_selftest/efi_selftest_tpl.c
>> @@ -0,0 +1,214 @@
>> +/*
>> + * efi_selftest_events
>> + *
>> + * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk at gmx.de>
>> + *
>> + * SPDX-License-Identifier:     GPL-2.0+
>> + *
>> + * This unit test uses timer events to check the handling of
>> + * task priority levels.
>> + */
>> +
>> +#include <efi_selftest.h>
>> +
>> +static struct efi_event *event_notify;
>> +static struct efi_event *event_wait;
>> +static unsigned int counter;
>> +static struct efi_boot_services *boottime;
>> +
>> +/*
>> + * Notification function, increments a counter.
>> + *
>> + * @event      notified event
>> + * @context    pointer to the counter
>> + */
>> +static void EFIAPI notify(struct efi_event *event, void *context)
>> +{
>> +       if (!context)
>> +               return;
> 
> Does this ever happen in practice? If not, why check it?
> 
>> +       ++*(unsigned int *)context;
> 
> Similar comment to previous patch about using a struct here.
> 
>> +}
>> +
>> +/*
>> + * Setup unit test.
>> + *
>> + * Create two timer events.
>> + * One with EVT_NOTIFY_SIGNAL, the other with EVT_NOTIFY_WAIT.
>> + *
>> + * @handle:    handle of the loaded image
>> + * @systable:  system table
> 
> @return ...
> 
>> + */
>> +static int setup(const efi_handle_t handle,
>> +                const struct efi_system_table *systable)
>> +{
>> +       efi_status_t ret;
>> +
>> +       boottime = systable->boottime;
>> +
>> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL,
>> +                                    TPL_CALLBACK, notify, (void *)&counter,
>> +                                    &event_notify);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("could not create event\n");
>> +               return 1;
> 
> Why not return ret?

We return int an not efi_status_t which is UINTN (= size_t).

The code might be more readable by using

#define EFI_ST_SUCCESS 0
#define EFI_ST_FAILURE 1

Regards

Heinrich

> 
>> +       }
>> +       ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT,
>> +                                    TPL_HIGH_LEVEL, notify, NULL, &event_wait);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("could not create event\n");
>> +               return 1;
>> +       }
>> +       return 0;
>> +}
>> +
>> +/*
>> + * Tear down unit test.
>> + *
>> + * Close the events created in setup.
>> + */
>> +static int teardown(void)
>> +{
>> +       efi_status_t ret;
>> +
>> +       if (event_notify) {
>> +               ret = boottime->close_event(event_notify);
>> +               event_notify = NULL;
>> +               if (ret != EFI_SUCCESS) {
>> +                       efi_st_error("could not close event\n");
>> +                       return 1;
>> +               }
>> +       }
>> +       if (event_wait) {
>> +               ret = boottime->close_event(event_wait);
>> +               event_wait = NULL;
>> +               if (ret != EFI_SUCCESS) {
>> +                       efi_st_error("could not close event\n");
>> +                       return 1;
>> +               }
>> +       }
>> +       boottime->restore_tpl(TPL_APPLICATION);
>> +       return 0;
>> +}
>> +
>> +/*
>> + * Execute unit test.
>> + *
>> + * Run a 10 ms periodic timer and check that it is called 10 times
>> + * while waiting for 100 ms single shot timer.
>> + *
>> + * Raise the TPL level to the level of the 10 ms timer and observe
>> + * that the notification function is not called again.
>> + *
>> + * Lower the TPL level and check that the queued notification
>> + * function is called.
>> + */
>> +static int execute(void)
>> +{
>> +       unsigned long index;
>> +       efi_status_t ret;
>> +       UINTN old_tpl;
>> +
>> +       /* Set 10 ms timer */
>> +       counter = 0;
>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not set timer\n");
>> +               return 1;
>> +       }
>> +       /* Set 100 ms timer */
>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not set timer\n");
>> +               return 1;
>> +       }
>> +       index = 5;
>> +       ret = boottime->wait_for_event(1, &event_wait, &index);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not wait for event\n");
>> +               return 1;
>> +       }
>> +       ret = boottime->check_event(event_wait);
>> +       if (ret != EFI_NOT_READY) {
>> +               efi_st_error("Signaled state was not cleared.\n");
>> +               efi_st_printf("ret = %u\n", (unsigned int)ret);
>> +               return 1;
>> +       }
>> +       if (index != 0) {
>> +               efi_st_error("WaitForEvent returned wrong index\n");
>> +               return 1;
>> +       }
>> +       efi_st_printf("Counter with TPL level TPL_APPLICATION: %u\n", counter);
>> +       if (counter < 8 || counter > 12) {
>> +               efi_st_error("Incorrect timing of events\n");
>> +               return 1;
>> +       }
>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0);
>> +       if (index != 0) {
>> +               efi_st_error("Could not cancel timer\n");
>> +               return 1;
>> +       }
>> +       /* Raise TPL level */
>> +       old_tpl = boottime->raise_tpl(TPL_CALLBACK);
>> +       if (old_tpl != TPL_APPLICATION) {
>> +               efi_st_error("Initial TPL level was not TPL_APPLICATION");
>> +               return 1;
>> +       }
>> +       /* Set 10 ms timer */
>> +       counter = 0;
>> +       ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);
>> +       if (index != 0) {
>> +               efi_st_error("Could not set timer\n");
>> +               return 1;
>> +       }
>> +       /* Set 100 ms timer */
>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not set timer\n");
>> +               return 1;
>> +       }
>> +       do {
>> +               ret = boottime->check_event(event_wait);
>> +       } while (ret == EFI_NOT_READY);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not check event\n");
>> +               return 1;
>> +       }
>> +       efi_st_printf("Counter with TPL level TPL_CALLBACK: %u\n", counter);
>> +       if (counter != 0) {
>> +               efi_st_error("Suppressed timer fired\n");
>> +               return 1;
>> +       }
>> +       /* Set 1 ms timer */
>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not set timer\n");
>> +               return 1;
>> +       }
>> +       /* Restore the old TPL level */
>> +       boottime->restore_tpl(TPL_APPLICATION);
>> +       ret = boottime->wait_for_event(1, &event_wait, &index);
>> +       if (ret != EFI_SUCCESS) {
>> +               efi_st_error("Could not wait for event\n");
>> +               return 1;
>> +       }
>> +       efi_st_printf("Counter with TPL level TPL_APPLICATION: %u\n", counter);
>> +       if (counter < 1) {
>> +               efi_st_error("Queued timer event did not fire\n");
>> +               return 1;
>> +       }
>> +       ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0);
>> +       if (index != 0) {
>> +               efi_st_error("Could not cancel timer\n");
>> +               return 1;
>> +       }
>> +
>> +       return 0;
>> +}
>> +
>> +EFI_UNIT_TEST(tpl) = {
>> +       .name = "task priority levels",
>> +       .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
>> +       .setup = setup,
>> +       .execute = execute,
>> +       .teardown = teardown,
> 
> In general I suggest you put the filename as a prefix on these
> function names. It makes __func__ much more useful in the function,
> plus the name is more meaningful.
> 
> E.g. efi_selftest_tpl_execute()
> 
> Regards,
> Simon
> 



More information about the U-Boot mailing list