| Message ID | 20190327044042.13707-9-takahiro.akashi@linaro.org |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | Heinrich Schuchardt |
| Headers | show |
| Series | efi_loader: rework bootefi/bootmgr | expand |
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 <takahiro.akashi@linaro.org> > --- > 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()? 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); >
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 <takahiro.akashi@linaro.org> > >--- > > 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. -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); > > >
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 <takahiro.akashi@linaro.org> >>> --- >>> 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. 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); >>> >> >
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 <takahiro.akashi@linaro.org> > >>> --- > >>> 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. -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); > >>> > >> > >
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 <takahiro.akashi@linaro.org> >>>>> --- >>>>> 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. 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); >>>>> >>>> >>> >
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 <takahiro.akashi@linaro.org> > >>>>>--- > >>>>> 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); > >>>>> > >>>> > >>> > > >
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(); + + /* 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);
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 <takahiro.akashi@linaro.org> --- cmd/bootefi.c | 42 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 8 deletions(-)