linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Vegard Nossum <vegard.nossum@gmail.com>
To: linux-kernel@vger.kernel.org, linux-mm@kvack.org
Cc: Ingo Molnar <mingo@elte.hu>, Pekka Enberg <penberg@cs.helsinki.fi>
Subject: [PATCH] kmemcheck: rip out REP instruction emulation
Date: Sat, 21 Feb 2009 14:36:02 +0100	[thread overview]
Message-ID: <1235223364-2097-3-git-send-email-vegard.nossum@gmail.com> (raw)
In-Reply-To: <1235223364-2097-1-git-send-email-vegard.nossum@gmail.com>

As it turns out, disabling the "fast strings" of the P4 fixed the
REP single-stepping issue, so this code is not needed anymore.

Celebrate, for we just got rid of a LOT of complexity and pain.

Signed-off-by: Vegard Nossum <vegard.nossum@gmail.com>
---
 arch/x86/mm/kmemcheck/kmemcheck.c |  119 +------------------------------------
 arch/x86/mm/kmemcheck/opcode.c    |   13 +----
 arch/x86/mm/kmemcheck/opcode.h    |    3 +-
 3 files changed, 3 insertions(+), 132 deletions(-)

diff --git a/arch/x86/mm/kmemcheck/kmemcheck.c b/arch/x86/mm/kmemcheck/kmemcheck.c
index 056b4f1..b944f1d 100644
--- a/arch/x86/mm/kmemcheck/kmemcheck.c
+++ b/arch/x86/mm/kmemcheck/kmemcheck.c
@@ -113,18 +113,6 @@ struct kmemcheck_context {
 	unsigned long n_addrs;
 	unsigned long flags;
 
-	/*
-	 * The address of the REP prefix if we are currently emulating a
-	 * REP instruction; otherwise 0.
-	 */
-	const uint8_t *rep;
-
-	/* The address of the REX prefix. */
-	const uint8_t *rex;
-
-	/* Address of the primary instruction opcode. */
-	const uint8_t *insn;
-
 	/* Data size of the instruction that caused a fault. */
 	unsigned int size;
 };
@@ -241,12 +229,6 @@ void kmemcheck_hide(struct pt_regs *regs)
 		return;
 	}
 
-	if (data->rep) {
-		/* Save state and take it up later. */
-		regs->ip = (unsigned long) data->rep;
-		data->rep = NULL;
-	}
-
 	if (kmemcheck_enabled)
 		n = kmemcheck_hide_all();
 	else
@@ -513,8 +495,6 @@ enum kmemcheck_method {
 static void kmemcheck_access(struct pt_regs *regs,
 	unsigned long fallback_address, enum kmemcheck_method fallback_method)
 {
-	const uint8_t *rep_prefix;
-	const uint8_t *rex_prefix;
 	const uint8_t *insn;
 	const uint8_t *insn_primary;
 	unsigned int size;
@@ -533,55 +513,7 @@ static void kmemcheck_access(struct pt_regs *regs,
 	insn = (const uint8_t *) regs->ip;
 	insn_primary = kmemcheck_opcode_get_primary(insn);
 
-	kmemcheck_opcode_decode(insn, &rep_prefix, &rex_prefix, &size);
-
-	if (rep_prefix && *rep_prefix == 0xf3) {
-		/*
-		 * Due to an incredibly silly Intel bug, REP MOVS and
-		 * REP STOS instructions may generate just one single-
-		 * stepping trap on Pentium 4 CPUs. Other CPUs, including
-		 * AMDs, seem to generate traps after each repetition.
-		 *
-		 * What we do is really a very ugly hack; we increment the
-		 * instruction pointer before returning so that the next
-		 * time around we'll hit an ordinary MOVS or STOS
-		 * instruction. Now, in the debug exception, we know that
-		 * the instruction is really a REP MOVS/STOS, so instead
-		 * of clearing the single-stepping flag, we just continue
-		 * single-stepping the instruction until we're done.
-		 *
-		 * We currently don't handle REP MOVS/STOS instructions
-		 * which have other (additional) instruction prefixes in
-		 * front of REP, so we BUG on those.
-		 */
-		switch (insn_primary[0]) {
-			/* REP MOVS */
-		case 0xa4:
-		case 0xa5:
-			BUG_ON(regs->ip != (unsigned long) rep_prefix);
-
-			kmemcheck_copy(regs, regs->si, regs->di, size);
-			data->rep = rep_prefix;
-			data->rex = rex_prefix;
-			data->insn = insn_primary;
-			data->size = size;
-			regs->ip = (unsigned long) data->rep + 1;
-			goto out;
-
-			/* REP STOS */
-		case 0xaa:
-		case 0xab:
-			BUG_ON(regs->ip != (unsigned long) rep_prefix);
-
-			kmemcheck_write(regs, regs->di, size);
-			data->rep = rep_prefix;
-			data->rex = rex_prefix;
-			data->insn = insn_primary;
-			data->size = size;
-			regs->ip = (unsigned long) data->rep + 1;
-			goto out;
-		}
-	}
+	kmemcheck_opcode_decode(insn, &size);
 
 	switch (insn_primary[0]) {
 #ifdef CONFIG_KMEMCHECK_BITOPS_OK
@@ -693,59 +625,10 @@ bool kmemcheck_fault(struct pt_regs *regs, unsigned long address,
 bool kmemcheck_trap(struct pt_regs *regs)
 {
 	struct kmemcheck_context *data = &__get_cpu_var(kmemcheck_context);
-	unsigned long cx;
-#ifdef CONFIG_X86_64
-	uint32_t ecx;
-#endif
 
 	if (!kmemcheck_active(regs))
 		return false;
 
-	if (!data->rep) {
-		kmemcheck_hide(regs);
-		return true;
-	}
-
-	/*
-	 * We're emulating a REP MOVS/STOS instruction. Are we done yet?
-	 * Of course, 64-bit needs to handle CX/ECX/RCX differently...
-	 */
-#ifdef CONFIG_X86_64
-	if (data->rex && data->rex[0] & 0x08) {
-		cx = regs->cx - 1;
-		regs->cx = cx;
-	} else {
-		/* Without REX, 64-bit wants to use %ecx by default. */
-		ecx = regs->cx - 1;
-		cx = ecx;
-		regs->cx = (regs->cx & ~((1UL << 32) - 1)) | ecx;
-	}
-#else
-	cx = regs->cx - 1;
-	regs->cx = cx;
-#endif
-	if (cx) {
-		unsigned long rep = (unsigned long) data->rep;
-		kmemcheck_hide(regs);
-		/* Without the REP prefix, we have to do this ourselves... */
-		data->rep = (void *) rep;
-		regs->ip = rep + 1;
-
-		switch (data->insn[0]) {
-		case 0xa4:
-		case 0xa5:
-			kmemcheck_copy(regs, regs->si, regs->di, data->size);
-			break;
-		case 0xaa:
-		case 0xab:
-			kmemcheck_write(regs, regs->di, data->size);
-			break;
-		}
-
-		kmemcheck_show(regs);
-		return true;
-	}
-
 	/* We're done. */
 	kmemcheck_hide(regs);
 	return true;
diff --git a/arch/x86/mm/kmemcheck/opcode.c b/arch/x86/mm/kmemcheck/opcode.c
index 88a9662..3dff500 100644
--- a/arch/x86/mm/kmemcheck/opcode.c
+++ b/arch/x86/mm/kmemcheck/opcode.c
@@ -27,30 +27,20 @@ static bool opcode_is_rex_prefix(uint8_t b)
  * that we care about. Moreover, the ones who invented this instruction set
  * should be shot.
  */
-void kmemcheck_opcode_decode(const uint8_t *op,
-	const uint8_t **rep_prefix, const uint8_t **rex_prefix,
-	unsigned int *size)
+void kmemcheck_opcode_decode(const uint8_t *op, unsigned int *size)
 {
 	/* Default operand size */
 	int operand_size_override = 4;
 
-	*rep_prefix = NULL;
-
 	/* prefixes */
 	for (; opcode_is_prefix(*op); ++op) {
-		if (*op == 0xf2 || *op == 0xf3)
-			*rep_prefix = op;
 		if (*op == 0x66)
 			operand_size_override = 2;
 	}
 
-	*rex_prefix = NULL;
-
 #ifdef CONFIG_X86_64
 	/* REX prefix */
 	if (opcode_is_rex_prefix(*op)) {
-		*rex_prefix = op;
-
 		if (*op & 0x08) {
 			*size = 8;
 			return;
@@ -87,4 +77,3 @@ const uint8_t *kmemcheck_opcode_get_primary(const uint8_t *op)
 		++op;
 	return op;
 }
-
diff --git a/arch/x86/mm/kmemcheck/opcode.h b/arch/x86/mm/kmemcheck/opcode.h
index f744d8e..6956aad 100644
--- a/arch/x86/mm/kmemcheck/opcode.h
+++ b/arch/x86/mm/kmemcheck/opcode.h
@@ -3,8 +3,7 @@
 
 #include <linux/types.h>
 
-void kmemcheck_opcode_decode(const uint8_t *op,
-	const uint8_t **rep_pfx, const uint8_t **rex_pfx, unsigned int *size);
+void kmemcheck_opcode_decode(const uint8_t *op, unsigned int *size);
 const uint8_t *kmemcheck_opcode_get_primary(const uint8_t *op);
 
 #endif
-- 
1.6.0.6

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

  parent reply	other threads:[~2009-02-21 13:36 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-21 13:36 Vegard Nossum
2009-02-21 13:36 ` [PATCH] kmemcheck: disable fast string operations on P4 CPUs Vegard Nossum
2009-02-21 16:21   ` Pekka Enberg
2009-02-22  3:13   ` H. Peter Anvin
2009-02-22 10:51     ` Vegard Nossum
2009-02-21 13:36 ` Vegard Nossum [this message]
2009-02-21 16:21   ` [PATCH] kmemcheck: rip out REP instruction emulation Pekka Enberg
2009-02-21 13:36 ` [PATCH] kmemcheck: add hooks for page- and sg-dma-mappings Vegard Nossum
2009-02-21 16:22   ` Pekka Enberg
2009-02-21 17:13     ` Vegard Nossum
2009-02-21 17:15       ` Pekka Enberg
2009-02-21 13:36 ` [PATCH] kmemcheck: add hooks for the page allocator Vegard Nossum
2009-02-21 15:23   ` Ingo Molnar
2009-02-21 16:34   ` Pekka Enberg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1235223364-2097-3-git-send-email-vegard.nossum@gmail.com \
    --to=vegard.nossum@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mingo@elte.hu \
    --cc=penberg@cs.helsinki.fi \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox