diff mbox series

[3/4] powerpc: Handle prefixed instructions in show_user_instructions()

Message ID 20200602052728.18227-3-jniethe5@gmail.com (mailing list archive)
State Changes Requested
Headers show
Series [1/4] powerpc: Add a ppc_inst_as_str() helper | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/merge (00ec79b0b767994422c43792d73ff1327714a73f)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/next (30df74d67d48949da87e3a5b57c381763e8fd526)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linus/master (f359287765c04711ff54fbd11645271d8e5ff763)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch powerpc/fixes (2f26ed1764b42a8c40d9c48441c73a70d805decf)
snowpatch_ozlabs/apply_patch warning Failed to apply on branch linux-next (e7b08814b16b80a0bf76eeca16317f8c2ed23b8c)
snowpatch_ozlabs/apply_patch fail Failed to apply to any branch

Commit Message

Jordan Niethe June 2, 2020, 5:27 a.m. UTC
Currently prefixed instructions are treated as two word instructions by
show_user_instructions(), treat them as a single instruction. '<' and
'>' are placed around the instruction at the NIP, and for prefixed
instructions this is placed around the prefix only. Make the '<' and '>'
wrap the prefix and suffix.

Currently showing a prefixed instruction looks like:
fbe1fff8 39200000 06000000 a3e30000 <04000000> f7e40000 ebe1fff8 4e800020

Make it look like:
0xfbe1fff8 0x39200000 0x06000000 0xa3e30000 <0x04000000 0xf7e40000> 0xebe1fff8 0x4e800020 0x00000000 0x00000000

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
 arch/powerpc/kernel/process.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Christophe Leroy Feb. 22, 2022, 2:34 p.m. UTC | #1
Le 02/06/2020 à 07:27, Jordan Niethe a écrit :
> Currently prefixed instructions are treated as two word instructions by
> show_user_instructions(), treat them as a single instruction. '<' and
> '>' are placed around the instruction at the NIP, and for prefixed
> instructions this is placed around the prefix only. Make the '<' and '>'
> wrap the prefix and suffix.
> 
> Currently showing a prefixed instruction looks like:
> fbe1fff8 39200000 06000000 a3e30000 <04000000> f7e40000 ebe1fff8 4e800020
> 
> Make it look like:
> 0xfbe1fff8 0x39200000 0x06000000 0xa3e30000 <0x04000000 0xf7e40000> 0xebe1fff8 0x4e800020 0x00000000 0x00000000

Is it really needed to have the leading 0x ?

And is there a reason for that two 0x00000000 at the end of the new line 
that we don't have at the end of the old line ?

This is initially split into 8 instructions per line in order to fit in 
a 80 columns screen/terminal.

Could you make it such that it still fits within 80 cols ?

Same for patch 4 on show_user_instructions()

Christophe
Jordan Niethe March 14, 2022, 11 p.m. UTC | #2
On Wed, Feb 23, 2022 at 1:34 AM Christophe Leroy
<christophe.leroy@csgroup.eu> wrote:
>
>
>
> Le 02/06/2020 à 07:27, Jordan Niethe a écrit :
> > Currently prefixed instructions are treated as two word instructions by
> > show_user_instructions(), treat them as a single instruction. '<' and
> > '>' are placed around the instruction at the NIP, and for prefixed
> > instructions this is placed around the prefix only. Make the '<' and '>'
> > wrap the prefix and suffix.
> >
> > Currently showing a prefixed instruction looks like:
> > fbe1fff8 39200000 06000000 a3e30000 <04000000> f7e40000 ebe1fff8 4e800020
> >
> > Make it look like:
> > 0xfbe1fff8 0x39200000 0x06000000 0xa3e30000 <0x04000000 0xf7e40000> 0xebe1fff8 0x4e800020 0x00000000 0x00000000
>
> Is it really needed to have the leading 0x ?
You are right, that is not consistent with how instructions are usually dumped.
That formatting comes from ppc_inst_as_str(), which when mpe merged he
removed the leading 0x.

>
> And is there a reason for that two 0x00000000 at the end of the new line
> that we don't have at the end of the old line ?
No, that is wrong.
>
> This is initially split into 8 instructions per line in order to fit in
> a 80 columns screen/terminal.
>
> Could you make it such that it still fits within 80 cols ?
Sure that makes sense.
>
> Same for patch 4 on show_user_instructions()
>
> Christophe
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
index 048d64c4e115..b3f73e398d00 100644
--- a/arch/powerpc/kernel/process.c
+++ b/arch/powerpc/kernel/process.c
@@ -1292,7 +1292,8 @@  void show_user_instructions(struct pt_regs *regs)
 	unsigned long pc;
 	int n = NR_INSN_TO_PRINT;
 	struct seq_buf s;
-	char buf[96]; /* enough for 8 times 9 + 2 chars */
+	char buf[8 * sizeof("0x00000000 0x00000000") + 2];
+	struct ppc_inst instr;
 
 	pc = regs->nip - (NR_INSN_TO_PRINT * 3 / 4 * sizeof(int));
 
@@ -1303,14 +1304,15 @@  void show_user_instructions(struct pt_regs *regs)
 
 		seq_buf_clear(&s);
 
-		for (i = 0; i < 8 && n; i++, n--, pc += sizeof(int)) {
-			int instr;
+		for (i = 0; i < 8 && n; i++, n--, pc += ppc_inst_len(instr)) {
 
-			if (probe_user_read(&instr, (void __user *)pc, sizeof(instr))) {
+			if (probe_user_read_inst(&instr, (void __user *)pc)) {
 				seq_buf_printf(&s, "XXXXXXXX ");
+				instr = ppc_inst(PPC_INST_NOP);
 				continue;
 			}
-			seq_buf_printf(&s, regs->nip == pc ? "<%08x> " : "%08x ", instr);
+			seq_buf_printf(&s, regs->nip == pc ? "<%s> " : "%s ",
+				       ppc_inst_as_str(instr));
 		}
 
 		if (!seq_buf_has_overflowed(&s))