public inbox for linuxppc-dev@ozlabs.org 
 help / color / mirror / Atom feed
From: Alistair Popple <alistair@popple•id.au>
To: Jordan Niethe <jniethe5@gmail•com>
Cc: npiggin@gmail•com, bala24@linux•ibm.com,
	linuxppc-dev@lists•ozlabs.org, dja@axtens•net
Subject: Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
Date: Tue, 07 Apr 2020 11:39:17 +1000	[thread overview]
Message-ID: <40677526.2jpsrtoC1b@townsend> (raw)
In-Reply-To: <CACzsE9qhUXC3YkLALLrqJvk0vJLru1j4YbTrQvQOaF3rGom=kw@mail.gmail.com>

On Monday, 6 April 2020 8:42:57 PM AEST Jordan Niethe wrote:
> On Mon, 6 Apr 2020, 7:52 pm Alistair Popple, <alistair@popple•id.au> wrote:
> > > diff --git a/arch/powerpc/include/asm/inst.h
> > > b/arch/powerpc/include/asm/inst.h index 70b37a35a91a..7e23e7146c66
> > > 100644
> > > --- a/arch/powerpc/include/asm/inst.h
> > > +++ b/arch/powerpc/include/asm/inst.h
> > > @@ -8,23 +8,67 @@
> > > 
> > >  struct ppc_inst {
> > >  
> > >          u32 val;
> > > 
> > > +#ifdef __powerpc64__
> > > +        u32 suffix;
> > > +#endif /* __powerpc64__ */
> > > 
> > >  } __packed;
> > > 
> > > -#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > > +static inline int ppc_inst_opcode(struct ppc_inst x)
> > > +{
> > > +     return x.val >> 26;
> > > +}
> > > 
> > >  static inline u32 ppc_inst_val(struct ppc_inst x)
> > >  {
> > >  
> > >       return x.val;
> > >  
> > >  }
> > > 
> > > -static inline bool ppc_inst_len(struct ppc_inst x)
> > > +#ifdef __powerpc64__
> > > +#define ppc_inst(x) ((struct ppc_inst){ .val = (x), .suffix = 0xff })
> > > +
> > > +#define ppc_inst_prefix(x, y) ((struct ppc_inst){ .val = (x), .suffix =
> > 
> > (y)
> > 
> > > }) +
> > > +static inline u32 ppc_inst_suffix(struct ppc_inst x)
> > > 
> > >  {
> > > 
> > > -     return sizeof(struct ppc_inst);
> > > +     return x.suffix;
> > > 
> > >  }
> > > 
> > > -static inline int ppc_inst_opcode(struct ppc_inst x)
> > > +static inline bool ppc_inst_prefixed(struct ppc_inst x) {
> > > +     return ((ppc_inst_val(x) >> 26) == 1) && ppc_inst_suffix(x) !=
> > 
> > 0xff;
> > 
> > > +}
> > > +
> > > +static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > > 
> > >  {
> > > 
> > > -     return x.val >> 26;
> > > +     return ppc_inst_prefix(swab32(ppc_inst_val(x)),
> > > +                            swab32(ppc_inst_suffix(x)));
> > > +}
> > > +
> > > +static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > > +{
> > > +     u32 val, suffix = 0xff;
> > > +     val = *(u32 *)ptr;
> > > +     if ((val >> 26) == 1)
> > > +             suffix = *((u32 *)ptr + 1);
> > > +     return ppc_inst_prefix(val, suffix);
> > > +}
> > > +
> > > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> > 
> > x)
> > 
> > > +{
> > > +     if (ppc_inst_prefixed(x)) {
> > > +             *(u32 *)ptr = x.val;
> > > +             *((u32 *)ptr + 1) = x.suffix;
> > > +     } else {
> > > +             *(u32 *)ptr = x.val;
> > > +     }
> > > +}
> > > +
> > > +#else
> > > +
> > > +#define ppc_inst(x) ((struct ppc_inst){ .val = x })
> > > +
> > > +static inline bool ppc_inst_prefixed(ppc_inst x)
> > > +{
> > > +     return 0;
> > > 
> > >  }
> > >  
> > >  static inline struct ppc_inst ppc_inst_swab(struct ppc_inst x)
> > > 
> > > @@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
> > > ppc_inst x) return ppc_inst(swab32(ppc_inst_val(x)));
> > > 
> > >  }
> > > 
> > > +static inline u32 ppc_inst_val(struct ppc_inst x)
> > > +{
> > > +     return x.val;
> > > +}
> > > +
> > > 
> > >  static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
> > >  {
> > >  
> > >       return *ptr;
> > >  
> > >  }
> > > 
> > > +static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst
> > 
> > x)
> > 
> > > +{
> > > +     *ptr = x;
> > > +}
> > > +
> > > +#endif /* __powerpc64__ */
> > > +
> > > 
> > >  static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
> > >  {
> > >  
> > >       return !memcmp(&x, &y, sizeof(struct ppc_inst));
> > >  
> > >  }
> > 
> > Apologies for not picking this up earlier, I was hoping to get to the
> > bottom
> > of the issue I was seeing before you sent out v5. However the above
> > definition
> > of instruction equality does not seem correct because it does not consider
> > the
> > case when an instruction is not prefixed - a non-prefixed instruction
> > should be
> > considered equal if the first 32-bit opcode/value is the same. Something
> > 
> > like:
> >         if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
> >         
> >                 return false;
> >         
> >         else if (ppc_inst_prefixed(x))
> >         
> >                 return !memcmp(&x, &y, sizeof(struct ppc_inst));
> >         
> >         else
> >         
> >                 return x.val == y.val;
> > 
> > This was causing failures in ftrace_modify_code() as it would falsely
> > detect
> > two non-prefixed instructions as being not equal due to differences in the
> > suffix.
> 
> Hm I was intending that non prefixed instructions would always have the
> suffix set to the same value. If that's not happening, something must be
> wrong with where the instructions are created.
> 

Ok, based on your comment I found the problem. Your last patch series defined 
read_inst() in  ftrace.c and ended that function with:

	ppc_inst_write(p, inst);
	return 0;

This is called from ftrace_modify_code(), where p is uninitialised. In the 
last series ppc_inst_write() function would only write/initialise the suffix if 
it was a prefixed instruction, hence leaving it uninitialised in this instance 
which lead to the problems checking equality.

I suspect read_instr() should have finished with this instead:

	*p = inst;
	return 0;

However your new patch series replaces it with probe_kernel_read_inst() which 
looks to do the right thing:

+       *inst = ppc_inst_prefix(val, suffix);
+
+       return 0;

As the suffix in *inst will always get written now, so my previous comment 
appears invalid.

- Alistair

> > - Alistair
> > 
> > > +static inline int ppc_inst_len(struct ppc_inst x)
> > > +{
> > > +     return (ppc_inst_prefixed(x)) ? 8  : 4;
> > > +}
> > > +
> > > 
> > >  #endif /* _ASM_INST_H */
> > > 
> > > diff --git a/arch/powerpc/include/asm/kprobes.h
> > > b/arch/powerpc/include/asm/kprobes.h index 66b3f2983b22..4fc0e15e23a5
> > > 100644
> > > --- a/arch/powerpc/include/asm/kprobes.h
> > > +++ b/arch/powerpc/include/asm/kprobes.h
> > > @@ -43,7 +43,7 @@ extern kprobe_opcode_t optprobe_template_ret[];
> > > 
> > >  extern kprobe_opcode_t optprobe_template_end[];
> > >  
> > >  /* Fixed instruction size for powerpc */
> > > 
> > > -#define MAX_INSN_SIZE                1
> > > +#define MAX_INSN_SIZE                2
> > > 
> > >  #define MAX_OPTIMIZED_LENGTH sizeof(kprobe_opcode_t) /* 4 bytes */
> > >  #define MAX_OPTINSN_SIZE     (optprobe_template_end -
> > 
> > optprobe_template_entry)
> > 
> > >  #define RELATIVEJUMP_SIZE    sizeof(kprobe_opcode_t) /* 4 bytes */
> > > 
> > > diff --git a/arch/powerpc/include/asm/uaccess.h
> > > b/arch/powerpc/include/asm/uaccess.h index c0a35e4586a5..5a3f486ddf02
> > > 100644
> > > --- a/arch/powerpc/include/asm/uaccess.h
> > > +++ b/arch/powerpc/include/asm/uaccess.h
> > > @@ -105,11 +105,34 @@ static inline int __access_ok(unsigned long addr,
> > > unsigned long size, #define __put_user_inatomic(x, ptr) \
> > > 
> > >       __put_user_nosleep((__typeof__(*(ptr)))(x), (ptr), sizeof(*(ptr)))
> > > 
> > > -#define __get_user_instr(x, ptr) \
> > > -     __get_user_nocheck((x).val, (u32 *)(ptr), sizeof(u32), true)
> > > +#define __get_user_instr(x, ptr)                     \
> > > +({                                                   \
> > > +     long __gui_ret = 0;                             \
> > > +     unsigned int prefix, suffix;                    \
> > > +     __gui_ret = __get_user(prefix, (unsigned int __user *)ptr);
> > > 
> >      \
> > > 
> > > +     if (!__gui_ret && (prefix >> 26) == 1) {        \
> > > +             __gui_ret = __get_user(suffix, (unsigned int __user *)ptr
> > 
> > + 1); \
> > 
> > > +             (x) = ppc_inst_prefix(prefix, suffix);  \
> > > +     } else {                                        \
> > > +             (x) = ppc_inst(prefix);                 \
> > > +     }                                               \
> > > +     __gui_ret;                                      \
> > > +})
> > > +
> > > +#define __get_user_instr_inatomic(x, ptr)            \
> > > +({                                                   \
> > > +     long __gui_ret = 0;                             \
> > > +     unsigned int prefix, suffix;                    \
> > > +     __gui_ret = __get_user_inatomic(prefix, (unsigned int __user
> > 
> > *)ptr);            \
> > 
> > > +     if (!__gui_ret && (prefix >> 26) == 1) {        \
> > > +             __gui_ret = __get_user_inatomic(suffix, (unsigned int
> > 
> > __user *)ptr +
> > 
> > > 1);   \ +             (x) = ppc_inst_prefix(prefix, suffix);  \
> > > +     } else {                                        \
> > > +             (x) = ppc_inst(prefix);                 \
> > > +     }                                               \
> > > +     __gui_ret;                                      \
> > > +})
> > > 
> > > -#define __get_user_instr_inatomic(x, ptr) \
> > > -     __get_user_nosleep((x).val, (u32 *)(ptr), sizeof(u32))
> > > 
> > >  extern long __put_user_bad(void);
> > >  
> > >  /*
> > > 
> > > diff --git a/arch/powerpc/include/asm/uprobes.h
> > > b/arch/powerpc/include/asm/uprobes.h index 7e3b329ba2d3..5bf65f5d44a9
> > > 100644
> > > --- a/arch/powerpc/include/asm/uprobes.h
> > > +++ b/arch/powerpc/include/asm/uprobes.h
> > > @@ -15,7 +15,7 @@
> > > 
> > >  typedef ppc_opcode_t uprobe_opcode_t;
> > > 
> > > -#define MAX_UINSN_BYTES              4
> > > +#define MAX_UINSN_BYTES              8
> > > 
> > >  #define UPROBE_XOL_SLOT_BYTES        (MAX_UINSN_BYTES)
> > >  
> > >  /* The following alias is needed for reference from arch-agnostic code
> > 
> > */
> > 
> > > diff --git a/arch/powerpc/kernel/optprobes.c
> > > b/arch/powerpc/kernel/optprobes.c index 684640b8fa2e..689daf430161
> > > 100644
> > > --- a/arch/powerpc/kernel/optprobes.c
> > > +++ b/arch/powerpc/kernel/optprobes.c
> > > @@ -159,38 +159,38 @@ void patch_imm32_load_insns(unsigned int val,
> > > kprobe_opcode_t *addr)
> > > 
> > >  /*
> > >  
> > >   * Generate instructions to load provided immediate 64-bit value
> > > 
> > > - * to register 'r3' and patch these instructions at 'addr'.
> > > + * to register 'reg' and patch these instructions at 'addr'.
> > > 
> > >   */
> > > 
> > > -void patch_imm64_load_insns(unsigned long val, kprobe_opcode_t *addr)
> > > +void patch_imm64_load_insns(unsigned long val, int reg, kprobe_opcode_t
> > > *addr) {
> > > -     /* lis r3,(op)@highest */
> > > -     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> > > 
> > > ___PPC_RT(3) | +      /* lis reg,(op)@highest */
> > > +     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ADDIS
> > > 
> > > ___PPC_RT(reg) | ((val >> 48) & 0xffff)));
> > > 
> > >       addr++;
> > > 
> > > -     /* ori r3,r3,(op)@higher */
> > > -     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(3) | -                        ___PPC_RS(3) | ((val >> 32) &
> > 
> > 0xffff)));
> > 
> > > +     /* ori reg,reg,(op)@higher */
> > > +     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(reg) | +                      ___PPC_RS(reg) | ((val >> 32) &
> > 
> > 0xffff)));
> > 
> > >       addr++;
> > > 
> > > -     /* rldicr r3,r3,32,31 */
> > > -     patch_instruction((struct ppc_inst *)addr,
> > 
> > ppc_inst(PPC_INST_RLDICR |
> > 
> > > ___PPC_RA(3) | -                        ___PPC_RS(3) | __PPC_SH64(32) |
> > 
> > __PPC_ME64(31)));
> > 
> > > +     /* rldicr reg,reg,32,31 */
> > > +     patch_instruction((struct ppc_inst *)addr,
> > 
> > ppc_inst(PPC_INST_RLDICR |
> > 
> > > ___PPC_RA(reg) | +                      ___PPC_RS(reg) | __PPC_SH64(32)
> > 
> > __PPC_ME64(31)));
> > 
> > >       addr++;
> > > 
> > > -     /* oris r3,r3,(op)@h */
> > > -     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS
> > > |
> > > ___PPC_RA(3) | -                        ___PPC_RS(3) | ((val >> 16) &
> > 
> > 0xffff)));
> > 
> > > +     /* oris reg,reg,(op)@h */
> > > +     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORIS
> > > |
> > > ___PPC_RA(reg) | +                      ___PPC_RS(reg) | ((val >> 16) &
> > 
> > 0xffff)));
> > 
> > >       addr++;
> > > 
> > > -     /* ori r3,r3,(op)@l */
> > > -     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(3) | -                        ___PPC_RS(3) | (val & 0xffff)));
> > > +     /* ori reg,reg,(op)@l */
> > > +     patch_instruction((struct ppc_inst *)addr, ppc_inst(PPC_INST_ORI |
> > > ___PPC_RA(reg) | +                      ___PPC_RS(reg) | (val &
> > 
> > 0xffff)));
> > 
> > >  }
> > >  
> > >  int arch_prepare_optimized_kprobe(struct optimized_kprobe *op, struct
> > > 
> > > kprobe *p) {
> > > -     struct ppc_inst branch_op_callback, branch_emulate_step;
> > > +     struct ppc_inst branch_op_callback, branch_emulate_step, temp;
> > > 
> > >       kprobe_opcode_t *op_callback_addr, *emulate_step_addr, *buff;
> > >       long b_offset;
> > >       unsigned long nip, size;
> > > 
> > > @@ -240,7 +240,7 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p) * Fixup the template with
> > > 
> > > instructions to:
> > >        * 1. load the address of the actual probepoint
> > >        */
> > > 
> > > -     patch_imm64_load_insns((unsigned long)op, buff + TMPL_OP_IDX);
> > > +     patch_imm64_load_insns((unsigned long)op, 3, buff + TMPL_OP_IDX);
> > > 
> > >       /*
> > >       
> > >        * 2. branch to optimized_callback() and emulate_step()
> > > 
> > > @@ -271,7 +271,11 @@ int arch_prepare_optimized_kprobe(struct
> > > optimized_kprobe *op, struct kprobe *p) /*
> > > 
> > >        * 3. load instruction to be emulated into relevant register, and
> > >        */
> > > 
> > > -     patch_imm32_load_insns(*p->ainsn.insn, buff + TMPL_INSN_IDX);
> > > +     temp = ppc_inst_read((struct ppc_inst *)p->ainsn.insn);
> > > +     patch_imm64_load_insns(ppc_inst_val(temp) |
> > > +                            ((u64)ppc_inst_suffix(temp) << 32),
> > > +                            4,
> > > +                            buff + TMPL_INSN_IDX);
> > > 
> > >       /*
> > >       
> > >        * 4. branch back from trampoline
> > > 
> > > diff --git a/arch/powerpc/kernel/optprobes_head.S
> > > b/arch/powerpc/kernel/optprobes_head.S index cf383520843f..ff8ba4d3824d
> > > 100644
> > > --- a/arch/powerpc/kernel/optprobes_head.S
> > > +++ b/arch/powerpc/kernel/optprobes_head.S
> > > 
> > > @@ -94,6 +94,9 @@ optprobe_template_insn:
> > >       /* 2, Pass instruction to be emulated in r4 */
> > >       nop
> > >       nop
> > > 
> > > +     nop
> > > +     nop
> > > +     nop
> > > 
> > >       .global optprobe_template_call_emulate
> > >  
> > >  optprobe_template_call_emulate:
> > > diff --git a/arch/powerpc/kernel/trace/ftrace.c
> > > b/arch/powerpc/kernel/trace/ftrace.c index e78742613b36..16041a5c86d5
> > > 100644
> > > --- a/arch/powerpc/kernel/trace/ftrace.c
> > > +++ b/arch/powerpc/kernel/trace/ftrace.c
> > > @@ -41,11 +41,35 @@
> > > 
> > >  #define      NUM_FTRACE_TRAMPS       8
> > >  static unsigned long ftrace_tramps[NUM_FTRACE_TRAMPS];
> > > 
> > > +#ifdef __powerpc64__
> > > 
> > >  static long
> > >  probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > >  {
> > > 
> > > -     return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE);
> > > +     u32 val, suffix = 0;
> > > +     long err;
> > > +
> > > +     err = probe_kernel_read((void *)&val,
> > > +                             src, sizeof(val));
> > > +     if (err)
> > > +             return err;
> > > +
> > > +     if ((val >> 26) == 1)
> > > +             err = probe_kernel_read((void *)&suffix,
> > > +                                     src + 4, MCOUNT_INSN_SIZE);
> > > +     if (err)
> > > +             return err;
> > > +
> > > +     *inst = ppc_inst_prefix(val, suffix);
> > > +
> > > +     return 0;
> > > 
> > >  }
> > > 
> > > +#else
> > > +static long
> > > +probe_kernel_read_inst(struct ppc_inst *inst, const void *src)
> > > +{
> > > +     return probe_kernel_read((void *)inst, src, MCOUNT_INSN_SIZE)
> > > +}
> > > +#endif
> > > 
> > >  static struct ppc_inst
> > >  ftrace_call_replace(unsigned long ip, unsigned long addr, int link)
> > > 
> > > diff --git a/arch/powerpc/lib/code-patching.c
> > > b/arch/powerpc/lib/code-patching.c index c329ad657302..b4007e03d8fa
> > 
> > 100644
> > 
> > > --- a/arch/powerpc/lib/code-patching.c
> > > +++ b/arch/powerpc/lib/code-patching.c
> > > @@ -24,12 +24,19 @@ static int __patch_instruction(struct ppc_inst
> > > *exec_addr, struct ppc_inst instr {
> > > 
> > >       int err = 0;
> > > 
> > > -     __put_user_asm(ppc_inst_val(instr), patch_addr, err, "stw");
> > > -     if (err)
> > > -             return err;
> > > -
> > > -     asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > 
> > (patch_addr),
> > 
> > > -                                                         "r"
> > 
> > (exec_addr));
> > 
> > > +     if (!ppc_inst_prefixed(instr)) {
> > > +             __put_user_asm(ppc_inst_val(instr), patch_addr, err,
> > 
> > "stw");
> > 
> > > +             if (err)
> > > +                     return err;
> > > +             asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > 
> > (patch_addr),
> > 
> > > +                                                                 "r"
> > 
> > (exec_addr));
> > 
> > > +     } else {
> > > +             __put_user_asm((u64)ppc_inst_suffix(instr) << 32 |
> > 
> > ppc_inst_val(instr),
> > 
> > > patch_addr, err, "std"); +            if (err)
> > > +                     return err;
> > > +             asm ("dcbst 0, %0; sync; icbi 0,%1; sync; isync" :: "r"
> > 
> > (patch_addr),
> > 
> > > +                                                                 "r"
> > 
> > (exec_addr));
> > 
> > > +     }
> > > 
> > >       return 0;
> > >  
> > >  }
> > > 
> > > diff --git a/arch/powerpc/lib/feature-fixups.c
> > > b/arch/powerpc/lib/feature-fixups.c index f00dd13b1c3c..5519cec83cc8
> > 
> > 100644
> > 
> > > --- a/arch/powerpc/lib/feature-fixups.c
> > > +++ b/arch/powerpc/lib/feature-fixups.c
> > > @@ -84,12 +84,13 @@ static int patch_feature_section(unsigned long
> > > value,
> > > struct fixup_entry *fcur) src = alt_start;
> > > 
> > >       dest = start;
> > > 
> > > -     for (; src < alt_end; src++, dest++) {
> > > +     for (; src < alt_end; src = (void *)src +
> > > ppc_inst_len(ppc_inst_read(src)), +        (dest = (void *)dest +
> > > ppc_inst_len(ppc_inst_read(dest)))) { if (patch_alt_instruction(src,
> > 
> > dest,
> > 
> > > alt_start, alt_end))
> > > 
> > >                       return 1;
> > >       
> > >       }
> > > 
> > > -     for (; dest < end; dest++)
> > > +     for (; dest < end; dest = (void *)dest +
> > > ppc_inst_len(ppc_inst(PPC_INST_NOP))) raw_patch_instruction(dest,
> > > ppc_inst(PPC_INST_NOP));
> > > 
> > >       return 0;
> > > 
> > > diff --git a/arch/powerpc/lib/sstep.c b/arch/powerpc/lib/sstep.c
> > > index 52ddd3122dc8..8b285bf11218 100644
> > > --- a/arch/powerpc/lib/sstep.c
> > > +++ b/arch/powerpc/lib/sstep.c
> > > @@ -1169,10 +1169,12 @@ int analyse_instr(struct instruction_op *op,
> > 
> > const
> > 
> > > struct pt_regs *regs, unsigned long int imm;
> > > 
> > >       unsigned long int val, val2;
> > >       unsigned int mb, me, sh;
> > > 
> > > -     unsigned int word;
> > > +     unsigned int word, suffix;
> > > 
> > >       long ival;
> > >       
> > >       word = ppc_inst_val(instr);
> > > 
> > > +     suffix = ppc_inst_suffix(instr);
> > > +
> > > 
> > >       op->type = COMPUTE;
> > >       
> > >       opcode = word >> 26;
> > > 
> > > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > > index 6f3bcdcfc9c7..b704aebb099a 100644
> > > --- a/arch/powerpc/xmon/xmon.c
> > > +++ b/arch/powerpc/xmon/xmon.c
> > > @@ -761,8 +761,8 @@ static int xmon_bpt(struct pt_regs *regs)
> > > 
> > >       /* Are we at the trap at bp->instr[1] for some bp? */
> > >       bp = in_breakpoint_table(regs->nip, &offset);
> > > 
> > > -     if (bp != NULL && offset == 4) {
> > > -             regs->nip = bp->address + 4;
> > > +     if (bp != NULL && (offset == 4 || offset == 8)) {
> > > +             regs->nip = bp->address + offset;
> > > 
> > >               atomic_dec(&bp->ref_count);
> > >               return 1;
> > >       
> > >       }
> > > 
> > > @@ -863,7 +863,7 @@ static struct bpt *in_breakpoint_table(unsigned long
> > > nip, unsigned long *offp) if (off >= sizeof(bpt_table))
> > > 
> > >               return NULL;
> > >       
> > >       *offp = off % BPT_SIZE;
> > > 
> > > -     if (*offp != 0 && *offp != 4)
> > > +     if (*offp != 0 && *offp != 4 && *offp != 8)
> > > 
> > >               return NULL;
> > >       
> > >       return bpts + (off / BPT_SIZE);
> > >  
> > >  }
> > > 
> > > diff --git a/arch/powerpc/xmon/xmon_bpts.S
> > 
> > b/arch/powerpc/xmon/xmon_bpts.S
> > 
> > > index ebb2dbc70ca8..09058eb6abbd 100644
> > > --- a/arch/powerpc/xmon/xmon_bpts.S
> > > +++ b/arch/powerpc/xmon/xmon_bpts.S
> > > @@ -3,6 +3,8 @@
> > > 
> > >  #include <asm/asm-compat.h>
> > >  #include "xmon_bpts.h"
> > > 
> > > +/* Prefixed instructions can not cross 64 byte boundaries */
> > > +.align 6
> > > 
> > >  .global bpt_table
> > > 
> > >  bpt_table:
> > > -     .space NBPTS * 8
> > > +     .space NBPTS * 16





  reply	other threads:[~2020-04-07  1:41 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-06  8:09 [PATCH v5 00/21] Initial Prefixed Instruction support Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 01/21] powerpc/xmon: Remove store_inst() for patch_instruction() Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 02/21] powerpc/xmon: Move out-of-line instructions to text section Jordan Niethe
2020-04-07  6:45   ` Balamuruhan S
2020-04-09  6:11   ` Christophe Leroy
2020-04-09  7:26     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 03/21] powerpc: Change calling convention for create_branch() et. al Jordan Niethe
2020-04-06 10:25   ` kbuild test robot
2020-04-07  6:10   ` Balamuruhan S
2020-04-07  6:35     ` Jordan Niethe
2020-04-07  6:59       ` Balamuruhan S
2020-04-06  8:09 ` [PATCH v5 04/21] powerpc: Use a macro for creating instructions from u32s Jordan Niethe
2020-04-07  6:40   ` Balamuruhan S
2020-04-07  8:27     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 05/21] powerpc: Use a function for getting the instruction op code Jordan Niethe
2020-04-06  8:22   ` Christophe Leroy
2020-04-06  9:38     ` Jordan Niethe
2020-04-07  7:04   ` Balamuruhan S
2020-04-07  8:32     ` Jordan Niethe
2020-04-08 18:21   ` Segher Boessenkool
2020-04-09  4:48     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 06/21] powerpc: Use an accessor for instructions Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 07/21] powerpc: Use a function for byte swapping instructions Jordan Niethe
2020-04-07  7:42   ` Balamuruhan S
2020-04-06  8:09 ` [PATCH v5 08/21] powerpc: Introduce functions for instruction equality Jordan Niethe
2020-04-07  7:37   ` Balamuruhan S
2020-04-06  8:09 ` [PATCH v5 09/21] powerpc: Use a datatype for instructions Jordan Niethe
2020-04-06 10:34   ` kbuild test robot
2020-04-06 10:35   ` kbuild test robot
2020-04-07 10:30   ` Balamuruhan S
2020-04-08  2:11     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 10/21] powerpc: Use a function for reading instructions Jordan Niethe
2020-04-07 10:42   ` Balamuruhan S
2020-04-06  8:09 ` [PATCH v5 11/21] powerpc: Define and use __get_user_instr{, inatomic}() Jordan Niethe
2020-04-07 10:48   ` Balamuruhan S
2020-04-08  2:13     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 12/21] powerpc: Introduce a function for reporting instruction length Jordan Niethe
2020-04-07 11:14   ` Balamuruhan S
2020-04-08  2:14     ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 13/21] powerpc/xmon: Use a function for reading instructions Jordan Niethe
2020-04-07 11:30   ` Balamuruhan S
2020-04-08  2:18     ` Jordan Niethe
2020-04-09  5:04       ` Balamuruhan S
2020-04-09  5:14         ` Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 14/21] powerpc/xmon: Move insertion of breakpoint for xol'ing Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 15/21] powerpc: Make test_translate_branch() independent of instruction length Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 16/21] powerpc: Enable Prefixed Instructions Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 17/21] powerpc: Define new SRR1 bits for a future ISA version Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type Jordan Niethe
2020-04-06  9:52   ` Alistair Popple
2020-04-06 10:25     ` Christophe Leroy
2020-04-06 11:13       ` Jordan Niethe
2020-04-08 18:11       ` Segher Boessenkool
2020-04-08 18:43         ` Christophe Leroy
2020-04-06 10:42     ` Jordan Niethe
2020-04-07  1:39       ` Alistair Popple [this message]
2020-04-06 11:04   ` kbuild test robot
2020-04-13 12:04   ` Balamuruhan S
2020-04-15  4:40     ` Jordan Niethe
2020-04-15  8:14       ` Balamuruhan S
2020-04-06  8:09 ` [PATCH v5 19/21] powerpc: Support prefixed instructions in alignment handler Jordan Niethe
2020-04-06  8:09 ` [PATCH v5 20/21] powerpc sstep: Add support for prefixed load/stores Jordan Niethe
2020-04-06 11:29   ` kbuild test robot
2020-04-06  8:09 ` [PATCH v5 21/21] powerpc sstep: Add support for prefixed fixed-point arithmetic Jordan Niethe
2020-04-09  6:39 ` [PATCH v5 00/21] Initial Prefixed Instruction support Christophe Leroy
2020-04-09  7:28   ` Jordan Niethe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=40677526.2jpsrtoC1b@townsend \
    --to=alistair@popple$(echo .)id.au \
    --cc=bala24@linux$(echo .)ibm.com \
    --cc=dja@axtens$(echo .)net \
    --cc=jniethe5@gmail$(echo .)com \
    --cc=linuxppc-dev@lists$(echo .)ozlabs.org \
    --cc=npiggin@gmail$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox