[v1,01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL

Message ID 20170910132236.14318-2-robdclark@gmail.com
State New
Headers show
Series
  • [v1,01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL
Related show

Commit Message

Rob Clark Sept. 10, 2017, 1:22 p.m.
From: Leif Lindholm <leif.lindholm@linaro.org>

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 include/efi_api.h                          | 30 +++++++++++
 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 | 83 ++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 lib/efi_loader/efi_device_path_utilities.c

Comments

Heinrich Schuchardt Oct. 4, 2017, 5:13 p.m. | #1
On 09/10/2017 03:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  include/efi_api.h                          | 30 +++++++++++
>  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 | 83 ++++++++++++++++++++++++++++++
>  5 files changed, 120 insertions(+)
>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index c3b9032a48..57468dd972 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -506,6 +506,36 @@ 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
> +{
> +	UINTN(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,
> +		UINTN *device_path_instance_size);

The sequence is wrong. It does not match
UEFI Spec 2.7 Errata A, August 2017.

EDK2 uses the same sequence as the spec see

EdkCompatibilityPkg/Foundation/Efi/Protocol/DevicePathUtilities/DevicePathUtilities.h

Put is_device_path_multi_instance before create_device_node.

> +	struct efi_device_path *(EFIAPI *create_device_node)(
> +		uint8_t node_type,
> +		uint8_t node_sub_type,
> +		uint16_t node_length);
> +	bool(EFIAPI *is_device_path_multi_instance)(
> +		const struct efi_device_path *device_path);
> +};
> +
>  #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 43b12b94fa..c009828db9 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -58,6 +58,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;
>  
>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>  
> @@ -68,6 +69,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 930c0e218e..f5e69dd078 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -16,6 +16,7 @@ always := $(efiprogs-y)
>  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 3860feb79b..8bb243d673 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -775,6 +775,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>  	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..4b97080f35
> --- /dev/null
> +++ b/lib/efi_loader/efi_device_path_utilities.c
> @@ -0,0 +1,83 @@
> +/*
> + *  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;
> +

None of the function definitions below matches the definition in the
protocol structure. EFIAPI is missing. Rob tried to correct this in a
follow up patch. But it should be corrected here.

> +static UINTN get_device_path_size(const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(0);
> +}
> +
> +static struct efi_device_path *duplicate_device_path(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *append_device_path(
> +	const struct efi_device_path *src1,
> +	const struct efi_device_path *src2)
> +{
> +	EFI_ENTRY("%p, %p", src1, src2);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *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(NULL);
> +}
> +
> +static struct efi_device_path *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 *get_next_device_path_instance(
> +	struct efi_device_path **device_path_instance,
> +	UINTN *device_path_instance_size)
> +{
> +	EFI_ENTRY("%p, %p", device_path_instance, device_path_instance_size);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *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);
> +}
> +
> +static bool is_device_path_multi_instance(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(false);
> +}
> +
> +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,

Use the same sequence as the UEFI spec.
is_device_path_multi_instance followed by create_device_node

As these functions do not return efi_status_t an EFI application can
expect that a handle exposing this protocol correctly implements all of
them.

Follow up-patch
efi_loader: start fleshing out efi_device_path_utilities
only delivers part of it.

Best regards

Heinrich

> +	.create_device_node = create_device_node,
> +	.is_device_path_multi_instance = is_device_path_multi_instance,
> +};
>
Heinrich Schuchardt Oct. 4, 2017, 5:29 p.m. | #2
On 10/04/2017 07:13 PM, Heinrich Schuchardt wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h                          | 30 +++++++++++
>>  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 | 83 ++++++++++++++++++++++++++++++
>>  5 files changed, 120 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index c3b9032a48..57468dd972 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -506,6 +506,36 @@ 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
>> +{

checkpatch returns an error:

ERROR: open brace '{' following struct go on the same line
#30: FILE: include/efi_api.h:514:
+struct efi_device_path_utilities_protocol
+{

Regards

Heinrich

>> +	UINTN(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);
Heinrich Schuchardt Oct. 4, 2017, 6 p.m. | #3
On 10/04/2017 07:13 PM, Heinrich Schuchardt wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h                          | 30 +++++++++++
>>  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 | 83 ++++++++++++++++++++++++++++++
>>  5 files changed, 120 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>

<snip>

>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 3860feb79b..8bb243d673 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -775,6 +775,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>>  	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;
>> +

Do not add a protocol that is not properly implemented yet.

Regards

Heinrich

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index c3b9032a48..57468dd972 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -506,6 +506,36 @@  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
+{
+	UINTN(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,
+		UINTN *device_path_instance_size);
+	struct efi_device_path *(EFIAPI *create_device_node)(
+		uint8_t node_type,
+		uint8_t node_sub_type,
+		uint16_t node_length);
+	bool(EFIAPI *is_device_path_multi_instance)(
+		const struct efi_device_path *device_path);
+};
+
 #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 43b12b94fa..c009828db9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -58,6 +58,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;
 
 uint16_t *efi_dp_str(struct efi_device_path *dp);
 
@@ -68,6 +69,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 930c0e218e..f5e69dd078 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -16,6 +16,7 @@  always := $(efiprogs-y)
 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 3860feb79b..8bb243d673 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -775,6 +775,10 @@  void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
 	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..4b97080f35
--- /dev/null
+++ b/lib/efi_loader/efi_device_path_utilities.c
@@ -0,0 +1,83 @@ 
+/*
+ *  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 UINTN get_device_path_size(const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(0);
+}
+
+static struct efi_device_path *duplicate_device_path(
+	const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *append_device_path(
+	const struct efi_device_path *src1,
+	const struct efi_device_path *src2)
+{
+	EFI_ENTRY("%p, %p", src1, src2);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *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(NULL);
+}
+
+static struct efi_device_path *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 *get_next_device_path_instance(
+	struct efi_device_path **device_path_instance,
+	UINTN *device_path_instance_size)
+{
+	EFI_ENTRY("%p, %p", device_path_instance, device_path_instance_size);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *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);
+}
+
+static bool is_device_path_multi_instance(
+	const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(false);
+}
+
+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,
+	.create_device_node = create_device_node,
+	.is_device_path_multi_instance = is_device_path_multi_instance,
+};