linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/8] perf tools: kaslr fixes
@ 2014-01-24 15:10 Adrian Hunter
  2014-01-24 15:10 ` [PATCH 1/8] perf tools: Fix symbol annotation for relocated kernel Adrian Hunter
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Hi

Here are some patches that improve perf tools
handling of relocation.

This has become an issue as mentioned in this
thread:

	http://marc.info/?l=linux-kernel&m=139030004314756


It looks like the changes I made to vmlinux and
module mapping to allow object code to be read
broke the symbol annotation for relocated kernels.
The first patch fixes that.

There were some other issues though:
	- ref_reloc_sym was not always set up
	- mustn't use kcore if the kernel has moved
	- kallsyms is not unique for a given
	buildid

These things need more testing but I thought it was
worth getting comments now.


Adrian Hunter (8):
      perf tools: Fix symbol annotation for relocated kernel
      perf tools: Add kallsyms__get_function_start()
      perf tools: Add machine__get_kallsyms_filename()
      perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()
      perf report: Get ref_reloc_sym from kernel map
      perf tools: Prevent the use of kcore if the kernel has moved
      perf tools: test does not need to set up ref_reloc_sym
      perf tools: Adjust kallsyms for relocated kernel

 tools/perf/builtin-record.c         | 10 ++----
 tools/perf/tests/vmlinux-kallsyms.c | 10 ------
 tools/perf/util/event.c             | 36 ++++++++++----------
 tools/perf/util/event.h             |  6 ++--
 tools/perf/util/machine.c           | 40 ++++++++++++++++++-----
 tools/perf/util/map.c               |  5 +--
 tools/perf/util/map.h               |  1 +
 tools/perf/util/symbol-elf.c        |  2 ++
 tools/perf/util/symbol.c            | 65 +++++++++++++++++++++++++++++++++----
 9 files changed, 120 insertions(+), 55 deletions(-)


Regards
Adrian

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

* [PATCH 1/8] perf tools: Fix symbol annotation for relocated kernel
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 2/8] perf tools: Add kallsyms__get_function_start() Adrian Hunter
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Kernel maps map memory addresses to file offsets.
For symbol annotation, objdump needs the object VMA
addresses.  For an unrelocated kernel, that is the
same as the memory address.

The addresses passed to objdump for symbol annotation
did not take into account kernel relocation.  This
patch fixes that.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/map.c        | 5 +++--
 tools/perf/util/map.h        | 1 +
 tools/perf/util/symbol-elf.c | 2 ++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c
index ee1dd68..b46f527 100644
--- a/tools/perf/util/map.c
+++ b/tools/perf/util/map.c
@@ -39,6 +39,7 @@ void map__init(struct map *map, enum map_type type,
 	map->start    = start;
 	map->end      = end;
 	map->pgoff    = pgoff;
+	map->reloc    = 0;
 	map->dso      = dso;
 	map->map_ip   = map__map_ip;
 	map->unmap_ip = map__unmap_ip;
@@ -288,7 +289,7 @@ u64 map__rip_2objdump(struct map *map, u64 rip)
 	if (map->dso->rel)
 		return rip - map->pgoff;
 
-	return map->unmap_ip(map, rip);
+	return map->unmap_ip(map, rip) - map->reloc;
 }
 
 /**
@@ -311,7 +312,7 @@ u64 map__objdump_2mem(struct map *map, u64 ip)
 	if (map->dso->rel)
 		return map->unmap_ip(map, ip + map->pgoff);
 
-	return ip;
+	return ip + map->reloc;
 }
 
 void map_groups__init(struct map_groups *mg)
diff --git a/tools/perf/util/map.h b/tools/perf/util/map.h
index 18068c6..257e513 100644
--- a/tools/perf/util/map.h
+++ b/tools/perf/util/map.h
@@ -36,6 +36,7 @@ struct map {
 	bool			erange_warned;
 	u32			priv;
 	u64			pgoff;
+	u64			reloc;
 	u32			maj, min; /* only valid for MMAP2 record */
 	u64			ino;      /* only valid for MMAP2 record */
 	u64			ino_generation;/* only valid for MMAP2 record */
diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
index 7594567..8ce52da 100644
--- a/tools/perf/util/symbol-elf.c
+++ b/tools/perf/util/symbol-elf.c
@@ -751,6 +751,8 @@ int dso__load_sym(struct dso *dso, struct map *map,
 			if (strcmp(elf_name, kmap->ref_reloc_sym->name))
 				continue;
 			kmap->ref_reloc_sym->unrelocated_addr = sym.st_value;
+			map->reloc = kmap->ref_reloc_sym->addr -
+				     kmap->ref_reloc_sym->unrelocated_addr;
 			break;
 		}
 	}
-- 
1.7.11.7


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

* [PATCH 2/8] perf tools: Add kallsyms__get_function_start()
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
  2014-01-24 15:10 ` [PATCH 1/8] perf tools: Fix symbol annotation for relocated kernel Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 3/8] perf tools: Add machine__get_kallsyms_filename() Adrian Hunter
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Separate out the logic used to find the start
address of the reference symbol used to track
kernel relocation.  kallsyms__get_function_start()
is used in subsequent patches.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/event.c | 18 +++++++++++++++---
 tools/perf/util/event.h |  3 +++
 2 files changed, 18 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 1fc1c2f..17476df 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -470,6 +470,17 @@ static int find_symbol_cb(void *arg, const char *name, char type,
 	return 1;
 }
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name)
+{
+	struct process_symbol_args args = { .name = symbol_name, };
+
+	if (kallsyms__parse(kallsyms_filename, &args, find_symbol_cb) <= 0)
+		return 0;
+
+	return args.start;
+}
+
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
 				       struct machine *machine,
@@ -480,13 +491,13 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	char path[PATH_MAX];
 	char name_buff[PATH_MAX];
 	struct map *map;
+	u64 start;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
 	 * available use this, and after it is use this as a fallback for older
 	 * kernels.
 	 */
-	struct process_symbol_args args = { .name = symbol_name, };
 	union perf_event *event = zalloc((sizeof(event->mmap) +
 					  machine->id_hdr_size));
 	if (event == NULL) {
@@ -513,7 +524,8 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		}
 	}
 
-	if (kallsyms__parse(filename, &args, find_symbol_cb) <= 0) {
+	start = kallsyms__get_function_start(filename, symbol_name);
+	if (!start) {
 		free(event);
 		return -ENOENT;
 	}
@@ -525,7 +537,7 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = args.start;
+	event->mmap.pgoff = start;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index faf6e21..66a0c03 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -279,4 +279,7 @@ size_t perf_event__fprintf_mmap2(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf_task(union perf_event *event, FILE *fp);
 size_t perf_event__fprintf(union perf_event *event, FILE *fp);
 
+u64 kallsyms__get_function_start(const char *kallsyms_filename,
+				 const char *symbol_name);
+
 #endif /* __PERF_RECORD_H */
-- 
1.7.11.7


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

* [PATCH 3/8] perf tools: Add machine__get_kallsyms_filename()
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
  2014-01-24 15:10 ` [PATCH 1/8] perf tools: Fix symbol annotation for relocated kernel Adrian Hunter
  2014-01-24 15:10 ` [PATCH 2/8] perf tools: Add kallsyms__get_function_start() Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 4/8] perf tools: Set up ref_reloc_sym in machine__create_kernel_maps() Adrian Hunter
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Separate out the logic used to make the
kallsyms full path name for a machine.
It will be reused in a subsequent patch.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/machine.c | 19 +++++++++++--------
 1 file changed, 11 insertions(+), 8 deletions(-)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index ded7459..290c2e6 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -496,19 +496,22 @@ static int symbol__in_kernel(void *arg, const char *name,
 	return 1;
 }
 
+static void machine__get_kallsyms_filename(struct machine *machine, char *buf,
+					   size_t bufsz)
+{
+	if (machine__is_default_guest(machine))
+		scnprintf(buf, bufsz, "%s", symbol_conf.default_guest_kallsyms);
+	else
+		scnprintf(buf, bufsz, "%s/proc/kallsyms", machine->root_dir);
+}
+
 /* Figure out the start address of kernel map from /proc/kallsyms */
 static u64 machine__get_kernel_start_addr(struct machine *machine)
 {
-	const char *filename;
-	char path[PATH_MAX];
+	char filename[PATH_MAX];
 	struct process_args args;
 
-	if (machine__is_default_guest(machine))
-		filename = (char *)symbol_conf.default_guest_kallsyms;
-	else {
-		sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-		filename = path;
-	}
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
 
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return 0;
-- 
1.7.11.7


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

* [PATCH 4/8] perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (2 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 3/8] perf tools: Add machine__get_kallsyms_filename() Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 5/8] perf report: Get ref_reloc_sym from kernel map Adrian Hunter
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

The ref_reloc_sym is always needed for the
kernel map in order to check for relocation.
Consequently set it up when the kernel map is
created.  Otherwise it was only beging set up
by 'perf record'.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/machine.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
index 290c2e6..58a4e92 100644
--- a/tools/perf/util/machine.c
+++ b/tools/perf/util/machine.c
@@ -835,6 +835,20 @@ static int machine__create_modules(struct machine *machine)
 int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
+	char filename[PATH_MAX];
+	const char *name;
+	u64 addr;
+
+	machine__get_kallsyms_filename(machine, filename, PATH_MAX);
+
+	name = "_text";
+	addr = kallsyms__get_function_start(filename, name);
+	if (!addr) {
+		name = "_stext";
+		addr = kallsyms__get_function_start("/proc/kallsyms", name);
+		if (!addr)
+			return -1;
+	}
 
 	if (kernel == NULL ||
 	    __machine__create_kernel_maps(machine, kernel) < 0)
@@ -853,6 +867,13 @@ int machine__create_kernel_maps(struct machine *machine)
 	 * Now that we have all the maps created, just set the ->end of them:
 	 */
 	map_groups__fixup_end(&machine->kmaps);
+
+	if (maps__set_kallsyms_ref_reloc_sym(machine->vmlinux_maps, name,
+					     addr)) {
+		machine__destroy_kernel_maps(machine);
+		return -1;
+	}
+
 	return 0;
 }
 
-- 
1.7.11.7


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

* [PATCH 5/8] perf report: Get ref_reloc_sym from kernel map
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (3 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 4/8] perf tools: Set up ref_reloc_sym in machine__create_kernel_maps() Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 6/8] perf tools: Prevent the use of kcore if the kernel has moved Adrian Hunter
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Now that ref_reloc_sym is set up when the kernel
map is created, 'perf report' does not need to
pass the symbol names to
perf_event__synthesize_kernel_mmap() which can
read the values needed from ref_reloc_sym directly.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/builtin-record.c | 10 ++--------
 tools/perf/util/event.c     | 26 ++++++--------------------
 tools/perf/util/event.h     |  3 +--
 3 files changed, 9 insertions(+), 30 deletions(-)

diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
index 3c394bf..af47531 100644
--- a/tools/perf/builtin-record.c
+++ b/tools/perf/builtin-record.c
@@ -287,10 +287,7 @@ static void perf_event__synthesize_guest_os(struct machine *machine, void *data)
 	 * have no _text sometimes.
 	 */
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record guest kernel [%d]'s reference"
 		       " relocation symbol.\n", machine->pid);
@@ -457,10 +454,7 @@ static int __cmd_record(struct record *rec, int argc, const char **argv)
 	}
 
 	err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-						 machine, "_text");
-	if (err < 0)
-		err = perf_event__synthesize_kernel_mmap(tool, process_synthesized_event,
-							 machine, "_stext");
+						 machine);
 	if (err < 0)
 		pr_err("Couldn't record kernel reference relocation symbol\n"
 		       "Symbol resolution may be skewed if relocation was used (e.g. kexec).\n"
diff --git a/tools/perf/util/event.c b/tools/perf/util/event.c
index 17476df..b0f3ca8 100644
--- a/tools/perf/util/event.c
+++ b/tools/perf/util/event.c
@@ -483,15 +483,13 @@ u64 kallsyms__get_function_start(const char *kallsyms_filename,
 
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name)
+				       struct machine *machine)
 {
 	size_t size;
-	const char *filename, *mmap_name;
-	char path[PATH_MAX];
+	const char *mmap_name;
 	char name_buff[PATH_MAX];
 	struct map *map;
-	u64 start;
+	struct kmap *kmap;
 	int err;
 	/*
 	 * We should get this from /sys/kernel/sections/.text, but till that is
@@ -513,31 +511,19 @@ int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 		 * see kernel/perf_event.c __perf_event_mmap
 		 */
 		event->header.misc = PERF_RECORD_MISC_KERNEL;
-		filename = "/proc/kallsyms";
 	} else {
 		event->header.misc = PERF_RECORD_MISC_GUEST_KERNEL;
-		if (machine__is_default_guest(machine))
-			filename = (char *) symbol_conf.default_guest_kallsyms;
-		else {
-			sprintf(path, "%s/proc/kallsyms", machine->root_dir);
-			filename = path;
-		}
-	}
-
-	start = kallsyms__get_function_start(filename, symbol_name);
-	if (!start) {
-		free(event);
-		return -ENOENT;
 	}
 
 	map = machine->vmlinux_maps[MAP__FUNCTION];
+	kmap = map__kmap(map);
 	size = snprintf(event->mmap.filename, sizeof(event->mmap.filename),
-			"%s%s", mmap_name, symbol_name) + 1;
+			"%s%s", mmap_name, kmap->ref_reloc_sym->name) + 1;
 	size = PERF_ALIGN(size, sizeof(u64));
 	event->mmap.header.type = PERF_RECORD_MMAP;
 	event->mmap.header.size = (sizeof(event->mmap) -
 			(sizeof(event->mmap.filename) - size) + machine->id_hdr_size);
-	event->mmap.pgoff = start;
+	event->mmap.pgoff = kmap->ref_reloc_sym->addr;
 	event->mmap.start = map->start;
 	event->mmap.len   = map->end - event->mmap.start;
 	event->mmap.pid   = machine->pid;
diff --git a/tools/perf/util/event.h b/tools/perf/util/event.h
index 66a0c03..851fa06 100644
--- a/tools/perf/util/event.h
+++ b/tools/perf/util/event.h
@@ -214,8 +214,7 @@ int perf_event__synthesize_threads(struct perf_tool *tool,
 				   struct machine *machine, bool mmap_data);
 int perf_event__synthesize_kernel_mmap(struct perf_tool *tool,
 				       perf_event__handler_t process,
-				       struct machine *machine,
-				       const char *symbol_name);
+				       struct machine *machine);
 
 int perf_event__synthesize_modules(struct perf_tool *tool,
 				   perf_event__handler_t process,
-- 
1.7.11.7


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

* [PATCH 6/8] perf tools: Prevent the use of kcore if the kernel has moved
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (4 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 5/8] perf report: Get ref_reloc_sym from kernel map Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 7/8] perf tools: test does not need to set up ref_reloc_sym Adrian Hunter
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Use of kcore is predicated upon it matching
the recorded data.  If the kernel has been
relocated at boot time (i.e. since the data
was recorded) then do not use kcore.

Note that this is possible to make a copy
of kcore at the time the data is recorded
using 'perf buildid-cache'.  Then perf tools
will use the copy because it does match the
data.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 39ce9ad..4ac1f87 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -976,6 +976,23 @@ static int validate_kcore_modules(const char *kallsyms_filename,
 	return 0;
 }
 
+static int validate_kcore_addresses(const char *kallsyms_filename,
+				    struct map *map)
+{
+	struct kmap *kmap = map__kmap(map);
+
+	if (kmap->ref_reloc_sym && kmap->ref_reloc_sym->name) {
+		u64 start;
+
+		start = kallsyms__get_function_start(kallsyms_filename,
+						     kmap->ref_reloc_sym->name);
+		if (start != kmap->ref_reloc_sym->addr)
+			return -EINVAL;
+	}
+
+	return validate_kcore_modules(kallsyms_filename, map);
+}
+
 struct kcore_mapfn_data {
 	struct dso *dso;
 	enum map_type type;
@@ -1019,8 +1036,8 @@ static int dso__load_kcore(struct dso *dso, struct map *map,
 					     kallsyms_filename))
 		return -EINVAL;
 
-	/* All modules must be present at their original addresses */
-	if (validate_kcore_modules(kallsyms_filename, map))
+	/* Modules and kernel must be present at their original addresses */
+	if (validate_kcore_addresses(kallsyms_filename, map))
 		return -EINVAL;
 
 	md.dso = dso;
@@ -1424,7 +1441,7 @@ static int find_matching_kcore(struct map *map, char *dir, size_t dir_sz)
 			continue;
 		scnprintf(kallsyms_filename, sizeof(kallsyms_filename),
 			  "%s/%s/kallsyms", dir, dent->d_name);
-		if (!validate_kcore_modules(kallsyms_filename, map)) {
+		if (!validate_kcore_addresses(kallsyms_filename, map)) {
 			strlcpy(dir, kallsyms_filename, dir_sz);
 			ret = 0;
 			break;
@@ -1479,7 +1496,7 @@ static char *dso__find_kallsyms(struct dso *dso, struct map *map)
 		if (fd != -1) {
 			close(fd);
 			/* If module maps match go with /proc/kallsyms */
-			if (!validate_kcore_modules("/proc/kallsyms", map))
+			if (!validate_kcore_addresses("/proc/kallsyms", map))
 				goto proc_kallsyms;
 		}
 
-- 
1.7.11.7


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

* [PATCH 7/8] perf tools: test does not need to set up ref_reloc_sym
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (5 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 6/8] perf tools: Prevent the use of kcore if the kernel has moved Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-24 15:10 ` [PATCH 8/8] perf tools: Adjust kallsyms for relocated kernel Adrian Hunter
  2014-01-26 18:43 ` [PATCH 0/8] perf tools: kaslr fixes Jiri Olsa
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

Now that ref_reloc_sym is set up by
machine__create_kernel_maps(), the
"vmlinux symtab matches kallsyms" test
does have to do it.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/tests/vmlinux-kallsyms.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c
index 2bd13ed..3d90880 100644
--- a/tools/perf/tests/vmlinux-kallsyms.c
+++ b/tools/perf/tests/vmlinux-kallsyms.c
@@ -26,7 +26,6 @@ int test__vmlinux_matches_kallsyms(void)
 	struct map *kallsyms_map, *vmlinux_map;
 	struct machine kallsyms, vmlinux;
 	enum map_type type = MAP__FUNCTION;
-	struct ref_reloc_sym ref_reloc_sym = { .name = "_stext", };
 	u64 mem_start, mem_end;
 
 	/*
@@ -70,14 +69,6 @@ int test__vmlinux_matches_kallsyms(void)
 	 */
 	kallsyms_map = machine__kernel_map(&kallsyms, type);
 
-	sym = map__find_symbol_by_name(kallsyms_map, ref_reloc_sym.name, NULL);
-	if (sym == NULL) {
-		pr_debug("dso__find_symbol_by_name ");
-		goto out;
-	}
-
-	ref_reloc_sym.addr = UM(sym->start);
-
 	/*
 	 * Step 5:
 	 *
@@ -89,7 +80,6 @@ int test__vmlinux_matches_kallsyms(void)
 	}
 
 	vmlinux_map = machine__kernel_map(&vmlinux, type);
-	map__kmap(vmlinux_map)->ref_reloc_sym = &ref_reloc_sym;
 
 	/*
 	 * Step 6:
-- 
1.7.11.7


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

* [PATCH 8/8] perf tools: Adjust kallsyms for relocated kernel
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (6 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 7/8] perf tools: test does not need to set up ref_reloc_sym Adrian Hunter
@ 2014-01-24 15:10 ` Adrian Hunter
  2014-01-26 18:43 ` [PATCH 0/8] perf tools: kaslr fixes Jiri Olsa
  8 siblings, 0 replies; 10+ messages in thread
From: Adrian Hunter @ 2014-01-24 15:10 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Peter Zijlstra, Ingo Molnar, linux-kernel, David Ahern,
	Frederic Weisbecker, Jiri Olsa, Mike Galbraith, Namhyung Kim,
	Paul Mackerras, Stephane Eranian

If the kernel is relocated at boot time, kallsyms
will not match data recorded previously.  That
does not matter for modules because they are
corrected anyway.  It also does not matter if
vmlinux is being used for symbols. But if perf
tools has only kallsyms then the symbols will not
match.  Fix by applying the delta gained by
comparing the old and current addresses of the
relocation reference symbol.

Signed-off-by: Adrian Hunter <adrian.hunter@intel.com>
---
 tools/perf/util/symbol.c | 40 ++++++++++++++++++++++++++++++++++++++--
 1 file changed, 38 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 4ac1f87..a9d758a 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -627,7 +627,7 @@ static int dso__split_kallsyms_for_kcore(struct dso *dso, struct map *map,
  * kernel range is broken in several maps, named [kernel].N, as we don't have
  * the original ELF section names vmlinux have.
  */
-static int dso__split_kallsyms(struct dso *dso, struct map *map,
+static int dso__split_kallsyms(struct dso *dso, struct map *map, u64 delta,
 			       symbol_filter_t filter)
 {
 	struct map_groups *kmaps = map__kmap(map)->kmaps;
@@ -692,6 +692,12 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			char dso_name[PATH_MAX];
 			struct dso *ndso;
 
+			if (delta) {
+				/* Kernel was relocated at boot time */
+				pos->start -= delta;
+				pos->end -= delta;
+			}
+
 			if (count == 0) {
 				curr_map = map;
 				goto filter_symbol;
@@ -721,6 +727,10 @@ static int dso__split_kallsyms(struct dso *dso, struct map *map,
 			curr_map->map_ip = curr_map->unmap_ip = identity__map_ip;
 			map_groups__insert(kmaps, curr_map);
 			++kernel_range;
+		} else if (delta) {
+			/* Kernel was relocated at boot time */
+			pos->start -= delta;
+			pos->end -= delta;
 		}
 filter_symbol:
 		if (filter && filter(curr_map, pos)) {
@@ -1130,15 +1140,41 @@ out_err:
 	return -EINVAL;
 }
 
+/*
+ * If the kernel is relocated at boot time, kallsyms won't match.  Compute the
+ * delta based on the relocation reference symbol.
+ */
+static int kallsyms__delta(struct map *map, const char *filename, u64 *delta)
+{
+	struct kmap *kmap = map__kmap(map);
+	u64 addr;
+
+	if (!kmap->ref_reloc_sym || !kmap->ref_reloc_sym->name)
+		return 0;
+
+	addr = kallsyms__get_function_start(filename,
+					    kmap->ref_reloc_sym->name);
+	if (!addr)
+		return -1;
+
+	*delta = addr - kmap->ref_reloc_sym->addr;
+	return 0;
+}
+
 int dso__load_kallsyms(struct dso *dso, const char *filename,
 		       struct map *map, symbol_filter_t filter)
 {
+	u64 delta = 0;
+
 	if (symbol__restricted_filename(filename, "/proc/kallsyms"))
 		return -1;
 
 	if (dso__load_all_kallsyms(dso, filename, map) < 0)
 		return -1;
 
+	if (kallsyms__delta(map, filename, &delta))
+		return -1;
+
 	symbols__fixup_duplicate(&dso->symbols[map->type]);
 	symbols__fixup_end(&dso->symbols[map->type]);
 
@@ -1150,7 +1186,7 @@ int dso__load_kallsyms(struct dso *dso, const char *filename,
 	if (!dso__load_kcore(dso, map, filename))
 		return dso__split_kallsyms_for_kcore(dso, map, filter);
 	else
-		return dso__split_kallsyms(dso, map, filter);
+		return dso__split_kallsyms(dso, map, delta, filter);
 }
 
 static int dso__load_perf_map(struct dso *dso, struct map *map,
-- 
1.7.11.7


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

* Re: [PATCH 0/8] perf tools: kaslr fixes
  2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
                   ` (7 preceding siblings ...)
  2014-01-24 15:10 ` [PATCH 8/8] perf tools: Adjust kallsyms for relocated kernel Adrian Hunter
@ 2014-01-26 18:43 ` Jiri Olsa
  8 siblings, 0 replies; 10+ messages in thread
From: Jiri Olsa @ 2014-01-26 18:43 UTC (permalink / raw)
  To: Adrian Hunter
  Cc: Arnaldo Carvalho de Melo, Peter Zijlstra, Ingo Molnar,
	linux-kernel, David Ahern, Frederic Weisbecker, Mike Galbraith,
	Namhyung Kim, Paul Mackerras, Stephane Eranian

On Fri, Jan 24, 2014 at 05:10:10PM +0200, Adrian Hunter wrote:
> Hi
> 
> Here are some patches that improve perf tools
> handling of relocation.
> 
> This has become an issue as mentioned in this
> thread:
> 
> 	http://marc.info/?l=linux-kernel&m=139030004314756
> 
> 
> It looks like the changes I made to vmlinux and
> module mapping to allow object code to be read
> broke the symbol annotation for relocated kernels.
> The first patch fixes that.
> 
> There were some other issues though:
> 	- ref_reloc_sym was not always set up
> 	- mustn't use kcore if the kernel has moved
> 	- kallsyms is not unique for a given
> 	buildid
> 
> These things need more testing but I thought it was
> worth getting comments now.
> 
> 
> Adrian Hunter (8):
>       perf tools: Fix symbol annotation for relocated kernel
>       perf tools: Add kallsyms__get_function_start()
>       perf tools: Add machine__get_kallsyms_filename()
>       perf tools: Set up ref_reloc_sym in machine__create_kernel_maps()
>       perf report: Get ref_reloc_sym from kernel map
>       perf tools: Prevent the use of kcore if the kernel has moved
>       perf tools: test does not need to set up ref_reloc_sym
>       perf tools: Adjust kallsyms for relocated kernel

cannot say much for the code changes (look ok ;-) ), but
it fixed the issue for me

Tested-by: Jiri Olsa <jolsa@redhat.com>

jirka

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

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

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-01-24 15:10 [PATCH 0/8] perf tools: kaslr fixes Adrian Hunter
2014-01-24 15:10 ` [PATCH 1/8] perf tools: Fix symbol annotation for relocated kernel Adrian Hunter
2014-01-24 15:10 ` [PATCH 2/8] perf tools: Add kallsyms__get_function_start() Adrian Hunter
2014-01-24 15:10 ` [PATCH 3/8] perf tools: Add machine__get_kallsyms_filename() Adrian Hunter
2014-01-24 15:10 ` [PATCH 4/8] perf tools: Set up ref_reloc_sym in machine__create_kernel_maps() Adrian Hunter
2014-01-24 15:10 ` [PATCH 5/8] perf report: Get ref_reloc_sym from kernel map Adrian Hunter
2014-01-24 15:10 ` [PATCH 6/8] perf tools: Prevent the use of kcore if the kernel has moved Adrian Hunter
2014-01-24 15:10 ` [PATCH 7/8] perf tools: test does not need to set up ref_reloc_sym Adrian Hunter
2014-01-24 15:10 ` [PATCH 8/8] perf tools: Adjust kallsyms for relocated kernel Adrian Hunter
2014-01-26 18:43 ` [PATCH 0/8] perf tools: kaslr fixes Jiri Olsa

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).