All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jordan Niethe <jniethe5@gmail.com>
To: linuxppc-dev@lists.ozlabs.org
Cc: alistair@popple.id.au, bala24@linux.ibm.com,
	Jordan Niethe <jniethe5@gmail.com>,
	dja@axtens.net
Subject: [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions
Date: Wed, 26 Feb 2020 15:07:11 +1100	[thread overview]
Message-ID: <20200226040716.32395-10-jniethe5@gmail.com> (raw)
In-Reply-To: <20200226040716.32395-1-jniethe5@gmail.com>

A prefixed instruction is composed of a word prefix and a word suffix.
It does not make sense to be able to have a breakpoint on the suffix of
a prefixed instruction, so make this impossible.

When leaving xmon_core() we check to see if we are currently at a
breakpoint. If this is the case, the breakpoint needs to be proceeded
from. Initially emulate_step() is tried, but if this fails then we need
to execute the saved instruction out of line. The NIP is set to the
address of bpt::instr[] for the current breakpoint.  bpt::instr[]
contains the instruction replaced by the breakpoint, followed by a trap
instruction.  After bpt::instr[0] is executed and we hit the trap we
enter back into xmon_bpt(). We know that if we got here and the offset
indicates we are at bpt::instr[1] then we have just executed out of line
so we can put the NIP back to the instruction after the breakpoint
location and continue on.

Adding prefixed instructions complicates this as the bpt::instr[1] needs
to be used to hold the suffix. To deal with this make bpt::instr[] big
enough for three word instructions.  bpt::instr[2] contains the trap,
and in the case of word instructions pad bpt::instr[1] with a noop.

No support for disassembling prefixed instructions.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v2: Rename sufx to suffix
v3: - Just directly use PPC_INST_NOP
    - Typo: plac -> place
    - Rename read_inst() to mread_inst(). Do not have it call mread().
---
 arch/powerpc/xmon/xmon.c | 90 ++++++++++++++++++++++++++++++++++------
 1 file changed, 78 insertions(+), 12 deletions(-)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index a673cf55641c..a73a35aa4a75 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -97,7 +97,8 @@ static long *xmon_fault_jmp[NR_CPUS];
 /* Breakpoint stuff */
 struct bpt {
 	unsigned long	address;
-	unsigned int	instr[2];
+	/* Prefixed instructions can not cross 64-byte boundaries */
+	unsigned int	instr[3] __aligned(64);
 	atomic_t	ref_count;
 	int		enabled;
 	unsigned long	pad;
@@ -120,6 +121,7 @@ static unsigned bpinstr = 0x7fe00008;	/* trap */
 static int cmds(struct pt_regs *);
 static int mread(unsigned long, void *, int);
 static int mwrite(unsigned long, void *, int);
+static int mread_instr(unsigned long, unsigned int *, unsigned int *);
 static int handle_fault(struct pt_regs *);
 static void byterev(unsigned char *, int);
 static void memex(void);
@@ -701,7 +703,7 @@ static int xmon_core(struct pt_regs *regs, int fromipi)
 		bp = at_breakpoint(regs->nip);
 		if (bp != NULL) {
 			int stepped = emulate_step(regs, bp->instr[0],
-						   PPC_NO_SUFFIX);
+						   bp->instr[1]);
 			if (stepped == 0) {
 				regs->nip = (unsigned long) &bp->instr[0];
 				atomic_inc(&bp->ref_count);
@@ -756,8 +758,8 @@ static int xmon_bpt(struct pt_regs *regs)
 
 	/* Are we at the trap at bp->instr[1] for some bp? */
 	bp = in_breakpoint_table(regs->nip, &offset);
-	if (bp != NULL && offset == 4) {
-		regs->nip = bp->address + 4;
+	if (bp != NULL && (offset == 4 || offset == 8)) {
+		regs->nip = bp->address + offset;
 		atomic_dec(&bp->ref_count);
 		return 1;
 	}
@@ -858,8 +860,9 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
 	if (off >= sizeof(bpts))
 		return NULL;
 	off %= sizeof(struct bpt);
-	if (off != offsetof(struct bpt, instr[0])
-	    && off != offsetof(struct bpt, instr[1]))
+	if (off != offsetof(struct bpt, instr[0]) &&
+	    off != offsetof(struct bpt, instr[1]) &&
+	    off != offsetof(struct bpt, instr[2]))
 		return NULL;
 	*offp = off - offsetof(struct bpt, instr[0]);
 	return (struct bpt *) (nip - off);
@@ -876,8 +879,16 @@ static struct bpt *new_breakpoint(unsigned long a)
 
 	for (bp = bpts; bp < &bpts[NBPTS]; ++bp) {
 		if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
+			/*
+			 * Prefixed instructions are two words, but regular
+			 * instructions are only one. Use a nop to pad out the
+			 * regular instructions so that we can place the trap
+			 * at the same place. For prefixed instructions the nop
+			 * will get overwritten during insert_bpts().
+			 */
 			bp->address = a;
-			patch_instruction(&bp->instr[1], bpinstr);
+			patch_instruction(&bp->instr[1], PPC_INST_NOP);
+			patch_instruction(&bp->instr[2], bpinstr);
 			return bp;
 		}
 	}
@@ -889,13 +900,15 @@ static struct bpt *new_breakpoint(unsigned long a)
 static void insert_bpts(void)
 {
 	int i;
-	struct bpt *bp;
+	unsigned int prefix;
+	struct bpt *bp, *bp2;
 
 	bp = bpts;
 	for (i = 0; i < NBPTS; ++i, ++bp) {
 		if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0)
 			continue;
-		if (mread(bp->address, &bp->instr[0], 4) != 4) {
+		if (!mread_instr(bp->address, &bp->instr[0],
+				 &bp->instr[1])) {
 			printf("Couldn't read instruction at %lx, "
 			       "disabling breakpoint there\n", bp->address);
 			bp->enabled = 0;
@@ -907,7 +920,34 @@ static void insert_bpts(void)
 			bp->enabled = 0;
 			continue;
 		}
+		/*
+		 * Check the address is not a suffix by looking for a prefix in
+		 * front of it.
+		 */
+		if ((mread(bp->address - 4, &prefix, 4) == 4) &&
+		    IS_PREFIX(prefix)) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
+		/*
+		 * We might still be a suffix - if the prefix has already been
+		 * replaced by a breakpoint we won't catch it with the above
+		 * test.
+		 */
+		bp2 = at_breakpoint(bp->address - 4);
+		if (bp2 && IS_PREFIX(bp2->instr[0])) {
+			printf("Breakpoint at %lx is on the second word of a "
+			       "prefixed instruction, disabling it\n",
+			       bp->address);
+			bp->enabled = 0;
+			continue;
+		}
 		patch_instruction(&bp->instr[0], bp->instr[0]);
+		if (IS_PREFIX(bp->instr[0]))
+			patch_instruction(&bp->instr[1], bp->instr[1]);
 		if (bp->enabled & BP_CIABR)
 			continue;
 		if (patch_instruction((unsigned int *)bp->address,
@@ -1155,14 +1195,14 @@ static int do_step(struct pt_regs *regs)
  */
 static int do_step(struct pt_regs *regs)
 {
-	unsigned int instr;
+	unsigned int instr, suffix;
 	int stepped;
 
 	force_enable_xmon();
 	/* check we are in 64-bit kernel mode, translation enabled */
 	if ((regs->msr & (MSR_64BIT|MSR_PR|MSR_IR)) == (MSR_64BIT|MSR_IR)) {
-		if (mread(regs->nip, &instr, 4) == 4) {
-			stepped = emulate_step(regs, instr, PPC_NO_SUFFIX);
+		if (mread_instr(regs->nip, &instr, &suffix)) {
+			stepped = emulate_step(regs, instr, suffix);
 			if (stepped < 0) {
 				printf("Couldn't single-step %s instruction\n",
 				       (IS_RFID(instr)? "rfid": "mtmsrd"));
@@ -2121,6 +2161,32 @@ mwrite(unsigned long adrs, void *buf, int size)
 	return n;
 }
 
+static int
+mread_instr(unsigned long adrs, unsigned int *instr,
+	    unsigned int *suffix)
+{
+	int n;
+
+	n = 0;
+	if (setjmp(bus_error_jmp) == 0) {
+		catch_memory_errors = 1;
+		sync();
+		*instr = *(unsigned int *)adrs;
+		sync();
+		if (!IS_PREFIX(*instr)) {
+			n = 4;
+		} else {
+			*suffix = *(unsigned int *)(adrs + 4);
+			n = 8;
+		}
+		sync();
+		/* wait a little while to see if we get a machine check */
+		__delay(200);
+	}
+	catch_memory_errors = 0;
+	return n;
+}
+
 static int fault_type;
 static int fault_except;
 static char *fault_chars[] = { "--", "**", "##" };
-- 
2.17.1


  parent reply	other threads:[~2020-02-26  4:28 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26  4:07 [PATCH v3 00/14] Initial Prefixed Instruction support Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 01/14] powerpc: Enable Prefixed Instructions Jordan Niethe
2020-02-26  6:46   ` Nicholas Piggin
2020-02-28  2:52     ` Jordan Niethe
2020-02-28  4:48       ` Nicholas Piggin
2020-03-03 23:20         ` Alistair Popple
2020-02-26  4:07 ` [PATCH v3 02/14] powerpc: Define new SRR1 bits for a future ISA version Jordan Niethe
2020-03-03 23:31   ` Alistair Popple
2020-02-26  4:07 ` [PATCH v3 03/14] powerpc sstep: Prepare to support prefixed instructions Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 04/14] powerpc sstep: Add support for prefixed load/stores Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 05/14] powerpc sstep: Add support for prefixed fixed-point arithmetic Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 06/14] powerpc: Support prefixed instructions in alignment handler Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 07/14] powerpc/traps: Check for prefixed instructions in facility_unavailable_exception() Jordan Niethe
2020-02-26  6:49   ` Nicholas Piggin
2020-02-26 23:52     ` Jordan Niethe
2020-02-28  1:57       ` Nicholas Piggin
2020-02-26  4:07 ` [PATCH v3 08/14] powerpc/xmon: Remove store_inst() for patch_instruction() Jordan Niethe
2020-02-26  7:00   ` Nicholas Piggin
2020-02-26 23:54     ` Jordan Niethe
2020-02-26  4:07 ` Jordan Niethe [this message]
2020-02-26  7:06   ` [PATCH v3 09/14] powerpc/xmon: Add initial support for prefixed instructions Nicholas Piggin
2020-02-27  0:11     ` Jordan Niethe
2020-02-27  7:14       ` Christophe Leroy
2020-02-28  0:37         ` Jordan Niethe
2020-02-28  1:23           ` Nicholas Piggin
2020-02-26  4:07 ` [PATCH v3 10/14] powerpc/xmon: Dump " Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 11/14] powerpc/kprobes: Support kprobes on " Jordan Niethe
2020-02-26  7:14   ` Nicholas Piggin
2020-02-27  0:58     ` Jordan Niethe
2020-02-28  1:47       ` Nicholas Piggin
2020-02-28  1:56         ` Nicholas Piggin
2020-02-28  3:23         ` Jordan Niethe
2020-02-28  4:53           ` Nicholas Piggin
2020-02-26  4:07 ` [PATCH v3 12/14] powerpc/uprobes: Add support for " Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 13/14] powerpc/hw_breakpoints: Initial " Jordan Niethe
2020-02-26  4:07 ` [PATCH v3 14/14] powerpc: Add prefix support to mce_find_instr_ea_and_pfn() Jordan Niethe

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=20200226040716.32395-10-jniethe5@gmail.com \
    --to=jniethe5@gmail.com \
    --cc=alistair@popple.id.au \
    --cc=bala24@linux.ibm.com \
    --cc=dja@axtens.net \
    --cc=linuxppc-dev@lists.ozlabs.org \
    /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 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.