On Thu, Oct 12, 2017 at 3:13 AM, Alexander Graf <ag...@suse.de> wrote: > > > On 12.10.17 00:02, Rob Clark wrote: >> On Wed, Oct 11, 2017 at 10:30 AM, Alexander Graf <ag...@suse.de> wrote: >>> >>> >>> On 10.10.17 14:22, Rob Clark wrote: >>>> From: Leif Lindholm <leif.lindh...@linaro.org> >>>> >>>> Enough implementation of the following protocols to run Shell.efi and >>>> SCT.efi: >>>> >>>> EfiHiiConfigRoutingProtocolGuid >>>> EfiHiiDatabaseProtocol >>>> EfiHiiStringProtocol >>>> >>>> We'll fill in the rest once SCT is running properly so we can validate >>>> the implementation against the conformance test suite. >>>> >>>> Initial skeleton written by Leif, and then implementation by myself. >>>> >>>> Cc: Leif Lindholm <leif.lindh...@linaro.org> >>>> Signed-off-by: Rob Clark <robdcl...@gmail.com> >>>> --- >>>> include/efi_api.h | 261 ++++++++++++++++++++++ >>>> include/efi_loader.h | 6 + >>>> lib/efi_loader/Makefile | 2 +- >>>> lib/efi_loader/efi_boottime.c | 9 + >>>> lib/efi_loader/efi_hii.c | 507 >>>> ++++++++++++++++++++++++++++++++++++++++++ >>>> 5 files changed, 784 insertions(+), 1 deletion(-) >>>> create mode 100644 lib/efi_loader/efi_hii.c >>>> >>>> diff --git a/include/efi_api.h b/include/efi_api.h >>>> index ffdba7fe1a..164147dc87 100644 >>>> --- a/include/efi_api.h >>>> +++ b/include/efi_api.h >>>> @@ -16,6 +16,7 @@ >>>> #define _EFI_API_H >>>> >>>> #include <efi.h> >>>> +#include <charset.h> >>>> >>>> #ifdef CONFIG_EFI_LOADER >>>> #include <asm/setjmp.h> >>>> @@ -536,6 +537,266 @@ struct efi_device_path_utilities_protocol { >>>> uint16_t node_length); >>>> }; >>>> >>>> +#define EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID \ >>>> + EFI_GUID(0x587e72d7, 0xcc50, 0x4f79, \ >>>> + 0x82, 0x09, 0xca, 0x29, 0x1f, 0xc1, 0xa1, 0x0f) >>>> + >>>> +typedef uint16_t efi_string_id_t; >>>> + >>>> +struct efi_hii_config_routing_protocol { >>>> + efi_status_t(EFIAPI *extract_config)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t request, >>>> + efi_string_t *progress, >>>> + efi_string_t *results); >>>> + efi_status_t(EFIAPI *export_config)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + efi_string_t *results); >>>> + efi_status_t(EFIAPI *route_config)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t configuration, >>>> + efi_string_t *progress); >>>> + efi_status_t(EFIAPI *block_to_config)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_request, >>>> + const uint8_t *block, >>>> + const efi_uintn_t block_size, >>>> + efi_string_t *config, >>>> + efi_string_t *progress); >>>> + efi_status_t(EFIAPI *config_to_block)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_resp, >>>> + const uint8_t *block, >>>> + const efi_uintn_t *block_size, >>>> + efi_string_t *progress); >>>> + efi_status_t(EFIAPI *get_alt_config)( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_resp, >>>> + const efi_guid_t *guid, >>>> + const efi_string_t name, >>>> + const struct efi_device_path *device_path, >>>> + const efi_string_t alt_cfg_id, >>>> + efi_string_t *alt_cfg_resp); >>>> +}; >>>> + >>>> +#define EFI_HII_DATABASE_PROTOCOL_GUID \ >>>> + EFI_GUID(0xef9fc172, 0xa1b2, 0x4693, \ >>>> + 0xb3, 0x27, 0x6d, 0x32, 0xfc, 0x41, 0x60, 0x42) >>>> + >>>> +typedef enum { >>>> + EFI_KEY_LCTRL, EFI_KEY_A0, EFI_KEY_LALT, EFI_KEY_SPACE_BAR, >>>> + EFI_KEY_A2, EFI_KEY_A3, EFI_KEY_A4, EFI_KEY_RCTRL, >>>> EFI_KEY_LEFT_ARROW, >>>> + EFI_KEY_DOWN_ARROW, EFI_KEY_RIGHT_ARROW, EFI_KEY_ZERO, >>>> + EFI_KEY_PERIOD, EFI_KEY_ENTER, EFI_KEY_LSHIFT, EFI_KEY_B0, >>>> + EFI_KEY_B1, EFI_KEY_B2, EFI_KEY_B3, EFI_KEY_B4, EFI_KEY_B5, >>>> EFI_KEY_B6, >>>> + EFI_KEY_B7, EFI_KEY_B8, EFI_KEY_B9, EFI_KEY_B10, EFI_KEY_RSHIFT, >>>> + EFI_KEY_UP_ARROW, EFI_KEY_ONE, EFI_KEY_TWO, EFI_KEY_THREE, >>>> + EFI_KEY_CAPS_LOCK, EFI_KEY_C1, EFI_KEY_C2, EFI_KEY_C3, EFI_KEY_C4, >>>> + EFI_KEY_C5, EFI_KEY_C6, EFI_KEY_C7, EFI_KEY_C8, EFI_KEY_C9, >>>> + EFI_KEY_C10, EFI_KEY_C11, EFI_KEY_C12, EFI_KEY_FOUR, EFI_KEY_FIVE, >>>> + EFI_KEY_SIX, EFI_KEY_PLUS, EFI_KEY_TAB, EFI_KEY_D1, EFI_KEY_D2, >>>> + EFI_KEY_D3, EFI_KEY_D4, EFI_KEY_D5, EFI_KEY_D6, EFI_KEY_D7, >>>> EFI_KEY_D8, >>>> + EFI_KEY_D9, EFI_KEY_D10, EFI_KEY_D11, EFI_KEY_D12, EFI_KEY_D13, >>>> + EFI_KEY_DEL, EFI_KEY_END, EFI_KEY_PG_DN, EFI_KEY_SEVEN, >>>> EFI_KEY_EIGHT, >>>> + EFI_KEY_NINE, EFI_KEY_E0, EFI_KEY_E1, EFI_KEY_E2, EFI_KEY_E3, >>>> + EFI_KEY_E4, EFI_KEY_E5, EFI_KEY_E6, EFI_KEY_E7, EFI_KEY_E8, >>>> EFI_KEY_E9, >>>> + EFI_KEY_E10, EFI_KEY_E11, EFI_KEY_E12, EFI_KEY_BACK_SPACE, >>>> + EFI_KEY_INS, EFI_KEY_HOME, EFI_KEY_PG_UP, EFI_KEY_NLCK, >>>> EFI_KEY_SLASH, >>>> + EFI_KEY_ASTERISK, EFI_KEY_MINUS, EFI_KEY_ESC, EFI_KEY_F1, EFI_KEY_F2, >>>> + EFI_KEY_F3, EFI_KEY_F4, EFI_KEY_F5, EFI_KEY_F6, EFI_KEY_F7, >>>> EFI_KEY_F8, >>>> + EFI_KEY_F9, EFI_KEY_F10, EFI_KEY_F11, EFI_KEY_F12, EFI_KEY_PRINT, >>>> + EFI_KEY_SLCK, EFI_KEY_PAUSE, >>>> +} efi_key; >>>> + >>>> +struct efi_key_descriptor { >>>> + efi_key key; >>>> + uint16_t unicode; >>>> + uint16_t shifted_unicode; >>>> + uint16_t alt_gr_unicode; >>>> + uint16_t shifted_alt_gr_unicode; >>>> + uint16_t modifier; >>>> + uint16_t affected_attribute; >>>> +}; >>>> + >>>> +struct efi_hii_keyboard_layout { >>>> + uint16_t layout_length; >>>> + efi_guid_t guid; >>>> + uint32_t layout_descriptor_string_offset; >>>> + uint8_t descriptor_count; >>>> + struct efi_key_descriptor descriptors[]; >>>> +}; >>>> + >>>> +struct efi_hii_package_list_header { >>>> + efi_guid_t package_list_guid; >>>> + uint32_t package_length; >>>> +} __packed; >>>> + >>>> +struct efi_hii_package_header { >>>> + uint32_t length : 24; >>>> + uint32_t type : 8; >>>> +} __packed; >>> >>> Bitfields are terribly evil - they're probably one of the worst defined >>> things in C. A different compiler may even give you different ordering >>> here. I've certainly seen bitfields explode in weird ways on >>> cross-endian conversions. >> >> edk2 defines it in the same way. And this is UEFI we are talking >> about here, big endian is strictly out of scope. > > I don't see why big endian is strictly out of scope. I don't want to > ignore it light heartedly. All we'd need to do is byte swap every > input/output there is today.
See section 1.9.1: " Supported processors are “little endian” machines. This distinction means that the low- order byte of a multibyte data item in memory is at the lowest address, while the high- order byte is at the highest address. Some supported 64-bit processors may be configured for both “little endian” and “big endian” operation. All implementations designed to conform to this specification use “little endian” operation. " even power9 got sane and is finally is switching to LE ;-) BR, -R > So please change it to not use bitmasks. I don't think we should copy > bad design decisions from edk2. > >> >> >>> Do you think you could just make that a uint32_t altogether and work >>> with MASK/SHIFT defines instead? >>> >>> >>>> + >>>> +#define EFI_HII_PACKAGE_TYPE_ALL 0x00 >>>> +#define EFI_HII_PACKAGE_TYPE_GUID 0x01 >>>> +#define EFI_HII_PACKAGE_FORMS 0x02 >>>> +#define EFI_HII_PACKAGE_STRINGS 0x04 >>>> +#define EFI_HII_PACKAGE_FONTS 0x05 >>>> +#define EFI_HII_PACKAGE_IMAGES 0x06 >>>> +#define EFI_HII_PACKAGE_SIMPLE_FONTS 0x07 >>>> +#define EFI_HII_PACKAGE_DEVICE_PATH 0x08 >>>> +#define EFI_HII_PACKAGE_KEYBOARD_LAYOUT 0x09 >>>> +#define EFI_HII_PACKAGE_ANIMATIONS 0x0A >>>> +#define EFI_HII_PACKAGE_END 0xDF >>>> +#define EFI_HII_PACKAGE_TYPE_SYSTEM_BEGIN 0xE0 >>>> +#define EFI_HII_PACKAGE_TYPE_SYSTEM_END 0xFF >>>> + >>>> +struct efi_hii_strings_package { >>>> + struct efi_hii_package_header header; >>>> + uint32_t header_size; >>>> + uint32_t string_info_offset; >>>> + uint16_t language_window[16]; >>>> + efi_string_id_t language_name; >>>> + uint8_t language[]; >>>> +} __packed; >>>> + >>>> +struct efi_hii_string_block { >>>> + uint8_t block_type; >>>> + /*uint8_t block_body[];*/ >>>> +} __packed; >>>> + >>>> +#define EFI_HII_SIBT_END 0x00 // The end of the string >>>> information. >>>> +#define EFI_HII_SIBT_STRING_SCSU 0x10 // Single string using >>>> default font information. >>>> +#define EFI_HII_SIBT_STRING_SCSU_FONT 0x11 // Single string with font >>>> information. >>>> +#define EFI_HII_SIBT_STRINGS_SCSU 0x12 // Multiple strings using >>>> default font information. >>>> +#define EFI_HII_SIBT_STRINGS_SCSU_FONT 0x13 // Multiple strings with font >>>> information. >>>> +#define EFI_HII_SIBT_STRING_UCS2 0x14 // Single UCS-2 string using >>>> default font information. >>>> +#define EFI_HII_SIBT_STRING_UCS2_FONT 0x15 // Single UCS-2 string with >>>> font information >>>> +#define EFI_HII_SIBT_STRINGS_UCS2 0x16 // Multiple UCS-2 strings >>>> using default font information. >>>> +#define EFI_HII_SIBT_STRINGS_UCS2_FONT 0x17 // Multiple UCS-2 strings >>>> with font information. >>>> +#define EFI_HII_SIBT_DUPLICATE 0x20 // Create a duplicate of an >>>> existing string. >>>> +#define EFI_HII_SIBT_SKIP2 0x21 // Skip a certain number of >>>> string identifiers. >>>> +#define EFI_HII_SIBT_SKIP1 0x22 // Skip a certain number of >>>> string identifiers. >>>> +#define EFI_HII_SIBT_EXT1 0x30 // For future expansion (one >>>> byte length field) >>>> +#define EFI_HII_SIBT_EXT2 0x31 // For future expansion (two >>>> byte length field) >>>> +#define EFI_HII_SIBT_EXT4 0x32 // For future expansion (four >>>> byte length field) >>>> +#define EFI_HII_SIBT_FONT 0x40 // Font information. >>>> + >>>> +struct efi_hii_sibt_string_ucs2_block { >>>> + struct efi_hii_string_block header; >>>> + uint16_t string_text[]; >>>> +} __packed; >>>> + >>>> +static inline struct efi_hii_string_block >>>> *efi_hii_sibt_string_ucs2_block_next( >>>> + struct efi_hii_sibt_string_ucs2_block *blk) >>>> +{ >>>> + return ((void *)blk) + sizeof(*blk) + >>>> + (utf16_strlen(blk->string_text) + 1) * 2; >>> >>> Since you're dealing with actual utf16, is this correct? One character >>> may as well span 4 bytes, right? >>> >>> I think we need a different function that actually tells us the bytes >>> occupied by a utf16 string. >> >> as mentioned on IRC (just repeating here for those who weren't >> following #u-boot), utf16_strlen() is actually telling us the number >> of 16b "bytes" in a utf16 string, not the number of "characters". >> Similar to strlen() with a utf8 string. >> >>>> +} >>>> + >>>> +typedef void *efi_hii_handle_t; >>>> + >>>> +struct efi_hii_database_protocol { >>>> + efi_status_t(EFIAPI *new_package_list)( >>>> + const struct efi_hii_database_protocol *this, >>>> + const struct efi_hii_package_list_header *package_list, >>>> + const efi_handle_t driver_handle, >>>> + efi_hii_handle_t *handle); >>>> + efi_status_t(EFIAPI *remove_package_list)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle); >>>> + efi_status_t(EFIAPI *update_package_list)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle, >>>> + const struct efi_hii_package_list_header *package_list); >>>> + efi_status_t(EFIAPI *list_package_lists)( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint8_t package_type, >>>> + const efi_guid_t *package_guid, >>>> + efi_uintn_t *handle_buffer_length, >>>> + efi_hii_handle_t *handle); >>>> + efi_status_t(EFIAPI *export_package_lists)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle, >>>> + efi_uintn_t *buffer_size, >>>> + struct efi_hii_package_list_header *buffer); >>>> + efi_status_t(EFIAPI *register_package_notify)( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint8_t package_type, >>>> + const efi_guid_t *package_guid, >>>> + const void *package_notify_fn, >>>> + efi_uintn_t notify_type, >>>> + efi_handle_t *notify_handle); >>>> + efi_status_t(EFIAPI *unregister_package_notify)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_handle_t notification_handle >>>> + ); >>>> + efi_status_t(EFIAPI *find_keyboard_layouts)( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint16_t *key_guid_buffer_length, >>>> + efi_guid_t *key_guid_buffer); >>>> + efi_status_t(EFIAPI *get_keyboard_layout)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_guid_t *key_guid, >>>> + uint16_t *keyboard_layout_length, >>>> + struct efi_hii_keyboard_layout *keyboard_layout); >>>> + efi_status_t(EFIAPI *set_keyboard_layout)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_guid_t *key_guid); >>>> + efi_status_t(EFIAPI *get_package_list_handle)( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t package_list_handle, >>>> + efi_handle_t *driver_handle); >>>> +}; >>>> + >>>> +#define EFI_HII_STRING_PROTOCOL_GUID \ >>>> + EFI_GUID(0x0fd96974, 0x23aa, 0x4cdc, \ >>>> + 0xb9, 0xcb, 0x98, 0xd1, 0x77, 0x50, 0x32, 0x2a) >>>> + >>>> +typedef uint32_t efi_hii_font_style_t; >>>> + >>>> +struct efi_font_info { >>>> + efi_hii_font_style_t font_style; >>>> + uint16_t font_size; >>>> + uint16_t font_name[1]; >>>> +}; >>>> + >>>> +struct efi_hii_string_protocol { >>>> + efi_status_t(EFIAPI *new_string)( >>>> + const struct efi_hii_string_protocol *this, >>>> + efi_hii_handle_t package_list, >>>> + efi_string_id_t *string_id, >>>> + const uint8_t *language, >>>> + const uint16_t *language_name, >>>> + const efi_string_t string, >>>> + const struct efi_font_info *string_font_info); >>>> + efi_status_t(EFIAPI *get_string)( >>>> + const struct efi_hii_string_protocol *this, >>>> + const uint8_t *language, >>>> + efi_hii_handle_t package_list, >>>> + efi_string_id_t string_id, >>>> + efi_string_t string, >>>> + efi_uintn_t *string_size, >>>> + struct efi_font_info **string_font_info); >>>> + efi_status_t(EFIAPI *set_string)( >>>> + const struct efi_hii_string_protocol *this, >>>> + efi_hii_handle_t package_list, >>>> + efi_string_id_t string_id, >>>> + const uint8_t *language, >>>> + const efi_string_t string, >>>> + const struct efi_font_info *string_font_info); >>>> + efi_status_t(EFIAPI *get_languages)( >>>> + const struct efi_hii_string_protocol *this, >>>> + efi_hii_handle_t package_list, >>>> + uint8_t *languages, >>>> + efi_uintn_t *languages_size); >>>> + efi_status_t(EFIAPI *get_secondary_languages)( >>>> + const struct efi_hii_string_protocol *this, >>>> + efi_hii_handle_t package_list, >>>> + const uint8_t *primary_language, >>>> + uint8_t *secondary_languages, >>>> + efi_uintn_t *secondary_languages_size); >>>> +}; >>>> + >>>> #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 5d37c1d75f..591bf07e7a 100644 >>>> --- a/include/efi_loader.h >>>> +++ b/include/efi_loader.h >>>> @@ -80,6 +80,9 @@ extern struct efi_simple_input_interface efi_con_in; >>>> extern const struct efi_console_control_protocol efi_console_control; >>>> extern const struct efi_device_path_to_text_protocol >>>> efi_device_path_to_text; >>>> 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; >>>> >>>> uint16_t *efi_dp_str(struct efi_device_path *dp); >>>> >>>> @@ -91,6 +94,9 @@ extern const efi_guid_t >>>> efi_guid_device_path_to_text_protocol; >>>> extern const efi_guid_t efi_simple_file_system_protocol_guid; >>>> extern const efi_guid_t efi_file_info_guid; >>>> 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 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 b6927b3b84..725e0cba85 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 >>>> +obj-y += efi_device_path_utilities.o efi_hii.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 92c778fcca..c179afc25a 100644 >>>> --- a/lib/efi_loader/efi_boottime.c >>>> +++ b/lib/efi_loader/efi_boottime.c >>>> @@ -1157,6 +1157,15 @@ void efi_setup_loaded_image(struct efi_loaded_image >>>> *info, struct efi_object *ob >>>> obj->protocols[4].protocol_interface = >>>> (void *)&efi_device_path_utilities; >>>> >>>> + obj->protocols[5].guid = &efi_guid_hii_string_protocol; >>>> + obj->protocols[5].protocol_interface = (void *)&efi_hii_string; >>>> + >>>> + obj->protocols[6].guid = &efi_guid_hii_database_protocol; >>>> + obj->protocols[6].protocol_interface = (void *)&efi_hii_database; >>>> + >>>> + obj->protocols[7].guid = &efi_guid_hii_config_routing_protocol; >>>> + obj->protocols[7].protocol_interface = (void >>>> *)&efi_hii_config_routing; >>>> + >>>> info->file_path = file_path; >>>> info->device_handle = efi_dp_find_obj(device_path, NULL); >>>> >>>> diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c >>>> new file mode 100644 >>>> index 0000000000..25c8e88a60 >>>> --- /dev/null >>>> +++ b/lib/efi_loader/efi_hii.c >>>> @@ -0,0 +1,507 @@ >>>> +/* >>>> + * EFI Human Interface Infrastructure ... interface >>>> + * >>>> + * Copyright (c) 2017 Leif Lindholm >>>> + * >>>> + * SPDX-License-Identifier: GPL-2.0+ >>>> + */ >>>> + >>>> +#include <common.h> >>>> +#include <malloc.h> >>>> +#include <efi_loader.h> >>>> + >>>> +const efi_guid_t efi_guid_hii_config_routing_protocol = >>>> + EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID; >>>> +const efi_guid_t efi_guid_hii_database_protocol = >>>> EFI_HII_DATABASE_PROTOCOL_GUID; >>>> +const efi_guid_t efi_guid_hii_string_protocol = >>>> EFI_HII_STRING_PROTOCOL_GUID; >>>> + >>>> +struct hii_package { >>>> + // TODO should there be an associated efi_object? >>>> + struct list_head string_tables; /* list of string_table */ >>>> + /* we could also track fonts, images, etc */ >>>> +}; >>>> + >>>> +struct string_table { >>>> + struct list_head link; >>>> + efi_string_id_t language_name; >>>> + char *language; >>>> + uint32_t nstrings; >>>> + /* NOTE: string id starts at 1 so value is stbl->strings[id-1] */ >>>> + struct { >>>> + efi_string_t string; >>>> + /* we could also track font info, etc */ >>>> + } strings[]; >>>> +}; >>>> + >>>> +static void free_strings_table(struct string_table *stbl) >>>> +{ >>>> + int i; >>>> + >>>> + for (i = 0; i < stbl->nstrings; i++) >>>> + free(stbl->strings[i].string); >>>> + free(stbl->language); >>>> + free(stbl); >>>> +} >>>> + >>>> +static struct hii_package *new_package(void) >>>> +{ >>>> + struct hii_package *hii = malloc(sizeof(*hii)); >>>> + INIT_LIST_HEAD(&hii->string_tables); >>>> + return hii; >>>> +} >>>> + >>>> +static void free_package(struct hii_package *hii) >>>> +{ >>>> + >>>> + while (!list_empty(&hii->string_tables)) { >>>> + struct string_table *stbl; >>>> + >>>> + stbl = list_first_entry(&hii->string_tables, >>>> + struct string_table, link); >>>> + list_del(&stbl->link); >>>> + free_strings_table(stbl); >>>> + } >>>> + >>>> + free(hii); >>>> +} >>>> + >>>> +static efi_status_t add_strings_package(struct hii_package *hii, >>>> + struct efi_hii_strings_package *strings_package) >>>> +{ >>>> + struct efi_hii_string_block *block; >>>> + void *end = ((void *)strings_package) + >>>> strings_package->header.length; >>>> + uint32_t nstrings = 0; >>>> + unsigned id = 0; >>>> + >>>> + debug("header_size: %08x\n", strings_package->header_size); >>>> + debug("string_info_offset: %08x\n", >>>> strings_package->string_info_offset); >>>> + debug("language_name: %u\n", strings_package->language_name); >>>> + debug("language: %s\n", strings_package->language); >>>> + >>>> + /* count # of string entries: */ >>>> + block = ((void *)strings_package) + >>>> strings_package->string_info_offset; >>>> + while ((void *)block < end) { >>>> + switch (block->block_type) { >>>> + case EFI_HII_SIBT_STRING_UCS2: { >>>> + struct efi_hii_sibt_string_ucs2_block *ucs2 = >>>> + (void *)block; >>>> + nstrings++; >>>> + block = efi_hii_sibt_string_ucs2_block_next(ucs2); >>>> + break; >>>> + } >>>> + case EFI_HII_SIBT_END: >>>> + block = end; >>>> + break; >>>> + default: >>>> + debug("unknown HII string block type: %02x\n", >>>> + block->block_type); >>>> + return EFI_INVALID_PARAMETER; >>>> + } >>>> + } >>>> + >>>> + struct string_table *stbl = malloc(sizeof(*stbl) + >>>> + (nstrings * sizeof(stbl->strings[0]))); >>>> + stbl->language_name = strings_package->language_name; >>>> + stbl->language = strdup((char *)strings_package->language); >>> >>> Where does the strings_package come from? And why is the language in it >>> in UTF8? >> >> The strings_package is a part of the "package list" blob passed in >> from (in this case) Shell.efi. The package list can actually contain >> a lot more (fonts/glyphs/images/forms), but not really sure how much >> of that we'll actually support. (HII seems to be able to do enough >> for rendering a full blown GUI.. kinda overkill, IMHO.. but Shell.efi >> wants it for silly reasons.) >> >> This is actually utf8.. it is a "RFC 4646 language code identifier". >> See appendix M. >> >> >>> >>>> + stbl->nstrings = nstrings; >>>> + >>>> + list_add(&stbl->link, &hii->string_tables); >>>> + >>>> + /* and now parse string entries and populate string_table */ >>>> + block = ((void *)strings_package) + >>>> strings_package->string_info_offset; >>>> + >>>> + while ((void *)block < end) { >>>> + switch (block->block_type) { >>>> + case EFI_HII_SIBT_STRING_UCS2: { >>>> + struct efi_hii_sibt_string_ucs2_block *ucs2 = >>>> + (void *)block; >>>> + id++; >>>> + debug("%4u: \"%ls\"\n", id, ucs2->string_text); >>>> + stbl->strings[id-1].string = >>>> + utf16_strdup(ucs2->string_text); >>>> + block = efi_hii_sibt_string_ucs2_block_next(ucs2); >>>> + break; >>>> + } >>>> + case EFI_HII_SIBT_END: >>>> + return EFI_SUCCESS; >>>> + default: >>>> + debug("unknown HII string block type: %02x\n", >>>> + block->block_type); >>>> + return EFI_INVALID_PARAMETER; >>>> + } >>>> + } >>>> + >>>> + return EFI_SUCCESS; >>>> +} >>>> + >>>> +/* >>>> + * EFI_HII_CONFIG_ROUTING_PROTOCOL >>>> + */ >>>> + >>>> +static efi_status_t EFIAPI extract_config( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t request, >>>> + efi_string_t *progress, >>>> + efi_string_t *results) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %p, %p", this, request, progress, results); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI export_config( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + efi_string_t *results) >>>> +{ >>>> + EFI_ENTRY("%p, %p", this, results); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI route_config( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t configuration, >>>> + efi_string_t *progress) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %p", this, configuration, progress); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI block_to_config( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_request, >>>> + const uint8_t *block, >>>> + const efi_uintn_t block_size, >>>> + efi_string_t *config, >>>> + efi_string_t *progress) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %p, %zu, %p, %p", this, config_request, >>>> block, >>>> + block_size, config, progress); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI config_to_block( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_resp, >>>> + const uint8_t *block, >>>> + const efi_uintn_t *block_size, >>>> + efi_string_t *progress) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %p, %p, %p", this, config_resp, block, >>>> + block_size, progress); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI get_alt_config( >>>> + const struct efi_hii_config_routing_protocol *this, >>>> + const efi_string_t config_resp, >>>> + const efi_guid_t *guid, >>>> + const efi_string_t name, >>>> + const struct efi_device_path *device_path, >>>> + const efi_string_t alt_cfg_id, >>>> + efi_string_t *alt_cfg_resp) >>>> +{ >>>> + EFI_ENTRY("%p, \"%ls\", %pUl, \"%ls\", %p, \"%ls\", %p", this, >>>> + config_resp, guid, name, device_path, alt_cfg_id, >>>> + alt_cfg_resp); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> + >>>> +/* >>>> + * EFI_HII_DATABASE_PROTOCOL >>>> + */ >>>> + >>>> +static efi_status_t EFIAPI new_package_list( >>>> + const struct efi_hii_database_protocol *this, >>>> + const struct efi_hii_package_list_header *package_list, >>>> + const efi_handle_t driver_handle, >>>> + efi_hii_handle_t *handle) >>>> +{ >>>> + efi_status_t ret = EFI_SUCCESS; >>>> + >>>> + EFI_ENTRY("%p, %p, %p, %p", this, package_list, driver_handle, >>>> handle); >>>> + >>>> + if (!package_list || !driver_handle) >>>> + return EFI_EXIT(EFI_INVALID_PARAMETER); >>>> + >>>> + struct hii_package *hii = new_package(); >>>> + struct efi_hii_package_header *package; >>>> + void *end = ((void *)package_list) + package_list->package_length; >>>> + >>>> + debug("package_list: %pUl (%u)\n", &package_list->package_list_guid, >>>> + package_list->package_length); >>>> + >>>> + package = ((void *)package_list) + sizeof(*package_list); >>>> + while ((void *)package < end) { >>>> + debug("package=%p, package type=%x, length=%u\n", package, >>>> + package->type, package->length); >>>> + switch (package->type) { >>>> + case EFI_HII_PACKAGE_STRINGS: >>>> + ret = add_strings_package(hii, >>>> + (struct efi_hii_strings_package *)package); >>>> + break; >>>> + default: >>>> + break; >>>> + } >>>> + >>>> + if (ret != EFI_SUCCESS) >>>> + goto error; >>>> + >>>> + package = ((void *)package) + package->length; >>>> + } >>>> + >>>> + // TODO in theory there is some notifications that should be sent.. >>>> + >>>> + *handle = hii; >>>> + >>>> + return EFI_EXIT(EFI_SUCCESS); >>>> + >>>> +error: >>>> + free_package(hii); >>>> + return EFI_EXIT(ret); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI remove_package_list( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle) >>>> +{ >>>> + struct hii_package *hii = handle; >>>> + EFI_ENTRY("%p, %p", this, handle); >>>> + free_package(hii); >>>> + return EFI_EXIT(EFI_SUCCESS); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI update_package_list( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle, >>>> + const struct efi_hii_package_list_header *package_list) >>>> +{ >>>> + EFI_ENTRY("%p, %p, %p", this, handle, package_list); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI list_package_lists( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint8_t package_type, >>>> + const efi_guid_t *package_guid, >>>> + efi_uintn_t *handle_buffer_length, >>>> + efi_hii_handle_t *handle) >>>> +{ >>>> + EFI_ENTRY("%p, %u, %pUl, %p, %p", this, package_type, package_guid, >>>> + handle_buffer_length, handle); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI export_package_lists( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t handle, >>>> + efi_uintn_t *buffer_size, >>>> + struct efi_hii_package_list_header *buffer) >>>> +{ >>>> + EFI_ENTRY("%p, %p, %p, %p", this, handle, buffer_size, buffer); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI register_package_notify( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint8_t package_type, >>>> + const efi_guid_t *package_guid, >>>> + const void *package_notify_fn, >>>> + efi_uintn_t notify_type, >>>> + efi_handle_t *notify_handle) >>>> +{ >>>> + EFI_ENTRY("%p, %u, %pUl, %p, %zu, %p", this, package_type, >>>> + package_guid, package_notify_fn, notify_type, >>>> + notify_handle); >>>> + return EFI_EXIT(EFI_OUT_OF_RESOURCES); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI unregister_package_notify( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_handle_t notification_handle) >>>> +{ >>>> + EFI_ENTRY("%p, %p", this, notification_handle); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI find_keyboard_layouts( >>>> + const struct efi_hii_database_protocol *this, >>>> + uint16_t *key_guid_buffer_length, >>>> + efi_guid_t *key_guid_buffer) >>>> +{ >>>> + EFI_ENTRY("%p, %p, %p", this, key_guid_buffer_length, >>>> key_guid_buffer); >>>> + return EFI_EXIT(EFI_NOT_FOUND); /* Invalid */ >>>> +} >>>> + >>>> +static efi_status_t EFIAPI get_keyboard_layout( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_guid_t *key_guid, >>>> + uint16_t *keyboard_layout_length, >>>> + struct efi_hii_keyboard_layout *keyboard_layout) >>>> +{ >>>> + EFI_ENTRY("%p, %pUl, %p, %p", this, key_guid, keyboard_layout_length, >>>> + keyboard_layout); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI set_keyboard_layout( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_guid_t *key_guid) >>>> +{ >>>> + EFI_ENTRY("%p, %pUl", this, key_guid); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI get_package_list_handle( >>>> + const struct efi_hii_database_protocol *this, >>>> + efi_hii_handle_t package_list_handle, >>>> + efi_handle_t *driver_handle) >>>> +{ >>>> + EFI_ENTRY("%p, %p, %p", this, package_list_handle, driver_handle); >>>> + return EFI_EXIT(EFI_INVALID_PARAMETER); >>>> +} >>>> + >>>> + >>>> +/* >>>> + * EFI_HII_STRING_PROTOCOL >>>> + */ >>>> + >>>> +static efi_status_t EFIAPI new_string( >>>> + const struct efi_hii_string_protocol *this, >>>> + efi_hii_handle_t package_list, >>>> + efi_string_id_t *string_id, >>>> + const uint8_t *language, >>>> + const uint16_t *language_name, >>>> + const efi_string_t string, >>>> + const struct efi_font_info *string_font_info) >>>> +{ >>>> + EFI_ENTRY("%p, %p, %p, \"%s\", %p, \"%ls\", %p", this, package_list, >>>> + string_id, language, language_name, string, >>>> + string_font_info); >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> +} >>>> + >>>> +static efi_status_t EFIAPI get_string( >>>> + const struct efi_hii_string_protocol *this, >>>> + const uint8_t *language, >>>> + efi_hii_handle_t package_list, >>>> + efi_string_id_t string_id, >>>> + efi_string_t string, >>>> + efi_uintn_t *string_size, >>>> + struct efi_font_info **string_font_info) >>>> +{ >>>> + struct hii_package *hii = package_list; >>>> + struct string_table *stbl; >>>> + >>>> + EFI_ENTRY("%p, \"%s\", %p, %u, %p, %p, %p", this, language, >>>> + package_list, string_id, string, string_size, >>>> + string_font_info); >>>> + >>>> + list_for_each_entry(stbl, &hii->string_tables, link) { >>>> + if (!strcmp((char *)language, (char *)stbl->language)) { >>>> + unsigned idx = string_id - 1; >>>> + if (idx > stbl->nstrings) >>>> + return EFI_EXIT(EFI_NOT_FOUND); >>>> + efi_string_t str = stbl->strings[idx].string; >>>> + size_t len = utf16_strlen(str) + 1; >>> >>> I assume that's wrong for sizing too? >> >> nope :-) >> >>> Also please try not to define variables mid-scope :). Just define them >>> above the if() and fill them after ... >> >> Hmm, I'd have the inverse comment if someone else wrote it and defined >> all the variables at the top ;-) >> >>> Then also for the sake of readability add a blank line after the >>> variable definitions and before the return. >>> >>> I think you'd also do yourself a favor if you reduced the indenting a >>> bit. Just abort the list_for_each when you found and entry and then >>> process it in the top level scope. >>> >>> static struct string_table *find_stbl_by_lang(const char *language) >>> { >>> list_for_each(...) { >>> if (matches) { >>> return stbl; >>> } >>> } >>> >>> return NULL; >>> } >>> >>> stbl = find_stbl_by_lang(lang); >>> if (!stbl) >>> return EFI_EXIT(EFI_NOT_FOUND) >> >> yeah, perhaps > > Please rework it for v2 :) > > > Alex _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot