You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2022/03/28 16:43:22 UTC

[GitHub] [incubator-nuttx] anchao opened a new pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

anchao opened a new pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896


   ## Summary
   
   arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR
   
   Signed-off-by: chao.an <an...@xiaomi.com>
   
   ## Impact
   
   cortex-a/r real device
   
   ## Testing
   
   CI- check


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r837134026



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       > ldmia sp, {r0-r12,r15}^
   
   reglist of xcp context is different with restore order.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1082288029


   @masayuki2009 should we merge this PR without waiting fix the cache issue?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836794852



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       I will try to do some experiments to see if
   ```
   add r0, sp, #(4 * REG_R13)
   ldmia r0, {r13, r14}^
   ldmia sp, {r0-r12,r15}^
   ```
   will work. Just for my curiosity.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836812622



##########
File path: arch/arm/src/armv7-a/arm_vectors.S
##########
@@ -196,20 +196,33 @@ arm_vectorirq:
 	mov		sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+#ifdef CONFIG_ARMV7A_DECODEFIQ
+	mov		r4, #(PSR_MODE_IRQ | PSR_I_BIT | PSR_F_BIT)
+#else
+	mov		r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+#endif
+	msr		cpsr_c, r4			/* Switch back IRQ mode */
+
 	/* Upon return from arm_decodeirq, r0 holds the pointer to the register
 	 * state save area to use to restore the registers.  This may or may not
 	 * be the same value that was passed to arm_decodeirq:  It will differ if a
 	 * context switch is required.
 	 */
 
-	/* Restore the CPSR, SYS mode registers and return */
+	/* Restore the CPSR, IRQ mode registers and return */
 
 	ldr		r1, [r0, #(4*REG_CPSR)]		/* Fetch the return SPSR */
 	msr		spsr_cxsf, r1			/* Set the return mode SPSR */
 
-	/* Life is simple when everything is SYS mode */
+	/* Life is simple when everything is IRQ mode */
 
-	ldmia		r0, {r0-r15}^			/* Return */
+	mov		r14, r0				/* (IRQ) r14=Register storage area */
+	ldmia		r14!, {r0-r12}			/* Restore common r0-r12 */

Review comment:
       I think if `PSR_F_BIT` is set this will restore `r8-r12` to FIQ mode copy of the registers and we will loose values when we return to USER/SYS mode.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836815783



##########
File path: arch/arm/src/armv7-a/arm_vectors.S
##########
@@ -667,20 +732,33 @@ arm_vectorfiq:
 	mov		sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back FIQ mode and return with shadow SPSR */
+
+#ifdef CONFIG_ARMV7A_DECODEFIQ
+	mov		r4, #(PSR_MODE_FIQ | PSR_I_BIT | PSR_F_BIT)
+#else
+	mov		r4, #(PSR_MODE_FIQ | PSR_I_BIT)
+#endif
+	msr		cpsr_c, r4			/* Switch back FIQ mode */
+
 	/* Upon return from arm_decodefiq, r0 holds the pointer to the register
 	 * state save area to use to restore the registers.  This may or may not
 	 * be the same value that was passed to arm_decodefiq:  It will differ if a
 	 * context switch is required.
 	 */
 
-	/* Restore the CPSR, SYS mode registers and return */
+	/* Restore the CPSR, FIQ mode registers and return */
 
 	ldr		r1, [r0, #(4*REG_CPSR)]		/* Fetch the return SPSR */
 	msr		spsr_cxsf, r1			/* Set the return mode SPSR */
 
-	/* Life is simple when everything is SYS mode */
+	/* Life is simple when everything is FIQ mode */
 
-	ldmia		r0, {r0-r15}^			/* Return */
+	mov		r14, r0				/* (FIQ) r14=Register storage area */
+	ldmia		r14!, {r0-r12}			/* Restore common r0-r12 */

Review comment:
       I think that in `PSR_MODE_FIQ` this will restore `r8-r12` to FIQ copy of the registers and we will loose the data after returning back to USER/SYS mode.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836776646



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       As we discussed before, if r13/r14/r15 with suffix ^ are restored, LDM will not restore r13 and r14 to (USR/SYS) mode registers,
   
   ```
   ldmia   r14, {r13, r14, r15}^
   
   (gdb) i r
   sp             0x9f                0x9f
   lr             0x1082619c          276980124
   pc             0x10800230          0x10800230 <arm_vectorirq+112>
   cpsr           0x80000192          -2147483246
   
   (gdb) x/4x 0x1082619c
   0x1082619c:     0x108262b0      0x10802c63      0x10802c62      0x2000007f
                          SP           LR             PC             CPSR
   
   **   Restore of r13/r14 will fail with participation of PC (r15):    **
   
   (gdb) i r
   sp             0x10826168          0x10826168      <-wrong
   lr             0x10800cd3          276827347      <-wrong
   pc             0x10802c62          0x10802c62 <uart_write+188>
   cpsr           0x2000007f          536871039
   ```
   
   
   So we must to restore r13/r14 and r15 separately:
   
   
   ```
   ldmia   r14, {r13, r14}^    /* Restore user mode r13/r14 */
   add   r14, r14, #(4*2)    /* (IRQ) r14=address of r15 storage */
   ldmia   r14, {r15}^     /* Return */
   
   (gdb) i r
   sp             0x9f                0x9f
   lr             0x108261a4          276980132
   pc             0x10800238          0x10800238 <arm_vectorirq+120>
   cpsr           0x80000192          -2147483246
   
   
   (gdb) x/4x 0x108261a4-8
   0x1082619c:     0x108262b0      0x10802c63      0x10802c62      0x2000007f
   
   sp             0x108262b0          0x108262b0
   lr             0x10802c63          276835427
   pc             0x10802c62          0x10802c62 <uart_write+188>
   cpsr           0x2000007f          536871039
   ```
   
   
   
   
   
   ```
   ^
   is an optional suffix, available in ARM state only. You must not use it in User mode or System mode. It has the following purposes:
   
   If the instruction is LDM (with any addressing mode) and reglist contains the PC (R15), in addition to the normal multiple register transfer, the SPSR is copied into the CPSR. This is for returning from exception handlers. Use this only from exception modes.
   Otherwise, data is transferred into or out of the User mode registers instead of the current mode registers.
   ```
   
   
   https://developer.arm.com/documentation/dui0489/c/Cihcadda




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 merged pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
masayuki2009 merged pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081285656


   @anchao 
   
   Did you test this PR with your Cortex-A7 SMP board?
   I noticed that a deadlock happens with the NXP sabre-6quad board (quad Cortex-A9).
   
   ```
   NuttShell (NSH) NuttX-3.6.1
   nsh> uname -a
   NuttX 3.6.1 e5122937af Mar 29 2022 09:35:19 arm sabre-6quad
   nsh> ps
     PID GROUP CPU PRI POLICY   TYPE    NPX STATE    EVENT     SIGMASK   STACK   USED  FILLED COMMAND
       0     0   0   0 FIFO     Kthread N-- Assigned           00000000 001000 000440  44.0%  CPU0 IDLE
       1     1   1   0 FIFO     Kthread N-- Running            00000000 001000 000664  66.4%  CPU1 IDLE
       2     2   2   0 FIFO     Kthread N-- Running            00000000 001000 000664  66.4%  CPU2 IDLE
       3     3   3   0 FIFO     Kthread N-- Running            00000000 001000 000664  66.4%  CPU3 IDLE
       4     4 --- 192 RR       Kthread --- Waiting  Semaphore 00000000 002016 000396  19.6%  hpwork 0x1082155c
       5     5   0 100 RR       Task    --- Running            00000000 002024 001188  58.6%  nsh_main
   nsh> smp
     Main[0]: Running on CPU0
     Main[0]: Initializing barrier
   Thread[1]: Started
     Main[0]: Thread 1 created
   Thread[1]: Running on CPU0
     Main[0]: Now running on CPU1
   Thread[2]: Started
     Main[0]: Thread 2 created
   Thread[2]: Running on CPU1
     Main[0]: Now running on CPU2
   Thread[3]: Started
     Main[0]: Thread 3 created
   Thread[3]: Running on CPU2
   Thread[4]: Started
     Main[0]: Thread 4 created
   Thread[4]: Running on CPU3
     Main[0]: Now running on CPU3
   Thread[5]: Started
   Thread[5]: Running on CPU0
   Thread[1]: Now running on CPU2
     Main[0]: Thread 5 created
   Thread[3]: Now running on CPU3
   Thread[4]: Now running on CPU1
   Thread[6]: Started
   Thread[6]: Running on CPU0
   Thread[2]: Now running on CPU2
   Thread[5]: Now running on CPU3
     Main[0]: Thread 6 created
     Main[0]: Now running on CPU1
   Thread[1]: Now running on CPU1
   Thread[3]: Now running on CPU0
   Thread[4]: Now running on CPU2
   Thread[7]: Started
   Thread[7]: Running on CPU3
   Thread[6]: Now running on CPU1
   Thread[2]: Now running on CPU0
   Thread[5]: Now running on CPU2
     Main[0]: Thread 7 created
   ```
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081501622


   @zhuyanlin111 please take a look.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836640459



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       Back to my original question: why we can't `ldmia r14, {r13, r14, r15}^`instead of 3 lines?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] anchao commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081376292


   > Did you test this PR with your Cortex-A7 SMP board? I noticed that a deadlock happens with the NXP sabre-6quad board (quad Cortex-A9).
   
   Yes, I have verify this PR based on cortex-a7 (dual core) SMP board, The smp test can work normally.
   Could you share more information about this issue, or can the smp test work normally if you revert ?https://github.com/apache/incubator-nuttx/pull/5734


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836794852



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       I will try to do some experiments to see if
   ```
   add r0, sp, #(4*REG_R13)
   ldmia r0, {r13, r14}^
   ldmia sp, {r0-r12,r15}^
   ```
   will work. Just for my curiosity.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r837213096



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       yeah. the `r15` will be filled with a wrong value, true. missed that.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836786887



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       Got it. Thank you again for detailed explanation.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081461790


   
   >Yes, I have verify this PR based on cortex-a7 (dual core) SMP board, The smp test can work normally.
   >Could you share more information about this issue, or can the smp test work normally if you revert ?https://github.com/apache/incubator-nuttx/pull/5734
   
   @anchao 
   Finally, I found that https://github.com/apache/incubator-nuttx/pull/5650 caused the above deadlock.
   If I revert the PR, it works.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] pkarashchenko commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
pkarashchenko commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r836794852



##########
File path: arch/arm/src/arm/arm_vectors.S
##########
@@ -136,12 +136,23 @@ arm_vectorirq:
 	mov	sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back IRQ mode and return with shadow SPSR */
+
+	mov	r4, #(PSR_MODE_IRQ | PSR_I_BIT)
+	msr	cpsr_c, r4		/* Switch back IRQ mode */
+
 	/* Restore the CPSR, SYS mode registers and return */
 
 	ldr	r0, [sp, #(4*REG_CPSR)]	/* Fetch the return SPSR */
 	msr	spsr_cxsf, r0		/* Set the return mode SPSR */
 
-	ldmia	sp, {r0-r15}^		/* Return */
+	/* Life is simple when everything is IRQ mode */
+
+	mov	r14, sp			/* (IRQ) r14=Register storage area */
+	ldmia	r14!, {r0-r12}		/* Restore common r0-r12 */
+	ldmia	r14, {r13, r14}^	/* Restore user mode r13/r14 */
+	add	r14, r14, #(4*2)	/* (IRQ) r14=address of r15 storage */
+	ldmia	r14, {r15}^		/* Return */

Review comment:
       I will try to do some experiments to see if
   ```
   add r0, sp, #(4*REG_R13)
   ldmia r0!, {r13, r14}^
   ldmia sp, {r0-r12,r15}^
   ```
   will work. Just for my curiosity.




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081280170


   @anchao 
   I think this PR fixes a bug reported in https://github.com/apache/incubator-nuttx/pull/5734
   So please add comments on the bug fixes in the commit messages.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] xiaoxiang781216 edited a comment on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081501622


   @zhuyanlin111 please take a look. Look like the qemu is different from the real hardware:(.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] anchao commented on a change in pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
anchao commented on a change in pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#discussion_r837165892



##########
File path: arch/arm/src/armv7-a/arm_vectors.S
##########
@@ -667,20 +732,33 @@ arm_vectorfiq:
 	mov		sp, r4				/* Restore the possibly unaligned stack pointer */
 #endif
 
+	/* Switch back FIQ mode and return with shadow SPSR */
+
+#ifdef CONFIG_ARMV7A_DECODEFIQ
+	mov		r4, #(PSR_MODE_FIQ | PSR_I_BIT | PSR_F_BIT)
+#else
+	mov		r4, #(PSR_MODE_FIQ | PSR_I_BIT)
+#endif
+	msr		cpsr_c, r4			/* Switch back FIQ mode */
+
 	/* Upon return from arm_decodefiq, r0 holds the pointer to the register
 	 * state save area to use to restore the registers.  This may or may not
 	 * be the same value that was passed to arm_decodefiq:  It will differ if a
 	 * context switch is required.
 	 */
 
-	/* Restore the CPSR, SYS mode registers and return */
+	/* Restore the CPSR, FIQ mode registers and return */
 
 	ldr		r1, [r0, #(4*REG_CPSR)]		/* Fetch the return SPSR */
 	msr		spsr_cxsf, r1			/* Set the return mode SPSR */
 
-	/* Life is simple when everything is SYS mode */
+	/* Life is simple when everything is FIQ mode */
 
-	ldmia		r0, {r0-r15}^			/* Return */
+	mov		r14, r0				/* (FIQ) r14=Register storage area */
+	ldmia		r14!, {r0-r12}			/* Restore common r0-r12 */

Review comment:
       Done




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] anchao commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1081548928


   > @anchao I think this PR fixes a bug reported in #5734 So please add comments on the bug fixes in the commit messages.
   
   @masayuki2009 san, I have update the commit message, please review again


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [incubator-nuttx] masayuki2009 commented on pull request #5896: arm/cortex-[a|r]: IRQ Switch return should with shadow SPSR

Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #5896:
URL: https://github.com/apache/incubator-nuttx/pull/5896#issuecomment-1082456977


   >@masayuki2009 should we merge this PR without waiting fix the cache issue?
   
   Yes, we can merge this PR, because the issue only happens on the sabre-6quad board.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org