[U-Boot] [PATCH v6 3/7] cmd: efidebug: add devices command

[Heinrich Schuchardt]

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.akashi at 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.

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)
> +{
> +	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().

> +		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.

> +			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(
>