[03/11] efi_loader: Initial EFI_UNICODE_COLLATION_PROTOCOL

Message ID 20171010122309.25313-4-robdclark@gmail.com
State New
Headers show
Series
  • [01/11] efi_loader: Initial EFI_DEVICE_PATH_UTILITIES_PROTOCOL
Related show

Commit Message

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

Not complete, but enough for Shell.efi and SCT.efi.

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

Cc: Leif Lindholm <leif.lindholm@linaro.org>
Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h             |  41 ++++++++++
 include/efi_loader.h          |   3 +
 lib/efi_loader/Makefile       |   2 +-
 lib/efi_loader/efi_boottime.c |   6 ++
 lib/efi_loader/efi_unicode.c  | 170 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 221 insertions(+), 1 deletion(-)
 create mode 100644 lib/efi_loader/efi_unicode.c

Comments

Alexander Graf Oct. 11, 2017, 2:36 p.m. | #1
On 10.10.17 14:22, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Not complete, but enough for Shell.efi and SCT.efi.
> 
> Initial skeleton written by Leif, and then implementation by myself.
> 
> Cc: Leif Lindholm <leif.lindholm@linaro.org>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  include/efi_api.h             |  41 ++++++++++
>  include/efi_loader.h          |   3 +
>  lib/efi_loader/Makefile       |   2 +-
>  lib/efi_loader/efi_boottime.c |   6 ++
>  lib/efi_loader/efi_unicode.c  | 170 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 221 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 164147dc87..38dd1240c1 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -797,6 +797,47 @@ struct efi_hii_string_protocol {
>  		efi_uintn_t *secondary_languages_size);
>  };
>  
> +/*
> + * Both UNICODE_COLLATION protocols seem to be the same thing, but
> + * advertised with two different GUID's because, why not?
> + */
> +
> +#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
> +	EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
> +		 0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
> +
> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
> +	EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
> +		 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
> +
> +struct efi_unicode_collation_protocol {
> +	efi_intn_t (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,
> +		efi_uintn_t fat_size,
> +		uint8_t *fat,
> +		efi_string_t string);
> +	bool (EFIAPI *str_to_fat)(
> +		struct efi_unicode_collation_protocol *this,
> +		efi_string_t string,
> +		efi_uintn_t 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 591bf07e7a..af6812b2b4 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -83,6 +83,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);
>  
> @@ -97,6 +98,8 @@ 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_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 725e0cba85..7ea96a4f1c 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -17,7 +17,7 @@ endif
>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
> -obj-y += efi_device_path_utilities.o 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 c179afc25a..b568f3f162 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -1166,6 +1166,12 @@ 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_protocol;
> +	obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
> +
> +	obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
> +	obj->protocols[9].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..2c6302df25
> --- /dev/null
> +++ b/lib/efi_loader/efi_unicode.c
> @@ -0,0 +1,170 @@
> +/*
> +*  EFI Unicode interface
> + *
> + *  Copyright (c) 2017 Leif Lindholm
> + *
> + *  SPDX-License-Identifier:     GPL-2.0+
> + */
> +
> +#include <common.h>
> +#include <charset.h>
> +#include <linux/ctype.h>
> +#include <efi_loader.h>
> +
> +const efi_guid_t efi_guid_unicode_collation_protocol =
> +	EFI_UNICODE_COLLATION_PROTOCOL_GUID;
> +
> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
> +	EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
> +
> +static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
> +{
> +	char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
> +	char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
> +
> +	*utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
> +	*utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
> +
> +	return strcasecmp(u1, u2);
> +}
> +
> +static efi_intn_t EFIAPI stri_coll(struct efi_unicode_collation_protocol *this,
> +				   efi_string_t s1,
> +				   efi_string_t s2)
> +{
> +	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
> +
> +	unsigned n1 = utf16_strlen(s1);
> +	unsigned n2 = utf16_strlen(s2);
> +
> +	return EFI_EXIT(matchn(s1, n1, s2, n2));
> +}
> +
> +static bool match(efi_string_t string, efi_string_t pattern)
> +{
> +	while (true) {
> +		uint16_t p = *pattern++;
> +		bool matches = false;
> +
> +		if (p == '\0' || *string == '\0') {
> +			/*
> +			 * End of pattern or string, succeed if
> +			 * end of both:
> +			 */
> +			return *string == p;
> +		}
> +
> +		switch (p) {
> +		case '*':
> +			/* Match zero or more chars: */
> +			while (*string != '\0') {
> +				if (match(string, pattern))
> +					return true;
> +				string++;
> +			}
> +			return match(string, pattern);
> +		case '?':
> +			/* Match any one char: */
> +			string++;
> +			break;
> +		case '[':
> +			/* Match char set, either [abc] or [a-c]: */
> +
> +			if (pattern[0] == '\0' || pattern[0] == ']') {
> +				/* invalid pattern */
> +				return false;
> +			}
> +
> +			if (pattern[1] == '-') {
> +				uint16_t lo, hi, c;
> +
> +				/* range: [a-c] */
> +				lo = pattern[0];
> +				hi = pattern[2];
> +
> +				if (hi == '\0' || hi == ']' || pattern[3] != ']') {
> +					/* invalid pattern */
> +					return false;
> +				}
> +
> +				c  = tolower(*string);
> +				lo = tolower(lo);
> +				hi = tolower(hi);
> +
> +				if (lo <= c && c <= hi)
> +					matches = true;
> +
> +				pattern += 4;
> +			} else {
> +				/* set: [abc] */
> +				while ((p = *pattern++) && p != ']')
> +					if (matchn(string, 1, &p, 1))
> +						matches = true;
> +			}
> +
> +			if (!matches)
> +				return false;
> +
> +			string++;
> +			break;
> +		default:
> +			if (matchn(string, 1, &p, 1))
> +				return false;
> +			string++;
> +			break;
> +		}
> +	}
> +}
> +
> +static bool EFIAPI 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(match(string, pattern));

EFI_EXIT wants a success/failure parameter, not true/false (which can be
randomly defined). Please either have match() return EFI return codes or
convert it in here :).

> +}
> +
> +static void EFIAPI str_lwr(struct efi_unicode_collation_protocol *this,
> +			   efi_string_t string)
> +{
> +	EFI_ENTRY("%p, \"%ls\"", this, string);
> +	EFI_EXIT(EFI_SUCCESS);

This is not implemented, right? Better indicate that in the EFI_EXIT()
hint so that someone tracing a log of calls will catch it later.

> +	return;

Not needed, no?

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

Same here

> +	return;
> +}
> +
> +static void EFIAPI fat_to_str(struct efi_unicode_collation_protocol *this,
> +			      efi_uintn_t fat_size,
> +			      uint8_t *fat,
> +			      efi_string_t string)
> +{
> +	EFI_ENTRY("%p, %zu, \"%s\", %p", this, fat_size, fat, string);
> +	EFI_EXIT(EFI_SUCCESS);

Same here

> +	return;
> +}
> +
> +static bool EFIAPI str_to_fat(struct efi_unicode_collation_protocol *this,
> +			      efi_string_t string,
> +			      efi_uintn_t fat_size,
> +			      uint8_t *fat)
> +{
> +	EFI_ENTRY("%p, \"%ls\", %zu, %p", this, string, fat_size, fat);
> +	return EFI_EXIT(false);

Again, please don't pass true/false to EFI_EXIT().


Alex

> +}
> +
> +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,
> +	.supported_languages = (uint8_t *)"eng",
> +};
>
Rob Clark Oct. 11, 2017, 8:30 p.m. | #2
On Wed, Oct 11, 2017 at 10:36 AM, Alexander Graf <agraf@suse.de> wrote:
>
>
> On 10.10.17 14:22, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Not complete, but enough for Shell.efi and SCT.efi.
>>
>> Initial skeleton written by Leif, and then implementation by myself.
>>
>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  include/efi_api.h             |  41 ++++++++++
>>  include/efi_loader.h          |   3 +
>>  lib/efi_loader/Makefile       |   2 +-
>>  lib/efi_loader/efi_boottime.c |   6 ++
>>  lib/efi_loader/efi_unicode.c  | 170 ++++++++++++++++++++++++++++++++++++++++++
>>  5 files changed, 221 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 164147dc87..38dd1240c1 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -797,6 +797,47 @@ struct efi_hii_string_protocol {
>>               efi_uintn_t *secondary_languages_size);
>>  };
>>
>> +/*
>> + * Both UNICODE_COLLATION protocols seem to be the same thing, but
>> + * advertised with two different GUID's because, why not?
>> + */
>> +
>> +#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
>> +     EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
>> +              0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>> +
>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>> +
>> +struct efi_unicode_collation_protocol {
>> +     efi_intn_t (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,
>> +             efi_uintn_t fat_size,
>> +             uint8_t *fat,
>> +             efi_string_t string);
>> +     bool (EFIAPI *str_to_fat)(
>> +             struct efi_unicode_collation_protocol *this,
>> +             efi_string_t string,
>> +             efi_uintn_t 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 591bf07e7a..af6812b2b4 100644
>> --- a/include/efi_loader.h
>> +++ b/include/efi_loader.h
>> @@ -83,6 +83,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);
>>
>> @@ -97,6 +98,8 @@ 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_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 725e0cba85..7ea96a4f1c 100644
>> --- a/lib/efi_loader/Makefile
>> +++ b/lib/efi_loader/Makefile
>> @@ -17,7 +17,7 @@ endif
>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>> -obj-y += efi_device_path_utilities.o 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 c179afc25a..b568f3f162 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -1166,6 +1166,12 @@ 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_protocol;
>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>> +
>> +     obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
>> +     obj->protocols[9].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..2c6302df25
>> --- /dev/null
>> +++ b/lib/efi_loader/efi_unicode.c
>> @@ -0,0 +1,170 @@
>> +/*
>> +*  EFI Unicode interface
>> + *
>> + *  Copyright (c) 2017 Leif Lindholm
>> + *
>> + *  SPDX-License-Identifier:     GPL-2.0+
>> + */
>> +
>> +#include <common.h>
>> +#include <charset.h>
>> +#include <linux/ctype.h>
>> +#include <efi_loader.h>
>> +
>> +const efi_guid_t efi_guid_unicode_collation_protocol =
>> +     EFI_UNICODE_COLLATION_PROTOCOL_GUID;
>> +
>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>> +
>> +static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
>> +{
>> +     char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
>> +     char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
>> +
>> +     *utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
>> +     *utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
>> +
>> +     return strcasecmp(u1, u2);
>> +}
>> +
>> +static efi_intn_t EFIAPI stri_coll(struct efi_unicode_collation_protocol *this,
>> +                                efi_string_t s1,
>> +                                efi_string_t s2)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>> +
>> +     unsigned n1 = utf16_strlen(s1);
>> +     unsigned n2 = utf16_strlen(s2);
>> +
>> +     return EFI_EXIT(matchn(s1, n1, s2, n2));
>> +}
>> +
>> +static bool match(efi_string_t string, efi_string_t pattern)
>> +{
>> +     while (true) {
>> +             uint16_t p = *pattern++;
>> +             bool matches = false;
>> +
>> +             if (p == '\0' || *string == '\0') {
>> +                     /*
>> +                      * End of pattern or string, succeed if
>> +                      * end of both:
>> +                      */
>> +                     return *string == p;
>> +             }
>> +
>> +             switch (p) {
>> +             case '*':
>> +                     /* Match zero or more chars: */
>> +                     while (*string != '\0') {
>> +                             if (match(string, pattern))
>> +                                     return true;
>> +                             string++;
>> +                     }
>> +                     return match(string, pattern);
>> +             case '?':
>> +                     /* Match any one char: */
>> +                     string++;
>> +                     break;
>> +             case '[':
>> +                     /* Match char set, either [abc] or [a-c]: */
>> +
>> +                     if (pattern[0] == '\0' || pattern[0] == ']') {
>> +                             /* invalid pattern */
>> +                             return false;
>> +                     }
>> +
>> +                     if (pattern[1] == '-') {
>> +                             uint16_t lo, hi, c;
>> +
>> +                             /* range: [a-c] */
>> +                             lo = pattern[0];
>> +                             hi = pattern[2];
>> +
>> +                             if (hi == '\0' || hi == ']' || pattern[3] != ']') {
>> +                                     /* invalid pattern */
>> +                                     return false;
>> +                             }
>> +
>> +                             c  = tolower(*string);
>> +                             lo = tolower(lo);
>> +                             hi = tolower(hi);
>> +
>> +                             if (lo <= c && c <= hi)
>> +                                     matches = true;
>> +
>> +                             pattern += 4;
>> +                     } else {
>> +                             /* set: [abc] */
>> +                             while ((p = *pattern++) && p != ']')
>> +                                     if (matchn(string, 1, &p, 1))
>> +                                             matches = true;
>> +                     }
>> +
>> +                     if (!matches)
>> +                             return false;
>> +
>> +                     string++;
>> +                     break;
>> +             default:
>> +                     if (matchn(string, 1, &p, 1))
>> +                             return false;
>> +                     string++;
>> +                     break;
>> +             }
>> +     }
>> +}
>> +
>> +static bool EFIAPI 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(match(string, pattern));
>
> EFI_EXIT wants a success/failure parameter, not true/false (which can be
> randomly defined). Please either have match() return EFI return codes or
> convert it in here :).

That isn't strictly true, it does use typeof(ret).. and while it masks
the resulting debug msg with ~EFI_ERROR_MASK, in practice it isn't a
problem with boolean return.  Just a bit funny.

And there already other EFI entry points that return something other
than efi_status_t..  trying to force them all into efi_status_t mold
makes them somewhat more awkward.

BR,
-R

>> +}
>> +
>> +static void EFIAPI str_lwr(struct efi_unicode_collation_protocol *this,
>> +                        efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>> +     EFI_EXIT(EFI_SUCCESS);
>
> This is not implemented, right? Better indicate that in the EFI_EXIT()
> hint so that someone tracing a log of calls will catch it later.
>
>> +     return;
>
> Not needed, no?
>
>> +}
>> +
>> +static void EFIAPI str_upr(struct efi_unicode_collation_protocol *this,
>> +                        efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\"", this, string);
>> +     EFI_EXIT(EFI_SUCCESS);
>
> Same here
>
>> +     return;
>> +}
>> +
>> +static void EFIAPI fat_to_str(struct efi_unicode_collation_protocol *this,
>> +                           efi_uintn_t fat_size,
>> +                           uint8_t *fat,
>> +                           efi_string_t string)
>> +{
>> +     EFI_ENTRY("%p, %zu, \"%s\", %p", this, fat_size, fat, string);
>> +     EFI_EXIT(EFI_SUCCESS);
>
> Same here
>
>> +     return;
>> +}
>> +
>> +static bool EFIAPI str_to_fat(struct efi_unicode_collation_protocol *this,
>> +                           efi_string_t string,
>> +                           efi_uintn_t fat_size,
>> +                           uint8_t *fat)
>> +{
>> +     EFI_ENTRY("%p, \"%ls\", %zu, %p", this, string, fat_size, fat);
>> +     return EFI_EXIT(false);
>
> Again, please don't pass true/false to EFI_EXIT().
>
>
> Alex
>
>> +}
>> +
>> +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,
>> +     .supported_languages = (uint8_t *)"eng",
>> +};
>>
Alexander Graf Oct. 11, 2017, 8:47 p.m. | #3
On 11.10.17 22:30, Rob Clark wrote:
> On Wed, Oct 11, 2017 at 10:36 AM, Alexander Graf <agraf@suse.de> wrote:
>>
>>
>> On 10.10.17 14:22, Rob Clark wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> Not complete, but enough for Shell.efi and SCT.efi.
>>>
>>> Initial skeleton written by Leif, and then implementation by myself.
>>>
>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  include/efi_api.h             |  41 ++++++++++
>>>  include/efi_loader.h          |   3 +
>>>  lib/efi_loader/Makefile       |   2 +-
>>>  lib/efi_loader/efi_boottime.c |   6 ++
>>>  lib/efi_loader/efi_unicode.c  | 170 ++++++++++++++++++++++++++++++++++++++++++
>>>  5 files changed, 221 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 164147dc87..38dd1240c1 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -797,6 +797,47 @@ struct efi_hii_string_protocol {
>>>               efi_uintn_t *secondary_languages_size);
>>>  };
>>>
>>> +/*
>>> + * Both UNICODE_COLLATION protocols seem to be the same thing, but
>>> + * advertised with two different GUID's because, why not?
>>> + */
>>> +
>>> +#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
>>> +     EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
>>> +              0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>> +
>>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>>> +
>>> +struct efi_unicode_collation_protocol {
>>> +     efi_intn_t (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,
>>> +             efi_uintn_t fat_size,
>>> +             uint8_t *fat,
>>> +             efi_string_t string);
>>> +     bool (EFIAPI *str_to_fat)(
>>> +             struct efi_unicode_collation_protocol *this,
>>> +             efi_string_t string,
>>> +             efi_uintn_t 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 591bf07e7a..af6812b2b4 100644
>>> --- a/include/efi_loader.h
>>> +++ b/include/efi_loader.h
>>> @@ -83,6 +83,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);
>>>
>>> @@ -97,6 +98,8 @@ 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_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 725e0cba85..7ea96a4f1c 100644
>>> --- a/lib/efi_loader/Makefile
>>> +++ b/lib/efi_loader/Makefile
>>> @@ -17,7 +17,7 @@ endif
>>>  obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>>  obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>  obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>>> -obj-y += efi_device_path_utilities.o 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 c179afc25a..b568f3f162 100644
>>> --- a/lib/efi_loader/efi_boottime.c
>>> +++ b/lib/efi_loader/efi_boottime.c
>>> @@ -1166,6 +1166,12 @@ 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_protocol;
>>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>>> +
>>> +     obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
>>> +     obj->protocols[9].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..2c6302df25
>>> --- /dev/null
>>> +++ b/lib/efi_loader/efi_unicode.c
>>> @@ -0,0 +1,170 @@
>>> +/*
>>> +*  EFI Unicode interface
>>> + *
>>> + *  Copyright (c) 2017 Leif Lindholm
>>> + *
>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>> + */
>>> +
>>> +#include <common.h>
>>> +#include <charset.h>
>>> +#include <linux/ctype.h>
>>> +#include <efi_loader.h>
>>> +
>>> +const efi_guid_t efi_guid_unicode_collation_protocol =
>>> +     EFI_UNICODE_COLLATION_PROTOCOL_GUID;
>>> +
>>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>>> +
>>> +static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
>>> +{
>>> +     char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
>>> +     char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
>>> +
>>> +     *utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
>>> +     *utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
>>> +
>>> +     return strcasecmp(u1, u2);
>>> +}
>>> +
>>> +static efi_intn_t EFIAPI stri_coll(struct efi_unicode_collation_protocol *this,
>>> +                                efi_string_t s1,
>>> +                                efi_string_t s2)
>>> +{
>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>>> +
>>> +     unsigned n1 = utf16_strlen(s1);
>>> +     unsigned n2 = utf16_strlen(s2);
>>> +
>>> +     return EFI_EXIT(matchn(s1, n1, s2, n2));
>>> +}
>>> +
>>> +static bool match(efi_string_t string, efi_string_t pattern)
>>> +{
>>> +     while (true) {
>>> +             uint16_t p = *pattern++;
>>> +             bool matches = false;
>>> +
>>> +             if (p == '\0' || *string == '\0') {
>>> +                     /*
>>> +                      * End of pattern or string, succeed if
>>> +                      * end of both:
>>> +                      */
>>> +                     return *string == p;
>>> +             }
>>> +
>>> +             switch (p) {
>>> +             case '*':
>>> +                     /* Match zero or more chars: */
>>> +                     while (*string != '\0') {
>>> +                             if (match(string, pattern))
>>> +                                     return true;
>>> +                             string++;
>>> +                     }
>>> +                     return match(string, pattern);
>>> +             case '?':
>>> +                     /* Match any one char: */
>>> +                     string++;
>>> +                     break;
>>> +             case '[':
>>> +                     /* Match char set, either [abc] or [a-c]: */
>>> +
>>> +                     if (pattern[0] == '\0' || pattern[0] == ']') {
>>> +                             /* invalid pattern */
>>> +                             return false;
>>> +                     }
>>> +
>>> +                     if (pattern[1] == '-') {
>>> +                             uint16_t lo, hi, c;
>>> +
>>> +                             /* range: [a-c] */
>>> +                             lo = pattern[0];
>>> +                             hi = pattern[2];
>>> +
>>> +                             if (hi == '\0' || hi == ']' || pattern[3] != ']') {
>>> +                                     /* invalid pattern */
>>> +                                     return false;
>>> +                             }
>>> +
>>> +                             c  = tolower(*string);
>>> +                             lo = tolower(lo);
>>> +                             hi = tolower(hi);
>>> +
>>> +                             if (lo <= c && c <= hi)
>>> +                                     matches = true;
>>> +
>>> +                             pattern += 4;
>>> +                     } else {
>>> +                             /* set: [abc] */
>>> +                             while ((p = *pattern++) && p != ']')
>>> +                                     if (matchn(string, 1, &p, 1))
>>> +                                             matches = true;
>>> +                     }
>>> +
>>> +                     if (!matches)
>>> +                             return false;
>>> +
>>> +                     string++;
>>> +                     break;
>>> +             default:
>>> +                     if (matchn(string, 1, &p, 1))
>>> +                             return false;
>>> +                     string++;
>>> +                     break;
>>> +             }
>>> +     }
>>> +}
>>> +
>>> +static bool EFIAPI 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(match(string, pattern));
>>
>> EFI_EXIT wants a success/failure parameter, not true/false (which can be
>> randomly defined). Please either have match() return EFI return codes or
>> convert it in here :).
> 
> That isn't strictly true, it does use typeof(ret).. and while it masks
> the resulting debug msg with ~EFI_ERROR_MASK, in practice it isn't a
> problem with boolean return.  Just a bit funny.
> 
> And there already other EFI entry points that return something other
> than efi_status_t..  trying to force them all into efi_status_t mold
> makes them somewhat more awkward.

Well, anything that casts well into an integer is reasonably valid for
EFI_EXIT(). But with bool things aren't always as clear as they should
be. I'm for example not 100% sure if the return value of a bool function
can never be different from 0 or 1.

So please just use something more obvious and keep bool values as inputs
for conditional branches :). Even something like EFI_EXIT(foo ? EFI_TRUE
: EFI_FALSE); would work for me. Or you just make the return type of
those functions an enum efi_bool.

The resulting code should basically be the same if the compiler is
smart, but at least I'm 100% sure when reading it that it does "the
right thing".


Alex
Alexander Graf Oct. 12, 2017, 11:54 a.m. | #4
On 10/11/2017 10:47 PM, Alexander Graf wrote:
>
> On 11.10.17 22:30, Rob Clark wrote:
>> On Wed, Oct 11, 2017 at 10:36 AM, Alexander Graf <agraf@suse.de> wrote:
>>>
>>> On 10.10.17 14:22, Rob Clark wrote:
>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>>
>>>> Not complete, but enough for Shell.efi and SCT.efi.
>>>>
>>>> Initial skeleton written by Leif, and then implementation by myself.
>>>>
>>>> Cc: Leif Lindholm <leif.lindholm@linaro.org>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>>   include/efi_api.h             |  41 ++++++++++
>>>>   include/efi_loader.h          |   3 +
>>>>   lib/efi_loader/Makefile       |   2 +-
>>>>   lib/efi_loader/efi_boottime.c |   6 ++
>>>>   lib/efi_loader/efi_unicode.c  | 170 ++++++++++++++++++++++++++++++++++++++++++
>>>>   5 files changed, 221 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 164147dc87..38dd1240c1 100644
>>>> --- a/include/efi_api.h
>>>> +++ b/include/efi_api.h
>>>> @@ -797,6 +797,47 @@ struct efi_hii_string_protocol {
>>>>                efi_uintn_t *secondary_languages_size);
>>>>   };
>>>>
>>>> +/*
>>>> + * Both UNICODE_COLLATION protocols seem to be the same thing, but
>>>> + * advertised with two different GUID's because, why not?
>>>> + */
>>>> +
>>>> +#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
>>>> +     EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
>>>> +              0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
>>>> +
>>>> +#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
>>>> +     EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
>>>> +              0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
>>>> +
>>>> +struct efi_unicode_collation_protocol {
>>>> +     efi_intn_t (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,
>>>> +             efi_uintn_t fat_size,
>>>> +             uint8_t *fat,
>>>> +             efi_string_t string);
>>>> +     bool (EFIAPI *str_to_fat)(
>>>> +             struct efi_unicode_collation_protocol *this,
>>>> +             efi_string_t string,
>>>> +             efi_uintn_t 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 591bf07e7a..af6812b2b4 100644
>>>> --- a/include/efi_loader.h
>>>> +++ b/include/efi_loader.h
>>>> @@ -83,6 +83,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);
>>>>
>>>> @@ -97,6 +98,8 @@ 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_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 725e0cba85..7ea96a4f1c 100644
>>>> --- a/lib/efi_loader/Makefile
>>>> +++ b/lib/efi_loader/Makefile
>>>> @@ -17,7 +17,7 @@ endif
>>>>   obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
>>>>   obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
>>>>   obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
>>>> -obj-y += efi_device_path_utilities.o 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 c179afc25a..b568f3f162 100644
>>>> --- a/lib/efi_loader/efi_boottime.c
>>>> +++ b/lib/efi_loader/efi_boottime.c
>>>> @@ -1166,6 +1166,12 @@ 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_protocol;
>>>> +     obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
>>>> +
>>>> +     obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
>>>> +     obj->protocols[9].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..2c6302df25
>>>> --- /dev/null
>>>> +++ b/lib/efi_loader/efi_unicode.c
>>>> @@ -0,0 +1,170 @@
>>>> +/*
>>>> +*  EFI Unicode interface
>>>> + *
>>>> + *  Copyright (c) 2017 Leif Lindholm
>>>> + *
>>>> + *  SPDX-License-Identifier:     GPL-2.0+
>>>> + */
>>>> +
>>>> +#include <common.h>
>>>> +#include <charset.h>
>>>> +#include <linux/ctype.h>
>>>> +#include <efi_loader.h>
>>>> +
>>>> +const efi_guid_t efi_guid_unicode_collation_protocol =
>>>> +     EFI_UNICODE_COLLATION_PROTOCOL_GUID;
>>>> +
>>>> +const efi_guid_t efi_guid_unicode_collation_protocol2 =
>>>> +     EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
>>>> +
>>>> +static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
>>>> +{
>>>> +     char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
>>>> +     char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
>>>> +
>>>> +     *utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
>>>> +     *utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
>>>> +
>>>> +     return strcasecmp(u1, u2);
>>>> +}
>>>> +
>>>> +static efi_intn_t EFIAPI stri_coll(struct efi_unicode_collation_protocol *this,
>>>> +                                efi_string_t s1,
>>>> +                                efi_string_t s2)
>>>> +{
>>>> +     EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
>>>> +
>>>> +     unsigned n1 = utf16_strlen(s1);
>>>> +     unsigned n2 = utf16_strlen(s2);
>>>> +
>>>> +     return EFI_EXIT(matchn(s1, n1, s2, n2));
>>>> +}
>>>> +
>>>> +static bool match(efi_string_t string, efi_string_t pattern)
>>>> +{
>>>> +     while (true) {
>>>> +             uint16_t p = *pattern++;
>>>> +             bool matches = false;
>>>> +
>>>> +             if (p == '\0' || *string == '\0') {
>>>> +                     /*
>>>> +                      * End of pattern or string, succeed if
>>>> +                      * end of both:
>>>> +                      */
>>>> +                     return *string == p;
>>>> +             }
>>>> +
>>>> +             switch (p) {
>>>> +             case '*':
>>>> +                     /* Match zero or more chars: */
>>>> +                     while (*string != '\0') {
>>>> +                             if (match(string, pattern))
>>>> +                                     return true;
>>>> +                             string++;
>>>> +                     }
>>>> +                     return match(string, pattern);
>>>> +             case '?':
>>>> +                     /* Match any one char: */
>>>> +                     string++;
>>>> +                     break;
>>>> +             case '[':
>>>> +                     /* Match char set, either [abc] or [a-c]: */
>>>> +
>>>> +                     if (pattern[0] == '\0' || pattern[0] == ']') {
>>>> +                             /* invalid pattern */
>>>> +                             return false;
>>>> +                     }
>>>> +
>>>> +                     if (pattern[1] == '-') {
>>>> +                             uint16_t lo, hi, c;
>>>> +
>>>> +                             /* range: [a-c] */
>>>> +                             lo = pattern[0];
>>>> +                             hi = pattern[2];
>>>> +
>>>> +                             if (hi == '\0' || hi == ']' || pattern[3] != ']') {
>>>> +                                     /* invalid pattern */
>>>> +                                     return false;
>>>> +                             }
>>>> +
>>>> +                             c  = tolower(*string);
>>>> +                             lo = tolower(lo);
>>>> +                             hi = tolower(hi);
>>>> +
>>>> +                             if (lo <= c && c <= hi)
>>>> +                                     matches = true;
>>>> +
>>>> +                             pattern += 4;
>>>> +                     } else {
>>>> +                             /* set: [abc] */
>>>> +                             while ((p = *pattern++) && p != ']')
>>>> +                                     if (matchn(string, 1, &p, 1))
>>>> +                                             matches = true;
>>>> +                     }
>>>> +
>>>> +                     if (!matches)
>>>> +                             return false;
>>>> +
>>>> +                     string++;
>>>> +                     break;
>>>> +             default:
>>>> +                     if (matchn(string, 1, &p, 1))
>>>> +                             return false;
>>>> +                     string++;
>>>> +                     break;
>>>> +             }
>>>> +     }
>>>> +}
>>>> +
>>>> +static bool EFIAPI 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(match(string, pattern));
>>> EFI_EXIT wants a success/failure parameter, not true/false (which can be
>>> randomly defined). Please either have match() return EFI return codes or
>>> convert it in here :).
>> That isn't strictly true, it does use typeof(ret).. and while it masks
>> the resulting debug msg with ~EFI_ERROR_MASK, in practice it isn't a
>> problem with boolean return.  Just a bit funny.
>>
>> And there already other EFI entry points that return something other
>> than efi_status_t..  trying to force them all into efi_status_t mold
>> makes them somewhat more awkward.
> Well, anything that casts well into an integer is reasonably valid for
> EFI_EXIT(). But with bool things aren't always as clear as they should
> be. I'm for example not 100% sure if the return value of a bool function
> can never be different from 0 or 1.

So I just double-checked with one of our compiler guys. Since we compile 
with at least C99, bool is defined as _Bool which is defined as an 
unsigned 1-bit data type that is defined as 0 or 1.

In a nutshell, keeping it as bool is fine I hope.


Alex

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 164147dc87..38dd1240c1 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -797,6 +797,47 @@  struct efi_hii_string_protocol {
 		efi_uintn_t *secondary_languages_size);
 };
 
+/*
+ * Both UNICODE_COLLATION protocols seem to be the same thing, but
+ * advertised with two different GUID's because, why not?
+ */
+
+#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
+	EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
+		 0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
+#define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
+	EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
+		 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
+
+struct efi_unicode_collation_protocol {
+	efi_intn_t (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,
+		efi_uintn_t fat_size,
+		uint8_t *fat,
+		efi_string_t string);
+	bool (EFIAPI *str_to_fat)(
+		struct efi_unicode_collation_protocol *this,
+		efi_string_t string,
+		efi_uintn_t 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 591bf07e7a..af6812b2b4 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -83,6 +83,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);
 
@@ -97,6 +98,8 @@  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_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 725e0cba85..7ea96a4f1c 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -17,7 +17,7 @@  endif
 obj-$(CONFIG_CMD_BOOTEFI_HELLO) += helloworld_efi.o
 obj-y += efi_image_loader.o efi_boottime.o efi_runtime.o efi_console.o
 obj-y += efi_memory.o efi_device_path_to_text.o efi_device_path.o
-obj-y += efi_device_path_utilities.o 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 c179afc25a..b568f3f162 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1166,6 +1166,12 @@  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_protocol;
+	obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
+
+	obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
+	obj->protocols[9].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..2c6302df25
--- /dev/null
+++ b/lib/efi_loader/efi_unicode.c
@@ -0,0 +1,170 @@ 
+/*
+*  EFI Unicode interface
+ *
+ *  Copyright (c) 2017 Leif Lindholm
+ *
+ *  SPDX-License-Identifier:     GPL-2.0+
+ */
+
+#include <common.h>
+#include <charset.h>
+#include <linux/ctype.h>
+#include <efi_loader.h>
+
+const efi_guid_t efi_guid_unicode_collation_protocol =
+	EFI_UNICODE_COLLATION_PROTOCOL_GUID;
+
+const efi_guid_t efi_guid_unicode_collation_protocol2 =
+	EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
+
+static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
+{
+	char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
+	char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
+
+	*utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
+	*utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
+
+	return strcasecmp(u1, u2);
+}
+
+static efi_intn_t EFIAPI stri_coll(struct efi_unicode_collation_protocol *this,
+				   efi_string_t s1,
+				   efi_string_t s2)
+{
+	EFI_ENTRY("%p, \"%ls\", \"%ls\"", this, s1, s2);
+
+	unsigned n1 = utf16_strlen(s1);
+	unsigned n2 = utf16_strlen(s2);
+
+	return EFI_EXIT(matchn(s1, n1, s2, n2));
+}
+
+static bool match(efi_string_t string, efi_string_t pattern)
+{
+	while (true) {
+		uint16_t p = *pattern++;
+		bool matches = false;
+
+		if (p == '\0' || *string == '\0') {
+			/*
+			 * End of pattern or string, succeed if
+			 * end of both:
+			 */
+			return *string == p;
+		}
+
+		switch (p) {
+		case '*':
+			/* Match zero or more chars: */
+			while (*string != '\0') {
+				if (match(string, pattern))
+					return true;
+				string++;
+			}
+			return match(string, pattern);
+		case '?':
+			/* Match any one char: */
+			string++;
+			break;
+		case '[':
+			/* Match char set, either [abc] or [a-c]: */
+
+			if (pattern[0] == '\0' || pattern[0] == ']') {
+				/* invalid pattern */
+				return false;
+			}
+
+			if (pattern[1] == '-') {
+				uint16_t lo, hi, c;
+
+				/* range: [a-c] */
+				lo = pattern[0];
+				hi = pattern[2];
+
+				if (hi == '\0' || hi == ']' || pattern[3] != ']') {
+					/* invalid pattern */
+					return false;
+				}
+
+				c  = tolower(*string);
+				lo = tolower(lo);
+				hi = tolower(hi);
+
+				if (lo <= c && c <= hi)
+					matches = true;
+
+				pattern += 4;
+			} else {
+				/* set: [abc] */
+				while ((p = *pattern++) && p != ']')
+					if (matchn(string, 1, &p, 1))
+						matches = true;
+			}
+
+			if (!matches)
+				return false;
+
+			string++;
+			break;
+		default:
+			if (matchn(string, 1, &p, 1))
+				return false;
+			string++;
+			break;
+		}
+	}
+}
+
+static bool EFIAPI 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(match(string, pattern));
+}
+
+static void EFIAPI str_lwr(struct efi_unicode_collation_protocol *this,
+			   efi_string_t string)
+{
+	EFI_ENTRY("%p, \"%ls\"", this, string);
+	EFI_EXIT(EFI_SUCCESS);
+	return;
+}
+
+static void EFIAPI str_upr(struct efi_unicode_collation_protocol *this,
+			   efi_string_t string)
+{
+	EFI_ENTRY("%p, \"%ls\"", this, string);
+	EFI_EXIT(EFI_SUCCESS);
+	return;
+}
+
+static void EFIAPI fat_to_str(struct efi_unicode_collation_protocol *this,
+			      efi_uintn_t fat_size,
+			      uint8_t *fat,
+			      efi_string_t string)
+{
+	EFI_ENTRY("%p, %zu, \"%s\", %p", this, fat_size, fat, string);
+	EFI_EXIT(EFI_SUCCESS);
+	return;
+}
+
+static bool EFIAPI str_to_fat(struct efi_unicode_collation_protocol *this,
+			      efi_string_t string,
+			      efi_uintn_t fat_size,
+			      uint8_t *fat)
+{
+	EFI_ENTRY("%p, \"%ls\", %zu, %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,
+	.supported_languages = (uint8_t *)"eng",
+};