[PATCH v7 04/28] powerpc/xmon: Use bitwise calculations in_breakpoint_table()

[Jordan Niethe]

On Tue, May 5, 2020 at 5:08 PM Michael Ellerman <mpe at ellerman.id.au> wrote:
>
> Jordan Niethe <jniethe5 at gmail.com> writes:
> > A modulo operation is used for calculating the current offset from a
> > breakpoint within the breakpoint table. As instruction lengths are
> > always a power of 2, this can be replaced with a bitwise 'and'. The
> > current check for word alignment can be replaced with checking that the
> > lower 2 bits are not set.
> >
> > Suggested-by: Christophe Leroy <christophe.leroy at c-s.fr>
> > Signed-off-by: Jordan Niethe <jniethe5 at gmail.com>
> > ---
> > v6: New to series
> > ---
> >  arch/powerpc/xmon/xmon.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> > index bbfea22f4a96..e122f0c8a044 100644
> > --- a/arch/powerpc/xmon/xmon.c
> > +++ b/arch/powerpc/xmon/xmon.c
> > @@ -857,8 +857,8 @@ static struct bpt *in_breakpoint_table(unsigned long nip, unsigned long *offp)
> >       off = nip - (unsigned long) bpt_table;
> >       if (off >= sizeof(bpt_table))
> >               return NULL;
> > -     *offp = off % BPT_SIZE;
> > -     if (*offp != 0 && *offp != 4)
> > +     *offp = off & (BPT_SIZE - 1);
> > +     if (off & 3)
> >               return NULL;
>
> It would be even better if you didn't hard code the 3 wouldn't it?
>
The three is just checking word alignment, which I think was the
intention of the previous
-     if (*offp != 0 && *offp != 4)
But using BPT_SIZE is is a different calculation.
BPT_SIZE == 2 * sizeof(unsigned int) == 8
Which would mean the trap of the breakpoint pair of instructions would
return NULL.

> eg:
>
> +       *offp = off & (BPT_SIZE - 1);
> +       if (off & (BPT_SIZE - 1))
>                 return NULL;

>
> cheers