On Wed, Jan 30, 2019 at 07:49:37AM +0100, Heinrich Schuchardt wrote:
On 1/30/19 6:48 AM, AKASHI Takahiro wrote:
On Tue, Jan 29, 2019 at 11:33:31PM +0100, Heinrich Schuchardt wrote:
On 1/29/19 3:59 AM, AKASHI Takahiro wrote:
efi_disk_create() will initialize efi_disk attributes for each device, either UCLASS_BLK or UCLASS_PARTITION.
Currently (temporarily), efi_disk_obj structure is embedded into blk_desc to hold efi-specific attributes.
Signed-off-by: AKASHI Takahiro takahiro.akashi@linaro.org
drivers/block/blk-uclass.c | 9 ++ drivers/core/device.c | 3 + include/blk.h | 24 +++++ include/dm/device.h | 4 + lib/efi_loader/efi_disk.c | 192 ++++++++++++++++++++++++++----------- 5 files changed, 174 insertions(+), 58 deletions(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index d4ca30f23fc1..28c45d724113 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -657,6 +657,9 @@ UCLASS_DRIVER(blk) = { .per_device_platdata_auto_alloc_size = sizeof(struct blk_desc), };
+/* FIXME */ +extern int efi_disk_create(struct udevice *dev);
U_BOOT_DRIVER(blk_partition) = { .name = "blk_partition", .id = UCLASS_PARTITION, @@ -695,6 +698,12 @@ int blk_create_partitions(struct udevice *parent) part_data->partnum = part; part_data->gpt_part_info = info;
ret = efi_disk_create(dev);if (ret) {device_unbind(dev);return ret;}- disks++; }
diff --git a/drivers/core/device.c b/drivers/core/device.c index 0d15e5062b66..8625fccb0dcb 100644 --- a/drivers/core/device.c +++ b/drivers/core/device.c @@ -67,6 +67,9 @@ static int device_bind_common(struct udevice *parent, const struct driver *drv, dev->parent = parent; dev->driver = drv; dev->uclass = uc; +#ifdef CONFIG_EFI_LOADER
- INIT_LIST_HEAD(&dev->efi_obj.protocols);
This looks like an incomplete initialization of efi_obj. Why don't we use efi_create_handle.
I think that it is more or less a matter of implementation. I will address this issue later if necessary.
Why should a device be aware of struct efi_obj? We only need a handle to install protocols.
+#endif
dev->seq = -1; dev->req_seq = -1; diff --git a/include/blk.h b/include/blk.h index d0c033aece0f..405f6f790d66 100644 --- a/include/blk.h +++ b/include/blk.h @@ -8,6 +8,7 @@ #define BLK_H
#include <efi.h> +#include <efi_api.h>
#ifdef CONFIG_SYS_64BIT_LBA typedef uint64_t lbaint_t; @@ -53,6 +54,26 @@ enum sig_type { SIG_TYPE_COUNT /* Number of signature types */ };
+/* FIXME */
Fix what?
For simplicity, this data structure was copied from efi_disk.c in this initial release. As the implementation goes *sophisticated*, some members may go away or move somewhere, say in blk_desc itself.
+/**
- struct efi_disk_obj - EFI disk object
- @ops: EFI disk I/O protocol interface
- @media: block I/O media information
- @dp: device path to the block device
- @part: partition
- @volume: simple file system protocol of the partition
- @offset: offset into disk for simple partition
- */
+struct efi_disk_obj {
- struct efi_block_io ops;
- struct efi_block_io_media media;
- struct efi_device_path *dp;
- unsigned int part;
- struct efi_simple_file_system_protocol *volume;
- lbaint_t offset;
+};
/*
- With driver model (CONFIG_BLK) this is uclass platform data, accessible
- with dev_get_uclass_platdata(dev)
@@ -92,6 +113,9 @@ struct blk_desc { * device. Once these functions are removed we can drop this field. */ struct udevice *bdev; +#ifdef CONFIG_EFI_LOADER
- struct efi_disk_obj efi_disk;
This must be a handle (i.e. a pointer). Otherwise when the last protocol is removed we try to free memory that was never malloc'ed.
Who actually frees?
see UEFI spec for UninstallProtocolInterface():
"If the last protocol interface is removed from a handle, the handle is freed and is no longer valid."
Got it. So, under the current implementation, any efi_object must be allocated by efi code itself and all derived efi objects have an efi_object as the first member. (We can lift this restriction by adding object-specific "free" function, like calling (handle->free)(handle) instead of free(handle) though.)
Umm. This will make it a bit difficult to remove efi_disk_obj from our code.
-Takahiro Akashi
Best regards
Heinrich
+#endif #else unsigned long (*block_read)(struct blk_desc *block_dev, lbaint_t start, diff --git a/include/dm/device.h b/include/dm/device.h index 27a6d7b9fdb0..121bfb46b1a0 100644 --- a/include/dm/device.h +++ b/include/dm/device.h @@ -12,6 +12,7 @@
#include <dm/ofnode.h> #include <dm/uclass-id.h> +#include <efi_loader.h> #include <fdtdec.h> #include <linker_lists.h> #include <linux/compat.h> @@ -146,6 +147,9 @@ struct udevice { #ifdef CONFIG_DEVRES struct list_head devres_head; #endif +#ifdef CONFIG_EFI_LOADER
- struct efi_object efi_obj;
+#endif };
/* Maximum sequence number supported */ diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c index c037526ad2d0..84fa0ddfba14 100644 --- a/lib/efi_loader/efi_disk.c +++ b/lib/efi_loader/efi_disk.c @@ -14,33 +14,6 @@
const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID;
-/**
- struct efi_disk_obj - EFI disk object
- @header: EFI object header
- @ops: EFI disk I/O protocol interface
- @ifname: interface name for block device
- @dev_index: device index of block device
- @media: block I/O media information
- @dp: device path to the block device
- @part: partition
- @volume: simple file system protocol of the partition
- @offset: offset into disk for simple partition
- @desc: internal block device descriptor
- */
-struct efi_disk_obj {
- struct efi_object header;
- struct efi_block_io ops;
- const char *ifname;
- int dev_index;
- struct efi_block_io_media media;
- struct efi_device_path *dp;
- unsigned int part;
- struct efi_simple_file_system_protocol *volume;
- lbaint_t offset;
- struct blk_desc *desc;
-};
static efi_status_t EFIAPI efi_disk_reset(struct efi_block_io *this, char extended_verification) { @@ -64,7 +37,7 @@ 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);
- desc = (struct blk_desc *) diskobj->desc;
- desc = container_of(diskobj, struct blk_desc, efi_disk); blksz = desc->blksz; blocks = buffer_size / blksz; lba += diskobj->offset;
@@ -217,6 +190,7 @@ efi_fs_from_path(struct efi_device_path *full_path) return handler->protocol_interface; }
+#ifndef CONFIG_BLK /*
- Create a handle for a partition or disk
@@ -343,6 +317,136 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc,
return disks; } +#else +static int efi_disk_create_common(efi_handle_t handle,
struct efi_disk_obj *disk,struct blk_desc *desc)+{
- efi_status_t ret;
- /* Hook up to the device list */
- efi_add_handle(handle);
- /* Fill in EFI IO Media info (for read/write callbacks) */
- disk->media.removable_media = desc->removable;
- disk->media.media_present = 1;
- disk->media.block_size = desc->blksz;
- disk->media.io_align = desc->blksz;
- disk->media.last_block = desc->lba - disk->offset;
- disk->ops.media = &disk->media;
- /* add protocols */
- disk->ops = block_io_disk_template;
- ret = efi_add_protocol(handle, &efi_block_io_guid, &disk->ops);
- if (ret != EFI_SUCCESS)
goto err;- ret = efi_add_protocol(handle, &efi_guid_device_path, disk->dp);
- if (ret != EFI_SUCCESS)
goto err;- return 0;
+err:
- /* FIXME: undo adding protocols */
- return -1;
+}
+/*
- Create a handle for a raw disk
- @dev uclass device
- @return 0 on success, -1 otherwise
- */
+int efi_disk_create_raw(struct udevice *dev) +{
- struct blk_desc *desc = dev_get_uclass_platdata(dev);
- struct efi_disk_obj *disk = &desc->efi_disk;
- /* Don't add empty devices */
- if (!desc->lba)
return -1;- /* raw block device */
- disk->offset = 0;
- disk->part = 0;
- disk->dp = efi_dp_from_part(desc, 0);
- /* efi IO media */
- disk->media.logical_partition = 0;
- return efi_disk_create_common(&dev->efi_obj, disk, desc);
+}
+/*
- Create a handle for a partition
- @dev uclass device
- @return 0 on success, -1 otherwise
- */
+int efi_disk_create_part(struct udevice *dev) +{
- struct udevice *parent = dev->parent;
- struct blk_desc *desc = dev_get_uclass_platdata(parent);
- struct blk_desc *this;
- struct disk_part *pdata = dev_get_uclass_platdata(dev);
- struct efi_disk_obj *disk;
- struct efi_device_path *node;
- int ret;
- /* dummy block device */
- this = malloc(sizeof(*this));
- if (!this)
return -1;- disk = &this->efi_disk;
- /* logical disk partition */
- disk->offset = pdata->gpt_part_info.start;
- disk->part = pdata->partnum;
- node = efi_dp_part_node(desc, disk->part);
- disk->dp = efi_dp_append_node(desc->efi_disk.dp, node);
- efi_free_pool(node);
- /* efi IO media */
- disk->media.logical_partition = 1;
- ret = efi_disk_create_common(&dev->efi_obj, disk, desc);
- if (ret)
goto err;- /* partition may support file system access */
- disk->volume = efi_simple_file_system(desc, disk->part, disk->dp);
- ret = efi_add_protocol(&dev->efi_obj,
&efi_simple_file_system_protocol_guid,disk->volume);- if (ret != EFI_SUCCESS)
goto err;- return 0;
+err:
- free(this);
- /* FIXME: undo create */
- return -1;
+}
+int efi_disk_create(struct udevice *dev) +{
- enum uclass_id id;
- id = device_get_uclass_id(dev);
- if (id == UCLASS_BLK)
return efi_disk_create_raw(dev);- else if (id == UCLASS_PARTITION)
return efi_disk_create_part(dev);- else
return -1;+} +#endif /* CONFIG_BLK */
/*
- U-Boot doesn't have a list of all online disk devices. So when running our
@@ -357,38 +461,10 @@ int efi_disk_create_partitions(efi_handle_t parent, struct blk_desc *desc, */ efi_status_t efi_disk_register(void) { +#ifndef CONFIG_BLK struct efi_disk_obj *disk; int disks = 0; efi_status_t ret; -#ifdef CONFIG_BLK
- struct udevice *dev;
- for (uclass_first_device_check(UCLASS_BLK, &dev); dev;
uclass_next_device_check(&dev)) {struct blk_desc *desc = dev_get_uclass_platdata(dev);const char *if_typename = blk_get_if_type_name(desc->if_type);/* Add block device for the full device */printf("Scanning disk %s...\n", dev->name);Who cares for this output? If really needed make it debug().
Please note that this is a line to be deleted.
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);Who minds if it is a CD-ROM drive with no disk inserted? Debug() might be adequate here.
Ditto
continue;}if (ret) {printf("ERROR: failure to add disk device %s, r = %lu\n",dev->name, ret & ~EFI_ERROR_MASK);return ret;}disks++;/* Partitions show up as block devices in EFI */disks += efi_disk_create_partitions(&disk->header, desc, if_typename,desc->devnum, dev->name);- }
-#else int i, if_type;
/* Search for all available disk devices */ @@ -435,8 +511,8 @@ efi_status_t efi_disk_register(void) if_typename, i, devname); } } -#endif printf("Found %d disks\n", disks);
I would prefer this to be debug() or eliminated.
I didn't change anything on this line and the function, efi_disk_register(), will eventually go away.
Thanks, -Takahiro Akashi
Best regards
Heinrich
+#endif
return EFI_SUCCESS; }