On 24.04.25 22:19, Ying-Chun Liu (PaulLiu) wrote:
From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
This commit adds the functionality of generate EFI_DEBUG_IMAGE_INFO
while loading the image.
This feature is described in UEFI Spec 2.10. Section 18.4.3.
Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>
Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
---
V2: use Kconfig options to turn on/off this feature.
---
include/efi_api.h | 2 +
include/efi_loader.h | 5 ++
lib/efi_loader/efi_boottime.c | 140 ++++++++++++++++++++++++++++++++++
3 files changed, 147 insertions(+)
diff --git a/include/efi_api.h b/include/efi_api.h
index 0f4245091f1..ba87af17c1f 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -571,6 +571,8 @@ struct efi_loaded_image {
#define EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS 0x01
#define EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED 0x02
+#define EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL 0x01
+
struct efi_debug_image_info_normal {
u32 image_info_type;
struct efi_loaded_image *loaded_image_protocol_instance;
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 370bc042f70..babe23a8a83 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -617,6 +617,11 @@ efi_status_t efi_root_node_register(void);
efi_status_t efi_initialize_system_table(void);
/* Called by bootefi to initialize debug */
efi_status_t efi_initialize_system_table_pointer(void);
+/* Called by efi_load_image for register debug info */
+void efi_core_new_debug_image_info_entry(u32 image_info_type,
+ struct efi_loaded_image *loaded_image,
+ efi_handle_t image_handle);
+void efi_core_remove_debug_image_info_entry(efi_handle_t image_handle);
/* efi_runtime_detach() - detach unimplemented runtime functions */
void efi_runtime_detach(void);
/* efi_convert_pointer() - convert pointer to virtual address */
diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
index e2665459e44..0d4df5ee6ae 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2130,6 +2130,14 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
*image_handle = NULL;
free(info);
}
+
+ if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
+ if (*image_handle) {
+
efi_core_new_debug_image_info_entry(EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL,
+ info,
+ *image_handle);
+ }
+ }
error:
return EFI_EXIT(ret);
}
@@ -3359,6 +3367,9 @@ efi_status_t EFIAPI efi_unload_image(efi_handle_t
image_handle)
ret = EFI_INVALID_PARAMETER;
goto out;
}
+ if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
+ efi_core_remove_debug_image_info_entry(image_handle);
+ }
switch (efiobj->type) {
case EFI_OBJECT_TYPE_STARTED_IMAGE:
/* Call the unload function */
@@ -4045,6 +4056,10 @@ efi_m_debug_info_table_header = {
NULL
};
Please, describe this field.
+static u32 efi_m_max_table_entries;
+
+#define EFI_DEBUG_TABLE_ENTRY_SIZE (sizeof(void *))
Wouldn't it be more readable to use sizeof(union efi_debug_image_info *)
instead of this constant?
+
const efi_guid_t efi_debug_image_info_table_guid =
EFI_DEBUG_IMAGE_INFO_TABLE_GUID;
@@ -4095,3 +4110,128 @@ efi_status_t efi_initialize_system_table_pointer(void)
return ret;
}
+
+/**
+ * Add a new efi_loaded_image structure to the efi_debug_image_info Table.
+ * Re-Allocates the table if it's not large enough to accomidate another
+ * entry.
+ *
+ * @param image_info_type type of debug image information
+ * @param loaded_image pointer to the loaded image protocol for the image
+ * being loaded
+ * @param image_handle image handle for the image being loaded
+ **/
+void efi_core_new_debug_image_info_entry(u32 image_info_type,
+ struct efi_loaded_image *loaded_image,
+ efi_handle_t image_handle)
Reallocation can fail. The return value type void seems not to be adequate.
+{
+ union efi_debug_image_info *table;
+ union efi_debug_image_info *new_table;
+ u32 index;
+ u32 table_size;
+
+ /* Set the flag indicating that we're in the process of updating
+ * the table.
+ */
+ efi_m_debug_info_table_header.update_status |=
+ EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+
+ table = efi_m_debug_info_table_header.efi_debug_image_info_table;
+
+ if (efi_m_debug_info_table_header.table_size < efi_m_max_table_entries)
{
+ /* We still have empty entries in the Table, find the first
+ * empty entry.
+ */
+ index = 0;
+ while (table[index].normal_image)
+ index++;
+ } else {
+ /* table is full, re-allocate another page for a larger
"Re-allocate the buffer increasing the size by 4 KiB" would be clearer.
+ * table.
+ */
+ table_size = efi_m_max_table_entries *
EFI_DEBUG_TABLE_ENTRY_SIZE;
+ efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA,
+ table_size + EFI_PAGE_SIZE,
+ (void **)&new_table);
+
+ if (!new_table) {
+ efi_m_debug_info_table_header.update_status &=
+ ~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+ return;
+ }
+
+ memset(new_table, 0, table_size + EFI_PAGE_SIZE);
Please, carve out an efi_realloc() function and place it in
lib/efi_loader/efi_memory.c beside efi_alloc().
+
+ /* Copy the old table into the new one. */
+ memcpy(new_table, table, table_size);
+ /* Free the old table. */
+ efi_free_pool(table);
+ /* Update the table header. */
+ table = new_table;
+ efi_m_debug_info_table_header.efi_debug_image_info_table =
+ new_table;
+
+ /* Enlarge the max table entries and set the first empty
+ * entry index to be the original max table entries.
+ */
+ index = efi_m_max_table_entries;
+ efi_m_max_table_entries +=
+ EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE;
+ }
+
+ /* Allocate data for new entry. */
+ efi_allocate_pool(EFI_RUNTIME_SERVICES_DATA,
+ sizeof(struct efi_debug_image_info_normal),
+ (void **)(&table[index].normal_image));
efi_allocate_pool() will consume a whole page. Shouldn't we have an
array of consecutive structures and use realloc_pool() to enlarge it if
we load too many images?
+ if (table[index].normal_image) {
+ /* Update the entry. */
+ table[index].normal_image->image_info_type = image_info_type;
+ table[index].normal_image->loaded_image_protocol_instance =
+ loaded_image;
All protocols become invalid after ExitBootServices(). Why would you use
EFI_RUNTIME_SERVICES_DATA to store references to something that does not
exist at runtime?
EDK II uses AllocateZeroPool() which does not use EfiRuntimeServicesData.
Best regards
Heinrich
+ table[index].normal_image->image_handle = image_handle;
+
+ /* Increase the number of EFI_DEBUG_IMAGE_INFO elements and
+ * set the efi_m_debug_info_table_header in modified status.
+ */
+ efi_m_debug_info_table_header.table_size++;
+ efi_m_debug_info_table_header.update_status |=
+ EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED;
+ }
+
+ efi_m_debug_info_table_header.update_status &=
+ ~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+}
+
+void efi_core_remove_debug_image_info_entry(efi_handle_t image_handle)
+{
+ union efi_debug_image_info *table;
+ u32 index;
+
+ efi_m_debug_info_table_header.update_status |=
+ EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+
+ table = efi_m_debug_info_table_header.efi_debug_image_info_table;
+
+ for (index = 0; index < efi_m_max_table_entries; index++) {
+ if (table[index].normal_image &&
+ table[index].normal_image->image_handle == image_handle) {
+ /* Found a match. Free up the record, then NULL the
+ * pointer to indicate the slot is free.
+ */
+ efi_free_pool(table[index].normal_image);
+ table[index].normal_image = NULL;
+
+ /* Decrease the number of EFI_DEBUG_IMAGE_INFO
+ * elements and set the efi_m_debug_info_table_header
+ * in modified status.
+ */
+ efi_m_debug_info_table_header.table_size--;
+ efi_m_debug_info_table_header.update_status |=
+ EFI_DEBUG_IMAGE_INFO_TABLE_MODIFIED;
+ break;
+ }
+ }
+
+ efi_m_debug_info_table_header.update_status &=
+ ~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+}