[PATCH v8 1/5] efi: add EFI_SYSTEM_TABLE_POINTER for debug
Paul Liu
paul.liu at linaro.org
Fri Jul 4 13:59:53 CEST 2025
Hi Heinrich, Ilias,
Sorry for the late reply. I'm a bit worried about this change. This
structure (efi_system_table_pointer), is not packed.
So its actual size is a bit larger. The crc32 field is not at the end of
the structure. Actually there are 4 bytes padding after it. But the CRC32
function is calculating the crc sum on the whole structure. So it might
produce random value if we don't zero those padding bytes.
Yours,
Paul
On Thu, 3 Jul 2025 at 08:25, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
> On 03.07.25 08:28, Ying-Chun Liu (PaulLiu) wrote:
> > From: "Ying-Chun Liu (PaulLiu)" <paul.liu at 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.liu at linaro.org>
> > Reviewed-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
> > Tested-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
> > Cc: Peter Robinson <pbrobinson at gmail.com>
> > Cc: Simon Glass <sjg at chromium.org>
> > ---
> > V2: add Kconfig options to turn on/off this feature.
> > V3: make efi_initialize_system_table_pointer() do the job as described.
> > V4: use efi_alloc_aligned_pages() for system_table_pointer.
> > V5: move the code into a separate module.
> > V6: Use macros to replace size_4MB
> > V7: Fix the code based on the review. Update license field.
> > V8: Fix pointer casting that causes qemu_arm_defconfig failed. Modify
> KConfig
> > so that more defconfigs enabled by default.
> > ---
> > include/efi_api.h | 16 ++++++++++++
> > include/efi_loader.h | 2 ++
> > lib/efi_loader/Kconfig | 9 +++++++
> > lib/efi_loader/Makefile | 1 +
> > lib/efi_loader/efi_debug_support.c | 40 ++++++++++++++++++++++++++++++
> > lib/efi_loader/efi_setup.c | 7 ++++++
> > 6 files changed, 75 insertions(+)
> > create mode 100644 lib/efi_loader/efi_debug_support.c
> >
> > 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..621db8b5c79 100644
> > --- a/lib/efi_loader/Kconfig
> > +++ b/lib/efi_loader/Kconfig
> > @@ -71,6 +71,15 @@ config EFI_SECURE_BOOT
> > config EFI_SIGNATURE_SUPPORT
> > bool
> >
> > +config EFI_DEBUG_SUPPORT
> > + bool "EFI Debug Support"
> > + default y if !HAS_BOARD_SIZE_LIMIT
> > + help
> > + Select this option if you want to setup the EFI Debug Support
> > + Table and the EFI_SYSTEM_TABLE_POINTER 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/Makefile b/lib/efi_loader/Makefile
> > index cf050e5385d..51ccf1cd87e 100644
> > --- a/lib/efi_loader/Makefile
> > +++ b/lib/efi_loader/Makefile
> > @@ -70,6 +70,7 @@ obj-$(CONFIG_EFI_RISCV_BOOT_PROTOCOL) += efi_riscv.o
> > obj-$(CONFIG_EFI_LOAD_FILE2_INITRD) += efi_load_initrd.o
> > obj-$(CONFIG_EFI_SIGNATURE_SUPPORT) += efi_signature.o
> > obj-$(CONFIG_EFI_ECPT) += efi_conformance.o
> > +obj-$(CONFIG_EFI_DEBUG_SUPPORT) += efi_debug_support.o
> >
> > EFI_VAR_SEED_FILE := $(subst $\",,$(CONFIG_EFI_VAR_SEED_FILE))
> > $(obj)/efi_var_seed.o: $(srctree)/$(EFI_VAR_SEED_FILE)
> > diff --git a/lib/efi_loader/efi_debug_support.c
> b/lib/efi_loader/efi_debug_support.c
> > new file mode 100644
> > index 00000000000..7dc5fa93af9
> > --- /dev/null
> > +++ b/lib/efi_loader/efi_debug_support.c
> > @@ -0,0 +1,40 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + * EFI debug support
> > + *
> > + * Copyright (c) 2025 Ying-Chun Liu, Linaro Ltd. <paul.liu at linaro.org>
> > + */
> > +
> > +#include <efi_loader.h>
> > +#include <linux/sizes.h>
> > +#include <u-boot/crc.h>
> > +
> > +struct efi_system_table_pointer __efi_runtime_data * systab_pointer =
> NULL;
> > +
> > +/**
> > + * efi_initialize_system_table_pointer() - Initialize system table
> pointer
> > + *
> > + * Return: status code
> > + */
> > +efi_status_t efi_initialize_system_table_pointer(void)
> > +{
> > + /* Allocate efi_system_table_pointer structure with 4MB alignment.
> */
> > + systab_pointer = efi_alloc_aligned_pages(sizeof(struct
> efi_system_table_pointer),
> > + EFI_RUNTIME_SERVICES_DATA,
> > + SZ_4M);
> > +
> > + if (!systab_pointer) {
> > + log_err("Installing EFI system table pointer failed\n");
> > + return EFI_OUT_OF_RESOURCES;
> > + }
> > +
> > + memset(systab_pointer, 0, sizeof(struct efi_system_table_pointer));
>
> systab_pointer->crc32 = 0;
>
> would have been an alternative. Maybe Ilias wants to change that when
> merging.
>
> Reviewed-by: Heinrich Schuchardt <xypron.glpk at gmx.de>
>
> > +
> > + systab_pointer->signature = EFI_SYSTEM_TABLE_SIGNATURE;
> > + systab_pointer->efi_system_table_base = (uintptr_t)&systab;
> > + systab_pointer->crc32 = crc32(0,
> > + (const unsigned char
> *)systab_pointer,
> > + sizeof(struct
> efi_system_table_pointer));
> > +
> > + return EFI_SUCCESS;
> > +}
> > diff --git a/lib/efi_loader/efi_setup.c b/lib/efi_loader/efi_setup.c
> > index 48f91da5df7..78c5059256a 100644
> > --- a/lib/efi_loader/efi_setup.c
> > +++ b/lib/efi_loader/efi_setup.c
> > @@ -278,6 +278,13 @@ 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)) {
> > + 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)
>
>
More information about the U-Boot
mailing list