xiaoxiang781216 commented on a change in pull request #3626:
URL: https://github.com/apache/incubator-nuttx/pull/3626#discussion_r638428183
##########
File path: arch/arm/src/armv6-m/arm_svcall.c
##########
@@ -298,34 +298,65 @@ int arm_svcall(int irq, FAR void *context, FAR void *arg)
break;
#endif
+#if !defined(CONFIG_BUILD_FLAT) && !defined(CONFIG_DISABLE_PTHREAD)
+
/* R0=SYS_pthread_start: This a user pthread start
*
- * void up_pthread_start(pthread_startroutine_t entrypt,
- * pthread_addr_t arg) noreturn_function;
+ * void up_pthread_start(pthread_trampoline_t startup,
+ * pthread_startroutine_t entrypt, pthread_addr_t arg)
*
* At this point, the following values are saved in context:
*
* R0 = SYS_pthread_start
- * R1 = entrypt
- * R2 = arg
+ * R1 = startup
+ * R2 = entrypt
+ * R3 = arg
*/
-#if defined(CONFIG_BUILD_PROTECTED) && !defined(CONFIG_DISABLE_PTHREAD)
case SYS_pthread_start:
{
/* Set up to return to the user-space pthread start-up function in
* unprivileged mode.
*/
- regs[REG_PC] = (uint32_t)USERSPACE->pthread_startup;
+ regs[REG_PC] = (uint32_t)regs[REG_R1] & ~1; /* startup */
regs[REG_EXC_RETURN] = EXC_RETURN_UNPRIVTHR;
- /* Change the parameter ordering to match the expectation of struct
- * userpace_s pthread_startup:
+ /* Change the parameter ordering to match the expectation of the
+ * user space pthread_startup:
+ */
+
+ regs[REG_R0] = regs[REG_R2]; /* pthread entry */
+ regs[REG_R1] = regs[REG_R3]; /* arg */
+ }
+ break;
+
+ /* R0=SYS_pthread_exit: This pthread_exit call in user-space
+ *
+ * void up_pthread_exit(pthread_exitroutine_t exit,
+ * FAR void *exit_value)
+ *
+ * At this point, the following values are saved in context:
+ *
+ * R0 = SYS_pthread_exit
+ * R1 = pthread_exit trampoline routine
+ * R2 = exit_value
+ */
+
+ case SYS_pthread_exit:
+ {
+ /* Set up to return to the user-space pthread start-up function in
+ * unprivileged mode.
+ */
+
+ regs[REG_PC] = (uint32_t)regs[REG_R1] & ~1; /* startup */
Review comment:
> > should we keep the thumb bit?
>
> I don't think that the Thumb bit should be set. This is the saved value of
program counter and should not have bit 0 set. Bit 0 works only for call
instructions (and a few other places in the ISA). We need to double check this.
regs[REG_R1] isn't the saved program counter, but point to
start(pthread_trampoline_t) function which should always have bit0 set in
Cortex-M context.
>
> This is equivalent to the branch:
>
> ```
> mov Rx, PC
> ```
>
> This just jumps to the address in RX. This is the absolute address version
of the relative branch B instruction. The B and BL instructions and BX and BLX
instructions differ in that the BX and BLX instructions "interpret" bit 0 as
the Thumb indication. That thumb indication is NOT written to the PC. The BX
and BLX instructions use bit 0 to set the Thumb bit in the control/status
register. See
https://topic.alibabacloud.com/a/the-difference-between-the-assembly-jump-instruction-b-bl-bx-blx-and-bxj_8_8_10244895.html
. Bit 0 of the PC should never be set and never indicates Thumb mode.
>
Yes, Reading from PC should return an even address, but function pointer or
LR(by BL, BLX) will set bit0 for thumb more.
> I believe that setting bit 0 in the PC would cause an unaligned access
failure.
No, PC modificaiton instrution move the bit0 to EPSR T-bit:
https://stackoverflow.com/questions/18655916/arm-cortex-m0-m3-m4why-pc-is-always-even-number-in-thumb-state
From the datasheet, we need ensure bit0 of new PC always 1 to avoid LOOKUP.
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
