[PATCH] .debug ignore - to squash with efi:keep efi

[PATCH] .debug ignore - to squash with efi:keep efi

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

---
 .gitignore | 1 +
 1 file changed, 1 insertion(+)

diff --git a/.gitignore b/.gitignore
index 29f352c..2168e01 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,7 @@ tags
 *.flat
 *.efi
 *.elf
+*.debug
 *.patch
 .pc
 patches
-- 
2.34.1

[kvm-unit-tests PATCH v3 0/6] arm64: improve debuggability

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

My recent experience in debugging ARM64 tests on EFI was not as fun as I
expected it to be.

There were several reasons for that besides the questionable definition
of "fun":

1. ARM64 is not compiled with frame pointers and there is no stack
   unwinder when the stack is dumped.

2. Building an EFI drops the debug information.

3. The addresses that are printed on dump_stack() and the use of GDB
   are hard because taking code relocation into account is non trivial.

The patches help both ARM64 and EFI for this matter. The image address
is printed when EFI is used to allow the use of GDB. Symbols are emitted
into a separate debug file. The frame pointer is included and special
entry is added upon an exception to allow backtracing across
exceptions.

[ PowerPC: Please ack patches 1,2 ]
[ x86: Please ack patches 1,2,5 ]

v2->v3:
* Consider PowerPC for reloc and related fixes [Andrew]

v1->v2:
* Andrew comments [see in individual patches]
* Few cleanups

Nadav Amit (6):
  efi: keep efi debug information in a separate file
  lib/stack: print base addresses on relocation setups
  arm64: enable frame pointer and support stack unwinding
  arm64: stack: update trace stack on exception
  efi: print address of image
  arm64: dump stack on bad exception

 .gitignore              |  1 +
 Makefile                |  2 +-
 arm/Makefile.arm        |  3 --
 arm/Makefile.arm64      |  2 ++
 arm/Makefile.common     |  8 +++++-
 arm/cstart64.S          | 13 +++++++++
 lib/arm64/asm-offsets.c |  6 +++-
 lib/arm64/asm/stack.h   |  3 ++
 lib/arm64/processor.c   |  1 +
 lib/arm64/stack.c       | 62 +++++++++++++++++++++++++++++++++++++++++
 lib/efi.c               |  4 +++
 lib/stack.c             | 31 +++++++++++++++++++--
 powerpc/Makefile.common |  1 +
 x86/Makefile.common     |  5 +++-
 14 files changed, 133 insertions(+), 9 deletions(-)
 create mode 100644 lib/arm64/stack.c

-- 
2.34.1

[kvm-unit-tests PATCH v3 1/6] efi: keep efi debug information in a separate file

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Debugging tests that run on EFI is hard because the debug information is
not included in the EFI file. Dump it into a separeate .debug file to
allow the use of gdb or pretty_print_stacks script.

Signed-off-by: Nadav Amit <namit@vmware.com>

---

v2->v3:
* Add *.debug to .gitignore [Andrew]

v1->v2:
* Making clean should remove .debug [Andrew]
* x86 EFI support [Andrew]
---
 .gitignore          | 1 +
 arm/Makefile.common | 5 ++++-
 x86/Makefile.common | 5 ++++-
 3 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/.gitignore b/.gitignore
index 29f352c..2168e01 100644
--- a/.gitignore
+++ b/.gitignore
@@ -7,6 +7,7 @@ tags
 *.flat
 *.efi
 *.elf
+*.debug
 *.patch
 .pc
 patches
diff --git a/arm/Makefile.common b/arm/Makefile.common
index d60cf8c..9b45a8f 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -78,6 +78,9 @@ ifeq ($(CONFIG_EFI),y)
 
 %.efi: %.so
 	$(call arch_elf_check, $^)
+	$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(OBJCOPY) --strip-debug $^
+	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
 	$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .dynamic -j .dynsym \
 		-j .rel -j .rela -j .rel.* -j .rela.* -j .rel* -j .rela* \
@@ -103,7 +106,7 @@ $(libeabi): $(eabiobjs)
 	$(AR) rcs $@ $^
 
 arm_clean: asm_offsets_clean
-	$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi} $(libeabi) $(eabiobjs) \
+	$(RM) $(TEST_DIR)/*.{o,flat,elf,so,efi,debug} $(libeabi) $(eabiobjs) \
 	      $(TEST_DIR)/.*.d $(TEST_DIR)/efi/.*.d lib/arm/.*.d
 
 generated-files = $(asm-offsets)
diff --git a/x86/Makefile.common b/x86/Makefile.common
index 9f2bc93..c42c3e4 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -54,6 +54,9 @@ ifeq ($(CONFIG_EFI),y)
 	@chmod a-x $@
 
 %.efi: %.so
+	$(OBJCOPY) --only-keep-debug $^ $@.debug
+	$(OBJCOPY) --strip-debug $^
+	$(OBJCOPY) --add-gnu-debuglink=$@.debug $^
 	$(OBJCOPY) \
 		-j .text -j .sdata -j .data -j .dynamic -j .dynsym -j .rel \
 		-j .rela -j .reloc -S --target=$(FORMAT) $< $@
@@ -124,4 +127,4 @@ arch_clean:
 	$(RM) $(TEST_DIR)/*.o $(TEST_DIR)/*.flat $(TEST_DIR)/*.elf \
 	$(TEST_DIR)/.*.d lib/x86/.*.d \
 	$(TEST_DIR)/efi/*.o $(TEST_DIR)/efi/.*.d \
-	$(TEST_DIR)/*.so $(TEST_DIR)/*.efi
+	$(TEST_DIR)/*.so $(TEST_DIR)/*.efi $(TEST_DIR)/*.debug
-- 
2.34.1

[kvm-unit-tests PATCH v3 2/6] lib/stack: print base addresses on relocation setups

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Making sense from dumped stacks when running EFI tests is very hard due
to the relocation. Fix it by adjusting the address back to the original
address.

Introduce CONFIG_RELOC, which would be set on arm64 and on EFI configs.

Suggested-by: Andrew Jones <andrew.jones@linux.dev>
Signed-off-by: Nadav Amit <namit@vmware.com>

---
v2->v3:
* Mark PowerPC as relocatable as well [Andrew]
* Fixing incorrect Makefile [Andrew]

v1->v2:
* Introduce CONFIG_RELOC to support ARM64 [Andrew]
---
 Makefile                |  2 +-
 arm/Makefile.arm64      |  1 +
 lib/stack.c             | 31 +++++++++++++++++++++++++++++--
 powerpc/Makefile.common |  1 +
 4 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 307bc29..ae79059 100644
--- a/Makefile
+++ b/Makefile
@@ -40,7 +40,7 @@ OBJDIRS += $(LIBFDT_objdir)
 
 # EFI App
 ifeq ($(CONFIG_EFI),y)
-EFI_CFLAGS := -DCONFIG_EFI
+EFI_CFLAGS := -DCONFIG_EFI -DCONFIG_RELOC
 # The following CFLAGS and LDFLAGS come from:
 #   - GNU-EFI/Makefile.defaults
 #   - GNU-EFI/apps/Makefile
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
index eada7f9..85b3348 100644
--- a/arm/Makefile.arm64
+++ b/arm/Makefile.arm64
@@ -12,6 +12,7 @@ CFLAGS += -mstrict-align
 
 mno_outline_atomics := $(call cc-option, -mno-outline-atomics, "")
 CFLAGS += $(mno_outline_atomics)
+CFLAGS += -DCONFIG_RELOC
 
 define arch_elf_check =
 	$(if $(shell ! $(READELF) -rW $(1) >&/dev/null && echo "nok"),
diff --git a/lib/stack.c b/lib/stack.c
index bdb23fd..dd6bfa8 100644
--- a/lib/stack.c
+++ b/lib/stack.c
@@ -6,13 +6,38 @@
  */
 
 #include <libcflat.h>
+#include <stdbool.h>
 #include <stack.h>
 
 #define MAX_DEPTH 20
 
+#ifdef CONFIG_RELOC
+extern char _text, _etext;
+
+static bool base_address(const void *rebased_addr, unsigned long *addr)
+{
+	unsigned long ra = (unsigned long)rebased_addr;
+	unsigned long start = (unsigned long)&_text;
+	unsigned long end = (unsigned long)&_etext;
+
+	if (ra < start || ra >= end)
+		return false;
+
+	*addr = ra - start;
+	return true;
+}
+#else
+static bool base_address(const void *rebased_addr, unsigned long *addr)
+{
+	*addr = (unsigned long)rebased_addr;
+	return true;
+}
+#endif
+
 static void print_stack(const void **return_addrs, int depth,
 			bool top_is_return_address)
 {
+	unsigned long addr;
 	int i = 0;
 
 	printf("\tSTACK:");
@@ -20,12 +45,14 @@ static void print_stack(const void **return_addrs, int depth,
 	/* @addr indicates a non-return address, as expected by the stack
 	 * pretty printer script. */
 	if (depth > 0 && !top_is_return_address) {
-		printf(" @%lx", (unsigned long) return_addrs[0]);
+		if (base_address(return_addrs[0], &addr))
+			printf(" @%lx", addr);
 		i++;
 	}
 
 	for (; i < depth; i++) {
-		printf(" %lx", (unsigned long) return_addrs[i]);
+		if (base_address(return_addrs[i], &addr))
+			printf(" %lx", addr);
 	}
 	printf("\n");
 }
diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
index 8ce0034..c2e976e 100644
--- a/powerpc/Makefile.common
+++ b/powerpc/Makefile.common
@@ -24,6 +24,7 @@ CFLAGS += -ffreestanding
 CFLAGS += -O2 -msoft-float -mno-altivec $(mabi_no_altivec)
 CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
 CFLAGS += -Wa,-mregnames
+CFLAGS += -DCONFIG_RELOC
 
 # We want to keep intermediate files
 .PRECIOUS: %.o
-- 
2.34.1

[kvm-unit-tests PATCH v3 3/6] arm64: enable frame pointer and support stack unwinding

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Enable frame pointers for arm64 and perform stack unwinding based on
arm64 convention.

Signed-off-by: Nadav Amit <namit@vmware.com>

---
v1->v2:
* Adding SPDX [checkpatch]
* Moving some unused declarations to next patch [Andrew]
* Adding recursion prevention
---
 arm/Makefile.arm      |  3 ---
 arm/Makefile.arm64    |  1 +
 arm/Makefile.common   |  3 +++
 lib/arm64/asm/stack.h |  3 +++
 lib/arm64/stack.c     | 44 +++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 51 insertions(+), 3 deletions(-)
 create mode 100644 lib/arm64/stack.c

diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index 2ce00f5..7fd39f3 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -11,9 +11,6 @@ ifeq ($(CONFIG_EFI),y)
 $(error Cannot build arm32 tests as EFI apps)
 endif
 
-# stack.o relies on frame pointers.
-KEEP_FRAME_POINTER := y
-
 CFLAGS += $(machine)
 CFLAGS += -mcpu=$(PROCESSOR)
 CFLAGS += -mno-unaligned-access
diff --git a/arm/Makefile.arm64 b/arm/Makefile.arm64
index 85b3348..960880f 100644
--- a/arm/Makefile.arm64
+++ b/arm/Makefile.arm64
@@ -22,6 +22,7 @@ define arch_elf_check =
 endef
 
 cstart.o = $(TEST_DIR)/cstart64.o
+cflatobjs += lib/arm64/stack.o
 cflatobjs += lib/arm64/processor.o
 cflatobjs += lib/arm64/spinlock.o
 cflatobjs += lib/arm64/gic-v3-its.o lib/arm64/gic-v3-its-cmd.o
diff --git a/arm/Makefile.common b/arm/Makefile.common
index 9b45a8f..bc86e44 100644
--- a/arm/Makefile.common
+++ b/arm/Makefile.common
@@ -22,6 +22,9 @@ $(TEST_DIR)/sieve.elf: AUXFLAGS = 0x1
 ##################################################################
 AUXFLAGS ?= 0x0
 
+# stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
 CFLAGS += -std=gnu99
 CFLAGS += -ffreestanding
 CFLAGS += -O2
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
index d000624..be486cf 100644
--- a/lib/arm64/asm/stack.h
+++ b/lib/arm64/asm/stack.h
@@ -5,4 +5,7 @@
 #error Do not directly include <asm/stack.h>. Just use <stack.h>.
 #endif
 
+#define HAVE_ARCH_BACKTRACE_FRAME
+#define HAVE_ARCH_BACKTRACE
+
 #endif
diff --git a/lib/arm64/stack.c b/lib/arm64/stack.c
new file mode 100644
index 0000000..a2024e8
--- /dev/null
+++ b/lib/arm64/stack.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Backtrace support.
+ */
+#include <libcflat.h>
+#include <stdbool.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
+{
+	const void *fp = frame;
+	static bool walking;
+	void *lr;
+	int depth;
+
+	if (walking) {
+		printf("RECURSIVE STACK WALK!!!\n");
+		return 0;
+	}
+	walking = true;
+
+	/*
+	 * ARM64 stack grows down. fp points to the previous fp on the stack,
+	 * and lr is just above it
+	 */
+	for (depth = 0; fp && depth < max_depth; ++depth) {
+
+		asm volatile ("ldp %0, %1, [%2]"
+				  : "=r" (fp), "=r" (lr)
+				  : "r" (fp)
+				  : );
+
+		return_addrs[depth] = lr;
+	}
+
+	walking = false;
+	return depth;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+	return backtrace_frame(__builtin_frame_address(0),
+			       return_addrs, max_depth);
+}
-- 
2.34.1

[kvm-unit-tests PATCH v3 4/6] arm64: stack: update trace stack on exception

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Using gdb for backtracing or dumping the stack following an exception is
not very helpful as the exact location of the exception is not saved.

Add an additional frame to save the location of the exception.

One delicate point is dealing with the pretty_print_stacks script. When
the stack is dumped, the script would not print the right address for
the exception address: for every return address it deducts "1" before
looking for the instruction location in the code (using addr2line). As a
somewhat hacky solution add "1" for the exception address when dumping
the stack.

Signed-off-by: Nadav Amit <namit@vmware.com>

---
v1->v2:
* .globl before label [Andrew]
* Some comments [Andrew]
---
 arm/cstart64.S          | 13 +++++++++++++
 lib/arm64/asm-offsets.c |  6 +++++-
 lib/arm64/stack.c       | 18 ++++++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

diff --git a/arm/cstart64.S b/arm/cstart64.S
index cbd6b51..865a96d 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -314,6 +314,13 @@ exceptions_init:
 	mrs	x2, spsr_el1
 	stp	x1, x2, [sp, #S_PC]
 
+	/*
+	 * Save a frame pointer using the link to allow unwinding of
+	 * exceptions.
+	 */
+	stp	x29, x1, [sp, #S_FP]
+	add 	x29, sp, #S_FP
+
 	mov	x0, \vec
 	mov	x1, sp
 	mrs	x2, esr_el1
@@ -349,6 +356,9 @@ exceptions_init:
 	eret
 .endm
 
+.globl vector_stub_start
+vector_stub_start:
+
 vector_stub	el1t_sync,     0
 vector_stub	el1t_irq,      1
 vector_stub	el1t_fiq,      2
@@ -369,6 +379,9 @@ vector_stub	el0_irq_32,   13
 vector_stub	el0_fiq_32,   14
 vector_stub	el0_error_32, 15
 
+.globl vector_stub_end
+vector_stub_end:
+
 .section .text.ex
 
 .macro ventry, label
diff --git a/lib/arm64/asm-offsets.c b/lib/arm64/asm-offsets.c
index 53a1277..80de023 100644
--- a/lib/arm64/asm-offsets.c
+++ b/lib/arm64/asm-offsets.c
@@ -25,6 +25,10 @@ int main(void)
 	OFFSET(S_PSTATE, pt_regs, pstate);
 	OFFSET(S_ORIG_X0, pt_regs, orig_x0);
 	OFFSET(S_SYSCALLNO, pt_regs, syscallno);
-	DEFINE(S_FRAME_SIZE, sizeof(struct pt_regs));
+
+	/* FP and LR (16 bytes) go on the frame above pt_regs */
+	DEFINE(S_FP, sizeof(struct pt_regs));
+	DEFINE(S_FRAME_SIZE, (sizeof(struct pt_regs) + 16));
+
 	return 0;
 }
diff --git a/lib/arm64/stack.c b/lib/arm64/stack.c
index a2024e8..82611f4 100644
--- a/lib/arm64/stack.c
+++ b/lib/arm64/stack.c
@@ -6,12 +6,16 @@
 #include <stdbool.h>
 #include <stack.h>
 
+extern char vector_stub_start, vector_stub_end;
+
 int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
 {
 	const void *fp = frame;
 	static bool walking;
 	void *lr;
 	int depth;
+	bool is_exception = false;
+	unsigned long addr;
 
 	if (walking) {
 		printf("RECURSIVE STACK WALK!!!\n");
@@ -31,6 +35,20 @@ int backtrace_frame(const void *frame, const void **return_addrs, int max_depth)
 				  : );
 
 		return_addrs[depth] = lr;
+
+		/*
+		 * If this is an exception, add 1 to the pointer so when the
+		 * pretty_print_stacks script is run it would get the right
+		 * address (it deducts 1 to find the call address, but we want
+		 * the actual address).
+		 */
+		if (is_exception)
+			return_addrs[depth] += 1;
+
+		/* Check if we are in the exception handlers for the next entry */
+		addr = (unsigned long)lr;
+		is_exception = (addr >= (unsigned long)&vector_stub_start &&
+				addr < (unsigned long)&vector_stub_end);
 	}
 
 	walking = false;
-- 
2.34.1

[kvm-unit-tests PATCH v3 5/6] efi: print address of image

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Using gdb to debug tests that are run from efi is very hard without
knowing the base address in which the image was loaded. Print the
address so it can be used while debugging.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 lib/efi.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/efi.c b/lib/efi.c
index 2e127a4..2091771 100644
--- a/lib/efi.c
+++ b/lib/efi.c
@@ -19,6 +19,8 @@ extern int __argc, __envc;
 extern char *__argv[100];
 extern char *__environ[200];
 
+extern char _text;
+
 extern int main(int argc, char **argv, char **envp);
 
 efi_system_table_t *efi_system_table = NULL;
@@ -363,6 +365,8 @@ efi_status_t efi_main(efi_handle_t handle, efi_system_table_t *sys_tab)
 		goto efi_main_error;
 	}
 
+	printf("Address of image is: 0x%lx\n", (unsigned long)&_text);
+
 	/* Run the test case */
 	ret = main(__argc, __argv, __environ);
 
-- 
2.34.1

[kvm-unit-tests PATCH v3 6/6] arm64: dump stack on bad exception

[Nadav Amit]

From: Nadav Amit <namit@vmware.com>

Upon a bad exception, the stack is rather useful for debugging, splat it
out.

Signed-off-by: Nadav Amit <namit@vmware.com>
---
 lib/arm64/processor.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index 831207c..5bcad67 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -130,6 +130,7 @@ static void bad_exception(enum vector v, struct pt_regs *regs,
 	printf("Vector: %d (%s)\n", v, vector_names[v]);
 	printf("ESR_EL1: %8s%08x, ec=%#x (%s)\n", "", esr, ec, ec_names[ec]);
 	printf("FAR_EL1: %016lx (%svalid)\n", far, far_valid ? "" : "not ");
+	dump_stack();
 	printf("Exception frame registers:\n");
 	show_regs(regs);
 	abort();
-- 
2.34.1

Re: [PATCH] .debug ignore - to squash with efi:keep efi

[Andrew Jones]

On Wed, Jun 28, 2023 at 12:13:48AM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>

Missing s-o-b.

> 
> ---
>  .gitignore | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/.gitignore b/.gitignore
> index 29f352c..2168e01 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -7,6 +7,7 @@ tags
>  *.flat
>  *.efi
>  *.elf
> +*.debug
>  *.patch
>  .pc
>  patches
> -- 
> 2.34.1
>

The patch threading is busted. Everything in the thread, including the
cover letter, is in reply to this patch.

Thanks,
drew

Re: [PATCH] .debug ignore - to squash with efi:keep efi

[Andrew Jones]

On Wed, Jun 28, 2023 at 10:19:03AM +0200, Andrew Jones wrote:
> On Wed, Jun 28, 2023 at 12:13:48AM +0000, Nadav Amit wrote:
> > From: Nadav Amit <namit@vmware.com>
> 
> Missing s-o-b.
> 
> > 
> > ---
> >  .gitignore | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/.gitignore b/.gitignore
> > index 29f352c..2168e01 100644
> > --- a/.gitignore
> > +++ b/.gitignore
> > @@ -7,6 +7,7 @@ tags
> >  *.flat
> >  *.efi
> >  *.elf
> > +*.debug
> >  *.patch
> >  .pc
> >  patches
> > -- 
> > 2.34.1
> >
> 
> The patch threading is busted. Everything in the thread, including the
> cover letter, is in reply to this patch.
>

OK, I see the .gitignore hunk in patch 1, which is good, since it
certainly doesn't need its own patch. I'll just ignore this "patch".

Thanks,
drew

Re: [PATCH] .debug ignore - to squash with efi:keep efi

[Nadav Amit]

> On Jun 28, 2023, at 1:22 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> !! External Email
> 
> On Wed, Jun 28, 2023 at 10:19:03AM +0200, Andrew Jones wrote:
>> On Wed, Jun 28, 2023 at 12:13:48AM +0000, Nadav Amit wrote:
>>> From: Nadav Amit <namit@vmware.com>
>> 
>> Missing s-o-b.
>> 
>>> 
>>> ---
>>> .gitignore | 1 +
>>> 1 file changed, 1 insertion(+)
>>> 
>>> diff --git a/.gitignore b/.gitignore
>>> index 29f352c..2168e01 100644
>>> --- a/.gitignore
>>> +++ b/.gitignore
>>> @@ -7,6 +7,7 @@ tags
>>> *.flat
>>> *.efi
>>> *.elf
>>> +*.debug
>>> *.patch
>>> .pc
>>> patches
>>> --
>>> 2.34.1
>>> 
>> 
>> The patch threading is busted. Everything in the thread, including the
>> cover letter, is in reply to this patch.
>> 
> 
> OK, I see the .gitignore hunk in patch 1, which is good, since it
> certainly doesn't need its own patch. I'll just ignore this "patch”.

Embarrassing… Will send v4.

Re: [PATCH] .debug ignore - to squash with efi:keep efi

[Nadav Amit]

> On Jun 28, 2023, at 10:30 AM, Nadav Amit <namit@vmware.com> wrote:
> 
> 
> 
>> On Jun 28, 2023, at 1:22 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
>> 
>> OK, I see the .gitignore hunk in patch 1, which is good, since it
>> certainly doesn't need its own patch. I'll just ignore this "patch”.
> 
> Embarrassing… Will send v4.

My double mistake. So I see this patch was not supposed to be sent, but other
than that - you should have the series. Let me know if anything is wrong
(as otherwise I won’t send v4).

Re: [PATCH] .debug ignore - to squash with efi:keep efi

[Andrew Jones]

On Wed, Jun 28, 2023 at 05:33:52PM +0000, Nadav Amit wrote:
> 
> 
> > On Jun 28, 2023, at 10:30 AM, Nadav Amit <namit@vmware.com> wrote:
> > 
> > 
> > 
> >> On Jun 28, 2023, at 1:22 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
> >> 
> >> OK, I see the .gitignore hunk in patch 1, which is good, since it
> >> certainly doesn't need its own patch. I'll just ignore this "patch”.
> > 
> > Embarrassing… Will send v4.
> 
> My double mistake. So I see this patch was not supposed to be sent, but other
> than that - you should have the series. Let me know if anything is wrong
> (as otherwise I won’t send v4).

The series looks good. I plan to queue it, but was hoping to see some acks
from x86 and maybe ppc people first.

(I had to switch to this email address because I can't authenticate to
the mail service used for my @linux.dev address right now... Hopefully
that'll get resolved soon.)

Thanks,
drew

Re: [kvm-unit-tests PATCH v3 2/6] lib/stack: print base addresses on relocation setups

[Andrew Jones]

On Wed, Jun 28, 2023 at 12:13:51AM +0000, Nadav Amit wrote:
...
> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
> index 8ce0034..c2e976e 100644
> --- a/powerpc/Makefile.common
> +++ b/powerpc/Makefile.common
> @@ -24,6 +24,7 @@ CFLAGS += -ffreestanding
>  CFLAGS += -O2 -msoft-float -mno-altivec $(mabi_no_altivec)
>  CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
>  CFLAGS += -Wa,-mregnames
> +CFLAGS += -DCONFIG_RELOC
>

I dropped this change. powerpc doesn't define _text and _etext.
I'll leave it to the ppc people to decide if they want this and
want to add _text and _etext to get it.

Thanks,
drew

Re: [kvm-unit-tests PATCH v3 0/6] arm64: improve debuggability

[Andrew Jones]

On Wed, Jun 28, 2023 at 12:13:49AM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
> 
> My recent experience in debugging ARM64 tests on EFI was not as fun as I
> expected it to be.
> 
> There were several reasons for that besides the questionable definition
> of "fun":
> 
> 1. ARM64 is not compiled with frame pointers and there is no stack
>    unwinder when the stack is dumped.
> 
> 2. Building an EFI drops the debug information.
> 
> 3. The addresses that are printed on dump_stack() and the use of GDB
>    are hard because taking code relocation into account is non trivial.
> 
> The patches help both ARM64 and EFI for this matter. The image address
> is printed when EFI is used to allow the use of GDB. Symbols are emitted
> into a separate debug file. The frame pointer is included and special
> entry is added upon an exception to allow backtracing across
> exceptions.
> 
> [ PowerPC: Please ack patches 1,2 ]
> [ x86: Please ack patches 1,2,5 ]
> 
> v2->v3:
> * Consider PowerPC for reloc and related fixes [Andrew]
> 
> v1->v2:
> * Andrew comments [see in individual patches]
> * Few cleanups
> 
> Nadav Amit (6):
>   efi: keep efi debug information in a separate file
>   lib/stack: print base addresses on relocation setups
>   arm64: enable frame pointer and support stack unwinding
>   arm64: stack: update trace stack on exception
>   efi: print address of image
>   arm64: dump stack on bad exception
> 
>  .gitignore              |  1 +
>  Makefile                |  2 +-
>  arm/Makefile.arm        |  3 --
>  arm/Makefile.arm64      |  2 ++
>  arm/Makefile.common     |  8 +++++-
>  arm/cstart64.S          | 13 +++++++++
>  lib/arm64/asm-offsets.c |  6 +++-
>  lib/arm64/asm/stack.h   |  3 ++
>  lib/arm64/processor.c   |  1 +
>  lib/arm64/stack.c       | 62 +++++++++++++++++++++++++++++++++++++++++
>  lib/efi.c               |  4 +++
>  lib/stack.c             | 31 +++++++++++++++++++--
>  powerpc/Makefile.common |  1 +
>  x86/Makefile.common     |  5 +++-
>  14 files changed, 133 insertions(+), 9 deletions(-)
>  create mode 100644 lib/arm64/stack.c
> 
> -- 
> 2.34.1
>

Merged. arm64 backtraces will be really nice to have!

Thanks, Nadav!

drew

Re: [kvm-unit-tests PATCH v3 2/6] lib/stack: print base addresses on relocation setups

[Nadav Amit]

> On Jul 1, 2023, at 4:34 AM, Andrew Jones <andrew.jones@linux.dev> wrote:
> 
> !! External Email
> 
> On Wed, Jun 28, 2023 at 12:13:51AM +0000, Nadav Amit wrote:
> ...
>> diff --git a/powerpc/Makefile.common b/powerpc/Makefile.common
>> index 8ce0034..c2e976e 100644
>> --- a/powerpc/Makefile.common
>> +++ b/powerpc/Makefile.common
>> @@ -24,6 +24,7 @@ CFLAGS += -ffreestanding
>> CFLAGS += -O2 -msoft-float -mno-altivec $(mabi_no_altivec)
>> CFLAGS += -I $(SRCDIR)/lib -I $(SRCDIR)/lib/libfdt -I lib
>> CFLAGS += -Wa,-mregnames
>> +CFLAGS += -DCONFIG_RELOC
>> 
> 
> I dropped this change. powerpc doesn't define _text and _etext.
> I'll leave it to the ppc people to decide if they want this and
> want to add _text and _etext to get it.

Thanks for taking care of it. Sorry for not noticing. I am always
not inclined to change arch’s that I don’t use.