Re: [U-Boot] [PATCH v2 2/3] efi_loader: enumerate disk devices every time

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;

+}