[01/11] efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL

From: Leif Lindholm <leif.lindholm@linaro.org>

Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
rest as needed or once we have SCT running properly so there is a way to
validate the interface against the conformance test suite.

Initial skeleton written by Leif, and then implementation by myself.

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h                          | 34 +++++++++++-
 include/efi_loader.h                       |  2 +
 lib/efi_loader/Makefile                    |  1 +
 lib/efi_loader/efi_boottime.c              |  4 ++
 lib/efi_loader/efi_device_path_utilities.c | 88 ++++++++++++++++++++++++++++++
 5 files changed, 127 insertions(+), 2 deletions(-)
 create mode 100644 lib/efi_loader/efi_device_path_utilities.c

On 10/10/2017 02:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
> rest as needed or once we have SCT running properly so there is a way to
> validate the interface against the conformance test suite.
> 
> Initial skeleton written by Leif, and then implementation by myself.
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>   include/efi_api.h                          | 34 +++++++++++-
>   include/efi_loader.h                       |  2 +
>   lib/efi_loader/Makefile                    |  1 +
>   lib/efi_loader/efi_boottime.c              |  4 ++
>   lib/efi_loader/efi_device_path_utilities.c | 88 ++++++++++++++++++++++++++++++
>   5 files changed, 127 insertions(+), 2 deletions(-)
>   create mode 100644 lib/efi_loader/efi_device_path_utilities.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index a9a6494afe..ffdba7fe1a 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -28,8 +28,9 @@ enum efi_timer_delay {
>   	EFI_TIMER_RELATIVE = 2
>   };
>   
> -#define UINTN size_t
> -typedef long INTN;
> +#define UINTN size_t   /* TODO this should be removed in a future patch */
> +typedef size_t efi_uintn_t;
> +typedef ssize_t efi_intn_t;
>   typedef uint16_t *efi_string_t;
>   
>   #define EVT_TIMER				0x80000000
> @@ -506,6 +507,35 @@ struct efi_device_path_to_text_protocol
>   			bool allow_shortcuts);
>   };
>   
> +#define EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID \
> +	EFI_GUID(0x0379be4e, 0xd706, 0x437d, \
> +		 0xb0, 0x37, 0xed, 0xb8, 0x2f, 0xb7, 0x72, 0xa4)
> +
> +struct efi_device_path_utilities_protocol {
> +	efi_uintn_t (EFIAPI *get_device_path_size)(
> +		const struct efi_device_path *device_path);
> +	struct efi_device_path *(EFIAPI *duplicate_device_path)(
> +		const struct efi_device_path *device_path);
> +	struct efi_device_path *(EFIAPI *append_device_path)(
> +		const struct efi_device_path *src1,
> +		const struct efi_device_path *src2);
> +	struct efi_device_path *(EFIAPI *append_device_node)(
> +		const struct efi_device_path *device_path,
> +		const struct efi_device_path *device_node);
> +	struct efi_device_path *(EFIAPI *append_device_path_instance)(
> +		const struct efi_device_path *device_path,
> +		const struct efi_device_path *device_path_instance);
> +	struct efi_device_path *(EFIAPI *get_next_device_path_instance)(
> +		struct efi_device_path **device_path_instance,
> +		efi_uintn_t *device_path_instance_size);
> +	bool (EFIAPI *is_device_path_multi_instance)(
> +		const struct efi_device_path *device_path);
> +	struct efi_device_path *(EFIAPI *create_device_node)(
> +		uint8_t node_type,
> +		uint8_t node_sub_type,
> +		uint16_t node_length);
> +};
> +
>   #define EFI_GOP_GUID \
>   	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
>   		 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index e1179b7dcd..5d37c1d75f 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -79,6 +79,7 @@ extern const struct efi_simple_text_output_protocol efi_con_out;
>   extern struct efi_simple_input_interface efi_con_in;
>   extern const struct efi_console_control_protocol efi_console_control;
>   extern const struct efi_device_path_to_text_protocol efi_device_path_to_text;
> +extern const struct efi_device_path_utilities_protocol efi_device_path_utilities;

WARNING: line over 80 characters
#95: FILE: include/efi_loader.h:82:

Please, always run scripts/checkpatch.pl before submitting.

>   
>   uint16_t *efi_dp_str(struct efi_device_path *dp);
>   
> @@ -89,6 +90,7 @@ extern const efi_guid_t efi_guid_loaded_image;
>   extern const efi_guid_t efi_guid_device_path_to_text_protocol;
>   extern const efi_guid_t efi_simple_file_system_protocol_guid;
>   extern const efi_guid_t efi_file_info_guid;
> +extern const efi_guid_t efi_guid_device_path_utilities_protocol;
>   
>   extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>   extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index ddb978f650..b6927b3b84 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -17,6 +17,7 @@ endif
>   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>   obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>   obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
> +obj-y += efi_device_path_utilities.o
>   obj-y += efi_file.o efi_variable.o efi_bootmgr.o
>   obj-$(CONFIG_LCD) += efi_gop.o
>   obj-$(CONFIG_DM_VIDEO) += efi_gop.o
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 976d5822f7..92c778fcca 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1153,6 +1153,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob

Applying: efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL
error: patch failed: lib/efi_loader/efi_boottime.c:1153

Please, rebase.

>   	obj->protocols[3].protocol_interface =
>   		(void *)&efi_device_path_to_text;
>   
> +	obj->protocols[4].guid = &efi_guid_device_path_utilities_protocol;
> +	obj->protocols[4].protocol_interface =
> +		(void *)&efi_device_path_utilities;
> +
>   	info->file_path = file_path;
>   	info->device_handle = efi_dp_find_obj(device_path, NULL);
>   
> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
> new file mode 100644
> index 0000000000..9d90f14ee4
> --- /dev/null
> +++ b/lib/efi_loader/efi_device_path_utilities.c
> @@ -0,0 +1,88 @@
> +/*
> + *  EFI device path interface
> + *
> + *  Copyright (c) 2017 Leif Lindholm
> + *
> + *  SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +
> +const efi_guid_t efi_guid_device_path_utilities_protocol =
> +		EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
> +
> +static efi_uintn_t EFIAPI get_device_path_size(
> +	const struct efi_device_path *device_path)
> +{
> +	efi_uintn_t sz = 0;

WARNING: Missing a blank line after declarations
#158: FILE: lib/efi_loader/efi_device_path_utilities.c:19:

Regards

Heinrich

> +	EFI_ENTRY("%p", device_path);
> +	/* size includes the END node: */
> +	if (device_path)
> +		sz = efi_dp_size(device_path) + sizeof(struct efi_device_path);
> +	return EFI_EXIT(sz);
> +}
> +
> +static struct efi_device_path * EFIAPI duplicate_device_path(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(efi_dp_dup(device_path));
> +}
> +
> +static struct efi_device_path * EFIAPI append_device_path(
> +	const struct efi_device_path *src1,
> +	const struct efi_device_path *src2)
> +{
> +	EFI_ENTRY("%p, %p", src1, src2);
> +	return EFI_EXIT(efi_dp_append(src1, src2));
> +}
> +
> +static struct efi_device_path * EFIAPI append_device_node(
> +	const struct efi_device_path *device_path,
> +	const struct efi_device_path *device_node)
> +{
> +	EFI_ENTRY("%p, %p", device_path, device_node);
> +	return EFI_EXIT(efi_dp_append_node(device_path, device_node));
> +}
> +
> +static struct efi_device_path * EFIAPI append_device_path_instance(
> +	const struct efi_device_path *device_path,
> +	const struct efi_device_path *device_path_instance)
> +{
> +	EFI_ENTRY("%p, %p", device_path, device_path_instance);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path * EFIAPI get_next_device_path_instance(
> +	struct efi_device_path **device_path_instance,
> +	efi_uintn_t *device_path_instance_size)
> +{
> +	EFI_ENTRY("%p, %p", device_path_instance, device_path_instance_size);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static bool EFIAPI is_device_path_multi_instance(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(false);
> +}
> +
> +static struct efi_device_path * EFIAPI create_device_node(
> +	uint8_t node_type, uint8_t node_sub_type, uint16_t node_length)
> +{
> +	EFI_ENTRY("%u, %u, %u", node_type, node_sub_type, node_length);
> +	return EFI_EXIT(NULL);
> +}
> +
> +const struct efi_device_path_utilities_protocol efi_device_path_utilities = {
> +	.get_device_path_size = get_device_path_size,
> +	.duplicate_device_path = duplicate_device_path,
> +	.append_device_path = append_device_path,
> +	.append_device_node = append_device_node,
> +	.append_device_path_instance = append_device_path_instance,
> +	.get_next_device_path_instance = get_next_device_path_instance,
> +	.is_device_path_multi_instance = is_device_path_multi_instance,
> +	.create_device_node = create_device_node,
> +};
>

On 10.10.17 14:22, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
> rest as needed or once we have SCT running properly so there is a way to
> validate the interface against the conformance test suite.
> 
> Initial skeleton written by Leif, and then implementation by myself.
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  include/efi_api.h                          | 34 +++++++++++-
>  include/efi_loader.h                       |  2 +
>  lib/efi_loader/Makefile                    |  1 +
>  lib/efi_loader/efi_boottime.c              |  4 ++
>  lib/efi_loader/efi_device_path_utilities.c | 88 ++++++++++++++++++++++++++++++
>  5 files changed, 127 insertions(+), 2 deletions(-)
>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index a9a6494afe..ffdba7fe1a 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -28,8 +28,9 @@ enum efi_timer_delay {
>  	EFI_TIMER_RELATIVE = 2
>  };
>  
> -#define UINTN size_t
> -typedef long INTN;
> +#define UINTN size_t   /* TODO this should be removed in a future patch */

$ git grep UINTN | wc -l
13

Just send a preceding patch that introduces efi_uintn_t and replaces all
occurences of UINTN with it.

The uintn bits shouldn't be part of the
EFI_DEVICE_PATH_UTILITIES_PROTOCOL patch anyways :).

> +typedef size_t efi_uintn_t;
> +typedef ssize_t efi_intn_t;
>  typedef uint16_t *efi_string_t;


Alex

On Wed, Oct 11, 2017 at 10:07 AM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 10.10.17 14:22, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
>> rest as needed or once we have SCT running properly so there is a way to
>> validate the interface against the conformance test suite.
>>
>> Initial skeleton written by Leif, and then implementation by myself.
>>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  include/efi_api.h                          | 34 +++++++++++-
>>  include/efi_loader.h                       |  2 +
>>  lib/efi_loader/Makefile                    |  1 +
>>  lib/efi_loader/efi_boottime.c              |  4 ++
>>  lib/efi_loader/efi_device_path_utilities.c | 88 ++++++++++++++++++++++++++++++
>>  5 files changed, 127 insertions(+), 2 deletions(-)
>>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index a9a6494afe..ffdba7fe1a 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -28,8 +28,9 @@ enum efi_timer_delay {
>>       EFI_TIMER_RELATIVE = 2
>>  };
>>
>> -#define UINTN size_t
>> -typedef long INTN;
>> +#define UINTN size_t   /* TODO this should be removed in a future patch */
>
> $ git grep UINTN | wc -l
> 13
>
> Just send a preceding patch that introduces efi_uintn_t and replaces all
> occurences of UINTN with it.
>
> The uintn bits shouldn't be part of the
> EFI_DEVICE_PATH_UTILITIES_PROTOCOL patch anyways :).
>

Heinrich mentioned he was doing that, so I didn't want to step on
feet.  I figured this was the easiest approach regardless of the order
of merging patches (should be simple enough to drop the duplicate
efi_uintn_t)

BR,
-R

>> +typedef size_t efi_uintn_t;
>> +typedef ssize_t efi_intn_t;
>>  typedef uint16_t *efi_string_t;
>
>
> Alex

On 10/11/2017 10:32 PM, Rob Clark wrote:
> On Wed, Oct 11, 2017 at 10:07 AM, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 10.10.17 14:22, Rob Clark wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> Not complete, but enough for Shell.efi and SCT.efi.  We'll implement the
>>> rest as needed or once we have SCT running properly so there is a way to
>>> validate the interface against the conformance test suite.
>>>
>>> Initial skeleton written by Leif, and then implementation by myself.
>>>
>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>   include/efi_api.h                          | 34 +++++++++++-
>>>   include/efi_loader.h                       |  2 +
>>>   lib/efi_loader/Makefile                    |  1 +
>>>   lib/efi_loader/efi_boottime.c              |  4 ++
>>>   lib/efi_loader/efi_device_path_utilities.c | 88 ++++++++++++++++++++++++++++++
>>>   5 files changed, 127 insertions(+), 2 deletions(-)
>>>   create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>>
>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>> index a9a6494afe..ffdba7fe1a 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -28,8 +28,9 @@ enum efi_timer_delay {
>>>        EFI_TIMER_RELATIVE = 2
>>>   };
>>>
>>> -#define UINTN size_t
>>> -typedef long INTN;
>>> +#define UINTN size_t   /* TODO this should be removed in a future patch */
>>
>> $ git grep UINTN | wc -l
>> 13
>>
>> Just send a preceding patch that introduces efi_uintn_t and replaces all
>> occurences of UINTN with it.
>>
>> The uintn bits shouldn't be part of the
>> EFI_DEVICE_PATH_UTILITIES_PROTOCOL patch anyways :).
>>
> 
> Heinrich mentioned he was doing that, so I didn't want to step on
> feet.  I figured this was the easiest approach regardless of the order
> of merging patches (should be simple enough to drop the duplicate
> efi_uintn_t)

I do not mind if you put the change in one of your patches. Just do what 
is needed to get your patch series merged.

Best regards

Heinrich

> 
> BR,
> -R
> 
>>> +typedef size_t efi_uintn_t;
>>> +typedef ssize_t efi_intn_t;
>>>   typedef uint16_t *efi_string_t;
>>
>>
>> Alex
>