Re: [PATCH V2 2/4] powerpc: add helper to check if offset is within rel branch range

[Anju T Sudhakar]

Hi Masami,


Thank you for reviewing the patch set.


On Friday 16 December 2016 05:22 PM, Masami Hiramatsu wrote:

On Wed, 14 Dec 2016 21:48:30 +0530
Anju T Sudhakar <a...@linux.vnet.ibm.com> wrote:

From: "Naveen N. Rao" <naveen.n....@linux.vnet.ibm.com>

The coding is OK to me. Please add a description for this patch
here, e.g. what is done by this patch, what kind of branch
instruction will be covered, and why thse checks are needed etc.

Sure. I will give a description for this patch.


Thanks and Regards,

-Anju

Thank you,

Signed-off-by: Naveen N. Rao <naveen.n....@linux.vnet.ibm.com>
Signed-off-by: Anju T Sudhakar <a...@linux.vnet.ibm.com>
---
  arch/powerpc/include/asm/code-patching.h |  1 +
  arch/powerpc/lib/code-patching.c         | 24 +++++++++++++++++++++++-
  2 files changed, 24 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/code-patching.h 
b/arch/powerpc/include/asm/code-patching.h
index 2015b07..75ee4f4 100644
--- a/arch/powerpc/include/asm/code-patching.h
+++ b/arch/powerpc/include/asm/code-patching.h
@@ -22,6 +22,7 @@
  #define BRANCH_SET_LINK       0x1
  #define BRANCH_ABSOLUTE       0x2

+bool is_offset_in_branch_range(long offset);

  unsigned int create_branch(const unsigned int *addr,
                           unsigned long target, int flags);
  unsigned int create_cond_branch(const unsigned int *addr,
diff --git a/arch/powerpc/lib/code-patching.c b/arch/powerpc/lib/code-patching.c
index d5edbeb..f643451 100644
--- a/arch/powerpc/lib/code-patching.c
+++ b/arch/powerpc/lib/code-patching.c
@@ -32,6 +32,28 @@ int patch_branch(unsigned int *addr, unsigned long target, 
int flags)
        return patch_instruction(addr, create_branch(addr, target, flags));
  }

+bool is_offset_in_branch_range(long offset)

+{
+       /*
+        * Powerpc branch instruction is :
+        *
+        *  0         6                 30   31
+        *  +---------+----------------+---+---+
+        *  | opcode  |     LI         |AA |LK |
+        *  +---------+----------------+---+---+
+        *  Where AA = 0 and LK = 0
+        *
+        * LI is a signed 24 bits integer. The real branch offset is computed
+        * by: imm32 = SignExtend(LI:'0b00', 32);
+        *
+        * So the maximum forward branch should be:
+        *   (0x007fffff << 2) = 0x01fffffc =  0x1fffffc
+        * The maximum backward branch should be:
+        *   (0xff800000 << 2) = 0xfe000000 = -0x2000000
+        */
+       return (offset >= -0x2000000 && offset <= 0x1fffffc && !(offset & 0x3));
+}
+
  unsigned int create_branch(const unsigned int *addr,
                           unsigned long target, int flags)
  {
@@ -43,7 +65,7 @@ unsigned int create_branch(const unsigned int *addr,
                offset = offset - (unsigned long)addr;

/* Check we can represent the target in the instruction format */

-       if (offset < -0x2000000 || offset > 0x1fffffc || offset & 0x3)
+       if (!is_offset_in_branch_range(offset))
                return 0;

/* Mask out the flags and target, so they don't step on each other. */

--
2.7.4