[U-Boot] [PATCH 03/11] efi_loader: Initial EFI_UNICODE_COLLATION_PROTOCOL

Alexander Graf agraf at suse.de
Thu Oct 12 11:54:33 UTC 2017


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 at suse.de> wrote:
>>>
>>> On 10.10.17 14:22, Rob Clark wrote:
>>>> From: Leif Lindholm <leif.lindholm at 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 at linaro.org>
>>>> Signed-off-by: Rob Clark <robdclark at 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



More information about the U-Boot mailing list