[PATCH] powerpc/fsl_book3e: fix the relocatable bug in debug interrupt handler

[PATCH] powerpc/fsl_book3e: fix the relocatable bug in debug interrupt handler

[Yuanjie Huang]

PowerPC Book3E processor features hardware-supported single instruction
execution, and it is used for ptrace(PTRACE_SINGLESTEP, ...). When a debugger
loads a debuggee, it typically sets the CPU to yield debug interrupt on first
instruction complete or branch taken. However, the newly-forked child process
could run into instruction TLB miss exception handler when switched to, and
causes a debug interrupt in the exception entry sequence. This is not expected
by caller of ptrace(PTRACE_SINGLESTEP, ...), so the next instruction address
saved in DSRR0 is checked against the boundary of exception entry sequence, to
ensure the kernel only process the interrupt as a normal exception if the
address does not fall in the exception entry sequence.  Failure in obtaining
the correct boundary leads to such debug exception handled as from privileged
mode, and causes kernel oops.

The LOAD_REG_IMMEDIATE can't be used to load the boundary addresses when
relocatable enabled, so this patch replace them with LOAD_REG_ADDR_PIC. LR is
backed up and restored before and after calling LOAD_REG_ADDR_PIC, because
LOAD_REG_ADDR_PIC clobbers it.

Signed-off-by: Yuanjie Huang <Yuanjie.Huang@windriver.com>
---
 arch/powerpc/kernel/exceptions-64e.S | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
index 3e68d1c..c475f569 100644
--- a/arch/powerpc/kernel/exceptions-64e.S
+++ b/arch/powerpc/kernel/exceptions-64e.S
@@ -735,12 +735,24 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
 	andis.	r15,r14,(DBSR_IC|DBSR_BT)@h
 	beq+	1f
 
+#ifdef CONFIG_RELOCATABLE
+	mflr	r14
+	LOAD_REG_ADDR_PIC(r15,interrupt_base_book3e)
+	mtlr	r14
+	cmpld	cr0,r10,r15
+	blt+	cr0,1f
+	LOAD_REG_ADDR_PIC(r15,interrupt_end_book3e)
+	mtlr	r14
+	cmpld	cr0,r10,r15
+	bge+	cr0,1f
+#else
 	LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e)
 	LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e)
 	cmpld	cr0,r10,r14
 	cmpld	cr1,r10,r15
 	blt+	cr0,1f
 	bge+	cr1,1f
+#endif
 
 	/* here it looks like we got an inappropriate debug exception. */
 	lis	r14,(DBSR_IC|DBSR_BT)@h		/* clear the event */
@@ -799,12 +811,24 @@ kernel_dbg_exc:
 	andis.	r15,r14,(DBSR_IC|DBSR_BT)@h
 	beq+	1f
 
+#ifdef CONFIG_RELOCATABLE
+	mflr	r14
+	LOAD_REG_ADDR_PIC(r15,interrupt_base_book3e)
+	mtlr	r14
+	cmpld	cr0,r10,r15
+	blt+	cr0,1f
+	LOAD_REG_ADDR_PIC(r15,interrupt_end_book3e)
+	mtlr	r14
+	cmpld	cr0,r10,r15
+	bge+	cr0,1f
+#else
 	LOAD_REG_IMMEDIATE(r14,interrupt_base_book3e)
 	LOAD_REG_IMMEDIATE(r15,interrupt_end_book3e)
 	cmpld	cr0,r10,r14
 	cmpld	cr1,r10,r15
 	blt+	cr0,1f
 	bge+	cr1,1f
+#endif
 
 	/* here it looks like we got an inappropriate debug exception. */
 	lis	r14,(DBSR_IC|DBSR_BT)@h		/* clear the event */
-- 
2.5.0

Re: powerpc/fsl_book3e: fix the relocatable bug in debug interrupt handler

[Scott Wood]

[Please wrap commit messages at around 74 columns]

On Fri, Aug 07, 2015 at 02:58:10PM +0800, Yuanjie Huang wrote:
> PowerPC Book3E processor features hardware-supported single instruction
> execution, and it is used for ptrace(PTRACE_SINGLESTEP, ...).  When a
> debugger loads a debuggee, it typically sets the CPU to yield debug
> interrupt on first instruction complete or branch taken.  However, the
> newly-forked child process could run into instruction TLB miss
> exception handler when switched to, and causes a debug interrupt in the
> exception entry sequence.  This is not expected by caller of
> ptrace(PTRACE_SINGLESTEP, ...), so the next instruction address saved
> in DSRR0 is checked against the boundary of exception entry sequence,
> to ensure the kernel only process the interrupt as a normal exception
> if the address does not fall in the exception entry sequence.  Failure
> in obtaining the correct boundary leads to such debug exception handled
> as from privileged mode, and causes kernel oops.
> 
> The LOAD_REG_IMMEDIATE can't be used to load the boundary addresses
> when relocatable enabled, so this patch replace them with
> LOAD_REG_ADDR_PIC.  LR is backed up and restored before and after
> calling LOAD_REG_ADDR_PIC, because LOAD_REG_ADDR_PIC clobbers it.
> 
> Signed-off-by: Yuanjie Huang <Yuanjie.Huang@windriver.com>
> ---
>  arch/powerpc/kernel/exceptions-64e.S | 24 ++++++++++++++++++++++++
>  1 file changed, 24 insertions(+)
> 
> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
> index 3e68d1c..c475f569 100644
> --- a/arch/powerpc/kernel/exceptions-64e.S
> +++ b/arch/powerpc/kernel/exceptions-64e.S
> @@ -735,12 +735,24 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
>  	andis.	r15,r14,(DBSR_IC|DBSR_BT)@h
>  	beq+	1f
>  
> +#ifdef CONFIG_RELOCATABLE
> +	mflr	r14
> +	LOAD_REG_ADDR_PIC(r15,interrupt_base_book3e)
> +	mtlr	r14
> +	cmpld	cr0,r10,r15
> +	blt+	cr0,1f
> +	LOAD_REG_ADDR_PIC(r15,interrupt_end_book3e)
> +	mtlr	r14
> +	cmpld	cr0,r10,r15
> +	bge+	cr0,1f
> +#else

CONFIG_RELOCATABLE is not supported on 64-bit book3e without applying
additional patches, such as the RFC patchset I posted recently that
contained the patch "powerpc/book3e-64: rename interrupt_end_book3e with
__end_interrupts".  But if you've applied that patchset, then you
wouldn't be working with the name interrupt_base_book3e, so how are you
seeing this?

Also, why not use the RELOCATABLE version unconditionally?  I don't think
this is a performance-critical path.

-Scott

Re: powerpc/fsl_book3e: fix the relocatable bug in debug interrupt handler

[Huang, Yuanjie]

Hi Scott,

On 08/08/2015 10:29 AM, Scott Wood wrote:
> [Please wrap commit messages at around 74 columns]
Ok, I will when sending a new version.
>
> On Fri, Aug 07, 2015 at 02:58:10PM +0800, Yuanjie Huang wrote:
>> PowerPC Book3E processor features hardware-supported single instruction
>> execution, and it is used for ptrace(PTRACE_SINGLESTEP, ...).  When a
>> debugger loads a debuggee, it typically sets the CPU to yield debug
>> interrupt on first instruction complete or branch taken.  However, the
>> newly-forked child process could run into instruction TLB miss
>> exception handler when switched to, and causes a debug interrupt in the
>> exception entry sequence.  This is not expected by caller of
>> ptrace(PTRACE_SINGLESTEP, ...), so the next instruction address saved
>> in DSRR0 is checked against the boundary of exception entry sequence,
>> to ensure the kernel only process the interrupt as a normal exception
>> if the address does not fall in the exception entry sequence.  Failure
>> in obtaining the correct boundary leads to such debug exception handled
>> as from privileged mode, and causes kernel oops.
>>
>> The LOAD_REG_IMMEDIATE can't be used to load the boundary addresses
>> when relocatable enabled, so this patch replace them with
>> LOAD_REG_ADDR_PIC.  LR is backed up and restored before and after
>> calling LOAD_REG_ADDR_PIC, because LOAD_REG_ADDR_PIC clobbers it.
>>
>> Signed-off-by: Yuanjie Huang <Yuanjie.Huang@windriver.com>
>> ---
>>   arch/powerpc/kernel/exceptions-64e.S | 24 ++++++++++++++++++++++++
>>   1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S
>> index 3e68d1c..c475f569 100644
>> --- a/arch/powerpc/kernel/exceptions-64e.S
>> +++ b/arch/powerpc/kernel/exceptions-64e.S
>> @@ -735,12 +735,24 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
>>   	andis.	r15,r14,(DBSR_IC|DBSR_BT)@h
>>   	beq+	1f
>>   
>> +#ifdef CONFIG_RELOCATABLE
>> +	mflr	r14
>> +	LOAD_REG_ADDR_PIC(r15,interrupt_base_book3e)
>> +	mtlr	r14
>> +	cmpld	cr0,r10,r15
>> +	blt+	cr0,1f
>> +	LOAD_REG_ADDR_PIC(r15,interrupt_end_book3e)
>> +	mtlr	r14
>> +	cmpld	cr0,r10,r15
>> +	bge+	cr0,1f
>> +#else
> CONFIG_RELOCATABLE is not supported on 64-bit book3e without applying
> additional patches, such as the RFC patchset I posted recently that
> contained the patch "powerpc/book3e-64: rename interrupt_end_book3e with
> __end_interrupts".  But if you've applied that patchset, then you
> wouldn't be working with the name interrupt_base_book3e, so how are you
> seeing this?

Actually I have merged additional patches submitted but not merged to 
make CONFIG_RELOCATABLE work with 64-bit book3e. I am happy to delay 
this until those patches are merged, and sent an adjusted version. Shall 
I wait until they are merged?

> Also, why not use the RELOCATABLE version unconditionally?  I don't think
> this is a performance-critical path.

The difference is 15 instructions against 14, if it's not important we 
can surely use only RELOCATABLE version.

Best,
Yuanjie

> -Scott

Re: powerpc/fsl_book3e: fix the relocatable bug in debug interrupt handler

[Scott Wood]

On Mon, 2015-08-10 at 10:23 +0800, Huang, Yuanjie wrote:
> Hi Scott,
> 
> On 08/08/2015 10:29 AM, Scott Wood wrote:
> > [Please wrap commit messages at around 74 columns]
> Ok, I will when sending a new version.
> > 
> > On Fri, Aug 07, 2015 at 02:58:10PM +0800, Yuanjie Huang wrote:
> > > PowerPC Book3E processor features hardware-supported single instruction
> > > execution, and it is used for ptrace(PTRACE_SINGLESTEP, ...).  When a
> > > debugger loads a debuggee, it typically sets the CPU to yield debug
> > > interrupt on first instruction complete or branch taken.  However, the
> > > newly-forked child process could run into instruction TLB miss
> > > exception handler when switched to, and causes a debug interrupt in the
> > > exception entry sequence.  This is not expected by caller of
> > > ptrace(PTRACE_SINGLESTEP, ...), so the next instruction address saved
> > > in DSRR0 is checked against the boundary of exception entry sequence,
> > > to ensure the kernel only process the interrupt as a normal exception
> > > if the address does not fall in the exception entry sequence.  Failure
> > > in obtaining the correct boundary leads to such debug exception handled
> > > as from privileged mode, and causes kernel oops.
> > > 
> > > The LOAD_REG_IMMEDIATE can't be used to load the boundary addresses
> > > when relocatable enabled, so this patch replace them with
> > > LOAD_REG_ADDR_PIC.  LR is backed up and restored before and after
> > > calling LOAD_REG_ADDR_PIC, because LOAD_REG_ADDR_PIC clobbers it.
> > > 
> > > Signed-off-by: Yuanjie Huang <Yuanjie.Huang@windriver.com>
> > > ---
> > >   arch/powerpc/kernel/exceptions-64e.S | 24 ++++++++++++++++++++++++
> > >   1 file changed, 24 insertions(+)
> > > 
> > > diff --git a/arch/powerpc/kernel/exceptions-64e.S 
> > > b/arch/powerpc/kernel/exceptions-64e.S
> > > index 3e68d1c..c475f569 100644
> > > --- a/arch/powerpc/kernel/exceptions-64e.S
> > > +++ b/arch/powerpc/kernel/exceptions-64e.S
> > > @@ -735,12 +735,24 @@ END_FTR_SECTION_IFSET(CPU_FTR_ALTIVEC)
> > >           andis.  r15,r14,(DBSR_IC|DBSR_BT)@h
> > >           beq+    1f
> > >   
> > > +#ifdef CONFIG_RELOCATABLE
> > > + mflr    r14
> > > + LOAD_REG_ADDR_PIC(r15,interrupt_base_book3e)
> > > + mtlr    r14
> > > + cmpld   cr0,r10,r15
> > > + blt+    cr0,1f
> > > + LOAD_REG_ADDR_PIC(r15,interrupt_end_book3e)
> > > + mtlr    r14
> > > + cmpld   cr0,r10,r15
> > > + bge+    cr0,1f
> > > +#else
> > CONFIG_RELOCATABLE is not supported on 64-bit book3e without applying
> > additional patches, such as the RFC patchset I posted recently that
> > contained the patch "powerpc/book3e-64: rename interrupt_end_book3e with
> > __end_interrupts".  But if you've applied that patchset, then you
> > wouldn't be working with the name interrupt_base_book3e, so how are you
> > seeing this?
> 
> Actually I have merged additional patches submitted but not merged to 
> make CONFIG_RELOCATABLE work with 64-bit book3e. I am happy to delay 
> this until those patches are merged, and sent an adjusted version. Shall 
> I wait until they are merged?

Yes, or base the patch on top of them and cite the dependency below the --- 
line in the changelog.

-Scott
>