[U-Boot] [PATCH 02/11] efi_loader: Initial HII protocols

Rob Clark robdclark at gmail.com
Wed Oct 11 22:02:07 UTC 2017


On Wed, Oct 11, 2017 at 10:30 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>
>>
>> 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.lindholm at linaro.org>
>> Signed-off-by: Rob Clark <robdclark at 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.


> 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

BR,
-R

> work_with_stbl;
>
> return EFI_EXIT(EFI_SUCCESS);
>
>
> Alex
>
>> +                     if (*string_size < len * 2) {
>> +                             *string_size = len * 2;
>> +                             return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
>> +                     }
>> +                     memcpy(string, str, len * 2);
>> +                     *string_size = len * 2;
>> +                     return EFI_EXIT(EFI_SUCCESS);
>> +             }
>> +     }
>> +
>> +     return EFI_EXIT(EFI_NOT_FOUND);
>> +}
>> +
>> +static 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_ENTRY("%p, %p, %u, \"%s\", \"%ls\", %p", this, package_list,
>> +               string_id, language, string, string_font_info);
>> +     return EFI_EXIT(EFI_NOT_FOUND);
>> +}
>> +
>> +static 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)
>> +{
>> +     struct hii_package *hii = package_list;
>> +     struct string_table *stbl;
>> +     size_t len = 0;
>> +
>> +     EFI_ENTRY("%p, %p, %p, %p", this, package_list, languages,
>> +               languages_size);
>> +
>> +     /* figure out required size: */
>> +     list_for_each_entry(stbl, &hii->string_tables, link) {
>> +             len += strlen((char *)stbl->language) + 1;
>> +     }
>> +
>> +     if (*languages_size < len) {
>> +             *languages_size = len;
>> +             return EFI_EXIT(EFI_BUFFER_TOO_SMALL);
>> +     }
>> +
>> +     char *p = (char *)languages;
>> +     list_for_each_entry(stbl, &hii->string_tables, link) {
>> +             if (p != (char *)languages)
>> +                     p += sprintf(p, ";");
>> +             p += sprintf(p, "%s", stbl->language);
>> +     }
>> +
>> +     debug("languages: %s\n", languages);
>> +
>> +     return EFI_EXIT(EFI_SUCCESS);
>> +}
>> +
>> +static 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)
>> +{
>> +     EFI_ENTRY("%p, %p, \"%s\", %p, %p", this, package_list,
>> +               primary_language, secondary_languages,
>> +               secondary_languages_size);
>> +     return EFI_EXIT(EFI_NOT_FOUND);
>> +}
>> +
>> +const struct efi_hii_config_routing_protocol efi_hii_config_routing = {
>> +     .extract_config = extract_config,
>> +     .export_config = export_config,
>> +     .route_config = route_config,
>> +     .block_to_config = block_to_config,
>> +     .config_to_block = config_to_block,
>> +     .get_alt_config = get_alt_config
>> +};
>> +const struct efi_hii_database_protocol efi_hii_database = {
>> +     .new_package_list = new_package_list,
>> +     .remove_package_list = remove_package_list,
>> +     .update_package_list = update_package_list,
>> +     .list_package_lists = list_package_lists,
>> +     .export_package_lists = export_package_lists,
>> +     .register_package_notify = register_package_notify,
>> +     .unregister_package_notify = unregister_package_notify,
>> +     .find_keyboard_layouts = find_keyboard_layouts,
>> +     .get_keyboard_layout = get_keyboard_layout,
>> +     .set_keyboard_layout = set_keyboard_layout,
>> +     .get_package_list_handle = get_package_list_handle
>> +};
>> +const struct efi_hii_string_protocol efi_hii_string = {
>> +     .new_string = new_string,
>> +     .get_string = get_string,
>> +     .set_string = set_string,
>> +     .get_languages = get_languages,
>> +     .get_secondary_languages = get_secondary_languages
>> +};
>>


More information about the U-Boot mailing list