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 C3DC9C433F5 for ; Sat, 4 Dec 2021 18:44:18 +0000 (UTC) Received: from h2850616.stratoserver.net (localhost [IPv6:::1]) by phobos.denx.de (Postfix) with ESMTP id B2F8782F9D; Sat, 4 Dec 2021 19:44:15 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=fail (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=u-boot-bounces@lists.denx.de Authentication-Results: phobos.denx.de; dkim=pass (1024-bit key; secure) header.d=gmx.net header.i=@gmx.net header.b="KKjijC6F"; dkim-atps=neutral Received: by phobos.denx.de (Postfix, from userid 109) id CCE8E82FAB; Sat, 4 Dec 2021 19:44:13 +0100 (CET) Received: from mout.gmx.net (mout.gmx.net [212.227.17.22]) (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 6D314811B3 for ; Sat, 4 Dec 2021 19:44:08 +0100 (CET) Authentication-Results: phobos.denx.de; dmarc=pass (p=none dis=none) header.from=gmx.de Authentication-Results: phobos.denx.de; spf=pass smtp.mailfrom=xypron.glpk@gmx.de DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1638643443; bh=3pmfFsDiaXL876+ZksA7Cbo4CTEQNTtJL1peIK79FKc=; h=X-UI-Sender-Class:Date:Subject:To:Cc:References:From:In-Reply-To; b=KKjijC6FUC01edGhuwIbf8YcgA17d9iMhlxx22/ugkyayoS2YbaJKdOKZaSW7X+6K Cuqt7llrhKuDHms9YEg9Zv/AbWB8JBrk02COu8Z8rFItKcA1yEVmDaAeCo1ARKEDu7 o+4ApPQFoK9SZQD27rkVYhhJP7fWGZBaNmQQ5XLw= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from [192.168.123.35] ([88.152.144.157]) by mail.gmx.net (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MS3il-1mzOcD37pD-00TUCc; Sat, 04 Dec 2021 19:44:02 +0100 Message-ID: Date: Sat, 4 Dec 2021 19:43:57 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.3.1 Subject: Re: [PATCH v5 04/28] efi: Locate all block devices in the app Content-Language: en-US To: Simon Glass , U-Boot Mailing List Cc: Ilias Apalodimas , Christian Melki , Bin Meng , Tom Rini , Alexander Graf References: <20211204155657.2913911-1-sjg@chromium.org> <20211204085647.v5.4.I0774d3540ebe726f31838b851815e829fb6a4056@changeid> From: Heinrich Schuchardt In-Reply-To: <20211204085647.v5.4.I0774d3540ebe726f31838b851815e829fb6a4056@changeid> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable X-Provags-ID: V03:K1:+H+858Njlh8whEvbUcCldpVP2zVcqtaJtRpeDnCB2dKZN9Za0sr 3IDJCrhnhzjB8b6uwGGq9eXs7bwFUqMED4YunikZyShCnyuRSUPYS+OxW1bsqSGVFDMBfQ5 +k1S+r+MBDGPCSx18KwCUPr/kbzUvXOfzWUhMblzxrBpjKdSdAJF/6cXE1pu+DD5saEMRmE tRG3ciiGbj88GaCb1gF2A== X-UI-Out-Filterresults: notjunk:1;V03:K0:NNC9TuwmDjQ=:+UUJL8pOKDmidSIpqpjD8U hpGwLHlPxcJr4dePhcF9W6olTSbLsTfBqX3FSuM3hRM1sKJ0EkONhscMgSqGQUIfuqCNYlHJD f2ycmVMe3bzo4Rez8DI+fkgZijVPeXEouiG+53+56Qddo/dnNt3bMZuKh382HieDCbf0HghiL IPLHEUYIVJpkr2L9Syr+duU4PHkWn+U67VynKdr3Px2+E4IfL05hDWCRtghc0jyl03zVhkm+q W9CA1QJh7uhDpZ0Dh1GYeRizVYhKPbKGxNkMd8hCHJRzkJXTFfkQY4L0HisL9FZ3c/+DNmggC umfT3wnXyDnS5NHpdHiEsh/AI64rk1uveCYmSsMUPSgnAFnnaJ9ikAsY8ZWZBuL7Y0cns/Set KBS1vKwdJAm8qj5arVVN32A6+FQalxRya374IPZX2wMMTzvJCj9h0azGNWxwd99w/jyZX58TJ TKaj01OXjHSq9bzMlGz45pOFNdNl+gWgYcb6FQh/6NzrKpDgn2MAZSYVXTzKVE+uTV3TSWlbn zUebVW7uAJDRHJyxqdQOFDC83VABdY/teHKg87S7MgZb8hj5kNz6VVIOWFrQDjLH5xcTSeaM/ vTcbnT6Pl8qB+GslDkxADjAuUKoy41PzRTXsPg1+L2WjWvNswcXeYPXAqIthQTAyqQ7/0m3Qm 2YXINCrBfHlstY91FlLglJ32GrQGR92lrqhsNVrKXFI+Mg9j5UkmXO+/7hBkD3x2gUG+Cd9JB byF4vgJ91cKTFIrbD/1UklZ4Yzs0qnLWgvm9r2thOTXcjimp51i6sRwQ+5vkigtIDl7poXW8l tjicyLYSoW2SpEWj3JIE9bf9fPRxIP/196+dfhxaeshKd7/9U1lrnYPlBfS+O1VlRk3wyIa7t cDqwyOUqmBo3wKDc0izS0Hhju8lpts6Klo1CK2cUrSoHeBeGGTo0EcHgB4UFkZgJoGqRy6RDW zKMhTXsXqKhDzywuJWvDLR0PblRLoc7iaBzGzt1+w+YxE8LefwWy4Iv4maYHjd5OQETTAI1GZ wRRBk/G9YEBGo2gsbM22V/QxbQKTCLiIvsNPBofKYECEr/dvGQGOGXEXziWDWLCADsDsc0KVF 3nU+Y3QRIqMINI= X-BeenThere: u-boot@lists.denx.de X-Mailman-Version: 2.1.38 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.2 at phobos.denx.de X-Virus-Status: Clean On 12/4/21 16:56, Simon Glass wrote: > When starting the app, locate all block devices and make them available > to U-Boot. This allows listing partitions and accessing files in > filesystems. > > EFI also has the concept of 'disks', meaning boot media. For now, this > is not obviously useful in U-Boot, but add code to at least locate these= . > This can be expanded later as needed. > > Series-changes; 2 > - Store device path in struct efi_media_plat > - Don't export efi_bind_block() > - Only bind devices for media devices, not for partitions > - Show devices that are processed > - Update documentation > > Signed-off-by: Simon Glass > --- > > (no changes since v1) > > doc/develop/uefi/u-boot_on_efi.rst | 4 +- > include/efi.h | 6 +- > include/efi_api.h | 15 ++ > lib/efi/efi_app.c | 223 +++++++++++++++++++++++++++++ > 4 files changed, 243 insertions(+), 5 deletions(-) > > diff --git a/doc/develop/uefi/u-boot_on_efi.rst b/doc/develop/uefi/u-boo= t_on_efi.rst > index 5f2f850f071..8f81b799072 100644 > --- a/doc/develop/uefi/u-boot_on_efi.rst > +++ b/doc/develop/uefi/u-boot_on_efi.rst > @@ -265,9 +265,7 @@ This work could be extended in a number of ways: > > - Figure out how to solve the interrupt problem > > -- Add more drivers to the application side (e.g. block devices, USB, > - environment access). This would mostly be an academic exercise as a s= trong > - use case is not readily apparent, but it might be fun. > +- Add more drivers to the application side (e.g.USB, environment access= ). > > - Avoid turning off boot services in the stub. Instead allow U-Boot to= make > use of boot services in case it wants to. It is unclear what it migh= t want > diff --git a/include/efi.h b/include/efi.h > index 0ec5913ddd1..908c5dc6ebd 100644 > --- a/include/efi.h > +++ b/include/efi.h > @@ -419,10 +419,12 @@ struct efi_priv { > * > * @handle: handle of the controller on which this driver is installed > * @blkio: block io protocol proxied by this driver > + * @device_path: EFI path to the device > */ > struct efi_media_plat { > - efi_handle_t handle; > - struct efi_block_io *blkio; > + efi_handle_t handle; > + struct efi_block_io *blkio; > + struct efi_device_path *device_path; > }; > > /* Base address of the EFI image */ > diff --git a/include/efi_api.h b/include/efi_api.h > index 80109f012bc..ec9fa89a935 100644 > --- a/include/efi_api.h > +++ b/include/efi_api.h > @@ -2035,4 +2035,19 @@ struct efi_firmware_management_protocol { > const u16 *package_version_name); > }; > > +#define EFI_DISK_IO_PROTOCOL_GUID \ > + EFI_GUID(0xce345171, 0xba0b, 0x11d2, 0x8e, 0x4f, \ > + 0x00, 0xa0, 0xc9, 0x69, 0x72, 0x3b) > + > +struct efi_disk { > + u64 revision; > + efi_status_t (EFIAPI *read_disk)(struct efi_disk *this, u32 media_id, > + u64 offset, efi_uintn_t buffer_size, > + void *buffer); > + > + efi_status_t (EFIAPI *write_disk)(struct efi_disk *this, u32 media_id, > + u64 offset, efi_uintn_t buffer_size, > + void *buffer); > +}; > + > #endif > diff --git a/lib/efi/efi_app.c b/lib/efi/efi_app.c > index f61665686c5..e38d46b15db 100644 > --- a/lib/efi/efi_app.c > +++ b/lib/efi/efi_app.c > @@ -21,6 +21,9 @@ > #include > #include > #include > +#include > +#include > +#include > > DECLARE_GLOBAL_DATA_PTR; > > @@ -46,6 +49,64 @@ int efi_info_get(enum efi_entry_t type, void **datap,= int *sizep) > return -ENOSYS; > } > > +/** > + * efi_print_str() - Print a UFT-16 string to the U-Boot console > + * > + * @str: String to print > + */ > +static void efi_print_str(const u16 *str) > +{ > + while (*str) { > + int ch =3D *str++; > + > + if (ch > ' ' && ch < 127) > + putc(ch); > + } > +} > + > +/** > + * efi_bind_block() - bind a new block device to an EFI device > + * > + * Binds a new top-level EFI_MEDIA device as well as a child block devi= ce so > + * that the block device can be accessed in U-Boot. > + * > + * The device can then be accessed using 'part list efi 0', 'fat ls efi= 0:1', > + * for example, just like any other interface type. > + * > + * @handle: handle of the controller on which this driver is installed > + * @blkio: block io protocol proxied by this driver > + * @device_path: EFI device path structure for this > + * @len: Length of @device_path in bytes > + * @devp: Returns the bound device > + * @return 0 if OK, -ve on error > + */ > +int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio, > + struct efi_device_path *device_path, int len, > + struct udevice **devp) > +{ > + struct efi_media_plat plat; > + struct udevice *dev; > + char name[18]; > + int ret; > + > + plat.handle =3D handle; > + plat.blkio =3D blkio; > + plat.device_path =3D malloc(device_path->length); > + if (!plat.device_path) > + return log_msg_ret("path", -ENOMEM); > + memcpy(plat.device_path, device_path, device_path->length); > + ret =3D device_bind(dm_root(), DM_DRIVER_GET(efi_media), "efi_media", > + &plat, ofnode_null(), &dev); > + if (ret) > + return log_msg_ret("bind", ret); > + > + snprintf(name, sizeof(name), "efi_media_%x", dev_seq(dev)); > + device_set_name(dev, name); > + *devp =3D dev; > + > + return 0; > +} > + > static efi_status_t setup_memory(struct efi_priv *priv) > { > struct efi_boot_services *boot =3D priv->boot; > @@ -105,6 +166,168 @@ static void free_memory(struct efi_priv *priv) > global_data_ptr =3D NULL; > } > Missing function description > +static int setup_disks(void) > +{ > + /* This is not fully implemented yet */ > + return 0; > + > + efi_guid_t efi_disk_guid =3D EFI_DISK_IO_PROTOCOL_GUID; > + struct efi_boot_services *boot =3D efi_get_boot(); > + struct efi_disk *disk; > + int ret; > + > + if (!boot) > + return log_msg_ret("sys", -ENOSYS); > + ret =3D boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk); > + if (ret) > + return log_msg_ret("prot", -ENOTSUPP); > + > + return 0; > +} > + > +/** > + * devpath_is_partition() - Figure out if a device path is a partition > + * > + * Checks if a device path refers to a partition on some media device. = This > + * works by checking for a valid partition number in a hard-driver medi= a device > + * as the final component of the device path. > + * > + * @return true if a partition, false if not (e.g. it might be media wh= ich > + * contains partitions) Please, stick to the format for function descriptions defined in https://www.kernel.org/doc/html/latest/doc-guide/kernel-doc.html#function-= documentation The description for parameter @path: is missing. @return does not exist. Please, use Return:. > + */ > +static bool devpath_is_partition(const struct efi_device_path *path) > +{ > + const struct efi_device_path *p; > + bool was_part; > + > + for (p =3D path; p->type !=3D DEVICE_PATH_TYPE_END; > + p =3D (void *)p + p->length) { > + was_part =3D false; > + if (p->type =3D=3D DEVICE_PATH_TYPE_MEDIA_DEVICE && > + p->sub_type =3D=3D DEVICE_PATH_SUB_TYPE_HARD_DRIVE_PATH) { > + struct efi_device_path_hard_drive_path *hd =3D > + (void *)path; > + > + if (hd->partition_number) > + was_part =3D true; > + } > + } > + > + return was_part; > +} > + > +/** > + * setup_block() - Find all block devices and setup EFI devices for the= m > + * > + * Partitions are ignored, since U-Boot has partition handling. Errors = with > + * particular devices produce a warning but execution continues to try = to > + * find others. > + * > + * @return 0 if found, -ENOSYS if there is no boot-services table, -ENO= TSUPP > + * if a required protocol is not supported %s/@return/Return:/ > + */ > +static int setup_block(void) > +{ > + efi_guid_t efi_blkio_guid =3D EFI_BLOCK_IO_PROTOCOL_GUID; > + efi_guid_t efi_devpath_guid =3D EFI_DEVICE_PATH_PROTOCOL_GUID; > + efi_guid_t efi_pathutil_guid =3D EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GU= ID; > + efi_guid_t efi_pathtext_guid =3D EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID= ; > + struct efi_boot_services *boot =3D efi_get_boot(); > + struct efi_device_path_utilities_protocol *util; > + struct efi_device_path_to_text_protocol *text; > + struct efi_device_path *path; > + struct efi_block_io *blkio; > + efi_uintn_t num_handles; > + efi_handle_t *handle; > + int ret, i; > + > + if (!boot) > + return log_msg_ret("sys", -ENOSYS); > + > + /* Find all devices which support the block I/O protocol */ > + ret =3D boot->locate_handle_buffer(BY_PROTOCOL, &efi_blkio_guid, NULL, > + &num_handles, &handle); > + if (ret) > + return log_msg_ret("loc", -ENOTSUPP); > + log_debug("Found %d handles:\n", (int)num_handles); > + > + /* We need to look up the path size and convert it to text */ > + ret =3D boot->locate_protocol(&efi_pathutil_guid, NULL, (void **)&util= ); > + if (ret) > + return log_msg_ret("util", -ENOTSUPP); > + ret =3D boot->locate_protocol(&efi_pathtext_guid, NULL, (void **)&text= ); > + if (ret) > + return log_msg_ret("text", -ENOTSUPP); > + > + for (i =3D 0; i < num_handles; i++) { > + struct udevice *dev; > + const u16 *name; > + bool is_part; > + int len; > + > + ret =3D boot->handle_protocol(handle[i], &efi_devpath_guid, > + (void **)&path); > + if (ret) { > + log_warning("- devpath %d failed (ret=3D%d)\n", i, ret); > + continue; > + } > + > + ret =3D boot->handle_protocol(handle[i], &efi_blkio_guid, > + (void **)&blkio); > + if (ret) { > + log_warning("- blkio %d failed (ret=3D%d)\n", i, ret); > + continue; > + } > + > + name =3D text->convert_device_path_to_text(path, true, false); > + is_part =3D devpath_is_partition(path); > + > + if (!is_part) { > + len =3D util->get_device_path_size(path); > + ret =3D efi_bind_block(handle[i], blkio, path, len, &dev); > + if (ret) { > + log_warning("- blkio bind %d failed (ret=3D%d)\n", > + i, ret); > + continue; > + } > + } else { > + dev =3D NULL; > + } > + > + /* > + * Show the device name if we created one. Otherwise indicate > + * that it is a partition. > + * > + * We don't seem to have have a way to print unicode on the > + * U-Boot console at present, so use our own function. > + */ > + printf("%2d: %-12s ", i, dev ? dev->name : ""); > + efi_print_str(name); > + printf("\n"); > + } > + boot->free_pool(handle); > + > + return 0; > +} > + Missing function description. Best regards Heinrich > +int dm_scan_other(bool pre_reloc_only) > +{ > + if (gd->flags & GD_FLG_RELOC) { > + int ret; > + > + ret =3D setup_block(); > + if (ret) > + return ret; > + > + /* Not needed at present, but could be useful one day? */ > + ret =3D setup_disks(); > + if (ret) > + return ret; > + } > + > + return 0; > +} > + > /** > * efi_main() - Start an EFI image > * >