From mboxrd@z Thu Jan 1 00:00:00 1970 From: AKASHI Takahiro Date: Tue, 12 Feb 2019 16:35:50 +0900 Subject: [U-Boot] [RFC v2 02/15] efi_loader: boottime: convert efi_loaded_image_obj to DM In-Reply-To: <76c6bc9d-58c5-9b9e-3ab2-ac7b47d910f8@gmx.de> References: <20190208081542.2813-1-takahiro.akashi@linaro.org> <20190208081542.2813-3-takahiro.akashi@linaro.org> <20190212050741.GI20286@linaro.org> <76c6bc9d-58c5-9b9e-3ab2-ac7b47d910f8@gmx.de> Message-ID: <20190212073548.GL20286@linaro.org> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de On Tue, Feb 12, 2019 at 07:47:06AM +0100, Heinrich Schuchardt wrote: > On 2/12/19 6:07 AM, AKASHI Takahiro wrote: > > Heinrich, > > > > On Fri, Feb 08, 2019 at 06:53:00PM +0100, Heinrich Schuchardt wrote: > >> On 2/8/19 9:15 AM, AKASHI Takahiro wrote: > >>> Signed-off-by: AKASHI Takahiro > >>> --- > >>> include/efi_loader.h | 2 +- > >>> lib/efi_loader/efi_boottime.c | 61 +++++++++++++++++++---------------- > >>> 2 files changed, 35 insertions(+), 28 deletions(-) > >>> > >>> diff --git a/include/efi_loader.h b/include/efi_loader.h > >>> index 4d5e22564a72..5882cd7dd3b0 100644 > >>> --- a/include/efi_loader.h > >>> +++ b/include/efi_loader.h > >>> @@ -367,7 +367,7 @@ efi_status_t efi_install_configuration_table(const efi_guid_t *guid, void *table > >>> /* Sets up a loaded image */ > >>> efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > >>> struct efi_device_path *file_path, > >>> - struct efi_loaded_image_obj **handle_ptr, > >>> + efi_handle_t *handle_ptr, > >>> struct efi_loaded_image **info_ptr); > >>> efi_status_t efi_load_image_from_path(struct efi_device_path *file_path, > >>> void **buffer); > >>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c > >>> index d23e4fbbdf23..624156d4578b 100644 > >>> --- a/lib/efi_loader/efi_boottime.c > >>> +++ b/lib/efi_loader/efi_boottime.c > >>> @@ -1690,29 +1690,29 @@ static efi_status_t EFIAPI efi_install_configuration_table_ext(efi_guid_t *guid, > >>> */ > >>> efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > >>> struct efi_device_path *file_path, > >>> - struct efi_loaded_image_obj **handle_ptr, > >>> + efi_handle_t *handle_ptr, > >>> struct efi_loaded_image **info_ptr) > >>> { > >>> efi_status_t ret; > >>> struct efi_loaded_image *info; > >>> - struct efi_loaded_image_obj *obj; > >>> + struct udevice *dev; > >>> > >>> info = calloc(1, sizeof(*info)); > >>> if (!info) > >>> return EFI_OUT_OF_RESOURCES; > >>> - obj = calloc(1, sizeof(*obj)); > >>> - if (!obj) { > >>> + > >>> + ret = device_bind_driver(dm_root(), "efi_loaded_image", "(IMG)", &dev); > >> > >> A loaded image is not a device. There is nothing in the EFI world > >> relating the loaded image directly to the root of the device tree. > > > > Please notice that any loaded image is linked to "efi_root", > > not "root of the device tree." > > > > Such a relationship is an analogy to the current implementation > > where *global* protocols are added to "efi_root" for now. > > Yes we are lacking the necessary handles to show that the USB keyboard > in connected to a USB controller installed on a PCI bus. > > But what has this to do with loaded images? When we call the U-Boot > `load` command to load a kernel image we do not update the device model. That's right. > So why should be do it for a kernel image loaded via LoadImage()? The difference here is that multiple instances of loaded image, either application or driver, can co-exist at the same time in EFI world. So distinguishing those by handles may make some sense. -Takahiro Akashi > A kernel image isn't a device. So let's keep it separate from the device > mmodel. > > Best regards > > Heinrich > > > > > -Takahiro Akashi > > > >> We should not create a meaningless link here. > >> > >> Best regards > >> > >> Heinrich > >> > >> > >>> + if (ret) { > >>> free(info); > >>> return EFI_OUT_OF_RESOURCES; > >>> } > >>> > >>> - /* Add internal object to object list */ > >>> - efi_add_handle(&obj->header); > >>> + efi_add_handle(dev); > >>> > >>> if (info_ptr) > >>> *info_ptr = info; > >>> if (handle_ptr) > >>> - *handle_ptr = obj; > >>> + *handle_ptr = dev; > >>> > >>> info->revision = EFI_LOADED_IMAGE_PROTOCOL_REVISION; > >>> info->file_path = file_path; > >>> @@ -1724,7 +1724,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > >>> * When asking for the device path interface, return > >>> * bootefi_device_path > >>> */ > >>> - ret = efi_add_protocol(&obj->header, > >>> + ret = efi_add_protocol(dev, > >>> &efi_guid_device_path, device_path); > >>> if (ret != EFI_SUCCESS) > >>> goto failure; > >>> @@ -1734,24 +1734,24 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > >>> * When asking for the loaded_image interface, just > >>> * return handle which points to loaded_image_info > >>> */ > >>> - ret = efi_add_protocol(&obj->header, > >>> + ret = efi_add_protocol(dev, > >>> &efi_guid_loaded_image, info); > >>> if (ret != EFI_SUCCESS) > >>> goto failure; > >>> > >>> - ret = efi_add_protocol(&obj->header, > >>> + ret = efi_add_protocol(dev, > >>> &efi_guid_hii_string_protocol, > >>> (void *)&efi_hii_string); > >>> if (ret != EFI_SUCCESS) > >>> goto failure; > >>> > >>> - ret = efi_add_protocol(&obj->header, > >>> + ret = efi_add_protocol(dev, > >>> &efi_guid_hii_database_protocol, > >>> (void *)&efi_hii_database); > >>> if (ret != EFI_SUCCESS) > >>> goto failure; > >>> > >>> - ret = efi_add_protocol(&obj->header, > >>> + ret = efi_add_protocol(dev, > >>> &efi_guid_hii_config_routing_protocol, > >>> (void *)&efi_hii_config_routing); > >>> if (ret != EFI_SUCCESS) > >>> @@ -1835,9 +1835,8 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, > >>> efi_uintn_t source_size, > >>> efi_handle_t *image_handle) > >>> { > >>> + struct efi_loaded_image_obj *obj; > >>> struct efi_loaded_image *info = NULL; > >>> - struct efi_loaded_image_obj **image_obj = > >>> - (struct efi_loaded_image_obj **)image_handle; > >>> efi_status_t ret; > >>> > >>> EFI_ENTRY("%d, %p, %pD, %p, %zd, %p", boot_policy, parent_image, > >>> @@ -1864,24 +1863,29 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy, > >>> * file parts: > >>> */ > >>> efi_dp_split_file_path(file_path, &dp, &fp); > >>> - ret = efi_setup_loaded_image(dp, fp, image_obj, &info); > >>> + ret = efi_setup_loaded_image(dp, fp, image_handle, &info); > >>> if (ret != EFI_SUCCESS) > >>> goto failure; > >>> } else { > >>> /* In this case, file_path is the "device" path, i.e. > >>> * something like a HARDWARE_DEVICE:MEMORY_MAPPED > >>> */ > >>> - ret = efi_setup_loaded_image(file_path, NULL, image_obj, &info); > >>> + ret = efi_setup_loaded_image(file_path, NULL, image_handle, > >>> + &info); > >>> if (ret != EFI_SUCCESS) > >>> goto error; > >>> } > >>> - (*image_obj)->entry = efi_load_pe(*image_obj, source_buffer, info); > >>> - if (!(*image_obj)->entry) { > >>> + > >>> + obj = (*image_handle)->platdata; > >>> + ret = efi_load_pe(obj, source_buffer, info); > >>> + if (ret != EFI_SUCCESS) { > >>> ret = EFI_UNSUPPORTED; > >>> goto failure; > >>> } > >>> + > >>> info->system_table = &systab; > >>> info->parent_handle = parent_image; > >>> + > >>> return EFI_EXIT(EFI_SUCCESS); > >>> failure: > >>> efi_delete_handle(*image_handle); > >>> @@ -1908,8 +1912,8 @@ static efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > >>> unsigned long *exit_data_size, > >>> s16 **exit_data) > >>> { > >>> - struct efi_loaded_image_obj *image_obj = > >>> - (struct efi_loaded_image_obj *)image_handle; > >>> + struct udevice *dev = image_handle; > >>> + struct efi_loaded_image_obj *image_obj = dev->platdata; > >>> efi_status_t ret; > >>> > >>> EFI_ENTRY("%p, %p, %p", image_handle, exit_data_size, exit_data); > >>> @@ -1975,12 +1979,12 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > >>> unsigned long exit_data_size, > >>> int16_t *exit_data) > >>> { > >>> + struct udevice *dev = image_handle; > >>> + struct efi_loaded_image_obj *image_obj = dev->platdata; > >>> /* > >>> * TODO: We should call the unload procedure of the loaded > >>> * image protocol. > >>> */ > >>> - struct efi_loaded_image_obj *image_obj = > >>> - (struct efi_loaded_image_obj *)image_handle; > >>> > >>> EFI_ENTRY("%p, %ld, %ld, %p", image_handle, exit_status, > >>> exit_data_size, exit_data); > >>> @@ -2013,12 +2017,9 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle, > >>> */ > >>> static efi_status_t EFIAPI efi_unload_image(efi_handle_t image_handle) > >>> { > >>> - struct efi_object *efiobj; > >>> - > >>> EFI_ENTRY("%p", image_handle); > >>> - efiobj = efi_search_obj(image_handle); > >>> - if (efiobj) > >>> - list_del(&efiobj->link); > >>> + > >>> + efi_remove_handle(image_handle); > >>> > >>> return EFI_EXIT(EFI_SUCCESS); > >>> } > >>> @@ -3398,6 +3399,12 @@ U_BOOT_DRIVER(efi_dumb_obj) = { > >>> .id = UCLASS_EFI_OBJECT, > >>> }; > >>> > >>> +U_BOOT_DRIVER(efi_image_obj) = { > >>> + .name = "efi_loaded_image", > >>> + .id = UCLASS_EFI_OBJECT, > >>> + .platdata_auto_alloc_size = sizeof(struct efi_loaded_image_obj), > >>> +}; > >>> + > >>> UCLASS_DRIVER(efi_obj) = { > >>> .name = "efi_object", > >>> .id = UCLASS_EFI_OBJECT, > >>> > >> > > >