linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/2] efi/arm64: add vmlinux link to PE/COFF debug table
@ 2017-01-25 10:39 Ard Biesheuvel
       [not found] ` <1485340759-28975-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-25 10:39 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
  Cc: Ard Biesheuvel

Much like GNU debug links, PE/COFF binaries can carry the path on the build
host to the binary containing the debugging symbols. Since the kernel binary
is loaded by UEFI at an a priori unknown offset in the 1:1 mapping of physical
memory, having this information is useful for the debugger automation to find
the file and the offset, and load the symbols automatically.

So if we have debugging symbols to begin with (CONFIG_DEBUG_INFO=y), add the
absolute path to vmlinux to the PE/COFF debug table.

v2: rebase onto v4.10-rc
    use strip rather than objcopy for first pass (#1)
    move debug table inside #ifdef CONFIG_EFI region in head.S (#2)

Ard Biesheuvel (2):
  efi: libstub: Preserve .debug sections after absolute relocation check
  arm64: efi: add vmlinux debug link to the Image binary

 arch/arm64/kernel/Makefile            |  4 +++
 arch/arm64/kernel/head.S              | 34 +++++++++++++++++++-
 arch/arm64/kernel/image.h             |  3 ++
 drivers/firmware/efi/libstub/Makefile | 24 +++++++++-----
 4 files changed, 56 insertions(+), 9 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2 1/2] efi: libstub: Preserve .debug sections after absolute relocation check
       [not found] ` <1485340759-28975-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-01-25 10:39   ` Ard Biesheuvel
  2017-01-25 10:39   ` [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary Ard Biesheuvel
  1 sibling, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-25 10:39 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
  Cc: Ard Biesheuvel

The build commands for the ARM and arm64 EFI stubs strip the .debug
sections and other sections that may legally contain absolute relocations,
in order to inspect the remaining sections for the presence of such
relocations.

This leaves us without debugging symbols in the stub for no good reason,
considering that these sections are omitted from the kernel binary anyway,
and that these relocations are thus only consumed by users of the ELF
binary, such as debuggers.

So move to 'strip' for performing the relocation check, and if it succeeds,
invoke objcopy as before, but leaving the .debug sections in place. Note
that these sections may refer to ksymtab/kcrctab contents, so leave those
in place as well.

Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/libstub/Makefile | 24 +++++++++++++-------
 1 file changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/firmware/efi/libstub/Makefile b/drivers/firmware/efi/libstub/Makefile
index d564d25df8ab..33e0e2f1a730 100644
--- a/drivers/firmware/efi/libstub/Makefile
+++ b/drivers/firmware/efi/libstub/Makefile
@@ -11,7 +11,7 @@ cflags-$(CONFIG_X86)		+= -m$(BITS) -D__KERNEL__ -O2 \
 				   -mno-mmx -mno-sse
 
 cflags-$(CONFIG_ARM64)		:= $(subst -pg,,$(KBUILD_CFLAGS))
-cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) -g0 \
+cflags-$(CONFIG_ARM)		:= $(subst -pg,,$(KBUILD_CFLAGS)) \
 				   -fno-builtin -fpic -mno-single-pic-base
 
 cflags-$(CONFIG_EFI_ARMSTUB)	+= -I$(srctree)/scripts/dtc/libfdt
@@ -60,7 +60,7 @@ CFLAGS_arm64-stub.o 		:= -DTEXT_OFFSET=$(TEXT_OFFSET)
 extra-$(CONFIG_EFI_ARMSTUB)	:= $(lib-y)
 lib-$(CONFIG_EFI_ARMSTUB)	:= $(patsubst %.o,%.stub.o,$(lib-y))
 
-STUBCOPY_FLAGS-y		:= -R .debug* -R *ksymtab* -R *kcrctab*
+STUBCOPY_RM-y			:= -R *ksymtab* -R *kcrctab*
 STUBCOPY_FLAGS-$(CONFIG_ARM64)	+= --prefix-alloc-sections=.init \
 				   --prefix-symbols=__efistub_
 STUBCOPY_RELOC-$(CONFIG_ARM64)	:= R_AARCH64_ABS
@@ -68,17 +68,25 @@ STUBCOPY_RELOC-$(CONFIG_ARM64)	:= R_AARCH64_ABS
 $(obj)/%.stub.o: $(obj)/%.o FORCE
 	$(call if_changed,stubcopy)
 
+#
+# Strip debug sections and some other sections that may legally contain
+# absolute relocations, so that we can inspect the remaining sections for
+# such relocations. If none are found, regenerate the output object, but
+# this time, use objcopy and leave all sections in place.
+#
 quiet_cmd_stubcopy = STUBCPY $@
-      cmd_stubcopy = if $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; then	\
-		     $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y)	\
-		     && (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \
-			 rm -f $@; /bin/false); else /bin/false; fi
+      cmd_stubcopy = if $(STRIP) --strip-debug $(STUBCOPY_RM-y) -o $@ $<; \
+		     then if $(OBJDUMP) -r $@ | grep $(STUBCOPY_RELOC-y); \
+		     then (echo >&2 "$@: absolute symbol references not allowed in the EFI stub"; \
+			   rm -f $@; /bin/false); 			  \
+		     else $(OBJCOPY) $(STUBCOPY_FLAGS-y) $< $@; fi	  \
+		     else /bin/false; fi
 
 #
 # ARM discards the .data section because it disallows r/w data in the
 # decompressor. So move our .data to .data.efistub, which is preserved
 # explicitly by the decompressor linker script.
 #
-STUBCOPY_FLAGS-$(CONFIG_ARM)	+= --rename-section .data=.data.efistub \
-				   -R ___ksymtab+sort -R ___kcrctab+sort
+STUBCOPY_FLAGS-$(CONFIG_ARM)	+= --rename-section .data=.data.efistub
+STUBCOPY_RM-$(CONFIG_ARM)	+= -R ___ksymtab+sort -R ___kcrctab+sort
 STUBCOPY_RELOC-$(CONFIG_ARM)	:= R_ARM_ABS
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
       [not found] ` <1485340759-28975-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-01-25 10:39   ` [PATCH v2 1/2] efi: libstub: Preserve .debug sections after absolute relocation check Ard Biesheuvel
@ 2017-01-25 10:39   ` Ard Biesheuvel
       [not found]     ` <1485340759-28975-3-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  1 sibling, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-25 10:39 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io
  Cc: Ard Biesheuvel

When building with debugging symbols, take the absolute path to the
vmlinux binary and add it to the special PE/COFF debug table entry.

These entries are used internally by EDK2 based* debug builds of UEFI
to populate the DebugImageInfo table, which can be used by debuggers
as well as by the OS itself to retrieve information about all loaded
PE/COFF executables. This is highly useful for source level debugging
of the UEFI stub.

* for AArch64, this probably means all of them

Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 arch/arm64/kernel/Makefile |  4 +++
 arch/arm64/kernel/head.S   | 34 +++++++++++++++++++-
 arch/arm64/kernel/image.h  |  3 ++
 3 files changed, 40 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
index 7d66bbaafc0c..6dbc0e5527f5 100644
--- a/arch/arm64/kernel/Makefile
+++ b/arch/arm64/kernel/Makefile
@@ -55,3 +55,7 @@ obj-y					+= $(arm64-obj-y) vdso/ probes/
 obj-m					+= $(arm64-obj-m)
 head-y					:= head.o
 extra-y					+= $(head-y) vmlinux.lds
+
+ifeq ($(CONFIG_EFI)$(CONFIG_DEBUG_INFO),yy)
+AFLAGS_head.o += -DVMLINUX_PATH="\"$(shell readlink -f $(objtree)/vmlinux)\""
+endif
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index 4b1abac3485a..b462ab1f11ef 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -149,7 +149,7 @@ extra_header_fields:
 	.quad	0				// SizeOfHeapReserve
 	.quad	0				// SizeOfHeapCommit
 	.long	0				// LoaderFlags
-	.long	0x6				// NumberOfRvaAndSizes
+	.long	(section_table - .) / 8		// NumberOfRvaAndSizes
 
 	.quad	0				// ExportTable
 	.quad	0				// ImportTable
@@ -158,6 +158,11 @@ extra_header_fields:
 	.quad	0				// CertificationTable
 	.quad	0				// BaseRelocationTable
 
+#ifdef CONFIG_DEBUG_INFO
+	.long	efi_debug_table - _head		// DebugTable
+	.long	efi_debug_table_size
+#endif
+
 	// Section table
 section_table:
 
@@ -204,6 +209,33 @@ section_table:
 	 */
 	.align 12
 efi_header_end:
+
+#ifdef CONFIG_DEBUG_INFO
+	__INITDATA
+
+efi_debug_table:
+	// EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
+	.long	0			// Characteristics
+	.long	0			// TimeDateStamp
+	.short	0			// MajorVersion
+	.short	0			// MinorVersion
+	.long	2			// Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW
+	.long	efi_debug_entry_size	// SizeOfData
+	.long	efi_debug_entry_offset	// RVA
+	.long	efi_debug_entry_offset	// FileOffset
+
+ENTRY(efi_debug_entry)
+	// EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
+	.long	0x3031424E		// Signature
+	.long	0			// Unknown
+	.long	0			// Unknown2
+	.long	0			// Unknown3
+
+	.asciz	VMLINUX_PATH
+
+	.set	efi_debug_entry_size, . - efi_debug_entry
+	.set	efi_debug_table_size, . - efi_debug_table
+#endif
 #endif
 
 	__INIT
diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
index c7fcb232fe47..ad5406f011c2 100644
--- a/arch/arm64/kernel/image.h
+++ b/arch/arm64/kernel/image.h
@@ -116,6 +116,9 @@ __efistub__end			= KALLSYMS_HIDE(_end);
 __efistub__edata		= KALLSYMS_HIDE(_edata);
 __efistub_screen_info		= KALLSYMS_HIDE(screen_info);
 
+#ifdef CONFIG_DEBUG_INFO
+efi_debug_entry_offset = efi_debug_entry - _text;
+#endif
 #endif
 
 #endif /* __ASM_IMAGE_H */
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
       [not found]     ` <1485340759-28975-3-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
@ 2017-01-25 11:45       ` Ard Biesheuvel
  2017-01-25 11:53       ` Mark Rutland
  1 sibling, 0 replies; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-25 11:45 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Will Deacon,
	Catalin Marinas, Leif Lindholm, Mark Rutland, Matt Fleming
  Cc: Ard Biesheuvel

On 25 January 2017 at 10:39, Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> wrote:
> When building with debugging symbols, take the absolute path to the
> vmlinux binary and add it to the special PE/COFF debug table entry.
>
> These entries are used internally by EDK2 based* debug builds of UEFI
> to populate the DebugImageInfo table, which can be used by debuggers
> as well as by the OS itself to retrieve information about all loaded
> PE/COFF executables. This is highly useful for source level debugging
> of the UEFI stub.
>
> * for AArch64, this probably means all of them
>
> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/Makefile |  4 +++
>  arch/arm64/kernel/head.S   | 34 +++++++++++++++++++-
>  arch/arm64/kernel/image.h  |  3 ++
>  3 files changed, 40 insertions(+), 1 deletion(-)
>

If desired, I can simplify this patch somewhat, i.e., drop the image.h
change, and reduce the visibility of efi_debug_entry

> diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile
> index 7d66bbaafc0c..6dbc0e5527f5 100644
> --- a/arch/arm64/kernel/Makefile
> +++ b/arch/arm64/kernel/Makefile
> @@ -55,3 +55,7 @@ obj-y                                 += $(arm64-obj-y) vdso/ probes/
>  obj-m                                  += $(arm64-obj-m)
>  head-y                                 := head.o
>  extra-y                                        += $(head-y) vmlinux.lds
> +
> +ifeq ($(CONFIG_EFI)$(CONFIG_DEBUG_INFO),yy)
> +AFLAGS_head.o += -DVMLINUX_PATH="\"$(shell readlink -f $(objtree)/vmlinux)\""
> +endif
> diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
> index 4b1abac3485a..b462ab1f11ef 100644
> --- a/arch/arm64/kernel/head.S
> +++ b/arch/arm64/kernel/head.S
> @@ -149,7 +149,7 @@ extra_header_fields:
>         .quad   0                               // SizeOfHeapReserve
>         .quad   0                               // SizeOfHeapCommit
>         .long   0                               // LoaderFlags
> -       .long   0x6                             // NumberOfRvaAndSizes
> +       .long   (section_table - .) / 8         // NumberOfRvaAndSizes
>
>         .quad   0                               // ExportTable
>         .quad   0                               // ImportTable
> @@ -158,6 +158,11 @@ extra_header_fields:
>         .quad   0                               // CertificationTable
>         .quad   0                               // BaseRelocationTable
>
> +#ifdef CONFIG_DEBUG_INFO
> +       .long   efi_debug_table - _head         // DebugTable
> +       .long   efi_debug_table_size
> +#endif
> +
>         // Section table
>  section_table:
>
> @@ -204,6 +209,33 @@ section_table:
>          */
>         .align 12
>  efi_header_end:
> +
> +#ifdef CONFIG_DEBUG_INFO
> +       __INITDATA
> +
> +efi_debug_table:
> +       // EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
> +       .long   0                       // Characteristics
> +       .long   0                       // TimeDateStamp
> +       .short  0                       // MajorVersion
> +       .short  0                       // MinorVersion
> +       .long   2                       // Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW
> +       .long   efi_debug_entry_size    // SizeOfData
> +       .long   efi_debug_entry_offset  // RVA
> +       .long   efi_debug_entry_offset  // FileOffset
> +
> +ENTRY(efi_debug_entry)
> +       // EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
> +       .long   0x3031424E              // Signature
> +       .long   0                       // Unknown
> +       .long   0                       // Unknown2
> +       .long   0                       // Unknown3
> +
> +       .asciz  VMLINUX_PATH
> +
> +       .set    efi_debug_entry_size, . - efi_debug_entry
> +       .set    efi_debug_table_size, . - efi_debug_table
> +#endif
>  #endif
>
>         __INIT
> diff --git a/arch/arm64/kernel/image.h b/arch/arm64/kernel/image.h
> index c7fcb232fe47..ad5406f011c2 100644
> --- a/arch/arm64/kernel/image.h
> +++ b/arch/arm64/kernel/image.h
> @@ -116,6 +116,9 @@ __efistub__end                      = KALLSYMS_HIDE(_end);
>  __efistub__edata               = KALLSYMS_HIDE(_edata);
>  __efistub_screen_info          = KALLSYMS_HIDE(screen_info);
>
> +#ifdef CONFIG_DEBUG_INFO
> +efi_debug_entry_offset = efi_debug_entry - _text;
> +#endif
>  #endif
>
>  #endif /* __ASM_IMAGE_H */
> --
> 2.7.4
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
       [not found]     ` <1485340759-28975-3-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
  2017-01-25 11:45       ` Ard Biesheuvel
@ 2017-01-25 11:53       ` Mark Rutland
  2017-01-25 12:00         ` Ard Biesheuvel
  1 sibling, 1 reply; 9+ messages in thread
From: Mark Rutland @ 2017-01-25 11:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
	will.deacon-5wv7dgnIgG8, catalin.marinas-5wv7dgnIgG8,
	leif.lindholm-QSEj5FYQhm4dnm+yROfE0A,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io

On Wed, Jan 25, 2017 at 10:39:19AM +0000, Ard Biesheuvel wrote:
> When building with debugging symbols, take the absolute path to the
> vmlinux binary and add it to the special PE/COFF debug table entry.
> 
> These entries are used internally by EDK2 based* debug builds of UEFI
> to populate the DebugImageInfo table, which can be used by debuggers
> as well as by the OS itself to retrieve information about all loaded
> PE/COFF executables. This is highly useful for source level debugging
> of the UEFI stub.

Does that mean EFI_IMAGE_DEBUG_DIRECTORY_ENTRY and friends are
EDK2-specific?

Or just that the way EDK2 happens to use those is EDK2-specific?

> * for AArch64, this probably means all of them
> 
> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
>  arch/arm64/kernel/Makefile |  4 +++
>  arch/arm64/kernel/head.S   | 34 +++++++++++++++++++-
>  arch/arm64/kernel/image.h  |  3 ++
>  3 files changed, 40 insertions(+), 1 deletion(-)

> +efi_debug_table:
> +	// EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
> +	.long	0			// Characteristics
> +	.long	0			// TimeDateStamp
> +	.short	0			// MajorVersion
> +	.short	0			// MinorVersion
> +	.long	2			// Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW

> +ENTRY(efi_debug_entry)
> +	// EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
> +	.long	0x3031424E		// Signature

Certainly not a blocker for this, but this reminds me that it would be
nice to de-magic the EFI constants used by the stub.

I took a stab a while back [1], for the existing definitions, but that
fell by the wayside.

Thanks,
Mark.

[1] git://git.kernel.org/pub/scm/linux/kernel/git/mark/linux.git efi-stub/definitions

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
  2017-01-25 11:53       ` Mark Rutland
@ 2017-01-25 12:00         ` Ard Biesheuvel
  2017-01-26 18:26           ` Mark Rutland
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-25 12:00 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-efi-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Will Deacon,
	Catalin Marinas, Leif Lindholm, Matt Fleming

On 25 January 2017 at 11:53, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Wed, Jan 25, 2017 at 10:39:19AM +0000, Ard Biesheuvel wrote:
>> When building with debugging symbols, take the absolute path to the
>> vmlinux binary and add it to the special PE/COFF debug table entry.
>>
>> These entries are used internally by EDK2 based* debug builds of UEFI
>> to populate the DebugImageInfo table, which can be used by debuggers
>> as well as by the OS itself to retrieve information about all loaded
>> PE/COFF executables. This is highly useful for source level debugging
>> of the UEFI stub.
>
> Does that mean EFI_IMAGE_DEBUG_DIRECTORY_ENTRY and friends are
> EDK2-specific?
>
> Or just that the way EDK2 happens to use those is EDK2-specific?
>

Those values are defined by the PE/COFF spec, and I assume that a
CodeView type entry in the debug table usually contains a NUL
terminated string as well, given that the EDK2 crowd is very
Wintel-heavy.

The significance of mentioning EDK2 here was that I thought that the
DebugImageInfo table was a PI construct rather than something
described in the UEFI spec. But looking more carefully, it seems that
this table is in fact a UEFI construct, so I should probably drop this
mention from the commit log

>> * for AArch64, this probably means all of them
>>
>> Cc: Matt Fleming <matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>>  arch/arm64/kernel/Makefile |  4 +++
>>  arch/arm64/kernel/head.S   | 34 +++++++++++++++++++-
>>  arch/arm64/kernel/image.h  |  3 ++
>>  3 files changed, 40 insertions(+), 1 deletion(-)
>
>> +efi_debug_table:
>> +     // EFI_IMAGE_DEBUG_DIRECTORY_ENTRY
>> +     .long   0                       // Characteristics
>> +     .long   0                       // TimeDateStamp
>> +     .short  0                       // MajorVersion
>> +     .short  0                       // MinorVersion
>> +     .long   2                       // Type == EFI_IMAGE_DEBUG_TYPE_CODEVIEW
>
>> +ENTRY(efi_debug_entry)
>> +     // EFI_IMAGE_DEBUG_CODEVIEW_NB10_ENTRY
>> +     .long   0x3031424E              // Signature
>
> Certainly not a blocker for this, but this reminds me that it would be
> nice to de-magic the EFI constants used by the stub.
>
> I took a stab a while back [1], for the existing definitions, but that
> fell by the wayside.
>

Good point.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
  2017-01-25 12:00         ` Ard Biesheuvel
@ 2017-01-26 18:26           ` Mark Rutland
  2017-01-26 18:33             ` Ard Biesheuvel
  0 siblings, 1 reply; 9+ messages in thread
From: Mark Rutland @ 2017-01-26 18:26 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Matt Fleming, Catalin Marinas, Will Deacon,
	Leif Lindholm, linux-arm-kernel

On Wed, Jan 25, 2017 at 12:00:44PM +0000, Ard Biesheuvel wrote:
> On 25 January 2017 at 11:53, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Jan 25, 2017 at 10:39:19AM +0000, Ard Biesheuvel wrote:
> >> When building with debugging symbols, take the absolute path to the
> >> vmlinux binary and add it to the special PE/COFF debug table entry.
> >>
> >> These entries are used internally by EDK2 based* debug builds of UEFI
> >> to populate the DebugImageInfo table, which can be used by debuggers
> >> as well as by the OS itself to retrieve information about all loaded
> >> PE/COFF executables. This is highly useful for source level debugging
> >> of the UEFI stub.
> >
> > Does that mean EFI_IMAGE_DEBUG_DIRECTORY_ENTRY and friends are
> > EDK2-specific?
> >
> > Or just that the way EDK2 happens to use those is EDK2-specific?
> 
> Those values are defined by the PE/COFF spec, and I assume that a
> CodeView type entry in the debug table usually contains a NUL
> terminated string as well, given that the EDK2 crowd is very
> Wintel-heavy.

So we don't actually have a definition of the format of a CodeView
entry, and we're guessing?

That does feel a little scary, especially given the fields are named
"Unknown". :(

> The significance of mentioning EDK2 here was that I thought that the
> DebugImageInfo table was a PI construct rather than something
> described in the UEFI spec. But looking more carefully, it seems that
> this table is in fact a UEFI construct, so I should probably drop this
> mention from the commit log

That would probably be for the best, yes.

Thanks,
Mark.

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
  2017-01-26 18:26           ` Mark Rutland
@ 2017-01-26 18:33             ` Ard Biesheuvel
  2017-01-26 18:44               ` Mark Rutland
  0 siblings, 1 reply; 9+ messages in thread
From: Ard Biesheuvel @ 2017-01-26 18:33 UTC (permalink / raw)
  To: Mark Rutland
  Cc: linux-efi, Matt Fleming, Catalin Marinas, Will Deacon,
	Leif Lindholm, linux-arm-kernel

On 26 January 2017 at 18:26, Mark Rutland <mark.rutland@arm.com> wrote:
> On Wed, Jan 25, 2017 at 12:00:44PM +0000, Ard Biesheuvel wrote:
>> On 25 January 2017 at 11:53, Mark Rutland <mark.rutland@arm.com> wrote:
>> > On Wed, Jan 25, 2017 at 10:39:19AM +0000, Ard Biesheuvel wrote:
>> >> When building with debugging symbols, take the absolute path to the
>> >> vmlinux binary and add it to the special PE/COFF debug table entry.
>> >>
>> >> These entries are used internally by EDK2 based* debug builds of UEFI
>> >> to populate the DebugImageInfo table, which can be used by debuggers
>> >> as well as by the OS itself to retrieve information about all loaded
>> >> PE/COFF executables. This is highly useful for source level debugging
>> >> of the UEFI stub.
>> >
>> > Does that mean EFI_IMAGE_DEBUG_DIRECTORY_ENTRY and friends are
>> > EDK2-specific?
>> >
>> > Or just that the way EDK2 happens to use those is EDK2-specific?
>>
>> Those values are defined by the PE/COFF spec, and I assume that a
>> CodeView type entry in the debug table usually contains a NUL
>> terminated string as well, given that the EDK2 crowd is very
>> Wintel-heavy.
>
> So we don't actually have a definition of the format of a CodeView
> entry, and we're guessing?
>
> That does feel a little scary, especially given the fields are named
> "Unknown". :(
>

No, we are emitting them in exactly the same way as the EDK2 tooling
emits them, which is not entirely unreasonable given that
a) the EDK2 tooling was created by Intel engineers in an attempt to
generate Visual Studio compatible PE/COFF images, and
b) someone hacking on the arm64 Linux kernel on a UEFI system is
likely to be using EDK2 based firmware, and *highly* unlikely to be
using Visual Studio.

>> The significance of mentioning EDK2 here was that I thought that the
>> DebugImageInfo table was a PI construct rather than something
>> described in the UEFI spec. But looking more carefully, it seems that
>> this table is in fact a UEFI construct, so I should probably drop this
>> mention from the commit log
>
> That would probably be for the best, yes.
>

Yes I will update the commit log for v3

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary
  2017-01-26 18:33             ` Ard Biesheuvel
@ 2017-01-26 18:44               ` Mark Rutland
  0 siblings, 0 replies; 9+ messages in thread
From: Mark Rutland @ 2017-01-26 18:44 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: linux-efi, Matt Fleming, Catalin Marinas, Will Deacon,
	Leif Lindholm, linux-arm-kernel

On Thu, Jan 26, 2017 at 06:33:19PM +0000, Ard Biesheuvel wrote:
> On 26 January 2017 at 18:26, Mark Rutland <mark.rutland@arm.com> wrote:
> > On Wed, Jan 25, 2017 at 12:00:44PM +0000, Ard Biesheuvel wrote:
> >> On 25 January 2017 at 11:53, Mark Rutland <mark.rutland@arm.com> wrote:
> >> > On Wed, Jan 25, 2017 at 10:39:19AM +0000, Ard Biesheuvel wrote:
> >> >> When building with debugging symbols, take the absolute path to the
> >> >> vmlinux binary and add it to the special PE/COFF debug table entry.
> >> >>
> >> >> These entries are used internally by EDK2 based* debug builds of UEFI
> >> >> to populate the DebugImageInfo table, which can be used by debuggers
> >> >> as well as by the OS itself to retrieve information about all loaded
> >> >> PE/COFF executables. This is highly useful for source level debugging
> >> >> of the UEFI stub.
> >> >
> >> > Does that mean EFI_IMAGE_DEBUG_DIRECTORY_ENTRY and friends are
> >> > EDK2-specific?
> >> >
> >> > Or just that the way EDK2 happens to use those is EDK2-specific?
> >>
> >> Those values are defined by the PE/COFF spec, and I assume that a
> >> CodeView type entry in the debug table usually contains a NUL
> >> terminated string as well, given that the EDK2 crowd is very
> >> Wintel-heavy.
> >
> > So we don't actually have a definition of the format of a CodeView
> > entry, and we're guessing?
> >
> > That does feel a little scary, especially given the fields are named
> > "Unknown". :(
> 
> No, we are emitting them in exactly the same way as the EDK2 tooling
> emits them,

Ah. That's fine by me, then.

Thanks,
Mark,

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-01-26 18:44 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-25 10:39 [PATCH v2 0/2] efi/arm64: add vmlinux link to PE/COFF debug table Ard Biesheuvel
     [not found] ` <1485340759-28975-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-01-25 10:39   ` [PATCH v2 1/2] efi: libstub: Preserve .debug sections after absolute relocation check Ard Biesheuvel
2017-01-25 10:39   ` [PATCH v2 2/2] arm64: efi: add vmlinux debug link to the Image binary Ard Biesheuvel
     [not found]     ` <1485340759-28975-3-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-01-25 11:45       ` Ard Biesheuvel
2017-01-25 11:53       ` Mark Rutland
2017-01-25 12:00         ` Ard Biesheuvel
2017-01-26 18:26           ` Mark Rutland
2017-01-26 18:33             ` Ard Biesheuvel
2017-01-26 18:44               ` Mark Rutland

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).