From: AKASHI Takahiro <takahiro.akashi@linaro.org>
To: u-boot@lists.denx.de
Subject: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time
Date: Thu, 13 Dec 2018 16:58:29 +0900 [thread overview]
Message-ID: <20181213075827.GN21466@linaro.org> (raw)
In-Reply-To: <f5f94213-5d7b-2b52-9440-e4ba4bbbbd3b@gmx.de>
Heinrich,
On Tue, Dec 11, 2018 at 08:55:41PM +0100, Heinrich Schuchardt wrote:
> On 11/15/18 5:58 AM, AKASHI Takahiro wrote:
> > Currently, efi_init_obj_list() scan disk devices only once, and never
> > change a list of efi disk devices. This will possibly result in failing
> > to find a removable storage which may be added later on. See [1].
> >
> > In this patch, called is efi_disk_update() which is responsible for
> > re-scanning UCLASS_BLK devices and removing/adding efi disks if necessary.
> >
> > For example,
> >
> > => efishell devices
> > Scanning disk pci_mmc.blk...
> > Found 3 disks
> > Device Name
> > ============================================
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > => usb start
> > starting USB...
> > USB0: USB EHCI 1.00
> > scanning bus 0 for devices... 3 USB Device(s) found
> > scanning usb for storage devices... 1 Storage Device(s) found
> > => efishell devices
> > Scanning disk usb_mass_storage.lun0...
> > Device Name
> > ============================================
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/SD(0)/SD(0)/HD(2,MBR,0x086246ba,0x40800,0x3f800)
> > /VenHw(e61d73b9-a384-4acc-aeab-82e828f3628b)/USBClass(0,0,9,0,1)/USBClass(46f4,1,0,0,0)/HD(1,0x01,0,0x40,0x14fe4c)
> >
> > Without this patch, the last device, USB mass storage, won't show up.
> >
> > [1] https://lists.denx.de/pipermail/u-boot/2018-October/345307.html
> >
> > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
>
> Why should we try to fix something in the EFI subsystems that goes wrong
> in the handling of device enumeration.
No.
This is a natural result from how efi disks are currently implemented on u-boot.
Do you want to totally re-write/re-implement efi disks?
Furthermore, your comment here doesn't match your previous comment[1].
[1] https://lists.denx.de/pipermail/u-boot/2018-November/346860.html
-Takahiro Akashi
> @Marek
> Why should a 'usb start' command be needed to make a plugged in device
> available?
>
> Best regards
>
> Heirnich
>
>
>
> > ---
> > cmd/bootefi.c | 17 +++-
> > include/efi_loader.h | 4 +
> > lib/efi_loader/efi_disk.c | 191 ++++++++++++++++++++++++++++++++++++++
> > 3 files changed, 210 insertions(+), 2 deletions(-)
> >
> > diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> > index 3cefb4d0ecaa..82649e211fda 100644
> > --- a/cmd/bootefi.c
> > +++ b/cmd/bootefi.c
> > @@ -56,9 +56,22 @@ efi_status_t efi_init_obj_list(void)
> > */
> > efi_save_gd();
> >
> > - /* Initialize once only */
> > - if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED)
> > + if (efi_obj_list_initialized == EFI_SUCCESS) {
> > +#ifdef CONFIG_PARTITIONS
> > + ret = efi_disk_update();
> > + if (ret != EFI_SUCCESS)
> > + printf("+++ updating disks list failed\n");
> > +
> > + /*
> > + * It may sound odd, but most part of EFI should
> > + * yet work.
> > + */
> > +#endif
> > return efi_obj_list_initialized;
> > + } else if (efi_obj_list_initialized != OBJ_LIST_NOT_INITIALIZED) {
> > + /* Initialize once only */
> > + return efi_obj_list_initialized;
> > + }
> >
> > /* Initialize system table */
> > ret = efi_initialize_system_table();
> > diff --git a/include/efi_loader.h b/include/efi_loader.h
> > index 5cc3bded03fa..3bae1844befb 100644
> > --- a/include/efi_loader.h
> > +++ b/include/efi_loader.h
> > @@ -260,6 +260,10 @@ efi_status_t efi_initialize_system_table(void);
> > efi_status_t efi_console_register(void);
> > /* Called by bootefi to make all disk storage accessible as EFI objects */
> > efi_status_t efi_disk_register(void);
> > +/* Check validity of efi disk */
> > +bool efi_disk_is_valid(efi_handle_t handle);
> > +/* Called by bootefi to find and update disk storage information */
> > +efi_status_t efi_disk_update(void);
> > /* Create handles and protocols for the partitions of a block device */
> > int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
> > const char *if_typename, int diskid,
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index c037526ad2d0..0c4d79ee3fc9 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -14,10 +14,14 @@
> >
> > const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
> >
> > +#define _EFI_DISK_FLAG_DELETED 0x1 /* to be removed */
> > +#define _EFI_DISK_FLAG_INVALID 0x80000000 /* in stale state */
> > +
> > /**
> > * struct efi_disk_obj - EFI disk object
> > *
> > * @header: EFI object header
> > + * @flags: control flags
> > * @ops: EFI disk I/O protocol interface
> > * @ifname: interface name for block device
> > * @dev_index: device index of block device
> > @@ -30,6 +34,7 @@ const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
> > */
> > struct efi_disk_obj {
> > struct efi_object header;
> > + int flags;
> > struct efi_block_io ops;
> > const char *ifname;
> > int dev_index;
> > @@ -41,6 +46,35 @@ struct efi_disk_obj {
> > struct blk_desc *desc;
> > };
> >
> > +static void efi_disk_mark_deleted(struct efi_disk_obj *disk)
> > +{
> > + disk->flags |= _EFI_DISK_FLAG_DELETED;
> > +}
> > +
> > +static void efi_disk_clear_deleted(struct efi_disk_obj *disk)
> > +{
> > + disk->flags &= ~_EFI_DISK_FLAG_DELETED;
> > +}
> > +
> > +static bool efi_disk_deleted_marked(struct efi_disk_obj *disk)
> > +{
> > + return disk->flags & _EFI_DISK_FLAG_DELETED;
> > +}
> > +
> > +static void efi_disk_mark_invalid(struct efi_disk_obj *disk)
> > +{
> > + disk->flags |= _EFI_DISK_FLAG_INVALID;
> > +}
> > +
> > +bool efi_disk_is_valid(efi_handle_t handle)
> > +{
> > + struct efi_disk_obj *disk;
> > +
> > + disk = container_of(handle, struct efi_disk_obj, header);
> > +
> > + return !(disk->flags & _EFI_DISK_FLAG_INVALID);
> > +}
> > +
> > static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this,
> > char extended_verification)
> > {
> > @@ -64,6 +98,9 @@ static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this,
> > unsigned long n;
> >
> > diskobj = container_of(this, struct efi_disk_obj, ops);
> > + if (diskobj->flags & _EFI_DISK_FLAG_INVALID)
> > + return EFI_DEVICE_ERROR;
> > +
> > desc = (struct blk_desc *) diskobj->desc;
> > blksz = desc->blksz;
> > blocks = buffer_size / blksz;
> > @@ -440,3 +477,157 @@ efi_status_t efi_disk_register(void)
> >
> > return EFI_SUCCESS;
> > }
> > +
> > +/*
> > + * Mark all the block devices as "deleted," and return an array of
> > + * handles for later use. It should be freed in a caller.
> > + */
> > +static efi_status_t efi_disk_mark_deleted_all(efi_handle_t **handlesp)
> > +{
> > + efi_handle_t *handles = NULL;
> > + efi_uintn_t size = 0;
> > + int num, i;
> > + struct efi_disk_obj *disk;
> > + efi_status_t ret;
> > +
> > + ret = efi_locate_handle(BY_PROTOCOL, &efi_block_io_guid, NULL,
> > + &size, handles);
> > + if (ret == EFI_BUFFER_TOO_SMALL) {
> > + handles = calloc(1, size);
> > + if (!handles)
> > + return EFI_OUT_OF_RESOURCES;
> > +
> > + ret = efi_locate_handle(BY_PROTOCOL, &efi_block_io_guid, NULL,
> > + &size, handles);
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + free(handles);
> > + return ret;
> > + }
> > +
> > + num = size / sizeof(*handles);
> > + for (i = 0; i < num; i++) {
> > + disk = container_of(handles[i], struct efi_disk_obj, header);
> > + efi_disk_mark_deleted(disk);
> > + }
> > +
> > + *handlesp = handles;
> > +
> > + return num;
> > +}
> > +
> > +/*
> > + * Clear "deleted" flag for a block device which is identified with desc.
> > + * If desc is NULL, clear all devices.
> > + *
> > + * Return a number of disks cleared.
> > + */
> > +static int efi_disk_clear_deleted_matched(struct blk_desc *desc,
> > + efi_handle_t *handles, int num)
> > +{
> > + struct efi_disk_obj *disk;
> > + int disks, i;
> > +
> > + for (i = 0, disks = 0; i < num; i++) {
> > + disk = container_of(handles[i], struct efi_disk_obj, header);
> > +
> > + if (!desc || disk->desc == desc) {
> > + efi_disk_clear_deleted(disk);
> > + disks++;
> > + }
> > + }
> > +
> > + return disks;
> > +}
> > +
> > +/*
> > + * Do delete all the block devices marked as "deleted"
> > + */
> > +static efi_status_t efi_disk_do_delete(efi_handle_t *handles, int num)
> > +{
> > + struct efi_disk_obj *disk;
> > + int i;
> > +
> > + for (i = 0; i < num; i++) {
> > + disk = container_of(handles[i], struct efi_disk_obj, header);
> > +
> > + if (!efi_disk_deleted_marked(disk))
> > + continue;
> > +
> > + efi_disk_mark_invalid(disk);
> > + /*
> > + * TODO:
> > + * efi_delete_handle(handles[i]);
> > + */
> > + }
> > +
> > + return EFI_SUCCESS;
> > +}
> > +
> > +/*
> > + * efi_disk_update - recreate efi disk mappings after initialization
> > + *
> > + * @return efi error code
> > + */
> > +efi_status_t efi_disk_update(void)
> > +{
> > +#ifdef CONFIG_BLK
> > + efi_handle_t *handles = NULL;
> > + struct udevice *dev;
> > + struct blk_desc *desc;
> > + const char *if_typename;
> > + struct efi_disk_obj *disk;
> > + int n, disks = 0;
> > + efi_status_t ret;
> > +
> > + /* temporarily mark all the devices "deleted" */
> > + ret = efi_disk_mark_deleted_all(&handles);
> > + if (ret & EFI_ERROR_MASK) {
> > + printf("ERROR: Failed to rescan block devices.\n");
> > + return ret;
> > + }
> > +
> > + n = (int)ret;
> > + for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
> > + uclass_next_device_check(&dev)) {
> > + desc = dev_get_uclass_platdata(dev);
> > + if (n && efi_disk_clear_deleted_matched(desc, handles, n))
> > + /* existing device */
> > + continue;
> > +
> > + /* new device */
> > + if_typename = blk_get_if_type_name(desc->if_type);
> > +
> > + /* Add block device for the full device */
> > + printf("Scanning disk %s...\n", dev->name);
> > + ret = efi_disk_add_dev(NULL, NULL, if_typename,
> > + desc, desc->devnum, 0, 0, &disk);
> > + if (ret == EFI_NOT_READY) {
> > + printf("Disk %s not ready\n", dev->name);
> > + continue;
> > + }
> > + if (ret != EFI_SUCCESS) {
> > + printf("ERROR: failure to add disk device %s, r = %lu\n",
> > + dev->name, ret & ~EFI_ERROR_MASK);
> > + continue;
> > + }
> > + disks++;
> > +
> > + /* Partitions show up as block devices in EFI */
> > + disks += efi_disk_create_partitions(&disk->header, desc,
> > + if_typename,
> > + desc->devnum, dev->name);
> > + }
> > +
> > + if (n) {
> > + /* do delete "deleted" disks */
> > + ret = efi_disk_do_delete(handles, n);
> > +
> > + /* undo marking */
> > + efi_disk_clear_deleted_matched(NULL, handles, n);
> > +
> > + free(handles);
> > + }
> > +#endif
> > + return EFI_SUCCESS;
> > +}
> >
>
next prev parent reply other threads:[~2018-12-13 7:58 UTC|newest]
Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-11-15 4:58 [U-Boot] [PATCH v2 0/3] efi_loader: add removable device support AKASHI Takahiro
2018-11-15 4:58 ` [U-Boot] [PATCH v2 1/3] efi_loader: export efi_locate_handle() function AKASHI Takahiro
2018-11-15 4:58 ` [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time AKASHI Takahiro
2018-12-11 19:55 ` Heinrich Schuchardt
2018-12-13 7:58 ` AKASHI Takahiro [this message]
2019-01-09 1:05 ` AKASHI Takahiro
2019-01-09 9:06 ` Alexander Graf
2019-01-10 2:13 ` AKASHI Takahiro
2019-01-10 6:21 ` Alexander Graf
2019-01-10 7:26 ` AKASHI Takahiro
2019-01-10 7:30 ` Alexander Graf
2019-01-10 8:02 ` AKASHI Takahiro
2019-01-10 8:15 ` Alexander Graf
2019-01-10 9:16 ` AKASHI Takahiro
2019-01-10 9:22 ` Alexander Graf
2019-01-10 19:22 ` Heinrich Schuchardt
2019-01-11 5:08 ` AKASHI Takahiro
2019-01-11 4:29 ` AKASHI Takahiro
2019-01-11 7:57 ` Alexander Graf
2019-01-12 21:32 ` Simon Glass
2019-01-12 22:00 ` Alexander Graf
2019-01-16 21:34 ` Simon Glass
2019-01-22 8:29 ` AKASHI Takahiro
2019-01-22 9:08 ` Alexander Graf
2019-01-22 19:39 ` Simon Glass
2019-01-22 21:04 ` Heinrich Schuchardt
2019-01-23 8:06 ` AKASHI Takahiro
2019-01-23 21:58 ` Simon Glass
2019-01-24 0:53 ` AKASHI Takahiro
2019-01-24 20:18 ` Simon Glass
2019-01-24 21:19 ` Heinrich Schuchardt
2019-01-25 2:27 ` AKASHI Takahiro
2019-01-23 9:51 ` Alexander Graf
2019-01-23 22:01 ` Simon Glass
2019-01-25 8:27 ` AKASHI Takahiro
2019-01-25 8:52 ` Alexander Graf
2019-01-25 9:18 ` AKASHI Takahiro
2019-01-25 9:31 ` Alexander Graf
2019-01-28 8:56 ` AKASHI Takahiro
2019-01-28 9:36 ` Alexander Graf
2019-01-29 0:46 ` Simon Glass
2019-01-29 1:22 ` AKASHI Takahiro
2019-01-23 8:12 ` AKASHI Takahiro
2019-01-23 9:30 ` Alexander Graf
2019-01-10 12:57 ` Simon Glass
2019-01-11 4:51 ` AKASHI Takahiro
2019-01-11 8:00 ` Alexander Graf
2019-01-11 13:03 ` Mark Kettenis
2018-11-15 4:58 ` [U-Boot] [PATCH v2 3/3] efi_loader: remove block device details from efi file AKASHI Takahiro
2019-01-09 9:18 ` Alexander Graf
2019-01-10 0:37 ` AKASHI Takahiro
2019-01-10 6:22 ` Alexander Graf
2019-01-10 6:36 ` AKASHI Takahiro
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20181213075827.GN21466@linaro.org \
--to=takahiro.akashi@linaro.org \
--cc=u-boot@lists.denx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.