From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 16 Apr 2019 13:20:51 +0900 Subject: [U-Boot] [RFC v2 08/11] cmd: bootefi: carve out bootmgr code from do_bootefi() In-Reply-To: <380d6c85-56ec-ded6-08a9-28038c04f497@gmx.de> References: <20190327044042.13707-1-takahiro.akashi@linaro.org> <20190327044042.13707-9-takahiro.akashi@linaro.org> <20190412070653.GK7158@linaro.org> <20190412141935.GA10726@fireball> <380d6c85-56ec-ded6-08a9-28038c04f497@gmx.de> Message-ID: <20190416042050.GN7158@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 Fri, Apr 12, 2019 at 10:28:09PM +0200, Heinrich Schuchardt wrote: > On 4/12/19 4:19 PM, AKASHI Takahiro wrote: > >On Fri, Apr 12, 2019 at 10:58:25AM +0200, Heinrich Schuchardt wrote: > >> > >> > >>On 4/12/19 9:06 AM, AKASHI Takahiro wrote: > >>>On Fri, Apr 12, 2019 at 07:55:16AM +0200, Heinrich Schuchardt wrote: > >>>>On 3/27/19 5:40 AM, AKASHI Takahiro wrote: > >>>>>This is a preparatory patch for reworking do_bootefi() in later patch. > >>>>>do_bootmgr_exec() is renamed to do_efibootmgr() as we put all the necessary > >>>>>code into this function. > >>>>> > >>>>>Signed-off-by: AKASHI Takahiro > >>>>>--- > >>>>> cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++-------- > >>>>> 1 file changed, 34 insertions(+), 8 deletions(-) > >>>>> > >>>>>diff --git a/cmd/bootefi.c b/cmd/bootefi.c > >>>>>index e9d4881997a1..94b5bdeed3f1 100644 > >>>>>--- a/cmd/bootefi.c > >>>>>+++ b/cmd/bootefi.c > >>>>>@@ -309,22 +309,47 @@ err_add_protocol: > >>>>> return ret; > >>>>> } > >>>>> > >>>>>-static int do_bootefi_bootmgr_exec(void) > >>>>>+/** > >>>>>+ * do_efibootmgr() - execute EFI Boot Manager > >>>>>+ * > >>>>>+ * @fdt_opt: string of fdt start address > >>>>>+ * Return: status code > >>>>>+ * > >>>>>+ * Execute EFI Boot Manager > >>>>>+ */ > >>>>>+static int do_efibootmgr(const char *fdt_opt) > >>>>> { > >>>>> struct efi_device_path *device_path, *file_path; > >>>>> void *addr; > >>>>>- efi_status_t r; > >>>>>+ efi_status_t ret; > >>>>>+ > >>>>>+ /* Allow unaligned memory access */ > >>>>>+ allow_unaligned(); > >>>>>+ > >>>>>+ switch_to_non_secure_mode(); > >>>>>+ > >>>> > >>>>Shouldn't we move these two call to efi_init_obj_list()? > >>> > >>>Given the fact that efi_init_obj_list() is called without invoking > >>>any UEFI binary at some places, I'm not sure that it is the right > >>>place where switch_to_non_secure_mode() be called. > > > >If this is UEFI(and arm)-specific, it should be placed just > >before setjmp() in efi_start_image(). > > > >But I'm not sure whether we should call switch_to_non_secure_mode() > >even for a driver, which is logically part of U-Boot UEFI. > > switch_to_not_secure_mode() is a weak function which is implemented only > for armv7 and armv8. > > efi_init_obj_list() would be a safe place to call it. You also suggested initr_reloc_global_data(). No? Anyhow, I think you ignored my concern: > >But I'm not sure whether we should call switch_to_non_secure_mode() > >even for a driver, which is logically part of U-Boot UEFI. Since I think that we need discuss more, I will leave the code unchanged in the next version. -Takahiro Akashi > > Best regards > > Heinrich > > > > >-Takahiro Akashi > > > >>I think this could even be done in initr_reloc_global_data(). > >> > >>@Alex > >>What are your thoughts. > >> > >>Best regards > >> > >>Heinrich > >> > >>> > >>>-Takahiro Akashi > >>> > >>> > >>>>Best regards > >>>> > >>>>Heinrich > >>>> > >>>>>+ /* 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; > >>>>>+ } > >>>>>+ > >>>>>+ ret = efi_install_fdt(fdt_opt); > >>>>>+ if (ret != EFI_SUCCESS) > >>>>>+ return CMD_RET_FAILURE; > >>>>> > >>>>> addr = efi_bootmgr_load(&device_path, &file_path); > >>>>> if (!addr) > >>>>> return 1; > >>>>> > >>>>> printf("## Starting EFI application at %p ...\n", addr); > >>>>>- r = do_bootefi_exec(addr, device_path, file_path); > >>>>>+ ret = do_bootefi_exec(addr, device_path, file_path); > >>>>> printf("## Application terminated, r = %lu\n", > >>>>>- r & ~EFI_ERROR_MASK); > >>>>>+ ret & ~EFI_ERROR_MASK); > >>>>> > >>>>>- if (r != EFI_SUCCESS) > >>>>>+ if (ret != EFI_SUCCESS) > >>>>> return 1; > >>>>> > >>>>> return 0; > >>>>>@@ -463,6 +488,9 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > >>>>> > >>>>> if (argc < 2) > >>>>> return CMD_RET_USAGE; > >>>>>+ > >>>>>+ if (!strcmp(argv[1], "bootmgr")) > >>>>>+ return do_efibootmgr(argc > 2 ? argv[2] : NULL); > >>>>> #ifdef CONFIG_CMD_BOOTEFI_SELFTEST > >>>>> else if (!strcmp(argv[1], "selftest")) > >>>>> return do_efi_selftest(argc > 2 ? argv[2] : NULL); > >>>>>@@ -497,9 +525,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[]) > >>>>> memcpy(map_sysmem(addr, size), __efi_helloworld_begin, size); > >>>>> } else > >>>>> #endif > >>>>>- if (!strcmp(argv[1], "bootmgr")) { > >>>>>- return do_bootefi_bootmgr_exec(); > >>>>>- } else { > >>>>>+ { > >>>>> saddr = argv[1]; > >>>>> > >>>>> addr = simple_strtoul(saddr, NULL, 16); > >>>>> > >>>> > >>> > > >