2015-04-10 13:07:07

by Denys Vlasenko

Subject: [PATCH 1/4] perf/x86_64: get_regs_user: Do not guess user_regs->cs,ss,sp

After recent changes to syscall entry points, user_regs->{cs,ss,sp}
are always correct. (They used to be undefined while in syscalls).

We can report them reliably, without guessing.

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Jiri Olsa <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Arnaldo Carvalho de Melo <[email protected]>
CC: [email protected]
CC: [email protected]
---
arch/x86/kernel/perf_regs.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 02a8720..7ab198a 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -131,8 +131,8 @@ void perf_get_regs_user(struct perf_regs *regs_user,
}

/*
- * RIP, flags, and the argument registers are usually saved.
- * orig_ax is probably okay, too.
+ * These registers are always saved on 64-bit syscall entry.
+ * On 32-bit entry points, they are saved too except r8..r11.
*/
regs_user_copy->ip = user_regs->ip;
regs_user_copy->cx = user_regs->cx;
@@ -145,9 +145,12 @@ void perf_get_regs_user(struct perf_regs *regs_user,
regs_user_copy->r11 = user_regs->r11;
regs_user_copy->orig_ax = user_regs->orig_ax;
regs_user_copy->flags = user_regs->flags;
+ regs_user_copy->sp = user_regs->sp;
+ regs_user_copy->cs = user_regs->cs;
+ regs_user_copy->ss = user_regs->ss;

/*
- * Don't even try to report the "rest" regs.
+ * Most system calls don't save these registers, don't report them.
*/
regs_user_copy->bx = -1;
regs_user_copy->bp = -1;
@@ -158,7 +161,7 @@ void perf_get_regs_user(struct perf_regs *regs_user,

/*
* For this to be at all useful, we need a reasonable guess for
- * sp and the ABI. Be careful: we're in NMI context, and we're
+ * the ABI. Be careful: we're in NMI context, and we're
* considering current to be the current task, so we should
* be careful not to look at any other percpu variables that might
* change during context switches.
@@ -167,9 +170,6 @@ void perf_get_regs_user(struct perf_regs *regs_user,
task_thread_info(current)->status & TS_COMPAT) {
/* Easy case: we're in a compat syscall. */
regs_user->abi = PERF_SAMPLE_REGS_ABI_32;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = user_regs->cs;
- regs_user_copy->ss = user_regs->ss;
} else if (user_regs->orig_ax != -1) {
/*
* We're probably in a 64-bit syscall.
@@ -177,17 +177,12 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* than just blindly copying user_regs.
*/
regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = __USER_CS;
- regs_user_copy->ss = __USER_DS;
- regs_user_copy->cx = -1; /* usually contains garbage */
+ /* usually contains return address (same as ->ip) */
+ regs_user_copy->cx = -1;
} else {
/* We're probably in an interrupt or exception. */
regs_user->abi = user_64bit_mode(user_regs) ?
PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = user_regs->cs;
- regs_user_copy->ss = user_regs->ss;
}

regs_user->regs = regs_user_copy;
--
1.8.1.4

2015-04-10 13:07:26

by Denys Vlasenko

Subject: [PATCH 2/4] perf/x86_64: get_regs_user: Do report user_regs->cx while we are in syscall

Yes, it is true that cx contains return address.
It's not clear why we trash it.
Stop doing that.

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Jiri Olsa <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Arnaldo Carvalho de Melo <[email protected]>
CC: [email protected]
CC: [email protected]
---
arch/x86/kernel/perf_regs.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 7ab198a..a8d4e48 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -177,8 +177,6 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* than just blindly copying user_regs.
*/
regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- /* usually contains return address (same as ->ip) */
- regs_user_copy->cx = -1;
} else {
/* We're probably in an interrupt or exception. */
regs_user->abi = user_64bit_mode(user_regs) ?
--
1.8.1.4

2015-04-10 13:07:31

by Denys Vlasenko

Subject: [PATCH 3/4] perf/x86_64: get_regs_user: Simplify regs_user->abi setting code

user_64bit_mode(regs) basically checks regs->cs to point to a 64-bit segment.
This check used to be unreliable here because regs->cs was not always correct
in syscalls.

Now regs->cs is always correct: in syscalls, in interrupts, in exceptions.
No need to emply heuristics here.

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Jiri Olsa <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Arnaldo Carvalho de Melo <[email protected]>
CC: [email protected]
CC: [email protected]
---
arch/x86/kernel/perf_regs.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index a8d4e48..8157d39 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -166,22 +166,8 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* be careful not to look at any other percpu variables that might
* change during context switches.
*/
- if (IS_ENABLED(CONFIG_IA32_EMULATION) &&
- task_thread_info(current)->status & TS_COMPAT) {
- /* Easy case: we're in a compat syscall. */
- regs_user->abi = PERF_SAMPLE_REGS_ABI_32;
- } else if (user_regs->orig_ax != -1) {
- /*
- * We're probably in a 64-bit syscall.
- * Warning: this code is severely racy. At least it's better
- * than just blindly copying user_regs.
- */
- regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- } else {
- /* We're probably in an interrupt or exception. */
- regs_user->abi = user_64bit_mode(user_regs) ?
- PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;
- }
+ regs_user->abi = user_64bit_mode(user_regs) ?
+ PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;

regs_user->regs = regs_user_copy;
}
--
1.8.1.4

2015-04-10 13:07:22

by Denys Vlasenko

Subject: [PATCH 4/4] perf/x86_64: get_regs_user: Report regs_user->ax too

I don't see why we report e.g. orix_ax, which is not always meaningful,
but don't report ax, which is meaningful.

Signed-off-by: Denys Vlasenko <[email protected]>
CC: Jiri Olsa <[email protected]>
CC: Peter Zijlstra <[email protected]>
CC: Paul Mackerras <[email protected]>
CC: Ingo Molnar <[email protected]>
CC: Arnaldo Carvalho de Melo <[email protected]>
CC: [email protected]
CC: [email protected]
---
arch/x86/kernel/perf_regs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 8157d39..da8cb98 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -135,6 +135,7 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* On 32-bit entry points, they are saved too except r8..r11.
*/
regs_user_copy->ip = user_regs->ip;
+ regs_user_copy->ax = user_regs->ax;
regs_user_copy->cx = user_regs->cx;
regs_user_copy->dx = user_regs->dx;
regs_user_copy->si = user_regs->si;
--
1.8.1.4

2015-04-12 17:26:35

by tip-bot for Vasyl Gomonovych

Subject: [tip:x86/asm] perf/x86/64: Do not guess user_regs->cs, ss, sp in get_regs_user()

Commit-ID: aa21df0424468dbd991440d41ba0f700c5997103
Gitweb: http://git.kernel.org/tip/aa21df0424468dbd991440d41ba0f700c5997103
Author: Denys Vlasenko <[email protected]>
AuthorDate: Fri, 10 Apr 2015 15:06:56 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 11 Apr 2015 13:08:52 +0200

perf/x86/64: Do not guess user_regs->cs, ss, sp in get_regs_user()

After recent changes to syscall entry points,
user_regs->{cs,ss,sp} are always correct. (They used to be
undefined while in syscalls).

We can report them reliably, without guessing.

Signed-off-by: Denys Vlasenko <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/perf_regs.c | 23 +++++++++--------------
1 file changed, 9 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 02a8720..7ab198a 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -131,8 +131,8 @@ void perf_get_regs_user(struct perf_regs *regs_user,
}

/*
- * RIP, flags, and the argument registers are usually saved.
- * orig_ax is probably okay, too.
+ * These registers are always saved on 64-bit syscall entry.
+ * On 32-bit entry points, they are saved too except r8..r11.
*/
regs_user_copy->ip = user_regs->ip;
regs_user_copy->cx = user_regs->cx;
@@ -145,9 +145,12 @@ void perf_get_regs_user(struct perf_regs *regs_user,
regs_user_copy->r11 = user_regs->r11;
regs_user_copy->orig_ax = user_regs->orig_ax;
regs_user_copy->flags = user_regs->flags;
+ regs_user_copy->sp = user_regs->sp;
+ regs_user_copy->cs = user_regs->cs;
+ regs_user_copy->ss = user_regs->ss;

/*
- * Don't even try to report the "rest" regs.
+ * Most system calls don't save these registers, don't report them.
*/
regs_user_copy->bx = -1;
regs_user_copy->bp = -1;
@@ -158,7 +161,7 @@ void perf_get_regs_user(struct perf_regs *regs_user,

/*
* For this to be at all useful, we need a reasonable guess for
- * sp and the ABI. Be careful: we're in NMI context, and we're
+ * the ABI. Be careful: we're in NMI context, and we're
* considering current to be the current task, so we should
* be careful not to look at any other percpu variables that might
* change during context switches.
@@ -167,9 +170,6 @@ void perf_get_regs_user(struct perf_regs *regs_user,
task_thread_info(current)->status & TS_COMPAT) {
/* Easy case: we're in a compat syscall. */
regs_user->abi = PERF_SAMPLE_REGS_ABI_32;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = user_regs->cs;
- regs_user_copy->ss = user_regs->ss;
} else if (user_regs->orig_ax != -1) {
/*
* We're probably in a 64-bit syscall.
@@ -177,17 +177,12 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* than just blindly copying user_regs.
*/
regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = __USER_CS;
- regs_user_copy->ss = __USER_DS;
- regs_user_copy->cx = -1; /* usually contains garbage */
+ /* usually contains return address (same as ->ip) */
+ regs_user_copy->cx = -1;
} else {
/* We're probably in an interrupt or exception. */
regs_user->abi = user_64bit_mode(user_regs) ?
PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;
- regs_user_copy->sp = user_regs->sp;
- regs_user_copy->cs = user_regs->cs;
- regs_user_copy->ss = user_regs->ss;
}

regs_user->regs = regs_user_copy;

2015-04-12 17:26:51

by tip-bot for Vasyl Gomonovych

Subject: [tip:x86/asm] perf/x86/64: Do report user_regs-> cx while we are in syscall, in get_regs_user()

Commit-ID: 5df71b396b2d1fdd9d9f5a33e2eda5dc27c5632d
Gitweb: http://git.kernel.org/tip/5df71b396b2d1fdd9d9f5a33e2eda5dc27c5632d
Author: Denys Vlasenko <[email protected]>
AuthorDate: Fri, 10 Apr 2015 15:06:57 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 11 Apr 2015 13:08:53 +0200

perf/x86/64: Do report user_regs->cx while we are in syscall, in get_regs_user()

Yes, it is true that cx contains return address.
It's not clear why we trash it.
Stop doing that.

Signed-off-by: Denys Vlasenko <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/perf_regs.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 7ab198a..a8d4e48 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -177,8 +177,6 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* than just blindly copying user_regs.
*/
regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- /* usually contains return address (same as ->ip) */
- regs_user_copy->cx = -1;
} else {
/* We're probably in an interrupt or exception. */
regs_user->abi = user_64bit_mode(user_regs) ?

2015-04-12 17:26:58

by tip-bot for Vasyl Gomonovych

Subject: [tip:x86/asm] perf/x86/64: Simplify regs_user-> abi setting code in get_regs_user()

Commit-ID: 32caa06091cc59651222cdc971dc21eaab36b097
Gitweb: http://git.kernel.org/tip/32caa06091cc59651222cdc971dc21eaab36b097
Author: Denys Vlasenko <[email protected]>
AuthorDate: Fri, 10 Apr 2015 15:06:58 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 11 Apr 2015 13:08:53 +0200

perf/x86/64: Simplify regs_user->abi setting code in get_regs_user()

user_64bit_mode(regs) basically checks regs->cs to point to a
64-bit segment. This check used to be unreliable here because
regs->cs was not always correct in syscalls.

Now regs->cs is always correct: in syscalls, in interrupts, in
exceptions. No need to emply heuristics here.

Signed-off-by: Denys Vlasenko <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/perf_regs.c | 18 ++----------------
1 file changed, 2 insertions(+), 16 deletions(-)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index a8d4e48..8157d39 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -166,22 +166,8 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* be careful not to look at any other percpu variables that might
* change during context switches.
*/
- if (IS_ENABLED(CONFIG_IA32_EMULATION) &&
- task_thread_info(current)->status & TS_COMPAT) {
- /* Easy case: we're in a compat syscall. */
- regs_user->abi = PERF_SAMPLE_REGS_ABI_32;
- } else if (user_regs->orig_ax != -1) {
- /*
- * We're probably in a 64-bit syscall.
- * Warning: this code is severely racy. At least it's better
- * than just blindly copying user_regs.
- */
- regs_user->abi = PERF_SAMPLE_REGS_ABI_64;
- } else {
- /* We're probably in an interrupt or exception. */
- regs_user->abi = user_64bit_mode(user_regs) ?
- PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;
- }
+ regs_user->abi = user_64bit_mode(user_regs) ?
+ PERF_SAMPLE_REGS_ABI_64 : PERF_SAMPLE_REGS_ABI_32;

regs_user->regs = regs_user_copy;
}

2015-04-12 17:27:15

by tip-bot for Vasyl Gomonovych

Subject: [tip:x86/asm] perf/x86/64: Report regs_user-> ax too in get_regs_user()

Commit-ID: 3b75232d55680ca166dffa274d0587d5faf0a016
Gitweb: http://git.kernel.org/tip/3b75232d55680ca166dffa274d0587d5faf0a016
Author: Denys Vlasenko <[email protected]>
AuthorDate: Fri, 10 Apr 2015 15:06:59 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Sat, 11 Apr 2015 13:08:53 +0200

perf/x86/64: Report regs_user->ax too in get_regs_user()

I don't see why we report e.g. orix_ax, which is not always
meaningful, but don't report ax, which is meaningful.

Signed-off-by: Denys Vlasenko <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Cc: Jiri Olsa <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/perf_regs.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/x86/kernel/perf_regs.c b/arch/x86/kernel/perf_regs.c
index 8157d39..da8cb98 100644
--- a/arch/x86/kernel/perf_regs.c
+++ b/arch/x86/kernel/perf_regs.c
@@ -135,6 +135,7 @@ void perf_get_regs_user(struct perf_regs *regs_user,
* On 32-bit entry points, they are saved too except r8..r11.
*/
regs_user_copy->ip = user_regs->ip;
+ regs_user_copy->ax = user_regs->ax;
regs_user_copy->cx = user_regs->cx;
regs_user_copy->dx = user_regs->dx;
regs_user_copy->si = user_regs->si;