| Message ID | 20171013173314.22304-7-xypron.glpk@gmx.de |
|---|---|
| State | Superseded, archived |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | efi_loader: implement SetWatchdogTimer | expand |
On 13.10.17 19:33, Heinrich Schuchardt wrote: > Environment variable efi_selftest is passed as load options > to the selftest application. It is used to select a single > test to be executed. > > Special value 'list' displays a list of all available tests. > > Tests get an on_request property. If this property is set > the tests are only executed if explicitly requested. > > The invocation of efi_selftest is changed to reflect that > bootefi selftest with efi_selftest = 'list' will call the > Exit bootservice. > > Environment variable bootargs is used as load options > for all other bootefi payloads. > > Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> > --- > v2 > use an environment variable to choose a test > --- > cmd/bootefi.c | 46 ++++++++++++++++- > include/efi_selftest.h | 18 +++++++ > lib/efi_selftest/efi_selftest.c | 90 +++++++++++++++++++++++++++++++-- > lib/efi_selftest/efi_selftest_console.c | 10 ++++ > lib/efi_selftest/efi_selftest_util.c | 11 +++- > 5 files changed, 168 insertions(+), 7 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 18176a1266..2d70137482 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -6,10 +6,12 @@ > * SPDX-License-Identifier: GPL-2.0+ > */ > > +#include <charset.h> > #include <common.h> > #include <command.h> > #include <dm.h> > #include <efi_loader.h> > +#include <efi_selftest.h> > #include <errno.h> > #include <libfdt.h> > #include <libfdt_env.h> > @@ -50,6 +52,32 @@ static void efi_init_obj_list(void) > efi_get_time_init(); > } > > +/* > + * Set the load options of an image from an environment variable. > + * > + * @loaded_image_info: the image > + * @env_var: name of the environment variable > + */ > +static void set_load_options(struct efi_loaded_image *loaded_image_info, > + const char *env_var) > +{ > + size_t size; > + const char *env = env_get(env_var); > + > + loaded_image_info->load_options = NULL; > + loaded_image_info->load_options_size = 0; > + if (!env) > + return; > + size = strlen(env) + 1; > + loaded_image_info->load_options = calloc(size, sizeof(u16)); > + if (!loaded_image_info->load_options) { > + printf("ERROR: Out of memory\n"); > + return; > + } > + utf8_to_utf16(loaded_image_info->load_options, (u8 *)env, size); > + loaded_image_info->load_options_size = size * 2; > +} > + > static void *copy_fdt(void *fdt) > { > u64 fdt_size = fdt_totalsize(fdt); > @@ -190,6 +218,8 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > efi_install_configuration_table(&fdt_guid, NULL); > } > > + /* Transfer environment variable bootargs as load options */ > + set_load_options(&loaded_image_info, "bootargs"); While I really want to see that change, please try not to sneak it in with the selftest one :). Just split that hunk out to a following patch and give it its own patch description. In case something goes wrong, we'd only need to revert a small patch then. > /* Load the EFI payload */ > entry = efi_load_pe(efi, &loaded_image_info); > if (!entry) { > @@ -237,6 +267,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, > > exit: > /* image has returned, loaded-image obj goes *poof*: */ > + free(loaded_image_info.load_options); This too is a change that doesn't fit the patch description? > list_del(&loaded_image_info_obj.link); > > return ret; > @@ -301,17 +332,26 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > > efi_setup_loaded_image(&loaded_image_info, > &loaded_image_info_obj, > - bootefi_device_path, bootefi_image_path); > + NULL, NULL); Why? > /* > * gd lives in a fixed register which may get clobbered while we > * execute the payload. So save it here and restore it on every > * callback entry > */ > efi_save_gd(); > + loaded_image_info.image_code_type = EFI_LOADER_CODE; > + loaded_image_info.image_data_type = EFI_LOADER_DATA; Also unrelated? Please split it out. > /* Initialize and populate EFI object list */ > if (!efi_obj_list_initalized) > efi_init_obj_list(); > - return efi_selftest(&loaded_image_info, &systab); > + /* Transfer environment variable efi_selftest as load options */ > + set_load_options(&loaded_image_info, "efi_selftest"); > + /* Execute the test */ > + r = efi_selftest(&loaded_image_info, &systab); > + efi_restore_gd(); > + free(loaded_image_info.load_options); > + list_del(&loaded_image_info_obj.link); That change too is unrelated to the patch description. Please split out. > + return r; > } else > #endif > if (!strcmp(argv[1], "bootmgr")) { > @@ -357,6 +397,8 @@ static char bootefi_help_text[] = > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > "bootefi selftest\n" > " - boot an EFI selftest application stored within U-Boot\n" > + " Use environment variable efi_selftest to select a single test.\n" > + " Use 'setenv efi_selftest list' to enumerate all tests.\n" > #endif > "bootmgr [fdt addr]\n" > " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" > diff --git a/include/efi_selftest.h b/include/efi_selftest.h > index 7ec42a0406..978ca2a7ea 100644 > --- a/include/efi_selftest.h > +++ b/include/efi_selftest.h > @@ -12,6 +12,7 @@ > #include <common.h> > #include <efi.h> > #include <efi_api.h> > +#include <efi_loader.h> > #include <linker_lists.h> > > #define EFI_ST_SUCCESS 0 > @@ -71,6 +72,15 @@ void efi_st_printf(const char *fmt, ...) > */ > int efi_st_memcmp(const void *buf1, const void *buf2, size_t length); > > +/* > + * Compare an u16 string to a char string. > + * > + * @buf1: u16 string > + * @buf2: char string > + * @return: 0 if both buffers contain the same bytes > + */ > +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2); > + > /* > * Reads an Unicode character from the input device. > * > @@ -88,6 +98,7 @@ u16 efi_st_get_key(void); > * @setup: set up the unit test > * @teardown: tear down the unit test > * @execute: execute the unit test > + * @on_request: test is only executed on request > */ > struct efi_unit_test { > const char *name; > @@ -96,10 +107,17 @@ struct efi_unit_test { > const struct efi_system_table *systable); > int (*execute)(void); > int (*teardown)(void); > + bool on_request; > }; > > /* Declare a new EFI unit test */ > #define EFI_UNIT_TEST(__name) \ > ll_entry_declare(struct efi_unit_test, __name, efi_unit_test) > > +#define EFI_SELFTEST_TABLE_GUID \ > + EFI_GUID(0xbc3ebe57, 0x09e5, 0xa59d, 0xdb, 0x87, \ > + 0xf5, 0x79, 0x61, 0x62, 0x06, 0xde) > + > +extern const efi_guid_t efi_selftest_table_guid; > + > #endif /* _EFI_SELFTEST_H */ > diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c > index 45d8d3d384..110284f9c7 100644 > --- a/lib/efi_selftest/efi_selftest.c > +++ b/lib/efi_selftest/efi_selftest.c > @@ -15,6 +15,8 @@ static const struct efi_runtime_services *runtime; > static efi_handle_t handle; > static u16 reset_message[] = L"Selftest completed"; > > +const efi_guid_t efi_selftest_table_guid = EFI_SELFTEST_TABLE_GUID; > + > /* > * Exit the boot services. > * > @@ -25,8 +27,8 @@ static u16 reset_message[] = L"Selftest completed"; > */ > void efi_st_exit_boot_services(void) > { > - unsigned long map_size = 0; > - unsigned long map_key; > + unsigned long map_size = 0; > + unsigned long map_key; Unrelated? > unsigned long desc_size; > u32 desc_version; > efi_status_t ret; > @@ -133,6 +135,41 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures) > return ret; > } > > +/* > + * Check that a test exists. > + * > + * @testname: name of the test > + * @return: test > + */ > +static struct efi_unit_test *find_test(const u16 *testname) > +{ > + struct efi_unit_test *test; > + > + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + if (!efi_st_strcmp_16_8(testname, test->name)) Why not just use UCS2 strings here and compare 16 to 16? Maybe you're using the name more often in normal situations? Either way, not a biggie :) > + return test; > + } > + efi_st_printf("\nTest '%ps' not found\n", testname); > + return NULL; > +} > + > +/* > + * List all available tests. > + */ > +static void list_all_tests(void) > +{ > + struct efi_unit_test *test; > + > + /* List all tests */ > + efi_st_printf("\nAvailable tests:\n"); > + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + efi_st_printf("'%s'%s\n", test->name, > + test->on_request ? " - on request" : ""); > + } > +} > + > /* > * Execute selftest of the EFI API > * > @@ -155,6 +192,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, > { > struct efi_unit_test *test; > unsigned int failures = 0; > + const u16 *testname = NULL; > + struct efi_loaded_image *loaded_image; > + efi_status_t ret; > > systable = systab; > boottime = systable->boottime; > @@ -163,14 +203,47 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, > con_out = systable->con_out; > con_in = systable->con_in; > > + ret = boottime->handle_protocol(image_handle, &efi_guid_loaded_image, > + (void **)&loaded_image); > + if (ret != EFI_SUCCESS) { > + efi_st_error("Cannot open loaded image protocol"); > + return ret; > + } > + > + if (loaded_image->load_options) > + testname = (u16 *)loaded_image->load_options; > + > + if (testname) { > + if (!efi_st_strcmp_16_8(testname, "list") || > + !find_test(testname)) { > + list_all_tests(); > + /* > + * TODO: > + * Once the Exit boottime service is correctly > + * implemented we should call > + * boottime->exit(image_handle, EFI_SUCCESS, 0, NULL); > + * here, cf. > + * https://lists.denx.de/pipermail/u-boot/2017-October/308720.html > + */ > + return EFI_SUCCESS; > + } > + } > + > efi_st_printf("\nTesting EFI API implementation\n"); > > - efi_st_printf("\nNumber of tests to execute: %u\n", > - ll_entry_count(struct efi_unit_test, efi_unit_test)); > + if (testname) > + efi_st_printf("\nSelected test: '%ps'\n", testname); > + else > + efi_st_printf("\nNumber of tests to execute: %u\n", > + ll_entry_count(struct efi_unit_test, > + efi_unit_test)); > > /* Execute boottime tests */ > for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + if (testname ? > + efi_st_strcmp_16_8(testname, test->name) : test->on_request) > + continue; > if (test->phase == EFI_EXECUTE_BEFORE_BOOTTIME_EXIT) { > setup(test, &failures); > execute(test, &failures); > @@ -181,6 +254,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, > /* Execute mixed tests */ > for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + if (testname ? > + efi_st_strcmp_16_8(testname, test->name) : test->on_request) > + continue; > if (test->phase == EFI_SETUP_BEFORE_BOOTTIME_EXIT) > setup(test, &failures); > } > @@ -189,6 +265,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, > > for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + if (testname ? > + efi_st_strcmp_16_8(testname, test->name) : test->on_request) > + continue; > if (test->phase == EFI_SETUP_BEFORE_BOOTTIME_EXIT) { > execute(test, &failures); > teardown(test, &failures); > @@ -198,6 +277,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, > /* Execute runtime tests */ > for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); > test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { > + if (testname ? > + efi_st_strcmp_16_8(testname, test->name) : test->on_request) This pattern occurs a few time - maybe make a helper function out of it? static bool test_is_active(const u16 *testname, struct efi_unit_test *test) { if (testname) return !efi_st_strcmp_16_8(testname, test->name); else return !test->on_request; } > + continue; > if (test->phase == EFI_SETUP_AFTER_BOOTTIME_EXIT) { > setup(test, &failures); > execute(test, &failures); > diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c > index 840e2290c6..6a7fd20da5 100644 > --- a/lib/efi_selftest/efi_selftest_console.c > +++ b/lib/efi_selftest/efi_selftest_console.c > @@ -142,6 +142,7 @@ void efi_st_printf(const char *fmt, ...) > const char *c; > u16 *pos = buf; > const char *s; > + const u16 *u; > > va_start(args, fmt); > > @@ -179,9 +180,18 @@ void efi_st_printf(const char *fmt, ...) > case 'p': > ++c; > switch (*c) { > + /* MAC address */ > case 'm': > mac(va_arg(args, void*), &pos); > break; > + > + /* u16 string */ > + case 's': > + u = va_arg(args, u16*); > + /* Ensure string fits into buffer */ > + for (; *u && pos < buf + 120; ++u) > + *pos++ = *u; > + break; > default: > --c; > pointer(va_arg(args, void*), &pos); > diff --git a/lib/efi_selftest/efi_selftest_util.c b/lib/efi_selftest/efi_selftest_util.c > index 5cffe383d8..1b17bf4d4b 100644 > --- a/lib/efi_selftest/efi_selftest_util.c > +++ b/lib/efi_selftest/efi_selftest_util.c > @@ -21,5 +21,14 @@ int efi_st_memcmp(const void *buf1, const void *buf2, size_t length) > ++pos1; > ++pos2; > } > - return EFI_ST_SUCCESS; > + return 0; Also unrelated ;) Alex > +} > + > +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2) > +{ > + for (; *buf1 || *buf2; ++buf1, ++buf2) { > + if (*buf1 != *buf2) > + return *buf1 - *buf2; > + } > + return 0; > } >
On 10/17/2017 09:48 AM, Alexander Graf wrote: > > > On 13.10.17 19:33, Heinrich Schuchardt wrote: >> Environment variable efi_selftest is passed as load options >> to the selftest application. It is used to select a single >> test to be executed. >> >> Special value 'list' displays a list of all available tests. >> >> Tests get an on_request property. If this property is set >> the tests are only executed if explicitly requested. >> >> The invocation of efi_selftest is changed to reflect that >> bootefi selftest with efi_selftest = 'list' will call the >> Exit bootservice. >> >> Environment variable bootargs is used as load options >> for all other bootefi payloads. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> v2 >> use an environment variable to choose a test >> --- >> cmd/bootefi.c | 46 ++++++++++++++++- >> include/efi_selftest.h | 18 +++++++ >> lib/efi_selftest/efi_selftest.c | 90 +++++++++++++++++++++++++++++++-- >> lib/efi_selftest/efi_selftest_console.c | 10 ++++ >> lib/efi_selftest/efi_selftest_util.c | 11 +++- >> 5 files changed, 168 insertions(+), 7 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index 18176a1266..2d70137482 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -6,10 +6,12 @@ >> * SPDX-License-Identifier: GPL-2.0+ >> */ >> >> +#include <charset.h> >> #include <common.h> >> #include <command.h> >> #include <dm.h> >> #include <efi_loader.h> >> +#include <efi_selftest.h> >> #include <errno.h> >> #include <libfdt.h> >> #include <libfdt_env.h> >> @@ -50,6 +52,32 @@ static void efi_init_obj_list(void) >> efi_get_time_init(); >> } >> >> +/* >> + * Set the load options of an image from an environment variable. >> + * >> + * @loaded_image_info: the image >> + * @env_var: name of the environment variable >> + */ >> +static void set_load_options(struct efi_loaded_image *loaded_image_info, >> + const char *env_var) >> +{ >> + size_t size; >> + const char *env = env_get(env_var); >> + >> + loaded_image_info->load_options = NULL; >> + loaded_image_info->load_options_size = 0; >> + if (!env) >> + return; >> + size = strlen(env) + 1; >> + loaded_image_info->load_options = calloc(size, sizeof(u16)); >> + if (!loaded_image_info->load_options) { >> + printf("ERROR: Out of memory\n"); >> + return; >> + } >> + utf8_to_utf16(loaded_image_info->load_options, (u8 *)env, size); >> + loaded_image_info->load_options_size = size * 2; >> +} >> + >> static void *copy_fdt(void *fdt) >> { >> u64 fdt_size = fdt_totalsize(fdt); >> @@ -190,6 +218,8 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> efi_install_configuration_table(&fdt_guid, NULL); >> } >> >> + /* Transfer environment variable bootargs as load options */ >> + set_load_options(&loaded_image_info, "bootargs"); > > While I really want to see that change, please try not to sneak it in > with the selftest one :). > > Just split that hunk out to a following patch and give it its own patch > description. In case something goes wrong, we'd only need to revert a > small patch then. > >> /* Load the EFI payload */ >> entry = efi_load_pe(efi, &loaded_image_info); >> if (!entry) { >> @@ -237,6 +267,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> >> exit: >> /* image has returned, loaded-image obj goes *poof*: */ >> + free(loaded_image_info.load_options); > > This too is a change that doesn't fit the patch description? > >> list_del(&loaded_image_info_obj.link); >> >> return ret; >> @@ -301,17 +332,26 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> >> efi_setup_loaded_image(&loaded_image_info, >> &loaded_image_info_obj, >> - bootefi_device_path, bootefi_image_path); >> + NULL, NULL); > > Why? We have neither a device nor an image path here. We do not want to use the values that where set by a prior call. bf19273e81eb efi_loader: Add mem-mapped for fallback provided a logic for helloworld. I will add a preceding patch to move that to efi_setup_loaded_image. Regards Heinrich
On 10/17/2017 09:48 AM, Alexander Graf wrote: > > > On 13.10.17 19:33, Heinrich Schuchardt wrote: >> Environment variable efi_selftest is passed as load options >> to the selftest application. It is used to select a single >> test to be executed. >> >> Special value 'list' displays a list of all available tests. >> >> Tests get an on_request property. If this property is set >> the tests are only executed if explicitly requested. >> >> The invocation of efi_selftest is changed to reflect that >> bootefi selftest with efi_selftest = 'list' will call the >> Exit bootservice. >> >> Environment variable bootargs is used as load options >> for all other bootefi payloads. >> >> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >> --- >> v2 >> use an environment variable to choose a test >> --- >> cmd/bootefi.c | 46 ++++++++++++++++- >> include/efi_selftest.h | 18 +++++++ >> lib/efi_selftest/efi_selftest.c | 90 +++++++++++++++++++++++++++++++-- >> lib/efi_selftest/efi_selftest_console.c | 10 ++++ >> lib/efi_selftest/efi_selftest_util.c | 11 +++- >> 5 files changed, 168 insertions(+), 7 deletions(-) >> >> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >> index 18176a1266..2d70137482 100644 >> --- a/cmd/bootefi.c >> +++ b/cmd/bootefi.c >> @@ -6,10 +6,12 @@ >> * SPDX-License-Identifier: GPL-2.0+ >> */ >> >> +#include <charset.h> >> #include <common.h> >> #include <command.h> >> #include <dm.h> >> #include <efi_loader.h> >> +#include <efi_selftest.h> >> #include <errno.h> >> #include <libfdt.h> >> #include <libfdt_env.h> >> @@ -50,6 +52,32 @@ static void efi_init_obj_list(void) >> efi_get_time_init(); >> } >> >> +/* >> + * Set the load options of an image from an environment variable. >> + * >> + * @loaded_image_info: the image >> + * @env_var: name of the environment variable >> + */ >> +static void set_load_options(struct efi_loaded_image *loaded_image_info, >> + const char *env_var) >> +{ >> + size_t size; >> + const char *env = env_get(env_var); >> + >> + loaded_image_info->load_options = NULL; >> + loaded_image_info->load_options_size = 0; >> + if (!env) >> + return; >> + size = strlen(env) + 1; >> + loaded_image_info->load_options = calloc(size, sizeof(u16)); >> + if (!loaded_image_info->load_options) { >> + printf("ERROR: Out of memory\n"); >> + return; >> + } >> + utf8_to_utf16(loaded_image_info->load_options, (u8 *)env, size); >> + loaded_image_info->load_options_size = size * 2; >> +} >> + >> static void *copy_fdt(void *fdt) >> { >> u64 fdt_size = fdt_totalsize(fdt); >> @@ -190,6 +218,8 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> efi_install_configuration_table(&fdt_guid, NULL); >> } >> >> + /* Transfer environment variable bootargs as load options */ >> + set_load_options(&loaded_image_info, "bootargs"); > > While I really want to see that change, please try not to sneak it in > with the selftest one :). > > Just split that hunk out to a following patch and give it its own patch > description. In case something goes wrong, we'd only need to revert a > small patch then. > >> /* Load the EFI payload */ >> entry = efi_load_pe(efi, &loaded_image_info); >> if (!entry) { >> @@ -237,6 +267,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >> >> exit: >> /* image has returned, loaded-image obj goes *poof*: */ >> + free(loaded_image_info.load_options); > > This too is a change that doesn't fit the patch description? > >> list_del(&loaded_image_info_obj.link); >> >> return ret; >> @@ -301,17 +332,26 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >> >> efi_setup_loaded_image(&loaded_image_info, >> &loaded_image_info_obj, >> - bootefi_device_path, bootefi_image_path); >> + NULL, NULL); > > Why? > >> /* >> * gd lives in a fixed register which may get clobbered while we >> * execute the payload. So save it here and restore it on every >> * callback entry >> */ >> efi_save_gd(); >> + loaded_image_info.image_code_type = EFI_LOADER_CODE; >> + loaded_image_info.image_data_type = EFI_LOADER_DATA; > > Also unrelated? Please split it out. > >> /* Initialize and populate EFI object list */ >> if (!efi_obj_list_initalized) >> efi_init_obj_list(); >> - return efi_selftest(&loaded_image_info, &systab); >> + /* Transfer environment variable efi_selftest as load options */ >> + set_load_options(&loaded_image_info, "efi_selftest"); >> + /* Execute the test */ >> + r = efi_selftest(&loaded_image_info, &systab); >> + efi_restore_gd(); >> + free(loaded_image_info.load_options); >> + list_del(&loaded_image_info_obj.link); > > That change too is unrelated to the patch description. Please split out. > >> + return r; >> } else >> #endif >> if (!strcmp(argv[1], "bootmgr")) { >> @@ -357,6 +397,8 @@ static char bootefi_help_text[] = >> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST >> "bootefi selftest\n" >> " - boot an EFI selftest application stored within U-Boot\n" >> + " Use environment variable efi_selftest to select a single test.\n" >> + " Use 'setenv efi_selftest list' to enumerate all tests.\n" >> #endif >> "bootmgr [fdt addr]\n" >> " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" >> diff --git a/include/efi_selftest.h b/include/efi_selftest.h >> index 7ec42a0406..978ca2a7ea 100644 >> --- a/include/efi_selftest.h >> +++ b/include/efi_selftest.h >> @@ -12,6 +12,7 @@ >> #include <common.h> >> #include <efi.h> >> #include <efi_api.h> >> +#include <efi_loader.h> >> #include <linker_lists.h> >> >> #define EFI_ST_SUCCESS 0 >> @@ -71,6 +72,15 @@ void efi_st_printf(const char *fmt, ...) >> */ >> int efi_st_memcmp(const void *buf1, const void *buf2, size_t length); >> >> +/* >> + * Compare an u16 string to a char string. >> + * >> + * @buf1: u16 string >> + * @buf2: char string >> + * @return: 0 if both buffers contain the same bytes >> + */ >> +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2); >> + >> /* >> * Reads an Unicode character from the input device. >> * >> @@ -88,6 +98,7 @@ u16 efi_st_get_key(void); >> * @setup: set up the unit test >> * @teardown: tear down the unit test >> * @execute: execute the unit test >> + * @on_request: test is only executed on request >> */ >> struct efi_unit_test { >> const char *name; >> @@ -96,10 +107,17 @@ struct efi_unit_test { >> const struct efi_system_table *systable); >> int (*execute)(void); >> int (*teardown)(void); >> + bool on_request; >> }; >> >> /* Declare a new EFI unit test */ >> #define EFI_UNIT_TEST(__name) \ >> ll_entry_declare(struct efi_unit_test, __name, efi_unit_test) >> >> +#define EFI_SELFTEST_TABLE_GUID \ >> + EFI_GUID(0xbc3ebe57, 0x09e5, 0xa59d, 0xdb, 0x87, \ >> + 0xf5, 0x79, 0x61, 0x62, 0x06, 0xde) >> + >> +extern const efi_guid_t efi_selftest_table_guid; >> + >> #endif /* _EFI_SELFTEST_H */ >> diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c >> index 45d8d3d384..110284f9c7 100644 >> --- a/lib/efi_selftest/efi_selftest.c >> +++ b/lib/efi_selftest/efi_selftest.c >> @@ -15,6 +15,8 @@ static const struct efi_runtime_services *runtime; >> static efi_handle_t handle; >> static u16 reset_message[] = L"Selftest completed"; >> >> +const efi_guid_t efi_selftest_table_guid = EFI_SELFTEST_TABLE_GUID; >> + >> /* >> * Exit the boot services. >> * >> @@ -25,8 +27,8 @@ static u16 reset_message[] = L"Selftest completed"; >> */ >> void efi_st_exit_boot_services(void) >> { >> - unsigned long map_size = 0; >> - unsigned long map_key; >> + unsigned long map_size = 0; >> + unsigned long map_key; > > Unrelated? > >> unsigned long desc_size; >> u32 desc_version; >> efi_status_t ret; >> @@ -133,6 +135,41 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures) >> return ret; >> } >> >> +/* >> + * Check that a test exists. >> + * >> + * @testname: name of the test >> + * @return: test >> + */ >> +static struct efi_unit_test *find_test(const u16 *testname) >> +{ >> + struct efi_unit_test *test; >> + >> + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); >> + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { >> + if (!efi_st_strcmp_16_8(testname, test->name)) > > Why not just use UCS2 strings here and compare 16 to 16? Maybe you're > using the name more often in normal situations? > > Either way, not a biggie :) I thought about defining the test names as UCS2. But wide strings need double the space. So I decided to stay with char * as far as possible to reduce code size. I remember that reviewing one patch you asked me to rearrange function arguments to save 16 bytes of compiled code size for a specific architecture. Best regards Heinrich
On 17.10.17 22:11, Heinrich Schuchardt wrote: > On 10/17/2017 09:48 AM, Alexander Graf wrote: >> >> >> On 13.10.17 19:33, Heinrich Schuchardt wrote: >>> Environment variable efi_selftest is passed as load options >>> to the selftest application. It is used to select a single >>> test to be executed. >>> >>> Special value 'list' displays a list of all available tests. >>> >>> Tests get an on_request property. If this property is set >>> the tests are only executed if explicitly requested. >>> >>> The invocation of efi_selftest is changed to reflect that >>> bootefi selftest with efi_selftest = 'list' will call the >>> Exit bootservice. >>> >>> Environment variable bootargs is used as load options >>> for all other bootefi payloads. >>> >>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> >>> --- >>> v2 >>> use an environment variable to choose a test >>> --- >>> cmd/bootefi.c | 46 ++++++++++++++++- >>> include/efi_selftest.h | 18 +++++++ >>> lib/efi_selftest/efi_selftest.c | 90 +++++++++++++++++++++++++++++++-- >>> lib/efi_selftest/efi_selftest_console.c | 10 ++++ >>> lib/efi_selftest/efi_selftest_util.c | 11 +++- >>> 5 files changed, 168 insertions(+), 7 deletions(-) >>> >>> diff --git a/cmd/bootefi.c b/cmd/bootefi.c >>> index 18176a1266..2d70137482 100644 >>> --- a/cmd/bootefi.c >>> +++ b/cmd/bootefi.c >>> @@ -6,10 +6,12 @@ >>> * SPDX-License-Identifier: GPL-2.0+ >>> */ >>> >>> +#include <charset.h> >>> #include <common.h> >>> #include <command.h> >>> #include <dm.h> >>> #include <efi_loader.h> >>> +#include <efi_selftest.h> >>> #include <errno.h> >>> #include <libfdt.h> >>> #include <libfdt_env.h> >>> @@ -50,6 +52,32 @@ static void efi_init_obj_list(void) >>> efi_get_time_init(); >>> } >>> >>> +/* >>> + * Set the load options of an image from an environment variable. >>> + * >>> + * @loaded_image_info: the image >>> + * @env_var: name of the environment variable >>> + */ >>> +static void set_load_options(struct efi_loaded_image *loaded_image_info, >>> + const char *env_var) >>> +{ >>> + size_t size; >>> + const char *env = env_get(env_var); >>> + >>> + loaded_image_info->load_options = NULL; >>> + loaded_image_info->load_options_size = 0; >>> + if (!env) >>> + return; >>> + size = strlen(env) + 1; >>> + loaded_image_info->load_options = calloc(size, sizeof(u16)); >>> + if (!loaded_image_info->load_options) { >>> + printf("ERROR: Out of memory\n"); >>> + return; >>> + } >>> + utf8_to_utf16(loaded_image_info->load_options, (u8 *)env, size); >>> + loaded_image_info->load_options_size = size * 2; >>> +} >>> + >>> static void *copy_fdt(void *fdt) >>> { >>> u64 fdt_size = fdt_totalsize(fdt); >>> @@ -190,6 +218,8 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >>> efi_install_configuration_table(&fdt_guid, NULL); >>> } >>> >>> + /* Transfer environment variable bootargs as load options */ >>> + set_load_options(&loaded_image_info, "bootargs"); >> >> While I really want to see that change, please try not to sneak it in >> with the selftest one :). >> >> Just split that hunk out to a following patch and give it its own patch >> description. In case something goes wrong, we'd only need to revert a >> small patch then. >> >>> /* Load the EFI payload */ >>> entry = efi_load_pe(efi, &loaded_image_info); >>> if (!entry) { >>> @@ -237,6 +267,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, >>> >>> exit: >>> /* image has returned, loaded-image obj goes *poof*: */ >>> + free(loaded_image_info.load_options); >> >> This too is a change that doesn't fit the patch description? >> >>> list_del(&loaded_image_info_obj.link); >>> >>> return ret; >>> @@ -301,17 +332,26 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) >>> >>> efi_setup_loaded_image(&loaded_image_info, >>> &loaded_image_info_obj, >>> - bootefi_device_path, bootefi_image_path); >>> + NULL, NULL); >> >> Why? >> >>> /* >>> * gd lives in a fixed register which may get clobbered while we >>> * execute the payload. So save it here and restore it on every >>> * callback entry >>> */ >>> efi_save_gd(); >>> + loaded_image_info.image_code_type = EFI_LOADER_CODE; >>> + loaded_image_info.image_data_type = EFI_LOADER_DATA; >> >> Also unrelated? Please split it out. >> >>> /* Initialize and populate EFI object list */ >>> if (!efi_obj_list_initalized) >>> efi_init_obj_list(); >>> - return efi_selftest(&loaded_image_info, &systab); >>> + /* Transfer environment variable efi_selftest as load options */ >>> + set_load_options(&loaded_image_info, "efi_selftest"); >>> + /* Execute the test */ >>> + r = efi_selftest(&loaded_image_info, &systab); >>> + efi_restore_gd(); >>> + free(loaded_image_info.load_options); >>> + list_del(&loaded_image_info_obj.link); >> >> That change too is unrelated to the patch description. Please split out. >> >>> + return r; >>> } else >>> #endif >>> if (!strcmp(argv[1], "bootmgr")) { >>> @@ -357,6 +397,8 @@ static char bootefi_help_text[] = >>> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST >>> "bootefi selftest\n" >>> " - boot an EFI selftest application stored within U-Boot\n" >>> + " Use environment variable efi_selftest to select a single test.\n" >>> + " Use 'setenv efi_selftest list' to enumerate all tests.\n" >>> #endif >>> "bootmgr [fdt addr]\n" >>> " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" >>> diff --git a/include/efi_selftest.h b/include/efi_selftest.h >>> index 7ec42a0406..978ca2a7ea 100644 >>> --- a/include/efi_selftest.h >>> +++ b/include/efi_selftest.h >>> @@ -12,6 +12,7 @@ >>> #include <common.h> >>> #include <efi.h> >>> #include <efi_api.h> >>> +#include <efi_loader.h> >>> #include <linker_lists.h> >>> >>> #define EFI_ST_SUCCESS 0 >>> @@ -71,6 +72,15 @@ void efi_st_printf(const char *fmt, ...) >>> */ >>> int efi_st_memcmp(const void *buf1, const void *buf2, size_t length); >>> >>> +/* >>> + * Compare an u16 string to a char string. >>> + * >>> + * @buf1: u16 string >>> + * @buf2: char string >>> + * @return: 0 if both buffers contain the same bytes >>> + */ >>> +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2); >>> + >>> /* >>> * Reads an Unicode character from the input device. >>> * >>> @@ -88,6 +98,7 @@ u16 efi_st_get_key(void); >>> * @setup: set up the unit test >>> * @teardown: tear down the unit test >>> * @execute: execute the unit test >>> + * @on_request: test is only executed on request >>> */ >>> struct efi_unit_test { >>> const char *name; >>> @@ -96,10 +107,17 @@ struct efi_unit_test { >>> const struct efi_system_table *systable); >>> int (*execute)(void); >>> int (*teardown)(void); >>> + bool on_request; >>> }; >>> >>> /* Declare a new EFI unit test */ >>> #define EFI_UNIT_TEST(__name) \ >>> ll_entry_declare(struct efi_unit_test, __name, efi_unit_test) >>> >>> +#define EFI_SELFTEST_TABLE_GUID \ >>> + EFI_GUID(0xbc3ebe57, 0x09e5, 0xa59d, 0xdb, 0x87, \ >>> + 0xf5, 0x79, 0x61, 0x62, 0x06, 0xde) >>> + >>> +extern const efi_guid_t efi_selftest_table_guid; >>> + >>> #endif /* _EFI_SELFTEST_H */ >>> diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c >>> index 45d8d3d384..110284f9c7 100644 >>> --- a/lib/efi_selftest/efi_selftest.c >>> +++ b/lib/efi_selftest/efi_selftest.c >>> @@ -15,6 +15,8 @@ static const struct efi_runtime_services *runtime; >>> static efi_handle_t handle; >>> static u16 reset_message[] = L"Selftest completed"; >>> >>> +const efi_guid_t efi_selftest_table_guid = EFI_SELFTEST_TABLE_GUID; >>> + >>> /* >>> * Exit the boot services. >>> * >>> @@ -25,8 +27,8 @@ static u16 reset_message[] = L"Selftest completed"; >>> */ >>> void efi_st_exit_boot_services(void) >>> { >>> - unsigned long map_size = 0; >>> - unsigned long map_key; >>> + unsigned long map_size = 0; >>> + unsigned long map_key; >> >> Unrelated? >> >>> unsigned long desc_size; >>> u32 desc_version; >>> efi_status_t ret; >>> @@ -133,6 +135,41 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures) >>> return ret; >>> } >>> >>> +/* >>> + * Check that a test exists. >>> + * >>> + * @testname: name of the test >>> + * @return: test >>> + */ >>> +static struct efi_unit_test *find_test(const u16 *testname) >>> +{ >>> + struct efi_unit_test *test; >>> + >>> + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); >>> + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { >>> + if (!efi_st_strcmp_16_8(testname, test->name)) >> >> Why not just use UCS2 strings here and compare 16 to 16? Maybe you're >> using the name more often in normal situations? >> >> Either way, not a biggie :) > > I thought about defining the test names as UCS2. But wide strings need > double the space. > > So I decided to stay with char * as far as possible to reduce code size. > > I remember that reviewing one patch you asked me to rearrange function > arguments to save 16 bytes of compiled code size for a specific > architecture. Yes, for code that is default =y, so we have much more potential to hit size constraints. Either way, I'm perfectly happy with leaving it at 8byte strings. Certainly makes them more readable :) Alex
diff --git a/cmd/bootefi.c b/cmd/bootefi.c index 18176a1266..2d70137482 100644 --- a/cmd/bootefi.c +++ b/cmd/bootefi.c @@ -6,10 +6,12 @@ * SPDX-License-Identifier: GPL-2.0+ */ +#include <charset.h> #include <common.h> #include <command.h> #include <dm.h> #include <efi_loader.h> +#include <efi_selftest.h> #include <errno.h> #include <libfdt.h> #include <libfdt_env.h> @@ -50,6 +52,32 @@ static void efi_init_obj_list(void) efi_get_time_init(); } +/* + * Set the load options of an image from an environment variable. + * + * @loaded_image_info: the image + * @env_var: name of the environment variable + */ +static void set_load_options(struct efi_loaded_image *loaded_image_info, + const char *env_var) +{ + size_t size; + const char *env = env_get(env_var); + + loaded_image_info->load_options = NULL; + loaded_image_info->load_options_size = 0; + if (!env) + return; + size = strlen(env) + 1; + loaded_image_info->load_options = calloc(size, sizeof(u16)); + if (!loaded_image_info->load_options) { + printf("ERROR: Out of memory\n"); + return; + } + utf8_to_utf16(loaded_image_info->load_options, (u8 *)env, size); + loaded_image_info->load_options_size = size * 2; +} + static void *copy_fdt(void *fdt) { u64 fdt_size = fdt_totalsize(fdt); @@ -190,6 +218,8 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, efi_install_configuration_table(&fdt_guid, NULL); } + /* Transfer environment variable bootargs as load options */ + set_load_options(&loaded_image_info, "bootargs"); /* Load the EFI payload */ entry = efi_load_pe(efi, &loaded_image_info); if (!entry) { @@ -237,6 +267,7 @@ static unsigned long do_bootefi_exec(void *efi, void *fdt, exit: /* image has returned, loaded-image obj goes *poof*: */ + free(loaded_image_info.load_options); list_del(&loaded_image_info_obj.link); return ret; @@ -301,17 +332,26 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) efi_setup_loaded_image(&loaded_image_info, &loaded_image_info_obj, - bootefi_device_path, bootefi_image_path); + NULL, NULL); /* * gd lives in a fixed register which may get clobbered while we * execute the payload. So save it here and restore it on every * callback entry */ efi_save_gd(); + loaded_image_info.image_code_type = EFI_LOADER_CODE; + loaded_image_info.image_data_type = EFI_LOADER_DATA; /* Initialize and populate EFI object list */ if (!efi_obj_list_initalized) efi_init_obj_list(); - return efi_selftest(&loaded_image_info, &systab); + /* Transfer environment variable efi_selftest as load options */ + set_load_options(&loaded_image_info, "efi_selftest"); + /* Execute the test */ + r = efi_selftest(&loaded_image_info, &systab); + efi_restore_gd(); + free(loaded_image_info.load_options); + list_del(&loaded_image_info_obj.link); + return r; } else #endif if (!strcmp(argv[1], "bootmgr")) { @@ -357,6 +397,8 @@ static char bootefi_help_text[] = #ifdef CONFIG_CMD_BOOTEFI_SELFTEST "bootefi selftest\n" " - boot an EFI selftest application stored within U-Boot\n" + " Use environment variable efi_selftest to select a single test.\n" + " Use 'setenv efi_selftest list' to enumerate all tests.\n" #endif "bootmgr [fdt addr]\n" " - load and boot EFI payload based on BootOrder/BootXXXX variables.\n" diff --git a/include/efi_selftest.h b/include/efi_selftest.h index 7ec42a0406..978ca2a7ea 100644 --- a/include/efi_selftest.h +++ b/include/efi_selftest.h @@ -12,6 +12,7 @@ #include <common.h> #include <efi.h> #include <efi_api.h> +#include <efi_loader.h> #include <linker_lists.h> #define EFI_ST_SUCCESS 0 @@ -71,6 +72,15 @@ void efi_st_printf(const char *fmt, ...) */ int efi_st_memcmp(const void *buf1, const void *buf2, size_t length); +/* + * Compare an u16 string to a char string. + * + * @buf1: u16 string + * @buf2: char string + * @return: 0 if both buffers contain the same bytes + */ +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2); + /* * Reads an Unicode character from the input device. * @@ -88,6 +98,7 @@ u16 efi_st_get_key(void); * @setup: set up the unit test * @teardown: tear down the unit test * @execute: execute the unit test + * @on_request: test is only executed on request */ struct efi_unit_test { const char *name; @@ -96,10 +107,17 @@ struct efi_unit_test { const struct efi_system_table *systable); int (*execute)(void); int (*teardown)(void); + bool on_request; }; /* Declare a new EFI unit test */ #define EFI_UNIT_TEST(__name) \ ll_entry_declare(struct efi_unit_test, __name, efi_unit_test) +#define EFI_SELFTEST_TABLE_GUID \ + EFI_GUID(0xbc3ebe57, 0x09e5, 0xa59d, 0xdb, 0x87, \ + 0xf5, 0x79, 0x61, 0x62, 0x06, 0xde) + +extern const efi_guid_t efi_selftest_table_guid; + #endif /* _EFI_SELFTEST_H */ diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c index 45d8d3d384..110284f9c7 100644 --- a/lib/efi_selftest/efi_selftest.c +++ b/lib/efi_selftest/efi_selftest.c @@ -15,6 +15,8 @@ static const struct efi_runtime_services *runtime; static efi_handle_t handle; static u16 reset_message[] = L"Selftest completed"; +const efi_guid_t efi_selftest_table_guid = EFI_SELFTEST_TABLE_GUID; + /* * Exit the boot services. * @@ -25,8 +27,8 @@ static u16 reset_message[] = L"Selftest completed"; */ void efi_st_exit_boot_services(void) { - unsigned long map_size = 0; - unsigned long map_key; + unsigned long map_size = 0; + unsigned long map_key; unsigned long desc_size; u32 desc_version; efi_status_t ret; @@ -133,6 +135,41 @@ static int teardown(struct efi_unit_test *test, unsigned int *failures) return ret; } +/* + * Check that a test exists. + * + * @testname: name of the test + * @return: test + */ +static struct efi_unit_test *find_test(const u16 *testname) +{ + struct efi_unit_test *test; + + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (!efi_st_strcmp_16_8(testname, test->name)) + return test; + } + efi_st_printf("\nTest '%ps' not found\n", testname); + return NULL; +} + +/* + * List all available tests. + */ +static void list_all_tests(void) +{ + struct efi_unit_test *test; + + /* List all tests */ + efi_st_printf("\nAvailable tests:\n"); + for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); + test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + efi_st_printf("'%s'%s\n", test->name, + test->on_request ? " - on request" : ""); + } +} + /* * Execute selftest of the EFI API * @@ -155,6 +192,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, { struct efi_unit_test *test; unsigned int failures = 0; + const u16 *testname = NULL; + struct efi_loaded_image *loaded_image; + efi_status_t ret; systable = systab; boottime = systable->boottime; @@ -163,14 +203,47 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, con_out = systable->con_out; con_in = systable->con_in; + ret = boottime->handle_protocol(image_handle, &efi_guid_loaded_image, + (void **)&loaded_image); + if (ret != EFI_SUCCESS) { + efi_st_error("Cannot open loaded image protocol"); + return ret; + } + + if (loaded_image->load_options) + testname = (u16 *)loaded_image->load_options; + + if (testname) { + if (!efi_st_strcmp_16_8(testname, "list") || + !find_test(testname)) { + list_all_tests(); + /* + * TODO: + * Once the Exit boottime service is correctly + * implemented we should call + * boottime->exit(image_handle, EFI_SUCCESS, 0, NULL); + * here, cf. + * https://lists.denx.de/pipermail/u-boot/2017-October/308720.html + */ + return EFI_SUCCESS; + } + } + efi_st_printf("\nTesting EFI API implementation\n"); - efi_st_printf("\nNumber of tests to execute: %u\n", - ll_entry_count(struct efi_unit_test, efi_unit_test)); + if (testname) + efi_st_printf("\nSelected test: '%ps'\n", testname); + else + efi_st_printf("\nNumber of tests to execute: %u\n", + ll_entry_count(struct efi_unit_test, + efi_unit_test)); /* Execute boottime tests */ for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (testname ? + efi_st_strcmp_16_8(testname, test->name) : test->on_request) + continue; if (test->phase == EFI_EXECUTE_BEFORE_BOOTTIME_EXIT) { setup(test, &failures); execute(test, &failures); @@ -181,6 +254,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, /* Execute mixed tests */ for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (testname ? + efi_st_strcmp_16_8(testname, test->name) : test->on_request) + continue; if (test->phase == EFI_SETUP_BEFORE_BOOTTIME_EXIT) setup(test, &failures); } @@ -189,6 +265,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (testname ? + efi_st_strcmp_16_8(testname, test->name) : test->on_request) + continue; if (test->phase == EFI_SETUP_BEFORE_BOOTTIME_EXIT) { execute(test, &failures); teardown(test, &failures); @@ -198,6 +277,9 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle, /* Execute runtime tests */ for (test = ll_entry_start(struct efi_unit_test, efi_unit_test); test < ll_entry_end(struct efi_unit_test, efi_unit_test); ++test) { + if (testname ? + efi_st_strcmp_16_8(testname, test->name) : test->on_request) + continue; if (test->phase == EFI_SETUP_AFTER_BOOTTIME_EXIT) { setup(test, &failures); execute(test, &failures); diff --git a/lib/efi_selftest/efi_selftest_console.c b/lib/efi_selftest/efi_selftest_console.c index 840e2290c6..6a7fd20da5 100644 --- a/lib/efi_selftest/efi_selftest_console.c +++ b/lib/efi_selftest/efi_selftest_console.c @@ -142,6 +142,7 @@ void efi_st_printf(const char *fmt, ...) const char *c; u16 *pos = buf; const char *s; + const u16 *u; va_start(args, fmt); @@ -179,9 +180,18 @@ void efi_st_printf(const char *fmt, ...) case 'p': ++c; switch (*c) { + /* MAC address */ case 'm': mac(va_arg(args, void*), &pos); break; + + /* u16 string */ + case 's': + u = va_arg(args, u16*); + /* Ensure string fits into buffer */ + for (; *u && pos < buf + 120; ++u) + *pos++ = *u; + break; default: --c; pointer(va_arg(args, void*), &pos); diff --git a/lib/efi_selftest/efi_selftest_util.c b/lib/efi_selftest/efi_selftest_util.c index 5cffe383d8..1b17bf4d4b 100644 --- a/lib/efi_selftest/efi_selftest_util.c +++ b/lib/efi_selftest/efi_selftest_util.c @@ -21,5 +21,14 @@ int efi_st_memcmp(const void *buf1, const void *buf2, size_t length) ++pos1; ++pos2; } - return EFI_ST_SUCCESS; + return 0; +} + +int efi_st_strcmp_16_8(const u16 *buf1, const char *buf2) +{ + for (; *buf1 || *buf2; ++buf1, ++buf2) { + if (*buf1 != *buf2) + return *buf1 - *buf2; + } + return 0; }
Environment variable efi_selftest is passed as load options to the selftest application. It is used to select a single test to be executed. Special value 'list' displays a list of all available tests. Tests get an on_request property. If this property is set the tests are only executed if explicitly requested. The invocation of efi_selftest is changed to reflect that bootefi selftest with efi_selftest = 'list' will call the Exit bootservice. Environment variable bootargs is used as load options for all other bootefi payloads. Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de> --- v2 use an environment variable to choose a test --- cmd/bootefi.c | 46 ++++++++++++++++- include/efi_selftest.h | 18 +++++++ lib/efi_selftest/efi_selftest.c | 90 +++++++++++++++++++++++++++++++-- lib/efi_selftest/efi_selftest_console.c | 10 ++++ lib/efi_selftest/efi_selftest_util.c | 11 +++- 5 files changed, 168 insertions(+), 7 deletions(-)