diff mbox

[RFC] ARM: kernel: irq: Simplify allocation of stack frame

Message ID 1381098649-21417-1-git-send-email-joelf@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Joel Fernandes Oct. 6, 2013, 10:30 p.m. UTC
On receiving IRQ exception in SVC mode, all the SVC mode registers are saved
onto the stack very early on.

The stack frame allocation code for IRQ entry during SVC mode (svc_entry) is
hard to read as 4-less is allocated initially only to be allocated later
implicity using the mov r3, [sp, #-4]! instruction. We make code easier to read
by allocating the 4 bytes on the stack frame in the beginning itself and remove
all instances where 4 bytes is adjusted.

Cc: Russell King <linux@arm.linux.org.uk>
Cc: Nicolas Pitre <nico@linaro.org>
Cc: Santosh Shilimkar <santosh.shilimkar@ti.com>
Signed-off-by: Joel Fernandes <joelf@ti.com>
---
 arch/arm/kernel/entry-armv.S | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Russell King - ARM Linux Oct. 6, 2013, 10:41 p.m. UTC | #1
On Sun, Oct 06, 2013 at 05:30:47PM -0500, Joel Fernandes wrote:
> On receiving IRQ exception in SVC mode, all the SVC mode registers are saved
> onto the stack very early on.
> 
> The stack frame allocation code for IRQ entry during SVC mode (svc_entry) is
> hard to read as 4-less is allocated initially only to be allocated later
> implicity using the mov r3, [sp, #-4]! instruction. We make code easier to read
> by allocating the 4 bytes on the stack frame in the beginning itself and remove
> all instances where 4 bytes is adjusted.

You omit to say that this results in saving one additional register
unnecessarily in the stmia.  We could use a stmib there instead which
would avoid that issue while keeping the rest of the change.
Joel Fernandes Oct. 7, 2013, 4:56 a.m. UTC | #2
On 10/06/2013 05:41 PM, Russell King - ARM Linux wrote:
> On Sun, Oct 06, 2013 at 05:30:47PM -0500, Joel Fernandes wrote:
>> On receiving IRQ exception in SVC mode, all the SVC mode registers are saved
>> onto the stack very early on.
>>
>> The stack frame allocation code for IRQ entry during SVC mode (svc_entry) is
>> hard to read as 4-less is allocated initially only to be allocated later
>> implicity using the mov r3, [sp, #-4]! instruction. We make code easier to read
>> by allocating the 4 bytes on the stack frame in the beginning itself and remove
>> all instances where 4 bytes is adjusted.
> 
> You omit to say that this results in saving one additional register

Ok, I will add this detail to the commit message.

> unnecessarily in the stmia.  We could use a stmib there instead which
> would avoid that issue while keeping the rest of the change.

But stmib is not available in THUMB mode, so this will break the THUMB builds.

usr_entry does something similar:
 ARM(   stmib   sp, {r1 - r12}  )
 THUMB( stmia   sp, {r0 - r12}  )

Let me know your suggestions about using ARM/THUMB macros or stmia for both
cases. Thanks.

Regards,

-Joel
Joel Fernandes Oct. 7, 2013, 4:28 p.m. UTC | #3
On 10/06/2013 05:41 PM, Russell King - ARM Linux wrote:
> On Sun, Oct 06, 2013 at 05:30:47PM -0500, Joel Fernandes wrote:
>> On receiving IRQ exception in SVC mode, all the SVC mode registers are saved
>> onto the stack very early on.
>>
>> The stack frame allocation code for IRQ entry during SVC mode (svc_entry) is
>> hard to read as 4-less is allocated initially only to be allocated later
>> implicity using the mov r3, [sp, #-4]! instruction. We make code easier to read
>> by allocating the 4 bytes on the stack frame in the beginning itself and remove
>> all instances where 4 bytes is adjusted.
> 
> You omit to say that this results in saving one additional register
> unnecessarily in the stmia.  We could use a stmib there instead which
> would avoid that issue while keeping the rest of the change.
> 

Hi Russel,

BTW I used ETM to check the number of cycles used to store the extra register
with and without this patch and with both cases it takes 7 cycles.

My platform uses Cortex-A8 (AM335x SoC).

Thanks,

-Joel
diff mbox

Patch

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 9cbe70c..fc5ac2a 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -149,7 +149,7 @@  ENDPROC(__und_invalid)
 	.macro	svc_entry, stack_hole=0
  UNWIND(.fnstart		)
  UNWIND(.save {r0 - pc}		)
-	sub	sp, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+	sub	sp, sp, #(S_FRAME_SIZE + \stack_hole)
 #ifdef CONFIG_THUMB2_KERNEL
  SPFIX(	str	r0, [sp]	)	@ temporarily saved
  SPFIX(	mov	r0, sp		)
@@ -159,14 +159,14 @@  ENDPROC(__und_invalid)
  SPFIX(	tst	sp, #4		)
 #endif
  SPFIX(	subeq	sp, sp, #4	)
-	stmia	sp, {r1 - r12}
+	stmia	sp, {r0 - r12}
 
 	ldmia	r0, {r3 - r5}
-	add	r7, sp, #S_SP - 4	@ here for interlock avoidance
+	add	r7, sp, #S_SP		@ here for interlock avoidance
 	mov	r6, #-1			@  ""  ""      ""       ""
-	add	r2, sp, #(S_FRAME_SIZE + \stack_hole - 4)
+	add	r2, sp, #(S_FRAME_SIZE + \stack_hole)
  SPFIX(	addeq	r2, r2, #4	)
-	str	r3, [sp, #-4]!		@ save the "real" r0 copied
+	str	r3, [sp]		@ save the "real" r0 copied
 					@ from the exception stack
 
 	mov	r3, lr