On Mon, 2020-04-06 at 18:09 +1000, Jordan Niethe wrote: > Currently in xmon, mread() is used for reading instructions. In > preparation for prefixed instructions, create and use a new function, > mread_instr(), especially for reading instructions. > > Signed-off-by: Jordan Niethe <jniet...@gmail.com> > --- > v5: New to series, seperated from "Add prefixed instructions to > instruction data type" > --- > arch/powerpc/xmon/xmon.c | 24 ++++++++++++++++++++---- > 1 file changed, 20 insertions(+), 4 deletions(-) > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index 5e3949322a6c..6f4cf01a58c1 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -125,6 +125,7 @@ extern unsigned int bpt_table[NBPTS * BPT_WORDS]; > 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, struct ppc_inst *); > static int handle_fault(struct pt_regs *); > static void byterev(unsigned char *, int); > static void memex(void); > @@ -899,7 +900,7 @@ static void insert_bpts(void) > for (i = 0; i < NBPTS; ++i, ++bp) { > if ((bp->enabled & (BP_TRAP|BP_CIABR)) == 0) > continue; > - if (mread(bp->address, &instr, 4) != 4) { > + if (!mread_instr(bp->address, &instr)) {
Are these checks made based on whether `ppc_inst_len()` returns bool from mread_instr() ? -- Bala > printf("Couldn't read instruction at %lx, " > "disabling breakpoint there\n", bp->address); > bp->enabled = 0; > @@ -949,7 +950,7 @@ static void remove_bpts(void) > for (i = 0; i < NBPTS; ++i, ++bp) { > if ((bp->enabled & (BP_TRAP|BP_CIABR)) != BP_TRAP) > continue; > - if (mread(bp->address, &instr, 4) == 4 > + if (mread_instr(bp->address, &instr) > && ppc_inst_equal(instr, ppc_inst(bpinstr)) > && patch_instruction( > (struct ppc_inst *)bp->address, ppc_inst_read(bp- > >instr)) != 0) > @@ -1165,7 +1166,7 @@ static int do_step(struct pt_regs *regs) > 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) { > + if (mread_instr(regs->nip, &instr)) { > stepped = emulate_step(regs, instr); > if (stepped < 0) { > printf("Couldn't single-step %s instruction\n", > @@ -1332,7 +1333,7 @@ static long check_bp_loc(unsigned long addr) > printf("Breakpoints may only be placed at kernel addresses\n"); > return 0; > } > - if (!mread(addr, &instr, sizeof(instr))) { > + if (!mread_instr(addr, &instr)) { > printf("Can't read instruction at address %lx\n", addr); > return 0; > } > @@ -2125,6 +2126,21 @@ mwrite(unsigned long adrs, void *buf, int size) > return n; > } > > +static int > +mread_instr(unsigned long adrs, struct ppc_inst *instr) > +{ > + if (setjmp(bus_error_jmp) == 0) { > + catch_memory_errors = 1; > + sync(); > + *instr = ppc_inst_read((struct ppc_inst *)adrs); > + sync(); > + /* wait a little while to see if we get a machine check */ > + __delay(200); > + } > + catch_memory_errors = 0; > + return ppc_inst_len(*instr); > +} > + > static int fault_type; > static int fault_except; > static char *fault_chars[] = { "--", "**", "##" };