[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub
Heinrich Schuchardt
xypron.glpk at gmx.de
Wed Oct 4 19:02:47 UTC 2017
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 at gmx.de> wrote:
>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>> From: Leif Lindholm <leif.lindholm at linaro.org>
>>>
>>> The commit message is missing.
>>>
>>> Fix all issues reported by checkpatch.
>>>
>>>>
>>>> Signed-off-by: Leif Lindholm <leif.lindholm at 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
>>>> +};
>>>>
>>>
>>
>
>
More information about the U-Boot
mailing list