Re: [PATCH v3 08/14] powerpc/xmon: Remove store_inst() for patch_instruction()
From: Nicholas Piggin <npiggin@gmail.com>
Date: 2020-02-26 07:07:34
Jordan Niethe's on February 26, 2020 2:07 pm:
quoted hunk ↗ jump to hunk
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 <redacted> --- arch/powerpc/xmon/xmon.c | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-)diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c index 897e512c6379..a673cf55641c 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));@@ -882,8 +877,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; } }@@ -913,7 +907,7 @@ static void insert_bpts(void) bp->enabled = 0; continue; } - store_inst(&bp->instr[0]); + patch_instruction(&bp->instr[0], bp->instr[0]);
Hmm that's a bit weird. Can you read instructions into a local variable first, do the checks on them, then patch them into their execution location? Otherwise, good cleanup. Thanks, Nick