On 14 January 2016 at 22:06, Alexander Graf <ag...@suse.de> wrote: > A EFI applications usually want to access storage devices to load data from. > > This patch adds support for EFI disk interfaces. It loops through all block > storage interfaces known to U-Boot and creates an EFI object for each existing > one. EFI applications can then through these objects call U-Boot's read and > write functions. > > Signed-off-by: Alexander Graf <ag...@suse.de> > Reviewed-by: Simon Glass <s...@chromium.org> > > --- > > v1 -> v2: > > - Move to block_drvr array > - Move to GPLv2+ > - Fix header order > - Document efi block object struct > - Use calloc rather than malloc & memset > --- > include/efi_loader.h | 1 + > lib/efi_loader/efi_disk.c | 218 > ++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 219 insertions(+) > create mode 100644 lib/efi_loader/efi_disk.c > > diff --git a/include/efi_loader.h b/include/efi_loader.h > index 0f821ff..9d6c322 100644 > --- a/include/efi_loader.h > +++ b/include/efi_loader.h > @@ -63,6 +63,7 @@ struct efi_object { > }; > extern struct list_head efi_obj_list; > > +int efi_disk_register(void); > efi_status_t efi_return_handle(void *handle, > efi_guid_t *protocol, void **protocol_interface, > void *agent_handle, void *controller_handle, > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c > new file mode 100644 > index 0000000..c589604 > --- /dev/null > +++ b/lib/efi_loader/efi_disk.c > @@ -0,0 +1,218 @@ > +/* > + * EFI application disk support > + * > + * Copyright (c) 2016 Alexander Graf > + * > + * SPDX-License-Identifier: GPL-2.0+ > + */ > + > +#include <common.h> > +#include <efi_loader.h> > +#include <inttypes.h> > +#include <part.h> > +#include <malloc.h> > + > +static const efi_guid_t efi_block_io_guid = BLOCK_IO_GUID; > + > +struct efi_disk_obj { > + /* Generic EFI object parent class data */ > + struct efi_object parent; > + /* EFI Interface callback struct for block I/O */ > + struct efi_block_io ops; > + /* U-Boot ifname for block device */ > + const char *ifname; > + /* U-Boot dev_index for block device */ > + int dev_index; > + /* EFI Interface Media descriptor struct, referenced by ops */ > + struct efi_block_io_media media; > + /* EFI device path to this block device */ > + struct efi_device_path_file_path *dp; > +}; > + > +static void ascii2unicode(u16 *unicode, char *ascii) > +{ > + while (*ascii) > + *(unicode++) = *(ascii++); > +} > + > +static efi_status_t efi_disk_open_block(void *handle, efi_guid_t *protocol, > + void **protocol_interface, void *agent_handle, > + void *controller_handle, uint32_t attributes) > +{ > + struct efi_disk_obj *diskobj = handle; > + > + *protocol_interface = &diskobj->ops; > + > + return EFI_SUCCESS; > +} > + > +static efi_status_t efi_disk_open_dp(void *handle, efi_guid_t *protocol, > + void **protocol_interface, void *agent_handle, > + void *controller_handle, uint32_t attributes) > +{ > + struct efi_disk_obj *diskobj = handle; > + > + *protocol_interface = diskobj->dp; > + > + return EFI_SUCCESS; > +} > + > +static efi_status_t efi_disk_reset(struct efi_block_io *this, > + char extended_verification) > +{ > + EFI_ENTRY("%p, %x", this, extended_verification); > + return EFI_EXIT(EFI_DEVICE_ERROR); > +} > + > +enum efi_disk_direction { > + EFI_DISK_READ, > + EFI_DISK_WRITE, > +}; > + > +static efi_status_t efi_disk_rw_blocks(struct efi_block_io *this, > + u32 media_id, u64 lba, unsigned long buffer_size, > + void *buffer, enum efi_disk_direction direction) > +{ > + struct efi_disk_obj *diskobj; > + struct block_dev_desc *desc; > + int blksz; > + int blocks; > + unsigned long n; > + > + EFI_ENTRY("%p, %x, %"PRIx64", %lx, %p", this, media_id, lba, > + buffer_size, buffer); > + > + diskobj = container_of(this, struct efi_disk_obj, ops); > + if (!(desc = get_dev(diskobj->ifname, diskobj->dev_index))) > + return EFI_EXIT(EFI_DEVICE_ERROR); > + blksz = desc->blksz; > + blocks = buffer_size / blksz; > + > +#ifdef DEBUG_EFI > + printf("EFI: %s:%d blocks=%x lba=%"PRIx64" blksz=%x dir=%d\n", > __func__, > + __LINE__, blocks, lba, blksz, direction); > +#endif > + > + /* We only support full block access */ > + if (buffer_size & (blksz - 1)) > + return EFI_EXIT(EFI_DEVICE_ERROR); > + > + if (direction == EFI_DISK_READ) > + n = desc->block_read(desc->dev, lba, blocks, buffer);
Shouldn't this be s/desc->dev/desc/? > + else > + n = desc->block_write(desc->dev, lba, blocks, buffer); Same here. > + > + /* We don't do interrupts, so check for timers cooperatively */ > + efi_timer_check(); > + > +#ifdef DEBUG_EFI > + printf("EFI: %s:%d n=%lx blocks=%x\n", __func__, __LINE__, n, blocks); > +#endif > + if (n != blocks) > + return EFI_EXIT(EFI_DEVICE_ERROR); > + > + return EFI_EXIT(EFI_SUCCESS); > +} > + > +static efi_status_t efi_disk_read_blocks(struct efi_block_io *this, > + u32 media_id, u64 lba, unsigned long buffer_size, > + void *buffer) > +{ > + return efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer, > + EFI_DISK_READ); > +} > + > +static efi_status_t efi_disk_write_blocks(struct efi_block_io *this, > + u32 media_id, u64 lba, unsigned long buffer_size, > + void *buffer) > +{ > + return efi_disk_rw_blocks(this, media_id, lba, buffer_size, buffer, > + EFI_DISK_WRITE); > +} > + > +static efi_status_t efi_disk_flush_blocks(struct efi_block_io *this) > +{ > + /* We always write synchronously */ > + return EFI_SUCCESS; > +} > + > +static const struct efi_block_io block_io_disk_template = { > + .reset = &efi_disk_reset, > + .read_blocks = &efi_disk_read_blocks, > + .write_blocks = &efi_disk_write_blocks, > + .flush_blocks = &efi_disk_flush_blocks, > +}; > + > +/* > + * U-Boot doesn't have a list of all online disk devices. So when running our > + * EFI payload, we scan through all of the potentially available ones and > + * store them in our object pool. This isn't quite true - you are actually using its list below. You could modify this to work without the table I suspect. But the way you have it seems good to me. > + * > + * This gets called from do_bootefi_exec(). > + */ > +int efi_disk_register(void) > +{ > + const struct block_drvr *cur_drvr; I think it would be better to use a function call to return the driver. Then we can implement it with driver model later. How about: block_dev_first(&cur_drvr) block_dev_next(&cur_drvr) > + int i; > + int disks = 0; > + > + /* Search for all available disk devices */ > + for (cur_drvr = block_drvr; cur_drvr->name; cur_drvr++) { > + printf("Scanning disks on %s...\n", cur_drvr->name); > + for (i = 0; i < 4; i++) { Please add a comment about the 4, or an enum/#define. > + block_dev_desc_t *desc; > + struct efi_disk_obj *diskobj; > + struct efi_device_path_file_path *dp; > + int objlen = sizeof(*diskobj) + (sizeof(*dp) * 2); > + char devname[16]; > + > + desc = get_dev(cur_drvr->name, i); > + if (!desc) > + continue; It's not clear how this checks that the device is actually valid. I think if desc->type == DEV_TYPE_UNKNOWN then you should ignore the device. For example an MMC card slot might be empty. > + > + diskobj = calloc(1, objlen); > + > + /* Fill in object data */ drop extra blank line, and below > + > + diskobj->parent.protocols[0].guid = > &efi_block_io_guid; > + diskobj->parent.protocols[0].open = > efi_disk_open_block; > + diskobj->parent.protocols[1].guid = > &efi_guid_device_path; > + diskobj->parent.protocols[1].open = efi_disk_open_dp; > + diskobj->parent.handle = diskobj; > + diskobj->ops = block_io_disk_template; > + diskobj->ifname = cur_drvr->name; > + diskobj->dev_index = i; > + > + /* Fill in EFI IO Media info (for read/write > callbacks) */ > + > + diskobj->media.removable_media = desc->removable; > + diskobj->media.media_present = 1; > + diskobj->media.block_size = desc->blksz; > + diskobj->media.io_align = desc->blksz; > + diskobj->media.last_block = desc->lba; > + diskobj->ops.media = &diskobj->media; > + > + /* Fill in device path */ > + > + dp = (void*)&diskobj[1]; > + diskobj->dp = dp; > + dp[0].dp.type = DEVICE_PATH_TYPE_MEDIA_DEVICE; > + dp[0].dp.sub_type = DEVICE_PATH_SUB_TYPE_FILE_PATH; > + dp[0].dp.length = sizeof(*dp); > + sprintf(devname, "%s%d", cur_drvr->name, i); I'd suggest snprintf(devname, ARRAY_SIZE(dp[0].str), ...) or perhaps a check. > + ascii2unicode(dp[0].str, devname); > + > + dp[1].dp.type = DEVICE_PATH_TYPE_END; > + dp[1].dp.sub_type = DEVICE_PATH_SUB_TYPE_END; > + dp[1].dp.length = sizeof(*dp); > + > + /* Hook up to the device list */ > + > + list_add_tail(&diskobj->parent.link, &efi_obj_list); > + disks++; > + } > + } > + printf("Found %d disks\n", disks); > + > + return 0; > +} > -- > 2.1.4 > Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot