[U-Boot] [PATCH v2 00/18] manage protocols in a linked list

[U-Boot] [PATCH v2 00/18] manage protocols in a linked list

[Heinrich Schuchardt]

Up to now the protocols of an EFI handle where contained in an
array of fixed size. With the patch series the protocols are
managed in a linked list. This both saves memory and gives
more flexibility.

The LocateDevicePath boot service is implemented according
to the UEFI specification.

A unit test for the LocateDevicePath boot service and the
device path to text protocol are added.

Some bug fixes are provided.

v2
	Enhance the helloworld example to check if the image
	handle is passed corectly. Adjust the related py
	test.

	Use a helper function to initialize EFI objects.

	Further minor bug fixes.

Heinrich Schuchardt (18):
  efi_loader: efi_bootmgr: do not make hidden assignments
  efi_loader: size of media device path node represenation
  efi_loader: efi_dp_str should print path not node
  efi_loader: fix efi_convert_device_node_to_text
  efi_loader: reimplement LocateDevicePath
  efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL
  efi_loader: efi_disk: use efi_add_protocol
  efi_loader: efi_net: use efi_add_protocol
  efi_loader: efi_gop: use efi_add_protocol
  efi_loader: simplify efi_open_protocol
  efi_loader: simplify find_obj
  efi_loader: manage protocols in a linked list
  efi_selftest: compile without special compiler flags
  efi_selftest: add missing line feed
  efi_loader: output load options in helloworld
  test/py: check return code of helloworld
  efi_loader: pass handle of loaded image
  efi_loader: helper function to add EFI object to list

 cmd/bootefi.c                              |   7 +-
 include/efi_loader.h                       |   8 +-
 lib/efi_loader/efi_bootmgr.c               |   6 +-
 lib/efi_loader/efi_boottime.c              | 286 +++++++++++++-----------
 lib/efi_loader/efi_device_path.c           |  43 ++--
 lib/efi_loader/efi_device_path_to_text.c   | 161 +++++++-------
 lib/efi_loader/efi_disk.c                  |  38 ++--
 lib/efi_loader/efi_gop.c                   |  16 +-
 lib/efi_loader/efi_net.c                   |  35 +--
 lib/efi_loader/helloworld.c                |  31 ++-
 lib/efi_selftest/Makefile                  |  24 +-
 lib/efi_selftest/efi_selftest.c            |   2 +-
 lib/efi_selftest/efi_selftest_devicepath.c | 340 +++++++++++++++++++++++++++++
 test/py/tests/test_efi_loader.py           |   2 +
 14 files changed, 710 insertions(+), 289 deletions(-)
 create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c

-- 
2.15.0

[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Heinrich Schuchardt]

Assignments should not be made in the middle of nowhere.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	Call efi_dp_str in debug mode only.
---
 lib/efi_loader/efi_bootmgr.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 857d88a879..a55f210f0b 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -120,11 +120,13 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
 
 	if (lo.attributes & LOAD_OPTION_ACTIVE) {
 		efi_status_t ret;
-		u16 *str = NULL;
+#ifdef _DEBUG
+		u16 *str = efi_dp_str(lo.file_path);
 
 		debug("%s: trying to load \"%ls\" from: %ls\n", __func__,
-		      lo.label, (str = efi_dp_str(lo.file_path)));
+		      lo.label, str);
 		efi_free_pool(str);
+#endif /* _DEBUG */
 
 		ret = efi_load_image_from_path(lo.file_path, &image);
 
-- 
2.15.0

[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Rob Clark]

On Sun, Nov 12, 2017 at 9:02 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Assignments should not be made in the middle of nowhere.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         Call efi_dp_str in debug mode only.
> ---
>  lib/efi_loader/efi_bootmgr.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 857d88a879..a55f210f0b 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -120,11 +120,13 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
>
>         if (lo.attributes & LOAD_OPTION_ACTIVE) {
>                 efi_status_t ret;
> -               u16 *str = NULL;
> +#ifdef _DEBUG
> +               u16 *str = efi_dp_str(lo.file_path);
>
>                 debug("%s: trying to load \"%ls\" from: %ls\n", __func__,
> -                     lo.label, (str = efi_dp_str(lo.file_path)));
> +                     lo.label, str);
>                 efi_free_pool(str);
> +#endif /* _DEBUG */

I was trying to avoid the #ifdef DEBUG in the first place.. ;-)

btw, don't have the code in front of me atm, but I thought _DEBUG was
unconditionally defined to either 0 or 1.. so maybe you mean '#if
_DEBUG' or '#ifdef DEBUG' (no underscore)?

btw2, possibly a better/cleaner option is add printf support for
format modifier to print device-paths?

BR,
-R

>
>                 ret = efi_load_image_from_path(lo.file_path, &image);
>
> --
> 2.15.0
>

[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Heinrich Schuchardt]

On 11/17/2017 03:04 PM, Rob Clark wrote:
> On Sun, Nov 12, 2017 at 9:02 AM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> Assignments should not be made in the middle of nowhere.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>          Call efi_dp_str in debug mode only.
>> ---
>>   lib/efi_loader/efi_bootmgr.c | 6 ++++--
>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>
>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> index 857d88a879..a55f210f0b 100644
>> --- a/lib/efi_loader/efi_bootmgr.c
>> +++ b/lib/efi_loader/efi_bootmgr.c
>> @@ -120,11 +120,13 @@ static void *try_load_entry(uint16_t n, struct efi_device_path **device_path,
>>
>>          if (lo.attributes & LOAD_OPTION_ACTIVE) {
>>                  efi_status_t ret;
>> -               u16 *str = NULL;
>> +#ifdef _DEBUG
>> +               u16 *str = efi_dp_str(lo.file_path);
>>
>>                  debug("%s: trying to load \"%ls\" from: %ls\n", __func__,
>> -                     lo.label, (str = efi_dp_str(lo.file_path)));
>> +                     lo.label, str);
>>                  efi_free_pool(str);
>> +#endif /* _DEBUG */
> 
> I was trying to avoid the #ifdef DEBUG in the first place.. ;-)
> 
> btw, don't have the code in front of me atm, but I thought _DEBUG was
> unconditionally defined to either 0 or 1.. so maybe you mean '#if
> _DEBUG' or '#ifdef DEBUG' (no underscore)?

Thanks for catching this

I will change this to
#if _DEBUG

I often found the necessity to change the value of _DEBUG multiple times 
within a single C file to get only the relevant debug messages. This is 
why I prefer checking _DEBUG.

> 
> btw2, possibly a better/cleaner option is add printf support for
> format modifier to print device-paths?

This is the only place where efi_dp_str is called.

Regards

Heinrich

> 
> BR,
> -R
> 
>>
>>                  ret = efi_load_image_from_path(lo.file_path, &image);
>>
>> --
>> 2.15.0
>>
> 

[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Rob Clark]

On Fri, Nov 17, 2017 at 12:46 PM, Heinrich Schuchardt
<xypron.glpk@gmx.de> wrote:
> On 11/17/2017 03:04 PM, Rob Clark wrote:
>>
>> On Sun, Nov 12, 2017 at 9:02 AM, Heinrich Schuchardt <xypron.glpk@gmx.de>
>> wrote:
>>>
>>> Assignments should not be made in the middle of nowhere.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> v2
>>>          Call efi_dp_str in debug mode only.
>>> ---
>>>   lib/efi_loader/efi_bootmgr.c | 6 ++++--
>>>   1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>>> index 857d88a879..a55f210f0b 100644
>>> --- a/lib/efi_loader/efi_bootmgr.c
>>> +++ b/lib/efi_loader/efi_bootmgr.c
>>> @@ -120,11 +120,13 @@ static void *try_load_entry(uint16_t n, struct
>>> efi_device_path **device_path,
>>>
>>>          if (lo.attributes & LOAD_OPTION_ACTIVE) {
>>>                  efi_status_t ret;
>>> -               u16 *str = NULL;
>>> +#ifdef _DEBUG
>>> +               u16 *str = efi_dp_str(lo.file_path);
>>>
>>>                  debug("%s: trying to load \"%ls\" from: %ls\n",
>>> __func__,
>>> -                     lo.label, (str = efi_dp_str(lo.file_path)));
>>> +                     lo.label, str);
>>>                  efi_free_pool(str);
>>> +#endif /* _DEBUG */
>>
>>
>> I was trying to avoid the #ifdef DEBUG in the first place.. ;-)
>>
>> btw, don't have the code in front of me atm, but I thought _DEBUG was
>> unconditionally defined to either 0 or 1.. so maybe you mean '#if
>> _DEBUG' or '#ifdef DEBUG' (no underscore)?
>
>
> Thanks for catching this
>
> I will change this to
> #if _DEBUG
>
> I often found the necessity to change the value of _DEBUG multiple times
> within a single C file to get only the relevant debug messages. This is why
> I prefer checking _DEBUG.
>
>>
>> btw2, possibly a better/cleaner option is add printf support for
>> format modifier to print device-paths?
>
>
> This is the only place where efi_dp_str is called.
>

I have had locally other callers while debugging things.. maybe I
should have left some of those in the code (and maybe I would have if
printf() supported it :-P)

Device-paths are fairly heavily used in UEFI.. so it tends to be
something you end up wanting to print in various places when
debugging.

BR,
-R

> Regards
>
> Heinrich
>
>
>>
>> BR,
>> -R
>>
>>>
>>>                  ret = efi_load_image_from_path(lo.file_path, &image);
>>>
>>> --
>>> 2.15.0
>>>
>>
>

[U-Boot] [PATCH v2 01/18] efi_loader: efi_bootmgr: do not make hidden assignments

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Assignments should not be made in the middle of nowhere.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         Call efi_dp_str in debug mode only.
> ---
>  lib/efi_loader/efi_bootmgr.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 02/18] efi_loader: size of media device path node represenation

[Heinrich Schuchardt]

In the format specifier we want to specify the maximum width
in case an ending \0 is missing.

So slen must be used as precision and not as field width.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_device_path_to_text.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index 62771338f0..98617cf163 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -153,7 +153,7 @@ static char *dp_media(char *s, struct efi_device_path *dp)
 		struct efi_device_path_file_path *fp =
 			(struct efi_device_path_file_path *)dp;
 		int slen = (dp->length - sizeof(*dp)) / 2;
-		s += sprintf(s, "/%-*ls", slen, fp->str);
+		s += sprintf(s, "/%-.*ls", slen, fp->str);
 		break;
 	}
 	default:
-- 
2.15.0

[U-Boot] [PATCH v2 02/18] efi_loader: size of media device path node represenation

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> In the format specifier we want to specify the maximum width
> in case an ending \0 is missing.
>
> So slen must be used as precision and not as field width.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_device_path_to_text.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 03/18] efi_loader: efi_dp_str should print path not node

[Heinrich Schuchardt]

efi_dp_str is meant to print a device path and not a device
node.

The old coding only worked because efi_convert_device_node_to_text
was screwed up to expect paths instead of nodes.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_device_path_to_text.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index 98617cf163..ad248cb492 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -208,13 +208,6 @@ static uint16_t *efi_convert_device_node_to_text(
 	return out;
 }
 
-/* helper for debug prints.. efi_free_pool() the result. */
-uint16_t *efi_dp_str(struct efi_device_path *dp)
-{
-	return efi_convert_device_node_to_text(dp, true, true);
-}
-
-
 static uint16_t EFIAPI *efi_convert_device_node_to_text_ext(
 		struct efi_device_path *device_node,
 		bool display_only,
@@ -251,6 +244,12 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
 	return buffer;
 }
 
+/* helper for debug prints.. efi_free_pool() the result. */
+uint16_t *efi_dp_str(struct efi_device_path *dp)
+{
+	return EFI_CALL(efi_convert_device_path_to_text(dp, true, true));
+}
+
 const struct efi_device_path_to_text_protocol efi_device_path_to_text = {
 	.convert_device_node_to_text = efi_convert_device_node_to_text_ext,
 	.convert_device_path_to_text = efi_convert_device_path_to_text,
-- 
2.15.0

[U-Boot] [PATCH v2 03/18] efi_loader: efi_dp_str should print path not node

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> efi_dp_str is meant to print a device path and not a device
> node.
>
> The old coding only worked because efi_convert_device_node_to_text
> was screwed up to expect paths instead of nodes.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_device_path_to_text.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 04/18] efi_loader: fix efi_convert_device_node_to_text

[Heinrich Schuchardt]

We need to implement to different functions for the
EFI_DEVICE_PATH_TO_TEXT_PROTOCOL:
ConvertDeviceNodeToText
ConvertDevicePathToText

A recent patch screwed up efi_convert_device_node_to_text
to expect a device path and not a node.

The patch makes both service functions work again.

efi_convert_device_node_to_text is renamed to
efi_convert_single_device_node_to_text and
efi_convert_device_node_to_text_ext is renamed to
efi_convert_device_node_to_text to avoid future
confusion.

A test of ConvertDeviceNodeToText will be provided in
a follow-up patch.

Fixes: adae4313cdd efi_loader: flesh out device-path to text
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_device_path_to_text.c | 148 +++++++++++++++++--------------
 1 file changed, 82 insertions(+), 66 deletions(-)

diff --git a/lib/efi_loader/efi_device_path_to_text.c b/lib/efi_loader/efi_device_path_to_text.c
index ad248cb492..3a9d5e4122 100644
--- a/lib/efi_loader/efi_device_path_to_text.c
+++ b/lib/efi_loader/efi_device_path_to_text.c
@@ -12,12 +12,31 @@
 #define MAC_OUTPUT_LEN 22
 #define UNKNOWN_OUTPUT_LEN 23
 
+#define MAX_NODE_LEN 512
+#define MAX_PATH_LEN 1024
+
 const efi_guid_t efi_guid_device_path_to_text_protocol =
 		EFI_DEVICE_PATH_TO_TEXT_PROTOCOL_GUID;
 
+static u16 *efi_str_to_u16(char *str)
+{
+	efi_uintn_t len;
+	u16 *out;
+	efi_status_t ret;
+
+	len = strlen(str) + 1;
+	ret = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, len * sizeof(u16),
+				(void **)&out);
+	if (ret != EFI_SUCCESS)
+		return NULL;
+	ascii2unicode(out, str);
+	out[len - 1] = 0;
+	return out;
+}
+
 static char *dp_unknown(char *s, struct efi_device_path *dp)
 {
-	s += sprintf(s, "/UNKNOWN(%04x,%04x)", dp->type, dp->sub_type);
+	s += sprintf(s, "UNKNOWN(%04x,%04x)", dp->type, dp->sub_type);
 	return s;
 }
 
@@ -27,7 +46,7 @@ static char *dp_hardware(char *s, struct efi_device_path *dp)
 	case DEVICE_PATH_SUB_TYPE_MEMORY: {
 		struct efi_device_path_memory *mdp =
 			(struct efi_device_path_memory *)dp;
-		s += sprintf(s, "/MemoryMapped(0x%x,0x%llx,0x%llx)",
+		s += sprintf(s, "MemoryMapped(0x%x,0x%llx,0x%llx)",
 			     mdp->memory_type,
 			     mdp->start_address,
 			     mdp->end_address);
@@ -36,7 +55,7 @@ static char *dp_hardware(char *s, struct efi_device_path *dp)
 	case DEVICE_PATH_SUB_TYPE_VENDOR: {
 		struct efi_device_path_vendor *vdp =
 			(struct efi_device_path_vendor *)dp;
-		s += sprintf(s, "/VenHw(%pUl)", &vdp->guid);
+		s += sprintf(s, "VenHw(%pUl)", &vdp->guid);
 		break;
 	}
 	default:
@@ -52,7 +71,7 @@ static char *dp_acpi(char *s, struct efi_device_path *dp)
 	case DEVICE_PATH_SUB_TYPE_ACPI_DEVICE: {
 		struct efi_device_path_acpi_path *adp =
 			(struct efi_device_path_acpi_path *)dp;
-		s += sprintf(s, "/Acpi(PNP%04x", EISA_PNP_NUM(adp->hid));
+		s += sprintf(s, "Acpi(PNP%04x", EISA_PNP_NUM(adp->hid));
 		if (adp->uid)
 			s += sprintf(s, ",%d", adp->uid);
 		s += sprintf(s, ")");
@@ -71,7 +90,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp)
 	case DEVICE_PATH_SUB_TYPE_MSG_USB: {
 		struct efi_device_path_usb *udp =
 			(struct efi_device_path_usb *)dp;
-		s += sprintf(s, "/Usb(0x%x,0x%x)", udp->parent_port_number,
+		s += sprintf(s, "Usb(0x%x,0x%x)", udp->parent_port_number,
 			     udp->usb_interface);
 		break;
 	}
@@ -82,7 +101,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp)
 		if (mdp->if_type != 0 && mdp->if_type != 1)
 			break;
 
-		s += sprintf(s, "/MAC(%02x%02x%02x%02x%02x%02x,0x%1x)",
+		s += sprintf(s, "MAC(%02x%02x%02x%02x%02x%02x,0x%1x)",
 			mdp->mac.addr[0], mdp->mac.addr[1],
 			mdp->mac.addr[2], mdp->mac.addr[3],
 			mdp->mac.addr[4], mdp->mac.addr[5],
@@ -94,7 +113,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp)
 		struct efi_device_path_usb_class *ucdp =
 			(struct efi_device_path_usb_class *)dp;
 
-		s += sprintf(s, "/USBClass(%x,%x,%x,%x,%x)",
+		s += sprintf(s, "USBClass(%x,%x,%x,%x,%x)",
 			ucdp->vendor_id, ucdp->product_id,
 			ucdp->device_class, ucdp->device_subclass,
 			ucdp->device_protocol);
@@ -108,7 +127,7 @@ static char *dp_msging(char *s, struct efi_device_path *dp)
 					"SDCard" : "MMC";
 		struct efi_device_path_sd_mmc_path *sddp =
 			(struct efi_device_path_sd_mmc_path *)dp;
-		s += sprintf(s, "/%s(Slot%u)", typename, sddp->slot_number);
+		s += sprintf(s, "%s(Slot%u)", typename, sddp->slot_number);
 		break;
 	}
 	default:
@@ -128,15 +147,15 @@ static char *dp_media(char *s, struct efi_device_path *dp)
 
 		switch (hddp->signature_type) {
 		case SIG_TYPE_MBR:
-			s += sprintf(s, "/HD(Part%d,Sig%08x)",
+			s += sprintf(s, "HD(Part%d,Sig%08x)",
 				     hddp->partition_number,
 				     *(uint32_t *)sig);
 			break;
 		case SIG_TYPE_GUID:
-			s += sprintf(s, "/HD(Part%d,Sig%pUl)",
+			s += sprintf(s, "HD(Part%d,Sig%pUl)",
 				     hddp->partition_number, sig);
 		default:
-			s += sprintf(s, "/HD(Part%d,MBRType=%02x,SigType=%02x)",
+			s += sprintf(s, "HD(Part%d,MBRType=%02x,SigType=%02x)",
 				     hddp->partition_number, hddp->partmap_type,
 				     hddp->signature_type);
 		}
@@ -146,14 +165,16 @@ static char *dp_media(char *s, struct efi_device_path *dp)
 	case DEVICE_PATH_SUB_TYPE_CDROM_PATH: {
 		struct efi_device_path_cdrom_path *cddp =
 			(struct efi_device_path_cdrom_path *)dp;
-		s += sprintf(s, "/CDROM(0x%x)", cddp->boot_entry);
+		s += sprintf(s, "CDROM(0x%x)", cddp->boot_entry);
 		break;
 	}
 	case DEVICE_PATH_SUB_TYPE_FILE_PATH: {
 		struct efi_device_path_file_path *fp =
 			(struct efi_device_path_file_path *)dp;
 		int slen = (dp->length - sizeof(*dp)) / 2;
-		s += sprintf(s, "/%-.*ls", slen, fp->str);
+		if (slen > MAX_NODE_LEN - 2)
+			slen = MAX_NODE_LEN - 2;
+		s += sprintf(s, "%-.*ls", slen, fp->str);
 		break;
 	}
 	default:
@@ -163,65 +184,56 @@ static char *dp_media(char *s, struct efi_device_path *dp)
 	return s;
 }
 
-static uint16_t *efi_convert_device_node_to_text(
-		struct efi_device_path *dp,
-		bool display_only,
-		bool allow_shortcuts)
+/*
+ * Converts a single node to a char string.
+ *
+ * @buffer		output buffer
+ * @dp			device path or node
+ * @return		end of string
+ */
+static char *efi_convert_single_device_node_to_text(
+		char *buffer,
+		struct efi_device_path *dp)
 {
-	unsigned long len;
-	efi_status_t r;
-	char buf[512];  /* this ought be be big enough for worst case */
-	char *str = buf;
-	uint16_t *out;
-
-	while (dp) {
-		switch (dp->type) {
-		case DEVICE_PATH_TYPE_HARDWARE_DEVICE:
-			str = dp_hardware(str, dp);
-			break;
-		case DEVICE_PATH_TYPE_ACPI_DEVICE:
-			str = dp_acpi(str, dp);
-			break;
-		case DEVICE_PATH_TYPE_MESSAGING_DEVICE:
-			str = dp_msging(str, dp);
-			break;
-		case DEVICE_PATH_TYPE_MEDIA_DEVICE:
-			str = dp_media(str, dp);
-			break;
-		default:
-			str = dp_unknown(str, dp);
-		}
+	char *str = buffer;
 
-		dp = efi_dp_next(dp);
+	switch (dp->type) {
+	case DEVICE_PATH_TYPE_HARDWARE_DEVICE:
+		str = dp_hardware(str, dp);
+		break;
+	case DEVICE_PATH_TYPE_ACPI_DEVICE:
+		str = dp_acpi(str, dp);
+		break;
+	case DEVICE_PATH_TYPE_MESSAGING_DEVICE:
+		str = dp_msging(str, dp);
+		break;
+	case DEVICE_PATH_TYPE_MEDIA_DEVICE:
+		str = dp_media(str, dp);
+		break;
+	default:
+		str = dp_unknown(str, dp);
 	}
 
-	*str++ = '\0';
-
-	len = str - buf;
-	r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, 2 * len, (void **)&out);
-	if (r != EFI_SUCCESS)
-		return NULL;
-
-	ascii2unicode(out, buf);
-	out[len - 1] = 0;
-
-	return out;
+	*str = '\0';
+	return str;
 }
 
-static uint16_t EFIAPI *efi_convert_device_node_to_text_ext(
+static uint16_t EFIAPI *efi_convert_device_node_to_text(
 		struct efi_device_path *device_node,
 		bool display_only,
 		bool allow_shortcuts)
 {
-	uint16_t *buffer;
+	char str[MAX_NODE_LEN];
+	uint16_t *text;
 
 	EFI_ENTRY("%p, %d, %d", device_node, display_only, allow_shortcuts);
 
-	buffer = efi_convert_device_node_to_text(device_node, display_only,
-						 allow_shortcuts);
+	efi_convert_single_device_node_to_text(str, device_node);
+
+	text = efi_str_to_u16(str);
 
 	EFI_EXIT(EFI_SUCCESS);
-	return buffer;
+	return text;
 }
 
 static uint16_t EFIAPI *efi_convert_device_path_to_text(
@@ -229,19 +241,23 @@ static uint16_t EFIAPI *efi_convert_device_path_to_text(
 		bool display_only,
 		bool allow_shortcuts)
 {
-	uint16_t *buffer;
+	uint16_t *text;
+	char buffer[MAX_PATH_LEN];
+	char *str = buffer;
 
 	EFI_ENTRY("%p, %d, %d", device_path, display_only, allow_shortcuts);
 
-	/*
-	 * Our device paths are all of depth one. So its is sufficient to
-	 * to convert the first node.
-	 */
-	buffer = efi_convert_device_node_to_text(device_path, display_only,
-						 allow_shortcuts);
+	while (device_path &&
+	       str + MAX_NODE_LEN < buffer + MAX_PATH_LEN) {
+		*str++ = '/';
+		str = efi_convert_single_device_node_to_text(str, device_path);
+		device_path = efi_dp_next(device_path);
+	}
+
+	text = efi_str_to_u16(buffer);
 
 	EFI_EXIT(EFI_SUCCESS);
-	return buffer;
+	return text;
 }
 
 /* helper for debug prints.. efi_free_pool() the result. */
@@ -251,6 +267,6 @@ uint16_t *efi_dp_str(struct efi_device_path *dp)
 }
 
 const struct efi_device_path_to_text_protocol efi_device_path_to_text = {
-	.convert_device_node_to_text = efi_convert_device_node_to_text_ext,
+	.convert_device_node_to_text = efi_convert_device_node_to_text,
 	.convert_device_path_to_text = efi_convert_device_path_to_text,
 };
-- 
2.15.0

[U-Boot] [PATCH v2 04/18] efi_loader: fix efi_convert_device_node_to_text

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> We need to implement to different functions for the
> EFI_DEVICE_PATH_TO_TEXT_PROTOCOL:
> ConvertDeviceNodeToText
> ConvertDevicePathToText
>
> A recent patch screwed up efi_convert_device_node_to_text
> to expect a device path and not a node.
>
> The patch makes both service functions work again.
>
> efi_convert_device_node_to_text is renamed to
> efi_convert_single_device_node_to_text and
> efi_convert_device_node_to_text_ext is renamed to
> efi_convert_device_node_to_text to avoid future
> confusion.
>
> A test of ConvertDeviceNodeToText will be provided in
> a follow-up patch.
>
> Fixes: adae4313cdd efi_loader: flesh out device-path to text
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_device_path_to_text.c | 148 +++++++++++++++++--------------
>  1 file changed, 82 insertions(+), 66 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 05/18] efi_loader: reimplement LocateDevicePath

[Heinrich Schuchardt]

The current implementation of efi_locate_device_path does not match
the UEFI specification. It completely ignores the protocol
parameters.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_boottime.c | 106 ++++++++++++++++++++++++++++++------------
 1 file changed, 76 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 9b5512f086..7457d54739 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1080,36 +1080,6 @@ static efi_status_t EFIAPI efi_locate_handle_ext(
 			buffer_size, buffer));
 }
 
-/*
- * Get the device path and handle of an device implementing a protocol.
- *
- * This function implements the LocateDevicePath service.
- * See the Unified Extensible Firmware Interface (UEFI) specification
- * for details.
- *
- * @protocol		GUID of the protocol
- * @device_path		device path
- * @device		handle of the device
- * @return		status code
- */
-static efi_status_t EFIAPI efi_locate_device_path(
-			const efi_guid_t *protocol,
-			struct efi_device_path **device_path,
-			efi_handle_t *device)
-{
-	struct efi_object *efiobj;
-
-	EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
-
-	efiobj = efi_dp_find_obj(*device_path, device_path);
-	if (!efiobj)
-		return EFI_EXIT(EFI_NOT_FOUND);
-
-	*device = efiobj->handle;
-
-	return EFI_EXIT(EFI_SUCCESS);
-}
-
 /* Collapses configuration table entries, removing index i */
 static void efi_remove_configuration_table(int i)
 {
@@ -1813,6 +1783,82 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
+/*
+ * Get the device path and handle of an device implementing a protocol.
+ *
+ * This function implements the LocateDevicePath service.
+ * See the Unified Extensible Firmware Interface (UEFI) specification
+ * for details.
+ *
+ * @protocol		GUID of the protocol
+ * @device_path		device path
+ * @device		handle of the device
+ * @return		status code
+ */
+static efi_status_t EFIAPI efi_locate_device_path(
+			const efi_guid_t *protocol,
+			struct efi_device_path **device_path,
+			efi_handle_t *device)
+{
+	struct efi_device_path *dp;
+	size_t i;
+	struct efi_handler *handler;
+	efi_handle_t *handles;
+	size_t len, len_dp;
+	size_t len_best = 0;
+	efi_uintn_t no_handles;
+	u8 *remainder;
+	efi_status_t ret;
+
+	EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
+
+	if (!protocol || !device_path || !*device_path || !device) {
+		ret = EFI_INVALID_PARAMETER;
+		goto out;
+	}
+
+	/* Find end of device path */
+	len = efi_dp_size(*device_path);
+
+	/* Get all handles implementing the protocol */
+	ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, protocol, NULL,
+						&no_handles, &handles));
+	if (ret != EFI_SUCCESS)
+		goto out;
+
+	for (i = 0; i < no_handles; ++i) {
+		/* Find the device path protocol */
+		ret = efi_search_protocol(handles[i], &efi_guid_device_path,
+					  &handler);
+		if (ret != EFI_SUCCESS)
+			continue;
+		dp = (struct efi_device_path *)handler->protocol_interface;
+		len_dp = efi_dp_size(dp);
+		/*
+		 * This handle can only be a better fit
+		 * if its device path length is longer then the best fit and
+		 * if its device path length is shorter of equal the searched
+		 * device path.
+		 */
+		if (len_dp <= len_best || len_dp > len)
+			continue;
+		/* Check if dp is a subpath of device_path. */
+		if (memcmp(*device_path, dp, len_dp))
+			continue;
+		*device = handles[i];
+		len_best = len_dp;
+	}
+	if (len_best) {
+		remainder = (u8 *)*device_path + len_best;
+		*device_path = (struct efi_device_path *)remainder;
+		ret = EFI_SUCCESS;
+	} else {
+		ret = EFI_NOT_FOUND;
+	}
+out:
+	return EFI_EXIT(ret);
+}
+
 /*
  * Install multiple protocol interfaces.
  *
-- 
2.15.0

[U-Boot] [PATCH v2 05/18] efi_loader: reimplement LocateDevicePath

[Simon Glass]

Hi Heinrich,

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The current implementation of efi_locate_device_path does not match
> the UEFI specification. It completely ignores the protocol
> parameters.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_boottime.c | 106 ++++++++++++++++++++++++++++++------------
>  1 file changed, 76 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Nits below

>
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 9b5512f086..7457d54739 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1080,36 +1080,6 @@ static efi_status_t EFIAPI efi_locate_handle_ext(
>                         buffer_size, buffer));
>  }
>
> -/*
> - * Get the device path and handle of an device implementing a protocol.
> - *
> - * This function implements the LocateDevicePath service.
> - * See the Unified Extensible Firmware Interface (UEFI) specification
> - * for details.

Oh yes, I will get right on that now :-)

> - *
> - * @protocol           GUID of the protocol
> - * @device_path                device path
> - * @device             handle of the device
> - * @return             status code
> - */
> -static efi_status_t EFIAPI efi_locate_device_path(
> -                       const efi_guid_t *protocol,
> -                       struct efi_device_path **device_path,
> -                       efi_handle_t *device)
> -{
> -       struct efi_object *efiobj;
> -
> -       EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
> -
> -       efiobj = efi_dp_find_obj(*device_path, device_path);
> -       if (!efiobj)
> -               return EFI_EXIT(EFI_NOT_FOUND);
> -
> -       *device = efiobj->handle;
> -
> -       return EFI_EXIT(EFI_SUCCESS);
> -}
> -
>  /* Collapses configuration table entries, removing index i */
>  static void efi_remove_configuration_table(int i)
>  {
> @@ -1813,6 +1783,82 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
>         return EFI_EXIT(EFI_NOT_FOUND);
>  }
>
> +/*
> + * Get the device path and handle of an device implementing a protocol.

a device?

> + *
> + * This function implements the LocateDevicePath service.
> + * See the Unified Extensible Firmware Interface (UEFI) specification
> + * for details.
> + *
> + * @protocol           GUID of the protocol
> + * @device_path                device path
> + * @device             handle of the device
> + * @return             status code
> + */
> +static efi_status_t EFIAPI efi_locate_device_path(
> +                       const efi_guid_t *protocol,
> +                       struct efi_device_path **device_path,
> +                       efi_handle_t *device)
> +{
> +       struct efi_device_path *dp;
> +       size_t i;
> +       struct efi_handler *handler;
> +       efi_handle_t *handles;
> +       size_t len, len_dp;
> +       size_t len_best = 0;
> +       efi_uintn_t no_handles;

Is there none?

I think num_handles is better, or handles_count.

> +       u8 *remainder;
> +       efi_status_t ret;
> +
> +       EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
> +
> +       if (!protocol || !device_path || !*device_path || !device) {
> +               ret = EFI_INVALID_PARAMETER;
> +               goto out;
> +       }
> +
> +       /* Find end of device path */
> +       len = efi_dp_size(*device_path);
> +
> +       /* Get all handles implementing the protocol */
> +       ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, protocol, NULL,
> +                                               &no_handles, &handles));
> +       if (ret != EFI_SUCCESS)
> +               goto out;
> +
> +       for (i = 0; i < no_handles; ++i) {
> +               /* Find the device path protocol */
> +               ret = efi_search_protocol(handles[i], &efi_guid_device_path,
> +                                         &handler);
> +               if (ret != EFI_SUCCESS)
> +                       continue;
> +               dp = (struct efi_device_path *)handler->protocol_interface;
> +               len_dp = efi_dp_size(dp);
> +               /*
> +                * This handle can only be a better fit
> +                * if its device path length is longer then the best fit and
> +                * if its device path length is shorter of equal the searched
> +                * device path.

Format to ~77 cols?

> +                */
> +               if (len_dp <= len_best || len_dp > len)
> +                       continue;
> +               /* Check if dp is a subpath of device_path. */

Can you drop the period?

> +               if (memcmp(*device_path, dp, len_dp))
> +                       continue;
> +               *device = handles[i];
> +               len_best = len_dp;
> +       }
> +       if (len_best) {
> +               remainder = (u8 *)*device_path + len_best;
> +               *device_path = (struct efi_device_path *)remainder;
> +               ret = EFI_SUCCESS;
> +       } else {
> +               ret = EFI_NOT_FOUND;
> +       }
> +out:
> +       return EFI_EXIT(ret);
> +}
> +
>  /*
>   * Install multiple protocol interfaces.
>   *
> --
> 2.15.0
>
Regards,
Simon

[U-Boot] [PATCH v2 05/18] efi_loader: reimplement LocateDevicePath

[Heinrich Schuchardt]

On 11/17/2017 03:06 PM, Simon Glass wrote:
> Hi Heinrich,
> 
> On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> The current implementation of efi_locate_device_path does not match
>> the UEFI specification. It completely ignores the protocol
>> parameters.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>          no change
>> ---
>>   lib/efi_loader/efi_boottime.c | 106 ++++++++++++++++++++++++++++++------------
>>   1 file changed, 76 insertions(+), 30 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> Nits below
> 
>>
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 9b5512f086..7457d54739 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -1080,36 +1080,6 @@ static efi_status_t EFIAPI efi_locate_handle_ext(
>>                          buffer_size, buffer));
>>   }
>>
>> -/*
>> - * Get the device path and handle of an device implementing a protocol.
>> - *
>> - * This function implements the LocateDevicePath service.
>> - * See the Unified Extensible Firmware Interface (UEFI) specification
>> - * for details.
> 
> Oh yes, I will get right on that now :-)
> 
>> - *
>> - * @protocol           GUID of the protocol
>> - * @device_path                device path
>> - * @device             handle of the device
>> - * @return             status code
>> - */
>> -static efi_status_t EFIAPI efi_locate_device_path(
>> -                       const efi_guid_t *protocol,
>> -                       struct efi_device_path **device_path,
>> -                       efi_handle_t *device)
>> -{
>> -       struct efi_object *efiobj;
>> -
>> -       EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
>> -
>> -       efiobj = efi_dp_find_obj(*device_path, device_path);
>> -       if (!efiobj)
>> -               return EFI_EXIT(EFI_NOT_FOUND);
>> -
>> -       *device = efiobj->handle;
>> -
>> -       return EFI_EXIT(EFI_SUCCESS);
>> -}
>> -
>>   /* Collapses configuration table entries, removing index i */
>>   static void efi_remove_configuration_table(int i)
>>   {
>> @@ -1813,6 +1783,82 @@ static efi_status_t EFIAPI efi_locate_protocol(const efi_guid_t *protocol,
>>          return EFI_EXIT(EFI_NOT_FOUND);
>>   }
>>
>> +/*
>> + * Get the device path and handle of an device implementing a protocol.
> 
> a device?
> 
>> + *
>> + * This function implements the LocateDevicePath service.
>> + * See the Unified Extensible Firmware Interface (UEFI) specification
>> + * for details.
>> + *
>> + * @protocol           GUID of the protocol
>> + * @device_path                device path
>> + * @device             handle of the device
>> + * @return             status code
>> + */
>> +static efi_status_t EFIAPI efi_locate_device_path(
>> +                       const efi_guid_t *protocol,
>> +                       struct efi_device_path **device_path,
>> +                       efi_handle_t *device)
>> +{
>> +       struct efi_device_path *dp;
>> +       size_t i;
>> +       struct efi_handler *handler;
>> +       efi_handle_t *handles;
>> +       size_t len, len_dp;
>> +       size_t len_best = 0;
>> +       efi_uintn_t no_handles;
> 
> Is there none?
> 
> I think num_handles is better, or handles_count.

In the UEFI spec this is the name of the parameter of LocateHandleBuffer 
and it is also the parameter name in efi_locate_handle_buffer.

I get confused when using different names for the same thing.

> 
>> +       u8 *remainder;
>> +       efi_status_t ret;
>> +
>> +       EFI_ENTRY("%pUl, %p, %p", protocol, device_path, device);
>> +
>> +       if (!protocol || !device_path || !*device_path || !device) {
>> +               ret = EFI_INVALID_PARAMETER;
>> +               goto out;
>> +       }
>> +
>> +       /* Find end of device path */
>> +       len = efi_dp_size(*device_path);
>> +
>> +       /* Get all handles implementing the protocol */
>> +       ret = EFI_CALL(efi_locate_handle_buffer(BY_PROTOCOL, protocol, NULL,
>> +                                               &no_handles, &handles));
>> +       if (ret != EFI_SUCCESS)
>> +               goto out;
>> +
>> +       for (i = 0; i < no_handles; ++i) {
>> +               /* Find the device path protocol */
>> +               ret = efi_search_protocol(handles[i], &efi_guid_device_path,
>> +                                         &handler);
>> +               if (ret != EFI_SUCCESS)
>> +                       continue;
>> +               dp = (struct efi_device_path *)handler->protocol_interface;
>> +               len_dp = efi_dp_size(dp);
>> +               /*
>> +                * This handle can only be a better fit
>> +                * if its device path length is longer then the best fit and
>> +                * if its device path length is shorter of equal the searched
>> +                * device path.
> 
> Format to ~77 cols?

I deliberately put the two conditions in separate lines to make the text 
easier to read. I could have added bullet points but that looked like 
overkill to me.

> 
>> +                */
>> +               if (len_dp <= len_best || len_dp > len)
>> +                       continue;
>> +               /* Check if dp is a subpath of device_path. */
> 
> Can you drop the period?

Sure.

Regards

Heinrich

> 
>> +               if (memcmp(*device_path, dp, len_dp))
>> +                       continue;
>> +               *device = handles[i];
>> +               len_best = len_dp;
>> +       }
>> +       if (len_best) {
>> +               remainder = (u8 *)*device_path + len_best;
>> +               *device_path = (struct efi_device_path *)remainder;
>> +               ret = EFI_SUCCESS;
>> +       } else {
>> +               ret = EFI_NOT_FOUND;
>> +       }
>> +out:
>> +       return EFI_EXIT(ret);
>> +}
>> +
>>   /*
>>    * Install multiple protocol interfaces.
>>    *
>> --
>> 2.15.0
>>
> Regards,
> Simon
> 

[U-Boot] [PATCH v2 06/18] efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL

[Heinrich Schuchardt]

Provide a test for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL protocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_selftest/Makefile                  |   3 +
 lib/efi_selftest/efi_selftest_devicepath.c | 340 +++++++++++++++++++++++++++++
 2 files changed, 343 insertions(+)
 create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index 1851c17db6..d280eca5c3 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -11,6 +11,8 @@ CFLAGS_efi_selftest.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)
+CFLAGS_efi_selftest_devicepathe.o := $(CFLAGS_EFI)
+CFLAGS_REMOVE_efi_selftest_devicepath.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
 CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
 CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
@@ -33,6 +35,7 @@ CFLAGS_REMOVE_efi_selftest_watchdog.o := $(CFLAGS_NON_EFI)
 obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \
 efi_selftest.o \
 efi_selftest_console.o \
+efi_selftest_devicepath.o \
 efi_selftest_events.o \
 efi_selftest_exitbootservices.o \
 efi_selftest_gop.o \
diff --git a/lib/efi_selftest/efi_selftest_devicepath.c b/lib/efi_selftest/efi_selftest_devicepath.c
new file mode 100644
index 0000000000..6c6185c050
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_devicepath.c
@@ -0,0 +1,340 @@
+/*
+ * efi_selftest_devicepath
+ *
+ * Copyright (c) 2017 Heinrich Schuchardt <xypron.glpk@gmx.de>
+ *
+ * SPDX-License-Identifier:     GPL-2.0+
+ *
+ * This unit test checks the following protocol services:
+ * DevicePathToText
+ */
+
+#include <efi_selftest.h>
+
+static struct efi_boot_services *boottime;
+
+static efi_handle_t handle1;
+static efi_handle_t handle2;
+static efi_handle_t handle3;
+
+struct interface {
+	void (EFIAPI * inc)(void);
+} interface;
+
+static efi_guid_t guid_protocol =
+	EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+		 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0x7d);
+
+static efi_guid_t guid_vendor1 =
+	EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+		 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xb1);
+
+static efi_guid_t guid_vendor2 =
+	EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+		 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xa2);
+
+static efi_guid_t guid_vendor3 =
+	EFI_GUID(0xdbca4c98, 0x6cb0, 0x694d,
+		 0x08, 0x72, 0x81, 0x9c, 0x65, 0x0c, 0xbb, 0xc3);
+
+static u8 *dp1;
+static u8 *dp2;
+static u8 *dp3;
+
+struct efi_device_path_to_text_protocol *device_path_to_text;
+
+/*
+ * Setup unit test.
+ *
+ * Create three handles. Install a new protocol on two of them and
+ * provice device paths.
+ *
+ * handle1
+ *   guid interface
+ * handle2
+ *   guid interface
+ * handle3
+ *
+ * @handle:	handle of the loaded image
+ * @systable:	system table
+ */
+static int setup(const efi_handle_t img_handle,
+		 const struct efi_system_table *systable)
+{
+	struct efi_device_path_vendor vendor_node;
+	struct efi_device_path end_node;
+	efi_status_t ret;
+
+	boottime = systable->boottime;
+
+	ret = boottime->locate_protocol(&efi_guid_device_path_to_text_protocol,
+					NULL, (void **)&device_path_to_text);
+	if (ret != EFI_SUCCESS) {
+		device_path_to_text = NULL;
+		efi_st_printf(
+			"Device path to text protocol is not available.\n");
+		return EFI_ST_FAILURE;
+	}
+
+	ret = boottime->allocate_pool(EFI_LOADER_DATA,
+				      sizeof(struct efi_device_path_vendor) +
+				      sizeof(struct efi_device_path),
+				      (void **)&dp1);
+	if (ret != EFI_SUCCESS)
+		goto out_of_memory;
+
+	ret = boottime->allocate_pool(EFI_LOADER_DATA, 2 *
+				      sizeof(struct efi_device_path_vendor) +
+				      sizeof(struct efi_device_path),
+				      (void **)&dp2);
+	if (ret != EFI_SUCCESS)
+		goto out_of_memory;
+
+	ret = boottime->allocate_pool(EFI_LOADER_DATA, 3 *
+				      sizeof(struct efi_device_path_vendor) +
+				      sizeof(struct efi_device_path),
+				      (void **)&dp3);
+	if (ret != EFI_SUCCESS)
+		goto out_of_memory;
+
+	vendor_node.dp.type = DEVICE_PATH_TYPE_HARDWARE_DEVICE;
+	vendor_node.dp.sub_type = DEVICE_PATH_SUB_TYPE_VENDOR;
+	vendor_node.dp.length = sizeof(struct efi_device_path_vendor);
+
+	boottime->copy_mem(&vendor_node.guid, &guid_vendor1,
+			   sizeof(efi_guid_t));
+	boottime->copy_mem(dp1, &vendor_node,
+			   sizeof(struct efi_device_path_vendor));
+	boottime->copy_mem(dp2, &vendor_node,
+			   sizeof(struct efi_device_path_vendor));
+	boottime->copy_mem(dp3, &vendor_node,
+			   sizeof(struct efi_device_path_vendor));
+
+	boottime->copy_mem(&vendor_node.guid, &guid_vendor2,
+			   sizeof(efi_guid_t));
+	boottime->copy_mem(dp2 + sizeof(struct efi_device_path_vendor),
+			   &vendor_node, sizeof(struct efi_device_path_vendor));
+	boottime->copy_mem(dp3 + sizeof(struct efi_device_path_vendor),
+			   &vendor_node, sizeof(struct efi_device_path_vendor));
+
+	boottime->copy_mem(&vendor_node.guid, &guid_vendor3,
+			   sizeof(efi_guid_t));
+	boottime->copy_mem(dp3 + 2 * sizeof(struct efi_device_path_vendor),
+			   &vendor_node, sizeof(struct efi_device_path_vendor));
+
+	end_node.type = DEVICE_PATH_TYPE_END;
+	end_node.sub_type = DEVICE_PATH_SUB_TYPE_END;
+	end_node.length = sizeof(struct efi_device_path);
+	boottime->copy_mem(dp1 + sizeof(struct efi_device_path_vendor),
+			   &end_node, sizeof(struct efi_device_path));
+	boottime->copy_mem(dp2 + 2 * sizeof(struct efi_device_path_vendor),
+			   &end_node, sizeof(struct efi_device_path));
+	boottime->copy_mem(dp3 + 3 * sizeof(struct efi_device_path_vendor),
+			   &end_node, sizeof(struct efi_device_path));
+
+	ret = boottime->install_protocol_interface(&handle1,
+						   &efi_guid_device_path,
+						   EFI_NATIVE_INTERFACE,
+						   dp1);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->install_protocol_interface(&handle1,
+						   &guid_protocol,
+						   EFI_NATIVE_INTERFACE,
+						   &interface);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->install_protocol_interface(&handle2,
+						   &efi_guid_device_path,
+						   EFI_NATIVE_INTERFACE,
+						   dp2);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->install_protocol_interface(&handle2,
+						   &guid_protocol,
+						   EFI_NATIVE_INTERFACE,
+						   &interface);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	ret = boottime->install_protocol_interface(&handle3,
+						   &efi_guid_device_path,
+						   EFI_NATIVE_INTERFACE,
+						   dp3);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("InstallProtocolInterface failed\n");
+		return EFI_ST_FAILURE;
+	}
+	return EFI_ST_SUCCESS;
+
+out_of_memory:
+	efi_st_error("Out of memory\n");
+	return EFI_ST_FAILURE;
+}
+
+/*
+ * Tear down unit test.
+ *
+ */
+static int teardown(void)
+{
+	efi_status_t ret;
+
+	ret = boottime->uninstall_protocol_interface(&handle1,
+						     &efi_guid_device_path,
+						     dp1);
+	if (ret != EFI_SUCCESS)
+		efi_st_todo("UninstallProtocolInterface failed\n");
+	ret = boottime->uninstall_protocol_interface(&handle1,
+						     &guid_protocol,
+						     &interface);
+	if (ret != EFI_SUCCESS)
+		efi_st_todo("UninstallProtocolInterface failed\n");
+	ret = boottime->uninstall_protocol_interface(&handle2,
+						     &efi_guid_device_path,
+						     dp2);
+	if (ret != EFI_SUCCESS)
+		efi_st_todo("UninstallProtocolInterface failed\n");
+	ret = boottime->uninstall_protocol_interface(&handle2,
+						     &guid_protocol,
+						     &interface);
+	if (ret != EFI_SUCCESS)
+		efi_st_todo("UninstallProtocolInterface failed\n");
+	ret = boottime->uninstall_protocol_interface(&handle3,
+						     &efi_guid_device_path,
+						     dp3);
+	if (ret != EFI_SUCCESS)
+		efi_st_todo("UninstallProtocolInterface failed\n");
+	if (dp1) {
+		ret = boottime->free_pool(dp1);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("FreePool failed\n");
+			return EFI_ST_FAILURE;
+		}
+	}
+	if (dp2) {
+		ret = boottime->free_pool(dp2);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("FreePool failed\n");
+			return EFI_ST_FAILURE;
+		}
+	}
+	if (dp3) {
+		ret = boottime->free_pool(dp3);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("FreePool failed\n");
+			return EFI_ST_FAILURE;
+		}
+	}
+	return EFI_ST_SUCCESS;
+}
+
+/*
+ * Execute unit test.
+ *
+ */
+static int execute(void)
+{
+	struct efi_device_path *remaining_dp;
+	void *handle;
+	/*
+	 * This device path node ends with the letter 't' of 'u-boot'.
+	 * The following '.bin' does not belong to the node but is
+	 * helps to test the correct truncation.
+	 */
+	struct {
+		struct efi_device_path dp;
+		u16 text[12];
+	} __packed dp_node = {
+			{ DEVICE_PATH_TYPE_MEDIA_DEVICE,
+			  DEVICE_PATH_SUB_TYPE_FILE_PATH,
+			  sizeof(struct efi_device_path) + 12},
+			L"u-boot.bin",
+		};
+	u16 *string;
+	efi_status_t ret;
+
+	/* Test ConvertDevicePathToText */
+	string = device_path_to_text->convert_device_path_to_text(
+			(struct efi_device_path *)dp2, true, false);
+	if (!string) {
+		efi_st_error("ConvertDevicePathToText failed\n");
+		return EFI_ST_FAILURE;
+	}
+	efi_st_printf("dp2: %ps\n", string);
+	if (efi_st_strcmp_16_8(
+		string,
+		"/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbb1)/VenHw(dbca4c98-6cb0-694d-0872-819c650cbba2)")
+	    ) {
+		efi_st_error("Incorrect text from ConvertDevicePathToText\n");
+		return EFI_ST_FAILURE;
+	}
+
+	ret = boottime->free_pool(string);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("FreePool failed\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/* Test ConvertDeviceNodeToText */
+	string = device_path_to_text->convert_device_node_to_text(
+			(struct efi_device_path *)&dp_node, true, false);
+	if (!string) {
+		efi_st_error("ConvertDeviceNodeToText failed\n");
+		return EFI_ST_FAILURE;
+	}
+	efi_st_printf("dp_node: %ps\n", string);
+	ret = boottime->free_pool(string);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("FreePool failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (efi_st_strcmp_16_8(string, "u-boot")) {
+		efi_st_error(
+			"Incorrect conversion by ConvertDeviceNodeToText\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/* Test LocateDevicePath */
+	remaining_dp = (struct efi_device_path *)dp3;
+	ret = boottime->locate_device_path(&guid_protocol, &remaining_dp,
+					   &handle);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("LocateDevicePath failed\n");
+		return EFI_ST_FAILURE;
+	}
+	if (handle != handle2) {
+		efi_st_error("LocateDevicePath returned wrong handle\n");
+		return EFI_ST_FAILURE;
+	}
+	string = device_path_to_text->convert_device_path_to_text(remaining_dp,
+								  true, false);
+	if (!string) {
+		efi_st_error("ConvertDevicePathToText failed\n");
+		return EFI_ST_FAILURE;
+	}
+	efi_st_printf("remaining device path: %ps\n", string);
+	if (efi_st_strcmp_16_8(string,
+			       "/VenHw(dbca4c98-6cb0-694d-0872-819c650cbbc3)")
+	    ) {
+		efi_st_error("LocateDevicePath: wrong remaining device path\n");
+		return EFI_ST_FAILURE;
+	}
+
+	return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(devicepath) = {
+	.name = "device path",
+	.phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+	.setup = setup,
+	.execute = execute,
+	.teardown = teardown,
+};
-- 
2.15.0

[U-Boot] [PATCH v2 06/18] efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Provide a test for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL protocol.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_selftest/Makefile                  |   3 +
>  lib/efi_selftest/efi_selftest_devicepath.c | 340 +++++++++++++++++++++++++++++
>  2 files changed, 343 insertions(+)
>  create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c

Reviewed-by: Simon Glass <sjg@chromium.org>

I wish we could just use 0 instead of EFI_SUCCESS

and if (!xx) to check for success.

The source code is looking too much like EFI for my liking!

Regards,
Simon

[U-Boot] [PATCH v2 06/18] efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL

[Heinrich Schuchardt]

On 11/17/2017 03:06 PM, Simon Glass wrote:
> On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> Provide a test for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL protocol.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>          no change
>> ---
>>   lib/efi_selftest/Makefile                  |   3 +
>>   lib/efi_selftest/efi_selftest_devicepath.c | 340 +++++++++++++++++++++++++++++
>>   2 files changed, 343 insertions(+)
>>   create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> I wish we could just use 0 instead of EFI_SUCCESS

0 does not convey any meaning to me. This is why I tend to use

* NULL instead of 0 and
* EINVAL instead of 22.

But obviously this is a matter of taste.

Regards

Heinrich

> 
> and if (!xx) to check for success.
> 
> The source code is looking too much like EFI for my liking!
> 
> Regards,
> Simon
> 

[U-Boot] [PATCH v2 06/18] efi_selftest: test EFI_DEVICE_PATH_TO_TEXT_PROTOCOL

[Simon Glass]

Hi Heinrich,

On 17 November 2017 at 11:28, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>
> On 11/17/2017 03:06 PM, Simon Glass wrote:
>>
>> On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>
>>> Provide a test for the EFI_DEVICE_PATH_TO_TEXT_PROTOCOL protocol.
>>>
>>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>>> ---
>>> v2
>>>          no change
>>> ---
>>>   lib/efi_selftest/Makefile                  |   3 +
>>>   lib/efi_selftest/efi_selftest_devicepath.c | 340 +++++++++++++++++++++++++++++
>>>   2 files changed, 343 insertions(+)
>>>   create mode 100644 lib/efi_selftest/efi_selftest_devicepath.c
>>
>>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>
>> I wish we could just use 0 instead of EFI_SUCCESS
>
>
> 0 does not convey any meaning to me. This is why I tend to use
>
> * NULL instead of 0 and
> * EINVAL instead of 22.
>
> But obviously this is a matter of taste.

Sure, it's no big deal to me. Certainly in drive rmodel I have used 0
for success. To me, EFI_SUCCESS is just UEFI code style bleeding into
U-Boot :-)

>
> Regards
>
> Heinrich
>
>
>>
>> and if (!xx) to check for success.
>>
>> The source code is looking too much like EFI for my liking!
>>
>> Regards,
>> Simon
>>
>

Regards,
Simon

[U-Boot] [PATCH v2 07/18] efi_loader: efi_disk: use efi_add_protocol

[Heinrich Schuchardt]

Use efi_add_protocol to install protocols.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_disk.c | 39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index c6f0d732c1..1d6cf3122f 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -213,33 +213,40 @@ static void efi_disk_add_dev(const char *name,
 			     unsigned int part)
 {
 	struct efi_disk_obj *diskobj;
+	efi_status_t ret;
 
 	/* Don't add empty devices */
 	if (!desc->lba)
 		return;
 
 	diskobj = calloc(1, sizeof(*diskobj));
-	if (!diskobj) {
-		printf("ERROR: Out of memory\n");
-		return;
-	}
+	if (!diskobj)
+		goto out_of_memory;
+
+	/* Hook up to the device list */
+	list_add_tail(&diskobj->parent.link, &efi_obj_list);
 
 	/* Fill in object data */
 	diskobj->dp = efi_dp_from_part(desc, part);
 	diskobj->part = part;
-	diskobj->parent.protocols[0].guid = &efi_block_io_guid;
-	diskobj->parent.protocols[0].protocol_interface = &diskobj->ops;
-	diskobj->parent.protocols[1].guid = &efi_guid_device_path;
-	diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
+	diskobj->parent.handle = diskobj;
+	ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid,
+			       &diskobj->ops);
+	if (ret != EFI_SUCCESS)
+		goto out_of_memory;
+	ret = efi_add_protocol(diskobj->parent.handle, &efi_guid_device_path,
+			       diskobj->dp);
+	if (ret != EFI_SUCCESS)
+		goto out_of_memory;
 	if (part >= 1) {
 		diskobj->volume = efi_simple_file_system(desc, part,
 							 diskobj->dp);
-		diskobj->parent.protocols[2].guid =
-			&efi_simple_file_system_protocol_guid;
-		diskobj->parent.protocols[2].protocol_interface =
-			diskobj->volume;
+		ret = efi_add_protocol(diskobj->parent.handle,
+				       &efi_simple_file_system_protocol_guid,
+				       &diskobj->volume);
+		if (ret != EFI_SUCCESS)
+			goto out_of_memory;
 	}
-	diskobj->parent.handle = diskobj;
 	diskobj->ops = block_io_disk_template;
 	diskobj->ifname = if_typename;
 	diskobj->dev_index = dev_index;
@@ -253,9 +260,9 @@ static void efi_disk_add_dev(const char *name,
 	diskobj->media.io_align = desc->blksz;
 	diskobj->media.last_block = desc->lba - offset;
 	diskobj->ops.media = &diskobj->media;
-
-	/* Hook up to the device list */
-	list_add_tail(&diskobj->parent.link, &efi_obj_list);
+	return;
+out_of_memory:
+	printf("ERROR: Out of memory\n");
 }
 
 static int efi_disk_create_eltorito(struct blk_desc *desc,
-- 
2.15.0

[U-Boot] [PATCH v2 07/18] efi_loader: efi_disk: use efi_add_protocol

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Use efi_add_protocol to install protocols.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_disk.c | 39 +++++++++++++++++++++++----------------
>  1 file changed, 23 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

See below.

>
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index c6f0d732c1..1d6cf3122f 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -213,33 +213,40 @@ static void efi_disk_add_dev(const char *name,
>                              unsigned int part)
>  {
>         struct efi_disk_obj *diskobj;
> +       efi_status_t ret;
>
>         /* Don't add empty devices */
>         if (!desc->lba)
>                 return;
>
>         diskobj = calloc(1, sizeof(*diskobj));
> -       if (!diskobj) {
> -               printf("ERROR: Out of memory\n");
> -               return;
> -       }
> +       if (!diskobj)
> +               goto out_of_memory;
> +
> +       /* Hook up to the device list */
> +       list_add_tail(&diskobj->parent.link, &efi_obj_list);
>
>         /* Fill in object data */
>         diskobj->dp = efi_dp_from_part(desc, part);
>         diskobj->part = part;
> -       diskobj->parent.protocols[0].guid = &efi_block_io_guid;
> -       diskobj->parent.protocols[0].protocol_interface = &diskobj->ops;
> -       diskobj->parent.protocols[1].guid = &efi_guid_device_path;
> -       diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
> +       diskobj->parent.handle = diskobj;
> +       ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid,
> +                              &diskobj->ops);
> +       if (ret != EFI_SUCCESS)
> +               goto out_of_memory;
> +       ret = efi_add_protocol(diskobj->parent.handle, &efi_guid_device_path,
> +                              diskobj->dp);
> +       if (ret != EFI_SUCCESS)
> +               goto out_of_memory;
>         if (part >= 1) {
>                 diskobj->volume = efi_simple_file_system(desc, part,
>                                                          diskobj->dp);
> -               diskobj->parent.protocols[2].guid =
> -                       &efi_simple_file_system_protocol_guid;
> -               diskobj->parent.protocols[2].protocol_interface =
> -                       diskobj->volume;
> +               ret = efi_add_protocol(diskobj->parent.handle,
> +                                      &efi_simple_file_system_protocol_guid,
> +                                      &diskobj->volume);
> +               if (ret != EFI_SUCCESS)
> +                       goto out_of_memory;
>         }
> -       diskobj->parent.handle = diskobj;
>         diskobj->ops = block_io_disk_template;
>         diskobj->ifname = if_typename;
>         diskobj->dev_index = dev_index;
> @@ -253,9 +260,9 @@ static void efi_disk_add_dev(const char *name,
>         diskobj->media.io_align = desc->blksz;
>         diskobj->media.last_block = desc->lba - offset;
>         diskobj->ops.media = &diskobj->media;
> -
> -       /* Hook up to the device list */
> -       list_add_tail(&diskobj->parent.link, &efi_obj_list);
> +       return;
> +out_of_memory:
> +       printf("ERROR: Out of memory\n");

Can we not return this error?

>  }
>
>  static int efi_disk_create_eltorito(struct blk_desc *desc,
> --
> 2.15.0
>

[U-Boot] [PATCH v2 07/18] efi_loader: efi_disk: use efi_add_protocol

[Heinrich Schuchardt]

On 11/17/2017 03:06 PM, Simon Glass wrote:
> On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> Use efi_add_protocol to install protocols.
>>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>          no change
>> ---
>>   lib/efi_loader/efi_disk.c | 39 +++++++++++++++++++++++----------------
>>   1 file changed, 23 insertions(+), 16 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
> 
> See below.
> 
>>
>> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
>> index c6f0d732c1..1d6cf3122f 100644
>> --- a/lib/efi_loader/efi_disk.c
>> +++ b/lib/efi_loader/efi_disk.c
>> @@ -213,33 +213,40 @@ static void efi_disk_add_dev(const char *name,
>>                               unsigned int part)
>>   {
>>          struct efi_disk_obj *diskobj;
>> +       efi_status_t ret;
>>
>>          /* Don't add empty devices */
>>          if (!desc->lba)
>>                  return;
>>
>>          diskobj = calloc(1, sizeof(*diskobj));
>> -       if (!diskobj) {
>> -               printf("ERROR: Out of memory\n");
>> -               return;
>> -       }
>> +       if (!diskobj)
>> +               goto out_of_memory;
>> +
>> +       /* Hook up to the device list */
>> +       list_add_tail(&diskobj->parent.link, &efi_obj_list);
>>
>>          /* Fill in object data */
>>          diskobj->dp = efi_dp_from_part(desc, part);
>>          diskobj->part = part;
>> -       diskobj->parent.protocols[0].guid = &efi_block_io_guid;
>> -       diskobj->parent.protocols[0].protocol_interface = &diskobj->ops;
>> -       diskobj->parent.protocols[1].guid = &efi_guid_device_path;
>> -       diskobj->parent.protocols[1].protocol_interface = diskobj->dp;
>> +       diskobj->parent.handle = diskobj;
>> +       ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid,
>> +                              &diskobj->ops);
>> +       if (ret != EFI_SUCCESS)
>> +               goto out_of_memory;
>> +       ret = efi_add_protocol(diskobj->parent.handle, &efi_guid_device_path,
>> +                              diskobj->dp);
>> +       if (ret != EFI_SUCCESS)
>> +               goto out_of_memory;
>>          if (part >= 1) {
>>                  diskobj->volume = efi_simple_file_system(desc, part,
>>                                                           diskobj->dp);
>> -               diskobj->parent.protocols[2].guid =
>> -                       &efi_simple_file_system_protocol_guid;
>> -               diskobj->parent.protocols[2].protocol_interface =
>> -                       diskobj->volume;
>> +               ret = efi_add_protocol(diskobj->parent.handle,
>> +                                      &efi_simple_file_system_protocol_guid,
>> +                                      &diskobj->volume);
>> +               if (ret != EFI_SUCCESS)
>> +                       goto out_of_memory;
>>          }
>> -       diskobj->parent.handle = diskobj;
>>          diskobj->ops = block_io_disk_template;
>>          diskobj->ifname = if_typename;
>>          diskobj->dev_index = dev_index;
>> @@ -253,9 +260,9 @@ static void efi_disk_add_dev(const char *name,
>>          diskobj->media.io_align = desc->blksz;
>>          diskobj->media.last_block = desc->lba - offset;
>>          diskobj->ops.media = &diskobj->media;
>> -
>> -       /* Hook up to the device list */
>> -       list_add_tail(&diskobj->parent.link, &efi_obj_list);
>> +       return;
>> +out_of_memory:
>> +       printf("ERROR: Out of memory\n");
> 
> Can we not return this error?

This get's called when executing the bootefi command before the image is 
loaded.
Is there really anything better than writing a message?
Anyway if we have that little memory when coming from the U-Boot command 
line we are in deep trouble.

Thanks for all your reviewing.

Best regards

Heinrich

> 
>>   }
>>
>>   static int efi_disk_create_eltorito(struct blk_desc *desc,
>> --
>> 2.15.0
>>
> 

[U-Boot] [PATCH v2 08/18] efi_loader: efi_net: use efi_add_protocol

[Heinrich Schuchardt]

Use efi_add_protocol to add protocols.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_net.c | 34 ++++++++++++++++++++--------------
 1 file changed, 20 insertions(+), 14 deletions(-)

diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index a7b101e830..8b2f682351 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -292,20 +292,26 @@ int efi_net_register(void)
 
 	/* We only expose the "active" eth device, so one is enough */
 	netobj = calloc(1, sizeof(*netobj));
-	if (!netobj) {
-		printf("ERROR: Out of memory\n");
-		return 1;
-	}
+	if (!netobj)
+		goto out_of_memory;
+
+	/* Hook net up to the device list */
+	list_add_tail(&netobj->parent.link, &efi_obj_list);
 
 	/* Fill in object data */
-	netobj->parent.protocols[0].guid = &efi_net_guid;
-	netobj->parent.protocols[0].protocol_interface = &netobj->net;
-	netobj->parent.protocols[1].guid = &efi_guid_device_path;
-	netobj->parent.protocols[1].protocol_interface =
-		efi_dp_from_eth();
-	netobj->parent.protocols[2].guid = &efi_pxe_guid;
-	netobj->parent.protocols[2].protocol_interface = &netobj->pxe;
 	netobj->parent.handle = &netobj->net;
+	r = efi_add_protocol(netobj->parent.handle, &efi_net_guid,
+			     &netobj->net);
+	if (r != EFI_SUCCESS)
+		goto out_of_memory;
+	r = efi_add_protocol(netobj->parent.handle, &efi_guid_device_path,
+			     efi_dp_from_eth());
+	if (r != EFI_SUCCESS)
+		goto out_of_memory;
+	r = efi_add_protocol(netobj->parent.handle, &efi_pxe_guid,
+			     &netobj->pxe);
+	if (r != EFI_SUCCESS)
+		goto out_of_memory;
 	netobj->net.revision = EFI_SIMPLE_NETWORK_PROTOCOL_REVISION;
 	netobj->net.start = efi_net_start;
 	netobj->net.stop = efi_net_stop;
@@ -330,9 +336,6 @@ int efi_net_register(void)
 	if (dhcp_ack)
 		netobj->pxe_mode.dhcp_ack = *dhcp_ack;
 
-	/* Hook net up to the device list */
-	list_add_tail(&netobj->parent.link, &efi_obj_list);
-
 	/*
 	 * Create WaitForPacket event.
 	 */
@@ -365,4 +368,7 @@ int efi_net_register(void)
 	}
 
 	return 0;
+out_of_memory:
+	printf("ERROR: Out of memory\n");
+	return 1;
 }
-- 
2.15.0

[U-Boot] [PATCH v2 08/18] efi_loader: efi_net: use efi_add_protocol

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Use efi_add_protocol to add protocols.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_net.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

Same question about missing error-checking.

[U-Boot] [PATCH v2 09/18] efi_loader: efi_gop: use efi_add_protocol

[Heinrich Schuchardt]

Use efi_add_protocol to add protocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_gop.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index 7370eeee37..7b74d6ef33 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -132,6 +132,7 @@ int efi_gop_register(void)
 	u32 bpix, col, row;
 	u64 fb_base, fb_size;
 	void *fb;
+	efi_status_t ret;
 
 #ifdef CONFIG_DM_VIDEO
 	struct udevice *vdev;
@@ -178,10 +179,17 @@ int efi_gop_register(void)
 		return 1;
 	}
 
+	/* Hook up to the device list */
+	list_add_tail(&gopobj->parent.link, &efi_obj_list);
+
 	/* Fill in object data */
-	gopobj->parent.protocols[0].guid = &efi_gop_guid;
-	gopobj->parent.protocols[0].protocol_interface = &gopobj->ops;
 	gopobj->parent.handle = &gopobj->ops;
+	ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid,
+			       &gopobj->ops);
+	if (ret != EFI_SUCCESS) {
+		printf("ERROR: Out of memory\n");
+		return 1;
+	}
 	gopobj->ops.query_mode = gop_query_mode;
 	gopobj->ops.set_mode = gop_set_mode;
 	gopobj->ops.blt = gop_blt;
@@ -210,8 +218,5 @@ int efi_gop_register(void)
 	gopobj->bpix = bpix;
 	gopobj->fb = fb;
 
-	/* Hook up to the device list */
-	list_add_tail(&gopobj->parent.link, &efi_obj_list);
-
 	return 0;
 }
-- 
2.15.0

[U-Boot] [PATCH v2 09/18] efi_loader: efi_gop: use efi_add_protocol

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Use efi_add_protocol to add protocol.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_gop.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 10/18] efi_loader: simplify efi_open_protocol

[Heinrich Schuchardt]

Use function efi_search_protocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_boottime.c | 36 ++++++------------------------------
 1 file changed, 6 insertions(+), 30 deletions(-)

diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 7457d54739..2b3db162a1 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2051,8 +2051,7 @@ static efi_status_t EFIAPI efi_open_protocol(
 			void **protocol_interface, void *agent_handle,
 			void *controller_handle, uint32_t attributes)
 {
-	struct list_head *lhandle;
-	int i;
+	struct efi_handler *handler;
 	efi_status_t r = EFI_INVALID_PARAMETER;
 
 	EFI_ENTRY("%p, %pUl, %p, %p, %p, 0x%x", handle, protocol,
@@ -2065,8 +2064,6 @@ static efi_status_t EFIAPI efi_open_protocol(
 		goto out;
 	}
 
-	EFI_PRINT_GUID("protocol", protocol);
-
 	switch (attributes) {
 	case EFI_OPEN_PROTOCOL_BY_HANDLE_PROTOCOL:
 	case EFI_OPEN_PROTOCOL_GET_PROTOCOL:
@@ -2087,33 +2084,12 @@ static efi_status_t EFIAPI efi_open_protocol(
 		goto out;
 	}
 
-	list_for_each(lhandle, &efi_obj_list) {
-		struct efi_object *efiobj;
-		efiobj = list_entry(lhandle, struct efi_object, link);
-
-		if (efiobj->handle != handle)
-			continue;
-
-		for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-			struct efi_handler *handler = &efiobj->protocols[i];
-			const efi_guid_t *hprotocol = handler->guid;
-			if (!hprotocol)
-				continue;
-			if (!guidcmp(hprotocol, protocol)) {
-				if (attributes !=
-				    EFI_OPEN_PROTOCOL_TEST_PROTOCOL) {
-					*protocol_interface =
-						handler->protocol_interface;
-				}
-				r = EFI_SUCCESS;
-				goto out;
-			}
-		}
-		goto unsupported;
-	}
+	r = efi_search_protocol(handle, protocol, &handler);
+	if (r != EFI_SUCCESS)
+		goto out;
 
-unsupported:
-	r = EFI_UNSUPPORTED;
+	if (attributes != EFI_OPEN_PROTOCOL_TEST_PROTOCOL)
+		*protocol_interface = handler->protocol_interface;
 out:
 	return EFI_EXIT(r);
 }
-- 
2.15.0

[U-Boot] [PATCH v2 10/18] efi_loader: simplify efi_open_protocol

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Use function efi_search_protocol.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_boottime.c | 36 ++++++------------------------------
>  1 file changed, 6 insertions(+), 30 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 11/18] efi_loader: simplify find_obj

[Heinrich Schuchardt]

Use function efi_search_protocol.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_loader/efi_device_path.c | 43 ++++++++++++++++++----------------------
 1 file changed, 19 insertions(+), 24 deletions(-)

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index 024877161b..98034216da 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -128,32 +128,27 @@ static struct efi_object *find_obj(struct efi_device_path *dp, bool short_path,
 	struct efi_object *efiobj;
 
 	list_for_each_entry(efiobj, &efi_obj_list, link) {
-		int i;
-
-		for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-			struct efi_handler *handler = &efiobj->protocols[i];
-			struct efi_device_path *obj_dp;
-
-			if (!handler->guid)
-				break;
-
-			if (guidcmp(handler->guid, &efi_guid_device_path))
-				continue;
-
-			obj_dp = handler->protocol_interface;
-
-			do {
-				if (efi_dp_match(dp, obj_dp) == 0) {
-					if (rem) {
-						*rem = ((void *)dp) +
-							efi_dp_size(obj_dp);
-					}
-					return efiobj;
+		struct efi_handler *handler;
+		struct efi_device_path *obj_dp;
+		efi_status_t ret;
+
+		ret = efi_search_protocol(efiobj->handle,
+					  &efi_guid_device_path, &handler);
+		if (ret != EFI_SUCCESS)
+			continue;
+		obj_dp = handler->protocol_interface;
+
+		do {
+			if (efi_dp_match(dp, obj_dp) == 0) {
+				if (rem) {
+					*rem = ((void *)dp) +
+						efi_dp_size(obj_dp);
 				}
+				return efiobj;
+			}
 
-				obj_dp = shorten_path(efi_dp_next(obj_dp));
-			} while (short_path && obj_dp);
-		}
+			obj_dp = shorten_path(efi_dp_next(obj_dp));
+		} while (short_path && obj_dp);
 	}
 
 	return NULL;
-- 
2.15.0

[U-Boot] [PATCH v2 11/18] efi_loader: simplify find_obj

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Use function efi_search_protocol.

Use efi_search_protocol().

>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_loader/efi_device_path.c | 43 ++++++++++++++++++----------------------
>  1 file changed, 19 insertions(+), 24 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 12/18] efi_loader: manage protocols in a linked list

[Heinrich Schuchardt]

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 include/efi_loader.h          |   6 ++-
 lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++----------------------
 lib/efi_loader/efi_disk.c     |   1 +
 lib/efi_loader/efi_gop.c      |   1 +
 lib/efi_loader/efi_net.c      |   1 +
 5 files changed, 58 insertions(+), 58 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index e1f0af3496..a73bbc1269 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -101,6 +101,8 @@ extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
  * interface (usually a struct with callback functions), this struct maps the
  * protocol GUID to the respective protocol interface */
 struct efi_handler {
+	/* Link to the list of protocols of a handle */
+	struct list_head link;
 	const efi_guid_t *guid;
 	void *protocol_interface;
 };
@@ -115,8 +117,8 @@ struct efi_handler {
 struct efi_object {
 	/* Every UEFI object is part of a global object list */
 	struct list_head link;
-	/* We support up to 16 "protocols" an object can be accessed through */
-	struct efi_handler protocols[16];
+	/* The list of protocols */
+	struct list_head protocols;
 	/* The object spawner can either use this for data or as identifier */
 	void *handle;
 };
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 2b3db162a1..cee0cb1390 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -339,6 +339,7 @@ efi_status_t efi_create_handle(void **handle)
 		return r;
 	memset(obj, 0, sizeof(struct efi_object));
 	obj->handle = obj;
+	INIT_LIST_HEAD(&obj->protocols);
 	list_add_tail(&obj->link, &efi_obj_list);
 	*handle = obj;
 	return r;
@@ -715,18 +716,17 @@ efi_status_t efi_search_protocol(const void *handle,
 				 struct efi_handler **handler)
 {
 	struct efi_object *efiobj;
-	size_t i;
-	struct efi_handler *protocol;
+	struct list_head *lhandle;
 
 	if (!handle || !protocol_guid)
 		return EFI_INVALID_PARAMETER;
 	efiobj = efi_search_obj(handle);
 	if (!efiobj)
 		return EFI_INVALID_PARAMETER;
-	for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-		protocol = &efiobj->protocols[i];
-		if (!protocol->guid)
-			continue;
+	list_for_each(lhandle, &efiobj->protocols) {
+		struct efi_handler *protocol;
+
+		protocol = list_entry(lhandle, struct efi_handler, link);
 		if (!guidcmp(protocol->guid, protocol_guid)) {
 			if (handler)
 				*handler = protocol;
@@ -750,7 +750,6 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
 	struct efi_object *efiobj;
 	struct efi_handler *handler;
 	efi_status_t ret;
-	size_t i;
 
 	efiobj = efi_search_obj(handle);
 	if (!efiobj)
@@ -761,16 +760,10 @@ efi_status_t efi_add_protocol(const void *handle, const efi_guid_t *protocol,
 	handler = calloc(1, sizeof(struct efi_handler));
 	if (!handler)
 		return EFI_OUT_OF_RESOURCES;
-	/* Install protocol in first empty slot. */
-	for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-		handler = &efiobj->protocols[i];
-		if (handler->guid)
-			continue;
-		handler->guid = protocol;
-		handler->protocol_interface = protocol_interface;
-		return EFI_SUCCESS;
-	}
-	return EFI_OUT_OF_RESOURCES;
+	handler->guid = protocol;
+	handler->protocol_interface = protocol_interface;
+	list_add_tail(&handler->link, &efiobj->protocols);
+	return EFI_SUCCESS;
 }
 
 /*
@@ -790,10 +783,10 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
 	ret = efi_search_protocol(handle, protocol, &handler);
 	if (ret != EFI_SUCCESS)
 		return ret;
-	if (handler->protocol_interface != protocol_interface)
-		return EFI_NOT_FOUND;
-	handler->guid = NULL;
-	handler->protocol_interface = NULL;
+	if (guidcmp(handler->guid, protocol))
+		return EFI_INVALID_PARAMETER;
+	list_del(&handler->link);
+	free(handler);
 	return EFI_SUCCESS;
 }
 
@@ -806,17 +799,22 @@ efi_status_t efi_remove_protocol(const void *handle, const efi_guid_t *protocol,
 efi_status_t efi_remove_all_protocols(const void *handle)
 {
 	struct efi_object *efiobj;
-	struct efi_handler *handler;
-	size_t i;
+	struct list_head *lhandle;
+	struct list_head *pos;
 
 	efiobj = efi_search_obj(handle);
 	if (!efiobj)
 		return EFI_INVALID_PARAMETER;
+	list_for_each_safe(lhandle, pos, &efiobj->protocols) {
+		struct efi_handler *protocol;
+		efi_status_t ret;
 
-	for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-		handler = &efiobj->protocols[i];
-		handler->guid = NULL;
-		handler->protocol_interface = NULL;
+		protocol = list_entry(lhandle, struct efi_handler, link);
+
+		ret = efi_remove_protocol(handle, protocol->guid,
+					  protocol->protocol_interface);
+		if (ret != EFI_SUCCESS)
+			return ret;
 	}
 	return EFI_SUCCESS;
 }
@@ -1171,6 +1169,7 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
 	if (device_path)
 		info->device_handle = efi_dp_find_obj(device_path, NULL);
 
+	INIT_LIST_HEAD(&obj->protocols);
 	list_add_tail(&obj->link, &efi_obj_list);
 	/*
 	 * When asking for the device path interface, return
@@ -1648,8 +1647,7 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
 {
 	unsigned long buffer_size;
 	struct efi_object *efiobj;
-	unsigned long i, j;
-	struct list_head *lhandle;
+	struct list_head *protocol_handle;
 	efi_status_t r;
 
 	EFI_ENTRY("%p, %p, %p", handle, protocol_buffer,
@@ -1660,36 +1658,33 @@ static efi_status_t EFIAPI efi_protocols_per_handle(void *handle,
 
 	*protocol_buffer = NULL;
 	*protocol_buffer_count = 0;
-	list_for_each(lhandle, &efi_obj_list) {
-		efiobj = list_entry(lhandle, struct efi_object, link);
 
-		if (efiobj->handle != handle)
-			continue;
+	efiobj = efi_search_obj(handle);
+	if (!efiobj)
+		return EFI_EXIT(EFI_INVALID_PARAMETER);
 
-		/* Count protocols */
-		for (i = 0; i < ARRAY_SIZE(efiobj->protocols); i++) {
-			if (efiobj->protocols[i].guid)
-				++*protocol_buffer_count;
-		}
-		/* Copy guids */
-		if (*protocol_buffer_count) {
-			buffer_size = sizeof(efi_guid_t *) *
-					*protocol_buffer_count;
-			r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES,
-					      buffer_size,
-					      (void **)protocol_buffer);
-			if (r != EFI_SUCCESS)
-				return EFI_EXIT(r);
-			j = 0;
-			for (i = 0; i < ARRAY_SIZE(efiobj->protocols); ++i) {
-				if (efiobj->protocols[i].guid) {
-					(*protocol_buffer)[j] = (void *)
-						efiobj->protocols[i].guid;
-					++j;
-				}
-			}
+	/* Count protocols */
+	list_for_each(protocol_handle, &efiobj->protocols) {
+		++*protocol_buffer_count;
+	}
+
+	/* Copy guids */
+	if (*protocol_buffer_count) {
+		size_t j = 0;
+
+		buffer_size = sizeof(efi_guid_t *) * *protocol_buffer_count;
+		r = efi_allocate_pool(EFI_ALLOCATE_ANY_PAGES, buffer_size,
+				      (void **)protocol_buffer);
+		if (r != EFI_SUCCESS)
+			return EFI_EXIT(r);
+		list_for_each(protocol_handle, &efiobj->protocols) {
+			struct efi_handler *protocol;
+
+			protocol = list_entry(protocol_handle,
+					      struct efi_handler, link);
+			(*protocol_buffer)[j] = (void *)protocol->guid;
+			++j;
 		}
-		break;
 	}
 
 	return EFI_EXIT(EFI_SUCCESS);
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index 1d6cf3122f..af8db40e19 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -224,6 +224,7 @@ static void efi_disk_add_dev(const char *name,
 		goto out_of_memory;
 
 	/* Hook up to the device list */
+	INIT_LIST_HEAD(&diskobj->parent.protocols);
 	list_add_tail(&diskobj->parent.link, &efi_obj_list);
 
 	/* Fill in object data */
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index 7b74d6ef33..498184d754 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -180,6 +180,7 @@ int efi_gop_register(void)
 	}
 
 	/* Hook up to the device list */
+	INIT_LIST_HEAD(&gopobj->parent.protocols);
 	list_add_tail(&gopobj->parent.link, &efi_obj_list);
 
 	/* Fill in object data */
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 8b2f682351..74a67c5365 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -296,6 +296,7 @@ int efi_net_register(void)
 		goto out_of_memory;
 
 	/* Hook net up to the device list */
+	INIT_LIST_HEAD(&netobj->parent.protocols);
 	list_add_tail(&netobj->parent.link, &efi_obj_list);
 
 	/* Fill in object data */
-- 
2.15.0

[U-Boot] [PATCH v2 12/18] efi_loader: manage protocols in a linked list

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  include/efi_loader.h          |   6 ++-
>  lib/efi_loader/efi_boottime.c | 107 ++++++++++++++++++++----------------------
>  lib/efi_loader/efi_disk.c     |   1 +
>  lib/efi_loader/efi_gop.c      |   1 +
>  lib/efi_loader/efi_net.c      |   1 +
>  5 files changed, 58 insertions(+), 58 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 13/18] efi_selftest: compile without special compiler flags

[Heinrich Schuchardt]

As the selftest is not compiled as an EFI binary we do not
need special compiler flags.

This avoids the checkarmreloc error on vexpress_ca15_tc2.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	no change
---
 lib/efi_selftest/Makefile | 25 -------------------------
 1 file changed, 25 deletions(-)

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index d280eca5c3..837e86228e 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -7,31 +7,6 @@
 # This file only gets included with CONFIG_EFI_LOADER set, so all
 # object inclusion implicitly depends on it
 
-CFLAGS_efi_selftest.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_console.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_console.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_devicepathe.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_devicepath.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_events.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_events.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_exitbootservices.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_exitbootservices.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_gop.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_gop.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_manageprotocols.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_manageprotocols.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_snp.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_snp.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_textoutput.o := $(CFLAGS_EFI)
-CFLAGS_REVMOE_efi_selftest_textoutput.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_tpl.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_tpl.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_util.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_util.o := $(CFLAGS_NON_EFI)
-CFLAGS_efi_selftest_watchdog.o := $(CFLAGS_EFI)
-CFLAGS_REMOVE_efi_selftest_watchdog.o := $(CFLAGS_NON_EFI)
-
 obj-$(CONFIG_CMD_BOOTEFI_SELFTEST) += \
 efi_selftest.o \
 efi_selftest_console.o \
-- 
2.15.0

[U-Boot] [PATCH v2 13/18] efi_selftest: compile without special compiler flags

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> As the selftest is not compiled as an EFI binary we do not
> need special compiler flags.
>
> This avoids the checkarmreloc error on vexpress_ca15_tc2.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         no change
> ---
>  lib/efi_selftest/Makefile | 25 -------------------------
>  1 file changed, 25 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 14/18] efi_selftest: add missing line feed

[Heinrich Schuchardt]

Add a missing line feed for an error message.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 lib/efi_selftest/efi_selftest.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_selftest/efi_selftest.c b/lib/efi_selftest/efi_selftest.c
index 263b53f672..4e5a12c47c 100644
--- a/lib/efi_selftest/efi_selftest.c
+++ b/lib/efi_selftest/efi_selftest.c
@@ -239,7 +239,7 @@ efi_status_t EFIAPI efi_selftest(efi_handle_t image_handle,
 	ret = boottime->handle_protocol(image_handle, &efi_guid_loaded_image,
 					(void **)&loaded_image);
 	if (ret != EFI_SUCCESS) {
-		efi_st_error("Cannot open loaded image protocol");
+		efi_st_error("Cannot open loaded image protocol\n");
 		return ret;
 	}
 
-- 
2.15.0

[U-Boot] [PATCH v2 14/18] efi_selftest: add missing line feed

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Add a missing line feed for an error message.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         new patch
> ---
>  lib/efi_selftest/efi_selftest.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 15/18] efi_loader: output load options in helloworld

[Heinrich Schuchardt]

We need to test if we pass a valid image handle when loading
and EFI application. This cannot be done in efi_selftest as
it is not loaded as an image.

So let's enhance helloworld to write the load options to the
console.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 lib/efi_loader/helloworld.c | 37 +++++++++++++++++++++++++++++++++++--
 1 file changed, 35 insertions(+), 2 deletions(-)

diff --git a/lib/efi_loader/helloworld.c b/lib/efi_loader/helloworld.c
index 77130a36dd..e59c24c788 100644
--- a/lib/efi_loader/helloworld.c
+++ b/lib/efi_loader/helloworld.c
@@ -5,19 +5,52 @@
  * Written by Simon Glass <sjg@chromium.org>
  *
  * SPDX-License-Identifier:     GPL-2.0+
+ *
+ * This program demonstrates calling a boottime service.
+ * It writes a greeting and the load options to the console.
  */
 
 #include <common.h>
 #include <efi_api.h>
 
+/*
+ * Entry point of the EFI application.
+ *
+ * @handle	handle of the loaded image
+ * @systable	system table
+ * @return	status code
+ */
 efi_status_t EFIAPI efi_main(efi_handle_t handle,
 			     struct efi_system_table *systable)
 {
 	struct efi_simple_text_output_protocol *con_out = systable->con_out;
 	struct efi_boot_services *boottime = systable->boottime;
+	struct efi_loaded_image *loaded_image;
+	const efi_guid_t loaded_image_guid = LOADED_IMAGE_GUID;
+	efi_status_t ret;
 
 	con_out->output_string(con_out, L"Hello, world!\n");
-	boottime->exit(handle, 0, 0, NULL);
 
-	return EFI_SUCCESS;
+	/* Get the loaded image protocol */
+	ret = boottime->handle_protocol(handle, &loaded_image_guid,
+					(void **)&loaded_image);
+	if (ret != EFI_SUCCESS) {
+		con_out->output_string(con_out,
+				       L"Cannot open loaded image protocol\n");
+		goto out;
+	}
+	/* Output the load options */
+	con_out->output_string(con_out, L"Load options: ");
+	if (loaded_image->load_options_size && loaded_image->load_options)
+		con_out->output_string(con_out,
+				       (u16 *)loaded_image->load_options);
+	else
+		con_out->output_string(con_out, L"<none>");
+	con_out->output_string(con_out, L"\n");
+
+out:
+	boottime->exit(handle, ret, 0, NULL);
+
+	/* We should never arrive here */
+	return ret;
 }
-- 
2.15.0

[U-Boot] [PATCH v2 15/18] efi_loader: output load options in helloworld

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> We need to test if we pass a valid image handle when loading
> and EFI application. This cannot be done in efi_selftest as
> it is not loaded as an image.
>
> So let's enhance helloworld to write the load options to the
> console.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         new patch
> ---
>  lib/efi_loader/helloworld.c | 37 +++++++++++++++++++++++++++++++++++--
>  1 file changed, 35 insertions(+), 2 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 16/18] test/py: check return code of helloworld

[Heinrich Schuchardt]

Check that helloworld.efi returns EFI_SUCCESS.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 test/py/tests/test_efi_loader.py | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/py/tests/test_efi_loader.py b/test/py/tests/test_efi_loader.py
index 4d391e13ef..906ef2feaa 100644
--- a/test/py/tests/test_efi_loader.py
+++ b/test/py/tests/test_efi_loader.py
@@ -154,6 +154,8 @@ def test_efi_helloworld_net(u_boot_console):
     output = u_boot_console.run_command('bootefi %x' % addr)
     expected_text = 'Hello, world'
     assert expected_text in output
+    expected_text = '## Application terminated, r = 0'
+    assert expected_text in output
 
 @pytest.mark.buildconfigspec('cmd_bootefi_hello')
 def test_efi_helloworld_builtin(u_boot_console):
-- 
2.15.0

[U-Boot] [PATCH v2 16/18] test/py: check return code of helloworld

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> Check that helloworld.efi returns EFI_SUCCESS.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         new patch
> ---
>  test/py/tests/test_efi_loader.py | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 17/18] efi_loader: pass handle of loaded image

[Heinrich Schuchardt]

The handle of a loaded image is the value of the handle
member of the loaded image info object and not the
address of the loaded image info.

Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 cmd/bootefi.c                 | 7 ++++---
 lib/efi_loader/efi_boottime.c | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 67855ba685..e6b8ec11d5 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -254,7 +254,8 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt,
 		dcache_disable();	/* flush cache before switch to EL2 */
 
 		/* Move into EL2 and keep running there */
-		armv8_switch_to_el2((ulong)entry, (ulong)&loaded_image_info,
+		armv8_switch_to_el2((ulong)entry,
+				    (ulong)&loaded_image_info_obj.handle,
 				    (ulong)&systab, 0, (ulong)efi_run_in_el2,
 				    ES_TO_AARCH64);
 
@@ -263,7 +264,7 @@ static efi_status_t do_bootefi_exec(void *efi, void *fdt,
 	}
 #endif
 
-	ret = efi_do_enter(&loaded_image_info, &systab, entry);
+	ret = efi_do_enter(loaded_image_info_obj.handle, &systab, entry);
 
 exit:
 	/* image has returned, loaded-image obj goes *poof*: */
@@ -350,7 +351,7 @@ static int do_bootefi(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
 		/* Transfer environment variable efi_selftest as load options */
 		set_load_options(&loaded_image_info, "efi_selftest");
 		/* Execute the test */
-		r = efi_selftest(&loaded_image_info, &systab);
+		r = efi_selftest(loaded_image_info_obj.handle, &systab);
 		efi_restore_gd();
 		free(loaded_image_info.load_options);
 		list_del(&loaded_image_info_obj.link);
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index cee0cb1390..9218379a28 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1317,7 +1317,7 @@ static efi_status_t EFIAPI efi_load_image(bool boot_policy,
 
 	info->system_table = &systab;
 	info->parent_handle = parent_image;
-	*image_handle = info;
+	*image_handle = obj->handle;
 
 	return EFI_EXIT(EFI_SUCCESS);
 }
-- 
2.15.0

[U-Boot] [PATCH v2 17/18] efi_loader: pass handle of loaded image

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> The handle of a loaded image is the value of the handle
> member of the loaded image info object and not the
> address of the loaded image info.
>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         new patch
> ---
>  cmd/bootefi.c                 | 7 ++++---
>  lib/efi_loader/efi_boottime.c | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 18/18] efi_loader: helper function to add EFI object to list

[Heinrich Schuchardt]

To avoid duplicate coding provide a helper function that
initializes an EFI object and adds it to the EFI object
list.

efi_exit() is the only place where we dereference a handle
to obtain a protocol interface. Add a comment to the function.

Suggested-by: Alexander Graf <agraf@suse.de>
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
---
v2
	new patch
---
 include/efi_loader.h          |  2 ++
 lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++++++++++++-------
 lib/efi_loader/efi_disk.c     |  4 +---
 lib/efi_loader/efi_gop.c      |  4 +---
 lib/efi_loader/efi_net.c      |  4 +---
 5 files changed, 37 insertions(+), 16 deletions(-)

diff --git a/include/efi_loader.h b/include/efi_loader.h
index a73bbc1269..c0caabddb1 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -186,6 +186,8 @@ void efi_restore_gd(void);
 void efi_runtime_relocate(ulong offset, struct efi_mem_desc *map);
 /* Call this to set the current device name */
 void efi_set_bootdev(const char *dev, const char *devnr, const char *path);
+/* Add a new object to the object list. */
+void efi_add_handle(struct efi_object *obj);
 /* Create handle */
 efi_status_t efi_create_handle(void **handle);
 /* Call this to validate a handle and find the EFI object for it */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 9218379a28..ce38619831 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -321,6 +321,23 @@ static efi_status_t EFIAPI efi_free_pool_ext(void *buffer)
 	return EFI_EXIT(r);
 }
 
+/*
+ * Add a new object to the object list.
+ *
+ * The protocols list is initialized.
+ * The object handle is set.
+ *
+ * @obj	object to be added
+ */
+void efi_add_handle(struct efi_object *obj)
+{
+	if (!obj)
+		return;
+	INIT_LIST_HEAD(&obj->protocols);
+	obj->handle = obj;
+	list_add_tail(&obj->link, &efi_obj_list);
+}
+
 /*
  * Create handle.
  *
@@ -337,11 +353,8 @@ efi_status_t efi_create_handle(void **handle)
 			      (void **)&obj);
 	if (r != EFI_SUCCESS)
 		return r;
-	memset(obj, 0, sizeof(struct efi_object));
-	obj->handle = obj;
-	INIT_LIST_HEAD(&obj->protocols);
-	list_add_tail(&obj->link, &efi_obj_list);
-	*handle = obj;
+	efi_add_handle(obj);
+	*handle = obj->handle;
 	return r;
 }
 
@@ -1163,14 +1176,15 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
 {
 	efi_status_t ret;
 
+	/* Add internal object to object list */
+	efi_add_handle(obj);
+	/* efi_exit() assumes that the handle points to the info */
 	obj->handle = info;
 
 	info->file_path = file_path;
 	if (device_path)
 		info->device_handle = efi_dp_find_obj(device_path, NULL);
 
-	INIT_LIST_HEAD(&obj->protocols);
-	list_add_tail(&obj->link, &efi_obj_list);
 	/*
 	 * When asking for the device path interface, return
 	 * bootefi_device_path
@@ -1379,6 +1393,17 @@ static efi_status_t EFIAPI efi_exit(efi_handle_t image_handle,
 			efi_status_t exit_status, unsigned long exit_data_size,
 			int16_t *exit_data)
 {
+	/*
+	 * We require that the handle points to the original loaded
+	 * image protocol interface.
+	 *
+	 * For getting the longjmp address this is safer than locating
+	 * the protocol because the protocol may have been reinstalled
+	 * pointing to another memory location.
+	 *
+	 * TODO: We should call the unload procedure of the loaded
+	 *	 image protocol.
+	 */
 	struct efi_loaded_image *loaded_image_info = (void*)image_handle;
 
 	EFI_ENTRY("%p, %ld, %ld, %p", image_handle, exit_status,
diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index af8db40e19..68ba2cf7b2 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -224,13 +224,11 @@ static void efi_disk_add_dev(const char *name,
 		goto out_of_memory;
 
 	/* Hook up to the device list */
-	INIT_LIST_HEAD(&diskobj->parent.protocols);
-	list_add_tail(&diskobj->parent.link, &efi_obj_list);
+	efi_add_handle(&diskobj->parent);
 
 	/* Fill in object data */
 	diskobj->dp = efi_dp_from_part(desc, part);
 	diskobj->part = part;
-	diskobj->parent.handle = diskobj;
 	ret = efi_add_protocol(diskobj->parent.handle, &efi_block_io_guid,
 			       &diskobj->ops);
 	if (ret != EFI_SUCCESS)
diff --git a/lib/efi_loader/efi_gop.c b/lib/efi_loader/efi_gop.c
index 498184d754..3caddd5f84 100644
--- a/lib/efi_loader/efi_gop.c
+++ b/lib/efi_loader/efi_gop.c
@@ -180,11 +180,9 @@ int efi_gop_register(void)
 	}
 
 	/* Hook up to the device list */
-	INIT_LIST_HEAD(&gopobj->parent.protocols);
-	list_add_tail(&gopobj->parent.link, &efi_obj_list);
+	efi_add_handle(&gopobj->parent);
 
 	/* Fill in object data */
-	gopobj->parent.handle = &gopobj->ops;
 	ret = efi_add_protocol(gopobj->parent.handle, &efi_gop_guid,
 			       &gopobj->ops);
 	if (ret != EFI_SUCCESS) {
diff --git a/lib/efi_loader/efi_net.c b/lib/efi_loader/efi_net.c
index 74a67c5365..8c5d5b492c 100644
--- a/lib/efi_loader/efi_net.c
+++ b/lib/efi_loader/efi_net.c
@@ -296,11 +296,9 @@ int efi_net_register(void)
 		goto out_of_memory;
 
 	/* Hook net up to the device list */
-	INIT_LIST_HEAD(&netobj->parent.protocols);
-	list_add_tail(&netobj->parent.link, &efi_obj_list);
+	efi_add_handle(&netobj->parent);
 
 	/* Fill in object data */
-	netobj->parent.handle = &netobj->net;
 	r = efi_add_protocol(netobj->parent.handle, &efi_net_guid,
 			     &netobj->net);
 	if (r != EFI_SUCCESS)
-- 
2.15.0

[U-Boot] [PATCH v2 18/18] efi_loader: helper function to add EFI object to list

[Simon Glass]

On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> To avoid duplicate coding provide a helper function that
> initializes an EFI object and adds it to the EFI object
> list.
>
> efi_exit() is the only place where we dereference a handle

de-reference?

> to obtain a protocol interface. Add a comment to the function.
>
> Suggested-by: Alexander Graf <agraf@suse.de>
> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
> ---
> v2
>         new patch
> ---
>  include/efi_loader.h          |  2 ++
>  lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++++++++++++-------
>  lib/efi_loader/efi_disk.c     |  4 +---
>  lib/efi_loader/efi_gop.c      |  4 +---
>  lib/efi_loader/efi_net.c      |  4 +---
>  5 files changed, 37 insertions(+), 16 deletions(-)

Reviewed-by: Simon Glass <sjg@chromium.org>

[U-Boot] [PATCH v2 18/18] efi_loader: helper function to add EFI object to list

[Heinrich Schuchardt]

On 11/17/2017 03:07 PM, Simon Glass wrote:
> On 12 November 2017 at 07:02, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> To avoid duplicate coding provide a helper function that
>> initializes an EFI object and adds it to the EFI object
>> list.
>>
>> efi_exit() is the only place where we dereference a handle
> 
> de-reference?

Google spits out zillions of results for writing it without hyphen 
including online dictionaries.

Regards

Heinrich

> 
>> to obtain a protocol interface. Add a comment to the function.
>>
>> Suggested-by: Alexander Graf <agraf@suse.de>
>> Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
>> ---
>> v2
>>          new patch
>> ---
>>   include/efi_loader.h          |  2 ++
>>   lib/efi_loader/efi_boottime.c | 39 ++++++++++++++++++++++++++++++++-------
>>   lib/efi_loader/efi_disk.c     |  4 +---
>>   lib/efi_loader/efi_gop.c      |  4 +---
>>   lib/efi_loader/efi_net.c      |  4 +---
>>   5 files changed, 37 insertions(+), 16 deletions(-)
> 
> Reviewed-by: Simon Glass <sjg@chromium.org>
>