All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/3] x86/entry: Fix idtentry unwind hint
@ 2017-10-20 16:21 Josh Poimboeuf
  2017-10-20 16:21 ` [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages Josh Poimboeuf
                   ` (3 more replies)
  0 siblings, 4 replies; 12+ messages in thread
From: Josh Poimboeuf @ 2017-10-20 16:21 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Andrei Vagin, Ingo Molnar, torvalds, peterz, hpa,
	tglx, Andy Lutomirski

This fixes the following ORC warning in the 'int3' entry code:

  WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b

The ORC metadata had the wrong stack offset for the iret registers.

Their location on the stack is dependent on whether the exception has an
error code.

Reported-and-tested-by: Andrei Vagin <avagin@virtuozzo.com>
Fixes: 8c1f75587a18 ("x86/entry/64: Add unwind hint annotations")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/entry/entry_64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 49167258d587..f6cdb7a1455e 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -808,7 +808,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
 
 .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
 ENTRY(\sym)
-	UNWIND_HINT_IRET_REGS offset=8
+	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 
 	/* Sanity check */
 	.if \shift_ist != -1 && \paranoid == 0
-- 
2.13.6

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

* [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages
  2017-10-20 16:21 [PATCH 1/3] x86/entry: Fix idtentry unwind hint Josh Poimboeuf
@ 2017-10-20 16:21 ` Josh Poimboeuf
  2017-10-23 12:44   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
  2017-10-20 16:21 ` [PATCH 3/3] x86/asm: Don't use the confusing '.ifeq' directive Josh Poimboeuf
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-10-20 16:21 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Andrei Vagin, Ingo Molnar, torvalds, peterz, hpa,
	tglx, Andy Lutomirski

Improve the warning messages to show the relevant function name+offset.
This makes it much easier to diagnose problems with the ORC metadata.

Before:

  WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b

After:

  WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60

Reported-by: Andrei Vagin <avagin@virtuozzo.com>
Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/kernel/unwind_orc.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 570b70d3f604..b95007e7c1b3 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -86,8 +86,8 @@ static struct orc_entry *orc_find(unsigned long ip)
 		idx = (ip - LOOKUP_START_IP) / LOOKUP_BLOCK_SIZE;
 
 		if (unlikely((idx >= lookup_num_blocks-1))) {
-			orc_warn("WARNING: bad lookup idx: idx=%u num=%u ip=%lx\n",
-				 idx, lookup_num_blocks, ip);
+			orc_warn("WARNING: bad lookup idx: idx=%u num=%u ip=%pB\n",
+				 idx, lookup_num_blocks, (void *)ip);
 			return NULL;
 		}
 
@@ -96,8 +96,8 @@ static struct orc_entry *orc_find(unsigned long ip)
 
 		if (unlikely((__start_orc_unwind + start >= __stop_orc_unwind) ||
 			     (__start_orc_unwind + stop > __stop_orc_unwind))) {
-			orc_warn("WARNING: bad lookup value: idx=%u num=%u start=%u stop=%u ip=%lx\n",
-				 idx, lookup_num_blocks, start, stop, ip);
+			orc_warn("WARNING: bad lookup value: idx=%u num=%u start=%u stop=%u ip=%pB\n",
+				 idx, lookup_num_blocks, start, stop, (void *)ip);
 			return NULL;
 		}
 
@@ -373,7 +373,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_R10:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg R10 at ip %p\n",
+			orc_warn("missing regs for base reg R10 at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -382,7 +382,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_R13:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg R13 at ip %p\n",
+			orc_warn("missing regs for base reg R13 at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -391,7 +391,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_DI:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg DI at ip %p\n",
+			orc_warn("missing regs for base reg DI at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -400,7 +400,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_DX:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg DX at ip %p\n",
+			orc_warn("missing regs for base reg DX at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -408,7 +408,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown SP base reg %d for ip %p\n",
+		orc_warn("unknown SP base reg %d for ip %pB\n",
 			 orc->sp_reg, (void *)state->ip);
 		goto done;
 	}
@@ -436,7 +436,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_TYPE_REGS:
 		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, true)) {
-			orc_warn("can't dereference registers at %p for ip %p\n",
+			orc_warn("can't dereference registers at %p for ip %pB\n",
 				 (void *)sp, (void *)orig_ip);
 			goto done;
 		}
@@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_TYPE_REGS_IRET:
 		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
-			orc_warn("can't dereference iret registers at %p for ip %p\n",
+			orc_warn("can't dereference iret registers at %p for ip %pB\n",
 				 (void *)sp, (void *)orig_ip);
 			goto done;
 		}
@@ -465,7 +465,8 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown .orc_unwind entry type %d\n", orc->type);
+		orc_warn("unknown .orc_unwind entry type %d for ip %pB\n",
+			 orc->type, (void *)orig_ip);
 		break;
 	}
 
@@ -487,7 +488,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown BP base reg %d for ip %p\n",
+		orc_warn("unknown BP base reg %d for ip %pB\n",
 			 orc->bp_reg, (void *)orig_ip);
 		goto done;
 	}
@@ -496,7 +497,7 @@ bool unwind_next_frame(struct unwind_state *state)
 	if (state->stack_info.type == prev_type &&
 	    on_stack(&state->stack_info, (void *)state->sp, sizeof(long)) &&
 	    state->sp <= prev_sp) {
-		orc_warn("stack going in the wrong direction? ip=%p\n",
+		orc_warn("stack going in the wrong direction? ip=%pB\n",
 			 (void *)orig_ip);
 		goto done;
 	}
-- 
2.13.6

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

* [PATCH 3/3] x86/asm: Don't use the confusing '.ifeq' directive
  2017-10-20 16:21 [PATCH 1/3] x86/entry: Fix idtentry unwind hint Josh Poimboeuf
  2017-10-20 16:21 ` [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages Josh Poimboeuf
@ 2017-10-20 16:21 ` Josh Poimboeuf
  2017-10-23 12:44   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
  2017-10-23 12:43 ` [tip:x86/asm] x86/entry: Fix idtentry unwind hint tip-bot for Josh Poimboeuf
  2017-10-25 21:07 ` [PATCH 1/3] " Andrei Vagin
  3 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-10-20 16:21 UTC (permalink / raw)
  To: x86
  Cc: linux-kernel, Andrei Vagin, Ingo Molnar, torvalds, peterz, hpa,
	tglx, Andy Lutomirski

I find the '.ifeq <expression>' directive to be confusing.  Reading it
quickly seems to suggest its opposite meaning, or that it's missing an
argument.

Improve readability by replacing all of its x86 uses with
'.if <expression> == 0'.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
---
 arch/x86/entry/entry_64.S | 2 +-
 arch/x86/kernel/head_32.S | 2 +-
 arch/x86/kernel/head_64.S | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index f6cdb7a1455e..846e84a1d1f7 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -817,7 +817,7 @@ ENTRY(\sym)
 
 	ASM_CLAC
 
-	.ifeq \has_error_code
+	.if \has_error_code == 0
 	pushq	$-1				/* ORIG_RAX: no syscall to restart */
 	.endif
 
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 9ed3074d0d27..6e50f87765e5 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -401,7 +401,7 @@ ENTRY(early_idt_handler_array)
 	# 24(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushl $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushl $i		# 20(%esp) Vector number
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 99b1262c8a81..48875910607d 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -274,7 +274,7 @@ ENDPROC(start_cpu0)
 ENTRY(early_idt_handler_array)
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 		UNWIND_HINT_IRET_REGS
 		pushq $0	# Dummy error code, to make stack frame uniform
 	.else
-- 
2.13.6

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

* [tip:x86/asm] x86/entry: Fix idtentry unwind hint
  2017-10-20 16:21 [PATCH 1/3] x86/entry: Fix idtentry unwind hint Josh Poimboeuf
  2017-10-20 16:21 ` [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages Josh Poimboeuf
  2017-10-20 16:21 ` [PATCH 3/3] x86/asm: Don't use the confusing '.ifeq' directive Josh Poimboeuf
@ 2017-10-23 12:43 ` tip-bot for Josh Poimboeuf
  2017-10-25 21:07 ` [PATCH 1/3] " Andrei Vagin
  3 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-10-23 12:43 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: avagin, tglx, hpa, mingo, torvalds, peterz, luto, linux-kernel, jpoimboe

Commit-ID:  98990a33b77dda9babf91cb235654f6729e5702e
Gitweb:     https://git.kernel.org/tip/98990a33b77dda9babf91cb235654f6729e5702e
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 20 Oct 2017 11:21:33 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 23 Oct 2017 13:30:35 +0200

x86/entry: Fix idtentry unwind hint

This fixes the following ORC warning in the 'int3' entry code:

  WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b

The ORC metadata had the wrong stack offset for the iret registers.

Their location on the stack is dependent on whether the exception has an
error code.

Reported-and-tested-by: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: 8c1f75587a18 ("x86/entry/64: Add unwind hint annotations")
Link: http://lkml.kernel.org/r/931d57f0551ed7979d5e7e05370d445c8e5137f8.1508516398.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index 4916725..f6cdb7a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -808,7 +808,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
 
 .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
 ENTRY(\sym)
-	UNWIND_HINT_IRET_REGS offset=8
+	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
 
 	/* Sanity check */
 	.if \shift_ist != -1 && \paranoid == 0

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

* [tip:x86/asm] x86/unwind: Show function name+offset in ORC error messages
  2017-10-20 16:21 ` [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages Josh Poimboeuf
@ 2017-10-23 12:44   ` tip-bot for Josh Poimboeuf
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-10-23 12:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: peterz, avagin, mingo, jpoimboe, tglx, torvalds, hpa, linux-kernel, luto

Commit-ID:  58c3862b521ead4f69a24ef009a679cb3c519620
Gitweb:     https://git.kernel.org/tip/58c3862b521ead4f69a24ef009a679cb3c519620
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 20 Oct 2017 11:21:34 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 23 Oct 2017 13:30:36 +0200

x86/unwind: Show function name+offset in ORC error messages

Improve the warning messages to show the relevant function name+offset.
This makes it much easier to diagnose problems with the ORC metadata.

Before:

  WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b

After:

  WARNING: can't dereference iret registers at ffff880178f5ffe0 for ip int3+0x5b/0x60

Reported-by: Andrei Vagin <avagin@virtuozzo.com>
Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Fixes: ee9f8fce9964 ("x86/unwind: Add the ORC unwinder")
Link: http://lkml.kernel.org/r/6bada6b9eac86017e16bd79e1e77877935cb50bb.1508516398.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/kernel/unwind_orc.c | 29 +++++++++++++++--------------
 1 file changed, 15 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/unwind_orc.c b/arch/x86/kernel/unwind_orc.c
index 570b70d..b95007e 100644
--- a/arch/x86/kernel/unwind_orc.c
+++ b/arch/x86/kernel/unwind_orc.c
@@ -86,8 +86,8 @@ static struct orc_entry *orc_find(unsigned long ip)
 		idx = (ip - LOOKUP_START_IP) / LOOKUP_BLOCK_SIZE;
 
 		if (unlikely((idx >= lookup_num_blocks-1))) {
-			orc_warn("WARNING: bad lookup idx: idx=%u num=%u ip=%lx\n",
-				 idx, lookup_num_blocks, ip);
+			orc_warn("WARNING: bad lookup idx: idx=%u num=%u ip=%pB\n",
+				 idx, lookup_num_blocks, (void *)ip);
 			return NULL;
 		}
 
@@ -96,8 +96,8 @@ static struct orc_entry *orc_find(unsigned long ip)
 
 		if (unlikely((__start_orc_unwind + start >= __stop_orc_unwind) ||
 			     (__start_orc_unwind + stop > __stop_orc_unwind))) {
-			orc_warn("WARNING: bad lookup value: idx=%u num=%u start=%u stop=%u ip=%lx\n",
-				 idx, lookup_num_blocks, start, stop, ip);
+			orc_warn("WARNING: bad lookup value: idx=%u num=%u start=%u stop=%u ip=%pB\n",
+				 idx, lookup_num_blocks, start, stop, (void *)ip);
 			return NULL;
 		}
 
@@ -373,7 +373,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_R10:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg R10 at ip %p\n",
+			orc_warn("missing regs for base reg R10 at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -382,7 +382,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_R13:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg R13 at ip %p\n",
+			orc_warn("missing regs for base reg R13 at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -391,7 +391,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_DI:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg DI at ip %p\n",
+			orc_warn("missing regs for base reg DI at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -400,7 +400,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_REG_DX:
 		if (!state->regs || !state->full_regs) {
-			orc_warn("missing regs for base reg DX at ip %p\n",
+			orc_warn("missing regs for base reg DX at ip %pB\n",
 				 (void *)state->ip);
 			goto done;
 		}
@@ -408,7 +408,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown SP base reg %d for ip %p\n",
+		orc_warn("unknown SP base reg %d for ip %pB\n",
 			 orc->sp_reg, (void *)state->ip);
 		goto done;
 	}
@@ -436,7 +436,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_TYPE_REGS:
 		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, true)) {
-			orc_warn("can't dereference registers at %p for ip %p\n",
+			orc_warn("can't dereference registers at %p for ip %pB\n",
 				 (void *)sp, (void *)orig_ip);
 			goto done;
 		}
@@ -448,7 +448,7 @@ bool unwind_next_frame(struct unwind_state *state)
 
 	case ORC_TYPE_REGS_IRET:
 		if (!deref_stack_regs(state, sp, &state->ip, &state->sp, false)) {
-			orc_warn("can't dereference iret registers at %p for ip %p\n",
+			orc_warn("can't dereference iret registers at %p for ip %pB\n",
 				 (void *)sp, (void *)orig_ip);
 			goto done;
 		}
@@ -465,7 +465,8 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown .orc_unwind entry type %d\n", orc->type);
+		orc_warn("unknown .orc_unwind entry type %d for ip %pB\n",
+			 orc->type, (void *)orig_ip);
 		break;
 	}
 
@@ -487,7 +488,7 @@ bool unwind_next_frame(struct unwind_state *state)
 		break;
 
 	default:
-		orc_warn("unknown BP base reg %d for ip %p\n",
+		orc_warn("unknown BP base reg %d for ip %pB\n",
 			 orc->bp_reg, (void *)orig_ip);
 		goto done;
 	}
@@ -496,7 +497,7 @@ bool unwind_next_frame(struct unwind_state *state)
 	if (state->stack_info.type == prev_type &&
 	    on_stack(&state->stack_info, (void *)state->sp, sizeof(long)) &&
 	    state->sp <= prev_sp) {
-		orc_warn("stack going in the wrong direction? ip=%p\n",
+		orc_warn("stack going in the wrong direction? ip=%pB\n",
 			 (void *)orig_ip);
 		goto done;
 	}

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

* [tip:x86/asm] x86/asm: Don't use the confusing '.ifeq' directive
  2017-10-20 16:21 ` [PATCH 3/3] x86/asm: Don't use the confusing '.ifeq' directive Josh Poimboeuf
@ 2017-10-23 12:44   ` tip-bot for Josh Poimboeuf
  0 siblings, 0 replies; 12+ messages in thread
From: tip-bot for Josh Poimboeuf @ 2017-10-23 12:44 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: avagin, linux-kernel, jpoimboe, tglx, luto, mingo, torvalds, hpa, peterz

Commit-ID:  82c62fa0c49aa305104013cee4468772799bb391
Gitweb:     https://git.kernel.org/tip/82c62fa0c49aa305104013cee4468772799bb391
Author:     Josh Poimboeuf <jpoimboe@redhat.com>
AuthorDate: Fri, 20 Oct 2017 11:21:35 -0500
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 23 Oct 2017 13:31:34 +0200

x86/asm: Don't use the confusing '.ifeq' directive

I find the '.ifeq <expression>' directive to be confusing.  Reading it
quickly seems to suggest its opposite meaning, or that it's missing an
argument.

Improve readability by replacing all of its x86 uses with
'.if <expression> == 0'.

Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Andrei Vagin <avagin@virtuozzo.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/757da028e802c7e98d23fbab8d234b1063e161cf.1508516398.git.jpoimboe@redhat.com
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/entry/entry_64.S | 2 +-
 arch/x86/kernel/head_32.S | 2 +-
 arch/x86/kernel/head_64.S | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
index f6cdb7a..846e84a 100644
--- a/arch/x86/entry/entry_64.S
+++ b/arch/x86/entry/entry_64.S
@@ -817,7 +817,7 @@ ENTRY(\sym)
 
 	ASM_CLAC
 
-	.ifeq \has_error_code
+	.if \has_error_code == 0
 	pushq	$-1				/* ORIG_RAX: no syscall to restart */
 	.endif
 
diff --git a/arch/x86/kernel/head_32.S b/arch/x86/kernel/head_32.S
index 9ed3074..6e50f87 100644
--- a/arch/x86/kernel/head_32.S
+++ b/arch/x86/kernel/head_32.S
@@ -401,7 +401,7 @@ ENTRY(early_idt_handler_array)
 	# 24(%rsp) error code
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 	pushl $0		# Dummy error code, to make stack frame uniform
 	.endif
 	pushl $i		# 20(%esp) Vector number
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 42e32c2..311db1a 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -273,7 +273,7 @@ ENDPROC(start_cpu0)
 ENTRY(early_idt_handler_array)
 	i = 0
 	.rept NUM_EXCEPTION_VECTORS
-	.ifeq (EXCEPTION_ERRCODE_MASK >> i) & 1
+	.if ((EXCEPTION_ERRCODE_MASK >> i) & 1) == 0
 		UNWIND_HINT_IRET_REGS
 		pushq $0	# Dummy error code, to make stack frame uniform
 	.else

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
  2017-10-20 16:21 [PATCH 1/3] x86/entry: Fix idtentry unwind hint Josh Poimboeuf
                   ` (2 preceding siblings ...)
  2017-10-23 12:43 ` [tip:x86/asm] x86/entry: Fix idtentry unwind hint tip-bot for Josh Poimboeuf
@ 2017-10-25 21:07 ` Andrei Vagin
  2017-10-26 15:24   ` Josh Poimboeuf
  3 siblings, 1 reply; 12+ messages in thread
From: Andrei Vagin @ 2017-10-25 21:07 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, linux-kernel, Ingo Molnar, torvalds, peterz, hpa, tglx,
	Andy Lutomirski

Hi Josh,

Here is one more warning:
[    5.852094] WARNING: can't dereference iret registers at ffffb6ce01b7ffe0 for ip entry_SYSCALL_64_fastpath+0xa/0xc2

[avagin@laptop linux]$ git describe tip/auto-latest
v4.14-rc6-471-g376214a8543d

On Fri, Oct 20, 2017 at 11:21:33AM -0500, Josh Poimboeuf wrote:
> This fixes the following ORC warning in the 'int3' entry code:
> 
>   WARNING: can't dereference iret registers at ffff8801c5f17fe0 for ip ffffffff95f0d94b
> 
> The ORC metadata had the wrong stack offset for the iret registers.
> 
> Their location on the stack is dependent on whether the exception has an
> error code.
> 
> Reported-and-tested-by: Andrei Vagin <avagin@virtuozzo.com>
> Fixes: 8c1f75587a18 ("x86/entry/64: Add unwind hint annotations")
> Signed-off-by: Josh Poimboeuf <jpoimboe@redhat.com>
> ---
>  arch/x86/entry/entry_64.S | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 49167258d587..f6cdb7a1455e 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -808,7 +808,7 @@ apicinterrupt IRQ_WORK_VECTOR			irq_work_interrupt		smp_irq_work_interrupt
>  
>  .macro idtentry sym do_sym has_error_code:req paranoid=0 shift_ist=-1
>  ENTRY(\sym)
> -	UNWIND_HINT_IRET_REGS offset=8
> +	UNWIND_HINT_IRET_REGS offset=\has_error_code*8
>  
>  	/* Sanity check */
>  	.if \shift_ist != -1 && \paranoid == 0
> -- 
> 2.13.6
> 

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
  2017-10-25 21:07 ` [PATCH 1/3] " Andrei Vagin
@ 2017-10-26 15:24   ` Josh Poimboeuf
  2017-10-26 16:42     ` Andrei Vagin
  2017-11-13 22:06     ` Andrei Vagin
  0 siblings, 2 replies; 12+ messages in thread
From: Josh Poimboeuf @ 2017-10-26 15:24 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: x86, linux-kernel, Ingo Molnar, torvalds, peterz, hpa, tglx,
	Andy Lutomirski

On Wed, Oct 25, 2017 at 02:07:38PM -0700, Andrei Vagin wrote:
> Hi Josh,
> 
> Here is one more warning:
> [    5.852094] WARNING: can't dereference iret registers at ffffb6ce01b7ffe0 for ip entry_SYSCALL_64_fastpath+0xa/0xc2

Thanks, I hadn't seen this one yet.

I suspect this is in the middle of the ENABLE_INTERRUPTS() paravirt
code, which would mean this is another issue that will be fixed by my
"Make pv ops code generation more closely match reality" patches.

If you can share either the entry_64.o file or the .config, and what
virt platform it's running on (kvm, xen, native), I should be able to
confirm the issue.

I'm in Prague this week but I should have a v2 of those patches in a
week or two (will cc you).

-- 
Josh

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
  2017-10-26 15:24   ` Josh Poimboeuf
@ 2017-10-26 16:42     ` Andrei Vagin
  2017-11-13 22:06     ` Andrei Vagin
  1 sibling, 0 replies; 12+ messages in thread
From: Andrei Vagin @ 2017-10-26 16:42 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: x86, linux-kernel, Ingo Molnar, torvalds, peterz, hpa, tglx,
	Andy Lutomirski

On Thu, Oct 26, 2017 at 10:24:29AM -0500, Josh Poimboeuf wrote:
> On Wed, Oct 25, 2017 at 02:07:38PM -0700, Andrei Vagin wrote:
> > Hi Josh,
> > 
> > Here is one more warning:
> > [    5.852094] WARNING: can't dereference iret registers at ffffb6ce01b7ffe0 for ip entry_SYSCALL_64_fastpath+0xa/0xc2
> 
> Thanks, I hadn't seen this one yet.
> 
> I suspect this is in the middle of the ENABLE_INTERRUPTS() paravirt
> code, which would mean this is another issue that will be fixed by my
> "Make pv ops code generation more closely match reality" patches.
> 
> If you can share either the entry_64.o file or the .config, and what
> virt platform it's running on (kvm, xen, native), I should be able to
> confirm the issue.

The config is attached to this e-mail. It is a travis-ci vm. I think
they are hosted in Google Cloud:

[    0.000000] DMI: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
[    0.000000] Hypervisor detected: KVM

https://travis-ci.org/avagin/linux/jobs/292773933

> 
> I'm in Prague this week but I should have a v2 of those patches in a
> week or two (will cc you).

Good!

> 
> -- 
> Josh

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
  2017-10-26 15:24   ` Josh Poimboeuf
  2017-10-26 16:42     ` Andrei Vagin
@ 2017-11-13 22:06     ` Andrei Vagin
  2017-11-13 22:59       ` Josh Poimboeuf
  1 sibling, 1 reply; 12+ messages in thread
From: Andrei Vagin @ 2017-11-13 22:06 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Andrei Vagin, X86 ML, LKML, Ingo Molnar, Linus Torvalds,
	Peter Zijlstra, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski

Hi Josh,

On Thu, Oct 26, 2017 at 8:24 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> On Wed, Oct 25, 2017 at 02:07:38PM -0700, Andrei Vagin wrote:
>> Hi Josh,
>>
>> Here is one more warning:
>> [    5.852094] WARNING: can't dereference iret registers at ffffb6ce01b7ffe0 for ip entry_SYSCALL_64_fastpath+0xa/0xc2
>
> Thanks, I hadn't seen this one yet.
>
> I suspect this is in the middle of the ENABLE_INTERRUPTS() paravirt
> code, which would mean this is another issue that will be fixed by my
> "Make pv ops code generation more closely match reality" patches.
>
> If you can share either the entry_64.o file or the .config, and what
> virt platform it's running on (kvm, xen, native), I should be able to
> confirm the issue.
>
> I'm in Prague this week but I should have a v2 of those patches in a
> week or two (will cc you).

Do you have any news? We still see this warning in the kernel log. Thanks!

>
> --
> Josh

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
  2017-11-13 22:06     ` Andrei Vagin
@ 2017-11-13 22:59       ` Josh Poimboeuf
       [not found]         ` <AM4PR08MB07393706B7D76768D4817AC2CF0C0@AM4PR08MB0739.eurprd08.prod.outlook.com>
  0 siblings, 1 reply; 12+ messages in thread
From: Josh Poimboeuf @ 2017-11-13 22:59 UTC (permalink / raw)
  To: Andrei Vagin
  Cc: Andrei Vagin, X86 ML, LKML, Ingo Molnar, Linus Torvalds,
	Peter Zijlstra, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski

On Mon, Nov 13, 2017 at 02:06:35PM -0800, Andrei Vagin wrote:
> Hi Josh,
> 
> On Thu, Oct 26, 2017 at 8:24 AM, Josh Poimboeuf <jpoimboe@redhat.com> wrote:
> > On Wed, Oct 25, 2017 at 02:07:38PM -0700, Andrei Vagin wrote:
> >> Hi Josh,
> >>
> >> Here is one more warning:
> >> [    5.852094] WARNING: can't dereference iret registers at ffffb6ce01b7ffe0 for ip entry_SYSCALL_64_fastpath+0xa/0xc2
> >
> > Thanks, I hadn't seen this one yet.
> >
> > I suspect this is in the middle of the ENABLE_INTERRUPTS() paravirt
> > code, which would mean this is another issue that will be fixed by my
> > "Make pv ops code generation more closely match reality" patches.
> >
> > If you can share either the entry_64.o file or the .config, and what
> > virt platform it's running on (kvm, xen, native), I should be able to
> > confirm the issue.
> >
> > I'm in Prague this week but I should have a v2 of those patches in a
> > week or two (will cc you).
> 
> Do you have any news? We still see this warning in the kernel log. Thanks!

Sorry, a bunch of other work got in the way.  I haven't forgotten about
it.  It's now (almost) at the top of my TODO list.

-- 
Josh

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

* Re: [PATCH 1/3] x86/entry: Fix idtentry unwind hint
       [not found]         ` <AM4PR08MB07393706B7D76768D4817AC2CF0C0@AM4PR08MB0739.eurprd08.prod.outlook.com>
@ 2017-12-20  6:34           ` Josh Poimboeuf
  0 siblings, 0 replies; 12+ messages in thread
From: Josh Poimboeuf @ 2017-12-20  6:34 UTC (permalink / raw)
  To: Andrey Vagin
  Cc: Andrei Vagin, X86 ML, LKML, Ingo Molnar, Linus Torvalds,
	Peter Zijlstra, H. Peter Anvin, Thomas Gleixner, Andy Lutomirski

On Wed, Dec 20, 2017 at 05:41:44AM +0000, Andrey Vagin wrote:
> Hi Josh,
> 
> 
> Now I see these two warnings on Linus' tree:
> 
> [    1.902454] WARNING: stack recursion on stack type 1
> [    1.902466] WARNING: can't dereference iret registers at 00000000cd089a12 for ip entry_SYSCALL_64_fastpath+0x5/0x86

This still looks like the same issue where ORC is getting confused by
paravirt patching.  Unfortunately the patches which fix this got
preempted by other work again.  I haven't forgotten about it.

2017 is out the window, but hopefully in January I'll get a chance to
revive the patches.

-- 
Josh

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

end of thread, other threads:[~2017-12-20  6:34 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-20 16:21 [PATCH 1/3] x86/entry: Fix idtentry unwind hint Josh Poimboeuf
2017-10-20 16:21 ` [PATCH 2/3] x86/unwind: Show function name+offset in ORC error messages Josh Poimboeuf
2017-10-23 12:44   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
2017-10-20 16:21 ` [PATCH 3/3] x86/asm: Don't use the confusing '.ifeq' directive Josh Poimboeuf
2017-10-23 12:44   ` [tip:x86/asm] " tip-bot for Josh Poimboeuf
2017-10-23 12:43 ` [tip:x86/asm] x86/entry: Fix idtentry unwind hint tip-bot for Josh Poimboeuf
2017-10-25 21:07 ` [PATCH 1/3] " Andrei Vagin
2017-10-26 15:24   ` Josh Poimboeuf
2017-10-26 16:42     ` Andrei Vagin
2017-11-13 22:06     ` Andrei Vagin
2017-11-13 22:59       ` Josh Poimboeuf
     [not found]         ` <AM4PR08MB07393706B7D76768D4817AC2CF0C0@AM4PR08MB0739.eurprd08.prod.outlook.com>
2017-12-20  6:34           ` Josh Poimboeuf

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.