| Message ID | 20190208081542.2813-3-takahiro.akashi@linaro.org |
|---|---|
| State | RFC |
| Delegated to: | Alexander Graf |
| Headers | show |
| Series | dm, efi: integrate efi objects into DM | expand |
On 2/8/19 9:15 AM, AKASHI Takahiro wrote: > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> > --- > 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. 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, >
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 <takahiro.akashi@linaro.org> > > --- > > 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. -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, > > >
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 <takahiro.akashi@linaro.org> >>> --- >>> 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. So why should be do it for a kernel image loaded via LoadImage()? 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, >>> >> >
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 <takahiro.akashi@linaro.org> > >>> --- > >>> 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, > >>> > >> > > >
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); + 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,
Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org> --- include/efi_loader.h | 2 +- lib/efi_loader/efi_boottime.c | 61 +++++++++++++++++++---------------- 2 files changed, 35 insertions(+), 28 deletions(-)