diff mbox series

[v4,02/16] xmon: Move out-of-line instructions to text section

Message ID 20200320051809.24332-3-jniethe5@gmail.com (mailing list archive)
State Superseded
Headers show
Series Initial Prefixed Instruction support | expand

Checks

Context Check Description
snowpatch_ozlabs/apply_patch success Successfully applied on branch powerpc/merge (8a445cbcb9f5090cb07ec6cbb89a8a1fc99a0ff7)
snowpatch_ozlabs/checkpatch warning total: 0 errors, 1 warnings, 1 checks, 63 lines checked
snowpatch_ozlabs/needsstable success Patch has no Fixes tags

Commit Message

Jordan Niethe March 20, 2020, 5:17 a.m. UTC
To execute an instruction out of line after a breakpoint, the NIP is set
to the address of struct bpt::instr. Here a copy of the instruction that
was replaced with a breakpoint is kept, along with a trap so normal flow
can be resumed after XOLing. The struct bpt's are located within the
data section. This is problematic as the data section may be marked as
no execute.

Instead of each struct bpt holding the instructions to be XOL'd, make a
new array, bpt_table[], with enough space to hold instructions for the
number of supported breakpoints. Place this array in the text section.
Make struct bpt::instr a pointer to the instructions in bpt_table[]
associated with that breakpoint. This association is a simple mapping:
bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
the copied instruction followed by a trap, so 2 words per breakpoint.

Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
---
v4: New to series
---
 arch/powerpc/kernel/vmlinux.lds.S |  2 +-
 arch/powerpc/xmon/xmon.c          | 22 +++++++++++++---------
 2 files changed, 14 insertions(+), 10 deletions(-)

Comments

Balamuruhan S March 23, 2020, 5:59 a.m. UTC | #1
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> To execute an instruction out of line after a breakpoint, the NIP is
> set
> to the address of struct bpt::instr. Here a copy of the instruction
> that
> was replaced with a breakpoint is kept, along with a trap so normal
> flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked
> as
> no execute.
> 
> Instead of each struct bpt holding the instructions to be XOL'd, make
> a
> new array, bpt_table[], with enough space to hold instructions for
> the
> number of supported breakpoints. Place this array in the text
> section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple
> mapping:
> bpts[n] -> bpt_table[n * words per breakpoint].

Can it separate commits ?
	* introduce the array bpt_table[] and make struct bpt::instr a 
		pointer to the instructions in bpt_table[].
	* place the array in text section.

> Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
>  arch/powerpc/kernel/vmlinux.lds.S |  2 +-
>  arch/powerpc/xmon/xmon.c          | 22 +++++++++++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
>  		ALIGN_FUNCTION();
>  #endif
>  		/* careful! __ftr_alt_* sections need to be close to
> .text */
> -		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text);
> +		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text .text.xmon_bpts);
>  #ifdef CONFIG_PPC64
>  		*(.tramp.ftrace.text);
>  #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
>  /* Breakpoint stuff */
>  struct bpt {
>  	unsigned long	address;
> -	unsigned int	instr[2];
> +	unsigned int	*instr;
>  	atomic_t	ref_count;
>  	int		enabled;
>  	unsigned long	pad;
> @@ -109,6 +109,7 @@ struct bpt {
>  #define BP_DABR		4
>  
>  #define NBPTS	256
> +#define BPT_WORDS	2
>  static struct bpt bpts[NBPTS];
>  static struct bpt dabr;
>  static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008;	/* trap
> */
>  
>  #define BP_NUM(bp)	((bp) - bpts + 1)
>  
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> BPT_WORDS];
> +
>  /* Prototypes */
>  static int cmds(struct pt_regs *);
>  static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long
> pc)
>  static struct bpt *in_breakpoint_table(unsigned long nip, unsigned
> long *offp)
>  {
>  	unsigned long off;
> +	unsigned long bp_off;
>  
> -	off = nip - (unsigned long) bpts;
> -	if (off >= sizeof(bpts))
> +	off = nip - (unsigned long) bpt_table;
> +	if (off >= sizeof(bpt_table))
>  		return NULL;
> -	off %= sizeof(struct bpt);
> -	if (off != offsetof(struct bpt, instr[0])
> -	    && off != offsetof(struct bpt, instr[1]))
> +	bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> +	if (bp_off != 0 && bp_off != 4)
>  		return NULL;
> -	*offp = off - offsetof(struct bpt, instr[0]);
> -	return (struct bpt *) (nip - off);
> +	*offp = bp_off;
> +	return bpts + ((off - bp_off) / (sizeof(unsigned int) *
> BPT_WORDS));

`(off - bp_off) / (sizeof(unsigned int) * BPT_WORDS)` seems to be the
actual breakpoint offset. Can we have something like,

#define NBPTS  256
#define BPT_WORDS      2
#define BPT_WORDS_SIZE (sizeof(unsigned int) * BPT_WORDS)
#define BPT_OFFSET(off, bp_word_off) ((off - bp_word_off) / 				BPT_WOR
DS_SIZE);
:::
:::
:::
bp_word_off = off % BPT_WORDS_SIZE;
if (bp_word_off != 0 && bp_word_off != 4)
        return NULL;
*offp = bp_word_off;
return bpts + BPT_OFFSET(off, bp_word_off);

-- Bala
>  }
 
 static struct bpt *new_breakpoint(unsigned long a)
@@ -876,7
> +879,8 @@ 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;
-			patch_instruction(&bp->instr[1], bpinstr);
+	
> 		bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
+	
> 		patch_instruction(bp->instr + 1, bpinstr);
 		
> 	return bp;
 		}
 	}
Balamuruhan S March 23, 2020, 6:05 a.m. UTC | #2
On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> To execute an instruction out of line after a breakpoint, the NIP is
> set
> to the address of struct bpt::instr. Here a copy of the instruction
> that
> was replaced with a breakpoint is kept, along with a trap so normal
> flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked
> as
> no execute.
> 
> Instead of each struct bpt holding the instructions to be XOL'd, make
> a
> new array, bpt_table[], with enough space to hold instructions for
> the
> number of supported breakpoints. Place this array in the text
> section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple
> mapping:
> bpts[n] -> bpt_table[n * words per breakpoint].

Can we have it in separate commits ?
	* introduce the array bpt_table[] and make struct bpt::instr a 
		pointer to the instructions in bpt_table[].
	* place the array in text section.

> Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> ---
> v4: New to series
> ---
>  arch/powerpc/kernel/vmlinux.lds.S |  2 +-
>  arch/powerpc/xmon/xmon.c          | 22 +++++++++++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
>  		ALIGN_FUNCTION();
>  #endif
>  		/* careful! __ftr_alt_* sections need to be close to
> .text */
> -		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text);
> +		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> __ftr_alt_* .ref.text .text.xmon_bpts);
>  #ifdef CONFIG_PPC64
>  		*(.tramp.ftrace.text);
>  #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
>  /* Breakpoint stuff */
>  struct bpt {
>  	unsigned long	address;
> -	unsigned int	instr[2];
> +	unsigned int	*instr;
>  	atomic_t	ref_count;
>  	int		enabled;
>  	unsigned long	pad;
> @@ -109,6 +109,7 @@ struct bpt {
>  #define BP_DABR		4
>  
>  #define NBPTS	256
> +#define BPT_WORDS	2
>  static struct bpt bpts[NBPTS];
>  static struct bpt dabr;
>  static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008;	/* trap
> */
>  
>  #define BP_NUM(bp)	((bp) - bpts + 1)
>  
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> BPT_WORDS];
> +
>  /* Prototypes */
>  static int cmds(struct pt_regs *);
>  static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long
> pc)
>  static struct bpt *in_breakpoint_table(unsigned long nip, unsigned
> long *offp)
>  {
>  	unsigned long off;
> +	unsigned long bp_off;
>  
> -	off = nip - (unsigned long) bpts;
> -	if (off >= sizeof(bpts))
> +	off = nip - (unsigned long) bpt_table;
> +	if (off >= sizeof(bpt_table))
>  		return NULL;
> -	off %= sizeof(struct bpt);
> -	if (off != offsetof(struct bpt, instr[0])
> -	    && off != offsetof(struct bpt, instr[1]))
> +	bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> +	if (bp_off != 0 && bp_off != 4)
>  		return NULL;
> -	*offp = off - offsetof(struct bpt, instr[0]);
> -	return (struct bpt *) (nip - off);
> +	*offp = bp_off;
> +	return bpts + ((off - bp_off) / (sizeof(unsigned int) *
> BPT_WORDS));

`(off - bp_off) / (sizeof(unsigned int) * BPT_WORDS)` seems to be the
actual breakpoint offset. Can we have something like,

#define NBPTS  256
#define BPT_WORDS      2
#define BPT_WORDS_SIZE (sizeof(unsigned int) * BPT_WORDS)
#define BPT_OFFSET(off, bp_word_off) ((off - bp_word_off) / \					BPT_WORDS_SIZE)
;
:::
:::
:::
bp_word_off = off % BPT_WORDS_SIZE;
if (bp_word_off != 0 && bp_word_off != 4)
        return NULL;
*offp = bp_word_off;
return bpts + BPT_OFFSET(off, bp_word_off);

-- Bala
Nicholas Piggin March 23, 2020, 6:22 a.m. UTC | #3
Jordan Niethe's on March 20, 2020 3:17 pm:
> To execute an instruction out of line after a breakpoint, the NIP is set
> to the address of struct bpt::instr. Here a copy of the instruction that
> was replaced with a breakpoint is kept, along with a trap so normal flow
> can be resumed after XOLing. The struct bpt's are located within the
> data section. This is problematic as the data section may be marked as
> no execute.
> 
> Instead of each struct bpt holding the instructions to be XOL'd, make a
> new array, bpt_table[], with enough space to hold instructions for the
> number of supported breakpoints. Place this array in the text section.
> Make struct bpt::instr a pointer to the instructions in bpt_table[]
> associated with that breakpoint. This association is a simple mapping:
> bpts[n] -> bpt_table[n * words per breakpoint]. Currently we only need
> the copied instruction followed by a trap, so 2 words per breakpoint.
> 
> Signed-off-by: Jordan Niethe <jniethe5@gmail.com>

Reviewed-by: Nicholas Piggin <npiggin@gmail.com>

> ---
> v4: New to series
> ---
>  arch/powerpc/kernel/vmlinux.lds.S |  2 +-
>  arch/powerpc/xmon/xmon.c          | 22 +++++++++++++---------
>  2 files changed, 14 insertions(+), 10 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
> index b4c89a1acebb..e90845b8c300 100644
> --- a/arch/powerpc/kernel/vmlinux.lds.S
> +++ b/arch/powerpc/kernel/vmlinux.lds.S
> @@ -86,7 +86,7 @@ SECTIONS
>  		ALIGN_FUNCTION();
>  #endif
>  		/* careful! __ftr_alt_* sections need to be close to .text */
> -		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text);
> +		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text .text.xmon_bpts);
>  #ifdef CONFIG_PPC64
>  		*(.tramp.ftrace.text);
>  #endif
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index 02e3bd62cab4..7875d1a37770 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
>  /* Breakpoint stuff */
>  struct bpt {
>  	unsigned long	address;
> -	unsigned int	instr[2];
> +	unsigned int	*instr;
>  	atomic_t	ref_count;
>  	int		enabled;
>  	unsigned long	pad;
> @@ -109,6 +109,7 @@ struct bpt {
>  #define BP_DABR		4
>  
>  #define NBPTS	256
> +#define BPT_WORDS	2
>  static struct bpt bpts[NBPTS];
>  static struct bpt dabr;
>  static struct bpt *iabr;
> @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008;	/* trap */
>  
>  #define BP_NUM(bp)	((bp) - bpts + 1)
>  
> +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS];
> +
>  /* Prototypes */
>  static int cmds(struct pt_regs *);
>  static int mread(unsigned long, void *, int);
> @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long pc)
>  static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
>  {
>  	unsigned long off;
> +	unsigned long bp_off;
>  
> -	off = nip - (unsigned long) bpts;
> -	if (off >= sizeof(bpts))
> +	off = nip - (unsigned long) bpt_table;
> +	if (off >= sizeof(bpt_table))
>  		return NULL;
> -	off %= sizeof(struct bpt);
> -	if (off != offsetof(struct bpt, instr[0])
> -	    && off != offsetof(struct bpt, instr[1]))
> +	bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> +	if (bp_off != 0 && bp_off != 4)
>  		return NULL;
> -	*offp = off - offsetof(struct bpt, instr[0]);
> -	return (struct bpt *) (nip - off);
> +	*offp = bp_off;
> +	return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
>  }
>  
>  static struct bpt *new_breakpoint(unsigned long a)
> @@ -876,7 +879,8 @@ 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;
> -			patch_instruction(&bp->instr[1], bpinstr);
> +			bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
> +			patch_instruction(bp->instr + 1, bpinstr);
>  			return bp;
>  		}
>  	}
> -- 
> 2.17.1
> 
>
Jordan Niethe March 23, 2020, 9:26 a.m. UTC | #4
On Mon, Mar 23, 2020 at 5:05 PM Balamuruhan S <bala24@linux.ibm.com> wrote:
>
> On Fri, 2020-03-20 at 16:17 +1100, Jordan Niethe wrote:
> > To execute an instruction out of line after a breakpoint, the NIP is
> > set
> > to the address of struct bpt::instr. Here a copy of the instruction
> > that
> > was replaced with a breakpoint is kept, along with a trap so normal
> > flow
> > can be resumed after XOLing. The struct bpt's are located within the
> > data section. This is problematic as the data section may be marked
> > as
> > no execute.
> >
> > Instead of each struct bpt holding the instructions to be XOL'd, make
> > a
> > new array, bpt_table[], with enough space to hold instructions for
> > the
> > number of supported breakpoints. Place this array in the text
> > section.
> > Make struct bpt::instr a pointer to the instructions in bpt_table[]
> > associated with that breakpoint. This association is a simple
> > mapping:
> > bpts[n] -> bpt_table[n * words per breakpoint].
>
> Can we have it in separate commits ?
>         * introduce the array bpt_table[] and make struct bpt::instr a
>                 pointer to the instructions in bpt_table[].
>         * place the array in text section.
Yeah I can split it if that would be clearer.
>
> > Currently we only need
> > the copied instruction followed by a trap, so 2 words per breakpoint.
> >
> > Signed-off-by: Jordan Niethe <jniethe5@gmail.com>
> > ---
> > v4: New to series
> > ---
> >  arch/powerpc/kernel/vmlinux.lds.S |  2 +-
> >  arch/powerpc/xmon/xmon.c          | 22 +++++++++++++---------
> >  2 files changed, 14 insertions(+), 10 deletions(-)
> >
> > diff --git a/arch/powerpc/kernel/vmlinux.lds.S
> > b/arch/powerpc/kernel/vmlinux.lds.S
> > index b4c89a1acebb..e90845b8c300 100644
> > --- a/arch/powerpc/kernel/vmlinux.lds.S
> > +++ b/arch/powerpc/kernel/vmlinux.lds.S
> > @@ -86,7 +86,7 @@ SECTIONS
> >               ALIGN_FUNCTION();
> >  #endif
> >               /* careful! __ftr_alt_* sections need to be close to
> > .text */
> > -             *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> > __ftr_alt_* .ref.text);
> > +             *(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup
> > __ftr_alt_* .ref.text .text.xmon_bpts);
> >  #ifdef CONFIG_PPC64
> >               *(.tramp.ftrace.text);
> >  #endif
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index 02e3bd62cab4..7875d1a37770 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -97,7 +97,7 @@ static long *xmon_fault_jmp[NR_CPUS];
> >  /* Breakpoint stuff */
> >  struct bpt {
> >       unsigned long   address;
> > -     unsigned int    instr[2];
> > +     unsigned int    *instr;
> >       atomic_t        ref_count;
> >       int             enabled;
> >       unsigned long   pad;
> > @@ -109,6 +109,7 @@ struct bpt {
> >  #define BP_DABR              4
> >
> >  #define NBPTS        256
> > +#define BPT_WORDS    2
> >  static struct bpt bpts[NBPTS];
> >  static struct bpt dabr;
> >  static struct bpt *iabr;
> > @@ -116,6 +117,8 @@ static unsigned bpinstr = 0x7fe00008;     /* trap
> > */
> >
> >  #define BP_NUM(bp)   ((bp) - bpts + 1)
> >
> > +static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS *
> > BPT_WORDS];
> > +
> >  /* Prototypes */
> >  static int cmds(struct pt_regs *);
> >  static int mread(unsigned long, void *, int);
> > @@ -852,16 +855,16 @@ static struct bpt *at_breakpoint(unsigned long
> > pc)
> >  static struct bpt *in_breakpoint_table(unsigned long nip, unsigned
> > long *offp)
> >  {
> >       unsigned long off;
> > +     unsigned long bp_off;
> >
> > -     off = nip - (unsigned long) bpts;
> > -     if (off >= sizeof(bpts))
> > +     off = nip - (unsigned long) bpt_table;
> > +     if (off >= sizeof(bpt_table))
> >               return NULL;
> > -     off %= sizeof(struct bpt);
> > -     if (off != offsetof(struct bpt, instr[0])
> > -         && off != offsetof(struct bpt, instr[1]))
> > +     bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
> > +     if (bp_off != 0 && bp_off != 4)
> >               return NULL;
> > -     *offp = off - offsetof(struct bpt, instr[0]);
> > -     return (struct bpt *) (nip - off);
> > +     *offp = bp_off;
> > +     return bpts + ((off - bp_off) / (sizeof(unsigned int) *
> > BPT_WORDS));
>
> `(off - bp_off) / (sizeof(unsigned int) * BPT_WORDS)` seems to be the
> actual breakpoint offset. Can we have something like,
>
> #define NBPTS  256
> #define BPT_WORDS      2
> #define BPT_WORDS_SIZE (sizeof(unsigned int) * BPT_WORDS)
> #define BPT_OFFSET(off, bp_word_off) ((off - bp_word_off) / \                                   BPT_WORDS_SIZE)
> ;
> :::
> :::
> :::
> bp_word_off = off % BPT_WORDS_SIZE;
> if (bp_word_off != 0 && bp_word_off != 4)
>         return NULL;
> *offp = bp_word_off;
> return bpts + BPT_OFFSET(off, bp_word_off);
I do agree this is not very clear int terms of the calculations I
don't really want to introduce some macros to be used just once. I
will try to think of a way to make the calculations look more clear.
>
> -- Bala
>
diff mbox series

Patch

diff --git a/arch/powerpc/kernel/vmlinux.lds.S b/arch/powerpc/kernel/vmlinux.lds.S
index b4c89a1acebb..e90845b8c300 100644
--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -86,7 +86,7 @@  SECTIONS
 		ALIGN_FUNCTION();
 #endif
 		/* careful! __ftr_alt_* sections need to be close to .text */
-		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text);
+		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text .text.xmon_bpts);
 #ifdef CONFIG_PPC64
 		*(.tramp.ftrace.text);
 #endif
diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index 02e3bd62cab4..7875d1a37770 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -97,7 +97,7 @@  static long *xmon_fault_jmp[NR_CPUS];
 /* Breakpoint stuff */
 struct bpt {
 	unsigned long	address;
-	unsigned int	instr[2];
+	unsigned int	*instr;
 	atomic_t	ref_count;
 	int		enabled;
 	unsigned long	pad;
@@ -109,6 +109,7 @@  struct bpt {
 #define BP_DABR		4
 
 #define NBPTS	256
+#define BPT_WORDS	2
 static struct bpt bpts[NBPTS];
 static struct bpt dabr;
 static struct bpt *iabr;
@@ -116,6 +117,8 @@  static unsigned bpinstr = 0x7fe00008;	/* trap */
 
 #define BP_NUM(bp)	((bp) - bpts + 1)
 
+static unsigned int __section(.text.xmon_bpts) bpt_table[NBPTS * BPT_WORDS];
+
 /* Prototypes */
 static int cmds(struct pt_regs *);
 static int mread(unsigned long, void *, int);
@@ -852,16 +855,16 @@  static struct bpt *at_breakpoint(unsigned long pc)
 static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
 {
 	unsigned long off;
+	unsigned long bp_off;
 
-	off = nip - (unsigned long) bpts;
-	if (off >= sizeof(bpts))
+	off = nip - (unsigned long) bpt_table;
+	if (off >= sizeof(bpt_table))
 		return NULL;
-	off %= sizeof(struct bpt);
-	if (off != offsetof(struct bpt, instr[0])
-	    && off != offsetof(struct bpt, instr[1]))
+	bp_off = off % (sizeof(unsigned int) * BPT_WORDS);
+	if (bp_off != 0 && bp_off != 4)
 		return NULL;
-	*offp = off - offsetof(struct bpt, instr[0]);
-	return (struct bpt *) (nip - off);
+	*offp = bp_off;
+	return bpts + ((off - bp_off) / (sizeof(unsigned int) * BPT_WORDS));
 }
 
 static struct bpt *new_breakpoint(unsigned long a)
@@ -876,7 +879,8 @@  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;
-			patch_instruction(&bp->instr[1], bpinstr);
+			bp->instr = bpt_table + ((bp - bpts) * BPT_WORDS);
+			patch_instruction(bp->instr + 1, bpinstr);
 			return bp;
 		}
 	}