[PATCH 0/2] efi: fix block io revision and test

[PATCH 0/2] efi: fix block io revision and test

[Vincent Stehlé]

Make sure we set the Revision field of our EFI_BLOCK_IO_PROTOCOL structures
to a valid value and add a unit test to verify that.

This can be tested on the sandbox with the following command:

  ./u-boot -T -c 'setenv efi_selftest block device; bootefi selftest'

Signed-off-by: Vincent Stehlé <vincent.stehle@arm.com>
---
Vincent Stehlé (2):
      lib/efi_loader: fix block io revision
      lib/efi_selftest: test block io revision

 lib/efi_loader/efi_disk.c                    |  1 +
 lib/efi_selftest/efi_selftest_block_device.c | 50 ++++++++++++++++++++++++++++
 2 files changed, 51 insertions(+)
---
base-commit: 1a8b7ad50aed5f5f0a4ccbfd455233919c097293
change-id: 20260608-rev-772dedbb2fda

Best regards,
-- 
Vincent.

[PATCH 1/2] lib/efi_loader: fix block io revision

[Vincent Stehlé]

The Revision field of the EFI_BLOCK_IO_PROTOCOL structure must be set to
one of the two valid values [1], but this is not initialized in the
efi_loader; fix it.

Link: https://uefi.org/specs/UEFI/2.11/13_Protocols_Media_Access.html#efi-block-io-protocol [1]
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_loader/efi_disk.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
index f8a57539ec6..4a3ace3a304 100644
--- a/lib/efi_loader/efi_disk.c
+++ b/lib/efi_loader/efi_disk.c
@@ -305,6 +305,7 @@ static efi_status_t EFIAPI efi_disk_flush_blocks(struct efi_block_io *this)
 }
 
 static const struct efi_block_io block_io_disk_template = {
+	.revision = EFI_BLOCK_IO_PROTOCOL_REVISION3,
 	.reset = &efi_disk_reset,
 	.read_blocks = &efi_disk_read_blocks,
 	.write_blocks = &efi_disk_write_blocks,

-- 
2.53.0

[PATCH 2/2] lib/efi_selftest: test block io revision

[Vincent Stehlé]

Enhance the unit test to verify all Revision fields of all the
EFI_BLOCK_IO_PROTOCOL structures.
As the unit test registers its own block io protocol for test purposes,
make sure to initialize its revision properly, as it will be verified as
well.

This can run on the sandbox with the following command:

  ./u-boot -T -c 'setenv efi_selftest block device; bootefi selftest'

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_block_device.c | 50 ++++++++++++++++++++++++++++
 1 file changed, 50 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
index 9c4be834eeb..6fade3e9159 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -170,6 +170,7 @@ static efi_status_t decompress(u8 **image)
 static struct efi_block_io_media media;
 
 static struct efi_block_io block_io = {
+	.revision = EFI_BLOCK_IO_PROTOCOL_REVISION3,
 	.media = &media,
 	.reset = reset,
 	.read_blocks = read_blocks,
@@ -603,6 +604,55 @@ static int execute(void)
 		return EFI_ST_FAILURE;
 	}
 
+	/* Get all handles with block io. */
+	ret = boottime->locate_handle_buffer(BY_PROTOCOL,
+					     &block_io_protocol_guid, NULL,
+					     &no_handles, &handles);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to locate block io handles\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Verify all handles with block io.
+	 * If an error is encountered, the loop exits early instead of
+	 * returning, to free the handles buffer.
+	 */
+	for (i = 0; i < no_handles; ++i) {
+		u64 rev;
+
+		ret = boottime->open_protocol(handles[i],
+					      &block_io_protocol_guid,
+					      (void *)&block_io_protocol,
+					      NULL, NULL,
+					      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to open block io protocol %d\n",
+				     (unsigned int)i);
+			break;
+		}
+
+		/* Verify block io revision. */
+		rev = block_io_protocol->revision;
+		if (rev != EFI_BLOCK_IO_PROTOCOL_REVISION2 &&
+		    rev != EFI_BLOCK_IO_PROTOCOL_REVISION3) {
+			efi_st_error("Bad block io revision %d\n",
+				     (unsigned int)rev);
+			break;
+		}
+	}
+
+	/* Free handles buffer. */
+	ret = boottime->free_pool(handles);
+	if (ret != EFI_SUCCESS) {
+		efi_st_error("Failed to free block io handles\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/* If we exited the loop on block io handles early this is a failure. */
+	if (i != no_handles)
+		return EFI_ST_FAILURE;
+
 	return EFI_ST_SUCCESS;
 }
 

-- 
2.53.0

Re: [PATCH 1/2] lib/efi_loader: fix block io revision

[Heinrich Schuchardt]

On 6/9/26 10:07, Vincent Stehlé wrote:
> The Revision field of the EFI_BLOCK_IO_PROTOCOL structure must be set to
> one of the two valid values [1], but this is not initialized in the
> efi_loader; fix it.
> 
> Link: https://uefi.org/specs/UEFI/2.11/13_Protocols_Media_Access.html#efi-block-io-protocol [1]
> 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>

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

> ---
>   lib/efi_loader/efi_disk.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> index f8a57539ec6..4a3ace3a304 100644
> --- a/lib/efi_loader/efi_disk.c
> +++ b/lib/efi_loader/efi_disk.c
> @@ -305,6 +305,7 @@ static efi_status_t EFIAPI efi_disk_flush_blocks(struct efi_block_io *this)
>   }
>   
>   static const struct efi_block_io block_io_disk_template = {
> +	.revision = EFI_BLOCK_IO_PROTOCOL_REVISION3,
>   	.reset = &efi_disk_reset,
>   	.read_blocks = &efi_disk_read_blocks,
>   	.write_blocks = &efi_disk_write_blocks,
> 

Re: [PATCH 2/2] lib/efi_selftest: test block io revision

[Heinrich Schuchardt]

On 6/9/26 10:07, Vincent Stehlé wrote:
> Enhance the unit test to verify all Revision fields of all the
> EFI_BLOCK_IO_PROTOCOL structures.
> As the unit test registers its own block io protocol for test purposes,
> make sure to initialize its revision properly, as it will be verified as
> well.
> 
> This can run on the sandbox with the following command:
> 
>    ./u-boot -T -c 'setenv efi_selftest block device; bootefi selftest'
> 
> 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_block_device.c | 50 ++++++++++++++++++++++++++++
>   1 file changed, 50 insertions(+)
> 
> diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
> index 9c4be834eeb..6fade3e9159 100644
> --- a/lib/efi_selftest/efi_selftest_block_device.c
> +++ b/lib/efi_selftest/efi_selftest_block_device.c
> @@ -170,6 +170,7 @@ static efi_status_t decompress(u8 **image)
>   static struct efi_block_io_media media;
>   
>   static struct efi_block_io block_io = {
> +	.revision = EFI_BLOCK_IO_PROTOCOL_REVISION3,
>   	.media = &media,
>   	.reset = reset,
>   	.read_blocks = read_blocks,
> @@ -603,6 +604,55 @@ static int execute(void)
>   		return EFI_ST_FAILURE;
>   	}
>   
> +	/* Get all handles with block io. */
> +	ret = boottime->locate_handle_buffer(BY_PROTOCOL,
> +					     &block_io_protocol_guid, NULL,
> +					     &no_handles, &handles);
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("Failed to locate block io handles\n");
> +		return EFI_ST_FAILURE;
> +	}

Thank you for providing this test.

The test should pass when invoking U-Boot without any block device.

EFI_NOT_FOUND should not be reported as failure instead you should leave 
with EFI_ST_SUCCESS.

> +
> +	/*
> +	 * Verify all handles with block io.
> +	 * If an error is encountered, the loop exits early instead of
> +	 * returning, to free the handles buffer.
> +	 */
> +	for (i = 0; i < no_handles; ++i) {
> +		u64 rev;
> +
> +		ret = boottime->open_protocol(handles[i],
> +					      &block_io_protocol_guid,
> +					      (void *)&block_io_protocol,
> +					      NULL, NULL,
> +					      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
> +		if (ret != EFI_SUCCESS) {
> +			efi_st_error("Failed to open block io protocol %d\n",
> +				     (unsigned int)i);
> +			break;
> +		}
> +
> +		/* Verify block io revision. */
> +		rev = block_io_protocol->revision;
> +		if (rev != EFI_BLOCK_IO_PROTOCOL_REVISION2 &&
> +		    rev != EFI_BLOCK_IO_PROTOCOL_REVISION3) {
> +			efi_st_error("Bad block io revision %d\n",

The field is unsigned. So we need %u here.

Best regards

Heinrich

> +				     (unsigned int)rev);
> +			break;
> +		}
> +	}
> +
> +	/* Free handles buffer. */
> +	ret = boottime->free_pool(handles);
> +	if (ret != EFI_SUCCESS) {
> +		efi_st_error("Failed to free block io handles\n");
> +		return EFI_ST_FAILURE;
> +	}
> +
> +	/* If we exited the loop on block io handles early this is a failure. */
> +	if (i != no_handles)
> +		return EFI_ST_FAILURE;
> +
>   	return EFI_ST_SUCCESS;
>   }
>   
> 

Re: [PATCH 2/2] lib/efi_selftest: test block io revision

[Vincent Stehlé]

Hi Heinrich,

Thank you for your review.
I agree with your two remarks and I will send a v2.

Best regards,
Vincent.

[PATCH v2] lib/efi_selftest: test block io revision

[Vincent Stehlé]

Enhance the unit test to verify all Revision fields of all the
EFI_BLOCK_IO_PROTOCOL structures.
As the unit test registers its own block io protocol for test purposes,
make sure to initialize its revision properly, as it will be verified as
well.

This can run on the sandbox with the following command:

  ./u-boot -T -c 'setenv efi_selftest block device; bootefi selftest'

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>
---
Changes for v2:
- Deal gracefully with no block io protocol found.
- Print revision with %u in case of error as it is unsigned.
---
 lib/efi_selftest/efi_selftest_block_device.c | 56 ++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/lib/efi_selftest/efi_selftest_block_device.c b/lib/efi_selftest/efi_selftest_block_device.c
index 9c4be834eeb..d2a1f2dc5d4 100644
--- a/lib/efi_selftest/efi_selftest_block_device.c
+++ b/lib/efi_selftest/efi_selftest_block_device.c
@@ -170,6 +170,7 @@ static efi_status_t decompress(u8 **image)
 static struct efi_block_io_media media;
 
 static struct efi_block_io block_io = {
+	.revision = EFI_BLOCK_IO_PROTOCOL_REVISION3,
 	.media = &media,
 	.reset = reset,
 	.read_blocks = read_blocks,
@@ -603,6 +604,61 @@ static int execute(void)
 		return EFI_ST_FAILURE;
 	}
 
+	/* Get all handles with block io. */
+	ret = boottime->locate_handle_buffer(BY_PROTOCOL,
+					     &block_io_protocol_guid, NULL,
+					     &no_handles, &handles);
+	switch (ret) {
+	case EFI_SUCCESS:
+	case EFI_NOT_FOUND:	/* no_handles == 0, handles == NULL */
+		break;
+	default:
+		efi_st_error("Locate handle buffer failed\n");
+		return EFI_ST_FAILURE;
+	}
+
+	/*
+	 * Verify all handles with block io.
+	 * If an error is encountered, the loop exits early instead of
+	 * returning, to free the handles buffer.
+	 */
+	for (i = 0; i < no_handles; ++i) {
+		u64 rev;
+
+		ret = boottime->open_protocol(handles[i],
+					      &block_io_protocol_guid,
+					      (void *)&block_io_protocol,
+					      NULL, NULL,
+					      EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to open block io protocol %d\n",
+				     (unsigned int)i);
+			break;
+		}
+
+		/* Verify block io revision. */
+		rev = block_io_protocol->revision;
+		if (rev != EFI_BLOCK_IO_PROTOCOL_REVISION2 &&
+		    rev != EFI_BLOCK_IO_PROTOCOL_REVISION3) {
+			efi_st_error("Bad block io revision %u\n",
+				     (unsigned int)rev);
+			break;
+		}
+	}
+
+	/* Free handles buffer. */
+	if (handles) {
+		ret = boottime->free_pool(handles);
+		if (ret != EFI_SUCCESS) {
+			efi_st_error("Failed to free block io handles\n");
+			return EFI_ST_FAILURE;
+		}
+	}
+
+	/* If we exited the loop on block io handles early this is a failure. */
+	if (i != no_handles)
+		return EFI_ST_FAILURE;
+
 	return EFI_ST_SUCCESS;
 }
 

---
base-commit: fcda974e36033ad5331a9b4a4a551af4e141ad7d
change-id: 20260608-rev-772dedbb2fda

Best regards,
-- 
Vincent.