| Message ID | 20200320051809.24332-2-jniethe5@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Initial Prefixed Instruction support | expand |
| Context | Check | Description |
|---|---|---|
| snowpatch_ozlabs/apply_patch | success | Successfully applied on branch powerpc/merge (8a445cbcb9f5090cb07ec6cbb89a8a1fc99a0ff7) |
| snowpatch_ozlabs/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 64 lines checked |
| snowpatch_ozlabs/needsstable | success | Patch has no Fixes tags |
Jordan Niethe's on March 20, 2020 3:17 pm: > For modifying instructions in xmon, patch_instruction() can serve the > same role that store_inst() is performing with the advantage of not > being specific to xmon. In some places patch_instruction() is already > being using followed by store_inst(). In these cases just remove the > store_inst(). Otherwise replace store_inst() with patch_instruction(). LGTM Reviewed-by: Nicholas Piggin <npiggin@gmail.com> > > Signed-off-by: Jordan Niethe <jniethe5@gmail.com> > --- > v4: Read into a local variable > --- > arch/powerpc/xmon/xmon.c | 18 +++++------------- > 1 file changed, 5 insertions(+), 13 deletions(-) > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c > index e8c84d265602..02e3bd62cab4 100644 > --- a/arch/powerpc/xmon/xmon.c > +++ b/arch/powerpc/xmon/xmon.c > @@ -325,11 +325,6 @@ static inline void sync(void) > asm volatile("sync; isync"); > } > > -static inline void store_inst(void *p) > -{ > - asm volatile ("dcbst 0,%0; sync; icbi 0,%0; isync" : : "r" (p)); > -} > - > static inline void cflush(void *p) > { > asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p)); > @@ -881,8 +876,7 @@ static struct bpt *new_breakpoint(unsigned long a) > for (bp = bpts; bp < &bpts[NBPTS]; ++bp) { > if (!bp->enabled && atomic_read(&bp->ref_count) == 0) { > bp->address = a; > - bp->instr[1] = bpinstr; > - store_inst(&bp->instr[1]); > + patch_instruction(&bp->instr[1], bpinstr); > return bp; > } > } > @@ -894,25 +888,26 @@ static struct bpt *new_breakpoint(unsigned long a) > static void insert_bpts(void) > { > int i; > + unsigned int instr; > struct bpt *bp; > > 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(bp->address, &instr, 4) != 4) { > printf("Couldn't read instruction at %lx, " > "disabling breakpoint there\n", bp->address); > bp->enabled = 0; > continue; > } > - if (IS_MTMSRD(bp->instr[0]) || IS_RFID(bp->instr[0])) { > + if (IS_MTMSRD(instr) || IS_RFID(instr)) { > printf("Breakpoint at %lx is on an mtmsrd or rfid " > "instruction, disabling it\n", bp->address); > bp->enabled = 0; > continue; > } > - store_inst(&bp->instr[0]); > + patch_instruction(bp->instr, instr); > if (bp->enabled & BP_CIABR) > continue; > if (patch_instruction((unsigned int *)bp->address, > @@ -922,7 +917,6 @@ static void insert_bpts(void) > bp->enabled &= ~BP_TRAP; > continue; > } > - store_inst((void *)bp->address); > } > } > > @@ -957,8 +951,6 @@ static void remove_bpts(void) > (unsigned int *)bp->address, bp->instr[0]) != 0) > printf("Couldn't remove breakpoint at %lx\n", > bp->address); > - else > - store_inst((void *)bp->address); > } > } > > -- > 2.17.1 > >
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index e8c84d265602..02e3bd62cab4 100644 --- a/arch/powerpc/xmon/xmon.c +++ b/arch/powerpc/xmon/xmon.c @@ -325,11 +325,6 @@ static inline void sync(void) asm volatile("sync; isync"); } -static inline void store_inst(void *p) -{ - asm volatile ("dcbst 0,%0; sync; icbi 0,%0; isync" : : "r" (p)); -} - static inline void cflush(void *p) { asm volatile ("dcbf 0,%0; icbi 0,%0" : : "r" (p)); @@ -881,8 +876,7 @@ static struct bpt *new_breakpoint(unsigned long a) for (bp = bpts; bp < &bpts[NBPTS]; ++bp) { if (!bp->enabled && atomic_read(&bp->ref_count) == 0) { bp->address = a; - bp->instr[1] = bpinstr; - store_inst(&bp->instr[1]); + patch_instruction(&bp->instr[1], bpinstr); return bp; } } @@ -894,25 +888,26 @@ static struct bpt *new_breakpoint(unsigned long a) static void insert_bpts(void) { int i; + unsigned int instr; struct bpt *bp; 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(bp->address, &instr, 4) != 4) { printf("Couldn't read instruction at %lx, " "disabling breakpoint there\n", bp->address); bp->enabled = 0; continue; } - if (IS_MTMSRD(bp->instr[0]) || IS_RFID(bp->instr[0])) { + if (IS_MTMSRD(instr) || IS_RFID(instr)) { printf("Breakpoint at %lx is on an mtmsrd or rfid " "instruction, disabling it\n", bp->address); bp->enabled = 0; continue; } - store_inst(&bp->instr[0]); + patch_instruction(bp->instr, instr); if (bp->enabled & BP_CIABR) continue; if (patch_instruction((unsigned int *)bp->address, @@ -922,7 +917,6 @@ static void insert_bpts(void) bp->enabled &= ~BP_TRAP; continue; } - store_inst((void *)bp->address); } } @@ -957,8 +951,6 @@ static void remove_bpts(void) (unsigned int *)bp->address, bp->instr[0]) != 0) printf("Couldn't remove breakpoint at %lx\n", bp->address); - else - store_inst((void *)bp->address); } }
For modifying instructions in xmon, patch_instruction() can serve the same role that store_inst() is performing with the advantage of not being specific to xmon. In some places patch_instruction() is already being using followed by store_inst(). In these cases just remove the store_inst(). Otherwise replace store_inst() with patch_instruction(). Signed-off-by: Jordan Niethe <jniethe5@gmail.com> --- v4: Read into a local variable --- arch/powerpc/xmon/xmon.c | 18 +++++------------- 1 file changed, 5 insertions(+), 13 deletions(-)