Hi Vincent,
On 2026-09-22T14:58:39, Vincent Stehlé <[email protected]> wrote:
> efi_selftest: add minimal fmp test
>
> Add a minimal test for the UEFI Firmware Management Protocol.
> At this point the test only does the following:
>
> - Get current firmware images info.
> - For each image, verify consistency of the authentication flags.
>
> This can be tested on the sandbox with the following command:
>
> ./u-boot -T -c "setenv efi_selftest firmware management protocol; \
> bootefi selftest"
>
> Signed-off-by: Vincent Stehlé <[email protected]>
> Cc: Tom Rini <[email protected]>
> Cc: Heinrich Schuchardt <[email protected]>
> Cc: Ilias Apalodimas <[email protected]>
>
> lib/efi_selftest/Makefile | 2 +
> lib/efi_selftest/efi_selftest_fmp.c | 135
> ++++++++++++++++++++++++++++++++++++
> 2 files changed, 137 insertions(+)
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c
> b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + /* Verify each image info */
> + for (i = 0; i < desc_cnt; i++) {
> + const struct efi_firmware_image_descriptor *inf = &img_info[i];
The spec says callers must step through the array using
DescriptorSize, since firmware may return a larger (newer) descriptor
than the one we know about. U-Boot happens to use sizeof(), but since
this is a protocol test, please can you use desc_size as the stride,
e.g. (void *)img_info + i * desc_size, and perhaps check that
desc_size >= sizeof(*inf)?
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c
> b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + /* Verify authentication */
> + if (CONFIG_IS_ENABLED(EFI_CAPSULE_AUTHENTICATE) &&
> + (attr_supp & IMAGE_ATTRIBUTE_IMAGE_UPDATABLE) &&
> + !(attr_supp & IMAGE_ATTRIBUTE_AUTHENTICATION_REQUIRED)) {
> + efi_st_error("Bad AttributesSupported %u\n",
> + (unsigned int)attr_supp);
> + goto out;
> + }
efi_firmware.c uses IS_ENABLED(CONFIG_EFI_CAPSULE_AUTHENTICATE), so
please match that. There is no SPL variant of this option, so
CONFIG_IS_ENABLED() just adds confusion.
This only checks one direction. You could also check that any bit set
in AttributesSetting is also set in AttributesSupported, and that
AUTHENTICATION_REQUIRED is not set in AttributesSetting when
authentication is disabled. That would also catch regressions on
boards without CONFIG_EFI_CAPSULE_AUTHENTICATE. What do you think?
> diff --git a/lib/efi_selftest/efi_selftest_fmp.c
> b/lib/efi_selftest/efi_selftest_fmp.c
> @@ -0,0 +1,135 @@
> + ret = boottime->locate_protocol(&firmware_management_protocol_guid,
> + NULL, (void **)&fmp);
This only tests the first FMP instance. With
CONFIG_EFI_CAPSULE_FIRMWARE_FIT and _RAW both enabled on sandbox there
can be two, so using locate_handle_buffer() and checking each one
would make the test more useful as it grows. I'm fine with leaving
that for later though, since the patch says 'minimal'.
Regards,
Simon
