From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Wed, 9 Jan 2019 10:05:33 +0900 Subject: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time In-Reply-To: <20181213075827.GN21466@linaro.org> References: <20181115045810.28736-1-takahiro.akashi@linaro.org> <20181115045810.28736-3-takahiro.akashi@linaro.org> <20181213075827.GN21466@linaro.org> Message-ID: <20190109010532.GN9033@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Heinrich, Do you have any further comments here? Otherwise, I'd like to send out v3 shortly. -Takahiro Akashi On Thu, Dec 13, 2018 at 04:58:29PM +0900, AKASHI Takahiro wrote: > 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 > > > > 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; > > > +} > > > > >