Re: [PATCH v7 5/6] efi: selftest: add selftest for EFI_DEBUG_SUPPORT

[Heinrich Schuchardt]

On 01.07.25 05:49, Ying-Chun Liu (PaulLiu) wrote:
From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
Add selftest to check the installed configuration table that has
the correct GUID.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>
Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
---
V7: add selftest for EFI_DEBUG_SUPPORT
---
  lib/efi_selftest/Makefile                     |  2 +
  lib/efi_selftest/efi_selftest_debug_support.c | 58 +++++++++++++++++++
  2 files changed, 60 insertions(+)
  create mode 100644 lib/efi_selftest/efi_selftest_debug_support.c

diff --git a/lib/efi_selftest/Makefile b/lib/efi_selftest/Makefile
index d78bf7d6191..842433f68aa 100644
--- a/lib/efi_selftest/Makefile
+++ b/lib/efi_selftest/Makefile
@@ -78,6 +78,8 @@ endif
  
  obj-$(CONFIG_EFI_ESRT) += efi_selftest_esrt.o
  
+obj-$(CONFIG_EFI_DEBUG_SUPPORT) += efi_selftest_debug_support.o
+
  targets += \
  efi_miniapp_file_image_exception.h \
  efi_miniapp_file_image_exit.h \
diff --git a/lib/efi_selftest/efi_selftest_debug_support.c 
b/lib/efi_selftest/efi_selftest_debug_support.c
new file mode 100644
index 00000000000..9cfa44c05a2
--- /dev/null
+++ b/lib/efi_selftest/efi_selftest_debug_support.c
@@ -0,0 +1,58 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * efi_selftest_debug_support
+ *
+ * Copyright (c) 2025 Ying-Chun Liu, Linaro Ltd. <paul....@linaro.org>
+ *
+ * Test the EFI_DEBUG_SUPPORT
+ */
+
+#include <efi_loader.h>
+#include <efi_selftest.h>
+
+static const struct efi_system_table *local_systable;
+
+/**
+ * efi_st_debug_support_setup() - setup test
+ *
+ * @handle:    handle of the loaded image
+ * @systable:  system table
+ * Return:     status code
+ */
+static int efi_st_debug_support_setup(const efi_handle_t img_handle,
+                                     const struct efi_system_table *systable)
+{
+       local_systable = systable;
+
+       return EFI_ST_SUCCESS;
+}

Thanks for adding a unit test.

The setup function is not needed. You can access st_systable in the 
execute function. The variable is set here:

lib/efi_selftest/efi_selftest.c:260:    st_systable = systab;

+
+/**
+ * efi_st_debug_support_execute() - execute test
+ *
+ * Test EFI_DEBUG_SUPPORT tables.
+ *
+ * Return:     status code
+ */
+static int efi_st_debug_support_execute(void)
+{
+       struct efi_debug_image_info_table_header 
*efi_st_debug_info_table_header = NULL;
+       efi_guid_t efi_debug_image_info_table_guid = 
EFI_DEBUG_IMAGE_INFO_TABLE_GUID;
+
+       /* find EFI_DEBUG_IMAGE_INFO_TABLE */

Please, use efi_st_get_config_table().

+       for (int idx = 0; idx < local_systable->nr_tables; idx++)
+               if (!guidcmp(&efi_debug_image_info_table_guid, 
&local_systable->tables[idx].guid))
+                       efi_st_debug_info_table_header = 
local_systable->tables[idx].table;
+
+       if (!efi_st_debug_info_table_header)

Please, add an error message, e.g.

efi_st_error("Missing EFI_DEBUG_IMAGE_INFO_TABLE\n");

If we would load EFI binaries like efi_selftest_miniapp_exit.efi here, 
we could check that the table is correctly updated. But that can be 
added later. My testing using dbginfodump.efi showed a reasonable content.

Best regards

Heinrich

+               return EFI_ST_FAILURE;
+
+       return EFI_ST_SUCCESS;
+}
+
+EFI_UNIT_TEST(debug_support) = {
+       .name = "debug_support",
+       .phase = EFI_EXECUTE_BEFORE_BOOTTIME_EXIT,
+       .execute = efi_st_debug_support_execute,
+       .setup = efi_st_debug_support_setup,
+};