On 09/25/2017 04:12 AM, Simon Glass wrote:
> On 15 September 2017 at 02:06, Heinrich Schuchardt <xypron.g...@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.g...@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.g...@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
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
