On Thu, Nov 01, 2018 at 08:09:45AM +0100, Heinrich Schuchardt wrote:
> On 11/01/2018 05:47 AM, AKASHI Takahiro wrote:
> > From: Leif Lindholm <leif.lindh...@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 oterh packages, exept 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.lindh...@linaro.org>
> > Signed-off-by: Rob Clark <robdcl...@gmail.com>
> > Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> > ---
> > include/efi_api.h | 233 +++++++++
> > 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, 1136 insertions(+)
> > create mode 100644 lib/efi_loader/efi_hii.c
> >
> > diff --git a/include/efi_api.h b/include/efi_api.h
> > index 88a60070f6ab..ec1b759d810a 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,238 @@ 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;
> > + 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;
> > + 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;
> > +
>
> Here you are taking Alex's comment into account where he recommended to
> avoid bit-fields.
>
> Please, add a comment indicating the sub-fields hidden in 'fields', e.g.
Sure.
> /**
> * 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;
> */
>
> (The double colon :: is the Sphinx indicator for preformatted text.)
>
> > +struct efi_hii_package_header {
> > + u32 fields;
> > +} __packed;
> > +
> > +#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
> > +
>
> These macros are U-Boot specific. Please, describe them.
I think that the meanings will be trivial with a description above.
Or do you mean that we should say clearly that those are u-boot specific?
Thanks,
-Takahiro Akashi
> > +#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)
> > +
>
> Best regards
>
> Heinrich
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
