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

[Heinrich Schuchardt]

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

Just enumerated handles with a loaded image protocol.

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

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