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.

Sign up for GitHub

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

arch/risc-v: Implement up_this_task using Thread Pointer (TP) #15563

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
+41 −2
Draft

arch/risc-v: Implement up_this_task using Thread Pointer (TP) #15563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
28 changes: 28 additions & 0 deletions arch/risc-v/include/irq.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -749,6 +749,34 @@ int up_this_cpu(void);
* Inline Functions
****************************************************************************/

/****************************************************************************
* Schedule acceleration macros
****************************************************************************/

/* When thread local storage is disabled (the usual case), the TP
* (Thread Pointer) register is available for use as a general purpose
* register. We use it to store the current task's TCB pointer for quick
* access.
* Note: If you want to use TLS (CONFIG_SCHED_THREAD_LOCAL), your toolchain
* must be compiled with --enable-tls option to properly support
* thread-local storage.
*/

#ifndef CONFIG_SCHED_THREAD_LOCAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could use TP when system calls are enabled as well, even when SCHED_THREAD_LOCAL is in use.

The current logic in exception_common just needs to be modified to swap user TP with kernel TP (from the scratch area) always (not just when a system call is taken).

In flat mode where the kernel symbols are directly visible/callable this obviously does not work.

#define up_this_task() \
({ \
struct tcb_s* t; \
__asm__ __volatile__("mv %0, tp" : "=r"(t)); \
t; \
})

/* Update the current task pointer stored in TP register */
#define up_update_task(t) \
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should call this when a context switch occurs.

{ \
__asm__ __volatile__("mv tp, %0" : : "r"(t)); \
}
#endif /* CONFIG_SCHED_THREAD_LOCAL */

/****************************************************************************
* Name: up_irq_save
*
Expand Down
9 changes: 7 additions & 2 deletions arch/risc-v/src/common/riscv_cpustart.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
#include <nuttx/arch.h>
#include <nuttx/spinlock.h>
#include <nuttx/sched_note.h>
#include <arch/irq.h>

#include "sched/sched.h"
#include "init/init.h"
Expand Down Expand Up @@ -69,6 +70,8 @@

void riscv_cpu_boot(int cpu)
{
struct tcb_s *tcb;

/* Clear IPI for CPU(cpu) */

riscv_ipi_clear(cpu);
Expand Down Expand Up @@ -100,9 +103,9 @@ void riscv_cpu_boot(int cpu)

sinfo("CPU%d Started\n", this_cpu());

#ifdef CONFIG_STACK_COLORATION
struct tcb_s *tcb = this_task();
tcb = current_task(this_cpu());

#ifdef CONFIG_STACK_COLORATION
/* If stack debug is enabled, then fill the stack with a
* recognizable value that we can use later to test for high
* water marks.
Expand All @@ -111,6 +114,8 @@ void riscv_cpu_boot(int cpu)
riscv_stack_color(tcb->stack_alloc_ptr, 0);
#endif

up_update_task(tcb);

/* TODO: Setup FPU */

/* Clear machine software interrupt for CPU(cpu) */
Expand Down
2 changes: 2 additions & 0 deletions arch/risc-v/src/common/riscv_initialstate.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,8 @@ void up_initial_state(struct tcb_s *tcb)
#ifdef CONFIG_SCHED_THREAD_LOCAL
xcp->regs[REG_TP] = (uintptr_t)tcb->stack_alloc_ptr +
sizeof(struct tls_info_s);
#else
xcp->regs[REG_TP] = (uintptr_t)tcb;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not necessary, just set tcb -> tp directly when a context switch occurs.

#endif

/* Set the initial value of the interrupt context register.
Expand Down
4 changes: 4 additions & 0 deletions arch/risc-v/src/common/riscv_macros.S
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@
#ifdef RISCV_SAVE_GP
REGSTORE x3, REG_X3(\in) /* gp */
#endif
#ifdef CONFIG_SCHED_THREAD_LOCAL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unnecessary as well, TP will remain stable when the user process is running, as well as during a trap. Only time when TP needs to be updated is when a context switch occurs.

REGSTORE x4, REG_X4(\in) /* tp */
#endif
REGSTORE x5, REG_X5(\in) /* t0 */
REGSTORE x6, REG_X6(\in) /* t1 */
REGSTORE x7, REG_X7(\in) /* t2 */
Expand Down Expand Up @@ -201,7 +203,9 @@
#ifdef RISCV_SAVE_GP
REGLOAD x3, REG_X3(\out) /* gp */
#endif
#ifdef CONFIG_SCHED_THREAD_LOCAL
REGLOAD x4, REG_X4(\out) /* tp */
#endif
REGLOAD x5, REG_X5(\out) /* t0 */
REGLOAD x6, REG_X6(\out) /* t1 */
REGLOAD x7, REG_X7(\out) /* t2 */
Expand Down
Loading