Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{Bp-13444} arm: g_current_regs is only used to determine if we are in irq #13972

Merged
merged 4 commits into from
Oct 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 7 additions & 3 deletions arch/arm/src/arm/arm_doirq.c
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,8 @@

uint32_t *arm_doirq(int irq, uint32_t *regs)
{
struct tcb_s *tcb = this_task();

board_autoled_on(LED_INIRQ);
#ifdef CONFIG_SUPPRESS_INTERRUPTS
PANIC();
Expand All @@ -71,6 +73,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
*/

up_set_current_regs(regs);
tcb->xcp.regs = regs;

/* Acknowledge the interrupt */

Expand All @@ -79,6 +82,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
/* Deliver the IRQ */

irq_dispatch(irq, regs);
tcb = this_task();

/* Check for a context switch. If a context switch occurred, then
* current_regs will have a different value than it did on entry. If an
Expand All @@ -87,7 +91,7 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
* returning from the interrupt.
*/

if (regs != up_current_regs())
if (regs != tcb->xcp.regs)
{
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously
Expand All @@ -104,9 +108,9 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)
* crashes.
*/

g_running_tasks[this_cpu()] = this_task();
g_running_tasks[this_cpu()] = tcb;

regs = up_current_regs();
regs = tcb->xcp.regs;
}

/* Set current_regs to NULL to indicate that we are no longer in an
Expand Down
67 changes: 6 additions & 61 deletions arch/arm/src/arm/arm_schedulesigaction.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,70 +89,15 @@ void up_schedule_sigaction(struct tcb_s *tcb, sig_deliver_t sigdeliver)
* being delivered to the currently executing task.
*/

sinfo("rtcb=%p current_regs=%p\n", this_task(), up_current_regs());
sinfo("rtcb=%p current_regs=%p\n", this_task(),
this_task()->xcp.regs);

if (tcb == this_task())
if (tcb == this_task() && !up_interrupt_context())
{
/* CASE 1: We are not in an interrupt handler and
* a task is signalling itself for some reason.
*/

if (!up_current_regs())
{
/* In this case just deliver the signal now. */

sigdeliver(tcb);
tcb->xcp.sigdeliver = NULL;
}

/* CASE 2: We are in an interrupt handler AND the
* interrupted task is the same as the one that
* must receive the signal, then we will have to modify
* the return state as well as the state in the TCB.
*
* Hmmm... there looks like a latent bug here: The following
* logic would fail in the strange case where we are in an
* interrupt handler, the thread is signalling itself, but
* a context switch to another task has occurred so that
* current_regs does not refer to the thread of this_task()!
*/

else
{
/* Save the return lr and cpsr and one scratch register
* These will be restored by the signal trampoline after
* the signals have been delivered.
*/

/* And make sure that the saved context in the TCB
* is the same as the interrupt return context.
*/

arm_savestate(tcb->xcp.saved_regs);
/* In this case just deliver the signal now. */

/* Duplicate the register context. These will be
* restored by the signal trampoline after the signal has been
* delivered.
*/

up_set_current_regs(up_current_regs() - XCPTCONTEXT_REGS);
memcpy(up_current_regs(), tcb->xcp.saved_regs,
XCPTCONTEXT_SIZE);

up_current_regs()[REG_SP] = (uint32_t)(up_current_regs() +
XCPTCONTEXT_REGS);

/* Then set up to vector to the trampoline with interrupts
* disabled
*/

up_current_regs()[REG_PC] = (uint32_t)arm_sigdeliver;
up_current_regs()[REG_CPSR] = PSR_MODE_SYS | PSR_I_BIT |
PSR_F_BIT;
#ifdef CONFIG_ARM_THUMB
up_current_regs()[REG_CPSR] |= PSR_T_BIT;
#endif
}
sigdeliver(tcb);
tcb->xcp.sigdeliver = NULL;
}

/* Otherwise, we are (1) signaling a task is not running
Expand Down
22 changes: 6 additions & 16 deletions arch/arm/src/arm/arm_syscall.c
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@

uint32_t *arm_syscall(uint32_t *regs)
{
struct tcb_s *tcb;
struct tcb_s *tcb = this_task();
uint32_t cmd;
int cpu;

Expand All @@ -66,6 +66,7 @@ uint32_t *arm_syscall(uint32_t *regs)
* current_regs is also used to manage interrupt level context switches.
*/

tcb->xcp.regs = regs;
up_set_current_regs(regs);

/* The SYSCALL command is in R0 on entry. Parameters follow in R1..R7 */
Expand Down Expand Up @@ -94,7 +95,7 @@ uint32_t *arm_syscall(uint32_t *regs)
* set will determine the restored context.
*/

up_set_current_regs((uint32_t *)regs[REG_R1]);
tcb->xcp.regs = (uint32_t *)regs[REG_R1];
DEBUGASSERT(up_current_regs());
}
break;
Expand Down Expand Up @@ -133,29 +134,18 @@ uint32_t *arm_syscall(uint32_t *regs)
break;
}

#ifdef CONFIG_ARCH_ADDRENV
/* Check for a context switch. If a context switch occurred, then
* current_regs will have a different value than it did on entry. If an
* interrupt level context switch has occurred, then establish the correct
* address environment before returning from the interrupt.
*/

if (regs != up_current_regs())
if (regs != tcb->xcp.regs)
{
#ifdef CONFIG_ARCH_ADDRENV
/* Make sure that the address environment for the previously
* running task is closed down gracefully (data caches dump,
* MMU flushed) and set up the address environment for the new
* thread at the head of the ready-to-run list.
*/

addrenv_switch(NULL);
}
#endif

/* Restore the cpu lock */

if (regs != up_current_regs())
{
/* Record the new "running" task. g_running_tasks[] is only used by
* assertion logic for reporting crashes.
*/
Expand All @@ -181,5 +171,5 @@ uint32_t *arm_syscall(uint32_t *regs)
* SYS_context_switch system call.
*/

return regs;
return tcb->xcp.regs;
}
11 changes: 7 additions & 4 deletions arch/arm/src/armv6-m/arm_doirq.c
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,16 @@

uint32_t *arm_doirq(int irq, uint32_t *regs)
{
struct tcb_s *tcb = this_task();

board_autoled_on(LED_INIRQ);
#ifdef CONFIG_SUPPRESS_INTERRUPTS
PANIC();
#else

if (regs[REG_EXC_RETURN] & EXC_RETURN_THREAD_MODE)
{
tcb->xcp.regs = regs;
up_set_current_regs(regs);
}

Expand All @@ -68,17 +71,17 @@ uint32_t *arm_doirq(int irq, uint32_t *regs)

if (regs[REG_EXC_RETURN] & EXC_RETURN_THREAD_MODE)
{
/* Restore the cpu lock */
tcb = this_task();

if (regs != up_current_regs())
if (regs != tcb->xcp.regs)
{
/* Record the new "running" task when context switch occurred.
* g_running_tasks[] is only used by assertion logic for reporting
* crashes.
*/

g_running_tasks[this_cpu()] = this_task();
regs = up_current_regs();
g_running_tasks[this_cpu()] = tcb;
regs = tcb->xcp.regs;
}

/* Update the current_regs to NULL. */
Expand Down
Loading
Loading