[U-Boot] [PATCH v2 3/3] efi_loader: remove block device details from efi file

[Alexander Graf]

On 15.11.18 05:58, AKASHI Takahiro wrote:
> Logically, details on u-boot block device used to implement efi file
> protocol are mostly unnecessary, as well as being duplicated, in
> efi_file structure.
> Moreover, a newly introduced flag, _EFI_DISK_FLAG_INVALID, should be
> honored in any file operations via efi file protocol.
> These observation suggests that those internal details be set behind
> efi_disk object.
> 
> So rework in this patch addresses all those issues above although
> there is little change in its functionality.
> 
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  include/efi_loader.h      |  6 +++++-
>  lib/efi_loader/efi_disk.c | 38 ++++++++++++++++++++++++++++++++++++--
>  lib/efi_loader/efi_file.c | 21 ++++++++-------------
>  3 files changed, 49 insertions(+), 16 deletions(-)
> 
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 3bae1844befb..108ef3fe52ee 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -264,6 +264,10 @@ efi_status_t efi_disk_register(void);
>  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);
> +/* Called by file protocol to set internal block io device */
> +int efi_disk_set_blk_dev(efi_handle_t disk);
> +/* Called by file protocol to get disk/partition information */
> +int efi_disk_get_info(efi_handle_t disk, disk_partition_t *part);
>  /* 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,
> @@ -355,7 +359,7 @@ void efi_signal_event(struct efi_event *event, bool check_tpl);
>  
>  /* open file system: */
>  struct efi_simple_file_system_protocol *efi_simple_file_system(
> -		struct blk_desc *desc, int part, struct efi_device_path *dp);
> +		efi_handle_t disk);
>  
>  /* open file from device-path: */
>  struct efi_file_handle *efi_file_from_path(struct efi_device_path *fp);
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index 0c4d79ee3fc9..180e8e10bb28 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -9,6 +9,7 @@
>  #include <blk.h>
>  #include <dm.h>
>  #include <efi_loader.h>
> +#include <fs.h>
>  #include <part.h>
>  #include <malloc.h>
>  
> @@ -254,6 +255,40 @@ efi_fs_from_path(struct efi_device_path *full_path)
>  	return handler->protocol_interface;
>  }
>  
> +/*
> + * Set block device for later block io's from file system protocol
> + *
> + * @disk	handle to uefi disk device
> + * @return	0 for success, -1 for failure
> + */
> +int efi_disk_set_blk_dev(efi_handle_t disk)
> +{
> +	struct efi_disk_obj *diskobj;
> +
> +	diskobj = container_of(disk, struct efi_disk_obj, header);
> +
> +	return fs_set_blk_dev_with_part(diskobj->desc, diskobj->part);
> +}
> +
> +/*
> + * Get disk/partition information
> + *
> + * @disk	handle to uefi disk device
> + * @part	pointer to disk/partition information to be returned
> + * @return	0 for success, -1 for failure
> + */
> +int efi_disk_get_info(efi_handle_t disk, disk_partition_t *part)
> +{
> +	struct efi_disk_obj *diskobj;
> +
> +	diskobj = container_of(disk, struct efi_disk_obj, header);
> +
> +	if (diskobj->part >= 1)
> +		return part_get_info(diskobj->desc, diskobj->part, part);
> +	else
> +		return part_get_info_whole_disk(diskobj->desc, part);
> +}
> +
>  /*
>   * Create a handle for a partition or disk
>   *
> @@ -308,8 +343,7 @@ static efi_status_t efi_disk_add_dev(
>  	if (ret != EFI_SUCCESS)
>  		return ret;
>  	if (part >= 1) {
> -		diskobj->volume = efi_simple_file_system(desc, part,
> -							 diskobj->dp);
> +		diskobj->volume = efi_simple_file_system(&diskobj->header);
>  		ret = efi_add_protocol(&diskobj->header,
>  				       &efi_simple_file_system_protocol_guid,
>  				       diskobj->volume);
> diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
> index beb4fba917d8..944383224f30 100644
> --- a/lib/efi_loader/efi_file.c
> +++ b/lib/efi_loader/efi_file.c
> @@ -17,9 +17,7 @@ const efi_guid_t efi_file_system_info_guid = EFI_FILE_SYSTEM_INFO_GUID;
>  
>  struct file_system {
>  	struct efi_simple_file_system_protocol base;
> -	struct efi_device_path *dp;
> -	struct blk_desc *desc;
> -	int part;
> +	efi_handle_t disk;

Is there a particular reason we can't just make this a struct
efi_disk_obj *?

Inside our own code base, we don't need to abstract things away to
handles, right? We also want to have the compiler catch the fact early
if people pass in non-disk-objects in as parameter.


Alex