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

On Tue, May 5, 2020 at 5:08 PM Michael Ellerman [off-list ref] wrote:

Jordan Niethe [off-list ref] writes:

quoted

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 <redacted>
Signed-off-by: Jordan Niethe <redacted>
---
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