Skip to contentSkip to conversation

perf tools: set kernel end address properly

PATCH In review Namhyung Kim 6 participants 27 messages Apr 16, 2018 – Apr 26, 2018

Subsystems (3)
perf events arm64 Performance events tooling arm64
lore
Share & tools
lore

Namhyung Kim

author maintainer
#0 lore raw

PATCH 1/1 perf tools: set kernel end address properly

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/util/machine.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
tools/perf/util/machine.c +8 -12
index 2eca8478e24f..be328416de61 100644
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];

@@ -1233,7 +1226,9 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
+	u64 end = ~0ULL;
 	int ret;
 
 	if (kernel == NULL)

@@ -1259,13 +1254,14 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		end = map->start;
+
+	machine__set_kernel_mmap(machine, addr, end);
 	return 0;
 }
Signature · 1 line
2.16.2
Acked-by on v1 1/1
#1 lore raw
Quoted 9 lines · Namhyung Kim

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

looks good

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 55 lines
---
 tools/perf/util/machine.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
tools/perf/util/machine.c +8 -12
index 2eca8478e24f..be328416de61 100644
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];

@@ -1233,7 +1226,9 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
+	u64 end = ~0ULL;
 	int ret;
 
 	if (kernel == NULL)

@@ -1259,13 +1254,14 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		end = map->start;
+
+	machine__set_kernel_mmap(machine, addr, end);
 	return 0;
 }
Signature · 1 line
2.16.2
maintainer
#2 lore raw

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

Quoted 65 lines
Quoted 2 lines
Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

looks good

Acked-by Jiri Olsa <[email protected]>

thanks,
jirka

Quoted 55 lines
---
 tools/perf/util/machine.c | 20 ++++++++------------
 1 file changed, 8 insertions(+), 12 deletions(-)
tools/perf/util/machine.c +8 -12
index 2eca8478e24f..be328416de61 100644
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];

@@ -1233,7 +1226,9 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
+	u64 end = ~0ULL;
 	int ret;
 
 	if (kernel == NULL)

@@ -1259,13 +1254,14 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		end = map->start;
+
+	machine__set_kernel_mmap(machine, addr, end);
 	return 0;
 }
Signature · 1 line
2.16.2
#3 lore raw
Quoted 9 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Quoted 5 lines

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Kim

maintainer
#4 lore raw

Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu:

Quoted 15 lines
Quoted 12 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

Quoted 6 lines
Quoted 2 lines

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Ok, -vv please

- Arnaldo

#5 lore raw
Quoted 16 lines · On Mon, 16 Apr 2018 13:58:00 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu:

Quoted 15 lines
Quoted 12 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Ok, -vv please

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!
$

Kim

#6 lore raw
Quoted 44 lines · On Mon, 16 Apr 2018 12:24:07 -0500 Kim Phillips <[email protected]> wrote:
Quoted 16 lines · On Mon, 16 Apr 2018 13:58:00 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu:

Quoted 15 lines
Quoted 12 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Ok, -vv please

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
Quoted 8 lines
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!

this patch's advertised "If there's no module after the kernel map, the end address will be ~0ULL." doesn't seem to be working: the value it gets for 'end' is 0xffff200008080000.

I even hardcoded 'end' variable to the value for '_end' in kallsyms prior to the machine__set_kernel_mmap() call, and test 1 still fails.

The problem is earlier on in the code, somewhere.

I've moved to working on the updated perf/urgent. Reverting the original patch doesn't make test 1 resume succeeding anymore.

Pointers appreciated.

Kim

author maintainer
#7 lore raw
Quoted 56 lines · Kim Phillips
Quoted 44 lines · On Mon, 16 Apr 2018 12:24:07 -0500 Kim Phillips <[email protected]> wrote:
Quoted 16 lines · On Mon, 16 Apr 2018 13:58:00 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu:

Quoted 15 lines
Quoted 12 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Ok, -vv please

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
Quoted 8 lines
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!
Quoted 3 lines

this patch's advertised "If there's no module after the kernel map, the end address will be ~0ULL." doesn't seem to be working: the value it gets for 'end' is 0xffff200008080000.

For the vmlinux, right?

To be precise, it should be "If there's no map after the kernel map". It seems vmlinux adds more maps after the kernel map for those .head, .init and .exit text sections.

Quoted 3 lines

I even hardcoded 'end' variable to the value for '_end' in kallsyms prior to the machine__set_kernel_mmap() call, and test 1 still fails.

I guess it was overwritten by the machine__set_kernel_mmap() then.

Quoted 2 lines

The problem is earlier on in the code, somewhere.

To be sure, could you please show me the /proc/kallsymb output around the missing symbols (do_undefinstr, ...)?

Quoted 5 lines

I've moved to working on the updated perf/urgent. Reverting the original patch doesn't make test 1 resume succeeding anymore.

Pointers appreciated.

The missing symbols are in the missing maps. They should be in the kernel maps for the kallsyms case IMHO. Could you please run the test with below patch?

Thanks,
Namhyung

---------------8<----------------

tools/perf/tests/vmlinux-kallsyms.c +11 -0
index 1e5adb65632a..b14ed70fa440 100644
@@ -163,6 +163,17 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
 
                                continue;
                        }
+               } else if (pair) {
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       pr_debug("ERR : %#" PRIx64 ": diff addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
#8 lore raw
Quoted 57 lines · On Tue, 17 Apr 2018 11:27:26 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 56 lines · Kim Phillips
Quoted 52 lines · On Mon, 16 Apr 2018 12:24:07 -0500 Kim Phillips <[email protected]> wrote:
Quoted 16 lines · On Mon, 16 Apr 2018 13:58:00 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu:

Quoted 15 lines
Quoted 12 lines · On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu:

Quoted 7 lines · Namhyung Kim
Quoted 6 lines

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly so the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

I wonder if it wouldn't be better to have it as last symbol + PAGE_SIZE or something like that...

Quoted 8 lines
Quoted 6 lines
Quoted 2 lines

But, anyway, Kim, does this fix things for you? Can I have your Tested-by?

No, perf test 1 still fails:

vmlinux symtab matches kallsyms: FAILED!

Ok, -vv please

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!
Quoted 5 lines
Quoted 3 lines

this patch's advertised "If there's no module after the kernel map, the end address will be ~0ULL." doesn't seem to be working: the value it gets for 'end' is 0xffff200008080000.

For the vmlinux, right?

yes, map__next(machine__kernel_map(machine)) has the start address of the single module currently loaded:

ffff200002290000 t $x [arm_ccn]
ffff200002290000 t arm_ccn_pmu_events_is_visible [arm_ccn]

The beginning of the kernel is..later:

ffff200008080000 t _head
ffff200008080000 T _text

and its end according to grep -w _end /proc/kallsyms is:

ffff20000d5f9000 B _end

but end was assigned to the beginning of arm_ccn (0xffff200002290000), which is upside-down.

Quoted 1 line

To be precise, it should be "If there's no map after the kernel map".

In numerical address order, in maps in map_groups__insert order, or some other order?

Quoted 2 lines

It seems vmlinux adds more maps after the kernel map for those .head, .init and .exit text sections.

those shouldn't matter, I don't think - x86 has .init and .exit also, and I don't necessarily see the .head addresses being involved. Let me know if this is indeed going to be a problem.

Quoted 4 lines
Quoted 2 lines

I even hardcoded 'end' variable to the value for '_end' in kallsyms prior to the machine__set_kernel_mmap() call, and test 1 still fails.

I guess it was overwritten by the machine__set_kernel_mmap() then.

Yes.

Quoted 4 lines
Quoted 1 line

The problem is earlier on in the code, somewhere.

To be sure, could you please show me the /proc/kallsymb output around the missing symbols (do_undefinstr, ...)?

see below

Quoted 8 lines
Quoted 4 lines

I've moved to working on the updated perf/urgent. Reverting the original patch doesn't make test 1 resume succeeding anymore.

Pointers appreciated.

The missing symbols are in the missing maps. They should be in the kernel maps for the kallsyms case IMHO. Could you please run the test with below patch?

Thanks! Turns out that in the case of no modules loaded, commit a257e02579e4270 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" added a 'veneer' symbol with a different start address, so instead of:

    ERR : 0xffff44eb0a6c9620: module_emit_adrp_veneer not on kallsyms

I got:

ERR : 0xffff44eb0a6c9620: diff addr v: module_emit_adrp_veneer k: 0xffff44eb0a6c9500 module_emit_plt_entry
: map v: [kernel] k: [kernel]

So it helped solve the no-modules-loaded case. I've updated my perf tool to the latest acme's perf/urgent (commit e6a0a610a60), added this patch "perf tools: set kernel end address properly", and this diff, enhanced/re-purposed it a bit, and included a fix for succeeding the test 1 when there are no modules loaded - see the end of this email. I doubt it's acceptable as-is, so please take a look.

Meanwhile, in order to help debug the module(s) loaded case, I'm providing:

- output of the above perf, called with 'test -vvvvv 1' with no modules loaded (now passes test):
- http://paste.ubuntu.com/p/xBgpjTyjFp/

- output of the above perf, called with 'test -vvvvv 1' with the arm-ccn module loaded (does not pass test):
- http://paste.ubuntu.com/p/nWYxC9c5y2/

- /proc/kallsyms with the module loaded:
- http://paste.ubuntu.com/p/pKskqyqdsK/

Thanks for your help,

Kim

tools/perf/arch/arm64/util/sym-handling.c +13 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /* 
+        * arm64 kernels compensating for a CPU erratum can put up a 
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}
tools/perf/tests/vmlinux-kallsyms.c +23 -0
index 1e5adb65632a..07064e76947d 100644
@@ -163,6 +163,29 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
 
                                continue;
                        }
+               } else if (pair) {
+                       s64 skew = mem_start - UM(pair->start);
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       /* 
+                        * arm64 kernels compensating for a CPU erratum can put up a 
+                        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+                        */
+                       if (llabs(skew) < page_size)
+                       {
+                               pr_debug("NO ERR FOR SKEW %ld: %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                        skew, mem_start, sym->name, UM(pair->start), pair->name);
+                               continue;
+                       }
+
+                       pr_debug("ERR : %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
author maintainer
#9 lore raw
Quoted 45 lines · Kim Phillips
Quoted 26 lines · On Tue, 17 Apr 2018 11:27:26 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 25 lines · Kim Phillips
Quoted 24 lines

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!
Quoted 5 lines
Quoted 3 lines

this patch's advertised "If there's no module after the kernel map, the end address will be ~0ULL." doesn't seem to be working: the value it gets for 'end' is 0xffff200008080000.

For the vmlinux, right?

yes, map__next(machine__kernel_map(machine)) has the start address of the single module currently loaded:

ffff200002290000 t $x [arm_ccn]
ffff200002290000 t arm_ccn_pmu_events_is_visible [arm_ccn]

The beginning of the kernel is..later:

ffff200008080000 t _head
ffff200008080000 T _text

Quoted 6 lines

and its end according to grep -w _end /proc/kallsyms is:

ffff20000d5f9000 B _end

but end was assigned to the beginning of arm_ccn (0xffff200002290000), which is upside-down.

So ARM64 has modules below the kernel.

Quoted 5 lines
Quoted 2 lines

To be precise, it should be "If there's no map after the kernel map".

In numerical address order, in maps in map_groups__insert order, or some other order?

The map_groups__insert() also sorts the map by address, so they should be identical. I think the problem is perf assumes the kernel is the first map in the kmaps. When it calls maps_groups__insert() it uses start address of 0 for the kernel map. It seems always true for x86 but not for ARM64.

While it changes the start address in machine__set_kernel_mmap() it doesn't change the order in the kmaps.

Could you please test below patch (on top) then?

Thanks,
Namhyung

---8<---

tools/perf/util/machine.c +13 -3
index be328416de61..0f3c4bc7b90f 100644
@@ -1228,7 +1228,6 @@ int machine__create_kernel_maps(struct machine *machine)
        const char *name = NULL;
        struct map *map;
        u64 addr = 0;
-       u64 end = ~0ULL;
        int ret;
 
        if (kernel == NULL)

@@ -1254,14 +1253,25 @@ int machine__create_kernel_maps(struct machine *machine)
                        machine__destroy_kernel_maps(machine);
                        return -1;
                }
+
+               /* we have a real start address now, so re-order the kmaps */
+               map = machine__kernel_map(machine);
+
+               map__get(map);
+               map_groups__remove(&machine->kmaps, map);
+
+               /* assume it's the last in the kmaps */
+               machine__set_kernel_mmap(machine, addr, ~0ULL);
+
+               map_groups__insert(&machine->kmaps, map);
+               map__put(map);
        }
 
        /* update end address of the kernel map using adjacent module address */
        map = map__next(machine__kernel_map(machine));
        if (map)
-               end = map->start;
+               machine__set_kernel_mmap(machine, addr, map->start);
 
-       machine__set_kernel_mmap(machine, addr, end);
        return 0;
 }
#10 lore raw
Quoted 46 lines · On Thu, 19 Apr 2018 11:37:12 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 45 lines · Kim Phillips
Quoted 26 lines · On Tue, 17 Apr 2018 11:27:26 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 25 lines · Kim Phillips
Quoted 24 lines

a perf/urgent from last week (commit 918965d4897) + this patch:

$ sudo ./perf test -vv 1 |& head 
 1: vmlinux symtab matches kallsyms                       :
--- start ---
test child forked, pid 6194
Looking at the vmlinux_path (8 entries long)
Using /lib/modules/4.16.0+/build/vmlinux for symbols
ERR : 0xffff200008081000: do_undefinstr not on kallsyms
ERR : 0xffff2000080810b8: do_sysinstr not on kallsyms
ERR : 0xffff200008081258: do_debug_exception not on kallsyms
ERR : 0xffff200008081648: do_mem_abort not on kallsyms
ERR : 0xffff2000080818b8: do_el0_irq_bp_hardening not on kallsyms
$ sudo ./perf test -vv 1 |& tail
ERR : 0xffff20000a1d37c8: tramp_exit_native not on kallsyms
ERR : 0xffff20000a1d37e8: tramp_exit_compat not on kallsyms
ERR : 0xffff20000a1d4000: __entry_tramp_text_end not on kallsyms
WARN: Maps only in vmlinux:
 ffff200008080000-ffff200008081000 10000 [kernel].head.text
 ffff20000aec0000-ffff20000aff7548 2e50000 [kernel].init.text
 ffff20000aff7548-ffff20000b0126d4 2f87548 [kernel].exit.text
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!
Quoted 5 lines
Quoted 3 lines

this patch's advertised "If there's no module after the kernel map, the end address will be ~0ULL." doesn't seem to be working: the value it gets for 'end' is 0xffff200008080000.

For the vmlinux, right?

yes, map__next(machine__kernel_map(machine)) has the start address of the single module currently loaded:

ffff200002290000 t $x [arm_ccn]
ffff200002290000 t arm_ccn_pmu_events_is_visible [arm_ccn]

The beginning of the kernel is..later:

ffff200008080000 t _head
ffff200008080000 T _text

Quoted 8 lines
Quoted 6 lines

and its end according to grep -w _end /proc/kallsyms is:

ffff20000d5f9000 B _end

but end was assigned to the beginning of arm_ccn (0xffff200002290000), which is upside-down.

So ARM64 has modules below the kernel.

right.

Quoted 6 lines
Quoted 4 lines
Quoted 1 line

To be precise, it should be "If there's no map after the kernel map".

In numerical address order, in maps in map_groups__insert order, or some other order?

The map_groups__insert() also sorts the map by address, so they should

Had missed that, sorry.

Quoted 9 lines

be identical. I think the problem is perf assumes the kernel is the first map in the kmaps. When it calls maps_groups__insert() it uses start address of 0 for the kernel map. It seems always true for x86 but not for ARM64.

While it changes the start address in machine__set_kernel_mmap() it doesn't change the order in the kmaps.

Could you please test below patch (on top) then?

Much better, thank you! Together with the "module_emit_adrp_veneer not on kallsyms" mitigation diff, test 1 now passes on arm64 systems both with modules loaded, and without, and with and without RANDOMIZE_BASE.

I would send a patch (From: you), but I think you'd do a better job on writing the commit text, so would you care to, please?

Thanks,

Kim

author maintainer
#11 lore raw
Quoted 10 lines · Kim Phillips
Quoted 8 lines · On Thu, 19 Apr 2018 11:37:12 +0900 Namhyung Kim <[email protected]> wrote:

I think the problem is perf assumes the kernel is the first map in the kmaps. When it calls maps_groups__insert() it uses start address of 0 for the kernel map. It seems always true for x86 but not for ARM64.

While it changes the start address in machine__set_kernel_mmap() it doesn't change the order in the kmaps.

Quoted 8 lines
Quoted 1 line

Could you please test below patch (on top) then?

Much better, thank you! Together with the "module_emit_adrp_veneer not on kallsyms" mitigation diff, test 1 now passes on arm64 systems both with modules loaded, and without, and with and without RANDOMIZE_BASE.

I would send a patch (From: you), but I think you'd do a better job on writing the commit text, so would you care to, please?

Sure, what about this?

Thanks,
Namhyung

Quoted 1 line

From 1d9fa3b21bd7a964af43dcc9d486a09917d795e9 Mon Sep 17 00:00:00 2001

From: Namhyung Kim <[email protected]>
Date: Mon, 19 Feb 2018 19:05:45 +0900
Subject: [PATCH] perf tools: set kernel end address properly

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly, the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Since it also changes the start address of the kernel map, it needs to re-insert the map to the kmaps in order to keep a correct ordering. Kim reported that it caused problems on ARM64.

Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>
---
 tools/perf/util/machine.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)
tools/perf/util/machine.c +18 -12
index 2eca8478e24f..32d50492505d 100644
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];

@@ -1233,6 +1226,7 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
 	int ret;
 

@@ -1259,13 +1253,25 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
+
+		/* we have a real start address now, so re-order the kmaps */
+		map = machine__kernel_map(machine);
+
+		map__get(map);
+		map_groups__remove(&machine->kmaps, map);
+
+		/* assume it's the last in the kmaps */
+		machine__set_kernel_mmap(machine, addr, ~0ULL);
+
+		map_groups__insert(&machine->kmaps, map);
+		map__put(map);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		machine__set_kernel_mmap(machine, addr, map->start);
+
 	return 0;
 }
Signature · 1 line
2.16.2
Acked-by
#12 lore raw
Quoted 7 lines · On Fri, 20 Apr 2018 08:59:15 +0900 Namhyung Kim <[email protected]> wrote:

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly, the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Quoted 7 lines

Since it also changes the start address of the kernel map, it needs to re-insert the map to the kmaps in order to keep a correct ordering. Kim reported that it caused problems on ARM64.

Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

---

Acked-by Kim Phillips <[email protected]>

Thanks,

Kim

maintainer
#13 lore raw

Em Fri, Apr 20, 2018 at 06:23:58PM -0500, Kim Phillips escreveu:

Quoted 14 lines
Quoted 11 lines · On Fri, 20 Apr 2018 08:59:15 +0900 Namhyung Kim <[email protected]> wrote:

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly, the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Since it also changes the start address of the kernel map, it needs to re-insert the map to the kmaps in order to keep a correct ordering. Kim reported that it caused problems on ARM64.

Quoted 5 lines
Quoted 3 lines
Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

---

Acked-by Kim Phillips <[email protected]>

So as you actually seem to have tested this, I'm taking this as a Tested-by instead, to better reflect what you did, ok?

- Arnaldo

#14 lore raw
Quoted 15 lines · On Mon, 23 Apr 2018 10:52:21 -0300 Arnaldo Carvalho de Melo <[email protected]>…

Em Fri, Apr 20, 2018 at 06:23:58PM -0500, Kim Phillips escreveu:

Quoted 14 lines
Quoted 11 lines · On Fri, 20 Apr 2018 08:59:15 +0900 Namhyung Kim <[email protected]> wrote:

The map_groups__fixup_end() was called to set end addresses of kernel map and module maps. But now machine__create_modules() is set the end address of modules properly, the only remaining piece is the kernel map. We can set it with adjacent module's address directly instead of calling the map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Since it also changes the start address of the kernel map, it needs to re-insert the map to the kmaps in order to keep a correct ordering. Kim reported that it caused problems on ARM64.

Quoted 8 lines
Quoted 5 lines
Quoted 3 lines
Reported-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>

---

Acked-by Kim Phillips <[email protected]>

So as you actually seem to have tested this, I'm taking this as a Tested-by instead, to better reflect what you did, ok?

Sure.

Thanks,

Kim

merge bot
#15 lore raw

[tip:perf/urgent] perf machine: Set main kernel end address properly

In reply to Namhyung Kim
perf machine: Set main kernel end address properly
Commit-ID:  ee05d21791db6db954bbb7b79bb18d88b5f6b7ff
Gitweb:     https://git.kernel.org/tip/ee05d21791db6db954bbb7b79bb18d88b5f6b7ff
Author:     Namhyung Kim <[email protected]>
AuthorDate: Mon, 19 Feb 2018 19:05:45 +0900
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 23 Apr 2018 10:52:55 -0300

perf machine: Set main kernel end address properly

map_groups__fixup_end() was called to set the end addresses of kernel and module maps. But now since machine__create_modules() sets the end address of modules properly, the only remaining piece is the kernel map.

We can set it with adjacent module's address directly instead of calling map_groups__fixup_end(). If there's no module after the kernel map, the end address will be ~0ULL.

Since it also changes the start address of the kernel map, it needs to re-insert the map to the kmaps in order to keep a correct ordering. Kim reported that it caused problems on ARM64.

Reported-by Kim Phillips <[email protected]>
Tested-by Kim Phillips <[email protected]>
Signed-off-by Namhyung Kim <[email protected]>
Cc Jiri Olsa <[email protected]>
Cc Peter Zijlstra <[email protected]>
Cc [email protected]
Link http://lkml.kernel.org/r/20180419235915.GA19067@sejong open here
Signed-off-by Arnaldo Carvalho de Melo <[email protected]>
---
 tools/perf/util/machine.c | 30 ++++++++++++++++++------------
 1 file changed, 18 insertions(+), 12 deletions(-)
tools/perf/util/machine.c +18 -12
index 2eca8478e24f..32d50492505d 100644
@@ -1019,13 +1019,6 @@ int machine__load_vmlinux_path(struct machine *machine, enum map_type type)
 	return ret;
 }
 
-static void map_groups__fixup_end(struct map_groups *mg)
-{
-	int i;
-	for (i = 0; i < MAP__NR_TYPES; ++i)
-		__map_groups__fixup_end(mg, i);
-}
-
 static char *get_kernel_version(const char *root_dir)
 {
 	char version[PATH_MAX];

@@ -1233,6 +1226,7 @@ int machine__create_kernel_maps(struct machine *machine)
 {
 	struct dso *kernel = machine__get_kernel(machine);
 	const char *name = NULL;
+	struct map *map;
 	u64 addr = 0;
 	int ret;
 

@@ -1259,13 +1253,25 @@ int machine__create_kernel_maps(struct machine *machine)
 			machine__destroy_kernel_maps(machine);
 			return -1;
 		}
-		machine__set_kernel_mmap(machine, addr, 0);
+
+		/* we have a real start address now, so re-order the kmaps */
+		map = machine__kernel_map(machine);
+
+		map__get(map);
+		map_groups__remove(&machine->kmaps, map);
+
+		/* assume it's the last in the kmaps */
+		machine__set_kernel_mmap(machine, addr, ~0ULL);
+
+		map_groups__insert(&machine->kmaps, map);
+		map__put(map);
 	}
 
-	/*
-	 * Now that we have all the maps created, just set the ->end of them:
-	 */
-	map_groups__fixup_end(&machine->kmaps);
+	/* update end address of the kernel map using adjacent module address */
+	map = map__next(machine__kernel_map(machine));
+	if (map)
+		machine__set_kernel_mmap(machine, addr, map->start);
+
 	return 0;
 }
author maintainer
#16 lore raw
In reply to Kim Phillips
Quoted 11 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +3 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 10 lines
tools/perf/arch/arm64/util/sym-handling.c +10 -0
+       /* 
+        * arm64 kernels compensating for a CPU erratum can put up a 
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

Quoted 8 lines
tools/perf/tests/vmlinux-kallsyms.c +0 -0
index 1e5adb65632a..07064e76947d 100644
@@ -163,6 +163,29 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
 
                                continue;
                        }

tools/perf/tests/vmlinux-kallsyms.c int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest · line 175

Quoted 10 lines
tools/perf/tests/vmlinux-kallsyms.c +10 -0
+               } else if (pair) {
+                       s64 skew = mem_start - UM(pair->start);
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       /* 
+                        * arm64 kernels compensating for a CPU erratum can put up a 
+                        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+                        */
+                       if (llabs(skew) < page_size)

It seems that we needs to check it's the ARM64 at least. If it's a rare case we might need to add more paranoid checks.

Thanks,
Namhyung

Quoted 16 lines
tools/perf/tests/vmlinux-kallsyms.c +13 -0
+                       {
+                               pr_debug("NO ERR FOR SKEW %ld: %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                        skew, mem_start, sym->name, UM(pair->start), pair->name);
+                               continue;
+                       }
+
+                       pr_debug("ERR : %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
#17 lore raw
Quoted 17 lines · On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 16 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +8 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /* 
+        * arm64 kernels compensating for a CPU erratum can put up a 
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
Quoted 8 lines

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 5 lines
tools/perf/arch/arm64/util/sym-handling.c +5 -0
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

AFACT, the veneer is a moniker and doesn't technically exist, and shouldn't be being looked-up. Both chunks of this diff are needed to pass perf test 1: this chunk above is because in arch__normalize_symbol_name(), we squash the perf test 1's "<veneer> not in *kallsyms*" problem, and in the below chunk, we prevent it coming up when the test code iterates over the *vmlinux* symbols. I.e. we need to prevent the veneer from coming up in both kallsyms *and* vmlinux.

Quoted 6 lines
Quoted 6 lines
tools/perf/tests/vmlinux-kallsyms.c +0 -0
index 1e5adb65632a..07064e76947d 100644
@@ -163,6 +163,29 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
Quoted 14 lines

tools/perf/tests/vmlinux-kallsyms.c int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest · line 175

Quoted 12 lines
tools/perf/tests/vmlinux-kallsyms.c +10 -0
                                continue;
                        }
+               } else if (pair) {
+                       s64 skew = mem_start - UM(pair->start);
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       /* 
+                        * arm64 kernels compensating for a CPU erratum can put up a 
+                        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+                        */
+                       if (llabs(skew) < page_size)

It seems that we needs to check it's the ARM64 at least. If it's a

OK.

Quoted 1 line

rare case we might need to add more paranoid checks.

It's certainly rare: Adding the authors of the veneer to cc for comments:

Will, Ard, how probable are veneer-style symbols such as the one introduced in commit a257e0257 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" to happen again in the future?

I would have thought WARNing on within-a-pagesize would be OK, Namhyung. Are you suggesting checking instead for a hardcoded veneer symbol string?

Thanks,

Kim

Quoted 16 lines
Quoted 16 lines
tools/perf/tests/vmlinux-kallsyms.c +13 -0
+                       {
+                               pr_debug("NO ERR FOR SKEW %ld: %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                        skew, mem_start, sym->name, UM(pair->start), pair->name);
+                               continue;
+                       }
+
+                       pr_debug("ERR : %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
author maintainer
#18 lore raw
Quoted 66 lines · Kim Phillips
Quoted 17 lines · On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 16 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +8 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /* 
+        * arm64 kernels compensating for a CPU erratum can put up a 
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
Quoted 8 lines

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 5 lines
tools/perf/arch/arm64/util/sym-handling.c +5 -0
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

AFACT, the veneer is a moniker and doesn't technically exist, and shouldn't be being looked-up. Both chunks of this diff are needed to pass perf test 1: this chunk above is because in arch__normalize_symbol_name(), we squash the perf test 1's "<veneer> not in *kallsyms*" problem, and in the below chunk, we prevent it coming up when the test code iterates over the *vmlinux* symbols. I.e. we need to prevent the veneer from coming up in both kallsyms *and* vmlinux.

Quoted 6 lines
Quoted 6 lines
tools/perf/tests/vmlinux-kallsyms.c +0 -0
index 1e5adb65632a..07064e76947d 100644
@@ -163,6 +163,29 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
Quoted 14 lines

tools/perf/tests/vmlinux-kallsyms.c int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest · line 175

Quoted 12 lines
tools/perf/tests/vmlinux-kallsyms.c +10 -0
                                continue;
                        }
+               } else if (pair) {
+                       s64 skew = mem_start - UM(pair->start);
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       /* 
+                        * arm64 kernels compensating for a CPU erratum can put up a 
+                        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+                        */
+                       if (llabs(skew) < page_size)

It seems that we needs to check it's the ARM64 at least. If it's a

OK.

Quoted 1 line

rare case we might need to add more paranoid checks.

It's certainly rare: Adding the authors of the veneer to cc for comments:

Quoted 7 lines

Will, Ard, how probable are veneer-style symbols such as the one introduced in commit a257e0257 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" to happen again in the future?

I would have thought WARNing on within-a-pagesize would be OK, Namhyung. Are you suggesting checking instead for a hardcoded veneer symbol string?

Anything to prevent false-negatives (possibly on other archs).

Thanks,
Namhyung

Quoted 21 lines

Thanks,

Kim

Quoted 16 lines
Quoted 16 lines
tools/perf/tests/vmlinux-kallsyms.c +13 -0
+                       {
+                               pr_debug("NO ERR FOR SKEW %ld: %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                        skew, mem_start, sym->name, UM(pair->start), pair->name);
+                               continue;
+                       }
+
+                       pr_debug("ERR : %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
#19 lore raw
In reply to Kim Phillips
Quoted 29 lines · Kim Phillips
Quoted 17 lines · On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 16 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +8 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /*
+        * arm64 kernels compensating for a CPU erratum can put up a
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
Quoted 8 lines

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 5 lines
tools/perf/arch/arm64/util/sym-handling.c +5 -0
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

Quoted 9 lines

AFACT, the veneer is a moniker and doesn't technically exist, and shouldn't be being looked-up. Both chunks of this diff are needed to pass perf test 1: this chunk above is because in arch__normalize_symbol_name(), we squash the perf test 1's "<veneer> not in *kallsyms*" problem, and in the below chunk, we prevent it coming up when the test code iterates over the *vmlinux* symbols. I.e. we need to prevent the veneer from coming up in both kallsyms *and* vmlinux.

I don't have all the context here, so I don't know what exactly arch__normalize_symbol_name() is trying to accomplish.

What I do know is that module_emit_adrp_veneer() and module_emit_plt_entry() are not part of the veneer themselves: they are ordinary routines that are part of the module loader, and which populate the allocated veneer space on demand when encountering ADRP instructions that need to be rerouted.

Quoted 23 lines
Quoted 6 lines
Quoted 6 lines
tools/perf/tests/vmlinux-kallsyms.c +0 -0
index 1e5adb65632a..07064e76947d 100644
@@ -163,6 +163,29 @@ int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest
Quoted 14 lines

tools/perf/tests/vmlinux-kallsyms.c int test__vmlinux_matches_kallsyms(struct test *test __maybe_unused, int subtest · line 175

Quoted 12 lines
tools/perf/tests/vmlinux-kallsyms.c +10 -0
                                continue;
                        }
+               } else if (pair) {
+                       s64 skew = mem_start - UM(pair->start);
+                       struct map *kmap = map_groups__find(&kallsyms.kmaps, type, mem_start);
+                       struct map *vmap = map_groups__find(&vmlinux.kmaps, type, mem_start);
+
+                       /*
+                        * arm64 kernels compensating for a CPU erratum can put up a
+                        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+                        */
+                       if (llabs(skew) < page_size)

It seems that we needs to check it's the ARM64 at least. If it's a

OK.

Quoted 9 lines
Quoted 1 line

rare case we might need to add more paranoid checks.

It's certainly rare: Adding the authors of the veneer to cc for comments:

Will, Ard, how probable are veneer-style symbols such as the one introduced in commit a257e0257 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" to happen again in the future?

Distro kernels typically enable full KASLR, so on systems that implement EFI_RNG_PROTOCOL, all function calls from modules into the kernel proper are redirected via veneers. (Note that these are bl instructions not adrp instructions though).

Quoted 4 lines

I would have thought WARNing on within-a-pagesize would be OK, Namhyung. Are you suggesting checking instead for a hardcoded veneer symbol string?

Veneers don't have symbol strings. Veneers are anonymous sequences of instructions living in a patch of R-X mapped module space somewhere. The only symbol strings are for the routines that generate these veneers, not for the veneers themselves.

Quoted 15 lines
Quoted 15 lines
Quoted 15 lines
tools/perf/tests/vmlinux-kallsyms.c +12 -0
+                               pr_debug("NO ERR FOR SKEW %ld: %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                        skew, mem_start, sym->name, UM(pair->start), pair->name);
+                               continue;
+                       }
+
+                       pr_debug("ERR : %#" PRIx64 ": diff start addr v: %s k: %#" PRIx64 " %s\n",
+                                mem_start, sym->name, UM(pair->start), pair->name);
+
+                       if (kmap && vmap) {
+                               pr_debug("    : map v: %s k: %s\n",
+                                        vmap->dso->short_name, kmap->dso->short_name);
+                       }
                } else
                        pr_debug("ERR : %#" PRIx64 ": %s not on kallsyms\n",
                                 mem_start, sym->name);
#20 lore raw
Quoted 39 lines · On Fri, 20 Apr 2018 10:10:32 +0200 Ard Biesheuvel <[email protected]>…
Quoted 29 lines · Kim Phillips
Quoted 17 lines · On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 16 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +8 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /*
+        * arm64 kernels compensating for a CPU erratum can put up a
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
Quoted 8 lines

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 5 lines
tools/perf/arch/arm64/util/sym-handling.c +5 -0
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

Quoted 8 lines

AFACT, the veneer is a moniker and doesn't technically exist, and shouldn't be being looked-up. Both chunks of this diff are needed to pass perf test 1: this chunk above is because in arch__normalize_symbol_name(), we squash the perf test 1's "<veneer> not in *kallsyms*" problem, and in the below chunk, we prevent it coming up when the test code iterates over the *vmlinux* symbols. I.e. we need to prevent the veneer from coming up in both kallsyms *and* vmlinux.

Quoted 2 lines

I don't have all the context here, so I don't know what exactly arch__normalize_symbol_name() is trying to accomplish.

Sorry about that. The problem is that perf test 1 ("vmlinux symtab matches kallsyms") is failing, because module_emit_adrp_veneer exists in /boot/vmlinux but doesn't exist in /proc/kallsyms, and, sure enough, it's not in the latter.

Quoted 5 lines

What I do know is that module_emit_adrp_veneer() and module_emit_plt_entry() are not part of the veneer themselves: they are ordinary routines that are part of the module loader, and which populate the allocated veneer space on demand when encountering ADRP instructions that need to be rerouted.

...

Quoted 15 lines
Quoted 9 lines
Quoted 1 line
Quoted 1 line

rare case we might need to add more paranoid checks.

It's certainly rare: Adding the authors of the veneer to cc for comments:

Will, Ard, how probable are veneer-style symbols such as the one introduced in commit a257e0257 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" to happen again in the future?

Distro kernels typically enable full KASLR, so on systems that implement EFI_RNG_PROTOCOL, all function calls from modules into the kernel proper are redirected via veneers. (Note that these are bl instructions not adrp instructions though).

Quoted 8 lines
Quoted 3 lines

I would have thought WARNing on within-a-pagesize would be OK, Namhyung. Are you suggesting checking instead for a hardcoded veneer symbol string?

Veneers don't have symbol strings. Veneers are anonymous sequences of instructions living in a patch of R-X mapped module space somewhere. The only symbol strings are for the routines that generate these veneers, not for the veneers themselves.

OK, thanks, so AFAICT this means the function module_emit_adrp_veneer() should still show up in /proc/kallsyms, but it's not. I'm also seeing some weird characters, e.g., here there's a byte with a binary 02 value following a weird symbol named 'L14472' (part of the xfs module?):

$ grep L1 /proc/kallsyms | hexdump -C | head
<snip>
00000540  30 30 30 30 30 20 72 20  2e 4c 31 34 34 37 32 02  |00000 r .L14472.|
<snip>

arch__normalize_symbol_name() is a place where architecture code can clean up symbol names in perf, and I thought module_emit_adrp_veneer() was a veneer itself, but it seems that's not the case, so the literal string check for it shouldn't be needed. The test is still failing though because it doesn't show up in kallsyms...

Thanks,

Kim

#21 lore raw
Quoted 90 lines · Kim Phillips
Quoted 39 lines · On Fri, 20 Apr 2018 10:10:32 +0200 Ard Biesheuvel <[email protected]>…
Quoted 29 lines · Kim Phillips
Quoted 17 lines · On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <[email protected]> wrote:
Quoted 16 lines · Kim Phillips
tools/perf/arch/arm64/util/sym-handling.c +8 -0
index 0051b1ee8450..5c4a2e208bbc 100644
@@ -20,3 +20,16 @@ bool elf__needs_adjust_symbols(GElf_Ehdr ehdr)
               ehdr.e_type == ET_DYN;
 }
 #endif
+
+const char *arch__normalize_symbol_name(const char *name)
+{
+       /*
+        * arm64 kernels compensating for a CPU erratum can put up a
+        * module_emit_adrp_veneer in place of a module_emit_plt_entry
+        */
+       if (name && strlen(name) >= 23 &&
Quoted 8 lines

tools/perf/arch/arm64/util/sym-handling.c bool elf__needs_adjust_symbols(GElf_Ehdr ehdr) · line 35

Quoted 5 lines
tools/perf/arch/arm64/util/sym-handling.c +5 -0
+           !strncmp(name, "module_emit_adrp_veneer", 23))
+               return "module_emit_plt_entry";
+
+       return name;
+}

I don't know it's always preferable or just for the test. It it's the latter it may be better to move it to the test code.

Quoted 8 lines

AFACT, the veneer is a moniker and doesn't technically exist, and shouldn't be being looked-up. Both chunks of this diff are needed to pass perf test 1: this chunk above is because in arch__normalize_symbol_name(), we squash the perf test 1's "<veneer> not in *kallsyms*" problem, and in the below chunk, we prevent it coming up when the test code iterates over the *vmlinux* symbols. I.e. we need to prevent the veneer from coming up in both kallsyms *and* vmlinux.

Quoted 2 lines

I don't have all the context here, so I don't know what exactly arch__normalize_symbol_name() is trying to accomplish.

Sorry about that. The problem is that perf test 1 ("vmlinux symtab matches kallsyms") is failing, because module_emit_adrp_veneer exists in /boot/vmlinux but doesn't exist in /proc/kallsyms, and, sure enough, it's not in the latter.

Quoted 5 lines

What I do know is that module_emit_adrp_veneer() and module_emit_plt_entry() are not part of the veneer themselves: they are ordinary routines that are part of the module loader, and which populate the allocated veneer space on demand when encountering ADRP instructions that need to be rerouted.

...

Quoted 15 lines
Quoted 9 lines
Quoted 1 line
Quoted 1 line

rare case we might need to add more paranoid checks.

It's certainly rare: Adding the authors of the veneer to cc for comments:

Will, Ard, how probable are veneer-style symbols such as the one introduced in commit a257e0257 "arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419" to happen again in the future?

Distro kernels typically enable full KASLR, so on systems that implement EFI_RNG_PROTOCOL, all function calls from modules into the kernel proper are redirected via veneers. (Note that these are bl instructions not adrp instructions though).

Quoted 8 lines
Quoted 3 lines

I would have thought WARNing on within-a-pagesize would be OK, Namhyung. Are you suggesting checking instead for a hardcoded veneer symbol string?

Veneers don't have symbol strings. Veneers are anonymous sequences of instructions living in a patch of R-X mapped module space somewhere. The only symbol strings are for the routines that generate these veneers, not for the veneers themselves.

OK, thanks, so AFAICT this means the function module_emit_adrp_veneer() should still show up in /proc/kallsyms, but it's not. I'm also seeing some weird characters, e.g., here there's a byte with a binary 02 value following a weird symbol named 'L14472' (part of the xfs module?):

$ grep L1 /proc/kallsyms | hexdump -C | head
<snip>
00000540  30 30 30 30 30 20 72 20  2e 4c 31 34 34 37 32 02  |00000 r .L14472.|
<snip>
Quoted 6 lines

arch__normalize_symbol_name() is a place where architecture code can clean up symbol names in perf, and I thought module_emit_adrp_veneer() was a veneer itself, but it seems that's not the case, so the literal string check for it shouldn't be needed. The test is still failing though because it doesn't show up in kallsyms...

This turns out to be an unintended side effect of the fact that we (I?) taught kallsyms to disregard symbols ending in "_veneer"

So we should probably rename the function, and everything will be fine.

#22 lore raw
Quoted 11 lines · On Tue, 24 Apr 2018 08:13:57 +0200 Ard Biesheuvel <[email protected]>…
Quoted 5 lines · Kim Phillips

arch__normalize_symbol_name() is a place where architecture code can clean up symbol names in perf, and I thought module_emit_adrp_veneer() was a veneer itself, but it seems that's not the case, so the literal string check for it shouldn't be needed. The test is still failing though because it doesn't show up in kallsyms...

This turns out to be an unintended side effect of the fact that we (I?) taught kallsyms to disregard symbols ending in "_veneer"

So we should probably rename the function, and everything will be fine.

ok so what should we rename module_emit_adrp_veneer to?:

module_emit_adrp_veneer_
module_emit_adrp_veneer_fn
module_emit_adrp_veneer_nokallsyms

or..?

Kim

#23 lore raw
Quoted 15 lines · Kim Phillips
Quoted 11 lines · On Tue, 24 Apr 2018 08:13:57 +0200 Ard Biesheuvel <[email protected]>…
Quoted 5 lines · Kim Phillips

arch__normalize_symbol_name() is a place where architecture code can clean up symbol names in perf, and I thought module_emit_adrp_veneer() was a veneer itself, but it seems that's not the case, so the literal string check for it shouldn't be needed. The test is still failing though because it doesn't show up in kallsyms...

This turns out to be an unintended side effect of the fact that we (I?) taught kallsyms to disregard symbols ending in "_veneer"

So we should probably rename the function, and everything will be fine.

Quoted 8 lines

ok so what should we rename module_emit_adrp_veneer to?:

module_emit_adrp_veneer_
module_emit_adrp_veneer_fn
module_emit_adrp_veneer_nokallsyms

or..?

How about

module_emit_veneer_for_adrp

?

#24 lore raw

PATCH 1/1 arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp

Commit a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") introduced a function whose name ends with "_veneer".

This clashes with commit bd8b22d2888e ("Kbuild: kallsyms: ignore veneers emitted by the ARM linker"), which removes symbols ending in "_veneer" from kallsyms.

The problem was manifested as 'perf test -vvvvv vmlinux' failed, correctly claiming the symbol 'module_emit_adrp_veneer' was present in vmlinux, but not in kallsyms.

...

    ERR : 0xffff00000809aa58: module_emit_adrp_veneer not on kallsyms

...
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!

Fix the problem by renaming module_emit_veneer_for_adrp, module_emit_veneer_for_adrp. Now the test passes.

Fixes a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419")
Cc Ard Biesheuvel <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Kim Phillips <[email protected]>
---
 arch/arm64/include/asm/module.h | 2 +-
 arch/arm64/kernel/module-plts.c | 2 +-
 arch/arm64/kernel/module.c      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/include/asm/module.h +1 -1
index b6dbbe3123a9..97d0ef12e2ff 100644
@@ -39,7 +39,7 @@ struct mod_arch_specific {
 u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 			  Elf64_Sym *sym);
 
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val);
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val);
 
 #ifdef CONFIG_RANDOMIZE_BASE
 extern u64 module_alloc_base;
arch/arm64/kernel/module-plts.c +1 -1
index fa3637284a3d..f0690c2ca3e0 100644
@@ -43,7 +43,7 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 }
 
 #ifdef CONFIG_ARM64_ERRATUM_843419
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val)
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val)
 {
 	struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
 							  &mod->arch.init;
arch/arm64/kernel/module.c +1 -1
index 719fde8dcc19..155fd91e78f4 100644
@@ -215,7 +215,7 @@ static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 		insn &= ~BIT(31);
 	} else {
 		/* out of range for ADR -> emit a veneer */
-		val = module_emit_adrp_veneer(mod, place, val & ~0xfff);
+		val = module_emit_veneer_for_adrp(mod, place, val & ~0xfff);
 		if (!val)
 			return -ENOEXEC;
 		insn = aarch64_insn_gen_branch_imm((u64)place, val,
Signature · 1 line
2.17.0
Acked-by on v1 1/1
#25 lore raw

Re: [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp

Quoted 13 lines · Kim Phillips

Commit a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") introduced a function whose name ends with "_veneer".

This clashes with commit bd8b22d2888e ("Kbuild: kallsyms: ignore veneers emitted by the ARM linker"), which removes symbols ending in "_veneer" from kallsyms.

The problem was manifested as 'perf test -vvvvv vmlinux' failed, correctly claiming the symbol 'module_emit_adrp_veneer' was present in vmlinux, but not in kallsyms.

...

Quoted 8 lines
    ERR : 0xffff00000809aa58: module_emit_adrp_veneer not on kallsyms

...
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!

Fix the problem by renaming module_emit_veneer_for_adrp, module_emit_veneer_for_adrp.

That sentence didn't come out right

Quoted 8 lines

Now the test passes.

Fixes a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419")
Cc Ard Biesheuvel <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Kim Phillips <[email protected]>
Acked-by Ard Biesheuvel <[email protected]>
Quoted 48 lines
---
 arch/arm64/include/asm/module.h | 2 +-
 arch/arm64/kernel/module-plts.c | 2 +-
 arch/arm64/kernel/module.c      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/include/asm/module.h +1 -1
index b6dbbe3123a9..97d0ef12e2ff 100644
@@ -39,7 +39,7 @@ struct mod_arch_specific {
 u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
                          Elf64_Sym *sym);
 
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val);
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val);
 
 #ifdef CONFIG_RANDOMIZE_BASE
 extern u64 module_alloc_base;
arch/arm64/kernel/module-plts.c +1 -1
index fa3637284a3d..f0690c2ca3e0 100644
@@ -43,7 +43,7 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 }
 
 #ifdef CONFIG_ARM64_ERRATUM_843419
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val)
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val)
 {
        struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
                                                          &mod->arch.init;
arch/arm64/kernel/module.c +1 -2
index 719fde8dcc19..155fd91e78f4 100644
@@ -215,7 +215,7 @@ static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
                insn &= ~BIT(31);
        } else {
                /* out of range for ADR -> emit a veneer */
-               val = module_emit_adrp_veneer(mod, place, val & ~0xfff);
+               val = module_emit_veneer_for_adrp(mod, place, val & ~0xfff);
                if (!val)
                        return -ENOEXEC;
                insn = aarch64_insn_gen_branch_imm((u64)place, val,
--

2.17.0

#26 lore raw

PATCH v2 1/1 arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp

Commit a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") introduced a function whose name ends with "_veneer".

This clashes with commit bd8b22d2888e ("Kbuild: kallsyms: ignore veneers emitted by the ARM linker"), which removes symbols ending in "_veneer" from kallsyms.

The problem was manifested as 'perf test -vvvvv vmlinux' failed, correctly claiming the symbol 'module_emit_adrp_veneer' was present in vmlinux, but not in kallsyms.

...

    ERR : 0xffff00000809aa58: module_emit_adrp_veneer not on kallsyms

...
test child finished with -1
---- end ----
vmlinux symtab matches kallsyms: FAILED!

Fix the problem by renaming module_emit_adrp_veneer to module_emit_veneer_for_adrp. Now the test passes.

Fixes a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419")
Acked-by Ard Biesheuvel <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Michal Marek <[email protected]>
Signed-off-by Kim Phillips <[email protected]>

---
v2: fixed naming and wording of last sentence in commit text.

 arch/arm64/include/asm/module.h | 2 +-
 arch/arm64/kernel/module-plts.c | 2 +-
 arch/arm64/kernel/module.c      | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
arch/arm64/include/asm/module.h +1 -1
index b6dbbe3123a9..97d0ef12e2ff 100644
@@ -39,7 +39,7 @@ struct mod_arch_specific {
 u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 			  Elf64_Sym *sym);
 
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val);
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val);
 
 #ifdef CONFIG_RANDOMIZE_BASE
 extern u64 module_alloc_base;
arch/arm64/kernel/module-plts.c +1 -1
index fa3637284a3d..f0690c2ca3e0 100644
@@ -43,7 +43,7 @@ u64 module_emit_plt_entry(struct module *mod, void *loc, const Elf64_Rela *rela,
 }
 
 #ifdef CONFIG_ARM64_ERRATUM_843419
-u64 module_emit_adrp_veneer(struct module *mod, void *loc, u64 val)
+u64 module_emit_veneer_for_adrp(struct module *mod, void *loc, u64 val)
 {
 	struct mod_plt_sec *pltsec = !in_init(mod, loc) ? &mod->arch.core :
 							  &mod->arch.init;
arch/arm64/kernel/module.c +1 -1
index 719fde8dcc19..155fd91e78f4 100644
@@ -215,7 +215,7 @@ static int reloc_insn_adrp(struct module *mod, __le32 *place, u64 val)
 		insn &= ~BIT(31);
 	} else {
 		/* out of range for ADR -> emit a veneer */
-		val = module_emit_adrp_veneer(mod, place, val & ~0xfff);
+		val = module_emit_veneer_for_adrp(mod, place, val & ~0xfff);
 		if (!val)
 			return -ENOEXEC;
 		insn = aarch64_insn_gen_branch_imm((u64)place, val,
Signature · 1 line
2.17.0