[v1,03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

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

Commit Message

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

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 include/efi_api.h             | 33 +++++++++++++++++++
 include/efi_loader.h          |  2 ++
 lib/efi_loader/Makefile       |  2 +-
 lib/efi_loader/efi_boottime.c |  3 ++
 lib/efi_loader/efi_unicode.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 112 insertions(+), 1 deletion(-)
 create mode 100644 lib/efi_loader/efi_unicode.c

Comments

Heinrich Schuchardt Oct. 4, 2017, 5:57 p.m. | #1
On 09/10/2017 03:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>

The commit message is missing.

Fix all issues reported by checkpatch.

> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  include/efi_api.h             | 33 +++++++++++++++++++
>  include/efi_loader.h          |  2 ++
>  lib/efi_loader/Makefile       |  2 +-
>  lib/efi_loader/efi_boottime.c |  3 ++
>  lib/efi_loader/efi_unicode.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 112 insertions(+), 1 deletion(-)
>  create mode 100644 lib/efi_loader/efi_unicode.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 932a3429a8..25f774f253 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol
>  		UINTN *secondary_languages_size);
>  };
>  
> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
> +	EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
> +		 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
> +
> +struct efi_unicode_collation_protocol
> +{

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

> +	INTN(EFIAPI *stri_coll)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t s1,
> +		efi_string_t s2);
> +	bool(EFIAPI *metai_match)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t string,
> +		efi_string_t pattern);
> +	void(EFIAPI *str_lwr)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t string);
> +	void(EFIAPI *str_upr)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t string);
> +	void(EFIAPI *fat_to_str)(
> +		struct efi_unicode_collation_protocol *this,
> +		UINTN fat_size,
> +		uint8_t *fat,
> +		efi_string_t string);
> +	bool(EFIAPI *str_to_fat)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t string,
> +		UINTN fat_size,
> +		uint8_t *fat);
> +	uint8_t *supported_languages;
> +};
> +
>  #define EFI_GOP_GUID \
>  	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
>  		 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index a89bb2fcd9..6668338d0b 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities
>  extern const struct efi_hii_config_routing_protocol efi_hii_config_routing;
>  extern const struct efi_hii_database_protocol efi_hii_database;
>  extern const struct efi_hii_string_protocol efi_hii_string;
> +extern const struct efi_unicode_collation_protocol efi_unicode_collation;
>  
>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>  
> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol;
>  extern const efi_guid_t efi_guid_hii_config_routing_protocol;
>  extern const efi_guid_t efi_guid_hii_database_protocol;
>  extern const efi_guid_t efi_guid_hii_string_protocol;
> +extern const efi_guid_t efi_guid_unicode_collation_protocol2;
>  
>  extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>  extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
> index e8fd6823a3..82b703bb39 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
> -obj-y += efi_device_path_utilities.o efi_hii.o
> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o
>  obj-y += efi_file.o efi_variable.o efi_bootmgr.o
>  obj-$(CONFIG_LCD) += efi_gop.o
>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
> index 4d1a16051b..04358e8aca 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>  	obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;
>  	obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;
>  

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

> +	obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
> +	obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
> +
>  	info->file_path = file_path;
>  	info->device_handle = efi_dp_find_obj(device_path, NULL);
>  
> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c
> new file mode 100644
> index 0000000000..fdf1a99812
> --- /dev/null
> +++ b/lib/efi_loader/efi_unicode.c
> @@ -0,0 +1,73 @@
> +/*
> +*  EFI Unicode interface
> + *
> + *  Copyright (c) 2017 Leif Lindholm
> + *
> + *  SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <efi_loader.h>
> +
> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
> +	EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
> +

None of the functions matches the definitions in the structure.

Add the missing EFIAPI.

Regards

Heinrich

> +INTN stri_coll(struct efi_unicode_collation_protocol *this,
> +	       efi_string_t s1,
> +	       efi_string_t s2)
> +{
> +	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
> +	return EFI_EXIT(0);
> +}
> +
> +bool metai_match(struct efi_unicode_collation_protocol *this,
> +		 efi_string_t string,
> +		 efi_string_t pattern)
> +{
> +	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern);
> +	return EFI_EXIT(false);
> +}
> +
> +void str_lwr(struct efi_unicode_collation_protocol *this,
> +	     efi_string_t string)
> +{
> +	EFI_ENTRY("%p, \"%ls\"", this, string);
> +	EFI_EXIT(0);

EFI_EXIT(EFI_SUCCESS);

> +	return;
> +}
> +
> +void str_upr(struct efi_unicode_collation_protocol *this,
> +	     efi_string_t string)
> +{
> +	EFI_ENTRY("%p, \"%ls\"", this, string);
> +	EFI_EXIT(0);

EFI_EXIT(EFI_SUCCESS);

> +	return;
> +}
> +
> +void fat_to_str(struct efi_unicode_collation_protocol *this,
> +		UINTN fat_size,
> +		uint8_t *fat,
> +		efi_string_t string)
> +{
> +	EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string);
> +	EFI_EXIT(0);

EFI_EXIT(EFI_SUCCESS);

> +	return;
> +}
> +
> +bool str_to_fat(struct efi_unicode_collation_protocol *this,
> +		efi_string_t string,
> +		UINTN fat_size,
> +		uint8_t *fat)
> +{
> +	EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat);
> +	return EFI_EXIT(false);
> +}
> +
> +const struct efi_unicode_collation_protocol efi_unicode_collation = {
> +	.stri_coll = stri_coll,
> +	.metai_match = metai_match,
> +	.str_lwr = str_lwr,
> +	.str_upr = str_upr,
> +	.fat_to_str = fat_to_str,
> +	.str_to_fat = str_to_fat
> +};
>
Rob Clark Oct. 4, 2017, 6:35 p.m. | #2
On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>
> The commit message is missing.
>
> Fix all issues reported by checkpatch.
>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h             | 33 +++++++++++++++++++
>>  include/efi_loader.h          |  2 ++
>>  lib/efi_loader/Makefile       |  2 +-
>>  lib/efi_loader/efi_boottime.c |  3 ++
>>  lib/efi_loader/efi_unicode.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 112 insertions(+), 1 deletion(-)
>>  create mode 100644 lib/efi_loader/efi_unicode.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index 932a3429a8..25f774f253 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol
>>               UINTN *secondary_languages_size);
>>  };
>>
>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>> +
>> +struct efi_unicode_collation_protocol
>> +{
>
> ERROR: open brace '{' following struct go on the same line
> #30: FILE: include/efi_api.h:748:
> +struct efi_unicode_collation_protocol
> +{
>
>> +     INTN(EFIAPI *stri_coll)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t s1,
>> +             efi_string_t s2);
>> +     bool(EFIAPI *metai_match)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string,
>> +             efi_string_t pattern);
>> +     void(EFIAPI *str_lwr)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string);
>> +     void(EFIAPI *str_upr)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string);
>> +     void(EFIAPI *fat_to_str)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             UINTN fat_size,
>> +             uint8_t *fat,
>> +             efi_string_t string);
>> +     bool(EFIAPI *str_to_fat)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string,
>> +             UINTN fat_size,
>> +             uint8_t *fat);
>> +     uint8_t *supported_languages;
>> +};
>> +
>>  #define EFI_GOP_GUID \
>>       EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
>>                0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>> index a89bb2fcd9..6668338d0b 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities
>>  extern const struct efi_hii_config_routing_protocol efi_hii_config_routing;
>>  extern const struct efi_hii_database_protocol efi_hii_database;
>>  extern const struct efi_hii_string_protocol efi_hii_string;
>> +extern const struct efi_unicode_collation_protocol efi_unicode_collation;
>>
>>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>>
>> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol;
>>  extern const efi_guid_t efi_guid_hii_config_routing_protocol;
>>  extern const efi_guid_t efi_guid_hii_database_protocol;
>>  extern const efi_guid_t efi_guid_hii_string_protocol;
>> +extern const efi_guid_t efi_guid_unicode_collation_protocol2;
>>
>>  extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>>  extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>> index e8fd6823a3..82b703bb39 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>> -obj-y += efi_device_path_utilities.o efi_hii.o
>> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o
>>  obj-y += efi_file.o efi_variable.o efi_bootmgr.o
>>  obj-$(CONFIG_LCD) += efi_gop.o
>>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 4d1a16051b..04358e8aca 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>>       obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;
>>       obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;
>>
>
> Do not add a protocol that is not properly implemented yet.

There is *no possible way* that I am going to completely implement
HII, so big nak on this comment!

In general, my plan was to implement the minimal parts of these
protocols to get to the point where we can run SCT before implementing
the rest, so we at least have a way to test implementation against
compliance tests written against the spec (instead of our own tests
written against our interpretation of the spec).  In the particular
case of HII, it includes things like forms based UI rendering, so I
don't think we'll ever implement the entire thing.  Or if someone did,
we'd probably want to make it something that can be disabled at
compile time to keep footprint down.

Anyways, until we get to the point of loading option ROMs from PCI
cards in u-boot I don't think we really need a setup menu or complete
HII implementation ;-)

>> +     obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>> +
>>       info->file_path = file_path;
>>       info->device_handle = efi_dp_find_obj(device_path, NULL);
>>
>> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c
>> new file mode 100644
>> index 0000000000..fdf1a99812
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_unicode.c
>> @@ -0,0 +1,73 @@
>> +/*
>> +*  EFI Unicode interface
>> + *
>> + *  Copyright (c) 2017 Leif Lindholm
>> + *
>> + *  SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <efi_loader.h>
>> +
>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>> +
>
> None of the functions matches the definitions in the structure.
>
> Add the missing EFIAPI.

I was talking to Peter Jones about this, and he suggested something
that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on
x86_64 (at least conditionally if EFI_LOADER is enabled).  That would
take a bit of work to annotate the os layer in sandbox to use sysv
abi, but that is a lot smaller of an interface than EFI_LOADER (which
is only going to continue to grow).

Anyways, other than that, I'll double check the function order issues
you mentioned, I mostly assumed that Leif had it right (and I don't
remember encountering any issues with that so far with
Shell.efi/SCT.efi).

Some of your comments could be resolved by just squashing my
"implement" patches with Leif's "stub" patches, but I didn't want to
do that without Leif's permission.

BR,
-R

> Regards
>
> Heinrich
>
>> +INTN stri_coll(struct efi_unicode_collation_protocol *this,
>> +            efi_string_t s1,
>> +            efi_string_t s2)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>> +     return EFI_EXIT(0);
>> +}
>> +
>> +bool metai_match(struct efi_unicode_collation_protocol *this,
>> +              efi_string_t string,
>> +              efi_string_t pattern)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern);
>> +     return EFI_EXIT(false);
>> +}
>> +
>> +void str_lwr(struct efi_unicode_collation_protocol *this,
>> +          efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>> +     EFI_EXIT(0);
>
> EFI_EXIT(EFI_SUCCESS);
>
>> +     return;
>> +}
>> +
>> +void str_upr(struct efi_unicode_collation_protocol *this,
>> +          efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>> +     EFI_EXIT(0);
>
> EFI_EXIT(EFI_SUCCESS);
>
>> +     return;
>> +}
>> +
>> +void fat_to_str(struct efi_unicode_collation_protocol *this,
>> +             UINTN fat_size,
>> +             uint8_t *fat,
>> +             efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string);
>> +     EFI_EXIT(0);
>
> EFI_EXIT(EFI_SUCCESS);
>
>> +     return;
>> +}
>> +
>> +bool str_to_fat(struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string,
>> +             UINTN fat_size,
>> +             uint8_t *fat)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat);
>> +     return EFI_EXIT(false);
>> +}
>> +
>> +const struct efi_unicode_collation_protocol efi_unicode_collation = {
>> +     .stri_coll = stri_coll,
>> +     .metai_match = metai_match,
>> +     .str_lwr = str_lwr,
>> +     .str_upr = str_upr,
>> +     .fat_to_str = fat_to_str,
>> +     .str_to_fat = str_to_fat
>> +};
>>
>
Heinrich Schuchardt Oct. 4, 2017, 6:57 p.m. | #3
On 10/04/2017 08:35 PM, Rob Clark wrote:
> On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> The commit message is missing.
>>
>> Fix all issues reported by checkpatch.
>>
>>>
>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>> ---
>>>  include/efi_api.h             | 33 +++++++++++++++++++
>>>  include/efi_loader.h          |  2 ++
>>>  lib/efi_loader/Makefile       |  2 +-
>>>  lib/efi_loader/efi_boottime.c |  3 ++
>>>  lib/efi_loader/efi_unicode.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 112 insertions(+), 1 deletion(-)
>>>  create mode 100644 lib/efi_loader/efi_unicode.c
>>>
>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>> index 932a3429a8..25f774f253 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol
>>>               UINTN *secondary_languages_size);
>>>  };
>>>
>>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>>> +
>>> +struct efi_unicode_collation_protocol
>>> +{
>>
>> ERROR: open brace '{' following struct go on the same line
>> #30: FILE: include/efi_api.h:748:
>> +struct efi_unicode_collation_protocol
>> +{
>>
>>> +     INTN(EFIAPI *stri_coll)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t s1,
>>> +             efi_string_t s2);
>>> +     bool(EFIAPI *metai_match)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string,
>>> +             efi_string_t pattern);
>>> +     void(EFIAPI *str_lwr)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string);
>>> +     void(EFIAPI *str_upr)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string);
>>> +     void(EFIAPI *fat_to_str)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             UINTN fat_size,
>>> +             uint8_t *fat,
>>> +             efi_string_t string);
>>> +     bool(EFIAPI *str_to_fat)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string,
>>> +             UINTN fat_size,
>>> +             uint8_t *fat);
>>> +     uint8_t *supported_languages;
>>> +};
>>> +
>>>  #define EFI_GOP_GUID \
>>>       EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
>>>                0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>> index a89bb2fcd9..6668338d0b 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities
>>>  extern const struct efi_hii_config_routing_protocol efi_hii_config_routing;
>>>  extern const struct efi_hii_database_protocol efi_hii_database;
>>>  extern const struct efi_hii_string_protocol efi_hii_string;
>>> +extern const struct efi_unicode_collation_protocol efi_unicode_collation;
>>>
>>>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>>>
>>> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol;
>>>  extern const efi_guid_t efi_guid_hii_config_routing_protocol;
>>>  extern const efi_guid_t efi_guid_hii_database_protocol;
>>>  extern const efi_guid_t efi_guid_hii_string_protocol;
>>> +extern const efi_guid_t efi_guid_unicode_collation_protocol2;
>>>
>>>  extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>>>  extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>>> index e8fd6823a3..82b703bb39 100644
>>> --- a/lib/efi_loader/Makefile
>>> +++ b/lib/efi_loader/Makefile
>>> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>>> -obj-y += efi_device_path_utilities.o efi_hii.o
>>> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o
>>>  obj-y += efi_file.o efi_variable.o efi_bootmgr.o
>>>  obj-$(CONFIG_LCD) += efi_gop.o
>>>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>> index 4d1a16051b..04358e8aca 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>>>       obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;
>>>       obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;
>>>
>>
>> Do not add a protocol that is not properly implemented yet.
> 
> There is *no possible way* that I am going to completely implement
> HII, so big nak on this comment!
> 
> In general, my plan was to implement the minimal parts of these
> protocols to get to the point where we can run SCT before implementing
> the rest, so we at least have a way to test implementation against
> compliance tests written against the spec (instead of our own tests
> written against our interpretation of the spec).  In the particular
> case of HII, it includes things like forms based UI rendering, so I
> don't think we'll ever implement the entire thing.  Or if someone did,
> we'd probably want to make it something that can be disabled at
> compile time to keep footprint down.
> 
> Anyways, until we get to the point of loading option ROMs from PCI
> cards in u-boot I don't think we really need a setup menu or complete
> HII implementation ;-)
> 
>>> +     obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
>>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>>> +
>>>       info->file_path = file_path;
>>>       info->device_handle = efi_dp_find_obj(device_path, NULL);
>>>
>>> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c
>>> new file mode 100644
>>> index 0000000000..fdf1a99812
>>> --- /dev/null
>>> +++ b/lib/efi_loader/efi_unicode.c
>>> @@ -0,0 +1,73 @@
>>> +/*
>>> +*  EFI Unicode interface
>>> + *
>>> + *  Copyright (c) 2017 Leif Lindholm
>>> + *
>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <efi_loader.h>
>>> +
>>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>>> +
>>
>> None of the functions matches the definitions in the structure.
>>
>> Add the missing EFIAPI.
> 
> I was talking to Peter Jones about this, and he suggested something
> that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on
> x86_64 (at least conditionally if EFI_LOADER is enabled).  That would
> take a bit of work to annotate the os layer in sandbox to use sysv
> abi, but that is a lot smaller of an interface than EFI_LOADER (which
> is only going to continue to grow).

Let's first get your patch series in before starting a new job.

Otherwise we will never stop blocking each other.

Regards

Heinrich

> 
> Anyways, other than that, I'll double check the function order issues
> you mentioned, I mostly assumed that Leif had it right (and I don't
> remember encountering any issues with that so far with
> Shell.efi/SCT.efi).
> 
> Some of your comments could be resolved by just squashing my
> "implement" patches with Leif's "stub" patches, but I didn't want to
> do that without Leif's permission.
> 
> BR,
> -R
> 
>> Regards
>>
>> Heinrich
>>
>>> +INTN stri_coll(struct efi_unicode_collation_protocol *this,
>>> +            efi_string_t s1,
>>> +            efi_string_t s2)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>>> +     return EFI_EXIT(0);
>>> +}
>>> +
>>> +bool metai_match(struct efi_unicode_collation_protocol *this,
>>> +              efi_string_t string,
>>> +              efi_string_t pattern)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern);
>>> +     return EFI_EXIT(false);
>>> +}
>>> +
>>> +void str_lwr(struct efi_unicode_collation_protocol *this,
>>> +          efi_string_t string)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>>> +     EFI_EXIT(0);
>>
>> EFI_EXIT(EFI_SUCCESS);
>>
>>> +     return;
>>> +}
>>> +
>>> +void str_upr(struct efi_unicode_collation_protocol *this,
>>> +          efi_string_t string)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>>> +     EFI_EXIT(0);
>>
>> EFI_EXIT(EFI_SUCCESS);
>>
>>> +     return;
>>> +}
>>> +
>>> +void fat_to_str(struct efi_unicode_collation_protocol *this,
>>> +             UINTN fat_size,
>>> +             uint8_t *fat,
>>> +             efi_string_t string)
>>> +{
>>> +     EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string);
>>> +     EFI_EXIT(0);
>>
>> EFI_EXIT(EFI_SUCCESS);
>>
>>> +     return;
>>> +}
>>> +
>>> +bool str_to_fat(struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string,
>>> +             UINTN fat_size,
>>> +             uint8_t *fat)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat);
>>> +     return EFI_EXIT(false);
>>> +}
>>> +
>>> +const struct efi_unicode_collation_protocol efi_unicode_collation = {
>>> +     .stri_coll = stri_coll,
>>> +     .metai_match = metai_match,
>>> +     .str_lwr = str_lwr,
>>> +     .str_upr = str_upr,
>>> +     .fat_to_str = fat_to_str,
>>> +     .str_to_fat = str_to_fat
>>> +};
>>>
>>
>
Peter Jones Oct. 4, 2017, 7:01 p.m. | #4
On Wed, Oct 04, 2017 at 06:35:46PM +0000, Rob Clark wrote:
> > Add the missing EFIAPI.
> 
> I was talking to Peter Jones about this, and he suggested something
> that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on
> x86_64 (at least conditionally if EFI_LOADER is enabled).  That would
> take a bit of work to annotate the os layer in sandbox to use sysv
> abi, but that is a lot smaller of an interface than EFI_LOADER (which
> is only going to continue to grow).

Worth noting that if we went this route, we'd also need to do
-mregparm=0 on 32-bit x86.
Heinrich Schuchardt Oct. 4, 2017, 7:02 p.m. | #5
On 10/04/2017 08:57 PM, Heinrich Schuchardt wrote:
> On 10/04/2017 08:35 PM, Rob Clark wrote:
>> On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> The commit message is missing.
>>>
>>> Fix all issues reported by checkpatch.
>>>
>>>>
>>>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>>>> ---
>>>>  include/efi_api.h             | 33 +++++++++++++++++++
>>>>  include/efi_loader.h          |  2 ++
>>>>  lib/efi_loader/Makefile       |  2 +-
>>>>  lib/efi_loader/efi_boottime.c |  3 ++
>>>>  lib/efi_loader/efi_unicode.c  | 73 +++++++++++++++++++++++++++++++++++++++++++
>>>>  5 files changed, 112 insertions(+), 1 deletion(-)
>>>>  create mode 100644 lib/efi_loader/efi_unicode.c
>>>>
>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>> index 932a3429a8..25f774f253 100644
>>>> --- a/include/efi_api.h
>>>> +++ b/include/efi_api.h
>>>> @@ -740,6 +740,39 @@ struct efi_hii_string_protocol
>>>>               UINTN *secondary_languages_size);
>>>>  };
>>>>
>>>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>>>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>>>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>>>> +
>>>> +struct efi_unicode_collation_protocol
>>>> +{
>>>
>>> ERROR: open brace '{' following struct go on the same line
>>> #30: FILE: include/efi_api.h:748:
>>> +struct efi_unicode_collation_protocol
>>> +{
>>>
>>>> +     INTN(EFIAPI *stri_coll)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t s1,
>>>> +             efi_string_t s2);
>>>> +     bool(EFIAPI *metai_match)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string,
>>>> +             efi_string_t pattern);
>>>> +     void(EFIAPI *str_lwr)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string);
>>>> +     void(EFIAPI *str_upr)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string);
>>>> +     void(EFIAPI *fat_to_str)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             UINTN fat_size,
>>>> +             uint8_t *fat,
>>>> +             efi_string_t string);
>>>> +     bool(EFIAPI *str_to_fat)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string,
>>>> +             UINTN fat_size,
>>>> +             uint8_t *fat);
>>>> +     uint8_t *supported_languages;
>>>> +};
>>>> +
>>>>  #define EFI_GOP_GUID \
>>>>       EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
>>>>                0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
>>>> diff --git a/include/efi_loader.h b/include/efi_loader.h
>>>> index a89bb2fcd9..6668338d0b 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -62,6 +62,7 @@ extern const struct efi_device_path_utilities_protocol efi_device_path_utilities
>>>>  extern const struct efi_hii_config_routing_protocol efi_hii_config_routing;
>>>>  extern const struct efi_hii_database_protocol efi_hii_database;
>>>>  extern const struct efi_hii_string_protocol efi_hii_string;
>>>> +extern const struct efi_unicode_collation_protocol efi_unicode_collation;
>>>>
>>>>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>>>>
>>>> @@ -76,6 +77,7 @@ extern const efi_guid_t efi_guid_device_path_utilities_protocol;
>>>>  extern const efi_guid_t efi_guid_hii_config_routing_protocol;
>>>>  extern const efi_guid_t efi_guid_hii_database_protocol;
>>>>  extern const efi_guid_t efi_guid_hii_string_protocol;
>>>> +extern const efi_guid_t efi_guid_unicode_collation_protocol2;
>>>>
>>>>  extern unsigned int __efi_runtime_start, __efi_runtime_stop;
>>>>  extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
>>>> diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
>>>> index e8fd6823a3..82b703bb39 100644
>>>> --- a/lib/efi_loader/Makefile
>>>> +++ b/lib/efi_loader/Makefile
>>>> @@ -16,7 +16,7 @@ always := $(efiprogs-y)
>>>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>>>> -obj-y += efi_device_path_utilities.o efi_hii.o
>>>> +obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o
>>>>  obj-y += efi_file.o efi_variable.o efi_bootmgr.o
>>>>  obj-$(CONFIG_LCD) += efi_gop.o
>>>>  obj-$(CONFIG_DM_VIDEO) += efi_gop.o
>>>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>>>> index 4d1a16051b..04358e8aca 100644
>>>> --- a/lib/efi_loader/efi_boottime.c
>>>> +++ b/lib/efi_loader/efi_boottime.c
>>>> @@ -788,6 +788,9 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>>>>       obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;
>>>>       obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;
>>>>
>>>
>>> Do not add a protocol that is not properly implemented yet.
>>
>> There is *no possible way* that I am going to completely implement
>> HII, so big nak on this comment!

But this patch implements nothing.

So move adding the protocol to the image to your follow-up patch.

Regards

Heinrich

>>
>> In general, my plan was to implement the minimal parts of these
>> protocols to get to the point where we can run SCT before implementing
>> the rest, so we at least have a way to test implementation against
>> compliance tests written against the spec (instead of our own tests
>> written against our interpretation of the spec).  In the particular
>> case of HII, it includes things like forms based UI rendering, so I
>> don't think we'll ever implement the entire thing.  Or if someone did,
>> we'd probably want to make it something that can be disabled at
>> compile time to keep footprint down.
>>
>> Anyways, until we get to the point of loading option ROMs from PCI
>> cards in u-boot I don't think we really need a setup menu or complete
>> HII implementation ;-)
>>
>>>> +     obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
>>>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>>>> +
>>>>       info->file_path = file_path;
>>>>       info->device_handle = efi_dp_find_obj(device_path, NULL);
>>>>
>>>> diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c
>>>> new file mode 100644
>>>> index 0000000000..fdf1a99812
>>>> --- /dev/null
>>>> +++ b/lib/efi_loader/efi_unicode.c
>>>> @@ -0,0 +1,73 @@
>>>> +/*
>>>> +*  EFI Unicode interface
>>>> + *
>>>> + *  Copyright (c) 2017 Leif Lindholm
>>>> + *
>>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <efi_loader.h>
>>>> +
>>>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>>>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>>>> +
>>>
>>> None of the functions matches the definitions in the structure.
>>>
>>> Add the missing EFIAPI.
>>
>> I was talking to Peter Jones about this, and he suggested something
>> that makes *way* more sense.. just build u-boot with '-mabi=ms_abi' on
>> x86_64 (at least conditionally if EFI_LOADER is enabled).  That would
>> take a bit of work to annotate the os layer in sandbox to use sysv
>> abi, but that is a lot smaller of an interface than EFI_LOADER (which
>> is only going to continue to grow).
> 
> Let's first get your patch series in before starting a new job.
> 
> Otherwise we will never stop blocking each other.
> 
> Regards
> 
> Heinrich
> 
>>
>> Anyways, other than that, I'll double check the function order issues
>> you mentioned, I mostly assumed that Leif had it right (and I don't
>> remember encountering any issues with that so far with
>> Shell.efi/SCT.efi).
>>
>> Some of your comments could be resolved by just squashing my
>> "implement" patches with Leif's "stub" patches, but I didn't want to
>> do that without Leif's permission.
>>
>> BR,
>> -R
>>
>>> Regards
>>>
>>> Heinrich
>>>
>>>> +INTN stri_coll(struct efi_unicode_collation_protocol *this,
>>>> +            efi_string_t s1,
>>>> +            efi_string_t s2)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>>>> +     return EFI_EXIT(0);
>>>> +}
>>>> +
>>>> +bool metai_match(struct efi_unicode_collation_protocol *this,
>>>> +              efi_string_t string,
>>>> +              efi_string_t pattern)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern);
>>>> +     return EFI_EXIT(false);
>>>> +}
>>>> +
>>>> +void str_lwr(struct efi_unicode_collation_protocol *this,
>>>> +          efi_string_t string)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>>>> +     EFI_EXIT(0);
>>>
>>> EFI_EXIT(EFI_SUCCESS);
>>>
>>>> +     return;
>>>> +}
>>>> +
>>>> +void str_upr(struct efi_unicode_collation_protocol *this,
>>>> +          efi_string_t string)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>>>> +     EFI_EXIT(0);
>>>
>>> EFI_EXIT(EFI_SUCCESS);
>>>
>>>> +     return;
>>>> +}
>>>> +
>>>> +void fat_to_str(struct efi_unicode_collation_protocol *this,
>>>> +             UINTN fat_size,
>>>> +             uint8_t *fat,
>>>> +             efi_string_t string)
>>>> +{
>>>> +     EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string);
>>>> +     EFI_EXIT(0);
>>>
>>> EFI_EXIT(EFI_SUCCESS);
>>>
>>>> +     return;
>>>> +}
>>>> +
>>>> +bool str_to_fat(struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string,
>>>> +             UINTN fat_size,
>>>> +             uint8_t *fat)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat);
>>>> +     return EFI_EXIT(false);
>>>> +}
>>>> +
>>>> +const struct efi_unicode_collation_protocol efi_unicode_collation = {
>>>> +     .stri_coll = stri_coll,
>>>> +     .metai_match = metai_match,
>>>> +     .str_lwr = str_lwr,
>>>> +     .str_upr = str_upr,
>>>> +     .fat_to_str = fat_to_str,
>>>> +     .str_to_fat = str_to_fat
>>>> +};
>>>>
>>>
>>
> 
>

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 932a3429a8..25f774f253 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -740,6 +740,39 @@  struct efi_hii_string_protocol
 		UINTN *secondary_languages_size);
 };
 
+#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
+	EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
+		 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
+
+struct efi_unicode_collation_protocol
+{
+	INTN(EFIAPI *stri_coll)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t s1,
+		efi_string_t s2);
+	bool(EFIAPI *metai_match)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t string,
+		efi_string_t pattern);
+	void(EFIAPI *str_lwr)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t string);
+	void(EFIAPI *str_upr)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t string);
+	void(EFIAPI *fat_to_str)(
+		struct efi_unicode_collation_protocol *this,
+		UINTN fat_size,
+		uint8_t *fat,
+		efi_string_t string);
+	bool(EFIAPI *str_to_fat)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t string,
+		UINTN fat_size,
+		uint8_t *fat);
+	uint8_t *supported_languages;
+};
+
 #define EFI_GOP_GUID \
 	EFI_GUID(0x9042a9de, 0x23dc, 0x4a38, \
 		 0x96, 0xfb, 0x7a, 0xde, 0xd0, 0x80, 0x51, 0x6a)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index a89bb2fcd9..6668338d0b 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -62,6 +62,7 @@  extern const struct efi_device_path_utilities_protocol efi_device_path_utilities
 extern const struct efi_hii_config_routing_protocol efi_hii_config_routing;
 extern const struct efi_hii_database_protocol efi_hii_database;
 extern const struct efi_hii_string_protocol efi_hii_string;
+extern const struct efi_unicode_collation_protocol efi_unicode_collation;
 
 uint16_t *efi_dp_str(struct efi_device_path *dp);
 
@@ -76,6 +77,7 @@  extern const efi_guid_t efi_guid_device_path_utilities_protocol;
 extern const efi_guid_t efi_guid_hii_config_routing_protocol;
 extern const efi_guid_t efi_guid_hii_database_protocol;
 extern const efi_guid_t efi_guid_hii_string_protocol;
+extern const efi_guid_t efi_guid_unicode_collation_protocol2;
 
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
 extern unsigned int __efi_runtime_rel_start, __efi_runtime_rel_stop;
diff --git a/lib/efi_loader/Makefile b/lib/efi_loader/Makefile
index e8fd6823a3..82b703bb39 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -16,7 +16,7 @@  always := $(efiprogs-y)
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
 obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
-obj-y += efi_device_path_utilities.o efi_hii.o
+obj-y += efi_device_path_utilities.o efi_hii.o efi_unicode.o
 obj-y += efi_file.o efi_variable.o efi_bootmgr.o
 obj-$(CONFIG_LCD) += efi_gop.o
 obj-$(CONFIG_DM_VIDEO) += efi_gop.o
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 4d1a16051b..04358e8aca 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -788,6 +788,9 @@  void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
 	obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol;
 	obj->protocols[7].protocol_interface = (void *)&efi_hii_config_routing;
 
+	obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
+	obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
+
 	info->file_path = file_path;
 	info->device_handle = efi_dp_find_obj(device_path, NULL);
 
diff --git a/lib/efi_loader/efi_unicode.c b/lib/efi_loader/efi_unicode.c
new file mode 100644
index 0000000000..fdf1a99812
--- /dev/null
+++ b/lib/efi_loader/efi_unicode.c
@@ -0,0 +1,73 @@ 
+/*
+*  EFI Unicode interface
+ *
+ *  Copyright (c) 2017 Leif Lindholm
+ *
+ *  SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <efi_loader.h>
+
+const efi_guid_t efi_guid_unicode_collation_protocol2 =
+	EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
+
+INTN stri_coll(struct efi_unicode_collation_protocol *this,
+	       efi_string_t s1,
+	       efi_string_t s2)
+{
+	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
+	return EFI_EXIT(0);
+}
+
+bool metai_match(struct efi_unicode_collation_protocol *this,
+		 efi_string_t string,
+		 efi_string_t pattern)
+{
+	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, string, pattern);
+	return EFI_EXIT(false);
+}
+
+void str_lwr(struct efi_unicode_collation_protocol *this,
+	     efi_string_t string)
+{
+	EFI_ENTRY("%p, \"%ls\"", this, string);
+	EFI_EXIT(0);
+	return;
+}
+
+void str_upr(struct efi_unicode_collation_protocol *this,
+	     efi_string_t string)
+{
+	EFI_ENTRY("%p, \"%ls\"", this, string);
+	EFI_EXIT(0);
+	return;
+}
+
+void fat_to_str(struct efi_unicode_collation_protocol *this,
+		UINTN fat_size,
+		uint8_t *fat,
+		efi_string_t string)
+{
+	EFI_ENTRY("%p, %lu, \"%s\", %p", this, fat_size, fat, string);
+	EFI_EXIT(0);
+	return;
+}
+
+bool str_to_fat(struct efi_unicode_collation_protocol *this,
+		efi_string_t string,
+		UINTN fat_size,
+		uint8_t *fat)
+{
+	EFI_ENTRY("%p, \"%ls\", %lu, %p", this, string, fat_size, fat);
+	return EFI_EXIT(false);
+}
+
+const struct efi_unicode_collation_protocol efi_unicode_collation = {
+	.stri_coll = stri_coll,
+	.metai_match = metai_match,
+	.str_lwr = str_lwr,
+	.str_upr = str_upr,
+	.fat_to_str = fat_to_str,
+	.str_to_fat = str_to_fat
+};