[U-Boot] [PATCH 1/4] efi_loader: mark started images

[Takahiro Akashi]

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 at 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?
For safe guard, you can add
    if (handle == root_node)
        return EFI_INVALID_PARAMETER;
at the beginning of unload_image.

-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
> >>>>
> >>>
> >>
> >
>