Am 22. Mai 2025 03:17:45 MESZ schrieb "Ying-Chun Liu (PaulLiu)" 
<paul...@debian.org>:
>From: "Ying-Chun Liu (PaulLiu)" <paul....@linaro.org>
>
>Add EFI_SYSTEM_TABLE_POINTER structure for remote debugger to locate
>the address of EFI_SYSTEM_TABLE.
>
>This feature is described in UEFI SPEC version 2.10. Section 18.4.2.
>The implementation ensures support for hardware-assisted debugging and
>provides a standardized mechanism for debuggers to discover the EFI
>system table.
>
>Signed-off-by: Ying-Chun Liu (PaulLiu) <paul....@linaro.org>
>Cc: Heinrich Schuchardt <xypron.g...@gmx.de>
>Cc: Ilias Apalodimas <ilias.apalodi...@linaro.org>
>Cc: Peter Robinson <pbrobin...@gmail.com>
>---
>V2: add Kconfig options to turn on/off this feature.
>V3: make efi_initialize_system_table_pointer() do the job as described.
>---
> include/efi_api.h             | 16 ++++++++++++
> include/efi_loader.h          |  2 ++
> lib/efi_loader/Kconfig        |  7 ++++++
> lib/efi_loader/efi_boottime.c | 46 +++++++++++++++++++++++++++++++++++
> lib/efi_loader/efi_setup.c    |  8 ++++++
> 5 files changed, 79 insertions(+)
>
>diff --git a/include/efi_api.h b/include/efi_api.h
>index eb61eafa028..6c4c1a0cc7b 100644
>--- a/include/efi_api.h
>+++ b/include/efi_api.h
>@@ -259,6 +259,22 @@ struct efi_capsule_result_variable_header {
>       efi_status_t capsule_status;
> } __packed;
> 
>+/**
>+ * struct efi_system_table_pointer - struct to store the pointer of system
>+ * table.
>+ * @signature: The signature of this struct.
>+ * @efi_system_table_base: The physical address of System Table.
>+ * @crc32: CRC32 checksum
>+ *
>+ * This struct is design for hardware debugger to search through memory to
>+ * get the address of EFI System Table.
>+ */
>+struct efi_system_table_pointer {
>+      u64 signature;
>+      efi_physical_addr_t efi_system_table_base;
>+      u32 crc32;
>+};
>+
> struct efi_memory_range {
>       efi_physical_addr_t     address;
>       u64                     length;
>diff --git a/include/efi_loader.h b/include/efi_loader.h
>index 8f9f2bcf1cb..8f065244d8e 100644
>--- a/include/efi_loader.h
>+++ b/include/efi_loader.h
>@@ -646,6 +646,8 @@ efi_status_t efi_tcg2_measure_dtb(void *dtb);
> efi_status_t efi_root_node_register(void);
> /* Called by bootefi to initialize runtime */
> efi_status_t efi_initialize_system_table(void);
>+/* Called by bootefi to initialize debug */
>+efi_status_t efi_initialize_system_table_pointer(void);
> /* 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/Kconfig b/lib/efi_loader/Kconfig
>index 7f02a83e2a2..d878fc9021d 100644
>--- a/lib/efi_loader/Kconfig
>+++ b/lib/efi_loader/Kconfig
>@@ -71,6 +71,13 @@ config EFI_SECURE_BOOT
> config EFI_SIGNATURE_SUPPORT
>       bool
> 
>+config EFI_DEBUG_SUPPORT_TABLE
>+      bool "EFI Debug Support Table"
>+      help
>+        Select this option if you want to setup the EFI Debug Support
>+        Table which is used by the debug agent or an external debugger to
>+        determine loaded image information in a quiescent manner.
>+
> menu "UEFI services"
> 
> config EFI_GET_TIME
>diff --git a/lib/efi_loader/efi_boottime.c b/lib/efi_loader/efi_boottime.c
>index dbebb37dc04..5a4349f8679 100644
>--- a/lib/efi_loader/efi_boottime.c
>+++ b/lib/efi_loader/efi_boottime.c
>@@ -4001,6 +4001,8 @@ struct efi_system_table __efi_runtime_data systab = {
>       .tables = NULL,
> };
> 
>+struct efi_system_table_pointer __efi_runtime_data * systab_pointer = NULL;
>+
> /**
>  * efi_initialize_system_table() - Initialize system table
>  *
>@@ -4035,3 +4037,47 @@ efi_status_t efi_initialize_system_table(void)
> 
>       return ret;
> }
>+
>+/**
>+ * efi_initialize_system_table() - Initialize system table pointer
>+ *
>+ * Return:    status code
>+ */
>+efi_status_t efi_initialize_system_table_pointer(void)
>+{
>+      efi_status_t ret;
>+      const int size_4MB = 0x00400000;
>+      int pages = efi_size_in_pages(sizeof(struct efi_system_table_pointer));
>+      int alignment_mask = size_4MB - 1;
>+      int real_pages = pages + efi_size_in_pages(size_4MB);
>+      u64 addr;
>+      u64 aligned_addr;
>+      u32 crc32_value;
>+
>+      /* Allocate configuration table array */
>+      ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
>+                               EFI_RUNTIME_SERVICES_DATA,
>+                               real_pages,
>+                               &addr);

You are allocating 8 MiB for a table that isn't even 1 KiB in size.

Can't we use an LMB allocation function instead and add the page to the EFI 
memory table afterwards?

Only for the EFI app and the sandbox physical and virtual addresses differ.

Best regards 

Heinrich



>+
>+      if (ret != EFI_SUCCESS) {
>+              log_err("Installing EFI system table pointer failed\n");
>+              return ret;
>+      }
>+
>+      /* alignment to 4MB boundary */
>+      aligned_addr = (addr + alignment_mask) & ~alignment_mask;
>+
>+      systab_pointer = (struct efi_system_table_pointer *)aligned_addr;
>+      memset(systab_pointer, 0, sizeof(struct efi_system_table_pointer));
>+
>+      systab_pointer->signature = EFI_SYSTEM_TABLE_SIGNATURE;
>+      systab_pointer->efi_system_table_base = (efi_physical_addr_t)&systab;
>+      systab_pointer->crc32 = 0;
>+      crc32_value = crc32(0,
>+                          (const unsigned char *)systab_pointer,
>+                          sizeof(struct efi_system_table_pointer));
>+      systab_pointer->crc32 = crc32_value;
>+
>+      return ret;
>+}
>diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
>index 48f91da5df7..a6b37d994f6 100644
>--- a/lib/efi_loader/efi_setup.c
>+++ b/lib/efi_loader/efi_setup.c
>@@ -278,6 +278,14 @@ efi_status_t efi_init_obj_list(void)
>       if (ret != EFI_SUCCESS)
>               goto out;
> 
>+      /* Initialize system table pointer */
>+      if (IS_ENABLED(CONFIG_EFI_DEBUG_SUPPORT_TABLE)) {
>+              ret = efi_initialize_system_table_pointer();
>+              if (ret != EFI_SUCCESS) {
>+                      goto out;
>+              }
>+      }
>+
>       if (IS_ENABLED(CONFIG_EFI_ECPT)) {
>               ret = efi_ecpt_register();
>               if (ret != EFI_SUCCESS)

Reply via email to