[PATCH 0/2] efi: fix and test hii keyboard layout pointer computation

[PATCH 0/2] efi: fix and test hii keyboard layout pointer computation

[Vincent Stehlé]

- Fix one scaled pointer arithmetic bug in EFI Loader HII implementation.
- Enhance the existing EFI HII unit test to catch the bug.

The unit test can run on the sandbox with the following command:

  ./u-boot -T -c "setenv efi_selftest HII database protocols; \
		  bootefi selftest"

Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
---
Vincent Stehlé (2):
      efi_loader: fix hii keyboard layout pointer computation
      efi_selftest: test hii keyboard layouts more

 lib/efi_loader/efi_hii.c                 |  3 +-
 lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
 lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
 3 files changed, 51 insertions(+), 31 deletions(-)
---
base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5
change-id: 20260512-layout-pointer-29d1cd52c070

Best regards,
-- 
Vincent.

[PATCH 1/2] efi_loader: fix hii keyboard layout pointer computation

[Vincent Stehlé]

The EFI_HII_KEYBOARD_LAYOUT field `layout_length' is expressed in bytes,
but we add it to the `layout' pointer with (scaled) pointer arithmetic.
When adding an HII keyboard package with multiple keyboard layouts, this
results in only the first layout being added correctly; fix it.

Fixes: 8d3b77e36e10 ("efi: hii: add keyboard layout package support")
Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
Cc: AKASHI Takahiro <akashi.tkhro@gmail.com>
---
 lib/efi_loader/efi_hii.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_hii.c b/lib/efi_loader/efi_hii.c
index 330d7c5830b..7bf51ad43d1 100644
--- a/lib/efi_loader/efi_hii.c
+++ b/lib/efi_loader/efi_hii.c
@@ -324,7 +324,8 @@ add_keyboard_package(struct efi_hii_packagelist *hii,
 		list_add_tail(&layout_data->link_sys,
 			      &efi_keyboard_layout_list);
 
-		layout += layout_length;
+		layout = (struct efi_hii_keyboard_layout *)
+			 ((uintptr_t)layout + layout_length);
 	}
 
 	list_add_tail(&package_data->link, &hii->keyboard_packages);

-- 
2.53.0

[PATCH 2/2] efi_selftest: test hii keyboard layouts more

[Vincent Stehlé]

The HII database test for keyboard layouts register two package lists with
two keyboard layouts each, but the test verifies only the GUID of the first
keyboard layout.
This does not catch the bugs happening with the keyboard layouts after the
first one in a package.

Verify all the keyboard layout GUIDs in the unit test to prevent this.

Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tom Rini <trini@konsulko.com>
---
 lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
 lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
 2 files changed, 49 insertions(+), 30 deletions(-)

diff --git a/lib/efi_selftest/efi_selftest_hii.c b/lib/efi_selftest/efi_selftest_hii.c
index 228dc296950..fdbb08fb417 100644
--- a/lib/efi_selftest/efi_selftest_hii.c
+++ b/lib/efi_selftest/efi_selftest_hii.c
@@ -452,8 +452,7 @@ out:
  * test_hii_database_get_keyboard_layout() - test retrieval of keyboard layout
  *
  * This test adds two package lists, each of which has two keyboard layouts
- * and then tries to get a handle to keyboard layout with a specific guid
- * and the current one.
+ * and then tries to get a handle to every keyboard layout and the current one.
  *
  * @Return:     status code
  */
@@ -463,7 +462,11 @@ static int test_hii_database_get_keyboard_layout(void)
 	struct efi_hii_keyboard_layout *kb_layout;
 	u16 kb_layout_size;
 	efi_status_t ret;
-	int result = EFI_ST_FAILURE;
+	int result = EFI_ST_FAILURE, i;
+	static efi_guid_t *const kb_layout_guids[] = {
+		&kb_layout_guid11, &kb_layout_guid12,
+		&kb_layout_guid21, &kb_layout_guid22
+	};
 
 	PRINT_TESTNAME;
 	ret = hii_database_protocol->new_package_list(hii_database_protocol,
@@ -484,33 +487,37 @@ static int test_hii_database_get_keyboard_layout(void)
 		goto out;
 	}
 
-	/* specific keyboard_layout(guid11) */
-	kb_layout = NULL;
-	kb_layout_size = 0;
-	ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
-			&kb_layout_guid11, &kb_layout_size, kb_layout);
-	if (ret != EFI_BUFFER_TOO_SMALL) {
-		efi_st_error("get_keyboard_layout returned %u\n",
-			     (unsigned int)ret);
-		goto out;
-	}
-	ret = boottime->allocate_pool(EFI_LOADER_DATA, kb_layout_size,
-				      (void **)&kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("AllocatePool failed\n");
-		goto out;
-	}
-	ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
-			&kb_layout_guid11, &kb_layout_size, kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("get_keyboard_layout returned %u\n",
-			     (unsigned int)ret);
-		goto out;
-	}
-	ret = boottime->free_pool(kb_layout);
-	if (ret != EFI_SUCCESS) {
-		efi_st_error("FreePool failed\n");
-		goto out;
+	/* Verify all keyboard layouts */
+	for (i = 0; i < ARRAY_SIZE(kb_layout_guids); i++) {
+		efi_guid_t *kb_layout_guid = kb_layout_guids[i];
+
+		kb_layout = NULL;
+		kb_layout_size = 0;
+		ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
+				kb_layout_guid, &kb_layout_size, kb_layout);
+		if (ret != EFI_BUFFER_TOO_SMALL) {
+			efi_st_error("get_keyboard_layout returned %u\n",
+				     (unsigned int)ret);
+			goto out;
+		}
+		ret = boottime->allocate_pool(EFI_LOADER_DATA, kb_layout_size,
+					      (void **)&kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("AllocatePool failed\n");
+			goto out;
+		}
+		ret = hii_database_protocol->get_keyboard_layout(hii_database_protocol,
+				kb_layout_guid, &kb_layout_size, kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("get_keyboard_layout returned %u\n",
+				     (unsigned int)ret);
+			goto out;
+		}
+		ret = boottime->free_pool(kb_layout);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("FreePool failed\n");
+			goto out;
+		}
 	}
 
 	/* current */
diff --git a/lib/efi_selftest/efi_selftest_hii_data.c b/lib/efi_selftest/efi_selftest_hii_data.c
index 5fc890112b4..2d29bb045d3 100644
--- a/lib/efi_selftest/efi_selftest_hii_data.c
+++ b/lib/efi_selftest/efi_selftest_hii_data.c
@@ -428,6 +428,18 @@ static efi_guid_t kb_layout_guid11 =
 	EFI_GUID(0x8d40e495, 0xe2aa, 0x4c6f,
 		 0x89, 0x70, 0x68, 0x85, 0x09, 0xee, 0xc7, 0xd2);
 
+static efi_guid_t kb_layout_guid12 =
+	EFI_GUID(0x2ae60b3e, 0xb9d6, 0x49d8,
+		 0x9a, 0x16, 0xc2, 0x48, 0xf1, 0xeb, 0xa8, 0xdb);
+
+static efi_guid_t kb_layout_guid21 =
+	EFI_GUID(0xe0f56a1f, 0xdf6b, 0x4a7e,
+		 0xa3, 0x9a, 0xe7, 0xa5, 0x19, 0x15, 0x45, 0xd6);
+
+static efi_guid_t kb_layout_guid22 =
+	EFI_GUID(0x47be6ac9, 0x54cc, 0x46f9,
+		 0xa2, 0x62, 0xd5, 0x3b, 0x25, 0x6a, 0x0c, 0x34);
+
 static efi_guid_t package_guid =
 	EFI_GUID(0x0387c95a, 0xd703, 0x2346,
 		 0xb2, 0xab, 0xd0, 0xc7, 0xdd, 0x90, 0x44, 0xf8);

-- 
2.53.0

Re: [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation

[Heinrich Schuchardt]

Am 12. Mai 2026 19:40:32 MESZ schrieb "Vincent Stehlé" <vincent.stehle@arm.com>:
>- Fix one scaled pointer arithmetic bug in EFI Loader HII implementation.
>- Enhance the existing EFI HII unit test to catch the bug.
>
>The unit test can run on the sandbox with the following command:
>
>  ./u-boot -T -c "setenv efi_selftest HII database protocols; \
>		  bootefi selftest"
>
>Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
>---
>Vincent Stehlé (2):
>      efi_loader: fix hii keyboard layout pointer computation
>      efi_selftest: test hii keyboard layouts more
>
> lib/efi_loader/efi_hii.c                 |  3 +-
> lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
> lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
> 3 files changed, 51 insertions(+), 31 deletions(-)
>---
>base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5
>change-id: 20260512-layout-pointer-29d1cd52c070
>
>Best regards,

Hello Vincent,

Off the list:

Did you find any real use for HII protocols in U-Boot except for running the SCT?

Are HII based EFI applications a thing?

Best regards

Heinrich

Re: [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation

[Vincent Stehlé]

On Tue, May 12, 2026 at 09:05:29PM +0200, Heinrich Schuchardt wrote:
> Am 12. Mai 2026 19:40:32 MESZ schrieb "Vincent Stehlé" <vincent.stehle@arm.com>:
> >- Fix one scaled pointer arithmetic bug in EFI Loader HII implementation.
> >- Enhance the existing EFI HII unit test to catch the bug.
> >
> >The unit test can run on the sandbox with the following command:
> >
> >  ./u-boot -T -c "setenv efi_selftest HII database protocols; \
> >		  bootefi selftest"
> >
> >Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
> >---
> >Vincent Stehlé (2):
> >      efi_loader: fix hii keyboard layout pointer computation
> >      efi_selftest: test hii keyboard layouts more
> >
> > lib/efi_loader/efi_hii.c                 |  3 +-
> > lib/efi_selftest/efi_selftest_hii.c      | 67 ++++++++++++++++++--------------
> > lib/efi_selftest/efi_selftest_hii_data.c | 12 ++++++
> > 3 files changed, 51 insertions(+), 31 deletions(-)
> >---
> >base-commit: 5732bd0f457b4c671e46574d64d4acb099c0f0a5
> >change-id: 20260512-layout-pointer-29d1cd52c070
> >
> >Best regards,
> 
> Hello Vincent,
> 
> Off the list:
> 
> Did you find any real use for HII protocols in U-Boot except for running the SCT?
> 
> Are HII based EFI applications a thing?

Hi Heinrich,

A fair question indeed, as the HII protocols do not seem to be used that much in
practice.

The UEFI Shell seems to be using them, though, which should be a sufficient
reason to support them, I think.

Those protocols are used as well a little in systemd-boot and iPXE for example,
but as far as I can tell, they are not used at all in Shim, GRUB, Linux, Xen,
the BSDs or Android GBL.

Best regards,
Vincent.

> 
> Best regards
> 
> Heinrich

Re: [PATCH 0/2] efi: fix and test hii keyboard layout pointer computation

[Michael Brown]

On 18/05/2026 10:48, Vincent Stehlé wrote:
> On Tue, May 12, 2026 at 09:05:29PM +0200, Heinrich Schuchardt wrote:
>> Did you find any real use for HII protocols in U-Boot except for running the SCT?
>>
>> Are HII based EFI applications a thing?
> 
> Hi Heinrich,
> 
> A fair question indeed, as the HII protocols do not seem to be used that much in
> practice.
> 
> The UEFI Shell seems to be using them, though, which should be a sufficient
> reason to support them, I think.
> 
> Those protocols are used as well a little in systemd-boot and iPXE for example,
> but as far as I can tell, they are not used at all in Shim, GRUB, Linux, Xen,
> the BSDs or Android GBL.

For the record: iPXE has zero interest in almost all of the HII 
protocols.  We provide an HII interface to allow external utilities to 
read and modify iPXE settings, but I don't believe anything actually 
uses this interface.  (There are some comments about the HII config 
protocols at 
https://github.com/ipxe/ipxe/blob/ca85200809eb0bf5571869264cb2bf7f0548180a/src/interface/efi/efi_snp_hii.c#L30-L49)

The only use case that iPXE itself has for any part of HII is the 
EFI_HII_FONT_PROTOCOL, which we use to obtain font glyphs in the cases 
when we are drawing our own framebuffer console instead of using ConOut.

Thanks,

Michael