From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Tue, 16 Apr 2019 07:21:26 +0200 Subject: [U-Boot] [RFC v3 05/10] cmd: bootefi: carve out efi_selftest code from do_bootefi() In-Reply-To: <20190416042428.5007-6-takahiro.akashi@linaro.org> References: <20190416042428.5007-1-takahiro.akashi@linaro.org> <20190416042428.5007-6-takahiro.akashi@linaro.org> Message-ID: <2d87af3f-5e23-38b9-dd9f-17c3e3de51f6@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On 4/16/19 6:24 AM, AKASHI Takahiro wrote: > This is a preparatory patch for reworking do_bootefi() in later patch. > > Efi_selftest code is unusual in terms of execution path in do_bootefi(), > which make that function complicated and hard to understand. With this > patch, all efi_selftest related code will be put in a separate function. > > The change also includes expanding efi_run_prepare() and efi_run_finish() > in do_bootefi_exec(). > > Signed-off-by: AKASHI Takahiro > --- > cmd/bootefi.c | 147 +++++++++++++++++++++++++++++++------------------- > 1 file changed, 92 insertions(+), 55 deletions(-) > > diff --git a/cmd/bootefi.c b/cmd/bootefi.c > index 0f58f51cbc76..a5dba6645ca2 100644 > --- a/cmd/bootefi.c > +++ b/cmd/bootefi.c > @@ -219,39 +219,6 @@ static efi_status_t efi_install_fdt(const char *fdt_opt) > return EFI_SUCCESS; > } > > -static efi_status_t bootefi_run_prepare(const char *load_options_path, > - struct efi_device_path *device_path, > - struct efi_device_path *image_path, > - struct efi_loaded_image_obj **image_objp, > - struct efi_loaded_image **loaded_image_infop) > -{ > - efi_status_t ret; > - > - ret = efi_setup_loaded_image(device_path, image_path, image_objp, > - loaded_image_infop); > - if (ret != EFI_SUCCESS) > - return ret; > - > - /* Transfer environment variable as load options */ > - set_load_options(*loaded_image_infop, load_options_path); > - > - return 0; > -} > - > -/** > - * bootefi_run_finish() - finish up after running an EFI test > - * > - * @loaded_image_info: Pointer to a struct which holds the loaded image info > - * @image_objj: Pointer to a struct which holds the loaded image object > - */ > -static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, > - struct efi_loaded_image *loaded_image_info) > -{ > - efi_restore_gd(); > - free(loaded_image_info->load_options); > - efi_delete_handle(&image_obj->header); > -} > - > /** > * do_bootefi_exec() - execute EFI binary > * > @@ -298,11 +265,14 @@ static efi_status_t do_bootefi_exec(void *efi, > assert(device_path && image_path); > } > > - ret = bootefi_run_prepare("bootargs", device_path, image_path, > - &image_obj, &loaded_image_info); > + ret = efi_setup_loaded_image(device_path, image_path, &image_obj, > + &loaded_image_info); > if (ret) > goto err_prepare; > > + /* Transfer environment variable as load options */ > + set_load_options(loaded_image_info, "bootargs"); > + > /* Load the EFI payload */ > ret = efi_load_pe(image_obj, efi, loaded_image_info); > if (ret != EFI_SUCCESS) > @@ -326,7 +296,9 @@ static efi_status_t do_bootefi_exec(void *efi, > > err_prepare: > /* image has returned, loaded-image obj goes *poof*: */ > - bootefi_run_finish(image_obj, loaded_image_info); > + efi_restore_gd(); > + free(loaded_image_info->load_options); > + efi_delete_handle(&image_obj->header); > > err_add_protocol: > if (mem_handle) > @@ -336,6 +308,25 @@ err_add_protocol: > } > > #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > +static efi_status_t bootefi_run_prepare(const char *load_options_path, > + struct efi_device_path *device_path, > + struct efi_device_path *image_path, > + struct efi_loaded_image_obj **image_objp, > + struct efi_loaded_image **loaded_image_infop) > +{ > + efi_status_t ret; > + > + ret = efi_setup_loaded_image(device_path, image_path, image_objp, > + loaded_image_infop); > + if (ret != EFI_SUCCESS) > + return ret; > + > + /* Transfer environment variable as load options */ > + set_load_options(*loaded_image_infop, load_options_path); > + > + return 0; > +} > + > /** > * bootefi_test_prepare() - prepare to run an EFI test > * > @@ -381,6 +372,64 @@ failure: > return ret; > } > > +/** > + * bootefi_run_finish() - finish up after running an EFI test > + * > + * @loaded_image_info: Pointer to a struct which holds the loaded image info > + * @image_obj: Pointer to a struct which holds the loaded image object > + */ > +static void bootefi_run_finish(struct efi_loaded_image_obj *image_obj, > + struct efi_loaded_image *loaded_image_info) > +{ > + efi_restore_gd(); > + free(loaded_image_info->load_options); > + efi_delete_handle(&image_obj->header); > +} > + > +/** > + * do_efi_selftest() - execute EFI Selftest > + * > + * @fdt_opt: string of fdt start address > + * Return: status code > + * > + * Execute EFI Selftest > + */ > +static int do_efi_selftest(const char *fdt_opt) > +{ > + struct efi_loaded_image_obj *image_obj; > + struct efi_loaded_image *loaded_image_info; > + efi_status_t ret; > + > + /* Allow unaligned memory access */ > + allow_unaligned(); > + > + switch_to_non_secure_mode(); > + > + /* Initialize EFI drivers */ > + ret = efi_init_obj_list(); > + if (ret != EFI_SUCCESS) { > + printf("Error: Cannot initialize UEFI sub-system, r = %lu\n", > + ret & ~EFI_ERROR_MASK); > + return CMD_RET_FAILURE; > + } allow_unaligned(), switch_to_non_secure_mode(), and efi_init_obj_list() are needed for any invocation do_bootefi(). Putting them here makes only sense if you want to create a completely separate command for the selftest. > + > + ret = efi_install_fdt(fdt_opt); > + if (ret == EFI_INVALID_PARAMETER) > + return CMD_RET_USAGE; > + else if (ret != EFI_SUCCESS) > + return CMD_RET_FAILURE; > + > + ret = bootefi_test_prepare(&image_obj, &loaded_image_info, > + "\\selftest", "efi_selftest"); > + if (ret != EFI_SUCCESS) > + return CMD_RET_FAILURE; > + > + /* Execute the test */ > + ret = EFI_CALL(efi_selftest(&image_obj->header, &systab)); > + bootefi_run_finish(image_obj, loaded_image_info); > + > + return ret != EFI_SUCCESS; > +} > #endif /* CONFIG_CMD_BOOTEFI_SELFTEST */ > > static int do_bootefi_bootmgr_exec(void) > @@ -412,6 +461,13 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > efi_status_t r; > unsigned long fdt_addr; > > + if (argc < 2) > + return CMD_RET_USAGE; > +#ifdef CONFIG_CMD_BOOTEFI_SELFTEST > + else if (!strcmp(argv[1], "selftest")) > + return do_efi_selftest(argc > 2 ? argv[2] : NULL); doc/README.commands describes the default way to handle sub-commands. I think that is where we should move in the long run. Maybe not in this patch series. Best regards Heinrich > +#endif > + > /* Allow unaligned memory access */ > allow_unaligned(); > > @@ -425,9 +481,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > return CMD_RET_FAILURE; > } > > - if (argc < 2) > - return CMD_RET_USAGE; > - > r = fdt_install_fdt(argc > 2 ? argv[2] : NULL); > if (r == EFI_INVALID_PARAMETER) > return CMD_RET_USAGE; > @@ -445,22 +498,6 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > addr = CONFIG_SYS_LOAD_ADDR; > memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); > } else > -#endif > -#ifdef CONFIG_CMD_BOOTEFI_SELFTEST > - if (!strcmp(argv[1], "selftest")) { > - struct efi_loaded_image_obj *image_obj; > - struct efi_loaded_image *loaded_image_info; > - > - r = bootefi_test_prepare(&image_obj, &loaded_image_info, > - "\\selftest", "efi_selftest"); > - if (r != EFI_SUCCESS) > - return CMD_RET_FAILURE; > - > - /* Execute the test */ > - r = EFI_CALL(efi_selftest(&image_obj->header, &systab)); > - bootefi_run_finish(image_obj, loaded_image_info); > - return r != EFI_SUCCESS; > - } else > #endif > if (!strcmp(argv[1], "bootmgr")) { > return do_bootefi_bootmgr_exec(); >