diff mbox

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

Message ID 1481732310-7779-5-git-send-email-anju@linux.vnet.ibm.com (mailing list archive)
State Superseded
Headers show

Commit Message

Anju T Sudhakar Dec. 14, 2016, 4:18 p.m. UTC 
From: "Naveen N. Rao" <naveen.n.rao@linux.vnet.ibm.com>

Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
Signed-off-by: Anju T Sudhakar <anju@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(-)

Comments

Masami Hiramatsu (Google) Dec. 16, 2016, 11:52 a.m. UTC | #1 
On Wed, 14 Dec 2016 21:48:30 +0530
Anju T Sudhakar <anju@linux.vnet.ibm.com> wrote:

> From: "Naveen N. Rao" <naveen.n.rao@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.

Thank you,

> Signed-off-by: Naveen N. Rao <naveen.n.rao@linux.vnet.ibm.com>
> Signed-off-by: Anju T Sudhakar <anju@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
>
Anju T Sudhakar Dec. 16, 2016, 4:31 p.m. UTC | #2 
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 <anju@linux.vnet.ibm.com> wrote:
>
>> From: "Naveen N. Rao" <naveen.n.rao@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.rao@linux.vnet.ibm.com>
>> Signed-off-by: Anju T Sudhakar <anju@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
>>
>
diff mbox

Patch

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. */