[U-Boot] [RESEND PATCH v2 2/6] efi_loader: Initial HII database protocols

Heinrich Schuchardt xypron.glpk at gmx.de
Sun Dec 16 20:36:38 UTC 2018


On 12/14/18 11:10 AM, AKASHI Takahiro wrote:
> From: Leif Lindholm <leif.lindholm at linaro.org>
> 
> This patch provides enough implementation of the following protocols to
> run EDKII's Shell.efi and UEFI SCT:
> 
>   * EfiHiiDatabaseProtocol
>   * EfiHiiStringProtocol
> 
> Not implemented are:
>   * ExportPackageLists()
>   * RegisterPackageNotify()/UnregisterPackageNotify()
>   * SetKeyboardLayout() (i.e. *current* keyboard layout)
> 
> HII database protocol can handle only:
>   * GUID package
>   * string package
>   * keyboard layout package
>   (The other packages, except Device path package, will be necessary
>    for interactive and graphical UI.)
> 
> We'll fill in the rest once SCT is running properly so we can validate
> the implementation against the conformance test suite.
> 
> Cc: Leif Lindholm <leif.lindholm at linaro.org>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> Signed-off-by: AKASHI Takahiro <takahiro.akashi at linaro.org>
> ---
>  include/efi_api.h             | 242 ++++++++++
>  include/efi_loader.h          |   4 +
>  lib/efi_loader/Makefile       |   1 +
>  lib/efi_loader/efi_boottime.c |  12 +
>  lib/efi_loader/efi_hii.c      | 886 ++++++++++++++++++++++++++++++++++
>  5 files changed, 1145 insertions(+)
>  create mode 100644 lib/efi_loader/efi_hii.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index aef77b6319de..c9dbd5a6037d 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -17,6 +17,7 @@
>  #define _EFI_API_H
>  
>  #include <efi.h>
> +#include <charset.h>
>  
>  #ifdef CONFIG_EFI_LOADER
>  #include <asm/setjmp.h>
> @@ -697,6 +698,247 @@ struct efi_device_path_utilities_protocol {
>  		uint16_t node_length);
>  };
>  
> +typedef u16 efi_string_id_t;
> +
> +#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;

Hello Takahiro,

with the patch I can start the EFI shell. But I am still trying to check
the different definitions in this file.

As mentioned in prior mail the size of enum is not defined in the C
spec. So better use u32.

> +	u16 unicode;
> +	u16 shifted_unicode;
> +	u16 alt_gr_unicode;
> +	u16 shifted_alt_gr_unicode;
> +	u16 modifier;
> +	u16 affected_attribute;
> +};
> +
> +struct efi_hii_keyboard_layout {
> +	u16 layout_length;
> +	efi_guid_t guid;

A patch to change the alignment of efi_guid_t to __alinged(8) has been
merged into efi-next.

> +	u32 layout_descriptor_string_offset;
> +	u8 descriptor_count;
> +	struct efi_key_descriptor descriptors[];
> +};
> +
> +struct efi_hii_package_list_header {
> +	efi_guid_t package_list_guid;
> +	u32 package_length;
> +} __packed;

You are defining several structures as __packed. It is unclear to me
where I can find this in the UEFI spec. Looking at EDK2 I could not find
the same __packed attributes.

If this packing is necessary a comment in the code would be helpful.

> +
> +/**
> + * struct efi_hii_package_header - EFI HII package header
> + *
> + * @fields:	'fields' replaces the bit-fields defined in the EFI
> + *		specification to to avoid possible compiler incompatibilities::
> + *
> + *		u32 length:24;
> + *		u32 type:8;
> + */
> +struct efi_hii_package_header {
> +	u32 fields;
> +} __packed;

Same here.

> +
> +#define __EFI_HII_PACKAGE_LEN_SHIFT	0
> +#define __EFI_HII_PACKAGE_TYPE_SHIFT	24
> +#define __EFI_HII_PACKAGE_LEN_MASK	0xffffff
> +#define __EFI_HII_PACKAGE_TYPE_MASK	0xff
> +
> +#define EFI_HII_PACKAGE_HEADER(header, field) \
> +		(((header)->fields >> __EFI_HII_PACKAGE_##field##_SHIFT) \
> +		 & __EFI_HII_PACKAGE_##field##_MASK)
> +#define efi_hii_package_len(header) \
> +		EFI_HII_PACKAGE_HEADER(header, LEN)
> +#define efi_hii_package_type(header) \
> +		EFI_HII_PACKAGE_HEADER(header, TYPE)
> +
> +#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;
> +	u32 header_size;
> +	u32 string_info_offset;
> +	u16 language_window[16];
> +	efi_string_id_t language_name;
> +	u8  language[];
> +} __packed;

Same here.

> +
> +struct efi_hii_string_block {
> +	u8 block_type;
> +	/* u8 block_body[]; */
> +} __packed;

Same here.

Best regards

Heinrich


More information about the U-Boot mailing list