[PATCH 14/35] efi: Locate all block devices in the app
Simon Glass
sjg at chromium.org
Thu Sep 9 22:09:12 CEST 2021
Hi Takahiro,
On Wed, 8 Sept 2021 at 19:11, AKASHI Takahiro
<takahiro.akashi at linaro.org> wrote:
>
> Hi Simon,
>
> On Wed, Sep 08, 2021 at 08:14:30PM +0200, Heinrich Schuchardt wrote:
> > On 9/8/21 3:33 PM, 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.
> >
> > UEFI firmware handles with the EFI_BLOCK_IO_PROTOCOL for raw access to
> > disks and partitions. It further provides the
> > EFI_SIMPLE_FILE_SYSTEM_PROTOCOL on these handles to access files on
> > formatted media.
>
> Do you want to implement "efifs" as a U-Boot's file system on top of
> SIMPLE_FILE_SYSTEM_PROTO?
> Just kidding.
Eek.
>
> I have one concern:
>
> > >
> > > Signed-off-by: Simon Glass <sjg at chromium.org>
> > > ---
> > >
> > > include/efi.h | 15 ++++++
> > > include/efi_api.h | 15 ++++++
> > > lib/efi/efi_app.c | 119 ++++++++++++++++++++++++++++++++++++++++++++++
> > > 3 files changed, 149 insertions(+)
> > >
> > > diff --git a/include/efi.h b/include/efi.h
> > > index 0ec5913ddd1..c0fddf7f6cd 100644
> > > --- a/include/efi.h
> > > +++ b/include/efi.h
> > > @@ -529,4 +529,19 @@ void efi_putc(struct efi_priv *priv, const char ch);
> > > */
> > > int efi_info_get(enum efi_entry_t type, void **datap, int *sizep);
> > >
> > > +/**
> > > + * 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 device 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
> > > + * @return 0 if OK, -ve on error
> > > + */
> > > +int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio);
> > > +
> > > #endif /* _LINUX_EFI_H */
> > > diff --git a/include/efi_api.h b/include/efi_api.h
> > > index c8f959bb720..0e88b3e5dbe 100644
> > > --- a/include/efi_api.h
> > > +++ b/include/efi_api.h
> > > @@ -1994,4 +1994,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..9ba48517422 100644
> > > --- a/lib/efi/efi_app.c
> > > +++ b/lib/efi/efi_app.c
> > > @@ -21,6 +21,9 @@
> > > #include <efi.h>
> > > #include <efi_api.h>
> > > #include <sysreset.h>
> > > +#include <dm/device-internal.h>
> > > +#include <dm/lists.h>
> > > +#include <dm/root.h>
> > >
> > > DECLARE_GLOBAL_DATA_PTR;
> > >
> > > @@ -46,6 +49,33 @@ int efi_info_get(enum efi_entry_t type, void **datap, int *sizep)
> > > return -ENOSYS;
> > > }
> > >
> > > +/**
> > > + * Create a block device so U-Boot can access an EFI device
> > > + *
> > > + * @handle: EFI handle to bind
> > > + * @blkio: block io protocol
> > > + * Return: 0 = success
> > > + */
> > > +int efi_bind_block(efi_handle_t handle, struct efi_block_io *blkio)
> > > +{
> > > + struct efi_media_plat plat;
> > > + struct udevice *dev;
> > > + char name[18];
> > > + int ret;
> > > +
> > > + plat.handle = handle;
> > > + plat.blkio = blkio;
> > > + ret = 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);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > static efi_status_t setup_memory(struct efi_priv *priv)
> > > {
> > > struct efi_boot_services *boot = priv->boot;
> > > @@ -105,6 +135,95 @@ static void free_memory(struct efi_priv *priv)
> > > global_data_ptr = NULL;
> > > }
> > >
> > > +static int setup_disks(void)
> > > +{
> > > + /* This is not fully implemented yet */
> > > + return 0;
> > > +
> > > + efi_guid_t efi_disk_guid = EFI_DISK_IO_PROTOCOL_GUID;
> > > + struct efi_boot_services *boot = efi_get_boot();
> > > + struct efi_disk *disk;
> > > + int ret;
> > > +
> > > + if (!boot)
> > > + return log_msg_ret("sys", -ENOSYS);
> > > + ret = boot->locate_protocol(&efi_disk_guid, NULL, (void **)&disk);
> > > + if (ret)
> > > + return log_msg_ret("prot", -ENOTSUPP);
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +static int setup_block(void)
> > > +{
> > > + efi_guid_t efi_blkio_guid = EFI_BLOCK_IO_PROTOCOL_GUID;
> > > + efi_guid_t efi_devpath_guid = EFI_DEVICE_PATH_PROTOCOL_GUID;
> > > + struct efi_boot_services *boot = efi_get_boot();
> > > + struct efi_block_io *blkio;
> > > + struct efi_device_path device_path;
> > > + efi_handle_t handle[100];
> > > + efi_uintn_t buf_size;
> > > + int num_handles;
> > > + int ret, i;
> > > +
> > > + if (!boot)
> > > + return log_msg_ret("sys", -ENOSYS);
> > > +
> > > + buf_size = sizeof(handle);
> > > + ret = boot->locate_handle(BY_PROTOCOL, &efi_blkio_guid, NULL,
> > > + &buf_size, handle);
> >
> > You could use LocateHandleBuffer() here which will allocate enough
> > memory for all matching handles.
> >
> > > + if (ret)
> > > + return log_msg_ret("loc", -ENOTSUPP);
> > > +
> > > + num_handles = buf_size / sizeof(efi_handle_t);
> > > + log_info("Found %d EFI handles\n", num_handles);
> > > +
> > > + for (i = 0; i < num_handles; i++) {
> > > + ret = boot->handle_protocol(handle[i], &efi_devpath_guid,
> > > + (void **)&device_path);
> >
> >
> > Why do you read the devicepath if you don't use it?
> >
> > > + if (ret) {
> > > + log_warning("- devpath %d failed (ret=%d)\n", i, ret);
> > > + continue;
> > > + }
> >
> > Here some analysis of devicepaths and installed protocols is missing to
> > find out which of the handles represents a block device and which
> > represents a partition:
> >
> > If the last devicepath node is type 4 , Media Device Path with SubType 1
> > Hard Drive and the partition number is non-zero it is a partition.
> >
> > If the devicepath without the last node relates to a handle with the
> > EFI_BLOCK_IO_PROTOCOL, this also indicates that the current handle is
> > for a partition.
> >
> > Best regards
> >
> > Heinrich
> >
> > > +
> > > + ret = boot->handle_protocol(handle[i], &efi_blkio_guid,
> > > + (void **)&blkio);
> > > + if (ret) {
> > > + log_warning("- blkio %d failed (ret=%d)\n", i, ret);
> > > + continue;
> > > + }
> > > +
> > > + ret = efi_bind_block(handle[i], blkio);
>
> Here you are trying to create a U-Boot block device for
> every UEFI block device (i.e.BLOCK_IO_PROTOCOL interface),
> but please remember that U-Boot UEFI has created UEFI block devices
> for all the existing U-Boot block devices at the initialization time.
Do you mean with EFI_LOADER? This code is for U-Boot as an app, where
EFI_LOADER is disabled, at least at present.
>
> So any physical disk may end up having one (original) U-Boot block device
> and another U-Boot block device rooted in UEFI object (and
> yet another U-Boot block device rooted in the second one and so on?).
Hopefully that is not an issue, for now. We will need to figure it out
later, if we enable EFI_LOADER.
Regards,
Simon
[..]
More information about the U-Boot
mailing list