Subject: Re: [PATCH v7 19/28] powerpc/xmon: Move insertion of breakpoint for xol'ing

From: Alistair Popple <[email-masked]>
Date: Tue, 05 May 2020 12:19:04 +1000

I can't see any side-effects from patching both instructions at the same time.

Reviewed-by: Alistair Popple <[email-masked]>

On Friday, 1 May 2020 1:42:11 PM AEST Jordan Niethe wrote:
> When a new breakpoint is created, the second instruction of that
> breakpoint is patched with a trap instruction. This assumes the length
> of the instruction is always the same. In preparation for prefixed
> instructions, remove this assumption. Insert the trap instruction at the
> same time the first instruction is inserted.
> 
> Signed-off-by: Jordan Niethe <[email-masked]>
> ---
>  arch/powerpc/xmon/xmon.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 1947821e425d..fb2563079046 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -878,7 +878,6 @@ static struct bpt *new_breakpoint(unsigned long a)
>  		if (!bp->enabled && atomic_read(&bp->ref_count) == 0) {
>  			bp->address = a;
>  			bp->instr = (void *)(bpt_table + ((bp - bpts) * BPT_WORDS));
> -			patch_instruction(bp->instr + 1, ppc_inst(bpinstr));
>  			return bp;
>  		}
>  	}
> @@ -910,6 +909,7 @@ static void insert_bpts(void)
>  			continue;
>  		}
>  		patch_instruction(bp->instr, instr);
> +		patch_instruction((void *)bp->instr + ppc_inst_len(instr),
> ppc_inst(bpinstr)); if (bp->enabled & BP_CIABR)
>  			continue;
>  		if (patch_instruction((struct ppc_inst *)bp->address,