[U-Boot] [RFC v2 08/11] cmd: bootefi: carve out bootmgr code from do_bootefi()

[Heinrich Schuchardt]

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 at 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);
>>>
>>
>