On Thu, Jan 10, 2019 at 07:22:49AM +0100, Alexander Graf wrote:
On 10.01.19 01:37, AKASHI Takahiro wrote:
Alex,
On Wed, Jan 09, 2019 at 10:18:16AM +0100, Alexander Graf wrote:
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@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);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 *?
Just because efi_disk_obj is an internally-defined structure in efi_disk.c.
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.
If you don't mind efi_disk_obj definition being moved to, say, efi_loader.h, I would follow your suggestion.
I don't think we need to move the definition, just the hint that the name exists.
If you add the following to efi_loader.h:
struct efi_disk_obj;
that should be enough to enable other subsystems (and APIs) to use pointers to that struct.
Ah, right. What we need to have in struct file_system is a *pointer*. So such a declaration is good enough.
Thanks, -Takahiro Akashi
Alex