diff mbox series

[v6,4/4] efi: add EFI_DEBUG_IMAGE_INFO for debug

Message ID 20250617120855.87492-5-paulliu@debian.org
State New
Headers show
Series Add EFI Debug Support Table feature | expand

Commit Message

Ying-Chun Liu (PaulLiu) June 17, 2025, 12:08 p.m. UTC
From: "Ying-Chun Liu (PaulLiu)" <paul.liu@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.
The implementation ensures support for hardware-assisted debugging and
provides a standardized mechanism for debuggers to discover the load
address of an EFI application.

Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Peter Robinson <pbrobinson@gmail.com>
Cc: Simon Glass <sjg@chromium.org>
---
V2: use Kconfig options to turn on/off this feature.
V3: Use efi_realloc to realloc the tables. Move tables to boot time.
V4: Use new efi_realloc().
V5: Fix function comments and move the code into a separate module.
V6: refine the code. Handling the error of efi_allocate_pool.
---
 include/efi_api.h                  |   2 +
 include/efi_loader.h               |   5 ++
 lib/efi_loader/efi_boottime.c      |   8 ++
 lib/efi_loader/efi_debug_support.c | 137 +++++++++++++++++++++++++++++
 4 files changed, 152 insertions(+)

Comments

Heinrich Schuchardt June 18, 2025, 7:17 a.m. UTC | #1
On 6/17/25 14:08, Ying-Chun Liu (PaulLiu) wrote:
> From: "Ying-Chun Liu (PaulLiu)" <paul.liu@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.
> The implementation ensures support for hardware-assisted debugging and
> provides a standardized mechanism for debuggers to discover the load
> address of an EFI application.
> 
> Signed-off-by: Ying-Chun Liu (PaulLiu) <paul.liu@linaro.org>
> Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
> Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
> Cc: Peter Robinson <pbrobinson@gmail.com>
> Cc: Simon Glass <sjg@chromium.org>
> ---
> V2: use Kconfig options to turn on/off this feature.
> V3: Use efi_realloc to realloc the tables. Move tables to boot time.
> V4: Use new efi_realloc().
> V5: Fix function comments and move the code into a separate module.
> V6: refine the code. Handling the error of efi_allocate_pool.
> ---
>   include/efi_api.h                  |   2 +
>   include/efi_loader.h               |   5 ++
>   lib/efi_loader/efi_boottime.c      |   8 ++
>   lib/efi_loader/efi_debug_support.c | 137 +++++++++++++++++++++++++++++
>   4 files changed, 152 insertions(+)
> 
> diff --git a/include/efi_api.h b/include/efi_api.h
> index 8da0a350ce3..77a05f752e5 100644
> --- a/include/efi_api.h
> +++ b/include/efi_api.h
> @@ -581,6 +581,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 - Store Debug Information for normal
>    * image.
> diff --git a/include/efi_loader.h b/include/efi_loader.h
> index 13ca2ec9a4e..22440b842e3 100644
> --- a/include/efi_loader.h
> +++ b/include/efi_loader.h
> @@ -650,6 +650,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 */
> +efi_status_t 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 dbebb37dc04..303b7543e16 100644
> --- a/lib/efi_loader/efi_boottime.c
> +++ b/lib/efi_loader/efi_boottime.c
> @@ -2129,6 +2129,12 @@ efi_status_t EFIAPI efi_load_image(bool boot_policy,
>   		*image_handle = NULL;
>   		free(info);
>   	}
> +
> +	if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT) && *image_handle) {
> +		efi_core_new_debug_image_info_entry(EFI_DEBUG_IMAGE_INFO_TYPE_NORMAL,
> +						    info,
> +						    *image_handle);
> +	}

Braces are not needed here.

>   error:
>   	return EFI_EXIT(ret);
>   }
> @@ -3359,6 +3365,8 @@ 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))
> +		efi_core_remove_debug_image_info_entry(image_handle);
>   	switch (efiobj->type) {
>   	case EFI_OBJECT_TYPE_STARTED_IMAGE:
>   		/* Call the unload function */
> diff --git a/lib/efi_loader/efi_debug_support.c b/lib/efi_loader/efi_debug_support.c
> index f2dfcc5e6b2..d8df95abf95 100644
> --- a/lib/efi_loader/efi_debug_support.c
> +++ b/lib/efi_loader/efi_debug_support.c
> @@ -17,6 +17,13 @@ struct efi_debug_image_info_table_header efi_m_debug_info_table_header = {
>   	NULL
>   };
>   
> +/* efi_m_max_table_entries is the maximum entries allocated for
> + * the efi_m_debug_info_table_header.efi_debug_image_info_table.
> + */
> +static u32 efi_m_max_table_entries;
> +
> +#define EFI_DEBUG_TABLE_ENTRY_SIZE  (sizeof(union efi_debug_image_info *))
> +
>   /**
>    * efi_initialize_system_table_pointer() - Initialize system table pointer
>    *
> @@ -50,3 +57,133 @@ efi_status_t efi_initialize_system_table_pointer(void)
>   
>   	return EFI_SUCCESS;
>   }
> +
> +/**
> + * efi_core_new_debug_image_info_entry() - Add a new efi_loaded_image structure to the
> + *                                         efi_debug_image_info Table.

Nits:
%s/Table/table/

> + *
> + * @image_info_type: type of debug image information
> + * @loaded_image:    pointer to the loaded image protocol for the image
> + *                   being loaded
> + * @image_handle:    image handle for the image being loaded
> + *
> + * Re-Allocates the table if it's not large enough to accommodate another
> + * entry.
> + *
> + * Return: status code
> + **/
> +efi_status_t efi_core_new_debug_image_info_entry(u32 image_info_type,
> +						 struct efi_loaded_image *loaded_image,
> +						 efi_handle_t image_handle)
> +{
> +	union efi_debug_image_info **table;
> +	u32 index;
> +	u32 table_size;
> +	efi_status_t ret;
> +
> +	/* 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) {
> +		/* table is full, re-allocate the buffer increasing the size
> +		 * by 4 KiB.
> +		 */
> +		table_size = efi_m_max_table_entries * EFI_DEBUG_TABLE_ENTRY_SIZE;
> +
> +		ret = efi_realloc((void **)table, table_size + EFI_PAGE_SIZE);
> +
> +		if (ret != EFI_SUCCESS) {
> +			efi_m_debug_info_table_header.update_status &=
> +				~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
> +			return ret;
> +		}
> +
> +		/* Enlarge the max table entries and set the first empty
> +		 * entry index to be the original max table entries.
> +		 */
> +		efi_m_max_table_entries +=
> +			EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE;
> +	}
> +
> +	/* We always put the next entry at the end of the currently consumed
> +	 * table (i.e. first free entry)
> +	 */
> +	index = efi_m_debug_info_table_header.table_size;
> +
> +	/* Allocate data for new entry. */
> +	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA,
> +				sizeof(union efi_debug_image_info),
> +				(void **)(&(*table)[index].normal_image));
> +	if (ret == EFI_SUCCESS && (*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;
> +		(*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;
> +	} else {
> +		log_err("Adding new efi_debug_image_info failed\n");
> +		return ret;
> +	}
> +
> +	efi_m_debug_info_table_header.update_status &=
> +		~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
> +
> +	return EFI_SUCCESS;
> +}
> +
> +/**
> + * efi_core_remove_debug_image_info_entry() - Remove an efi_debug_image_info entry.
> + *
> + * @image_handle:    image handle for the image being removed
> + **/
> +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 table entry.
> +			 * Move the tail of the table one slot to the front.
> +			 */
> +			efi_free_pool(table[index].normal_image);
> +
> +			memcpy(&table[index],
> +			       &table[index + 1],
> +			       (efi_m_debug_info_table_header.table_size -
> +				index - 1) * EFI_DEBUG_TABLE_ENTRY_SIZE);

You cannot use memcpy() for overlapping memory areas. This is described 
in the memcpy man page.

Use memmove() instead.

Best regards

Heinrich

> +
> +			/* 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--;
> +			table[efi_m_debug_info_table_header.table_size].normal_image =
> +				NULL;
> +			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;
> +}
diff mbox series

Patch

diff --git a/include/efi_api.h b/include/efi_api.h
index 8da0a350ce3..77a05f752e5 100644
--- a/include/efi_api.h
+++ b/include/efi_api.h
@@ -581,6 +581,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 - Store Debug Information for normal
  * image.
diff --git a/include/efi_loader.h b/include/efi_loader.h
index 13ca2ec9a4e..22440b842e3 100644
--- a/include/efi_loader.h
+++ b/include/efi_loader.h
@@ -650,6 +650,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 */
+efi_status_t 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 dbebb37dc04..303b7543e16 100644
--- a/lib/efi_loader/efi_boottime.c
+++ b/lib/efi_loader/efi_boottime.c
@@ -2129,6 +2129,12 @@  efi_status_t EFIAPI efi_load_image(bool boot_policy,
 		*image_handle = NULL;
 		free(info);
 	}
+
+	if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT) && *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 +3365,8 @@  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))
+		efi_core_remove_debug_image_info_entry(image_handle);
 	switch (efiobj->type) {
 	case EFI_OBJECT_TYPE_STARTED_IMAGE:
 		/* Call the unload function */
diff --git a/lib/efi_loader/efi_debug_support.c b/lib/efi_loader/efi_debug_support.c
index f2dfcc5e6b2..d8df95abf95 100644
--- a/lib/efi_loader/efi_debug_support.c
+++ b/lib/efi_loader/efi_debug_support.c
@@ -17,6 +17,13 @@  struct efi_debug_image_info_table_header efi_m_debug_info_table_header = {
 	NULL
 };
 
+/* efi_m_max_table_entries is the maximum entries allocated for
+ * the efi_m_debug_info_table_header.efi_debug_image_info_table.
+ */
+static u32 efi_m_max_table_entries;
+
+#define EFI_DEBUG_TABLE_ENTRY_SIZE  (sizeof(union efi_debug_image_info *))
+
 /**
  * efi_initialize_system_table_pointer() - Initialize system table pointer
  *
@@ -50,3 +57,133 @@  efi_status_t efi_initialize_system_table_pointer(void)
 
 	return EFI_SUCCESS;
 }
+
+/**
+ * efi_core_new_debug_image_info_entry() - Add a new efi_loaded_image structure to the
+ *                                         efi_debug_image_info Table.
+ *
+ * @image_info_type: type of debug image information
+ * @loaded_image:    pointer to the loaded image protocol for the image
+ *                   being loaded
+ * @image_handle:    image handle for the image being loaded
+ *
+ * Re-Allocates the table if it's not large enough to accommodate another
+ * entry.
+ *
+ * Return: status code
+ **/
+efi_status_t efi_core_new_debug_image_info_entry(u32 image_info_type,
+						 struct efi_loaded_image *loaded_image,
+						 efi_handle_t image_handle)
+{
+	union efi_debug_image_info **table;
+	u32 index;
+	u32 table_size;
+	efi_status_t ret;
+
+	/* 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) {
+		/* table is full, re-allocate the buffer increasing the size
+		 * by 4 KiB.
+		 */
+		table_size = efi_m_max_table_entries * EFI_DEBUG_TABLE_ENTRY_SIZE;
+
+		ret = efi_realloc((void **)table, table_size + EFI_PAGE_SIZE);
+
+		if (ret != EFI_SUCCESS) {
+			efi_m_debug_info_table_header.update_status &=
+				~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+			return ret;
+		}
+
+		/* Enlarge the max table entries and set the first empty
+		 * entry index to be the original max table entries.
+		 */
+		efi_m_max_table_entries +=
+			EFI_PAGE_SIZE / EFI_DEBUG_TABLE_ENTRY_SIZE;
+	}
+
+	/* We always put the next entry at the end of the currently consumed
+	 * table (i.e. first free entry)
+	 */
+	index = efi_m_debug_info_table_header.table_size;
+
+	/* Allocate data for new entry. */
+	ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA,
+				sizeof(union efi_debug_image_info),
+				(void **)(&(*table)[index].normal_image));
+	if (ret == EFI_SUCCESS && (*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;
+		(*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;
+	} else {
+		log_err("Adding new efi_debug_image_info failed\n");
+		return ret;
+	}
+
+	efi_m_debug_info_table_header.update_status &=
+		~EFI_DEBUG_IMAGE_INFO_UPDATE_IN_PROGRESS;
+
+	return EFI_SUCCESS;
+}
+
+/**
+ * efi_core_remove_debug_image_info_entry() - Remove an efi_debug_image_info entry.
+ *
+ * @image_handle:    image handle for the image being removed
+ **/
+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 table entry.
+			 * Move the tail of the table one slot to the front.
+			 */
+			efi_free_pool(table[index].normal_image);
+
+			memcpy(&table[index],
+			       &table[index + 1],
+			       (efi_m_debug_info_table_header.table_size -
+				index - 1) * EFI_DEBUG_TABLE_ENTRY_SIZE);
+
+			/* 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--;
+			table[efi_m_debug_info_table_header.table_size].normal_image =
+				NULL;
+			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;
+}