On 06/18/2018 04:08 PM, Simon Glass wrote:
> We plan to run more than one EFI test. In order to avoid duplicating code,
> create functions which handle preparing for running the test and cleaning
> up afterwards.
>
> Also shorten the awfully long variable names here.
>
> Signed-off-by: Simon Glass <s...@chromium.org>
> ---
>
> Changes in v8: None
> Changes in v7: None
> Changes in v6: None
> Changes in v5:
> - Drop call to efi_init_obj_list() which is now done in do_bootefi()
>
> Changes in v4: None
> Changes in v3:
> - Add new patch to split out test init/uninit into functions
>
> Changes in v2: None
>
> cmd/bootefi.c | 87 +++++++++++++++++++++++++++++++++++++--------------
> 1 file changed, 63 insertions(+), 24 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index f55a40dc84..2dfd297e78 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -356,6 +356,60 @@ exit:
> return ret;
> }
>
> +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> +/**
> + * bootefi_test_prepare() - prepare to run an EFI test
> + *
> + * This sets things up so we can call EFI functions. This involves preparing
> + * the 'gd' pointer and setting up the load ed image data structures.
> + *
> + * @image: Pointer to a struct which will hold the loaded image info
> + * @obj: Pointer to a struct which will hold the loaded image object
> + * @path: File path to the test being run (often just the test name with a
> + * backslash before it
> + * @test_func: Address of the test function that is being run
> + * @return 0 if OK, -ve on error
> + */
> +static efi_status_t bootefi_test_prepare(struct efi_loaded_image *image,
> + struct efi_object *obj,
> + const char *path, ulong test_func)
> +{
> + memset(image, '\0', sizeof(*image));
> + memset(obj, '\0', sizeof(*obj));
It is unclear why you are adding the memsets here.
> + /* Construct a dummy device path */
> + bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> + (uintptr_t)test_func,
> + (uintptr_t)test_func);
efi_dp_from_mem() may return NULL in case of an error.
> + bootefi_image_path = efi_dp_from_file(NULL, 0, path);
> + efi_setup_loaded_image(image, obj, bootefi_device_path,
> + bootefi_image_path);
The return code of efi_setup_loaded_image() should be checked. This
function should not return 0 if the return code is not EFI_SUCCESS.
Best regards
Heinrich
> + /*
> + * 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();
> +
> + /* Transfer environment variable efi_selftest as load options */
> + set_load_options(image, "efi_selftest");
> +
> + return 0;
> +}
> +
> +/**
> + * bootefi_test_finish() - finish up after running an EFI test
> + *
> + * @image: Pointer to a struct which holds the loaded image info
> + * @obj: Pointer to a struct which holds the loaded image object
> + */
> +static void bootefi_test_finish(struct efi_loaded_image *image,
> + struct efi_object *obj)
> +{
> + efi_restore_gd();
> + free(image->load_options);
> + list_del(&obj->link);
> +}
> +#endif /* CONFIG_CMD_BOOTEFI_SELFTEST */
> +
> static int do_bootefi_bootmgr_exec(void)
> {
> struct efi_device_path *device_path, *file_path;
> @@ -434,31 +488,16 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int
> argc, char * const argv[])
> #endif
> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST
> if (!strcmp(argv[1], "selftest")) {
> - struct efi_loaded_image loaded_image_info = {};
> - struct efi_object loaded_image_info_obj = {};
> -
> - /* Construct a dummy device path. */
> - bootefi_device_path = efi_dp_from_mem(EFI_RESERVED_MEMORY_TYPE,
> - (uintptr_t)&efi_selftest,
> - (uintptr_t)&efi_selftest);
> - bootefi_image_path = efi_dp_from_file(NULL, 0, "\\selftest");
> -
> - efi_setup_loaded_image(&loaded_image_info,
> - &loaded_image_info_obj,
> - bootefi_device_path, bootefi_image_path);
> - /*
> - * 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();
> - /* Transfer environment variable efi_selftest as load options */
> - set_load_options(&loaded_image_info, "efi_selftest");
> + struct efi_loaded_image image;
> + struct efi_object obj;
> +
> + if (bootefi_test_prepare(&image, &obj, "\\selftest",
> + (uintptr_t)&efi_selftest))
> + return CMD_RET_FAILURE;
> +
> /* Execute the test */
> - r = efi_selftest(loaded_image_info_obj.handle, &systab);
> - efi_restore_gd();
> - free(loaded_image_info.load_options);
> - list_del(&loaded_image_info_obj.link);
> + r = efi_selftest(obj.handle, &systab);
> + bootefi_test_finish(&image, &obj);
> return r != EFI_SUCCESS;
> } else
> #endif
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
