From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 17 Apr 2019 16:19:22 +0900 Subject: [U-Boot] [RFC v3 05/10] cmd: bootefi: carve out efi_selftest code from do_bootefi() In-Reply-To: <2d87af3f-5e23-38b9-dd9f-17c3e3de51f6@gmx.de> References: <20190416042428.5007-1-takahiro.akashi@linaro.org> <20190416042428.5007-6-takahiro.akashi@linaro.org> <2d87af3f-5e23-38b9-dd9f-17c3e3de51f6@gmx.de> Message-ID: <20190417071921.GR7158@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Apr 16, 2019 at 07:21:26AM +0200, Heinrich Schuchardt wrote: > 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. First of all, this is a preparation for do_efibootmgr(). In addition, as I suggested, we should also convert selftest to a standalone application. > >+ > >+ 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. Quit easy to follow. -Takahiro Akashi > 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(); > > >