On 2/20/19 3:20 AM, AKASHI Takahiro wrote:
> On Tue, Feb 19, 2019 at 08:46:52PM +0100, Heinrich Schuchardt wrote:
>> On 1/24/19 12:04 PM, AKASHI Takahiro wrote:
>>> "devices" command prints all the uefi variables on the system.
>>>
>>> => efi devices
>>> Scanning disk ahci_scsi.id0lun0...
>>> Scanning disk ahci_scsi.id1lun0...
>>> Found 4 disks
>>> Device Device Path
>>> ================ ====================
>>> 000000007ef07ea0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
>>> 000000007ef00c10 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(0,0)
>>> 000000007ef00dd0 /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)
>>> 000000007ef07be0
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(1,MBR,0x086246ba,0x800,0x40000)
>>> 000000007ef07510
>>> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/Scsi(1,0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
>>>
>>> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
>>> ---
>>> cmd/efidebug.c | 102 ++++++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 101 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/cmd/efidebug.c b/cmd/efidebug.c
>>> index d836576cf6e0..8a7f775b117a 100644
>>> --- a/cmd/efidebug.c
>>> +++ b/cmd/efidebug.c
>>> @@ -15,6 +15,102 @@
>>> #include <search.h>
>>> #include <linux/ctype.h>
>>>
>>> +#define BS systab.boottime
>>> +
>>
>> Please, add Sphinx style descriptions to all functions, their
>> parameters, and return values.
>
> Sure, but please clearify what you mean by "all."
> Do you expect descriptions for exactly all the functions,
> even for some helper functions of a few lines of code?
>
>> Why do you need this function? Can't you call LocateHandleBuffer() which
>> takes care of the allocation of the buffer?
>>
>>> +static int efi_get_handles_by_proto(efi_guid_t *guid, efi_handle_t
>>> **handlesp,
>>> + int *num)
>
> Right. I will use locate_handle_buffer here.
> But why does UEFI spec define functionally-duplicated functions,
> LocateHandle() and LocateHandleBuffer()?
>
>>> +{
>>> + efi_handle_t *handles = NULL;
>>> + efi_uintn_t size = 0;
>>> + efi_status_t ret;
>>> +
>>> + if (guid) {
>>> + ret = BS->locate_handle(BY_PROTOCOL, guid, NULL, &size,
>>> + handles);
>>
>> This will not work on i386. Please, always use EFI_CALL when invoking a
>> function which calls EFI_ENTRY().
>
> OK, but let me make sure.
> We don't have to use EFI_CALL() for any of (internal) efi_xxx() functions,
> say, efi_add_protocol(). Right?
EFI_ENTRY() and EFI_EXIT() change the content of the register to which
gd is mapped and executes an assert. So only functions starting with
EFI_ENTRY() should be called via EFI_CALL. Please, do not call other
functions via EFI_CALL().
Please, check that efi_save_gd() is called (e.g. via
efi_init_obj_list()) before the first EFI_CALL().
To get rid of the systab.boottime dereference it might be preferable to
change static functions to global once.
Best regards
Heinrich
>
>>> + if (ret == EFI_BUFFER_TOO_SMALL) {
>>> + handles = calloc(1, size);
>>> + if (!handles)
>>> + return -1;
>>> +
>>> + ret = BS->locate_handle(BY_PROTOCOL, guid, NULL, &size,
>>> + handles);
>>> + }
>>> + if (ret != EFI_SUCCESS) {
>>> + free(handles);
>>
>> The handles are not allocated by malloc(). Use FreePool() here.
>
> Will take care of that along with the change above.
>
> -Takahiro Akashi
>
>
>>> + return -1;
>>> + }
>>> + } else {
>>> + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size, NULL);
>>> + if (ret == EFI_BUFFER_TOO_SMALL) {
>>> + handles = calloc(1, size);
>>> + if (!handles)
>>> + return -1;
>>> +
>>> + ret = BS->locate_handle(ALL_HANDLES, NULL, NULL, &size,
>>> + handles);
>>> + }
>>> + if (ret != EFI_SUCCESS) {
>>> + free(handles);
>>
>> Same here.
>>
>>> + return -1;
>>> + }
>>> + }
>>> +
>>> + *handlesp = handles;
>>> + *num = size / sizeof(efi_handle_t);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static int efi_get_device_handle_info(efi_handle_t handle, u16
>>> **dev_path_text)
>>> +{
>>> + struct efi_device_path *dp;
>>> + efi_status_t ret;
>>> +
>>> + ret = BS->open_protocol(handle, &efi_guid_device_path,
>>> + (void **)&dp, NULL /* FIXME */, NULL,
>>> + EFI_OPEN_PROTOCOL_GET_PROTOCOL);
>>> + if (ret == EFI_SUCCESS) {
>>> + *dev_path_text = efi_dp_str(dp);
>>> + return 0;
>>> + } else {
>>> + return -1;
>>> + }
>>> +}
>>> +
>>> +#define EFI_HANDLE_WIDTH ((int)sizeof(efi_handle_t) * 2)
>>> +
>>> +static const char spc[] = " ";
>>> +static const char sep[] = "================";
>>> +
>>> +static int do_efi_show_devices(cmd_tbl_t *cmdtp, int flag,
>>> + int argc, char * const argv[])
>>> +{
>>> + efi_handle_t *handles;
>>> + u16 *dev_path_text;
>>> + int num, i;
>>> +
>>> + handles = NULL;
>>> + num = 0;
>>> + if (efi_get_handles_by_proto(NULL, &handles, &num))
>>> + return CMD_RET_FAILURE;
>>> +
>>> + if (!num)
>>> + return CMD_RET_SUCCESS;
>>> +
>>> + printf("Device%.*s Device Path\n", EFI_HANDLE_WIDTH - 6, spc);
>>> + printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep);
>>> + for (i = 0; i < num; i++) {
>>> + if (!efi_get_device_handle_info(handles[i], &dev_path_text)) {
>>> + printf("%p %ls\n", handles[i], dev_path_text);
>>> + efi_free_pool(dev_path_text);
>>> + }
>>> + }
>>> +
>>> + free(handles);
>>
>> FreePool()!
>>
>> Best regards
>>
>> Heinrich
>>
>>> +
>>> + return CMD_RET_SUCCESS;
>>> +}
>>> +
>>> static int do_efi_boot_add(cmd_tbl_t *cmdtp, int flag,
>>> int argc, char * const argv[])
>>> {
>>> @@ -406,6 +502,8 @@ static int do_efi_boot_opt(cmd_tbl_t *cmdtp, int flag,
>>>
>>> static cmd_tbl_t cmd_efidebug_sub[] = {
>>> U_BOOT_CMD_MKENT(boot, CONFIG_SYS_MAXARGS, 1, do_efi_boot_opt, "", ""),
>>> + U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices,
>>> + "", ""),
>>> };
>>>
>>> /* Interpreter command to configure UEFI environment */
>>> @@ -451,7 +549,9 @@ static char efidebug_help_text[] =
>>> " - set UEFI BootNext variable\n"
>>> "efidebug boot order [<bootid#1> [<bootid#2> [<bootid#3> [...]]]]\n"
>>> " - set/show UEFI boot order\n"
>>> - "\n";
>>> + "\n"
>>> + "efidebug devices\n"
>>> + " - show uefi devices\n";
>>> #endif
>>>
>>> U_BOOT_CMD(
>>>
>>
>
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
