[U-Boot] [PATCH v1 00/12] efi_loader+video: support for Shell.efi

[U-Boot] [PATCH v1 00/12] efi_loader+video: support for Shell.efi

[Rob Clark]

This patchset gets Shell.efi working to the point where we can start
running SCT.efi (the UEFI test suite).  There is more fat/fs work needed
so that SCT can actually write results to a file so we can even see what
is passing and what is not.

This applies on top of the "efi_loader: enough UEFI for standard distro
boot" patchset plus two patches from Heinrich which are also required:

  efi_loader: support 16 protocols per efi_object
  efi_loader: allow creating new handles

Leif added stubbed implementations for the additional protocols that
Shell.efi required, on top of what was added in the standard distro boot
patchset, and I fleshed out the implementation enough for what Shell/
SCT required.  There are still parts unimplemented, but IMHO the better
thing to do is concentrate on what is needed to get SCT running properly
so that we can implement the remaining bits having tests to test the
implementation.

There are 3 dm/video patches at the end, which aren't strictly required
but fix issues with Shell running on vidconsole.  (It is very convenient
for debugging to have Shell on screen with u-boot debug prints going to
serial.)

The last HACK patch is not intended to be merged, just to show the
remaining TODOs to have things working properly.

Leif Lindholm (3):
  efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL
  efi_loader: add stub HII protocols
  efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

Rob Clark (9):
  efi_loader: start fleshing out HII
  efi_loader: flesh out unicode protocol
  efi_loader: start fleshing out efi_device_path_utilities
  efi_loader: SIMPLE_TEXT_INPUT_EX plus wire up objects properly
  efi_loader: console support for color attributes
  dm: video: Fix cache flushes
  dm: video: Add basic ANSI escape sequence support
  dm: video: Add color ANSI escape sequence support
  HACK: efi_loader: hacks for SCT

 drivers/video/vidconsole-uclass.c          | 209 ++++++++++++
 drivers/video/video-uclass.c               |   4 +-
 include/config_distro_bootcmd.h            |   2 +-
 include/efi_api.h                          | 429 +++++++++++++++++++++++-
 include/efi_loader.h                       |  21 +-
 include/video.h                            |   7 +
 include/video_console.h                    |  11 +
 lib/efi_loader/Makefile                    |   1 +
 lib/efi_loader/efi_boottime.c              |  26 +-
 lib/efi_loader/efi_console.c               | 160 ++++++++-
 lib/efi_loader/efi_device_path_utilities.c |  87 +++++
 lib/efi_loader/efi_file.c                  |  11 +-
 lib/efi_loader/efi_hii.c                   | 505 +++++++++++++++++++++++++++++
 lib/efi_loader/efi_unicode.c               | 170 ++++++++++
 14 files changed, 1610 insertions(+), 33 deletions(-)
 create mode 100644 lib/efi_loader/efi_device_path_utilities.c
 create mode 100644 lib/efi_loader/efi_hii.c
 create mode 100644 lib/efi_loader/efi_unicode.c

-- 
2.13.5

[U-Boot] [PATCH v1 01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL

[Rob Clark]

From: Leif Lindholm <leif.lindholm@linaro.org>

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 include/efi_api.h                          | 30 +++++++++++
 include/efi_loader.h                       |  2 +
 lib/efi_loader/Makefile                    |  1 +
 lib/efi_loader/efi_boottime.c              |  4 ++
 lib/efi_loader/efi_device_path_utilities.c | 83 ++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+)
 create mode 100644 lib/efi_loader/efi_device_path_utilities.c

diff --git a/include/efi_api.h b/include/efi_api.h
index c3b9032a48..57468dd972 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -506,6 +506,36 @@ struct efi_device_path_to_text_protocol
 			bool allow_shortcuts);
 };
 
+#define EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID \
+	EFI_GUID(0x0379be4e, 0xd706, 0x437d, \
+		 0xb0, 0x37, 0xed, 0xb8, 0x2f, 0xb7, 0x72, 0xa4)
+
+struct efi_device_path_utilities_protocol
+{
+	UINTN(EFIAPI *get_device_path_size)(
+		const struct efi_device_path *device_path);
+	struct efi_device_path *(EFIAPI *duplicate_device_path)(
+		const struct efi_device_path *device_path);
+	struct efi_device_path *(EFIAPI *append_device_path)(
+		const struct efi_device_path *src1,
+		const struct efi_device_path *src2);
+	struct efi_device_path *(EFIAPI *append_device_node)(
+		const struct efi_device_path *device_path,
+		const struct efi_device_path *device_node);
+	struct efi_device_path *(EFIAPI *append_device_path_instance)(
+		const struct efi_device_path *device_path,
+		const struct efi_device_path *device_path_instance);
+	struct efi_device_path *(EFIAPI *get_next_device_path_instance)(
+		struct efi_device_path **device_path_instance,
+		UINTN *device_path_instance_size);
+	struct efi_device_path *(EFIAPI *create_device_node)(
+		uint8_t node_type,
+		uint8_t node_sub_type,
+		uint16_t node_length);
+	bool(EFIAPI *is_device_path_multi_instance)(
+		const struct efi_device_path *device_path);
+};
+
 #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 43b12b94fa..c009828db9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -58,6 +58,7 @@ extern const struct efi_simple_text_output_protocol efi_con_out;
 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;
 
 uint16_t *efi_dp_str(struct efi_device_path *dp);
 
@@ -68,6 +69,7 @@ extern const efi_guid_t efi_guid_loaded_image;
 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 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 930c0e218e..f5e69dd078 100644
--- a/lib/efi_loader/Makefile
+++ b/lib/efi_loader/Makefile
@@ -16,6 +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
 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 3860feb79b..8bb243d673 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -775,6 +775,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
 	obj->protocols[3].protocol_interface =
 		(void *)&efi_device_path_to_text;
 
+	obj->protocols[4].guid = &efi_guid_device_path_utilities_protocol;
+	obj->protocols[4].protocol_interface =
+		(void *)&efi_device_path_utilities;
+
 	info->file_path = file_path;
 	info->device_handle = efi_dp_find_obj(device_path, NULL);
 
diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
new file mode 100644
index 0000000000..4b97080f35
--- /dev/null
+++ b/lib/efi_loader/efi_device_path_utilities.c
@@ -0,0 +1,83 @@
+/*
+ *  EFI device path 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_device_path_utilities_protocol =
+		EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
+
+static UINTN get_device_path_size(const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(0);
+}
+
+static struct efi_device_path *duplicate_device_path(
+	const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *append_device_path(
+	const struct efi_device_path *src1,
+	const struct efi_device_path *src2)
+{
+	EFI_ENTRY("%p, %p", src1, src2);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *append_device_node(
+	const struct efi_device_path *device_path,
+	const struct efi_device_path *device_node)
+{
+	EFI_ENTRY("%p, %p", device_path, device_node);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *append_device_path_instance(
+	const struct efi_device_path *device_path,
+	const struct efi_device_path *device_path_instance)
+{
+	EFI_ENTRY("%p, %p", device_path, device_path_instance);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *get_next_device_path_instance(
+	struct efi_device_path **device_path_instance,
+	UINTN *device_path_instance_size)
+{
+	EFI_ENTRY("%p, %p", device_path_instance, device_path_instance_size);
+	return EFI_EXIT(NULL);
+}
+
+static struct efi_device_path *create_device_node(
+	uint8_t node_type, uint8_t node_sub_type, uint16_t node_length)
+{
+	EFI_ENTRY("%u, %u, %u", node_type, node_sub_type, node_length);
+	return EFI_EXIT(NULL);
+}
+
+static bool is_device_path_multi_instance(
+	const struct efi_device_path *device_path)
+{
+	EFI_ENTRY("%p", device_path);
+	return EFI_EXIT(false);
+}
+
+const struct efi_device_path_utilities_protocol efi_device_path_utilities = {
+	.get_device_path_size = get_device_path_size,
+	.duplicate_device_path = duplicate_device_path,
+	.append_device_path = append_device_path,
+	.append_device_node = append_device_node,
+	.append_device_path_instance = append_device_path_instance,
+	.get_next_device_path_instance = get_next_device_path_instance,
+	.create_device_node = create_device_node,
+	.is_device_path_multi_instance = is_device_path_multi_instance,
+};
-- 
2.13.5

[U-Boot] [PATCH v1 02/12] efi_loader: add stub HII protocols

[Rob Clark]

From: Leif Lindholm <leif.lindholm@linaro.org>

EfiHiiConfigRoutingProtocolGuid
EfiHiiDatabaseProtocol
EfiHiiStringProtocol

Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
---
 include/efi_api.h             | 204 ++++++++++++++++++++++++++++
 include/efi_loader.h          |   6 +
 lib/efi_loader/Makefile       |   2 +-
 lib/efi_loader/efi_boottime.c |   9 ++
 lib/efi_loader/efi_hii.c      | 303 ++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 523 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 57468dd972..932a3429a8 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -536,6 +536,210 @@ struct efi_device_path_utilities_protocol
 		const struct efi_device_path *device_path);
 };
 
+#define EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID \
+	EFI_GUID(0x587e72d7, 0xcc50, 0x4f79, \
+		 0x82, 0x09, 0xca, 0x29, 0x1f, 0xc1, 0xa1, 0x0f)
+
+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 UINTN 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 UINTN *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 {
+	EfiKeyLCtrl, EfiKeyA0, EfiKeyLAlt, EfiKeySpaceBar,
+	EfiKeyA2, EfiKeyA3, EfiKeyA4, EfiKeyRCtrl, EfiKeyLeftArrow,
+	EfiKeyDownArrow, EfiKeyRightArrow, EfiKeyZero,
+	EfiKeyPeriod, EfiKeyEnter, EfiKeyLShift, EfiKeyB0,
+	EfiKeyB1, EfiKeyB2, EfiKeyB3, EfiKeyB4, EfiKeyB5, EfiKeyB6,
+	EfiKeyB7, EfiKeyB8, EfiKeyB9, EfiKeyB10, EfiKeyRShift,
+	EfiKeyUpArrow, EfiKeyOne, EfiKeyTwo, EfiKeyThree,
+	EfiKeyCapsLock, EfiKeyC1, EfiKeyC2, EfiKeyC3, EfiKeyC4,
+	EfiKeyC5, EfiKeyC6, EfiKeyC7, EfiKeyC8, EfiKeyC9,
+	EfiKeyC10, EfiKeyC11, EfiKeyC12, EfiKeyFour, EfiKeyFive,
+	EfiKeySix, EfiKeyPlus, EfiKeyTab, EfiKeyD1, EfiKeyD2,
+	EfiKeyD3, EfiKeyD4, EfiKeyD5, EfiKeyD6, EfiKeyD7, EfiKeyD8,
+	EfiKeyD9, EfiKeyD10, EfiKeyD11, EfiKeyD12, EfiKeyD13,
+	EfiKeyDel, EfiKeyEnd, EfiKeyPgDn, EfiKeySeven, EfiKeyEight,
+	EfiKeyNine, EfiKeyE0, EfiKeyE1, EfiKeyE2, EfiKeyE3,
+	EfiKeyE4, EfiKeyE5, EfiKeyE6, EfiKeyE7, EfiKeyE8, EfiKeyE9,
+	EfiKeyE10, EfiKeyE11, EfiKeyE12, EfiKeyBackSpace,
+	EfiKeyIns, EfiKeyHome, EfiKeyPgUp, EfiKeyNLck, EfiKeySlash,
+	EfiKeyAsterisk, EfiKeyMinus, EfiKeyEsc, EfiKeyF1, EfiKeyF2,
+	EfiKeyF3, EfiKeyF4, EfiKeyF5, EfiKeyF6, EfiKeyF7, EfiKeyF8,
+	EfiKeyF9, EfiKeyF10, EfiKeyF11, EfiKeyF12, EfiKeyPrint,
+	EfiKeySLck, EfiKeyPause
+} 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;
+};
+
+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,
+		UINTN *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,
+		UINTN *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,
+		UINTN 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;
+typedef uint16_t efi_string_id_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,
+		UINTN *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,
+		UINTN *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,
+		UINTN *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 c009828db9..a89bb2fcd9 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -59,6 +59,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);
 
@@ -70,6 +73,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 f5e69dd078..e8fd6823a3 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
+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 8bb243d673..4d1a16051b 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -779,6 +779,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..cc68a28d2b
--- /dev/null
+++ b/lib/efi_loader/efi_hii.c
@@ -0,0 +1,303 @@
+/*
+ *  EFI Human Interface Infrastructure ... 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_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;
+
+
+/*
+ * EFI_HII_CONFIG_ROUTING_PROTOCOL
+ */
+
+static efi_status_t 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 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 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 block_to_config(
+	const struct efi_hii_config_routing_protocol *this,
+	const efi_string_t config_request,
+	const uint8_t *block,
+	const UINTN block_size,
+	efi_string_t *config,
+	efi_string_t *progress)
+{
+	EFI_ENTRY("%p, \"%ls\", %p, %lu, %p, %p", this, config_request, block,
+		  block_size, config, progress);
+	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+}
+
+static efi_status_t config_to_block(
+	const struct efi_hii_config_routing_protocol *this,
+	const efi_string_t config_resp,
+	const uint8_t *block,
+	const UINTN *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 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 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)
+{
+	/* Current shell start failure. */
+	/* Invoked from MdeModulePkg/Library/UefiHiiLib/HiiLib.c : HiiAddPackages */
+	/* (Via autogenerated .c file.) */
+	EFI_ENTRY("%p, %p, %p, %p", this, package_list, driver_handle, handle);
+	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+}
+
+static efi_status_t remove_package_list(
+	const struct efi_hii_database_protocol *this,
+	efi_hii_handle_t handle)
+{
+	EFI_ENTRY("%p, %p", this, handle);
+	return EFI_EXIT(EFI_NOT_FOUND);
+}
+
+static efi_status_t 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 list_package_lists(
+	const struct efi_hii_database_protocol *this,
+	uint8_t package_type,
+	const efi_guid_t *package_guid,
+	UINTN *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 export_package_lists(
+	const struct efi_hii_database_protocol *this,
+	efi_hii_handle_t handle,
+	UINTN *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 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,
+	UINTN notify_type,
+	efi_handle_t *notify_handle)
+{
+	EFI_ENTRY("%p, %u, %pUl, %p, %lu, %p", this, package_type,
+		  package_guid, package_notify_fn, notify_type,
+		  notify_handle);
+	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+}
+
+static efi_status_t 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 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 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 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 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 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, %p, %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 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,
+	UINTN *string_size,
+	struct efi_font_info **string_font_info)
+{
+	EFI_ENTRY("%p, %p, %p, %u, \"%ls\", %p, %p", this, language,
+		  package_list, string_id, string, string_size,
+		  string_font_info);
+	return EFI_EXIT(EFI_NOT_FOUND);
+}
+
+static efi_status_t 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, %p, \"%ls\", %p", this, package_list, string_id,
+		  language, string, string_font_info);
+	return EFI_EXIT(EFI_NOT_FOUND);
+}
+
+static efi_status_t get_languages(
+	const struct efi_hii_string_protocol *this,
+	efi_hii_handle_t package_list,
+	uint8_t *languages,
+	UINTN *languages_size)
+{
+	EFI_ENTRY("%p, %p, %p, %p", this, package_list, languages,
+		  languages_size);
+	return EFI_EXIT(EFI_NOT_FOUND);
+}
+
+static efi_status_t 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,
+	UINTN *secondary_languages_size)
+{
+	EFI_ENTRY("%p, %p, %p, %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
+};
-- 
2.13.5

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Rob Clark]

From: Leif Lindholm <leif.lindholm@linaro.org>

Signed-off-by: Leif Lindholm <leif.lindholm@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
+{
+	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;
 
+	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;
+
+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);
+	return;
+}
+
+void str_upr(struct efi_unicode_collation_protocol *this,
+	     efi_string_t string)
+{
+	EFI_ENTRY("%p, \"%ls\"", this, string);
+	EFI_EXIT(0);
+	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);
+	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
+};
-- 
2.13.5

[U-Boot] [PATCH v1 04/12] efi_loader: start fleshing out HII

[Rob Clark]

Not complete but enough for Shell.efi and SCT.efi.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h        |  68 +++++++++++-
 lib/efi_loader/efi_hii.c | 274 ++++++++++++++++++++++++++++++++++++++++-------
 2 files changed, 304 insertions(+), 38 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 25f774f253..4853b71497 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>
@@ -540,6 +541,8 @@ struct efi_device_path_utilities_protocol
 	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)(
@@ -630,7 +633,69 @@ 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;
+
+#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;
+}
 
 typedef void *efi_hii_handle_t;
 
@@ -693,7 +758,6 @@ struct efi_hii_database_protocol
 		 0xb9, 0xcb, 0x98, 0xd1, 0x77, 0x50, 0x32, 0x2a)
 
 typedef uint32_t efi_hii_font_style_t;
-typedef uint16_t efi_string_id_t;
 
 struct efi_font_info
 {
diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index cc68a28d2b..d818f409dc 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -7,6 +7,7 @@
  */
 
 #include <common.h>
+#include <malloc.h>
 #include <efi_loader.h>
 
 const efi_guid_t efi_guid_hii_config_routing_protocol =
@@ -14,12 +15,128 @@ const efi_guid_t efi_guid_hii_config_routing_protocol =
 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)
+{
+	for (int 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);
+	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 extract_config(
+static efi_status_t EFIAPI extract_config(
 	const struct efi_hii_config_routing_protocol *this,
 	const efi_string_t request,
 	efi_string_t *progress,
@@ -29,7 +146,7 @@ static efi_status_t extract_config(
 	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
 }
 
-static efi_status_t export_config(
+static efi_status_t EFIAPI export_config(
 	const struct efi_hii_config_routing_protocol *this,
 	efi_string_t *results)
 {
@@ -37,7 +154,7 @@ static efi_status_t export_config(
 	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
 }
 
-static efi_status_t route_config(
+static efi_status_t EFIAPI route_config(
 	const struct efi_hii_config_routing_protocol *this,
 	const efi_string_t configuration,
 	efi_string_t *progress)
@@ -46,7 +163,7 @@ static efi_status_t route_config(
 	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
 }
 
-static efi_status_t block_to_config(
+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,
@@ -54,12 +171,12 @@ static efi_status_t block_to_config(
 	efi_string_t *config,
 	efi_string_t *progress)
 {
-	EFI_ENTRY("%p, \"%ls\", %p, %lu, %p, %p", this, config_request, block,
+	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 config_to_block(
+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,
@@ -71,7 +188,7 @@ static efi_status_t config_to_block(
 	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
 }
 
-static efi_status_t get_alt_config(
+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,
@@ -91,28 +208,67 @@ static efi_status_t get_alt_config(
  * EFI_HII_DATABASE_PROTOCOL
  */
 
-static efi_status_t new_package_list(
+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)
 {
-	/* Current shell start failure. */
-	/* Invoked from MdeModulePkg/Library/UefiHiiLib/HiiLib.c : HiiAddPackages */
-	/* (Via autogenerated .c file.) */
+	efi_status_t ret = EFI_SUCCESS;
+
 	EFI_ENTRY("%p, %p, %p, %p", this, package_list, driver_handle, handle);
-	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+
+	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 remove_package_list(
+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);
-	return EFI_EXIT(EFI_NOT_FOUND);
+	free_package(hii);
+	return EFI_EXIT(EFI_SUCCESS);
 }
 
-static efi_status_t update_package_list(
+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)
@@ -121,7 +277,7 @@ static efi_status_t update_package_list(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t list_package_lists(
+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,
@@ -133,7 +289,7 @@ static efi_status_t list_package_lists(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t export_package_lists(
+static efi_status_t EFIAPI export_package_lists(
 	const struct efi_hii_database_protocol *this,
 	efi_hii_handle_t handle,
 	UINTN *buffer_size,
@@ -143,7 +299,7 @@ static efi_status_t export_package_lists(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t register_package_notify(
+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,
@@ -151,13 +307,13 @@ static efi_status_t register_package_notify(
 	UINTN notify_type,
 	efi_handle_t *notify_handle)
 {
-	EFI_ENTRY("%p, %u, %pUl, %p, %lu, %p", this, package_type,
+	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 unregister_package_notify(
+static efi_status_t EFIAPI unregister_package_notify(
 	const struct efi_hii_database_protocol *this,
 	efi_handle_t notification_handle)
 {
@@ -165,7 +321,7 @@ static efi_status_t unregister_package_notify(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t find_keyboard_layouts(
+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)
@@ -174,7 +330,7 @@ static efi_status_t find_keyboard_layouts(
 	return EFI_EXIT(EFI_NOT_FOUND); /* Invalid */
 }
 
-static efi_status_t get_keyboard_layout(
+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,
@@ -185,7 +341,7 @@ static efi_status_t get_keyboard_layout(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t set_keyboard_layout(
+static efi_status_t EFIAPI set_keyboard_layout(
 	const struct efi_hii_database_protocol *this,
 	efi_guid_t *key_guid)
 {
@@ -193,7 +349,7 @@ static efi_status_t set_keyboard_layout(
 	return EFI_EXIT(EFI_NOT_FOUND);
 }
 
-static efi_status_t get_package_list_handle(
+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)
@@ -207,7 +363,7 @@ static efi_status_t get_package_list_handle(
  * EFI_HII_STRING_PROTOCOL
  */
 
-static efi_status_t new_string(
+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,
@@ -216,13 +372,13 @@ static efi_status_t new_string(
 	const efi_string_t string,
 	const struct efi_font_info *string_font_info)
 {
-	EFI_ENTRY("%p, %p, %p, %p, %p, \"%ls\", %p", this, package_list,
+	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 get_string(
+static efi_status_t EFIAPI get_string(
 	const struct efi_hii_string_protocol *this,
 	const uint8_t *language,
 	efi_hii_handle_t package_list,
@@ -231,13 +387,34 @@ static efi_status_t get_string(
 	UINTN *string_size,
 	struct efi_font_info **string_font_info)
 {
-	EFI_ENTRY("%p, %p, %p, %u, \"%ls\", %p, %p", this, language,
+	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;
+			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 set_string(
+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,
@@ -245,31 +422,56 @@ static efi_status_t set_string(
 	const efi_string_t string,
 	const struct efi_font_info *string_font_info)
 {
-	EFI_ENTRY("%p, %p, %u, %p, \"%ls\", %p", this, package_list, string_id,
-		  language, string, 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 get_languages(
+static efi_status_t EFIAPI get_languages(
 	const struct efi_hii_string_protocol *this,
 	efi_hii_handle_t package_list,
 	uint8_t *languages,
 	UINTN *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);
-	return EFI_EXIT(EFI_NOT_FOUND);
+
+	/* 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 get_secondary_languages(
+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,
 	UINTN *secondary_languages_size)
 {
-	EFI_ENTRY("%p, %p, %p, %p, %p", this, package_list, primary_language,
-		  secondary_languages, 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);
 }
 
-- 
2.13.5

[U-Boot] [PATCH v1 05/12] efi_loader: flesh out unicode protocol

[Rob Clark]

Not complete, but enough for Shell.efi and SCT.efi.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h             |   9 +++
 include/efi_loader.h          |   1 +
 lib/efi_loader/efi_boottime.c |   5 +-
 lib/efi_loader/efi_unicode.c  | 143 +++++++++++++++++++++++++++++++++++-------
 4 files changed, 134 insertions(+), 24 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 4853b71497..5612dfad49 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -804,6 +804,15 @@ struct efi_hii_string_protocol
 		UINTN *secondary_languages_size);
 };
 
+/*
+ * Both UNICODE_COLLATION protocols seem to be the same thing, but
+ * advertised with two different GUID's because, why not?
+ */
+
+#define EFI_UNICODE_COLLATION_PROTOCOL_GUID \
+	EFI_GUID(0x1d85cd7f, 0xf43d, 0x11d2, \
+		 0x9a, 0x0c, 0x00, 0x90, 0x27, 0x3f, 0xc1, 0x4d)
+
 #define EFI_UNICODE_COLLATION_PROTOCOL2_GUID \
 	EFI_GUID(0xa4c751fc, 0x23ae, 0x4c3e, \
 		 0x92, 0xe9, 0x49, 0x64, 0xcf, 0x63, 0xf3, 0x49)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 6668338d0b..4864b3ac77 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -77,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_protocol;
 extern const efi_guid_t efi_guid_unicode_collation_protocol2;
 
 extern unsigned int __efi_runtime_start, __efi_runtime_stop;
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 04358e8aca..7b53570354 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -788,9 +788,12 @@ 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;
 
-	obj->protocols[8].guid = &efi_guid_unicode_collation_protocol2;
+	obj->protocols[8].guid = &efi_guid_unicode_collation_protocol;
 	obj->protocols[8].protocol_interface = (void *)&efi_unicode_collation;
 
+	obj->protocols[9].guid = &efi_guid_unicode_collation_protocol2;
+	obj->protocols[9].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
index fdf1a99812..8d8edfb45a 100644
--- a/lib/efi_loader/efi_unicode.c
+++ b/lib/efi_loader/efi_unicode.c
@@ -7,59 +7,155 @@
  */
 
 #include <common.h>
+#include <charset.h>
+#include <linux/ctype.h>
 #include <efi_loader.h>
 
+const efi_guid_t efi_guid_unicode_collation_protocol =
+	EFI_UNICODE_COLLATION_PROTOCOL_GUID;
+
 const efi_guid_t efi_guid_unicode_collation_protocol2 =
 	EFI_UNICODE_COLLATION_PROTOCOL2_GUID;
 
-INTN stri_coll(struct efi_unicode_collation_protocol *this,
-	       efi_string_t s1,
-	       efi_string_t s2)
+static int matchn(efi_string_t s1, unsigned n1, efi_string_t s2, unsigned n2)
+{
+	char u1[MAX_UTF8_PER_UTF16 * n1 + 1];
+	char u2[MAX_UTF8_PER_UTF16 * n2 + 1];
+
+	*utf16_to_utf8((u8 *)u1, s1, n1) = '\0';
+	*utf16_to_utf8((u8 *)u2, s2, n2) = '\0';
+
+	return strcasecmp(u1, u2);
+}
+
+static INTN EFIAPI 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);
+
+	unsigned n1 = utf16_strlen(s1);
+	unsigned n2 = utf16_strlen(s2);
+
+	return EFI_EXIT(matchn(s1, n1, s2, n2));
 }
 
-bool metai_match(struct efi_unicode_collation_protocol *this,
-		 efi_string_t string,
-		 efi_string_t pattern)
+static bool match(efi_string_t string, efi_string_t pattern)
+{
+	while (true) {
+		uint16_t p = *pattern++;
+		bool matches = false;
+
+		if (p == '\0' || *string == '\0') {
+			/*
+			 * End of pattern or string, succeed if
+			 * end of both:
+			 */
+			return *string == p;
+		}
+
+		switch (p) {
+		case '*':
+			/* Match zero or more chars: */
+			while (*string != '\0') {
+				if (match(string, pattern))
+					return true;
+				string++;
+			}
+			return match(string, pattern);
+		case '?':
+			/* Match any one char: */
+			string++;
+			break;
+		case '[':
+			/* Match char set, either [abc] or [a-c]: */
+
+			if (pattern[0] == '\0' || pattern[0] == ']') {
+				/* invalid pattern */
+				return false;
+			}
+
+			if (pattern[1] == '-') {
+				uint16_t lo, hi, c;
+
+				/* range: [a-c] */
+				lo = pattern[0];
+				hi = pattern[2];
+
+				if (hi == '\0' || hi == ']' || pattern[3] != ']') {
+					/* invalid pattern */
+					return false;
+				}
+
+				c  = tolower(*string);
+				lo = tolower(lo);
+				hi = tolower(hi);
+
+				if (lo <= c && c <= hi)
+					matches = true;
+
+				pattern += 4;
+			} else {
+				/* set: [abc] */
+				while ((p = *pattern++) && p != ']')
+					if (matchn(string, 1, &p, 1))
+						matches = true;
+			}
+
+			if (!matches)
+				return false;
+
+			string++;
+			break;
+		default:
+			if (matchn(string, 1, &p, 1))
+				return false;
+			string++;
+			break;
+		}
+	}
+}
+
+static bool EFIAPI 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);
+	return EFI_EXIT(match(string, pattern));
 }
 
-void str_lwr(struct efi_unicode_collation_protocol *this,
-	     efi_string_t string)
+static void EFIAPI str_lwr(struct efi_unicode_collation_protocol *this,
+			   efi_string_t string)
 {
 	EFI_ENTRY("%p, \"%ls\"", this, string);
 	EFI_EXIT(0);
 	return;
 }
 
-void str_upr(struct efi_unicode_collation_protocol *this,
-	     efi_string_t string)
+static void EFIAPI str_upr(struct efi_unicode_collation_protocol *this,
+			   efi_string_t string)
 {
 	EFI_ENTRY("%p, \"%ls\"", this, string);
 	EFI_EXIT(0);
 	return;
 }
 
-void fat_to_str(struct efi_unicode_collation_protocol *this,
-		UINTN fat_size,
-		uint8_t *fat,
-		efi_string_t string)
+static void EFIAPI 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_ENTRY("%p, %zu, \"%s\", %p", this, fat_size, fat, string);
 	EFI_EXIT(0);
 	return;
 }
 
-bool str_to_fat(struct efi_unicode_collation_protocol *this,
-		efi_string_t string,
-		UINTN fat_size,
-		uint8_t *fat)
+static bool EFIAPI 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);
+	EFI_ENTRY("%p, \"%ls\", %zu, %p", this, string, fat_size, fat);
 	return EFI_EXIT(false);
 }
 
@@ -69,5 +165,6 @@ const struct efi_unicode_collation_protocol efi_unicode_collation = {
 	.str_lwr = str_lwr,
 	.str_upr = str_upr,
 	.fat_to_str = fat_to_str,
-	.str_to_fat = str_to_fat
+	.str_to_fat = str_to_fat,
+	.supported_languages = (uint8_t *)"eng",
 };
-- 
2.13.5

[U-Boot] [PATCH v1 06/12] efi_loader: start fleshing out efi_device_path_utilities

[Rob Clark]

Not complete, but enough for Shell.efi and SCT.efi.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 lib/efi_loader/efi_device_path_utilities.c | 28 ++++++++++++++++------------
 1 file changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
index 4b97080f35..37b539e8ff 100644
--- a/lib/efi_loader/efi_device_path_utilities.c
+++ b/lib/efi_loader/efi_device_path_utilities.c
@@ -12,36 +12,40 @@
 const efi_guid_t efi_guid_device_path_utilities_protocol =
 		EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
 
-static UINTN get_device_path_size(const struct efi_device_path *device_path)
+static UINTN EFIAPI get_device_path_size(const struct efi_device_path *device_path)
 {
+	UINTN sz = 0;
 	EFI_ENTRY("%p", device_path);
-	return EFI_EXIT(0);
+	/* size includes the END node: */
+	if (device_path)
+		sz = efi_dp_size(device_path) + sizeof(struct efi_device_path);
+	return EFI_EXIT(sz);
 }
 
-static struct efi_device_path *duplicate_device_path(
+static struct efi_device_path * EFIAPI duplicate_device_path(
 	const struct efi_device_path *device_path)
 {
 	EFI_ENTRY("%p", device_path);
-	return EFI_EXIT(NULL);
+	return EFI_EXIT(efi_dp_dup(device_path));
 }
 
-static struct efi_device_path *append_device_path(
+static struct efi_device_path * EFIAPI append_device_path(
 	const struct efi_device_path *src1,
 	const struct efi_device_path *src2)
 {
 	EFI_ENTRY("%p, %p", src1, src2);
-	return EFI_EXIT(NULL);
+	return EFI_EXIT(efi_dp_append(src1, src2));
 }
 
-static struct efi_device_path *append_device_node(
+static struct efi_device_path * EFIAPI append_device_node(
 	const struct efi_device_path *device_path,
 	const struct efi_device_path *device_node)
 {
 	EFI_ENTRY("%p, %p", device_path, device_node);
-	return EFI_EXIT(NULL);
+	return EFI_EXIT(efi_dp_append_node(device_path, device_node));
 }
 
-static struct efi_device_path *append_device_path_instance(
+static struct efi_device_path * EFIAPI append_device_path_instance(
 	const struct efi_device_path *device_path,
 	const struct efi_device_path *device_path_instance)
 {
@@ -49,7 +53,7 @@ static struct efi_device_path *append_device_path_instance(
 	return EFI_EXIT(NULL);
 }
 
-static struct efi_device_path *get_next_device_path_instance(
+static struct efi_device_path * EFIAPI get_next_device_path_instance(
 	struct efi_device_path **device_path_instance,
 	UINTN *device_path_instance_size)
 {
@@ -57,14 +61,14 @@ static struct efi_device_path *get_next_device_path_instance(
 	return EFI_EXIT(NULL);
 }
 
-static struct efi_device_path *create_device_node(
+static struct efi_device_path * EFIAPI create_device_node(
 	uint8_t node_type, uint8_t node_sub_type, uint16_t node_length)
 {
 	EFI_ENTRY("%u, %u, %u", node_type, node_sub_type, node_length);
 	return EFI_EXIT(NULL);
 }
 
-static bool is_device_path_multi_instance(
+static bool EFIAPI is_device_path_multi_instance(
 	const struct efi_device_path *device_path)
 {
 	EFI_ENTRY("%p", device_path);
-- 
2.13.5

[U-Boot] [PATCH v1 07/12] efi_loader: SIMPLE_TEXT_INPUT_EX plus wire up objects properly

[Rob Clark]

I think we need the _EX version for SCT.. and either way we need to wire
up the corresponding objects in the systab properly.

This fixes some issues with SCT.efi.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h             |  60 +++++++++++++++++++-
 include/efi_loader.h          |  10 +---
 lib/efi_loader/efi_boottime.c |   3 +
 lib/efi_loader/efi_console.c  | 129 +++++++++++++++++++++++++++++++++++++-----
 4 files changed, 177 insertions(+), 25 deletions(-)

diff --git a/include/efi_api.h b/include/efi_api.h
index 5612dfad49..87c8ffe68e 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -242,11 +242,11 @@ struct efi_system_table {
 	struct efi_table_hdr hdr;
 	unsigned long fw_vendor;   /* physical addr of wchar_t vendor string */
 	u32 fw_revision;
-	unsigned long con_in_handle;
+	efi_handle_t con_in_handle;
 	struct efi_simple_input_interface *con_in;
-	unsigned long con_out_handle;
+	efi_handle_t con_out_handle;
 	struct efi_simple_text_output_protocol *con_out;
-	unsigned long stderr_handle;
+	efi_handle_t stderr_handle;
 	struct efi_simple_text_output_protocol *std_err;
 	struct efi_runtime_services *runtime;
 	struct efi_boot_services *boottime;
@@ -473,6 +473,60 @@ struct efi_simple_input_interface {
 	struct efi_event *wait_for_key;
 };
 
+
+#define EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID \
+	EFI_GUID(0xdd9e7534, 0x7762, 0x4698, \
+		 0x8c, 0x14, 0xf5, 0x85, 0x17, 0xa6, 0x25, 0xaa)
+
+/* key-shift state: */
+#define EFI_SHIFT_STATE_VALID     0x80000000
+#define EFI_RIGHT_SHIFT_PRESSED   0x00000001
+#define EFI_LEFT_SHIFT_PRESSED    0x00000002
+#define EFI_RIGHT_CONTROL_PRESSED 0x00000004
+#define EFI_LEFT_CONTROL_PRESSED  0x00000008
+#define EFI_RIGHT_ALT_PRESSED     0x00000010
+#define EFI_EFI_LEFT_ALT_PRESSED  0x00000020
+#define EFI_RIGHT_LOGO_PRESSED    0x00000040
+#define EFI_LEFT_LOGO_PRESSED     0x00000080
+#define EFI_MENU_KEY_PRESSED      0x00000100
+#define EFI_SYS_REQ_PRESSED       0x00000200
+
+/* key-toggle state: */
+#define EFI_TOGGLE_STATE_VALID 0x80
+#define EFI_SCROLL_LOCK_ACTIVE 0x01
+#define EFI_NUM_LOCK_ACTIVE    0x02
+#define EFI_CAPS_LOCK_ACTIVE   0x04
+
+struct efi_key_state {
+	uint32_t key_shift_state;
+	uint8_t  key_toggle_state;
+};
+
+struct efi_key_data {
+	struct efi_input_key key;
+	struct efi_key_state key_state;
+};
+
+struct efi_simple_text_input_ex_interface {
+	efi_status_t (EFIAPI *reset)(
+			struct efi_simple_text_input_ex_interface *this,
+			bool ExtendedVerification);
+	efi_status_t (EFIAPI *read_key_stroke)(
+			struct efi_simple_text_input_ex_interface *this,
+			struct efi_key_data *key_data);
+	struct efi_event *wait_for_key;
+	efi_status_t (EFIAPI *set_state)(
+			struct efi_simple_text_input_ex_interface *this,
+			uint8_t key_toggle_state);
+	efi_status_t (EFIAPI *register_key_notify)(
+			struct efi_simple_text_input_ex_interface *this,
+			efi_status_t (EFIAPI *notify_fn)(struct efi_key_data *key_data),
+			efi_handle_t *notify_handle);
+	efi_status_t (EFIAPI *unregister_key_notify)(
+			struct efi_simple_text_input_ex_interface *this,
+			efi_handle_t notify_handle);
+};
+
 #define CONSOLE_CONTROL_GUID \
 	EFI_GUID(0xf42f7782, 0x12e, 0x4c12, \
 		 0x99, 0x56, 0x49, 0xf9, 0x43, 0x4, 0xf7, 0x21)
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 4864b3ac77..72734b3a44 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -54,7 +54,9 @@ const char *__efi_nesting_dec(void);
 extern struct efi_runtime_services efi_runtime_services;
 extern struct efi_system_table systab;
 
+extern struct efi_object efi_console_output_obj;
 extern const struct efi_simple_text_output_protocol efi_con_out;
+extern struct efi_object efi_console_input_obj;
 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;
@@ -108,14 +110,6 @@ struct efi_object {
 	void *handle;
 };
 
-#define EFI_PROTOCOL_OBJECT(_guid, _protocol) (struct efi_object){	\
-	.protocols = {{							\
-		.guid = &(_guid),	 				\
-		.protocol_interface = (void *)(_protocol), 		\
-	}},								\
-	.handle = (void *)(_protocol),					\
-}
-
 /**
  * struct efi_event
  *
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 7b53570354..9628bef474 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -1394,8 +1394,11 @@ struct efi_system_table __efi_runtime_data systab = {
 		.headersize = sizeof(struct efi_table_hdr),
 	},
 	.fw_vendor = (long)firmware_vendor,
+	.con_in_handle = &efi_console_input_obj,
 	.con_in = (void*)&efi_con_in,
+	.con_out_handle = &efi_console_output_obj,
 	.con_out = (void*)&efi_con_out,
+	.stderr_handle = &efi_console_output_obj,
 	.std_err = (void*)&efi_con_out,
 	.runtime = (void*)&efi_runtime_services,
 	.boottime = (void*)&efi_boot_services,
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index afc725a2c7..2e13fdc096 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -50,6 +50,10 @@ const efi_guid_t efi_guid_console_control = CONSOLE_CONTROL_GUID;
 #define cESC '\x1b'
 #define ESC "\x1b"
 
+/*
+ * EFI_CONSOLE_CONTROL:
+ */
+
 static efi_status_t EFIAPI efi_cin_get_mode(
 			struct efi_console_control_protocol *this,
 			int *mode, char *uga_exists, char *std_in_locked)
@@ -97,6 +101,11 @@ static struct simple_text_output_mode efi_con_mode = {
 	.cursor_visible = 1,
 };
 
+
+/*
+ * EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL:
+ */
+
 static int term_read_reply(int *n, int maxnum, char end_char)
 {
 	char c;
@@ -364,6 +373,11 @@ const struct efi_simple_text_output_protocol efi_con_out = {
 	.mode = (void*)&efi_con_mode,
 };
 
+
+/*
+ * EFI_SIMPLE_TEXT_INPUT_PROTOCOL:
+ */
+
 static efi_status_t EFIAPI efi_cin_reset(
 			struct efi_simple_input_interface *this,
 			bool extended_verification)
@@ -372,9 +386,7 @@ static efi_status_t EFIAPI efi_cin_reset(
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
 
-static efi_status_t EFIAPI efi_cin_read_key_stroke(
-			struct efi_simple_input_interface *this,
-			struct efi_input_key *key)
+static efi_status_t read_key_stroke(struct efi_key_data *key_data)
 {
 	struct efi_input_key pressed_key = {
 		.scan_code = 0,
@@ -382,14 +394,12 @@ static efi_status_t EFIAPI efi_cin_read_key_stroke(
 	};
 	char ch;
 
-	EFI_ENTRY("%p, %p", this, key);
-
 	/* We don't do interrupts, so check for timers cooperatively */
 	efi_timer_check();
 
 	if (!tstc()) {
 		/* No key pressed */
-		return EFI_EXIT(EFI_NOT_READY);
+		return EFI_NOT_READY;
 	}
 
 	ch = getc();
@@ -438,9 +448,31 @@ static efi_status_t EFIAPI efi_cin_read_key_stroke(
 		ch = 0x08;
 	}
 	pressed_key.unicode_char = ch;
-	*key = pressed_key;
+	key_data->key = pressed_key;
 
-	return EFI_EXIT(EFI_SUCCESS);
+	/* TODO not sure if we have a way to get this from stdin?
+	 * We might need to use keyboard driver directly for _ex
+	 * stuff?
+	 */
+	memset(&key_data->key_state, 0, sizeof(key_data->key_state));
+
+	return EFI_SUCCESS;
+}
+
+static efi_status_t EFIAPI efi_cin_read_key_stroke(
+			struct efi_simple_input_interface *this,
+			struct efi_input_key *key)
+{
+	struct efi_key_data key_data;
+	efi_status_t ret;
+
+	EFI_ENTRY("%p, %p", this, key);
+
+	ret = read_key_stroke(&key_data);
+	if (ret == EFI_SUCCESS)
+		*key = key_data.key;
+
+	return EFI_EXIT(ret);
 }
 
 struct efi_simple_input_interface efi_con_in = {
@@ -465,12 +497,81 @@ static void EFIAPI efi_console_timer_notify(struct efi_event *event,
 }
 
 
-static struct efi_object efi_console_control_obj =
-	EFI_PROTOCOL_OBJECT(efi_guid_console_control, &efi_console_control);
-static struct efi_object efi_console_output_obj =
-	EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID, &efi_con_out);
-static struct efi_object efi_console_input_obj =
-	EFI_PROTOCOL_OBJECT(EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID, &efi_con_in);
+/*
+ * EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL
+ */
+
+static efi_status_t EFIAPI efi_cin_ex_reset(
+		struct efi_simple_text_input_ex_interface *this,
+		bool extended_verification)
+{
+	EFI_ENTRY("%p, %d", this, extended_verification);
+	return EFI_EXIT(EFI_UNSUPPORTED);
+}
+
+static efi_status_t EFIAPI efi_cin_ex_read_key_stroke(
+		struct efi_simple_text_input_ex_interface *this,
+		struct efi_key_data *key_data)
+{
+	EFI_ENTRY("%p, %p", this, key_data);
+	return EFI_EXIT(read_key_stroke(key_data));
+}
+
+static efi_status_t EFIAPI efi_cin_ex_set_state(
+		struct efi_simple_text_input_ex_interface *this,
+		uint8_t key_toggle_state)
+{
+	EFI_ENTRY("%p, %x", this, key_toggle_state);
+	return EFI_EXIT(EFI_SUCCESS);
+}
+
+static efi_status_t EFIAPI efi_cin_ex_register_key_notify(
+		struct efi_simple_text_input_ex_interface *this,
+		efi_status_t (EFIAPI *notify_fn)(struct efi_key_data *key_data),
+		efi_handle_t *notify_handle)
+{
+	EFI_ENTRY("%p, %p, %p", this, notify_fn, notify_handle);
+	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+}
+
+static efi_status_t EFIAPI efi_cin_ex_unregister_key_notify(
+		struct efi_simple_text_input_ex_interface *this,
+		efi_handle_t notify_handle)
+{
+	EFI_ENTRY("%p, %p", this, notify_handle);
+	return EFI_EXIT(EFI_INVALID_PARAMETER);
+}
+
+static struct efi_simple_text_input_ex_interface efi_con_in_ex = {
+		.reset = efi_cin_ex_reset,
+		.read_key_stroke = efi_cin_ex_read_key_stroke,
+		.wait_for_key = NULL,
+		.set_state = efi_cin_ex_set_state,
+		.register_key_notify = efi_cin_ex_register_key_notify,
+		.unregister_key_notify = efi_cin_ex_unregister_key_notify,
+};
+
+static struct efi_object efi_console_control_obj = {
+	.protocols =  {
+		{ &efi_guid_console_control, (void *)&efi_console_control },
+	},
+	.handle = &efi_console_control_obj,
+};
+
+struct efi_object efi_console_output_obj = {
+	.protocols = {
+		{&EFI_SIMPLE_TEXT_OUTPUT_PROTOCOL_GUID, (void *)&efi_con_out},
+	},
+	.handle = &efi_console_output_obj,
+};
+
+struct efi_object efi_console_input_obj = {
+	.protocols = {
+		{&EFI_SIMPLE_TEXT_INPUT_PROTOCOL_GUID,    (void *)&efi_con_in},
+		{&EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID, (void *)&efi_con_in_ex},
+	},
+	.handle = &efi_console_input_obj,
+};
 
 /* This gets called from do_bootefi_exec(). */
 int efi_console_register(void)
-- 
2.13.5

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Rob Clark]

Shell.efi uses this, and supporting color attributes makes things look
nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
Not all colors have a perfect match, but spec just says "Devices
supporting a different number of text colors are required to emulate the
above colors to the best of the device’s capabilities".

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 include/efi_api.h            | 29 +++++++++++++++++++++++++++++
 lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+)

diff --git a/include/efi_api.h b/include/efi_api.h
index 87c8ffe68e..3cc1dbac2e 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -426,6 +426,35 @@ struct simple_text_output_mode {
 	EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
 		 0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
 
+#define EFI_BLACK                0x00
+#define EFI_BLUE                 0x01
+#define EFI_GREEN                0x02
+#define EFI_CYAN                 0x03
+#define EFI_RED                  0x04
+#define EFI_MAGENTA              0x05
+#define EFI_BROWN                0x06
+#define EFI_LIGHTGRAY            0x07
+#define EFI_BRIGHT               0x08
+#define EFI_DARKGRAY             0x08
+#define EFI_LIGHTBLUE            0x09
+#define EFI_LIGHTGREEN           0x0a
+#define EFI_LIGHTCYAN            0x0b
+#define EFI_LIGHTRED             0x0c
+#define EFI_LIGHTMAGENTA         0x0d
+#define EFI_YELLOW               0x0e
+#define EFI_WHITE                0x0f
+#define EFI_BACKGROUND_BLACK     0x00
+#define EFI_BACKGROUND_BLUE      0x10
+#define EFI_BACKGROUND_GREEN     0x20
+#define EFI_BACKGROUND_CYAN      0x30
+#define EFI_BACKGROUND_RED       0x40
+#define EFI_BACKGROUND_MAGENTA   0x50
+#define EFI_BACKGROUND_BROWN     0x60
+#define EFI_BACKGROUND_LIGHTGRAY 0x70
+
+#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
+#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
+
 struct efi_simple_text_output_protocol {
 	void *reset;
 	efi_status_t (EFIAPI *output_string)(
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index 2e13fdc096..fcd65ca488 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
 	return EFI_EXIT(EFI_SUCCESS);
 }
 
+static const struct {
+	unsigned fg;
+	unsigned bg;
+} color[] = {
+	{ 30, 40 },     /* 0: black */
+	{ 34, 44 },     /* 1: blue */
+	{ 32, 42 },     /* 2: green */
+	{ 36, 46 },     /* 3: cyan */
+	{ 31, 41 },     /* 4: red */
+	{ 35, 45 },     /* 5: magenta */
+	{ 30, 40 },     /* 6: brown, map to black */
+	{ 37, 47 },     /* 7: light grey, map to white */
+	{ 37, 47 },     /* 8: bright, map to white */
+	{ 34, 44 },     /* 9: light blue, map to blue */
+	{ 32, 42 },     /* A: light green, map to green */
+	{ 36, 46 },     /* B: light cyan, map to cyan */
+	{ 31, 41 },     /* C: light red, map to red */
+	{ 35, 45 },     /* D: light magenta, map to magenta */
+	{ 33, 43 },     /* E: yellow */
+	{ 37, 47 },     /* F: white */
+};
+
 static efi_status_t EFIAPI efi_cout_set_attribute(
 			struct efi_simple_text_output_protocol *this,
 			unsigned long attribute)
 {
+	unsigned fg = EFI_ATTR_FG(attribute);
+	unsigned bg = EFI_ATTR_BG(attribute);
+
 	EFI_ENTRY("%p, %lx", this, attribute);
 
+	if (attribute)
+		printf(ESC"[%u;%um", color[fg].fg, color[bg].bg);
+	else
+		printf(ESC"[37;40m");
+
 	/* Just ignore attributes (colors) for now */
 	return EFI_EXIT(EFI_UNSUPPORTED);
 }
-- 
2.13.5

[U-Boot] [PATCH v1 09/12] dm: video: Fix cache flushes

[Rob Clark]

Content can come to screen via putc() and we cannot always rely on
updates ending with a puts().  This is the case with efi_console output
to vidconsole.  Fixes corruption with Shell.efi.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/video/vidconsole-uclass.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
index b5afd72227..e081d5a0ee 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -163,6 +163,7 @@ static void vidconsole_putc(struct stdio_dev *sdev, const char ch)
 	struct udevice *dev = sdev->priv;
 
 	vidconsole_put_char(dev, ch);
+	video_sync(dev->parent);
 }
 
 static void vidconsole_puts(struct stdio_dev *sdev, const char *s)
@@ -260,6 +261,8 @@ static int do_video_puts(cmd_tbl_t *cmdtp, int flag, int argc,
 	for (s = argv[1]; *s; s++)
 		vidconsole_put_char(dev, *s);
 
+	video_sync(dev->parent);
+
 	return 0;
 }
 
-- 
2.13.5

[U-Boot] [PATCH v1 10/12] dm: video: Add basic ANSI escape sequence support

[Rob Clark]

Really just the subset that is needed by efi_console.  Perhaps more will
be added later, for example color support would be useful to implement
efi_cout_set_attribute().

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++
 drivers/video/video-uclass.c      |   4 +-
 include/video.h                   |   7 +++
 include/video_console.h           |  11 ++++
 4 files changed, 131 insertions(+), 3 deletions(-)

diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
index e081d5a0ee..7998b4cf5f 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -9,6 +9,7 @@
  */
 
 #include <common.h>
+#include <linux/ctype.h>
 #include <dm.h>
 #include <video.h>
 #include <video_console.h>
@@ -107,12 +108,123 @@ static void vidconsole_newline(struct udevice *dev)
 	video_sync(dev->parent);
 }
 
+/*
+ * Parse a number from string that ends in a non-numeric character..
+ * sscanf() would be nice.  This is just enough for parsing ANSI escape
+ * sequences.
+ */
+static char *parsenum(char *s, int *num)
+{
+	int n = 0;
+
+	while (isdigit(*s)) {
+		n = (10 * n) + (*s - '0');
+		s++;
+	}
+
+	*num = n;
+	return s;
+}
+
+static void vidconsole_escape_char(struct udevice *dev, char ch)
+{
+	struct vidconsole_priv *priv = dev_get_uclass_priv(dev);
+
+	/* Sanity checking for bogus ESC sequences: */
+	if (priv->escape_len >= sizeof(priv->escape_buf))
+		goto error;
+	if (priv->escape_len == 0 && ch != '[')
+		goto error;
+
+	priv->escape_buf[priv->escape_len++] = ch;
+
+	/*
+	 * Escape sequences are terminated by a letter, so keep
+	 * accumulating until we get one:
+	 */
+	if (!isalpha(ch))
+		return;
+
+	/*
+	 * clear escape mode first, otherwise things will get highly
+	 * surprising if you hit any debug prints that come back to
+	 * this console.
+	 */
+	priv->escape = 0;
+
+	switch (ch) {
+	case 'H':
+	case 'f': {
+		int row, col;
+		char *s = priv->escape_buf;
+
+		/*
+		 * Set cursor position: [%d;%df or [%d;%dH
+		 */
+		s++;    /* [ */
+		s = parsenum(s, &row);
+		s++;    /* ; */
+		s = parsenum(s, &col);
+
+		priv->ycur = row * priv->y_charsize;
+		priv->xcur_frac = priv->xstart_frac +
+			VID_TO_POS(col * priv->x_charsize);
+
+		break;
+	}
+	case 'J': {
+		int mode;
+
+		/*
+		 * Clear part/all screen:
+		 *   [J or [0J - clear screen from cursor down
+		 *   [1J       - clear screen from cursor up
+		 *   [2J       - clear entire screen
+		 *
+		 * TODO we really only handle entire-screen case, others
+		 * probably require some additions to video-uclass (and
+		 * are not really needed yet by efi_console)
+		 */
+		parsenum(priv->escape_buf + 1, &mode);
+
+		if (mode == 2) {
+			video_clear(dev->parent);
+			video_sync(dev->parent);
+			priv->ycur = 0;
+			priv->xcur_frac = priv->xstart_frac;
+		} else {
+			debug("unsupported clear mode: %d\n", mode);
+		}
+		break;
+	}
+	default:
+		debug("unrecognized escape sequence: %*s\n",
+		      priv->escape_len, priv->escape_buf);
+	}
+
+	return;
+
+error:
+	/* something went wrong, just revert to normal mode: */
+	priv->escape = 0;
+	return;
+}
+
 int vidconsole_put_char(struct udevice *dev, char ch)
 {
 	struct vidconsole_priv *priv = dev_get_uclass_priv(dev);
 	int ret;
 
+	if (priv->escape) {
+		vidconsole_escape_char(dev, ch);
+		return 0;
+	}
+
 	switch (ch) {
+	case '\x1b':
+		priv->escape_len = 0;
+		priv->escape = 1;
+		break;
 	case '\a':
 		/* beep */
 		break;
diff --git a/drivers/video/video-uclass.c b/drivers/video/video-uclass.c
index 3036e3a1f2..0163039821 100644
--- a/drivers/video/video-uclass.c
+++ b/drivers/video/video-uclass.c
@@ -87,7 +87,7 @@ int video_reserve(ulong *addrp)
 	return 0;
 }
 
-static int video_clear(struct udevice *dev)
+void video_clear(struct udevice *dev)
 {
 	struct video_priv *priv = dev_get_uclass_priv(dev);
 
@@ -100,8 +100,6 @@ static int video_clear(struct udevice *dev)
 	} else {
 		memset(priv->fb, priv->colour_bg, priv->fb_size);
 	}
-
-	return 0;
 }
 
 /* Flush video activity to the caches */
diff --git a/include/video.h b/include/video.h
index 5b4e78b182..61ff653121 100644
--- a/include/video.h
+++ b/include/video.h
@@ -115,6 +115,13 @@ struct video_ops {
 int video_reserve(ulong *addrp);
 
 /**
+ * video_clear() - Clear a device's frame buffer to background color.
+ *
+ * @dev:	Device to clear
+ */
+void video_clear(struct udevice *dev);
+
+/**
  * video_sync() - Sync a device's frame buffer with its hardware
  *
  * Some frame buffers are cached or have a secondary frame buffer. This
diff --git a/include/video_console.h b/include/video_console.h
index 26047934da..9dce234bd9 100644
--- a/include/video_console.h
+++ b/include/video_console.h
@@ -29,6 +29,9 @@
  * @xsize_frac:	Width of the display in fractional units
  * @xstart_frac:	Left margin for the text console in fractional units
  * @last_ch:	Last character written to the text console on this line
+ * @escape:	TRUE if currently accumulating an ANSI escape sequence
+ * @escape_len:	Length of accumulated escape sequence so far
+ * @escape_buf:	Buffer to accumulate escape sequence
  */
 struct vidconsole_priv {
 	struct stdio_dev sdev;
@@ -42,6 +45,14 @@ struct vidconsole_priv {
 	int xsize_frac;
 	int xstart_frac;
 	int last_ch;
+	/*
+	 * ANSI escape sequences are accumulated character by character,
+	 * starting after the ESC char (0x1b) until the entire sequence
+	 * is consumed at which point it is acted upon.
+	 */
+	int escape;
+	int escape_len;
+	char escape_buf[32];
 };
 
 /**
-- 
2.13.5

[U-Boot] [PATCH v1 11/12] dm: video: Add color ANSI escape sequence support

[Rob Clark]

Note that this doesn't differentiate (due to lack of information in
video_priv) between different possible component orders for 32bpp.
But the main user at this point is efi_loader, and GOP expects xBGR
so any video drivers that this is incorrect for already have problems.
(Also, conveniently, this matches what simple-framebuffer bindings
expect for kernels that use the simple-framebuffer DT binding to
take over the bootloader display.)

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
 drivers/video/vidconsole-uclass.c | 94 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 94 insertions(+)

diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
index 7998b4cf5f..dae65a21ed 100644
--- a/drivers/video/vidconsole-uclass.c
+++ b/drivers/video/vidconsole-uclass.c
@@ -108,6 +108,41 @@ static void vidconsole_newline(struct udevice *dev)
 	video_sync(dev->parent);
 }
 
+static const struct {
+	unsigned r;
+	unsigned g;
+	unsigned b;
+} colors[] = {
+	{ 0x00, 0x00, 0x00 },  /* black */
+	{ 0xff, 0x00, 0x00 },  /* red */
+	{ 0x00, 0xff, 0x00 },  /* green */
+	{ 0xff, 0xff, 0x00 },  /* yellow */
+	{ 0x00, 0x00, 0xff },  /* blue */
+	{ 0xff, 0x00, 0xff },  /* magenta */
+	{ 0x00, 0xff, 0xff },  /* cyan */
+	{ 0xff, 0xff, 0xff },  /* white */
+};
+
+static void set_color(struct video_priv *priv, unsigned idx, unsigned *c)
+{
+	switch (priv->bpix) {
+	case VIDEO_BPP16:
+		*c = ((colors[idx].r >> 3) << 0) |
+		     ((colors[idx].g >> 2) << 5) |
+		     ((colors[idx].b >> 3) << 11);
+		break;
+	case VIDEO_BPP32:
+		*c = 0xff000000 |
+		     (colors[idx].r << 0) |
+		     (colors[idx].g << 8) |
+		     (colors[idx].b << 16);
+		break;
+	default:
+		/* unsupported, leave current color in place */
+		break;
+	}
+}
+
 /*
  * Parse a number from string that ends in a non-numeric character..
  * sscanf() would be nice.  This is just enough for parsing ANSI escape
@@ -197,6 +232,65 @@ static void vidconsole_escape_char(struct udevice *dev, char ch)
 		}
 		break;
 	}
+	case 'm': {
+		struct video_priv *vid_priv = dev_get_uclass_priv(dev->parent);
+		char *s = priv->escape_buf;
+		char *end = &priv->escape_buf[priv->escape_len];
+
+		/*
+		 * Set graphics mode: [%d;...;%dm
+		 *
+		 * Currently only supports the color attributes:
+		 *
+		 * Foreground Colors:
+		 *
+		 *   30	Black
+		 *   31	Red
+		 *   32	Green
+		 *   33	Yellow
+		 *   34	Blue
+		 *   35	Magenta
+		 *   36	Cyan
+		 *   37	White
+		 *
+		 * Background Colors:
+		 *
+		 *   40	Black
+		 *   41	Red
+		 *   42	Green
+		 *   43	Yellow
+		 *   44	Blue
+		 *   45	Magenta
+		 *   46	Cyan
+		 *   47	White
+		 */
+
+		s++;    /* [ */
+		while (s < end) {
+			int val;
+
+			s = parsenum(s, &val);
+			s++;
+
+			switch (val) {
+			case 30 ... 37:
+				/* fg color */
+				set_color(vid_priv, val - 30,
+					  (unsigned *)&vid_priv->colour_fg);
+				break;
+			case 40 ... 47:
+				/* bg color */
+				set_color(vid_priv, val - 40,
+					  (unsigned *)&vid_priv->colour_bg);
+				break;
+			default:
+				/* unknown/unsupported */
+				break;
+			}
+		}
+
+		break;
+	}
 	default:
 		debug("unrecognized escape sequence: %*s\n",
 		      priv->escape_len, priv->escape_buf);
-- 
2.13.5

[U-Boot] [PATCH v1 12/12] HACK: efi_loader: hacks for SCT

[Rob Clark]

Not intended to be merged, just to show the remaning TODOs to get
SCT.efi working.
---
 include/config_distro_bootcmd.h |  2 +-
 lib/efi_loader/efi_boottime.c   |  4 ++--
 lib/efi_loader/efi_console.c    |  3 ++-
 lib/efi_loader/efi_file.c       | 11 +++++++++--
 4 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/config_distro_bootcmd.h b/include/config_distro_bootcmd.h
index e0d0034ed3..e232a62996 100644
--- a/include/config_distro_bootcmd.h
+++ b/include/config_distro_bootcmd.h
@@ -88,7 +88,7 @@
 
 #ifdef CONFIG_EFI_LOADER
 #if defined(CONFIG_ARM64)
-#define BOOTEFI_NAME "bootaa64.efi"
+#define BOOTEFI_NAME "shell.efi"
 #elif defined(CONFIG_ARM)
 #define BOOTEFI_NAME "bootarm.efi"
 #endif
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index 9628bef474..5db8ea9090 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -172,7 +172,7 @@ static efi_status_t efi_unsupported(const char *funcname)
 static unsigned long EFIAPI efi_raise_tpl(UINTN new_tpl)
 {
 	EFI_ENTRY("0x%zx", new_tpl);
-	return EFI_EXIT(0);
+	return EFI_EXIT(TPL_APPLICATION);
 }
 
 static void EFIAPI efi_restore_tpl(UINTN old_tpl)
@@ -1021,7 +1021,7 @@ static efi_status_t EFIAPI efi_set_watchdog_timer(unsigned long timeout,
 {
 	EFI_ENTRY("%ld, 0x%"PRIx64", %ld, %p", timeout, watchdog_code,
 		  data_size, watchdog_data);
-	return efi_unsupported(__func__);
+	return EFI_EXIT(EFI_SUCCESS);
 }
 
 static efi_status_t EFIAPI efi_connect_controller(
diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
index fcd65ca488..139f7ea55b 100644
--- a/lib/efi_loader/efi_console.c
+++ b/lib/efi_loader/efi_console.c
@@ -561,7 +561,8 @@ static efi_status_t EFIAPI efi_cin_ex_register_key_notify(
 		efi_handle_t *notify_handle)
 {
 	EFI_ENTRY("%p, %p, %p", this, notify_fn, notify_handle);
-	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+//	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
+	return EFI_EXIT(EFI_SUCCESS);
 }
 
 static efi_status_t EFIAPI efi_cin_ex_unregister_key_notify(
diff --git a/lib/efi_loader/efi_file.c b/lib/efi_loader/efi_file.c
index 118d83ad7c..d66b0d0370 100644
--- a/lib/efi_loader/efi_file.c
+++ b/lib/efi_loader/efi_file.c
@@ -218,8 +218,8 @@ static efi_status_t EFIAPI efi_file_delete(struct efi_file_handle *file)
 {
 	struct file_handle *fh = to_fh(file);
 	EFI_ENTRY("%p", file);
-	file_close(fh);
-	return EFI_EXIT(EFI_WARN_DELETE_FAILURE);
+	// TODO need fs_rm()..
+	return EFI_EXIT(file_close(fh));
 }
 
 static efi_status_t file_read(struct file_handle *fh, u64 *buffer_size,
@@ -340,6 +340,13 @@ static efi_status_t EFIAPI efi_file_write(struct efi_file_handle *file,
 
 	EFI_ENTRY("%p, %p, %p", file, buffer_size, buffer);
 
+	// XXX hack because fat_write is horrible:
+	if (!fs_exists(fh->path)) {
+		char *p = fh->path;
+		while ((p = strchr(p, '/')))
+			*p++ = '_';
+	}
+
 	if (set_blk_dev(fh)) {
 		ret = EFI_DEVICE_ERROR;
 		goto error;
-- 
2.13.5

[U-Boot] [PATCH v1 10/12] dm: video: Add basic ANSI escape sequence support

[Simon Glass]

Hi Rob,

On 10 September 2017 at 07:22, Rob Clark <robdclark@gmail.com> wrote:
> Really just the subset that is needed by efi_console.  Perhaps more will
> be added later, for example color support would be useful to implement
> efi_cout_set_attribute().
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++
>  drivers/video/video-uclass.c      |   4 +-
>  include/video.h                   |   7 +++
>  include/video_console.h           |  11 ++++
>  4 files changed, 131 insertions(+), 3 deletions(-)

Can we put this behind an option (perhaps default on) to reduce code
size? Also please update test/dm/video.c to add a test.

[U-Boot] [PATCH v1 10/12] dm: video: Add basic ANSI escape sequence support

[Rob Clark]

On Tue, Sep 12, 2017 at 8:30 AM, Simon Glass <sjg@chromium.org> wrote:
> Hi Rob,
>
> On 10 September 2017 at 07:22, Rob Clark <robdclark@gmail.com> wrote:
>> Really just the subset that is needed by efi_console.  Perhaps more will
>> be added later, for example color support would be useful to implement
>> efi_cout_set_attribute().
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  drivers/video/vidconsole-uclass.c | 112 ++++++++++++++++++++++++++++++++++++++
>>  drivers/video/video-uclass.c      |   4 +-
>>  include/video.h                   |   7 +++
>>  include/video_console.h           |  11 ++++
>>  4 files changed, 131 insertions(+), 3 deletions(-)
>
> Can we put this behind an option (perhaps default on) to reduce code
> size? Also please update test/dm/video.c to add a test.

yup, already done.. I'll resend this and the other video patches
separately, so you can drop this one from the set.

BR,
-R

[U-Boot] [PATCH v1 01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL

[Heinrich Schuchardt]

On 09/10/2017 03:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  include/efi_api.h                          | 30 +++++++++++
>  include/efi_loader.h                       |  2 +
>  lib/efi_loader/Makefile                    |  1 +
>  lib/efi_loader/efi_boottime.c              |  4 ++
>  lib/efi_loader/efi_device_path_utilities.c | 83 ++++++++++++++++++++++++++++++
>  5 files changed, 120 insertions(+)
>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index c3b9032a48..57468dd972 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -506,6 +506,36 @@ struct efi_device_path_to_text_protocol
>  			bool allow_shortcuts);
>  };
>  
> +#define EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID \
> +	EFI_GUID(0x0379be4e, 0xd706, 0x437d, \
> +		 0xb0, 0x37, 0xed, 0xb8, 0x2f, 0xb7, 0x72, 0xa4)
> +
> +struct efi_device_path_utilities_protocol
> +{
> +	UINTN(EFIAPI *get_device_path_size)(
> +		const struct efi_device_path *device_path);
> +	struct efi_device_path *(EFIAPI *duplicate_device_path)(
> +		const struct efi_device_path *device_path);
> +	struct efi_device_path *(EFIAPI *append_device_path)(
> +		const struct efi_device_path *src1,
> +		const struct efi_device_path *src2);
> +	struct efi_device_path *(EFIAPI *append_device_node)(
> +		const struct efi_device_path *device_path,
> +		const struct efi_device_path *device_node);
> +	struct efi_device_path *(EFIAPI *append_device_path_instance)(
> +		const struct efi_device_path *device_path,
> +		const struct efi_device_path *device_path_instance);
> +	struct efi_device_path *(EFIAPI *get_next_device_path_instance)(
> +		struct efi_device_path **device_path_instance,
> +		UINTN *device_path_instance_size);

The sequence is wrong. It does not match
UEFI Spec 2.7 Errata A, August 2017.

EDK2 uses the same sequence as the spec see

EdkCompatibilityPkg/Foundation/Efi/Protocol/DevicePathUtilities/DevicePathUtilities.h

Put is_device_path_multi_instance before create_device_node.

> +	struct efi_device_path *(EFIAPI *create_device_node)(
> +		uint8_t node_type,
> +		uint8_t node_sub_type,
> +		uint16_t node_length);
> +	bool(EFIAPI *is_device_path_multi_instance)(
> +		const struct efi_device_path *device_path);
> +};
> +
>  #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 43b12b94fa..c009828db9 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -58,6 +58,7 @@ extern const struct efi_simple_text_output_protocol efi_con_out;
>  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;
>  
>  uint16_t *efi_dp_str(struct efi_device_path *dp);
>  
> @@ -68,6 +69,7 @@ extern const efi_guid_t efi_guid_loaded_image;
>  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 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 930c0e218e..f5e69dd078 100644
> --- a/lib/efi_loader/Makefile
> +++ b/lib/efi_loader/Makefile
> @@ -16,6 +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
>  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 3860feb79b..8bb243d673 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -775,6 +775,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>  	obj->protocols[3].protocol_interface =
>  		(void *)&efi_device_path_to_text;
>  
> +	obj->protocols[4].guid = &efi_guid_device_path_utilities_protocol;
> +	obj->protocols[4].protocol_interface =
> +		(void *)&efi_device_path_utilities;
> +
>  	info->file_path = file_path;
>  	info->device_handle = efi_dp_find_obj(device_path, NULL);
>  
> diff --git a/lib/efi_loader/efi_device_path_utilities.c b/lib/efi_loader/efi_device_path_utilities.c
> new file mode 100644
> index 0000000000..4b97080f35
> --- /dev/null
> +++ b/lib/efi_loader/efi_device_path_utilities.c
> @@ -0,0 +1,83 @@
> +/*
> + *  EFI device path 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_device_path_utilities_protocol =
> +		EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID;
> +

None of the function definitions below matches the definition in the
protocol structure. EFIAPI is missing. Rob tried to correct this in a
follow up patch. But it should be corrected here.

> +static UINTN get_device_path_size(const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(0);
> +}
> +
> +static struct efi_device_path *duplicate_device_path(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *append_device_path(
> +	const struct efi_device_path *src1,
> +	const struct efi_device_path *src2)
> +{
> +	EFI_ENTRY("%p, %p", src1, src2);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *append_device_node(
> +	const struct efi_device_path *device_path,
> +	const struct efi_device_path *device_node)
> +{
> +	EFI_ENTRY("%p, %p", device_path, device_node);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *append_device_path_instance(
> +	const struct efi_device_path *device_path,
> +	const struct efi_device_path *device_path_instance)
> +{
> +	EFI_ENTRY("%p, %p", device_path, device_path_instance);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *get_next_device_path_instance(
> +	struct efi_device_path **device_path_instance,
> +	UINTN *device_path_instance_size)
> +{
> +	EFI_ENTRY("%p, %p", device_path_instance, device_path_instance_size);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static struct efi_device_path *create_device_node(
> +	uint8_t node_type, uint8_t node_sub_type, uint16_t node_length)
> +{
> +	EFI_ENTRY("%u, %u, %u", node_type, node_sub_type, node_length);
> +	return EFI_EXIT(NULL);
> +}
> +
> +static bool is_device_path_multi_instance(
> +	const struct efi_device_path *device_path)
> +{
> +	EFI_ENTRY("%p", device_path);
> +	return EFI_EXIT(false);
> +}
> +
> +const struct efi_device_path_utilities_protocol efi_device_path_utilities = {
> +	.get_device_path_size = get_device_path_size,
> +	.duplicate_device_path = duplicate_device_path,
> +	.append_device_path = append_device_path,
> +	.append_device_node = append_device_node,
> +	.append_device_path_instance = append_device_path_instance,
> +	.get_next_device_path_instance = get_next_device_path_instance,

Use the same sequence as the UEFI spec.
is_device_path_multi_instance followed by create_device_node

As these functions do not return efi_status_t an EFI application can
expect that a handle exposing this protocol correctly implements all of
them.

Follow up-patch
efi_loader: start fleshing out efi_device_path_utilities
only delivers part of it.

Best regards

Heinrich

> +	.create_device_node = create_device_node,
> +	.is_device_path_multi_instance = is_device_path_multi_instance,
> +};
> 

[U-Boot] [PATCH v1 01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL

[Heinrich Schuchardt]

On 10/04/2017 07:13 PM, Heinrich Schuchardt wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h                          | 30 +++++++++++
>>  include/efi_loader.h                       |  2 +
>>  lib/efi_loader/Makefile                    |  1 +
>>  lib/efi_loader/efi_boottime.c              |  4 ++
>>  lib/efi_loader/efi_device_path_utilities.c | 83 ++++++++++++++++++++++++++++++
>>  5 files changed, 120 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index c3b9032a48..57468dd972 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -506,6 +506,36 @@ struct efi_device_path_to_text_protocol
>>  			bool allow_shortcuts);
>>  };
>>  
>> +#define EFI_DEVICE_PATH_UTILITIES_PROTOCOL_GUID \
>> +	EFI_GUID(0x0379be4e, 0xd706, 0x437d, \
>> +		 0xb0, 0x37, 0xed, 0xb8, 0x2f, 0xb7, 0x72, 0xa4)
>> +
>> +struct efi_device_path_utilities_protocol
>> +{

checkpatch returns an error:

ERROR: open brace '{' following struct go on the same line
#30: FILE: include/efi_api.h:514:
+struct efi_device_path_utilities_protocol
+{

Regards

Heinrich

>> +	UINTN(EFIAPI *get_device_path_size)(
>> +		const struct efi_device_path *device_path);
>> +	struct efi_device_path *(EFIAPI *duplicate_device_path)(
>> +		const struct efi_device_path *device_path);

[U-Boot] [PATCH v1 02/12] efi_loader: add stub HII protocols

[Heinrich Schuchardt]

On 09/10/2017 03:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>
> 
> EfiHiiConfigRoutingProtocolGuid
> EfiHiiDatabaseProtocol
> EfiHiiStringProtocol

Please, provide a proper commit message.

Resolve all issues reported by checkpatch.

> 
> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
> ---
>  include/efi_api.h             | 204 ++++++++++++++++++++++++++++
>  include/efi_loader.h          |   6 +
>  lib/efi_loader/Makefile       |   2 +-
>  lib/efi_loader/efi_boottime.c |   9 ++
>  lib/efi_loader/efi_hii.c      | 303 ++++++++++++++++++++++++++++++++++++++++++
>  5 files changed, 523 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 57468dd972..932a3429a8 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -536,6 +536,210 @@ struct efi_device_path_utilities_protocol
>  		const struct efi_device_path *device_path);
>  };
>  
> +#define EFI_HII_CONFIG_ROUTING_PROTOCOL_GUID \
> +	EFI_GUID(0x587e72d7, 0xcc50, 0x4f79, \
> +		 0x82, 0x09, 0xca, 0x29, 0x1f, 0xc1, 0xa1, 0x0f)
> +
> +struct efi_hii_config_routing_protocol
> +{

ERROR: open brace '{' following struct go on the same line
#34: FILE: include/efi_api.h:544:
+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 UINTN 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 UINTN *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 {

We do not use CamelCase.

> +	EfiKeyLCtrl, EfiKeyA0, EfiKeyLAlt, EfiKeySpaceBar,
> +	EfiKeyA2, EfiKeyA3, EfiKeyA4, EfiKeyRCtrl, EfiKeyLeftArrow,
> +	EfiKeyDownArrow, EfiKeyRightArrow, EfiKeyZero,
> +	EfiKeyPeriod, EfiKeyEnter, EfiKeyLShift, EfiKeyB0,
> +	EfiKeyB1, EfiKeyB2, EfiKeyB3, EfiKeyB4, EfiKeyB5, EfiKeyB6,
> +	EfiKeyB7, EfiKeyB8, EfiKeyB9, EfiKeyB10, EfiKeyRShift,
> +	EfiKeyUpArrow, EfiKeyOne, EfiKeyTwo, EfiKeyThree,
> +	EfiKeyCapsLock, EfiKeyC1, EfiKeyC2, EfiKeyC3, EfiKeyC4,
> +	EfiKeyC5, EfiKeyC6, EfiKeyC7, EfiKeyC8, EfiKeyC9,
> +	EfiKeyC10, EfiKeyC11, EfiKeyC12, EfiKeyFour, EfiKeyFive,
> +	EfiKeySix, EfiKeyPlus, EfiKeyTab, EfiKeyD1, EfiKeyD2,
> +	EfiKeyD3, EfiKeyD4, EfiKeyD5, EfiKeyD6, EfiKeyD7, EfiKeyD8,
> +	EfiKeyD9, EfiKeyD10, EfiKeyD11, EfiKeyD12, EfiKeyD13,
> +	EfiKeyDel, EfiKeyEnd, EfiKeyPgDn, EfiKeySeven, EfiKeyEight,
> +	EfiKeyNine, EfiKeyE0, EfiKeyE1, EfiKeyE2, EfiKeyE3,
> +	EfiKeyE4, EfiKeyE5, EfiKeyE6, EfiKeyE7, EfiKeyE8, EfiKeyE9,
> +	EfiKeyE10, EfiKeyE11, EfiKeyE12, EfiKeyBackSpace,
> +	EfiKeyIns, EfiKeyHome, EfiKeyPgUp, EfiKeyNLck, EfiKeySlash,
> +	EfiKeyAsterisk, EfiKeyMinus, EfiKeyEsc, EfiKeyF1, EfiKeyF2,
> +	EfiKeyF3, EfiKeyF4, EfiKeyF5, EfiKeyF6, EfiKeyF7, EfiKeyF8,
> +	EfiKeyF9, EfiKeyF10, EfiKeyF11, EfiKeyF12, EfiKeyPrint,
> +	EfiKeySLck, EfiKeyPause
> +} 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;
> +};
> +
> +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,
> +		UINTN *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,
> +		UINTN *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,
> +		UINTN 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;
> +typedef uint16_t efi_string_id_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,
> +		UINTN *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,
> +		UINTN *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,
> +		UINTN *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 c009828db9..a89bb2fcd9 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -59,6 +59,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);
>  
> @@ -70,6 +73,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 f5e69dd078..e8fd6823a3 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
> +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 8bb243d673..4d1a16051b 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -779,6 +779,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..cc68a28d2b
> --- /dev/null
> +++ b/lib/efi_loader/efi_hii.c
> @@ -0,0 +1,303 @@
> +/*
> + *  EFI Human Interface Infrastructure ... 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_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;
> +
> +
> +/*
> + * EFI_HII_CONFIG_ROUTING_PROTOCOL
> + */

None of the functions below matches the definitions in the structures.
Please, add the missing EFIAPI.

Regards

Heinrich

> +
> +static efi_status_t 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 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 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 block_to_config(
> +	const struct efi_hii_config_routing_protocol *this,
> +	const efi_string_t config_request,
> +	const uint8_t *block,
> +	const UINTN block_size,
> +	efi_string_t *config,
> +	efi_string_t *progress)
> +{
> +	EFI_ENTRY("%p, \"%ls\", %p, %lu, %p, %p", this, config_request, block,
> +		  block_size, config, progress);
> +	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
> +}
> +
> +static efi_status_t config_to_block(
> +	const struct efi_hii_config_routing_protocol *this,
> +	const efi_string_t config_resp,
> +	const uint8_t *block,
> +	const UINTN *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 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 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)
> +{
> +	/* Current shell start failure. */
> +	/* Invoked from MdeModulePkg/Library/UefiHiiLib/HiiLib.c : HiiAddPackages */
> +	/* (Via autogenerated .c file.) */
> +	EFI_ENTRY("%p, %p, %p, %p", this, package_list, driver_handle, handle);
> +	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
> +}
> +
> +static efi_status_t remove_package_list(
> +	const struct efi_hii_database_protocol *this,
> +	efi_hii_handle_t handle)
> +{
> +	EFI_ENTRY("%p, %p", this, handle);
> +	return EFI_EXIT(EFI_NOT_FOUND);
> +}
> +
> +static efi_status_t 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 list_package_lists(
> +	const struct efi_hii_database_protocol *this,
> +	uint8_t package_type,
> +	const efi_guid_t *package_guid,
> +	UINTN *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 export_package_lists(
> +	const struct efi_hii_database_protocol *this,
> +	efi_hii_handle_t handle,
> +	UINTN *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 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,
> +	UINTN notify_type,
> +	efi_handle_t *notify_handle)
> +{
> +	EFI_ENTRY("%p, %u, %pUl, %p, %lu, %p", this, package_type,
> +		  package_guid, package_notify_fn, notify_type,
> +		  notify_handle);
> +	return EFI_EXIT(EFI_OUT_OF_RESOURCES);
> +}
> +
> +static efi_status_t 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 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 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 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 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 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, %p, %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 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,
> +	UINTN *string_size,
> +	struct efi_font_info **string_font_info)
> +{
> +	EFI_ENTRY("%p, %p, %p, %u, \"%ls\", %p, %p", this, language,
> +		  package_list, string_id, string, string_size,
> +		  string_font_info);
> +	return EFI_EXIT(EFI_NOT_FOUND);
> +}
> +
> +static efi_status_t 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, %p, \"%ls\", %p", this, package_list, string_id,
> +		  language, string, string_font_info);
> +	return EFI_EXIT(EFI_NOT_FOUND);
> +}
> +
> +static efi_status_t get_languages(
> +	const struct efi_hii_string_protocol *this,
> +	efi_hii_handle_t package_list,
> +	uint8_t *languages,
> +	UINTN *languages_size)
> +{
> +	EFI_ENTRY("%p, %p, %p, %p", this, package_list, languages,
> +		  languages_size);
> +	return EFI_EXIT(EFI_NOT_FOUND);
> +}
> +
> +static efi_status_t 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,
> +	UINTN *secondary_languages_size)
> +{
> +	EFI_ENTRY("%p, %p, %p, %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
> +};
> 

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Heinrich Schuchardt]

On 09/10/2017 03:22 PM, Rob Clark wrote:
> From: Leif Lindholm <leif.lindholm@linaro.org>

The commit message is missing.

Fix all issues reported by checkpatch.

> 
> Signed-off-by: Leif Lindholm <leif.lindholm@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.

> +	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.

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
> +};
> 

[U-Boot] [PATCH v1 02/12] efi_loader: add stub HII protocols

[Heinrich Schuchardt]

On 10/04/2017 07:45 PM, Heinrich Schuchardt wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> EfiHiiConfigRoutingProtocolGuid
>> EfiHiiDatabaseProtocol
>> EfiHiiStringProtocol
> 
> Please, provide a proper commit message.
> 
> Resolve all issues reported by checkpatch.
> 
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h             | 204 ++++++++++++++++++++++++++++
>>  include/efi_loader.h          |   6 +
>>  lib/efi_loader/Makefile       |   2 +-

<snip>

>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 8bb243d673..4d1a16051b 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -779,6 +779,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;
>> +

Do not add a protocol that is not properly implemented yet.

Regards

Heinrich

[U-Boot] [PATCH v1 01/12] efi_loader: add stub EFI_DEVICE_PATH_UTILITIES_PROTOCOL

[Heinrich Schuchardt]

On 10/04/2017 07:13 PM, Heinrich Schuchardt wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org>
>> ---
>>  include/efi_api.h                          | 30 +++++++++++
>>  include/efi_loader.h                       |  2 +
>>  lib/efi_loader/Makefile                    |  1 +
>>  lib/efi_loader/efi_boottime.c              |  4 ++
>>  lib/efi_loader/efi_device_path_utilities.c | 83 ++++++++++++++++++++++++++++++
>>  5 files changed, 120 insertions(+)
>>  create mode 100644 lib/efi_loader/efi_device_path_utilities.c
>>

<snip>

>> diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>> index 3860feb79b..8bb243d673 100644
>> --- a/lib/efi_loader/efi_boottime.c
>> +++ b/lib/efi_loader/efi_boottime.c
>> @@ -775,6 +775,10 @@ void efi_setup_loaded_image(struct efi_loaded_image *info, struct efi_object *ob
>>  	obj->protocols[3].protocol_interface =
>>  		(void *)&efi_device_path_to_text;
>>  
>> +	obj->protocols[4].guid = &efi_guid_device_path_utilities_protocol;
>> +	obj->protocols[4].protocol_interface =
>> +		(void *)&efi_device_path_utilities;
>> +

Do not add a protocol that is not properly implemented yet.

Regards

Heinrich

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Rob Clark]

On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> From: Leif Lindholm <leif.lindholm@linaro.org>
>
> The commit message is missing.
>
> Fix all issues reported by checkpatch.
>
>>
>> Signed-off-by: Leif Lindholm <leif.lindholm@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!

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).

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
>> +};
>>
>

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Heinrich Schuchardt]

On 09/10/2017 03:22 PM, Rob Clark wrote:
> Shell.efi uses this, and supporting color attributes makes things look
> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
> Not all colors have a perfect match, but spec just says "Devices
> supporting a different number of text colors are required to emulate the
> above colors to the best of the device’s capabilities".
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>  2 files changed, 59 insertions(+)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 87c8ffe68e..3cc1dbac2e 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>  	EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>  		 0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>  
> +#define EFI_BLACK                0x00
> +#define EFI_BLUE                 0x01
> +#define EFI_GREEN                0x02
> +#define EFI_CYAN                 0x03
> +#define EFI_RED                  0x04
> +#define EFI_MAGENTA              0x05
> +#define EFI_BROWN                0x06
> +#define EFI_LIGHTGRAY            0x07
> +#define EFI_BRIGHT               0x08
> +#define EFI_DARKGRAY             0x08
> +#define EFI_LIGHTBLUE            0x09
> +#define EFI_LIGHTGREEN           0x0a
> +#define EFI_LIGHTCYAN            0x0b
> +#define EFI_LIGHTRED             0x0c
> +#define EFI_LIGHTMAGENTA         0x0d
> +#define EFI_YELLOW               0x0e
> +#define EFI_WHITE                0x0f
> +#define EFI_BACKGROUND_BLACK     0x00
> +#define EFI_BACKGROUND_BLUE      0x10
> +#define EFI_BACKGROUND_GREEN     0x20
> +#define EFI_BACKGROUND_CYAN      0x30
> +#define EFI_BACKGROUND_RED       0x40
> +#define EFI_BACKGROUND_MAGENTA   0x50
> +#define EFI_BACKGROUND_BROWN     0x60
> +#define EFI_BACKGROUND_LIGHTGRAY 0x70

Will we ever use these constants?


Where are the comments explaining the defines below?

> +
> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)

This saves 8 entries in the table below.
+#define EFI_ATTR_FG(attr)        ((attr) & 0x07)

> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)

Add
#define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)

> +
>  struct efi_simple_text_output_protocol {
>  	void *reset;
>  	efi_status_t (EFIAPI *output_string)(
> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
> index 2e13fdc096..fcd65ca488 100644
> --- a/lib/efi_loader/efi_console.c
> +++ b/lib/efi_loader/efi_console.c
> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>  	return EFI_EXIT(EFI_SUCCESS);
>  }
>  
> +static const struct {
> +	unsigned fg;
> +	unsigned bg;
> +} color[] = {
> +	{ 30, 40 },     /* 0: black */
> +	{ 34, 44 },     /* 1: blue */
> +	{ 32, 42 },     /* 2: green */
> +	{ 36, 46 },     /* 3: cyan */
> +	{ 31, 41 },     /* 4: red */
> +	{ 35, 45 },     /* 5: magenta */
> +	{ 30, 40 },     /* 6: brown, map to black */

This should be { 33, 43 }

> +	{ 37, 47 },     /* 7: light grey, map to white */

The entries below are redundant.

> +	{ 37, 47 },     /* 8: bright, map to white */
> +	{ 34, 44 },     /* 9: light blue, map to blue */
> +	{ 32, 42 },     /* A: light green, map to green */
> +	{ 36, 46 },     /* B: light cyan, map to cyan */
> +	{ 31, 41 },     /* C: light red, map to red */
> +	{ 35, 45 },     /* D: light magenta, map to magenta */
> +	{ 33, 43 },     /* E: yellow */
> +	{ 37, 47 },     /* F: white */
> +};
> +

No function without a comment explaining the parameters.

>  static efi_status_t EFIAPI efi_cout_set_attribute(
>  			struct efi_simple_text_output_protocol *this,
>  			unsigned long attribute)
>  {
> +	unsigned fg = EFI_ATTR_FG(attribute);
> +	unsigned bg = EFI_ATTR_BG(attribute);

Use unsigned int.

> +
>  	EFI_ENTRY("%p, %lx", this, attribute);
>  
> +	if (attribute)

Attribute 0 should be black on black. No need for any if here.

> +		printf(ESC"[%u;%um", color[fg].fg, color[bg].bg);

Use bold for light colors.

unsigned int bold;
if (EFI_ATTRIB_BOLD(attr))
	bold = 1;
else
	bold = 22;


printf(ESC"[%u;%u;%um", bold, color[fg].fg, color[bg].bg)

Best regards

Heinrich

> +	else
> +		printf(ESC"[37;40m");


> +
>  	/* Just ignore attributes (colors) for now */
>  	return EFI_EXIT(EFI_UNSUPPORTED);
>  }
> 

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Heinrich Schuchardt]

On 10/04/2017 08:35 PM, Rob Clark wrote:
> On Wed, Oct 4, 2017 at 1:57 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>
>> The commit message is missing.
>>
>> Fix all issues reported by checkpatch.
>>
>>>
>>> Signed-off-by: Leif Lindholm <leif.lindholm@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!
> 
> 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
>>> +};
>>>
>>
> 

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Peter Jones]

On Wed, Oct 04, 2017 at 06:35:46PM +0000, Rob Clark wrote:
> > 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).

Worth noting that if we went this route, we'd also need to do
-mregparm=0 on 32-bit x86.

-- 
  Peter

[U-Boot] [PATCH v1 03/12] efi_loader: add EFI_UNICODE_COLLATION_PROTOCOL stub

[Heinrich Schuchardt]

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@gmx.de> wrote:
>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>> From: Leif Lindholm <leif.lindholm@linaro.org>
>>>
>>> The commit message is missing.
>>>
>>> Fix all issues reported by checkpatch.
>>>
>>>>
>>>> Signed-off-by: Leif Lindholm <leif.lindholm@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
>>>> +};
>>>>
>>>
>>
> 
> 

[U-Boot] [PATCH v1 09/12] dm: video: Fix cache flushes

[Heinrich Schuchardt]

On 09/10/2017 03:22 PM, Rob Clark wrote:
> Content can come to screen via putc() and we cannot always rely on
> updates ending with a puts().

The commit message does not explain why you are modifying videconsole_puts.

Probably you wanted to refer to a newline character '\n' which leads to
calling video_sync without your patch. Please, update the commit message.

The code change looks correct to me.

Acked-by: Heinrich Schuchardt <xypron.glpk@gmx.de>

> This is the case with efi_console output
> to vidconsole.  Fixes corruption with Shell.efi.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/video/vidconsole-uclass.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/video/vidconsole-uclass.c b/drivers/video/vidconsole-uclass.c
> index b5afd72227..e081d5a0ee 100644
> --- a/drivers/video/vidconsole-uclass.c
> +++ b/drivers/video/vidconsole-uclass.c
> @@ -163,6 +163,7 @@ static void vidconsole_putc(struct stdio_dev *sdev, const char ch)
>  	struct udevice *dev = sdev->priv;
>  
>  	vidconsole_put_char(dev, ch);
> +	video_sync(dev->parent);
>  }
>  
>  static void vidconsole_puts(struct stdio_dev *sdev, const char *s)
> @@ -260,6 +261,8 @@ static int do_video_puts(cmd_tbl_t *cmdtp, int flag, int argc,
>  	for (s = argv[1]; *s; s++)
>  		vidconsole_put_char(dev, *s);
>  
> +	video_sync(dev->parent);
> +
>  	return 0;
>  }
>  
> 

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Rob Clark]

On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 09/10/2017 03:22 PM, Rob Clark wrote:
>> Shell.efi uses this, and supporting color attributes makes things look
>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>> Not all colors have a perfect match, but spec just says "Devices
>> supporting a different number of text colors are required to emulate the
>> above colors to the best of the device’s capabilities".
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>  2 files changed, 59 insertions(+)
>>
>> diff --git a/include/efi_api.h b/include/efi_api.h
>> index 87c8ffe68e..3cc1dbac2e 100644
>> --- a/include/efi_api.h
>> +++ b/include/efi_api.h
>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>
>> +#define EFI_BLACK                0x00
>> +#define EFI_BLUE                 0x01
>> +#define EFI_GREEN                0x02
>> +#define EFI_CYAN                 0x03
>> +#define EFI_RED                  0x04
>> +#define EFI_MAGENTA              0x05
>> +#define EFI_BROWN                0x06
>> +#define EFI_LIGHTGRAY            0x07
>> +#define EFI_BRIGHT               0x08
>> +#define EFI_DARKGRAY             0x08
>> +#define EFI_LIGHTBLUE            0x09
>> +#define EFI_LIGHTGREEN           0x0a
>> +#define EFI_LIGHTCYAN            0x0b
>> +#define EFI_LIGHTRED             0x0c
>> +#define EFI_LIGHTMAGENTA         0x0d
>> +#define EFI_YELLOW               0x0e
>> +#define EFI_WHITE                0x0f
>> +#define EFI_BACKGROUND_BLACK     0x00
>> +#define EFI_BACKGROUND_BLUE      0x10
>> +#define EFI_BACKGROUND_GREEN     0x20
>> +#define EFI_BACKGROUND_CYAN      0x30
>> +#define EFI_BACKGROUND_RED       0x40
>> +#define EFI_BACKGROUND_MAGENTA   0x50
>> +#define EFI_BACKGROUND_BROWN     0x60
>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>
> Will we ever use these constants?
>

possibly not, but it is useful to understand what is going on with
efi->ansi mapping, so I would prefer to keep them.

>
> Where are the comments explaining the defines below?
>
>> +
>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>
> This saves 8 entries in the table below.
> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>
>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>
> Add
> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>
>> +
>>  struct efi_simple_text_output_protocol {
>>       void *reset;
>>       efi_status_t (EFIAPI *output_string)(
>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>> index 2e13fdc096..fcd65ca488 100644
>> --- a/lib/efi_loader/efi_console.c
>> +++ b/lib/efi_loader/efi_console.c
>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>       return EFI_EXIT(EFI_SUCCESS);
>>  }
>>
>> +static const struct {
>> +     unsigned fg;
>> +     unsigned bg;
>> +} color[] = {
>> +     { 30, 40 },     /* 0: black */
>> +     { 34, 44 },     /* 1: blue */
>> +     { 32, 42 },     /* 2: green */
>> +     { 36, 46 },     /* 3: cyan */
>> +     { 31, 41 },     /* 4: red */
>> +     { 35, 45 },     /* 5: magenta */
>> +     { 30, 40 },     /* 6: brown, map to black */
>
> This should be { 33, 43 }
>
>> +     { 37, 47 },     /* 7: light grey, map to white */
>
> The entries below are redundant.
>
>> +     { 37, 47 },     /* 8: bright, map to white */
>> +     { 34, 44 },     /* 9: light blue, map to blue */
>> +     { 32, 42 },     /* A: light green, map to green */
>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>> +     { 31, 41 },     /* C: light red, map to red */
>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>> +     { 33, 43 },     /* E: yellow */
>> +     { 37, 47 },     /* F: white */
>> +};
>> +

I'm not totally convinced about mapping extra colors that UEFI defines
to bold.. unless you have some example of prior-art for this on other
platforms.

There are ansi extensions that allow for more colors, we could perhaps
map the extra EFI colors to the base sequence (presumably more widely
supported) followed by the extended sequence (which presumably not all
terminal emulators support)..  I'm not sure if it is worth the effort.
The current patch implements something that is at least fairly
reasonable and within the bounds of what the spec says (ie. "Devices
supporting a different number of text colors are required to emulate
the above colors to the best of the device’s capabilities.")

BR,
-R

>
> No function without a comment explaining the parameters.
>
>>  static efi_status_t EFIAPI efi_cout_set_attribute(
>>                       struct efi_simple_text_output_protocol *this,
>>                       unsigned long attribute)
>>  {
>> +     unsigned fg = EFI_ATTR_FG(attribute);
>> +     unsigned bg = EFI_ATTR_BG(attribute);
>
> Use unsigned int.
>
>> +
>>       EFI_ENTRY("%p, %lx", this, attribute);
>>
>> +     if (attribute)
>
> Attribute 0 should be black on black. No need for any if here.

yeah, except in practice this results in unreadable black on black
display.. I debugged my way through this the hard way.  The spec
doesn't make it clean, but in practice attribute==0 means to go back
to white on black.

BR,
-R

>> +             printf(ESC"[%u;%um", color[fg].fg, color[bg].bg);
>
> Use bold for light colors.
>
> unsigned int bold;
> if (EFI_ATTRIB_BOLD(attr))
>         bold = 1;
> else
>         bold = 22;
>
>
> printf(ESC"[%u;%u;%um", bold, color[fg].fg, color[bg].bg)
>
> Best regards
>
> Heinrich
>
>> +     else
>> +             printf(ESC"[37;40m");
>
>
>> +
>>       /* Just ignore attributes (colors) for now */
>>       return EFI_EXIT(EFI_UNSUPPORTED);
>>  }
>>
>

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Heinrich Schuchardt]

On 10/04/2017 10:54 PM, Rob Clark wrote:
> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>> Shell.efi uses this, and supporting color attributes makes things look
>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>> Not all colors have a perfect match, but spec just says "Devices
>>> supporting a different number of text colors are required to emulate the
>>> above colors to the best of the device’s capabilities".
>>>
>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>> ---
>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>  2 files changed, 59 insertions(+)
>>>
>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>> index 87c8ffe68e..3cc1dbac2e 100644
>>> --- a/include/efi_api.h
>>> +++ b/include/efi_api.h
>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>
>>> +#define EFI_BLACK                0x00
>>> +#define EFI_BLUE                 0x01
>>> +#define EFI_GREEN                0x02
>>> +#define EFI_CYAN                 0x03
>>> +#define EFI_RED                  0x04
>>> +#define EFI_MAGENTA              0x05
>>> +#define EFI_BROWN                0x06
>>> +#define EFI_LIGHTGRAY            0x07
>>> +#define EFI_BRIGHT               0x08
>>> +#define EFI_DARKGRAY             0x08
>>> +#define EFI_LIGHTBLUE            0x09
>>> +#define EFI_LIGHTGREEN           0x0a
>>> +#define EFI_LIGHTCYAN            0x0b
>>> +#define EFI_LIGHTRED             0x0c
>>> +#define EFI_LIGHTMAGENTA         0x0d
>>> +#define EFI_YELLOW               0x0e
>>> +#define EFI_WHITE                0x0f
>>> +#define EFI_BACKGROUND_BLACK     0x00
>>> +#define EFI_BACKGROUND_BLUE      0x10
>>> +#define EFI_BACKGROUND_GREEN     0x20
>>> +#define EFI_BACKGROUND_CYAN      0x30
>>> +#define EFI_BACKGROUND_RED       0x40
>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>> +#define EFI_BACKGROUND_BROWN     0x60
>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>
>> Will we ever use these constants?
>>
> 
> possibly not, but it is useful to understand what is going on with
> efi->ansi mapping, so I would prefer to keep them.
> 
>>
>> Where are the comments explaining the defines below?
>>
>>> +
>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>
>> This saves 8 entries in the table below.
>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>
>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>
>> Add
>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>
>>> +
>>>  struct efi_simple_text_output_protocol {
>>>       void *reset;
>>>       efi_status_t (EFIAPI *output_string)(
>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>> index 2e13fdc096..fcd65ca488 100644
>>> --- a/lib/efi_loader/efi_console.c
>>> +++ b/lib/efi_loader/efi_console.c
>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>       return EFI_EXIT(EFI_SUCCESS);
>>>  }
>>>
>>> +static const struct {
>>> +     unsigned fg;
>>> +     unsigned bg;
>>> +} color[] = {
>>> +     { 30, 40 },     /* 0: black */
>>> +     { 34, 44 },     /* 1: blue */
>>> +     { 32, 42 },     /* 2: green */
>>> +     { 36, 46 },     /* 3: cyan */
>>> +     { 31, 41 },     /* 4: red */
>>> +     { 35, 45 },     /* 5: magenta */
>>> +     { 30, 40 },     /* 6: brown, map to black */
>>
>> This should be { 33, 43 }
>>
>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>
>> The entries below are redundant.
>>
>>> +     { 37, 47 },     /* 8: bright, map to white */
>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>> +     { 32, 42 },     /* A: light green, map to green */
>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>> +     { 31, 41 },     /* C: light red, map to red */
>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>> +     { 33, 43 },     /* E: yellow */
>>> +     { 37, 47 },     /* F: white */
>>> +};
>>> +
> 
> I'm not totally convinced about mapping extra colors that UEFI defines
> to bold.. unless you have some example of prior-art for this on other
> platforms.

See
Standard ECMA-48 - Control Functions for Coded Character Sets
chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION

1 - bold or increased intensity
22 - normal colour or normal intensity (neither bold nor faint)

You can easily experiment in your bash shell like this:

printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";

You will find that "bold" prints bold and bright in the KDE konsole and
xterm.

Using colors 90-97 as foreground colors produces only bright but not
bold in the KDE konsole and xterm:

printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";

But these codes are not defined in ECMA-48.

Best regards

Heinrich

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Rob Clark]

On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 10/04/2017 10:54 PM, Rob Clark wrote:
>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>> Not all colors have a perfect match, but spec just says "Devices
>>>> supporting a different number of text colors are required to emulate the
>>>> above colors to the best of the device’s capabilities".
>>>>
>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>> ---
>>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>  2 files changed, 59 insertions(+)
>>>>
>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>> --- a/include/efi_api.h
>>>> +++ b/include/efi_api.h
>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>
>>>> +#define EFI_BLACK                0x00
>>>> +#define EFI_BLUE                 0x01
>>>> +#define EFI_GREEN                0x02
>>>> +#define EFI_CYAN                 0x03
>>>> +#define EFI_RED                  0x04
>>>> +#define EFI_MAGENTA              0x05
>>>> +#define EFI_BROWN                0x06
>>>> +#define EFI_LIGHTGRAY            0x07
>>>> +#define EFI_BRIGHT               0x08
>>>> +#define EFI_DARKGRAY             0x08
>>>> +#define EFI_LIGHTBLUE            0x09
>>>> +#define EFI_LIGHTGREEN           0x0a
>>>> +#define EFI_LIGHTCYAN            0x0b
>>>> +#define EFI_LIGHTRED             0x0c
>>>> +#define EFI_LIGHTMAGENTA         0x0d
>>>> +#define EFI_YELLOW               0x0e
>>>> +#define EFI_WHITE                0x0f
>>>> +#define EFI_BACKGROUND_BLACK     0x00
>>>> +#define EFI_BACKGROUND_BLUE      0x10
>>>> +#define EFI_BACKGROUND_GREEN     0x20
>>>> +#define EFI_BACKGROUND_CYAN      0x30
>>>> +#define EFI_BACKGROUND_RED       0x40
>>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>>> +#define EFI_BACKGROUND_BROWN     0x60
>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>
>>> Will we ever use these constants?
>>>
>>
>> possibly not, but it is useful to understand what is going on with
>> efi->ansi mapping, so I would prefer to keep them.
>>
>>>
>>> Where are the comments explaining the defines below?
>>>
>>>> +
>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>>
>>> This saves 8 entries in the table below.
>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>>
>>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>>
>>> Add
>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>
>>>> +
>>>>  struct efi_simple_text_output_protocol {
>>>>       void *reset;
>>>>       efi_status_t (EFIAPI *output_string)(
>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>> index 2e13fdc096..fcd65ca488 100644
>>>> --- a/lib/efi_loader/efi_console.c
>>>> +++ b/lib/efi_loader/efi_console.c
>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>       return EFI_EXIT(EFI_SUCCESS);
>>>>  }
>>>>
>>>> +static const struct {
>>>> +     unsigned fg;
>>>> +     unsigned bg;
>>>> +} color[] = {
>>>> +     { 30, 40 },     /* 0: black */
>>>> +     { 34, 44 },     /* 1: blue */
>>>> +     { 32, 42 },     /* 2: green */
>>>> +     { 36, 46 },     /* 3: cyan */
>>>> +     { 31, 41 },     /* 4: red */
>>>> +     { 35, 45 },     /* 5: magenta */
>>>> +     { 30, 40 },     /* 6: brown, map to black */
>>>
>>> This should be { 33, 43 }
>>>
>>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>>
>>> The entries below are redundant.
>>>
>>>> +     { 37, 47 },     /* 8: bright, map to white */
>>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>>> +     { 32, 42 },     /* A: light green, map to green */
>>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>>> +     { 31, 41 },     /* C: light red, map to red */
>>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>>> +     { 33, 43 },     /* E: yellow */
>>>> +     { 37, 47 },     /* F: white */
>>>> +};
>>>> +
>>
>> I'm not totally convinced about mapping extra colors that UEFI defines
>> to bold.. unless you have some example of prior-art for this on other
>> platforms.
>
> See
> Standard ECMA-48 - Control Functions for Coded Character Sets
> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>
> 1 - bold or increased intensity
> 22 - normal colour or normal intensity (neither bold nor faint)
>
> You can easily experiment in your bash shell like this:
>
> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>
> You will find that "bold" prints bold and bright in the KDE konsole and
> xterm.

but I think we don't want (potential) font changes, just color changes..

if you can find the code in edk2 that does this, I guess it would be a
reasonable precedent to follow.. but if not I wanted to avoid things
that might be specific to particular terminal emulators, since I
wasn't really looking forward to testing them all.  Otherwise I'd just
rely on the extension that allowed 256 colors..

BR,
-R

> Using colors 90-97 as foreground colors produces only bright but not
> bold in the KDE konsole and xterm:
>
> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>
> But these codes are not defined in ECMA-48.
>
> Best regards
>
> Heinrich
>

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Heinrich Schuchardt]

On 10/05/2017 01:19 AM, Rob Clark wrote:
> On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 10/04/2017 10:54 PM, Rob Clark wrote:
>>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>>> Not all colors have a perfect match, but spec just says "Devices
>>>>> supporting a different number of text colors are required to emulate the
>>>>> above colors to the best of the device’s capabilities".
>>>>>
>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>> ---
>>>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>>  2 files changed, 59 insertions(+)
>>>>>
>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>>> --- a/include/efi_api.h
>>>>> +++ b/include/efi_api.h
>>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>>
>>>>> +#define EFI_BLACK                0x00
>>>>> +#define EFI_BLUE                 0x01
>>>>> +#define EFI_GREEN                0x02
>>>>> +#define EFI_CYAN                 0x03
>>>>> +#define EFI_RED                  0x04
>>>>> +#define EFI_MAGENTA              0x05
>>>>> +#define EFI_BROWN                0x06
>>>>> +#define EFI_LIGHTGRAY            0x07
>>>>> +#define EFI_BRIGHT               0x08
>>>>> +#define EFI_DARKGRAY             0x08
>>>>> +#define EFI_LIGHTBLUE            0x09
>>>>> +#define EFI_LIGHTGREEN           0x0a
>>>>> +#define EFI_LIGHTCYAN            0x0b
>>>>> +#define EFI_LIGHTRED             0x0c
>>>>> +#define EFI_LIGHTMAGENTA         0x0d
>>>>> +#define EFI_YELLOW               0x0e
>>>>> +#define EFI_WHITE                0x0f
>>>>> +#define EFI_BACKGROUND_BLACK     0x00
>>>>> +#define EFI_BACKGROUND_BLUE      0x10
>>>>> +#define EFI_BACKGROUND_GREEN     0x20
>>>>> +#define EFI_BACKGROUND_CYAN      0x30
>>>>> +#define EFI_BACKGROUND_RED       0x40
>>>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>>>> +#define EFI_BACKGROUND_BROWN     0x60
>>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>>
>>>> Will we ever use these constants?
>>>>
>>>
>>> possibly not, but it is useful to understand what is going on with
>>> efi->ansi mapping, so I would prefer to keep them.
>>>
>>>>
>>>> Where are the comments explaining the defines below?
>>>>
>>>>> +
>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>>>
>>>> This saves 8 entries in the table below.
>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>>>
>>>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>>>
>>>> Add
>>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>>
>>>>> +
>>>>>  struct efi_simple_text_output_protocol {
>>>>>       void *reset;
>>>>>       efi_status_t (EFIAPI *output_string)(
>>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>>> index 2e13fdc096..fcd65ca488 100644
>>>>> --- a/lib/efi_loader/efi_console.c
>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>>       return EFI_EXIT(EFI_SUCCESS);
>>>>>  }
>>>>>
>>>>> +static const struct {
>>>>> +     unsigned fg;
>>>>> +     unsigned bg;
>>>>> +} color[] = {
>>>>> +     { 30, 40 },     /* 0: black */
>>>>> +     { 34, 44 },     /* 1: blue */
>>>>> +     { 32, 42 },     /* 2: green */
>>>>> +     { 36, 46 },     /* 3: cyan */
>>>>> +     { 31, 41 },     /* 4: red */
>>>>> +     { 35, 45 },     /* 5: magenta */
>>>>> +     { 30, 40 },     /* 6: brown, map to black */
>>>>
>>>> This should be { 33, 43 }
>>>>
>>>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>>>
>>>> The entries below are redundant.
>>>>
>>>>> +     { 37, 47 },     /* 8: bright, map to white */
>>>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>>>> +     { 32, 42 },     /* A: light green, map to green */
>>>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>>>> +     { 31, 41 },     /* C: light red, map to red */
>>>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>>>> +     { 33, 43 },     /* E: yellow */
>>>>> +     { 37, 47 },     /* F: white */
>>>>> +};
>>>>> +
>>>
>>> I'm not totally convinced about mapping extra colors that UEFI defines
>>> to bold.. unless you have some example of prior-art for this on other
>>> platforms.
>>
>> See
>> Standard ECMA-48 - Control Functions for Coded Character Sets
>> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>>
>> 1 - bold or increased intensity
>> 22 - normal colour or normal intensity (neither bold nor faint)
>>
>> You can easily experiment in your bash shell like this:
>>
>> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>>
>> You will find that "bold" prints bold and bright in the KDE konsole and
>> xterm.
> 
> but I think we don't want (potential) font changes, just color changes..
> 
> if you can find the code in edk2 that does this, I guess it would be a
> reasonable precedent to follow.. but if not I wanted to avoid things
> that might be specific to particular terminal emulators, since I
> wasn't really looking forward to testing them all.  Otherwise I'd just
> rely on the extension that allowed 256 colors..
> 
> BR,
> -R

The same problem seems has led the EDK folks to a similar solution.

See
MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c

Everything starts with this array:

{ ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };

The first '0' is replaced by either 0 or 1 depending on brightness.

mSetAttributeString[BRIGHT_CONTROL_OFFSET] =
  (CHAR16) ('0' + BrightControl);

The first '4', '0' is replaced by the foreground color.
The second '4', '0' is replaced by the background color.

ECMA 48 says:

0 - default rendition, cancels the effect of any preceding SGR

So you can use this instead of 22.

Best regards

Heinrich


> 
>> Using colors 90-97 as foreground colors produces only bright but not
>> bold in the KDE konsole and xterm:
>>
>> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>>
>> But these codes are not defined in ECMA-48.
>>
>> Best regards
>>
>> Heinrich
>>
> 

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Rob Clark]

On Wed, Oct 4, 2017 at 7:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
> On 10/05/2017 01:19 AM, Rob Clark wrote:
>> On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 10/04/2017 10:54 PM, Rob Clark wrote:
>>>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>>>> Not all colors have a perfect match, but spec just says "Devices
>>>>>> supporting a different number of text colors are required to emulate the
>>>>>> above colors to the best of the device’s capabilities".
>>>>>>
>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>> ---
>>>>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>>>  2 files changed, 59 insertions(+)
>>>>>>
>>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>>>> --- a/include/efi_api.h
>>>>>> +++ b/include/efi_api.h
>>>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>>>
>>>>>> +#define EFI_BLACK                0x00
>>>>>> +#define EFI_BLUE                 0x01
>>>>>> +#define EFI_GREEN                0x02
>>>>>> +#define EFI_CYAN                 0x03
>>>>>> +#define EFI_RED                  0x04
>>>>>> +#define EFI_MAGENTA              0x05
>>>>>> +#define EFI_BROWN                0x06
>>>>>> +#define EFI_LIGHTGRAY            0x07
>>>>>> +#define EFI_BRIGHT               0x08
>>>>>> +#define EFI_DARKGRAY             0x08
>>>>>> +#define EFI_LIGHTBLUE            0x09
>>>>>> +#define EFI_LIGHTGREEN           0x0a
>>>>>> +#define EFI_LIGHTCYAN            0x0b
>>>>>> +#define EFI_LIGHTRED             0x0c
>>>>>> +#define EFI_LIGHTMAGENTA         0x0d
>>>>>> +#define EFI_YELLOW               0x0e
>>>>>> +#define EFI_WHITE                0x0f
>>>>>> +#define EFI_BACKGROUND_BLACK     0x00
>>>>>> +#define EFI_BACKGROUND_BLUE      0x10
>>>>>> +#define EFI_BACKGROUND_GREEN     0x20
>>>>>> +#define EFI_BACKGROUND_CYAN      0x30
>>>>>> +#define EFI_BACKGROUND_RED       0x40
>>>>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>>>>> +#define EFI_BACKGROUND_BROWN     0x60
>>>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>>>
>>>>> Will we ever use these constants?
>>>>>
>>>>
>>>> possibly not, but it is useful to understand what is going on with
>>>> efi->ansi mapping, so I would prefer to keep them.
>>>>
>>>>>
>>>>> Where are the comments explaining the defines below?
>>>>>
>>>>>> +
>>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>>>>
>>>>> This saves 8 entries in the table below.
>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>>>>
>>>>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>>>>
>>>>> Add
>>>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>>>
>>>>>> +
>>>>>>  struct efi_simple_text_output_protocol {
>>>>>>       void *reset;
>>>>>>       efi_status_t (EFIAPI *output_string)(
>>>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>>>> index 2e13fdc096..fcd65ca488 100644
>>>>>> --- a/lib/efi_loader/efi_console.c
>>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>>>       return EFI_EXIT(EFI_SUCCESS);
>>>>>>  }
>>>>>>
>>>>>> +static const struct {
>>>>>> +     unsigned fg;
>>>>>> +     unsigned bg;
>>>>>> +} color[] = {
>>>>>> +     { 30, 40 },     /* 0: black */
>>>>>> +     { 34, 44 },     /* 1: blue */
>>>>>> +     { 32, 42 },     /* 2: green */
>>>>>> +     { 36, 46 },     /* 3: cyan */
>>>>>> +     { 31, 41 },     /* 4: red */
>>>>>> +     { 35, 45 },     /* 5: magenta */
>>>>>> +     { 30, 40 },     /* 6: brown, map to black */
>>>>>
>>>>> This should be { 33, 43 }
>>>>>
>>>>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>>>>
>>>>> The entries below are redundant.
>>>>>
>>>>>> +     { 37, 47 },     /* 8: bright, map to white */
>>>>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>>>>> +     { 32, 42 },     /* A: light green, map to green */
>>>>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>>>>> +     { 31, 41 },     /* C: light red, map to red */
>>>>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>>>>> +     { 33, 43 },     /* E: yellow */
>>>>>> +     { 37, 47 },     /* F: white */
>>>>>> +};
>>>>>> +
>>>>
>>>> I'm not totally convinced about mapping extra colors that UEFI defines
>>>> to bold.. unless you have some example of prior-art for this on other
>>>> platforms.
>>>
>>> See
>>> Standard ECMA-48 - Control Functions for Coded Character Sets
>>> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>>>
>>> 1 - bold or increased intensity
>>> 22 - normal colour or normal intensity (neither bold nor faint)
>>>
>>> You can easily experiment in your bash shell like this:
>>>
>>> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>>>
>>> You will find that "bold" prints bold and bright in the KDE konsole and
>>> xterm.
>>
>> but I think we don't want (potential) font changes, just color changes..
>>
>> if you can find the code in edk2 that does this, I guess it would be a
>> reasonable precedent to follow.. but if not I wanted to avoid things
>> that might be specific to particular terminal emulators, since I
>> wasn't really looking forward to testing them all.  Otherwise I'd just
>> rely on the extension that allowed 256 colors..
>>
>> BR,
>> -R
>
> The same problem seems has led the EDK folks to a similar solution.
>
> See
> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c

ok, I'll have a closer look at that.. I don't feel badly about doing
the same thing that edk2 does when there is doubt ;-)

BR,
-R


> Everything starts with this array:
>
> { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };
>
> The first '0' is replaced by either 0 or 1 depending on brightness.
>
> mSetAttributeString[BRIGHT_CONTROL_OFFSET] =
>   (CHAR16) ('0' + BrightControl);
>
> The first '4', '0' is replaced by the foreground color.
> The second '4', '0' is replaced by the background color.
>
> ECMA 48 says:
>
> 0 - default rendition, cancels the effect of any preceding SGR
>
> So you can use this instead of 22.
>
> Best regards
>
> Heinrich
>
>
>>
>>> Using colors 90-97 as foreground colors produces only bright but not
>>> bold in the KDE konsole and xterm:
>>>
>>> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>>>
>>> But these codes are not defined in ECMA-48.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>
>

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Rob Clark]

On Wed, Oct 4, 2017 at 8:00 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Wed, Oct 4, 2017 at 7:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>> On 10/05/2017 01:19 AM, Rob Clark wrote:
>>> On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>> On 10/04/2017 10:54 PM, Rob Clark wrote:
>>>>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>>>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>>>>> Not all colors have a perfect match, but spec just says "Devices
>>>>>>> supporting a different number of text colors are required to emulate the
>>>>>>> above colors to the best of the device’s capabilities".
>>>>>>>
>>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>>> ---
>>>>>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>>>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>>>>  2 files changed, 59 insertions(+)
>>>>>>>
>>>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>>>>> --- a/include/efi_api.h
>>>>>>> +++ b/include/efi_api.h
>>>>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>>>>
>>>>>>> +#define EFI_BLACK                0x00
>>>>>>> +#define EFI_BLUE                 0x01
>>>>>>> +#define EFI_GREEN                0x02
>>>>>>> +#define EFI_CYAN                 0x03
>>>>>>> +#define EFI_RED                  0x04
>>>>>>> +#define EFI_MAGENTA              0x05
>>>>>>> +#define EFI_BROWN                0x06
>>>>>>> +#define EFI_LIGHTGRAY            0x07
>>>>>>> +#define EFI_BRIGHT               0x08
>>>>>>> +#define EFI_DARKGRAY             0x08
>>>>>>> +#define EFI_LIGHTBLUE            0x09
>>>>>>> +#define EFI_LIGHTGREEN           0x0a
>>>>>>> +#define EFI_LIGHTCYAN            0x0b
>>>>>>> +#define EFI_LIGHTRED             0x0c
>>>>>>> +#define EFI_LIGHTMAGENTA         0x0d
>>>>>>> +#define EFI_YELLOW               0x0e
>>>>>>> +#define EFI_WHITE                0x0f
>>>>>>> +#define EFI_BACKGROUND_BLACK     0x00
>>>>>>> +#define EFI_BACKGROUND_BLUE      0x10
>>>>>>> +#define EFI_BACKGROUND_GREEN     0x20
>>>>>>> +#define EFI_BACKGROUND_CYAN      0x30
>>>>>>> +#define EFI_BACKGROUND_RED       0x40
>>>>>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>>>>>> +#define EFI_BACKGROUND_BROWN     0x60
>>>>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>>>>
>>>>>> Will we ever use these constants?
>>>>>>
>>>>>
>>>>> possibly not, but it is useful to understand what is going on with
>>>>> efi->ansi mapping, so I would prefer to keep them.
>>>>>
>>>>>>
>>>>>> Where are the comments explaining the defines below?
>>>>>>
>>>>>>> +
>>>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>>>>>
>>>>>> This saves 8 entries in the table below.
>>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>>>>>
>>>>>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>>>>>
>>>>>> Add
>>>>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>>>>
>>>>>>> +
>>>>>>>  struct efi_simple_text_output_protocol {
>>>>>>>       void *reset;
>>>>>>>       efi_status_t (EFIAPI *output_string)(
>>>>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>>>>> index 2e13fdc096..fcd65ca488 100644
>>>>>>> --- a/lib/efi_loader/efi_console.c
>>>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>>>>       return EFI_EXIT(EFI_SUCCESS);
>>>>>>>  }
>>>>>>>
>>>>>>> +static const struct {
>>>>>>> +     unsigned fg;
>>>>>>> +     unsigned bg;
>>>>>>> +} color[] = {
>>>>>>> +     { 30, 40 },     /* 0: black */
>>>>>>> +     { 34, 44 },     /* 1: blue */
>>>>>>> +     { 32, 42 },     /* 2: green */
>>>>>>> +     { 36, 46 },     /* 3: cyan */
>>>>>>> +     { 31, 41 },     /* 4: red */
>>>>>>> +     { 35, 45 },     /* 5: magenta */
>>>>>>> +     { 30, 40 },     /* 6: brown, map to black */
>>>>>>
>>>>>> This should be { 33, 43 }
>>>>>>
>>>>>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>>>>>
>>>>>> The entries below are redundant.
>>>>>>
>>>>>>> +     { 37, 47 },     /* 8: bright, map to white */
>>>>>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>>>>>> +     { 32, 42 },     /* A: light green, map to green */
>>>>>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>>>>>> +     { 31, 41 },     /* C: light red, map to red */
>>>>>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>>>>>> +     { 33, 43 },     /* E: yellow */
>>>>>>> +     { 37, 47 },     /* F: white */
>>>>>>> +};
>>>>>>> +
>>>>>
>>>>> I'm not totally convinced about mapping extra colors that UEFI defines
>>>>> to bold.. unless you have some example of prior-art for this on other
>>>>> platforms.
>>>>
>>>> See
>>>> Standard ECMA-48 - Control Functions for Coded Character Sets
>>>> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>>>>
>>>> 1 - bold or increased intensity
>>>> 22 - normal colour or normal intensity (neither bold nor faint)
>>>>
>>>> You can easily experiment in your bash shell like this:
>>>>
>>>> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>>>>
>>>> You will find that "bold" prints bold and bright in the KDE konsole and
>>>> xterm.
>>>
>>> but I think we don't want (potential) font changes, just color changes..
>>>
>>> if you can find the code in edk2 that does this, I guess it would be a
>>> reasonable precedent to follow.. but if not I wanted to avoid things
>>> that might be specific to particular terminal emulators, since I
>>> wasn't really looking forward to testing them all.  Otherwise I'd just
>>> rely on the extension that allowed 256 colors..
>>>
>>> BR,
>>> -R
>>
>> The same problem seems has led the EDK folks to a similar solution.
>>
>> See
>> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>
> ok, I'll have a closer look at that.. I don't feel badly about doing
> the same thing that edk2 does when there is doubt ;-)

hmm, the semi-annoying thing will be to have to implement the
vidconsole-uclass side of this.. I suppose I could ignore anything
other than 0 (normal) and 1 (bold).  Reverse wouldn't be too hard, but
blink would be hard I think without timer interrupts (same reason that
I didn't implement cursor yet)

> BR,
> -R
>
>
>> Everything starts with this array:
>>
>> { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };
>>
>> The first '0' is replaced by either 0 or 1 depending on brightness.
>>
>> mSetAttributeString[BRIGHT_CONTROL_OFFSET] =
>>   (CHAR16) ('0' + BrightControl);
>>
>> The first '4', '0' is replaced by the foreground color.
>> The second '4', '0' is replaced by the background color.
>>
>> ECMA 48 says:
>>
>> 0 - default rendition, cancels the effect of any preceding SGR
>>
>> So you can use this instead of 22.
>>
>> Best regards
>>
>> Heinrich
>>
>>
>>>
>>>> Using colors 90-97 as foreground colors produces only bright but not
>>>> bold in the KDE konsole and xterm:
>>>>
>>>> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>>>>
>>>> But these codes are not defined in ECMA-48.
>>>>
>>>> Best regards
>>>>
>>>> Heinrich
>>>>
>>>
>>

[U-Boot] [PATCH v1 08/12] efi_loader: console support for color attributes

[Heinrich Schuchardt]

On 10/05/2017 02:12 AM, Rob Clark wrote:
> On Wed, Oct 4, 2017 at 8:00 PM, Rob Clark <robdclark@gmail.com> wrote:
>> On Wed, Oct 4, 2017 at 7:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>> On 10/05/2017 01:19 AM, Rob Clark wrote:
>>>> On Wed, Oct 4, 2017 at 6:01 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>> On 10/04/2017 10:54 PM, Rob Clark wrote:
>>>>>> On Wed, Oct 4, 2017 at 2:53 PM, Heinrich Schuchardt <xypron.glpk@gmx.de> wrote:
>>>>>>> On 09/10/2017 03:22 PM, Rob Clark wrote:
>>>>>>>> Shell.efi uses this, and supporting color attributes makes things look
>>>>>>>> nicer.  Map the EFI fg/bg color attributes to ANSI escape sequences.
>>>>>>>> Not all colors have a perfect match, but spec just says "Devices
>>>>>>>> supporting a different number of text colors are required to emulate the
>>>>>>>> above colors to the best of the device’s capabilities".
>>>>>>>>
>>>>>>>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>>>>>>>> ---
>>>>>>>>  include/efi_api.h            | 29 +++++++++++++++++++++++++++++
>>>>>>>>  lib/efi_loader/efi_console.c | 30 ++++++++++++++++++++++++++++++
>>>>>>>>  2 files changed, 59 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/include/efi_api.h b/include/efi_api.h
>>>>>>>> index 87c8ffe68e..3cc1dbac2e 100644
>>>>>>>> --- a/include/efi_api.h
>>>>>>>> +++ b/include/efi_api.h
>>>>>>>> @@ -426,6 +426,35 @@ struct simple_text_output_mode {
>>>>>>>>       EFI_GUID(0x387477c2, 0x69c7, 0x11d2, \
>>>>>>>>                0x8e, 0x39, 0x0, 0xa0, 0xc9, 0x69, 0x72, 0x3b)
>>>>>>>>
>>>>>>>> +#define EFI_BLACK                0x00
>>>>>>>> +#define EFI_BLUE                 0x01
>>>>>>>> +#define EFI_GREEN                0x02
>>>>>>>> +#define EFI_CYAN                 0x03
>>>>>>>> +#define EFI_RED                  0x04
>>>>>>>> +#define EFI_MAGENTA              0x05
>>>>>>>> +#define EFI_BROWN                0x06
>>>>>>>> +#define EFI_LIGHTGRAY            0x07
>>>>>>>> +#define EFI_BRIGHT               0x08
>>>>>>>> +#define EFI_DARKGRAY             0x08
>>>>>>>> +#define EFI_LIGHTBLUE            0x09
>>>>>>>> +#define EFI_LIGHTGREEN           0x0a
>>>>>>>> +#define EFI_LIGHTCYAN            0x0b
>>>>>>>> +#define EFI_LIGHTRED             0x0c
>>>>>>>> +#define EFI_LIGHTMAGENTA         0x0d
>>>>>>>> +#define EFI_YELLOW               0x0e
>>>>>>>> +#define EFI_WHITE                0x0f
>>>>>>>> +#define EFI_BACKGROUND_BLACK     0x00
>>>>>>>> +#define EFI_BACKGROUND_BLUE      0x10
>>>>>>>> +#define EFI_BACKGROUND_GREEN     0x20
>>>>>>>> +#define EFI_BACKGROUND_CYAN      0x30
>>>>>>>> +#define EFI_BACKGROUND_RED       0x40
>>>>>>>> +#define EFI_BACKGROUND_MAGENTA   0x50
>>>>>>>> +#define EFI_BACKGROUND_BROWN     0x60
>>>>>>>> +#define EFI_BACKGROUND_LIGHTGRAY 0x70
>>>>>>>
>>>>>>> Will we ever use these constants?
>>>>>>>
>>>>>>
>>>>>> possibly not, but it is useful to understand what is going on with
>>>>>> efi->ansi mapping, so I would prefer to keep them.
>>>>>>
>>>>>>>
>>>>>>> Where are the comments explaining the defines below?
>>>>>>>
>>>>>>>> +
>>>>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x0f)
>>>>>>>
>>>>>>> This saves 8 entries in the table below.
>>>>>>> +#define EFI_ATTR_FG(attr)        ((attr) & 0x07)
>>>>>>>
>>>>>>>> +#define EFI_ATTR_BG(attr)        (((attr) >> 4) & 0x7)
>>>>>>>
>>>>>>> Add
>>>>>>> #define EFI_ATTR_BOLD(attr) (((attr) >> 3) & 0x01)
>>>>>>>
>>>>>>>> +
>>>>>>>>  struct efi_simple_text_output_protocol {
>>>>>>>>       void *reset;
>>>>>>>>       efi_status_t (EFIAPI *output_string)(
>>>>>>>> diff --git a/lib/efi_loader/efi_console.c b/lib/efi_loader/efi_console.c
>>>>>>>> index 2e13fdc096..fcd65ca488 100644
>>>>>>>> --- a/lib/efi_loader/efi_console.c
>>>>>>>> +++ b/lib/efi_loader/efi_console.c
>>>>>>>> @@ -316,12 +316,42 @@ static efi_status_t EFIAPI efi_cout_set_mode(
>>>>>>>>       return EFI_EXIT(EFI_SUCCESS);
>>>>>>>>  }
>>>>>>>>
>>>>>>>> +static const struct {
>>>>>>>> +     unsigned fg;
>>>>>>>> +     unsigned bg;
>>>>>>>> +} color[] = {
>>>>>>>> +     { 30, 40 },     /* 0: black */
>>>>>>>> +     { 34, 44 },     /* 1: blue */
>>>>>>>> +     { 32, 42 },     /* 2: green */
>>>>>>>> +     { 36, 46 },     /* 3: cyan */
>>>>>>>> +     { 31, 41 },     /* 4: red */
>>>>>>>> +     { 35, 45 },     /* 5: magenta */
>>>>>>>> +     { 30, 40 },     /* 6: brown, map to black */
>>>>>>>
>>>>>>> This should be { 33, 43 }
>>>>>>>
>>>>>>>> +     { 37, 47 },     /* 7: light grey, map to white */
>>>>>>>
>>>>>>> The entries below are redundant.
>>>>>>>
>>>>>>>> +     { 37, 47 },     /* 8: bright, map to white */
>>>>>>>> +     { 34, 44 },     /* 9: light blue, map to blue */
>>>>>>>> +     { 32, 42 },     /* A: light green, map to green */
>>>>>>>> +     { 36, 46 },     /* B: light cyan, map to cyan */
>>>>>>>> +     { 31, 41 },     /* C: light red, map to red */
>>>>>>>> +     { 35, 45 },     /* D: light magenta, map to magenta */
>>>>>>>> +     { 33, 43 },     /* E: yellow */
>>>>>>>> +     { 37, 47 },     /* F: white */
>>>>>>>> +};
>>>>>>>> +
>>>>>>
>>>>>> I'm not totally convinced about mapping extra colors that UEFI defines
>>>>>> to bold.. unless you have some example of prior-art for this on other
>>>>>> platforms.
>>>>>
>>>>> See
>>>>> Standard ECMA-48 - Control Functions for Coded Character Sets
>>>>> chapter 8.3.117 SGR - SELECT GRAPHIC RENDITION
>>>>>
>>>>> 1 - bold or increased intensity
>>>>> 22 - normal colour or normal intensity (neither bold nor faint)
>>>>>
>>>>> You can easily experiment in your bash shell like this:
>>>>>
>>>>> printf "\x1b[1;32;40m bold \x1b[22;32;40m normal\x1b[22;39;49m\n";
>>>>>
>>>>> You will find that "bold" prints bold and bright in the KDE konsole and
>>>>> xterm.
>>>>
>>>> but I think we don't want (potential) font changes, just color changes..
>>>>
>>>> if you can find the code in edk2 that does this, I guess it would be a
>>>> reasonable precedent to follow.. but if not I wanted to avoid things
>>>> that might be specific to particular terminal emulators, since I
>>>> wasn't really looking forward to testing them all.  Otherwise I'd just
>>>> rely on the extension that allowed 256 colors..
>>>>
>>>> BR,
>>>> -R
>>>
>>> The same problem seems has led the EDK folks to a similar solution.
>>>
>>> See
>>> MdeModulePkg/Universal/Console/TerminalDxe/TerminalConOut.c
>>
>> ok, I'll have a closer look at that.. I don't feel badly about doing
>> the same thing that edk2 does when there is doubt ;-)
> 
> hmm, the semi-annoying thing will be to have to implement the
> vidconsole-uclass side of this.. I suppose I could ignore anything
> other than 0 (normal) and 1 (bold).  Reverse wouldn't be too hard, but
> blink would be hard I think without timer interrupts (same reason that
> I didn't implement cursor yet)

It is not necessary that a cursor is blinking. The KDE konsole only has
a filled block (something like Unicode U+2588). For the colors you can
use a RGB lookup table. EDK uses these colors:

EFI_GRAPHICS_OUTPUT_BLT_PIXEL        mGraphicsEfiColors[16] = {
  //
  // B    G    R   reserved
  //
  {0x00, 0x00, 0x00, 0x00},  // BLACK
  {0x98, 0x00, 0x00, 0x00},  // LIGHTBLUE
  {0x00, 0x98, 0x00, 0x00},  // LIGHGREEN
  {0x98, 0x98, 0x00, 0x00},  // LIGHCYAN
  {0x00, 0x00, 0x98, 0x00},  // LIGHRED
  {0x98, 0x00, 0x98, 0x00},  // MAGENTA
  {0x00, 0x98, 0x98, 0x00},  // BROWN
  {0x98, 0x98, 0x98, 0x00},  // LIGHTGRAY
  {0x30, 0x30, 0x30, 0x00},  // DARKGRAY - BRIGHT BLACK
  {0xff, 0x00, 0x00, 0x00},  // BLUE
  {0x00, 0xff, 0x00, 0x00},  // LIME
  {0xff, 0xff, 0x00, 0x00},  // CYAN
  {0x00, 0x00, 0xff, 0x00},  // RED
  {0xff, 0x00, 0xff, 0x00},  // FUCHSIA
  {0x00, 0xff, 0xff, 0x00},  // YELLOW
  {0xff, 0xff, 0xff, 0x00}   // WHITE
};

Oracle prefers darker colors and brighter grays:
https://docs.oracle.com/cd/E19728-01/820-2550/term_em_colormaps.html

Best regards

Heinrich

> 
>> BR,
>> -R
>>
>>
>>> Everything starts with this array:
>>>
>>> { ESC, '[', '0', 'm', ESC, '[', '4', '0', 'm', ESC, '[', '4', '0', 'm', 0 };
>>>
>>> The first '0' is replaced by either 0 or 1 depending on brightness.
>>>
>>> mSetAttributeString[BRIGHT_CONTROL_OFFSET] =
>>>   (CHAR16) ('0' + BrightControl);
>>>
>>> The first '4', '0' is replaced by the foreground color.
>>> The second '4', '0' is replaced by the background color.
>>>
>>> ECMA 48 says:
>>>
>>> 0 - default rendition, cancels the effect of any preceding SGR
>>>
>>> So you can use this instead of 22.
>>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>
>>>>
>>>>> Using colors 90-97 as foreground colors produces only bright but not
>>>>> bold in the KDE konsole and xterm:
>>>>>
>>>>> printf "\x1b[92;40m bold \x1b[32;40m normal\x1b[22;39;49m\n";
>>>>>
>>>>> But these codes are not defined in ECMA-48.
>>>>>
>>>>> Best regards
>>>>>
>>>>> Heinrich
>>>>>
>>>>
>>>
>