On 6/24/26 11:26, 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é <[email protected]>
Cc: Heinrich Schuchardt <[email protected]>
Cc: Ilias Apalodimas <[email protected]>
Cc: Tom Rini <[email protected]>
---
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:
If EFI_SUCCESS and no_handles = 0 or handles = NULL, we have a bug.
+ case EFI_NOT_FOUND: /* no_handles == 0, handles == NULL */
+ break;
Only for EFI_SUCCESS updating the two variables is mandated in the UEFI
specification. Don't expect no_handles == 0 or handles = NULL here.
(I know EDK II and U-Boot currently do it. But here we are testing.)
In my v1 review I misguided you.
EFI_NOT_FOUND is a legal value but needs special treatment here:
In setup() we installed a block IO protocol. The protocol is not
uninstalled before teardown(). Execute() is only called if setup()
succeeded.
If we can't find any block IO protocol here, then something is badly
broken and we should error out with a specific error message, e.g.
"No block IO protocol found though one installed in setup\n"
+ 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;
Should we check that all function pointers are non-NULL, too?
Best regards
Heinrich
+ }
+ }
+
+ /* 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,
