[{"id":1774341,"web_url":"http://patchwork.ozlabs.org/comment/1774341/","msgid":"","list_archive_url":null,"date":"2017-09-25T02:11:53","subject":"Re: [U-Boot] [PATCH 06/10] efi_selftest: provide unit test for\n\tevent services","submitter":{"id":6170,"url":"http://patchwork.ozlabs.org/api/people/6170/","name":"Simon Glass","email":"sjg@chromium.org"},"content":"On 15 September 2017 at 02:06, Heinrich Schuchardt wrote:\n> This unit test uses timer events to check the implementation\n> of the following boottime services:\n> CreateEvent, CloseEvent, WaitForEvent, CheckEvent, SetTimer\n>\n> Signed-off-by: Heinrich Schuchardt \n> ---\n> lib/efi_selftest/Makefile | 5 +-\n> lib/efi_selftest/efi_selftest_events.c | 195 +++++++++++++++++++++++++++++++++\n> 2 files changed, 199 insertions(+), 1 deletion(-)\n> create mode 100644 lib/efi_selftest/efi_selftest_events.c\n>\n> diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile\n> index 34f5ff1838..a71c8bf937 100644\n> --- a/lib/efi_selftest/Makefile\n> +++ b/lib/efi_selftest/Makefile\n> @@ -11,7 +11,10 @@ CFLAGS_efi_selftest.o := $(CFLAGS_EFI)\n> CFLAGS_REMOVE_efi_selftest.o := $(CFLAGS_NON_EFI)\n> CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)\n> CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)\n> +CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)\n> +CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)\n>\n> obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \\\n> efi_selftest.o \\\n> -efi_selftest_console.o\n> +efi_selftest_console.o \\\n> +efi_selftest_events.o\n> diff --git a/lib/efi_selftest/efi_selftest_events.c b/lib/efi_selftest/efi_selftest_events.c\n> new file mode 100644\n> index 0000000000..c4f66952b9\n> --- /dev/null\n> +++ b/lib/efi_selftest/efi_selftest_events.c\n> @@ -0,0 +1,195 @@\n> +/*\n> + * efi_selftest_events\n> + *\n> + * Copyright (c) 2017 Heinrich Schuchardt \n> + *\n> + * SPDX-License-Identifier: GPL-2.0+\n> + *\n> + * This unit test uses timer events to check the implementation\n> + * of the following boottime services:\n> + * CreateEvent, CloseEvent, WaitForEvent, CheckEvent, SetTimer.\n> + */\n> +\n> +#include \n> +\n> +static struct efi_event *event_notify;\n> +static struct efi_event *event_wait;\n> +static unsigned int counter;\n> +static struct efi_boot_services *boottime;\n> +\n> +/*\n> + * Notification function, increments a counter.\n> + *\n> + * @event notified event\n> + * @context pointer to the counter\n> + */\n> +static void EFIAPI notify(struct efi_event *event, void *context)\n> +{\n> + if (!context)\n> + return;\n> + ++*(unsigned int *)context;\n\nThis is pretty ugly I think. Instead can you create a struct with a\nsingle int member and do something like:\n\nstruct notify_contact *ctx = context;\n\nctx->counter++;\n\n(a better name for counter, too if you can think of one)\n\n> +}\n> +\n> +/*\n> + * Setup unit test.\n> + *\n> + * Create two timer events.\n> + * One with EVT_NOTIFY_SIGNAL, the other with EVT_NOTIFY_WAIT.\n> + *\n> + * @handle: handle of the loaded image\n> + * @systable: system table\n> + */\n> +static int setup(const efi_handle_t handle,\n> + const struct efi_system_table *systable)\n> +{\n> + efi_status_t ret;\n> +\n> + boottime = systable->boottime;\n> +\n> + ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_SIGNAL,\n> + TPL_CALLBACK, notify, (void *)&counter,\n> + &event_notify);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"could not create event\\n\");\n> + return 1;\n> + }\n> + ret = boottime->create_event(EVT_TIMER | EVT_NOTIFY_WAIT,\n> + TPL_CALLBACK, notify, NULL, &event_wait);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"could not create event\\n\");\n> + return 1;\n> + }\n> + return 0;\n> +}\n> +\n> +/*\n> + * Tear down unit test.\n> + *\n> + * Close the events created in setup.\n> + */\n> +static int teardown(void)\n> +{\n> + efi_status_t ret;\n> +\n> + if (event_notify) {\n> + ret = boottime->close_event(event_notify);\n> + event_notify = NULL;\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"could not close event\\n\");\n> + return 1;\n> + }\n> + }\n> + if (event_wait) {\n> + ret = boottime->close_event(event_wait);\n> + event_wait = NULL;\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"could not close event\\n\");\n> + return 1;\n> + }\n> + }\n> + return 0;\n> +}\n> +\n> +/*\n> + * Execute unit test.\n> + *\n> + * Run a 10 ms periodic timer and check that it is called 10 times\n> + * while waiting for 100 ms single shot timer.\n> + *\n> + * Run a 100 ms single shot timer and check that it is called once\n> + * while waiting for 100 ms periodic timer for two periods.\n> + */\n> +static int execute(void)\n> +{\n> + unsigned long index;\n> + efi_status_t ret;\n> +\n> + /* Set 10 ms timer */\n> + counter = 0;\n> + ret = boottime->set_timer(event_notify, EFI_TIMER_PERIODIC, 100000);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"Could not set timer\\n\");\n> + return 1;\n> + }\n> + /* Set 100 ms timer */\n> + ret = boottime->set_timer(event_wait, EFI_TIMER_RELATIVE, 1000000);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"Could not set timer\\n\");\n> + return 1;\n> + }\n> +\n> + index = 5;\n\nIs this an non-zero arbitrary value? Please add a comment.\n\n> + ret = boottime->wait_for_event(1, &event_wait, &index);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"Could not wait for event\\n\");\n> + return 1;\n> + }\n> + ret = boottime->check_event(event_wait);\n> + if (ret != EFI_NOT_READY) {\n> + efi_st_error(\"Signaled state was not cleared.\\n\");\n> + efi_st_printf(\"ret = %u\\n\", (unsigned int)ret);\n> + return 1;\n> + }\n> + if (index != 0) {\n> + efi_st_error(\"WaitForEvent returned wrong index\\n\");\n> + return 1;\n> + }\n> + efi_st_printf(\"Counter periodic: %u\\n\", counter);\n> + if (counter < 8 || counter > 12) {\n> + efi_st_error(\"Incorrect timing of events\\n\");\n> + return 1;\n> + }\n> + ret = boottime->set_timer(event_notify, EFI_TIMER_STOP, 0);\n> + if (index != 0) {\n> + efi_st_error(\"Could not cancel timer\\n\");\n> + return 1;\n> + }\n> + /* Set 10 ms timer */\n> + counter = 0;\n> + ret = boottime->set_timer(event_notify, EFI_TIMER_RELATIVE, 100000);\n> + if (index != 0) {\n> + efi_st_error(\"Could not set timer\\n\");\n> + return 1;\n> + }\n> + /* Set 100 ms timer */\n> + ret = boottime->set_timer(event_wait, EFI_TIMER_PERIODIC, 1000000);\n> + if (index != 0) {\n> + efi_st_error(\"Could not set timer\\n\");\n> + return 1;\n> + }\n> + ret = boottime->wait_for_event(1, &event_wait, &index);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"Could not wait for event\\n\");\n> + return 1;\n> + }\n\nOne problem with this test is that it takes 100ms to run. I don't know\nif the EFI API has any test facilities, but with sandbox we can update\nthe timer to make the test take no time.\n\n(that's just a comment, no change needed here)\n\n> + efi_st_printf(\"Counter single shot: %u\\n\", counter);\n> + if (counter != 1) {\n> + efi_st_error(\"Single shot timer failed\\n\");\n> + return 1;\n> + }\n> + ret = boottime->wait_for_event(1, &event_wait, &index);\n> + if (ret != EFI_SUCCESS) {\n> + efi_st_error(\"Could not wait for event\\n\");\n> + return 1;\n> + }\n> + efi_st_printf(\"Stopped counter: %u\\n\", counter);\n> + if (counter != 1) {\n> + efi_st_error(\"Stopped timer fired\\n\");\n> + return 1;\n> + }\n> + ret = boottime->set_timer(event_wait, EFI_TIMER_STOP, 0);\n> + if (index != 0) {\n> + efi_st_error(\"Could not cancel timer\\n\");\n> + return 1;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +EFI_UNIT_TEST(events) = {\n> + .name = \"event services\",\n> + .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,\n> + .setup = setup,\n> + .execute = execute,\n> + .teardown = teardown,\n> +};\n> --\n> 2.11.0\n>\n\nRegards,\nSimon","headers":{"Return-Path":"","X-Original-To":"incoming@patchwork.ozlabs.org","Delivered-To":"patchwork-incoming@bilbo.ozlabs.org","Authentication-Results":["ozlabs.org;\n\tspf=none (mailfrom) smtp.mailfrom=lists.denx.de\n\t(client-ip=81.169.180.215; helo=lists.denx.de;\n\tenvelope-from=u-boot-bounces@lists.denx.de;\n\treceiver=)","ozlabs.org;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=google.com header.i=@google.com\n\theader.b=\"bJ0g5Rd3\"; \n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"MKWkFCn0\"; dkim-atps=neutral"],"Received":["from lists.denx.de (dione.denx.de [81.169.180.215])\n\tby ozlabs.org (Postfix) with ESMTP id 3y0nnb1bsrz9t3t\n\tfor ;\n\tMon, 25 Sep 2017 12:18:43 +1000 (AEST)","by lists.denx.de (Postfix, from userid 105)\n\tid D35D6C22184; Mon, 25 Sep 2017 02:14:32 +0000 (UTC)","from lists.denx.de (localhost [IPv6:::1])\n\tby lists.denx.de (Postfix) with ESMTP id 834E2C220B8;\n\tMon, 25 Sep 2017 02:14:06 +0000 (UTC)","by lists.denx.de (Postfix, from userid 105)\n\tid 23057C2216B; Mon, 25 Sep 2017 02:12:22 +0000 (UTC)","from mail-qk0-f182.google.com (mail-qk0-f182.google.com\n\t[209.85.220.182])\n\tby lists.denx.de (Postfix) with ESMTPS id 01F0DC21C45\n\tfor ; Mon, 25 Sep 2017 02:12:18 +0000 (UTC)","by mail-qk0-f182.google.com with SMTP id a128so5378058qkc.5\n\tfor ; Sun, 24 Sep 2017 19:12:17 -0700 (PDT)","by 10.200.37.200 with HTTP; Sun, 24 Sep 2017 19:11:53 -0700 (PDT)"],"X-Spam-Checker-Version":"SpamAssassin 3.4.0 (2014-02-07) on lists.denx.de","X-Spam-Level":"","X-Spam-Status":"No, score=-0.0 required=5.0 tests=RCVD_IN_DNSWL_NONE,\n\tRCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,\n\tT_DKIM_INVALID autolearn=unavailable\n\tautolearn_force=no version=3.4.0","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;\n\ts=20161025; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=Y76vnHxuWRKYwPx7oRm9IY6uZnr+tOx52t19XrrIk1g=;\n\tb=bJ0g5Rd3QNHPN/01R6wt8LizWgyr5gXhj3rZ3wnhgyZIUAGGV/qf7NuTulE4iwV3ZP\n\tmNvlDMbuALEhBf7zEK/y/P65lvBI6qs8B8vPkcoolp+7wEr8YlRZMXnXQenW0g10tRkH\n\t+OKdJ8U9PRZyglVKtxa9KKmjlUEUxVQu1k+BWuPw7K0L7Mj1R1jApSPupiuimv3f10qi\n\taJYyUrl2AyI0+tWG0eXwXjntzBbOHm1GhSqJzVeqN1+JzePNgOW9V4S9eBpCAFK4eMV1\n\t6cZYZ4h9Bs8gSOunmHCfoUluIwuwJzppsjq0N8cFRmZXK58nvGy9oISfCvdgFBxHQSdS\n\trSuQ==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=mime-version:sender:in-reply-to:references:from:date:message-id\n\t:subject:to:cc;\n\tbh=Y76vnHxuWRKYwPx7oRm9IY6uZnr+tOx52t19XrrIk1g=;\n\tb=MKWkFCn0Bm7zaAuWO+o1sBRK4Zz3faAnopRemndJVHm9+ahvOOyVUk6Amso6W/Vl5v\n\tQ0WqUPqVQT9pJGkYN+9ScDJYpgghkAHYEnN8GUntEjE3/CErX7xNjJsKYjLPMRNm+SBN\n\tx4H1Ng8VL+hjFLqwSZpXflFKNuI593Q1bGKGE="],"X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:sender:in-reply-to:references:from\n\t:date:message-id:subject:to:cc;\n\tbh=Y76vnHxuWRKYwPx7oRm9IY6uZnr+tOx52t19XrrIk1g=;\n\tb=nMPPD05gju0G96U8A16TDUFb/goxNkfjA1k6o0ryAd6IYNtQ9rjvuawNqCUzOld64c\n\t7R59JMCGZMc8XHLtk1lmSE8VceOzX+lI0uJoKE92XjI7LKbzH/2UMX+c5aGPu34oi1K9\n\tlf7HfJsbz9az+0Qa1pKj9241fuR8rY2VIjgLM4A2l+PNPXBjFfdI5kbM+rlL96AB+WjR\n\t7/PI7JmuQ559oc/BlMe+2Lg423jy37tiQtxebvVw9y2FXsbvm3tTUdDsLH/UeSLI2CF6\n\tDEoP+2wNTKyekt8K20tXoypiFVM/mxTphatwAeA7sbtWo3R+2hKi2VynKG2tDwh3PmoK\n\tt1dQ==","X-Gm-Message-State":"AHPjjUgyuT+4kGOtGx2MVo+OZE5kjvmjlZO2NPIBvUmVrDnFVRrjGHkV\n\t/thNmFdsXU0qStqQHDlf+v6sUNs2bl7VRBIQoIoq9Q==","X-Google-Smtp-Source":"AOwi7QALRddj0V3jIV+xoLmKGCEkNAWjplGzPSs+eSNpagaGaKAY0wxr/3vpy1aQAN+NgTRVJpjhzRBsIDT9zl0wlmU=","X-Received":"by 10.55.214.80 with SMTP id t77mr8479003qki.188.1506305536477; \n\tSun, 24 Sep 2017 19:12:16 -0700 (PDT)","MIME-Version":"1.0","In-Reply-To":"<20170915080619.25250-7-xypron.glpk@gmx.de>","References":"<20170915080619.25250-1-xypron.glpk@gmx.de>\n\t<20170915080619.25250-7-xypron.glpk@gmx.de>","From":"Simon Glass ","Date":"Sun, 24 Sep 2017 22:11:53 -0400","X-Google-Sender-Auth":"G_ZqHQI1jNZfqMN35ziLLN1aZX0","Message-ID":"","To":"Heinrich Schuchardt ","Cc":"=?utf-8?q?=C5=81ukasz_Majewski?= ,\n\tAndy Shevchenko , U-Boot Mailing List\n\t, Fabio Estevam ,\n\tMaxime Ripard ","Subject":"Re: [U-Boot] [PATCH 06/10] efi_selftest: provide unit test for\n\tevent services","X-BeenThere":"u-boot@lists.denx.de","X-Mailman-Version":"2.1.18","Precedence":"list","List-Id":"U-Boot discussion ","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"u-boot-bounces@lists.denx.de","Sender":"\"U-Boot\" "}}]