* [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature
@ 2016-04-19 14:28 Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 1/8] x86: change exit to abort again Andrew Jones
` (7 more replies)
0 siblings, 8 replies; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
The main motivator of this series is to get the arm build fixed.
522667917 "lib: backtrace printing" added a common backtrace()
function, but we missed that arm's __builtin_return_address only
accepts the input 0, and thus the function, which calls it with
all values 0 to 20, can't compile on arm. To fix backtrace() for
arm, the easiest thing to do is to override it with a backtrace
that works. So this series makes some cleanups to stack tracing
support and adds the support for ARM.
Andrew Jones (8):
x86: change exit to abort again
x86: trivial: there's no dump_stack.o
pretty_print_stacks: keep the 'STACK:' line
pretty_print_stacks: use elf file for the kernel
pretty_print_stacks: addr2line may need a cross prefix
stack: share api prototypes
stack: copy common asm/stack.h bits to all arches
arm: stack: add dump_stack support
arm/Makefile.arm | 4 ++++
configure | 2 ++
lib/arm/asm/stack.h | 11 +++++++++++
lib/arm/processor.c | 1 +
lib/arm/stack.c | 41 +++++++++++++++++++++++++++++++++++++++++
lib/arm64/asm/stack.h | 8 ++++++++
lib/powerpc/asm/stack.h | 8 ++++++++
lib/ppc64/asm/stack.h | 8 ++++++++
lib/stack.h | 9 +++++----
lib/x86/asm/stack.h | 3 ---
lib/x86/desc.c | 2 +-
scripts/pretty_print_stacks.py | 14 +++++++++++---
x86/Makefile.common | 2 +-
13 files changed, 101 insertions(+), 12 deletions(-)
create mode 100644 lib/arm/stack.c
--
2.4.11
^ permalink raw reply [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 1/8] x86: change exit to abort again
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:30 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o Andrew Jones
` (6 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
This is needed again because ed0a50e5 "x86: lib: debug dump
on unhandled exceptions" must have missed it in a rebase.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
lib/x86/desc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/lib/x86/desc.c b/lib/x86/desc.c
index 75765ef5d7804..402204ddcac44 100644
--- a/lib/x86/desc.c
+++ b/lib/x86/desc.c
@@ -96,7 +96,7 @@ static void unhandled_exception(struct ex_regs *regs, bool cpu)
#endif
);
dump_frame_stack((void*) regs->rip, (void*) regs->rbp);
- exit(7);
+ abort();
}
static void check_exception_table(struct ex_regs *regs)
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 1/8] x86: change exit to abort again Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:31 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line Andrew Jones
` (5 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
x86/Makefile.common | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/x86/Makefile.common b/x86/Makefile.common
index ca8036722a535..298e5f721ff15 100644
--- a/x86/Makefile.common
+++ b/x86/Makefile.common
@@ -20,7 +20,7 @@ $(libcflat): CFLAGS += -ffreestanding -I lib
CFLAGS += -m$(bits)
CFLAGS += -O1
-# dump_stack.o relies on frame pointers.
+# stack.o relies on frame pointers.
KEEP_FRAME_POINTER := y
libgcc := $(shell $(CC) -m$(bits) --print-libgcc-file-name)
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 1/8] x86: change exit to abort again Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:31 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel Andrew Jones
` (4 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
It's sometimes useful to have the addresses for searching in a
disassembly. Anyway, there's no reason to drop it.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
scripts/pretty_print_stacks.py | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
index ee5a52edf07cd..11090deb122b1 100755
--- a/scripts/pretty_print_stacks.py
+++ b/scripts/pretty_print_stacks.py
@@ -67,8 +67,9 @@ def main():
if line == '':
break
+ puts(line)
+
if not line.strip().startswith('STACK:'):
- puts(line)
continue
try:
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
` (2 preceding siblings ...)
2016-04-19 14:28 ` [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 14:53 ` Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix Andrew Jones
` (3 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
x86 doesn't seem to care, but other architectures do.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
scripts/pretty_print_stacks.py | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
index 11090deb122b1..db2f145323c45 100755
--- a/scripts/pretty_print_stacks.py
+++ b/scripts/pretty_print_stacks.py
@@ -58,7 +58,7 @@ def main():
sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
sys.exit(1)
- binary = sys.argv[1]
+ binary = sys.argv[1].replace("flat", "elf")
try:
while True:
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
` (3 preceding siblings ...)
2016-04-19 14:28 ` [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:33 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 6/8] stack: share api prototypes Andrew Jones
` (2 subsequent siblings)
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
configure | 2 ++
scripts/pretty_print_stacks.py | 9 ++++++++-
2 files changed, 10 insertions(+), 1 deletion(-)
diff --git a/configure b/configure
index ba6c55b3bc5b3..ca0ef83b942e5 100755
--- a/configure
+++ b/configure
@@ -7,6 +7,7 @@ ld=ld
objcopy=objcopy
objdump=objdump
ar=ar
+addr2line=addr2line
arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
host=$arch
cross_prefix=
@@ -154,6 +155,7 @@ LD=$cross_prefix$ld
OBJCOPY=$cross_prefix$objcopy
OBJDUMP=$cross_prefix$objdump
AR=$cross_prefix$ar
+ADDR2LINE=$cross_prefix$addr2line
API=$api
TEST_DIR=$testdir
FIRMWARE=$firmware
diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
index db2f145323c45..336fc681e45b2 100755
--- a/scripts/pretty_print_stacks.py
+++ b/scripts/pretty_print_stacks.py
@@ -5,6 +5,8 @@ import subprocess
import sys
import traceback
+config = {}
+
# Subvert output buffering.
def puts(string):
sys.stdout.write(string)
@@ -25,7 +27,7 @@ def pretty_print_stack(binary, line):
# Output like this:
# 0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
# (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
- cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
+ cmd = [config["ADDR2LINE"], '-e', binary, '-i', '-f', '--pretty', '--address']
cmd.extend(addrs)
p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
@@ -60,6 +62,11 @@ def main():
binary = sys.argv[1].replace("flat", "elf")
+ with open("config.mak") as config_file:
+ for line in config_file:
+ name, val = line.partition("=")[::2]
+ config[name.strip()] = val.strip()
+
try:
while True:
# Subvert input buffering.
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 6/8] stack: share api prototypes
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
` (4 preceding siblings ...)
2016-04-19 14:28 ` [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:37 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support Andrew Jones
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
lib/stack.h | 9 +++++----
lib/x86/asm/stack.h | 3 ---
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/lib/stack.h b/lib/stack.h
index bb6b9aa70876d..cfc66f442f997 100644
--- a/lib/stack.h
+++ b/lib/stack.h
@@ -4,7 +4,10 @@
#include <libcflat.h>
#include <asm/stack.h>
-#ifndef HAVE_ARCH_BACKTRACE_FRAME
+#ifdef HAVE_ARCH_BACKTRACE_FRAME
+extern int backtrace_frame(const void *frame, const void **return_addrs,
+ int max_depth);
+#else
static inline int
backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
int max_depth __unused)
@@ -13,8 +16,6 @@ backtrace_frame(const void *frame __unused, const void **return_addrs __unused,
}
#endif
-#ifndef HAVE_ARCH_BACKTRACE
-int backtrace(const void **return_addrs, int max_depth);
-#endif
+extern int backtrace(const void **return_addrs, int max_depth);
#endif
diff --git a/lib/x86/asm/stack.h b/lib/x86/asm/stack.h
index fc4766d37b172..b14e2c0fa012e 100644
--- a/lib/x86/asm/stack.h
+++ b/lib/x86/asm/stack.h
@@ -6,9 +6,6 @@
#endif
#define HAVE_ARCH_BACKTRACE_FRAME
-int backtrace_frame(const void *frame, const void **return_addrs, int max_depth);
-
#define HAVE_ARCH_BACKTRACE
-int backtrace(const void **return_addrs, int max_depth);
#endif
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
` (5 preceding siblings ...)
2016-04-19 14:28 ` [kvm-unit-tests PATCH 6/8] stack: share api prototypes Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:38 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support Andrew Jones
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
lib/arm/asm/stack.h | 8 ++++++++
lib/arm64/asm/stack.h | 8 ++++++++
lib/powerpc/asm/stack.h | 8 ++++++++
lib/ppc64/asm/stack.h | 8 ++++++++
4 files changed, 32 insertions(+)
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
index e69de29bb2d1d..fe97ac5d4b7d9 100644
--- a/lib/arm/asm/stack.h
+++ b/lib/arm/asm/stack.h
@@ -0,0 +1,8 @@
+#ifndef _ASMARM_STACK_H_
+#define _ASMARM_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#endif
diff --git a/lib/arm64/asm/stack.h b/lib/arm64/asm/stack.h
index e69de29bb2d1d..d0006242fdaaf 100644
--- a/lib/arm64/asm/stack.h
+++ b/lib/arm64/asm/stack.h
@@ -0,0 +1,8 @@
+#ifndef _ASMARM64_STACK_H_
+#define _ASMARM64_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#endif
diff --git a/lib/powerpc/asm/stack.h b/lib/powerpc/asm/stack.h
index e69de29bb2d1d..e1c46ee092a10 100644
--- a/lib/powerpc/asm/stack.h
+++ b/lib/powerpc/asm/stack.h
@@ -0,0 +1,8 @@
+#ifndef _ASMPOWERPC_STACK_H_
+#define _ASMPOWERPC_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#endif
diff --git a/lib/ppc64/asm/stack.h b/lib/ppc64/asm/stack.h
index e69de29bb2d1d..9734bbb8f7bd9 100644
--- a/lib/ppc64/asm/stack.h
+++ b/lib/ppc64/asm/stack.h
@@ -0,0 +1,8 @@
+#ifndef _ASMPPC64_STACK_H_
+#define _ASMPPC64_STACK_H_
+
+#ifndef _STACK_H_
+#error Do not directly include <asm/stack.h>. Just use <stack.h>.
+#endif
+
+#endif
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
` (6 preceding siblings ...)
2016-04-19 14:28 ` [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches Andrew Jones
@ 2016-04-19 14:28 ` Andrew Jones
2016-04-19 15:41 ` Peter Feiner
7 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:28 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
This actually fixes the arm build too, as arm's __builtin_return_address
doesn't allow any input other than zero, and thus the common backtrace()
wasn't compiling.
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
arm/Makefile.arm | 4 ++++
lib/arm/asm/stack.h | 3 +++
lib/arm/processor.c | 1 +
lib/arm/stack.c | 41 +++++++++++++++++++++++++++++++++++++++++
4 files changed, 49 insertions(+)
create mode 100644 lib/arm/stack.c
diff --git a/arm/Makefile.arm b/arm/Makefile.arm
index 946422872532d..92f375700c45e 100644
--- a/arm/Makefile.arm
+++ b/arm/Makefile.arm
@@ -8,12 +8,16 @@ ldarch = elf32-littlearm
kernel_offset = 0x10000
machine = -marm
+# stack.o relies on frame pointers.
+KEEP_FRAME_POINTER := y
+
CFLAGS += $(machine)
CFLAGS += -mcpu=$(PROCESSOR)
cstart.o = $(TEST_DIR)/cstart.o
cflatobjs += lib/arm/spinlock.o
cflatobjs += lib/arm/processor.o
+cflatobjs += lib/arm/stack.o
# arm specific tests
tests =
diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
index fe97ac5d4b7d9..ebcedc2ef938c 100644
--- a/lib/arm/asm/stack.h
+++ b/lib/arm/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/arm/processor.c b/lib/arm/processor.c
index 1cef46ab28647..54fdb87ef0196 100644
--- a/lib/arm/processor.c
+++ b/lib/arm/processor.c
@@ -108,6 +108,7 @@ void do_handle_exception(enum vector v, struct pt_regs *regs)
asm volatile("mrc p15, 0, %0, c5, c0, 1": "=r" (fsr));
printf("IFAR: %08lx IFSR: %08lx\n", far, fsr);
}
+ dump_frame_stack((void *)regs->ARM_pc, (void *)regs->ARM_fp);
abort();
}
diff --git a/lib/arm/stack.c b/lib/arm/stack.c
new file mode 100644
index 0000000000000..f58b731f277bf
--- /dev/null
+++ b/lib/arm/stack.c
@@ -0,0 +1,41 @@
+/*
+ * backtrace support (this is a modified lib/x86/stack.c)
+ *
+ * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.
+ */
+#include <libcflat.h>
+#include <stack.h>
+
+int backtrace_frame(const void *frame, const void **return_addrs,
+ int max_depth)
+{
+ static int walking;
+ int depth = 0;
+ const unsigned long *fp = (unsigned long *)frame;
+
+ if (walking) {
+ printf("RECURSIVE STACK WALK!!!\n");
+ return 0;
+ }
+ walking = 1;
+
+ for (depth = 0; depth < max_depth; depth++) {
+ if (!fp)
+ break;
+ return_addrs[depth] = (void *)fp[0];
+ if (return_addrs[depth] == 0)
+ break;
+ fp = (unsigned long *)fp[-1];
+ }
+
+ walking = 0;
+ return depth;
+}
+
+int backtrace(const void **return_addrs, int max_depth)
+{
+ return backtrace_frame(__builtin_frame_address(0),
+ return_addrs, max_depth);
+}
--
2.4.11
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel
2016-04-19 14:28 ` [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel Andrew Jones
@ 2016-04-19 14:53 ` Andrew Jones
2016-04-19 15:41 ` Peter Feiner
0 siblings, 1 reply; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 14:53 UTC (permalink / raw)
To: kvm; +Cc: pfeiner, pbonzini, rkrcmar
On Tue, Apr 19, 2016 at 04:28:06PM +0200, Andrew Jones wrote:
> x86 doesn't seem to care, but other architectures do.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> scripts/pretty_print_stacks.py | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> index 11090deb122b1..db2f145323c45 100755
> --- a/scripts/pretty_print_stacks.py
> +++ b/scripts/pretty_print_stacks.py
> @@ -58,7 +58,7 @@ def main():
> sys.stderr.write('usage: %s <kernel>\n' % sys.argv[0])
> sys.exit(1)
>
> - binary = sys.argv[1]
> + binary = sys.argv[1].replace("flat", "elf")
grr... I should have used (".flat", ".elf") since the string 'flat'
could easily show up in other parts of a test name.
>
> try:
> while True:
> --
> 2.4.11
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 1/8] x86: change exit to abort again
2016-04-19 14:28 ` [kvm-unit-tests PATCH 1/8] x86: change exit to abort again Andrew Jones
@ 2016-04-19 15:30 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:30 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> This is needed again because ed0a50e5 "x86: lib: debug dump
> on unhandled exceptions" must have missed it in a rebase.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o
2016-04-19 14:28 ` [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o Andrew Jones
@ 2016-04-19 15:31 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:31 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line
2016-04-19 14:28 ` [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line Andrew Jones
@ 2016-04-19 15:31 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:31 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> It's sometimes useful to have the addresses for searching in a
> disassembly. Anyway, there's no reason to drop it.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix
2016-04-19 14:28 ` [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix Andrew Jones
@ 2016-04-19 15:33 ` Peter Feiner
2016-04-19 15:45 ` Andrew Jones
0 siblings, 1 reply; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:33 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> configure | 2 ++
> scripts/pretty_print_stacks.py | 9 ++++++++-
> 2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index ba6c55b3bc5b3..ca0ef83b942e5 100755
> --- a/configure
> +++ b/configure
> @@ -7,6 +7,7 @@ ld=ld
> objcopy=objcopy
> objdump=objdump
> ar=ar
> +addr2line=addr2line
> arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
> host=$arch
> cross_prefix=
> @@ -154,6 +155,7 @@ LD=$cross_prefix$ld
> OBJCOPY=$cross_prefix$objcopy
> OBJDUMP=$cross_prefix$objdump
> AR=$cross_prefix$ar
> +ADDR2LINE=$cross_prefix$addr2line
> API=$api
> TEST_DIR=$testdir
> FIRMWARE=$firmware
> diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> index db2f145323c45..336fc681e45b2 100755
> --- a/scripts/pretty_print_stacks.py
> +++ b/scripts/pretty_print_stacks.py
> @@ -5,6 +5,8 @@ import subprocess
> import sys
> import traceback
>
> +config = {}
> +
> # Subvert output buffering.
> def puts(string):
> sys.stdout.write(string)
> @@ -25,7 +27,7 @@ def pretty_print_stack(binary, line):
> # Output like this:
> # 0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
> # (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
> - cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
> + cmd = [config["ADDR2LINE"], '-e', binary, '-i', '-f', '--pretty', '--address']
Perhaps config.get('ADDR2LINE', 'addr2line') so my old config still works?
> cmd.extend(addrs)
>
> p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> @@ -60,6 +62,11 @@ def main():
>
> binary = sys.argv[1].replace("flat", "elf")
>
> + with open("config.mak") as config_file:
> + for line in config_file:
> + name, val = line.partition("=")[::2]
Very elegant trick here! I always do name, _, val = line.partition("=")!
> + config[name.strip()] = val.strip()
> +
> try:
> while True:
> # Subvert input buffering.
> --
> 2.4.11
>
Otherwise, reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 6/8] stack: share api prototypes
2016-04-19 14:28 ` [kvm-unit-tests PATCH 6/8] stack: share api prototypes Andrew Jones
@ 2016-04-19 15:37 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:37 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches
2016-04-19 14:28 ` [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches Andrew Jones
@ 2016-04-19 15:38 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:38 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> Signed-off-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support
2016-04-19 14:28 ` [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support Andrew Jones
@ 2016-04-19 15:41 ` Peter Feiner
2016-04-19 15:55 ` Andrew Jones
0 siblings, 1 reply; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:41 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> This actually fixes the arm build too, as arm's __builtin_return_address
> doesn't allow any input other than zero, and thus the common backtrace()
> wasn't compiling.
>
> Signed-off-by: Andrew Jones <drjones@redhat.com>
> ---
> arm/Makefile.arm | 4 ++++
> lib/arm/asm/stack.h | 3 +++
> lib/arm/processor.c | 1 +
> lib/arm/stack.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 49 insertions(+)
> create mode 100644 lib/arm/stack.c
>
> diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> index 946422872532d..92f375700c45e 100644
> --- a/arm/Makefile.arm
> +++ b/arm/Makefile.arm
> @@ -8,12 +8,16 @@ ldarch = elf32-littlearm
> kernel_offset = 0x10000
> machine = -marm
>
> +# stack.o relies on frame pointers.
> +KEEP_FRAME_POINTER := y
> +
> CFLAGS += $(machine)
> CFLAGS += -mcpu=$(PROCESSOR)
>
> cstart.o = $(TEST_DIR)/cstart.o
> cflatobjs += lib/arm/spinlock.o
> cflatobjs += lib/arm/processor.o
> +cflatobjs += lib/arm/stack.o
>
> # arm specific tests
> tests =
> diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
> index fe97ac5d4b7d9..ebcedc2ef938c 100644
> --- a/lib/arm/asm/stack.h
> +++ b/lib/arm/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/arm/processor.c b/lib/arm/processor.c
> index 1cef46ab28647..54fdb87ef0196 100644
> --- a/lib/arm/processor.c
> +++ b/lib/arm/processor.c
> @@ -108,6 +108,7 @@ void do_handle_exception(enum vector v, struct pt_regs *regs)
> asm volatile("mrc p15, 0, %0, c5, c0, 1": "=r" (fsr));
> printf("IFAR: %08lx IFSR: %08lx\n", far, fsr);
> }
> + dump_frame_stack((void *)regs->ARM_pc, (void *)regs->ARM_fp);
> abort();
> }
>
> diff --git a/lib/arm/stack.c b/lib/arm/stack.c
> new file mode 100644
> index 0000000000000..f58b731f277bf
> --- /dev/null
> +++ b/lib/arm/stack.c
> @@ -0,0 +1,41 @@
> +/*
> + * backtrace support (this is a modified lib/x86/stack.c)
> + *
> + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> + *
> + * This work is licensed under the terms of the GNU LGPL, version 2.
> + */
> +#include <libcflat.h>
> +#include <stack.h>
> +
> +int backtrace_frame(const void *frame, const void **return_addrs,
> + int max_depth)
> +{
> + static int walking;
> + int depth = 0;
Nit: unnecessary initialization.
> + const unsigned long *fp = (unsigned long *)frame;
> +
> + if (walking) {
> + printf("RECURSIVE STACK WALK!!!\n");
> + return 0;
> + }
> + walking = 1;
> +
> + for (depth = 0; depth < max_depth; depth++) {
> + if (!fp)
Interesting difference in termination case here from x86. Instead of
checking for a null return address, you check for a null frame
address. Any particular reason why?
> + break;
> + return_addrs[depth] = (void *)fp[0];
> + if (return_addrs[depth] == 0)
> + break;
> + fp = (unsigned long *)fp[-1];
> + }
> +
> + walking = 0;
> + return depth;
> +}
> +
> +int backtrace(const void **return_addrs, int max_depth)
> +{
> + return backtrace_frame(__builtin_frame_address(0),
> + return_addrs, max_depth);
> +}
> --
> 2.4.11
>
Nice fix!
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel
2016-04-19 14:53 ` Andrew Jones
@ 2016-04-19 15:41 ` Peter Feiner
0 siblings, 0 replies; 20+ messages in thread
From: Peter Feiner @ 2016-04-19 15:41 UTC (permalink / raw)
To: Andrew Jones; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 7:53 AM, Andrew Jones <drjones@redhat.com> wrote:
> On Tue, Apr 19, 2016 at 04:28:06PM +0200, Andrew Jones wrote:
>> x86 doesn't seem to care, but other architectures do.
>>
>> Signed-off-by: Andrew Jones <drjones@redhat.com>
With "." fix,
Reviewed-by: Peter Feiner <pfeiner@google.com>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix
2016-04-19 15:33 ` Peter Feiner
@ 2016-04-19 15:45 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 15:45 UTC (permalink / raw)
To: Peter Feiner; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 08:33:26AM -0700, Peter Feiner wrote:
> On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > configure | 2 ++
> > scripts/pretty_print_stacks.py | 9 ++++++++-
> > 2 files changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/configure b/configure
> > index ba6c55b3bc5b3..ca0ef83b942e5 100755
> > --- a/configure
> > +++ b/configure
> > @@ -7,6 +7,7 @@ ld=ld
> > objcopy=objcopy
> > objdump=objdump
> > ar=ar
> > +addr2line=addr2line
> > arch=`uname -m | sed -e 's/i.86/i386/;s/arm.*/arm/;s/ppc64.*/ppc64/'`
> > host=$arch
> > cross_prefix=
> > @@ -154,6 +155,7 @@ LD=$cross_prefix$ld
> > OBJCOPY=$cross_prefix$objcopy
> > OBJDUMP=$cross_prefix$objdump
> > AR=$cross_prefix$ar
> > +ADDR2LINE=$cross_prefix$addr2line
> > API=$api
> > TEST_DIR=$testdir
> > FIRMWARE=$firmware
> > diff --git a/scripts/pretty_print_stacks.py b/scripts/pretty_print_stacks.py
> > index db2f145323c45..336fc681e45b2 100755
> > --- a/scripts/pretty_print_stacks.py
> > +++ b/scripts/pretty_print_stacks.py
> > @@ -5,6 +5,8 @@ import subprocess
> > import sys
> > import traceback
> >
> > +config = {}
> > +
> > # Subvert output buffering.
> > def puts(string):
> > sys.stdout.write(string)
> > @@ -25,7 +27,7 @@ def pretty_print_stack(binary, line):
> > # Output like this:
> > # 0x004002be: start64 at path/to/kvm-unit-tests/x86/cstart64.S:208
> > # (inlined by) test_ept_violation at path/to/kvm-unit-tests/x86/vmx_tests.c:1719 (discriminator 1)
> > - cmd = ['addr2line', '-e', binary, '-i', '-f', '--pretty', '--address']
> > + cmd = [config["ADDR2LINE"], '-e', binary, '-i', '-f', '--pretty', '--address']
>
> Perhaps config.get('ADDR2LINE', 'addr2line') so my old config still works?
OK, will do in v2.
>
> > cmd.extend(addrs)
> >
> > p = subprocess.Popen(cmd, stdout=subprocess.PIPE)
> > @@ -60,6 +62,11 @@ def main():
> >
> > binary = sys.argv[1].replace("flat", "elf")
> >
> > + with open("config.mak") as config_file:
> > + for line in config_file:
> > + name, val = line.partition("=")[::2]
>
> Very elegant trick here! I always do name, _, val = line.partition("=")!
>
> > + config[name.strip()] = val.strip()
> > +
> > try:
> > while True:
> > # Subvert input buffering.
> > --
> > 2.4.11
> >
>
> Otherwise, reviewed-by: Peter Feiner <pfeiner@google.com>
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support
2016-04-19 15:41 ` Peter Feiner
@ 2016-04-19 15:55 ` Andrew Jones
0 siblings, 0 replies; 20+ messages in thread
From: Andrew Jones @ 2016-04-19 15:55 UTC (permalink / raw)
To: Peter Feiner; +Cc: kvm, Paolo Bonzini, rkrcmar
On Tue, Apr 19, 2016 at 08:41:02AM -0700, Peter Feiner wrote:
> On Tue, Apr 19, 2016 at 7:28 AM, Andrew Jones <drjones@redhat.com> wrote:
> > This actually fixes the arm build too, as arm's __builtin_return_address
> > doesn't allow any input other than zero, and thus the common backtrace()
> > wasn't compiling.
> >
> > Signed-off-by: Andrew Jones <drjones@redhat.com>
> > ---
> > arm/Makefile.arm | 4 ++++
> > lib/arm/asm/stack.h | 3 +++
> > lib/arm/processor.c | 1 +
> > lib/arm/stack.c | 41 +++++++++++++++++++++++++++++++++++++++++
> > 4 files changed, 49 insertions(+)
> > create mode 100644 lib/arm/stack.c
> >
> > diff --git a/arm/Makefile.arm b/arm/Makefile.arm
> > index 946422872532d..92f375700c45e 100644
> > --- a/arm/Makefile.arm
> > +++ b/arm/Makefile.arm
> > @@ -8,12 +8,16 @@ ldarch = elf32-littlearm
> > kernel_offset = 0x10000
> > machine = -marm
> >
> > +# stack.o relies on frame pointers.
> > +KEEP_FRAME_POINTER := y
> > +
> > CFLAGS += $(machine)
> > CFLAGS += -mcpu=$(PROCESSOR)
> >
> > cstart.o = $(TEST_DIR)/cstart.o
> > cflatobjs += lib/arm/spinlock.o
> > cflatobjs += lib/arm/processor.o
> > +cflatobjs += lib/arm/stack.o
> >
> > # arm specific tests
> > tests =
> > diff --git a/lib/arm/asm/stack.h b/lib/arm/asm/stack.h
> > index fe97ac5d4b7d9..ebcedc2ef938c 100644
> > --- a/lib/arm/asm/stack.h
> > +++ b/lib/arm/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/arm/processor.c b/lib/arm/processor.c
> > index 1cef46ab28647..54fdb87ef0196 100644
> > --- a/lib/arm/processor.c
> > +++ b/lib/arm/processor.c
> > @@ -108,6 +108,7 @@ void do_handle_exception(enum vector v, struct pt_regs *regs)
> > asm volatile("mrc p15, 0, %0, c5, c0, 1": "=r" (fsr));
> > printf("IFAR: %08lx IFSR: %08lx\n", far, fsr);
> > }
> > + dump_frame_stack((void *)regs->ARM_pc, (void *)regs->ARM_fp);
> > abort();
> > }
> >
> > diff --git a/lib/arm/stack.c b/lib/arm/stack.c
> > new file mode 100644
> > index 0000000000000..f58b731f277bf
> > --- /dev/null
> > +++ b/lib/arm/stack.c
> > @@ -0,0 +1,41 @@
> > +/*
> > + * backtrace support (this is a modified lib/x86/stack.c)
> > + *
> > + * Copyright (C) 2016, Red Hat Inc, Andrew Jones <drjones@redhat.com>
> > + *
> > + * This work is licensed under the terms of the GNU LGPL, version 2.
> > + */
> > +#include <libcflat.h>
> > +#include <stack.h>
> > +
> > +int backtrace_frame(const void *frame, const void **return_addrs,
> > + int max_depth)
> > +{
> > + static int walking;
> > + int depth = 0;
>
> Nit: unnecessary initialization.
/me catches the nit and flings it over to lib/x86/stack.c :-)
I'll fix for v2.
>
> > + const unsigned long *fp = (unsigned long *)frame;
> > +
> > + if (walking) {
> > + printf("RECURSIVE STACK WALK!!!\n");
> > + return 0;
> > + }
> > + walking = 1;
> > +
> > + for (depth = 0; depth < max_depth; depth++) {
> > + if (!fp)
>
> Interesting difference in termination case here from x86. Instead of
> checking for a null return address, you check for a null frame
> address. Any particular reason why?
When I configured a unit test that had more than one vcpu, then
if I dereferenced a null frame, address zero, I actually got
non-zero junk. Then fp was getting updated to fffffffc, and when
I dereferenced that (an unmapped address) it led to an exception.
So I just threw this test in to make sure that can't happen. It'll
also keep me safe in case regs->ARM_fp is ever null in
do_handle_exception.
>
> > + break;
> > + return_addrs[depth] = (void *)fp[0];
> > + if (return_addrs[depth] == 0)
> > + break;
> > + fp = (unsigned long *)fp[-1];
> > + }
> > +
> > + walking = 0;
> > + return depth;
> > +}
> > +
> > +int backtrace(const void **return_addrs, int max_depth)
> > +{
> > + return backtrace_frame(__builtin_frame_address(0),
> > + return_addrs, max_depth);
> > +}
> > --
> > 2.4.11
> >
>
> Nice fix!
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2016-04-19 15:55 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-19 14:28 [kvm-unit-tests PATCH 0/8] arm: fix building by adding a feature Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 1/8] x86: change exit to abort again Andrew Jones
2016-04-19 15:30 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 2/8] x86: trivial: there's no dump_stack.o Andrew Jones
2016-04-19 15:31 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 3/8] pretty_print_stacks: keep the 'STACK:' line Andrew Jones
2016-04-19 15:31 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 4/8] pretty_print_stacks: use elf file for the kernel Andrew Jones
2016-04-19 14:53 ` Andrew Jones
2016-04-19 15:41 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 5/8] pretty_print_stacks: addr2line may need a cross prefix Andrew Jones
2016-04-19 15:33 ` Peter Feiner
2016-04-19 15:45 ` Andrew Jones
2016-04-19 14:28 ` [kvm-unit-tests PATCH 6/8] stack: share api prototypes Andrew Jones
2016-04-19 15:37 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 7/8] stack: copy common asm/stack.h bits to all arches Andrew Jones
2016-04-19 15:38 ` Peter Feiner
2016-04-19 14:28 ` [kvm-unit-tests PATCH 8/8] arm: stack: add dump_stack support Andrew Jones
2016-04-19 15:41 ` Peter Feiner
2016-04-19 15:55 ` Andrew Jones
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.