* [PATCH] perf tools: set kernel end address properly @ 2018-04-16 4:22 Namhyung Kim 2018-04-16 9:23 ` Jiri Olsa 0 siblings, 1 reply; 30+ messages in thread From: Namhyung Kim @ 2018-04-16 4:22 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Kim Phillips 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 <kim.phillips@arm.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/machine.c | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 2eca8478e24f..be328416de61 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -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; } -- 2.16.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 4:22 [PATCH] perf tools: set kernel end address properly Namhyung Kim @ 2018-04-16 9:23 ` Jiri Olsa 2018-04-16 13:51 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 30+ messages in thread From: Jiri Olsa @ 2018-04-16 9:23 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Kim Phillips On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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 <kim.phillips@arm.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> looks good Acked-by: Jiri Olsa <jolsa@kernel.org> thanks, jirka > --- > tools/perf/util/machine.c | 20 ++++++++------------ > 1 file changed, 8 insertions(+), 12 deletions(-) > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > index 2eca8478e24f..be328416de61 100644 > --- a/tools/perf/util/machine.c > +++ b/tools/perf/util/machine.c > @@ -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; > } > > -- > 2.16.2 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 9:23 ` Jiri Olsa @ 2018-04-16 13:51 ` Arnaldo Carvalho de Melo 2018-04-16 16:07 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-04-16 13:51 UTC (permalink / raw) To: Jiri Olsa Cc: Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Kim Phillips Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > Reported-by: Kim Phillips <kim.phillips@arm.com> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > looks good > > Acked-by: Jiri Olsa <jolsa@kernel.org> > > thanks, > jirka > > > --- > > tools/perf/util/machine.c | 20 ++++++++------------ > > 1 file changed, 8 insertions(+), 12 deletions(-) > > > > diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c > > index 2eca8478e24f..be328416de61 100644 > > --- a/tools/perf/util/machine.c > > +++ b/tools/perf/util/machine.c > > @@ -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; > > } > > > > -- > > 2.16.2 > > ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 13:51 ` Arnaldo Carvalho de Melo @ 2018-04-16 16:07 ` Kim Phillips 2018-04-16 16:58 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-16 16:07 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Mon, 16 Apr 2018 10:51:25 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? No, perf test 1 still fails: vmlinux symtab matches kallsyms: FAILED! Kim ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 16:07 ` Kim Phillips @ 2018-04-16 16:58 ` Arnaldo Carvalho de Melo 2018-04-16 17:24 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-04-16 16:58 UTC (permalink / raw) To: Kim Phillips Cc: Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu: > On Mon, 16 Apr 2018 10:51:25 -0300 > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > No, perf test 1 still fails: > > vmlinux symtab matches kallsyms: FAILED! Ok, -vv please - Arnaldo ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 16:58 ` Arnaldo Carvalho de Melo @ 2018-04-16 17:24 ` Kim Phillips 2018-04-16 22:48 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-16 17:24 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Jiri Olsa, Namhyung Kim, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Mon, 16 Apr 2018 13:58:00 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu: > > On Mon, 16 Apr 2018 10:51:25 -0300 > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > > > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 17:24 ` Kim Phillips @ 2018-04-16 22:48 ` Kim Phillips 2018-04-17 2:27 ` Namhyung Kim 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-16 22:48 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Mon, 16 Apr 2018 12:24:07 -0500 Kim Phillips <kim.phillips@arm.com> wrote: > On Mon, 16 Apr 2018 13:58:00 -0300 > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu: > > > On Mon, 16 Apr 2018 10:51:25 -0300 > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > > > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > > > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > > > > > 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! 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-16 22:48 ` Kim Phillips @ 2018-04-17 2:27 ` Namhyung Kim 2018-04-19 0:37 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Namhyung Kim @ 2018-04-17 2:27 UTC (permalink / raw) To: Kim Phillips Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Mon, Apr 16, 2018 at 05:48:11PM -0500, Kim Phillips wrote: > On Mon, 16 Apr 2018 12:24:07 -0500 > Kim Phillips <kim.phillips@arm.com> wrote: > > > On Mon, 16 Apr 2018 13:58:00 -0300 > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu: > > > > On Mon, 16 Apr 2018 10:51:25 -0300 > > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > > > > > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > > > > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > > > > > > > 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! > > 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. > > 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. > > 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, ...)? > > 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<---------------- diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index 1e5adb65632a..b14ed70fa440 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -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); ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-17 2:27 ` Namhyung Kim @ 2018-04-19 0:37 ` Kim Phillips 2018-04-19 2:37 ` Namhyung Kim 2018-04-19 2:54 ` [PATCH] perf tools: set " Namhyung Kim 0 siblings, 2 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-19 0:37 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Tue, 17 Apr 2018 11:27:26 +0900 Namhyung Kim <namhyung@kernel.org> wrote: > On Mon, Apr 16, 2018 at 05:48:11PM -0500, Kim Phillips wrote: > > On Mon, 16 Apr 2018 12:24:07 -0500 > > Kim Phillips <kim.phillips@arm.com> wrote: > > > > > On Mon, 16 Apr 2018 13:58:00 -0300 > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > > > Em Mon, Apr 16, 2018 at 11:07:30AM -0500, Kim Phillips escreveu: > > > > > On Mon, 16 Apr 2018 10:51:25 -0300 > > > > > Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > > > > > > > > > > > Em Mon, Apr 16, 2018 at 11:23:45AM +0200, Jiri Olsa escreveu: > > > > > > > On Mon, Apr 16, 2018 at 01:22:40PM +0900, Namhyung Kim 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 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? > > > > > > > > > > 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! > > > > 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. > 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? > 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. > > 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. > > 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 > > 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 diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c index 0051b1ee8450..5c4a2e208bbc 100644 --- a/tools/perf/arch/arm64/util/sym-handling.c +++ b/tools/perf/arch/arm64/util/sym-handling.c @@ -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; +} diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c index 1e5adb65632a..07064e76947d 100644 --- a/tools/perf/tests/vmlinux-kallsyms.c +++ b/tools/perf/tests/vmlinux-kallsyms.c @@ -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); ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 0:37 ` Kim Phillips @ 2018-04-19 2:37 ` Namhyung Kim 2018-04-19 23:20 ` Kim Phillips 2018-04-19 2:54 ` [PATCH] perf tools: set " Namhyung Kim 1 sibling, 1 reply; 30+ messages in thread From: Namhyung Kim @ 2018-04-19 2:37 UTC (permalink / raw) To: Kim Phillips Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > On Tue, 17 Apr 2018 11:27:26 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > > On Mon, Apr 16, 2018 at 05:48:11PM -0500, Kim Phillips wrote: > > > > 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! > > > > > > 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. So ARM64 has modules below the kernel. > > > 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<--- diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index be328416de61..0f3c4bc7b90f 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -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; } ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 2:37 ` Namhyung Kim @ 2018-04-19 23:20 ` Kim Phillips 2018-04-19 23:59 ` Namhyung Kim 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-19 23:20 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Thu, 19 Apr 2018 11:37:12 +0900 Namhyung Kim <namhyung@kernel.org> wrote: > On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > > On Tue, 17 Apr 2018 11:27:26 +0900 > > Namhyung Kim <namhyung@kernel.org> wrote: > > > On Mon, Apr 16, 2018 at 05:48:11PM -0500, Kim Phillips wrote: > > > > > 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! > > > > > > > > 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. > > So ARM64 has modules below the kernel. right. > > > 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. > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 23:20 ` Kim Phillips @ 2018-04-19 23:59 ` Namhyung Kim 2018-04-20 23:23 ` Kim Phillips 2018-04-26 5:51 ` [tip:perf/urgent] perf machine: Set main " tip-bot for Namhyung Kim 0 siblings, 2 replies; 30+ messages in thread From: Namhyung Kim @ 2018-04-19 23:59 UTC (permalink / raw) To: Kim Phillips Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Thu, Apr 19, 2018 at 06:20:24PM -0500, Kim Phillips wrote: > On Thu, 19 Apr 2018 11:37:12 +0900 > Namhyung Kim <namhyung@kernel.org> 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. > > > > 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 >From 1d9fa3b21bd7a964af43dcc9d486a09917d795e9 Mon Sep 17 00:00:00 2001 From: Namhyung Kim <namhyung@kernel.org> 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 <kim.phillips@arm.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> --- tools/perf/util/machine.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 2eca8478e24f..32d50492505d 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -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; } -- 2.16.2 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 23:59 ` Namhyung Kim @ 2018-04-20 23:23 ` Kim Phillips 2018-04-23 13:52 ` Arnaldo Carvalho de Melo 2018-04-26 5:51 ` [tip:perf/urgent] perf machine: Set main " tip-bot for Namhyung Kim 1 sibling, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-20 23:23 UTC (permalink / raw) To: Namhyung Kim Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Fri, 20 Apr 2018 08:59:15 +0900 Namhyung Kim <namhyung@kernel.org> 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. > > Reported-by: Kim Phillips <kim.phillips@arm.com> > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > --- Acked-by: Kim Phillips <kim.phillips@arm.com> Thanks, Kim ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-20 23:23 ` Kim Phillips @ 2018-04-23 13:52 ` Arnaldo Carvalho de Melo 2018-04-23 14:56 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Arnaldo Carvalho de Melo @ 2018-04-23 13:52 UTC (permalink / raw) To: Kim Phillips Cc: Namhyung Kim, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team Em Fri, Apr 20, 2018 at 06:23:58PM -0500, Kim Phillips escreveu: > On Fri, 20 Apr 2018 08:59:15 +0900 > Namhyung Kim <namhyung@kernel.org> 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. > > > > Reported-by: Kim Phillips <kim.phillips@arm.com> > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > --- > > Acked-by: Kim Phillips <kim.phillips@arm.com> 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-23 13:52 ` Arnaldo Carvalho de Melo @ 2018-04-23 14:56 ` Kim Phillips 0 siblings, 0 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-23 14:56 UTC (permalink / raw) To: Arnaldo Carvalho de Melo Cc: Namhyung Kim, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Mon, 23 Apr 2018 10:52:21 -0300 Arnaldo Carvalho de Melo <acme@kernel.org> wrote: > Em Fri, Apr 20, 2018 at 06:23:58PM -0500, Kim Phillips escreveu: > > On Fri, 20 Apr 2018 08:59:15 +0900 > > Namhyung Kim <namhyung@kernel.org> 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. > > > > > > Reported-by: Kim Phillips <kim.phillips@arm.com> > > > Signed-off-by: Namhyung Kim <namhyung@kernel.org> > > > --- > > > > Acked-by: Kim Phillips <kim.phillips@arm.com> > > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* [tip:perf/urgent] perf machine: Set main kernel end address properly 2018-04-19 23:59 ` Namhyung Kim 2018-04-20 23:23 ` Kim Phillips @ 2018-04-26 5:51 ` tip-bot for Namhyung Kim 1 sibling, 0 replies; 30+ messages in thread From: tip-bot for Namhyung Kim @ 2018-04-26 5:51 UTC (permalink / raw) To: linux-tip-commits Cc: namhyung, jolsa, hpa, kim.phillips, tglx, acme, mingo, linux-kernel, peterz Commit-ID: ee05d21791db6db954bbb7b79bb18d88b5f6b7ff Gitweb: https://git.kernel.org/tip/ee05d21791db6db954bbb7b79bb18d88b5f6b7ff Author: Namhyung Kim <namhyung@kernel.org> AuthorDate: Mon, 19 Feb 2018 19:05:45 +0900 Committer: Arnaldo Carvalho de Melo <acme@redhat.com> 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 <kim.phillips@arm.com> Tested-by: Kim Phillips <kim.phillips@arm.com> Signed-off-by: Namhyung Kim <namhyung@kernel.org> Cc: Jiri Olsa <jolsa@kernel.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: kernel-team@lge.com Link: http://lkml.kernel.org/r/20180419235915.GA19067@sejong Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com> --- tools/perf/util/machine.c | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c index 2eca8478e24f..32d50492505d 100644 --- a/tools/perf/util/machine.c +++ b/tools/perf/util/machine.c @@ -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; } ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 0:37 ` Kim Phillips 2018-04-19 2:37 ` Namhyung Kim @ 2018-04-19 2:54 ` Namhyung Kim 2018-04-19 23:33 ` Kim Phillips 1 sibling, 1 reply; 30+ messages in thread From: Namhyung Kim @ 2018-04-19 2:54 UTC (permalink / raw) To: Kim Phillips Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c > index 0051b1ee8450..5c4a2e208bbc 100644 > --- a/tools/perf/arch/arm64/util/sym-handling.c > +++ b/tools/perf/arch/arm64/util/sym-handling.c > @@ -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; > +} 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. > diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c > index 1e5adb65632a..07064e76947d 100644 > --- a/tools/perf/tests/vmlinux-kallsyms.c > +++ b/tools/perf/tests/vmlinux-kallsyms.c > @@ -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) 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 > + { > + 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); ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 2:54 ` [PATCH] perf tools: set " Namhyung Kim @ 2018-04-19 23:33 ` Kim Phillips 2018-04-20 0:11 ` Namhyung Kim 2018-04-20 8:10 ` Ard Biesheuvel 0 siblings, 2 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-19 23:33 UTC (permalink / raw) To: Namhyung Kim, Ard Biesheuvel, Will Deacon Cc: Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Thu, 19 Apr 2018 11:54:24 +0900 Namhyung Kim <namhyung@kernel.org> wrote: > On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c > > index 0051b1ee8450..5c4a2e208bbc 100644 > > --- a/tools/perf/arch/arm64/util/sym-handling.c > > +++ b/tools/perf/arch/arm64/util/sym-handling.c > > @@ -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; > > +} > > 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. > > diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c > > index 1e5adb65632a..07064e76947d 100644 > > --- a/tools/perf/tests/vmlinux-kallsyms.c > > +++ b/tools/perf/tests/vmlinux-kallsyms.c > > @@ -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) > > It seems that we needs to check it's the ARM64 at least. If it's a OK. > 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 > > + { > > + 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); ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 23:33 ` Kim Phillips @ 2018-04-20 0:11 ` Namhyung Kim 2018-04-20 8:10 ` Ard Biesheuvel 1 sibling, 0 replies; 30+ messages in thread From: Namhyung Kim @ 2018-04-20 0:11 UTC (permalink / raw) To: Kim Phillips Cc: Ard Biesheuvel, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Thu, Apr 19, 2018 at 06:33:13PM -0500, Kim Phillips wrote: > On Thu, 19 Apr 2018 11:54:24 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > > > On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > > > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c > > > index 0051b1ee8450..5c4a2e208bbc 100644 > > > --- a/tools/perf/arch/arm64/util/sym-handling.c > > > +++ b/tools/perf/arch/arm64/util/sym-handling.c > > > @@ -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; > > > +} > > > > 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. > > > > diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c > > > index 1e5adb65632a..07064e76947d 100644 > > > --- a/tools/perf/tests/vmlinux-kallsyms.c > > > +++ b/tools/perf/tests/vmlinux-kallsyms.c > > > @@ -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) > > > > It seems that we needs to check it's the ARM64 at least. If it's a > > OK. > > > 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? Anything to prevent false-negatives (possibly on other archs). Thanks, Namhyung > > Thanks, > > Kim > > > > + { > > > + 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); ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-19 23:33 ` Kim Phillips 2018-04-20 0:11 ` Namhyung Kim @ 2018-04-20 8:10 ` Ard Biesheuvel 2018-04-23 21:43 ` Kim Phillips 1 sibling, 1 reply; 30+ messages in thread From: Ard Biesheuvel @ 2018-04-20 8:10 UTC (permalink / raw) To: Kim Phillips Cc: Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On 20 April 2018 at 01:33, Kim Phillips <kim.phillips@arm.com> wrote: > On Thu, 19 Apr 2018 11:54:24 +0900 > Namhyung Kim <namhyung@kernel.org> wrote: > >> On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: >> > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c >> > index 0051b1ee8450..5c4a2e208bbc 100644 >> > --- a/tools/perf/arch/arm64/util/sym-handling.c >> > +++ b/tools/perf/arch/arm64/util/sym-handling.c >> > @@ -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; >> > +} >> >> 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. > 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. >> > diff --git a/tools/perf/tests/vmlinux-kallsyms.c b/tools/perf/tests/vmlinux-kallsyms.c >> > index 1e5adb65632a..07064e76947d 100644 >> > --- a/tools/perf/tests/vmlinux-kallsyms.c >> > +++ b/tools/perf/tests/vmlinux-kallsyms.c >> > @@ -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) >> >> It seems that we needs to check it's the ARM64 at least. If it's a > > OK. > >> 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). > 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. >> > + 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); ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-20 8:10 ` Ard Biesheuvel @ 2018-04-23 21:43 ` Kim Phillips 2018-04-24 6:13 ` Ard Biesheuvel 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-23 21:43 UTC (permalink / raw) To: Ard Biesheuvel Cc: Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Fri, 20 Apr 2018 10:10:32 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 20 April 2018 at 01:33, Kim Phillips <kim.phillips@arm.com> wrote: > > On Thu, 19 Apr 2018 11:54:24 +0900 > > Namhyung Kim <namhyung@kernel.org> wrote: > > > >> On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: > >> > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c > >> > index 0051b1ee8450..5c4a2e208bbc 100644 > >> > --- a/tools/perf/arch/arm64/util/sym-handling.c > >> > +++ b/tools/perf/arch/arm64/util/sym-handling.c > >> > @@ -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; > >> > +} > >> > >> 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. > > 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. > 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. ... > > >> 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). > > > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-23 21:43 ` Kim Phillips @ 2018-04-24 6:13 ` Ard Biesheuvel 2018-04-24 12:50 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Ard Biesheuvel @ 2018-04-24 6:13 UTC (permalink / raw) To: Kim Phillips Cc: Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On 23 April 2018 at 23:43, Kim Phillips <kim.phillips@arm.com> wrote: > On Fri, 20 Apr 2018 10:10:32 +0200 > Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> On 20 April 2018 at 01:33, Kim Phillips <kim.phillips@arm.com> wrote: >> > On Thu, 19 Apr 2018 11:54:24 +0900 >> > Namhyung Kim <namhyung@kernel.org> wrote: >> > >> >> On Wed, Apr 18, 2018 at 07:37:59PM -0500, Kim Phillips wrote: >> >> > diff --git a/tools/perf/arch/arm64/util/sym-handling.c b/tools/perf/arch/arm64/util/sym-handling.c >> >> > index 0051b1ee8450..5c4a2e208bbc 100644 >> >> > --- a/tools/perf/arch/arm64/util/sym-handling.c >> >> > +++ b/tools/perf/arch/arm64/util/sym-handling.c >> >> > @@ -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; >> >> > +} >> >> >> >> 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. >> >> 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. > >> 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. > ... >> > >> 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). >> >> > 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... > 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. ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-24 6:13 ` Ard Biesheuvel @ 2018-04-24 12:50 ` Kim Phillips 2018-04-24 13:07 ` Ard Biesheuvel 0 siblings, 1 reply; 30+ messages in thread From: Kim Phillips @ 2018-04-24 12:50 UTC (permalink / raw) To: Ard Biesheuvel Cc: Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On Tue, 24 Apr 2018 08:13:57 +0200 Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 23 April 2018 at 23:43, Kim Phillips <kim.phillips@arm.com> wrote: > > 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 ^ permalink raw reply [flat|nested] 30+ messages in thread
* Re: [PATCH] perf tools: set kernel end address properly 2018-04-24 12:50 ` Kim Phillips @ 2018-04-24 13:07 ` Ard Biesheuvel 2018-04-24 15:13 ` Kim Phillips 0 siblings, 1 reply; 30+ messages in thread From: Ard Biesheuvel @ 2018-04-24 13:07 UTC (permalink / raw) To: Kim Phillips Cc: Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team On 24 April 2018 at 14:50, Kim Phillips <kim.phillips@arm.com> wrote: > On Tue, 24 Apr 2018 08:13:57 +0200 > Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > >> On 23 April 2018 at 23:43, Kim Phillips <kim.phillips@arm.com> wrote: >> > 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..? > How about module_emit_veneer_for_adrp ? ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp 2018-04-24 13:07 ` Ard Biesheuvel @ 2018-04-24 15:13 ` Kim Phillips 0 siblings, 0 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-24 15:13 UTC (permalink / raw) To: Ard Biesheuvel Cc: Catalin Marinas, linux-arm-kernel, Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Michal Marek 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 <ard.biesheuvel@linaro.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Michal Marek <mmarek@suse.cz> Signed-off-by: Kim Phillips <kim.phillips@arm.com> --- 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(-) diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h index b6dbbe3123a9..97d0ef12e2ff 100644 --- a/arch/arm64/include/asm/module.h +++ b/arch/arm64/include/asm/module.h @@ -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; diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c index fa3637284a3d..f0690c2ca3e0 100644 --- a/arch/arm64/kernel/module-plts.c +++ b/arch/arm64/kernel/module-plts.c @@ -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; diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index 719fde8dcc19..155fd91e78f4 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp @ 2018-04-24 15:13 ` Kim Phillips 0 siblings, 0 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-24 15:13 UTC (permalink / raw) To: linux-arm-kernel 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 <ard.biesheuvel@linaro.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Michal Marek <mmarek@suse.cz> Signed-off-by: Kim Phillips <kim.phillips@arm.com> --- 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(-) diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h index b6dbbe3123a9..97d0ef12e2ff 100644 --- a/arch/arm64/include/asm/module.h +++ b/arch/arm64/include/asm/module.h @@ -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; diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c index fa3637284a3d..f0690c2ca3e0 100644 --- a/arch/arm64/kernel/module-plts.c +++ b/arch/arm64/kernel/module-plts.c @@ -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; diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index 719fde8dcc19..155fd91e78f4 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* Re: [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp 2018-04-24 15:13 ` Kim Phillips @ 2018-04-24 15:15 ` Ard Biesheuvel -1 siblings, 0 replies; 30+ messages in thread From: Ard Biesheuvel @ 2018-04-24 15:15 UTC (permalink / raw) To: Kim Phillips Cc: Catalin Marinas, linux-arm-kernel, Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Michal Marek On 24 April 2018 at 17:13, Kim Phillips <kim.phillips@arm.com> wrote: > 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. That sentence didn't come out right > Now the test passes. > > Fixes: a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Michal Marek <mmarek@suse.cz> > Signed-off-by: Kim Phillips <kim.phillips@arm.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > 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(-) > > diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h > index b6dbbe3123a9..97d0ef12e2ff 100644 > --- a/arch/arm64/include/asm/module.h > +++ b/arch/arm64/include/asm/module.h > @@ -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; > diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c > index fa3637284a3d..f0690c2ca3e0 100644 > --- a/arch/arm64/kernel/module-plts.c > +++ b/arch/arm64/kernel/module-plts.c > @@ -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; > diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c > index 719fde8dcc19..155fd91e78f4 100644 > --- a/arch/arm64/kernel/module.c > +++ b/arch/arm64/kernel/module.c > @@ -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 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp @ 2018-04-24 15:15 ` Ard Biesheuvel 0 siblings, 0 replies; 30+ messages in thread From: Ard Biesheuvel @ 2018-04-24 15:15 UTC (permalink / raw) To: linux-arm-kernel On 24 April 2018 at 17:13, Kim Phillips <kim.phillips@arm.com> wrote: > 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. That sentence didn't come out right > Now the test passes. > > Fixes: a257e02579e ("arm64/kernel: don't ban ADRP to work around Cortex-A53 erratum #843419") > Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org> > Cc: Will Deacon <will.deacon@arm.com> > Cc: Catalin Marinas <catalin.marinas@arm.com> > Cc: Michal Marek <mmarek@suse.cz> > Signed-off-by: Kim Phillips <kim.phillips@arm.com> Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > 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(-) > > diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h > index b6dbbe3123a9..97d0ef12e2ff 100644 > --- a/arch/arm64/include/asm/module.h > +++ b/arch/arm64/include/asm/module.h > @@ -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; > diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c > index fa3637284a3d..f0690c2ca3e0 100644 > --- a/arch/arm64/kernel/module-plts.c > +++ b/arch/arm64/kernel/module-plts.c > @@ -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; > diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c > index 719fde8dcc19..155fd91e78f4 100644 > --- a/arch/arm64/kernel/module.c > +++ b/arch/arm64/kernel/module.c > @@ -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 > ^ permalink raw reply [flat|nested] 30+ messages in thread
* [PATCH v2] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp 2018-04-24 15:15 ` Ard Biesheuvel @ 2018-04-24 15:39 ` Kim Phillips -1 siblings, 0 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-24 15:39 UTC (permalink / raw) To: Ard Biesheuvel Cc: Catalin Marinas, linux-arm-kernel, Namhyung Kim, Will Deacon, Arnaldo Carvalho de Melo, Jiri Olsa, Ingo Molnar, Peter Zijlstra, Jiri Olsa, LKML, kernel-team, Michal Marek 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 <ard.biesheuvel@linaro.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Michal Marek <mmarek@suse.cz> Signed-off-by: Kim Phillips <kim.phillips@arm.com> --- 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(-) diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h index b6dbbe3123a9..97d0ef12e2ff 100644 --- a/arch/arm64/include/asm/module.h +++ b/arch/arm64/include/asm/module.h @@ -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; diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c index fa3637284a3d..f0690c2ca3e0 100644 --- a/arch/arm64/kernel/module-plts.c +++ b/arch/arm64/kernel/module-plts.c @@ -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; diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index 719fde8dcc19..155fd91e78f4 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
* [PATCH v2] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp @ 2018-04-24 15:39 ` Kim Phillips 0 siblings, 0 replies; 30+ messages in thread From: Kim Phillips @ 2018-04-24 15:39 UTC (permalink / raw) To: linux-arm-kernel 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 <ard.biesheuvel@linaro.org> Cc: Will Deacon <will.deacon@arm.com> Cc: Catalin Marinas <catalin.marinas@arm.com> Cc: Michal Marek <mmarek@suse.cz> Signed-off-by: Kim Phillips <kim.phillips@arm.com> --- 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(-) diff --git a/arch/arm64/include/asm/module.h b/arch/arm64/include/asm/module.h index b6dbbe3123a9..97d0ef12e2ff 100644 --- a/arch/arm64/include/asm/module.h +++ b/arch/arm64/include/asm/module.h @@ -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; diff --git a/arch/arm64/kernel/module-plts.c b/arch/arm64/kernel/module-plts.c index fa3637284a3d..f0690c2ca3e0 100644 --- a/arch/arm64/kernel/module-plts.c +++ b/arch/arm64/kernel/module-plts.c @@ -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; diff --git a/arch/arm64/kernel/module.c b/arch/arm64/kernel/module.c index 719fde8dcc19..155fd91e78f4 100644 --- a/arch/arm64/kernel/module.c +++ b/arch/arm64/kernel/module.c @@ -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 ^ permalink raw reply related [flat|nested] 30+ messages in thread
end of thread, other threads:[~2018-04-26 5:52 UTC | newest] Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-16 4:22 [PATCH] perf tools: set kernel end address properly Namhyung Kim 2018-04-16 9:23 ` Jiri Olsa 2018-04-16 13:51 ` Arnaldo Carvalho de Melo 2018-04-16 16:07 ` Kim Phillips 2018-04-16 16:58 ` Arnaldo Carvalho de Melo 2018-04-16 17:24 ` Kim Phillips 2018-04-16 22:48 ` Kim Phillips 2018-04-17 2:27 ` Namhyung Kim 2018-04-19 0:37 ` Kim Phillips 2018-04-19 2:37 ` Namhyung Kim 2018-04-19 23:20 ` Kim Phillips 2018-04-19 23:59 ` Namhyung Kim 2018-04-20 23:23 ` Kim Phillips 2018-04-23 13:52 ` Arnaldo Carvalho de Melo 2018-04-23 14:56 ` Kim Phillips 2018-04-26 5:51 ` [tip:perf/urgent] perf machine: Set main " tip-bot for Namhyung Kim 2018-04-19 2:54 ` [PATCH] perf tools: set " Namhyung Kim 2018-04-19 23:33 ` Kim Phillips 2018-04-20 0:11 ` Namhyung Kim 2018-04-20 8:10 ` Ard Biesheuvel 2018-04-23 21:43 ` Kim Phillips 2018-04-24 6:13 ` Ard Biesheuvel 2018-04-24 12:50 ` Kim Phillips 2018-04-24 13:07 ` Ard Biesheuvel 2018-04-24 15:13 ` [PATCH] arm64/kernel: rename module_emit_adrp_veneer->module_emit_veneer_for_adrp Kim Phillips 2018-04-24 15:13 ` Kim Phillips 2018-04-24 15:15 ` Ard Biesheuvel 2018-04-24 15:15 ` Ard Biesheuvel 2018-04-24 15:39 ` [PATCH v2] " Kim Phillips 2018-04-24 15:39 ` Kim Phillips
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.