diff mbox series

[v2,2/2] arm64: efi: add vmlinux debug link to the Image binary

Message ID 1485340759-28975-3-git-send-email-ard.biesheuvel@linaro.org
State New
Headers show
Series efi/arm64: add vmlinux link to PE/COFF debug table | expand

Commit Message

Ard Biesheuvel Jan. 25, 2017, 10:39 a.m. UTC 
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@codeblueprint.co.uk>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.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(-)

-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Ard Biesheuvel Jan. 25, 2017, 11:45 a.m. UTC | #1 
On 25 January 2017 at 10:39, Ard Biesheuvel <ard.biesheuvel@linaro.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@codeblueprint.co.uk>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.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

>

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Rutland Jan. 25, 2017, 11:53 a.m. UTC | #2 
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@codeblueprint.co.uk>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Jan. 25, 2017, noon UTC | #3 
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.

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@codeblueprint.co.uk>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Jan. 26, 2017, 6:33 p.m. UTC | #4 
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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox series

Patch

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 */