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  wrote:


From: "Naveen N. Rao" 


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 
Signed-off-by: Anju T Sudhakar 
---
  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:
+*   (0x007f << 2) = 0x01fc =  0x1fc
+* The maximum backward branch should be:
+*   (0xff80 << 2) = 0xfe00 = -0x200
+*/
+   return (offset >= -0x200 && offset <= 0x1fc && !(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 < -0x200 || offset > 0x1fc || 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

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

[Masami Hiramatsu]

On Wed, 14 Dec 2016 21:48:30 +0530
Anju T Sudhakar  wrote:

> From: "Naveen N. Rao" 
> 

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.

Thank you,

> Signed-off-by: Naveen N. Rao 
> Signed-off-by: Anju T Sudhakar 
> ---
>  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:
> +  *   (0x007f << 2) = 0x01fc =  0x1fc
> +  * The maximum backward branch should be:
> +  *   (0xff80 << 2) = 0xfe00 = -0x200
> +  */
> + return (offset >= -0x200 && offset <= 0x1fc && !(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 < -0x200 || offset > 0x1fc || 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
> 


-- 
Masami Hiramatsu 

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

[Anju T Sudhakar]

From: "Naveen N. Rao" 

Signed-off-by: Naveen N. Rao 
Signed-off-by: Anju T Sudhakar 
---
 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_LINK0x1
 #define BRANCH_ABSOLUTE0x2
 
+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:
+*   (0x007f << 2) = 0x01fc =  0x1fc
+* The maximum backward branch should be:
+*   (0xff80 << 2) = 0xfe00 = -0x200
+*/
+   return (offset >= -0x200 && offset <= 0x1fc && !(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 < -0x200 || offset > 0x1fc || 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