On Tue, May 07, 2019 at 08:05:48AM +0200, Heinrich Schuchardt wrote:
On 5/7/19 7:58 AM, Takahiro Akashi wrote:
On Tue, May 07, 2019 at 07:53:41AM +0200, Heinrich Schuchardt wrote:
On 5/7/19 7:44 AM, Takahiro Akashi wrote:
On Tue, May 07, 2019 at 07:26:46AM +0200, Heinrich Schuchardt wrote:
On 5/7/19 5:02 AM, Takahiro Akashi wrote:
On Sat, May 04, 2019 at 10:36:33AM +0200, Heinrich Schuchardt wrote: >In UnloadImage() we need to know if an image is already started. > >Add a field to the handle structure identifying loaded and started images. > >Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de >--- > include/efi_loader.h | 13 +++++++++++++ > lib/efi_loader/efi_boottime.c | 2 ++ > 2 files changed, 15 insertions(+) > >diff --git a/include/efi_loader.h b/include/efi_loader.h >index 3fd9901d66..c2a449e5b6 100644 >--- a/include/efi_loader.h >+++ b/include/efi_loader.h >@@ -182,6 +182,18 @@ struct efi_handler { > struct list_head open_infos; > }; > >+/** >+ * enum efi_object_type - type of EFI object >+ * >+ * In UnloadImage we must be able to identify if the handle relates to a >+ * started image. >+ */ >+enum efi_object_type { >+ EFI_OBJECT_TYPE_UNDEFINED = 0, >+ EFI_OBJECT_TYPE_LOADED_IMAGE, >+ EFI_OBJECT_TYPE_STARTED_IMAGE, >+};
It sounds *status*, not *type*. In a separate patch, you added U_BOOT_FIRMWARE. We should distinguish status and type for future enhancement and to avoid any confusion.
Having both information in the same field makes the evaluation easier.
Currently I see not necessity for more handle types to be differentiated. Let's change it when the need arises.
I don't think we need U_BOOT_FIRMWARE if we use STARTED_IMAGE for root node. Then we can rename efi_object_type to efi_image_status.
This would allow calling UnloadImage() for the root node.
Who knows the address of root node?
Just enumerated handles with a loaded image protocol.
Remove it when enumerating handles if necessary.
For safe guard, you can add if (handle == root_node) return EFI_INVALID_PARAMETER; at the beginning of unload_image.
This would not decrease complexity.
I believe that it's much better and more understandable than confusing use of status and type.
-Takahiro Akashi
Regards
Heinrich
-Takahiro Akashi
Best regards
Heinrich
-Takahiro Akashi
Best regards
Heinrich
Thanks, -Takahiro Akashi
> /** > * struct efi_object - dereferenced EFI handle > * >@@ -204,6 +216,7 @@ struct efi_object { > struct list_head link; > /* The list of protocols */ > struct list_head protocols; >+ enum efi_object_type type; > }; > > /** >diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c >index 3ed08e7c37..dc444fccf6 100644 >--- a/lib/efi_loader/efi_boottime.c >+++ b/lib/efi_loader/efi_boottime.c >@@ -1554,6 +1554,7 @@ efi_status_t efi_setup_loaded_image(struct efi_device_path *device_path, > free(info); > return EFI_OUT_OF_RESOURCES; > } >+ obj->header.type = EFI_OBJECT_TYPE_LOADED_IMAGE; > > /* Add internal object to object list */ > efi_add_handle(&obj->header); >@@ -2678,6 +2679,7 @@ efi_status_t EFIAPI efi_start_image(efi_handle_t image_handle, > } > > current_image = image_handle; >+ image_obj->header.type = EFI_OBJECT_TYPE_STARTED_IMAGE; > EFI_PRINT("Jumping into 0x%p\n", image_obj->entry); > ret = EFI_CALL(image_obj->entry(image_handle, &systab)); > >-- >2.20.1 >