From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from phobos.denx.de (phobos.denx.de [85.214.62.61]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 66807C433FE for ; Wed, 5 Oct 2022 06:27:48 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id 5508084E37; Wed, 5 Oct 2022 08:27:46 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (2048-bit key; unprotected) header.d=canonical.com header.i=@canonical.com header.b="fES29pK5"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id D6A0284E3C; Wed, 5 Oct 2022 08:27:43 +0200 (CEST) Received: from smtp-relay-internal-1.canonical.com (smtp-relay-internal-1.canonical.com [185.125.188.123]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) (No client certificate requested) by phobos.denx.de (Postfix) with ESMTPS id B910F84DF7 for ; Wed, 5 Oct 2022 08:27:40 +0200 (CEST) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=canonical.com Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=heinrich.schuchardt@canonical.com Received: from mail-ej1-f69.google.com (mail-ej1-f69.google.com [209.85.218.69]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-1.canonical.com (Postfix) with ESMTPS id 2B1753F19B for ; Wed, 5 Oct 2022 06:27:39 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=canonical.com; s=20210705; t=1664951259; bh=eTVyxW+0zGwEgXJ9xSrHKcdlmeh2sgTN5jteEpLKipQ=; h=Message-ID:Date:MIME-Version:Subject:To:References:Cc:From: In-Reply-To:Content-Type; b=fES29pK5rjHEaE1Gl561bX0V4CUw1P8BY3LVxpFLobI4IaiphgdsbF8mQoOhgmXhl z2Xumap/4YtlmtFHQZkUdJTf7AJomKWZYTtFx59tKpKNkWcXjUJccmKGlljjx5TOwA egZAfmYbSw9NSoOpNbWyzex/eOj/I6mkHcekhn5+gtEXu96EPRwzI2OAXE0AAPXPSN 3C9rCQLlZpcOF6yui9i3P/pLgqMomuexD0eupClKeXRms2BzXz2MQ/sy8EIRSRBFPp u8BiqJGJjwZiEYWJry3fX2/xel+DBKhitVP5MCZekK9lQitzhSwqEMfNRziU74crzl x+GloGep6FE7A== Received: by mail-ej1-f69.google.com with SMTP id he8-20020a1709073d8800b007838dd153f4so5951978ejc.12 for ; Tue, 04 Oct 2022 23:27:39 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:in-reply-to:from:cc:references:to :content-language:subject:user-agent:mime-version:date:message-id :x-gm-message-state:from:to:cc:subject:date; bh=eTVyxW+0zGwEgXJ9xSrHKcdlmeh2sgTN5jteEpLKipQ=; b=M1qqIE35GNYOQm1oDJSuAvHthZhFsCaCjRz0XcUuqL3jRKKEshGvHd6lLFks3VKXjr 7aK0xA0Af/e7vVnO570lEVuD6Y+qWiLzwVaWZF0+YLI8so0MrszJ9nM0/hZ7KcXdNphT sTCXcoB7U2GN2Fpk8mYYbQ25Qby/+i56tFAXtRkj0EdI6VdhfgOby4NTYp/k7PeCSqnt czPgR7wuqVdBMoD1mqq77ZG4Wh1fnu5EKaMcuRJv82U7CM6Elq1tso+TGeIHMFMBrNvh 7mv0py1UCq+tjX39F0Mb+0wM1g2JNG5M+AtIWtHKy/cWRYpVJMhXM3P3bRrunR4mN65U la3Q== X-Gm-Message-State: ACrzQf1UoE/00VRKuTD5A3Ya2IzB/5zCYNee6b36maCchrt8wiUQSdh1 tvdMtztPtar9EUWc0+ROx2ejfba8LL8owH0ZhbpFvipRiMy2KEuptN6xgJ1VQalkqH+rMb3ms2M V52k49iKJs27okBIU6Z6x+3Vmdzv+n6c= X-Received: by 2002:a17:907:2d14:b0:784:8e6e:aec5 with SMTP id gs20-20020a1709072d1400b007848e6eaec5mr22811478ejc.477.1664951258762; Tue, 04 Oct 2022 23:27:38 -0700 (PDT) X-Google-Smtp-Source: AMsMyM53KYHv4A6evq2eD29gNIWo/LBNV26qHG2d9BYdyskG56b3JGTlkMZet/YUok0cT7qJt1ugSA== X-Received: by 2002:a17:907:2d14:b0:784:8e6e:aec5 with SMTP id gs20-20020a1709072d1400b007848e6eaec5mr22811460ejc.477.1664951258428; Tue, 04 Oct 2022 23:27:38 -0700 (PDT) Received: from [192.168.123.94] (ip-084-118-157-002.um23.pools.vodafone-ip.de. [84.118.157.2]) by smtp.gmail.com with ESMTPSA id d7-20020a170906304700b0073d9630cbafsm7987808ejd.126.2022.10.04.23.27.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 04 Oct 2022 23:27:38 -0700 (PDT) Message-ID: Date: Wed, 5 Oct 2022 08:27:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.3.1 Subject: Re: [PATCH 1/1] cmd: simplify command efidebug Content-Language: en-US To: AKASHI Takahiro References: <20221004134032.128142-1-heinrich.schuchardt@canonical.com> <20221005002330.GA34350@laputa> Cc: Ilias Apalodimas , u-boot@lists.denx.de From: Heinrich Schuchardt In-Reply-To: <20221005002330.GA34350@laputa> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.39 Precedence: list List-Id: U-Boot discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: u-boot-bounces@lists.denx.de Sender: "U-Boot" X-Virus-Scanned: clamav-milter 0.103.6 at phobos.denx.de X-Virus-Status: Clean On 10/5/22 02:23, AKASHI Takahiro wrote: > Heinrich, > > On Tue, Oct 04, 2022 at 03:40:32PM +0200, Heinrich Schuchardt wrote: >> Currently we have subcommands 'efidebug dh' which shows protocols per >> handle and 'efidebug devices' which shows the device path. None shows which >> U-Boot device matches the handle. > > If you know how a device path display format is constructed, > you can identify it. For instance, > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ -> efi_root > /VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,00) > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > -> host 0 > /HD(1,GPT,7e5c17c5-3f5f-49d0-ae96-511b21d7f273,0x800,0x3f7df) > ^ -> partition 1 > >> Change 'efidebug dh' to show the device path and the U-Boot device if any >> is associated with the handle. > > There is a good reason for two separate subcommands. While "dh" shows > all the handles which currently exist on the system, "devices" shows > only ones with device path protocol. > The number of handles can possibly be huge (try "dh" on EDK2). We only have to care about the low number of EFI handles in U-Boot which is block devices + 5. > For those who are interested in what devices are connected at some point, > "devices" subcommand is still useful. Having to look up information in two separate commands instead of one place is inconvenient. The output of efidebug devices does not contain any information which will not be shown by modified efidebug dh command. So the devices command becomes superfluous. > > The problem is not "devices" subcommand itself, but the display format > of device path. UEFI spec says, in "10.6 EFI Device Path Display Format Overview", > "This section describes the recommended conversion between an EFI Device > ^^^^^^^^^^^ > Path Protocol and text." > > So if there is more readable/understandable format, we may want to use it > for U-Boot UEFI (at least, as an alternate option of format). > Say, > /VenHw(root)/VenHw(host, 0)/HD(1,GPT,....) > I believe that it is simple and unambiguous. This might be implemented subject to the DisplayOnly argument of EFI_DEVICE_PATH_TO_TEXT_PROTOCOL.ConvertDevicePathToText() in a separate patch. It would require adding the GUIDs to list_guid[] in lib/uuid.c and switching between '%pUl' and '%pUs' in efi_convert_device_node_to_text. > >> Remove 'efidebug devices'. > > So please don't remove "devices". > Instead, I recommend that the same change be applied to this subcommand, > displaying both formats at "devices". There is no benefit in having two sub-commands showing the same information. Instead we should reduce the code size. Best regards Heinrich > > -Takahiro Akashi > > >> Old output of 'efidebug dh': >> >> Handle Protocols >> ================ ==================== >> 000000001b22e690 Device Path, Block IO >> 000000001b22e800 Device Path, Block IO, system, Simple File System >> >> New output of 'efidebug dh': >> >> 000000001b22e690 (host0) >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,00) >> Block IO >> >> 000000001b22e800 (host0:1) >> /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/VenHw(bbe4e671-5773-4ea1-9aab-3a7dbf40c482,00)/HD(1,GPT,7e5c17c5-3f5f-49d0-ae96-511b21d7f273,0x800,0x3f7df) >> Block IO >> system >> Simple File System >> >> Signed-off-by: Heinrich Schuchardt >> --- >> cmd/efidebug.c | 102 ++++++++----------------------------------------- >> 1 file changed, 15 insertions(+), 87 deletions(-) >> >> diff --git a/cmd/efidebug.c b/cmd/efidebug.c >> index 84e6ff5565..76a6b4d8eb 100644 >> --- a/cmd/efidebug.c >> +++ b/cmd/efidebug.c >> @@ -8,6 +8,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -344,79 +345,11 @@ static int do_efi_capsule(struct cmd_tbl *cmdtp, int flag, >> } >> #endif /* CONFIG_EFI_HAVE_CAPSULE_SUPPORT */ >> >> -/** >> - * efi_get_device_path_text() - get device path text >> - * >> - * Return the text representation of the device path of a handle. >> - * >> - * @handle: handle of UEFI device >> - * Return: >> - * Pointer to the device path text or NULL. >> - * The caller is responsible for calling FreePool(). >> - */ >> -static u16 *efi_get_device_path_text(efi_handle_t handle) >> -{ >> - struct efi_handler *handler; >> - efi_status_t ret; >> - >> - ret = efi_search_protocol(handle, &efi_guid_device_path, &handler); >> - if (ret == EFI_SUCCESS && handler->protocol_interface) { >> - struct efi_device_path *dp = handler->protocol_interface; >> - >> - return efi_dp_str(dp); >> - } else { >> - return NULL; >> - } >> -} >> - >> #define EFI_HANDLE_WIDTH ((int)sizeof(efi_handle_t) * 2) >> >> static const char spc[] = " "; >> static const char sep[] = "================"; >> >> -/** >> - * do_efi_show_devices() - show UEFI devices >> - * >> - * @cmdtp: Command table >> - * @flag: Command flag >> - * @argc: Number of arguments >> - * @argv: Argument array >> - * Return: CMD_RET_SUCCESS on success, CMD_RET_RET_FAILURE on failure >> - * >> - * Implement efidebug "devices" sub-command. >> - * Show all UEFI devices and their information. >> - */ >> -static int do_efi_show_devices(struct cmd_tbl *cmdtp, int flag, >> - int argc, char *const argv[]) >> -{ >> - efi_handle_t *handles; >> - efi_uintn_t num, i; >> - u16 *dev_path_text; >> - efi_status_t ret; >> - >> - ret = EFI_CALL(efi_locate_handle_buffer(ALL_HANDLES, NULL, NULL, >> - &num, &handles)); >> - if (ret != EFI_SUCCESS) >> - 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++) { >> - dev_path_text = efi_get_device_path_text(handles[i]); >> - if (dev_path_text) { >> - printf("%p %ls\n", handles[i], dev_path_text); >> - efi_free_pool(dev_path_text); >> - } >> - } >> - >> - efi_free_pool(handles); >> - >> - return CMD_RET_SUCCESS; >> -} >> - >> /** >> * efi_get_driver_handle_info() - get information of UEFI driver >> * >> @@ -535,26 +468,25 @@ static int do_efi_show_handles(struct cmd_tbl *cmdtp, int flag, >> if (!num) >> return CMD_RET_SUCCESS; >> >> - printf("Handle%.*s Protocols\n", EFI_HANDLE_WIDTH - 6, spc); >> - printf("%.*s ====================\n", EFI_HANDLE_WIDTH, sep); >> for (i = 0; i < num; i++) { >> - printf("%p", handles[i]); >> + struct efi_handler *handler; >> + >> + printf("\n%p", handles[i]); >> + if (handles[i]->dev) >> + printf(" (%s)", handles[i]->dev->name); >> + printf("\n"); >> + /* Print device path */ >> + ret = efi_search_protocol(handles[i], &efi_guid_device_path, >> + &handler); >> + if (ret == EFI_SUCCESS) >> + printf(" %pD\n", handler->protocol_interface); >> ret = EFI_CALL(BS->protocols_per_handle(handles[i], &guid, >> &count)); >> - if (ret || !count) { >> - putc('\n'); >> - continue; >> - } >> - >> + /* Print other protocols */ >> for (j = 0; j < count; j++) { >> - if (j) >> - printf(", "); >> - else >> - putc(' '); >> - >> - printf("%pUs", guid[j]); >> + if (guidcmp(guid[j], &efi_guid_device_path)) >> + printf(" %pUs\n", guid[j]); >> } >> - putc('\n'); >> } >> >> efi_free_pool(handles); >> @@ -1539,8 +1471,6 @@ static struct cmd_tbl cmd_efidebug_sub[] = { >> U_BOOT_CMD_MKENT(capsule, CONFIG_SYS_MAXARGS, 1, do_efi_capsule, >> "", ""), >> #endif >> - U_BOOT_CMD_MKENT(devices, CONFIG_SYS_MAXARGS, 1, do_efi_show_devices, >> - "", ""), >> U_BOOT_CMD_MKENT(drivers, CONFIG_SYS_MAXARGS, 1, do_efi_show_drivers, >> "", ""), >> U_BOOT_CMD_MKENT(dh, CONFIG_SYS_MAXARGS, 1, do_efi_show_handles, >> @@ -1630,8 +1560,6 @@ static char efidebug_help_text[] = >> #endif >> "\n" >> #endif >> - "efidebug devices\n" >> - " - show UEFI devices\n" >> "efidebug drivers\n" >> " - show UEFI drivers\n" >> "efidebug dh\n" >> -- >> 2.37.2 >>