casaroli commented on PR #20801:
URL: https://github.com/apache/nuttx/pull/20801#issuecomment-6266698119
Thank you for this PR! I tried it on master fc91db17fb7 with
`qemu-armv8a:knsh` (QEMU 11 with HVF, Arm GNU Toolchain 14.2.Rel1). Code on a
`makecontext()` stack works, and the ucontext tests pass. I found two things
that I think are worth a look:
**1. The kernel uses the user value of `TPIDR_EL0`.** `up_tls_info()` in
irq.h is defined for kernel code too, so `tls_get_info()` and `errno` in the
kernel read `TPIDR_EL0`. User code can write that register. The test below
points `TPIDR_EL0` to a copy of its TLS info and calls `open()` on a missing
file. The kernel then writes `ENOENT` into the copy:
```
x801kerr: open() -1, errno of the thread 0, copy changed at 12
x801kerr: kernel wrote errno through the user TPIDR_EL0 YES
```
So in a kernel or protected build, the kernel writes to an address that user
code selects. With `CONFIG_TLS_NCLEANUP > 0`, `tls_cleanup_push()` in the
kernel also writes a callback and its argument there. A possible fix: define
`up_tls_info()` only when `!defined(__KERNEL__)` (in arch/arch.h, as
include/nuttx/arch.h says), and keep the kernel `tls_get_info()` that reads the
TCB.
**2. A value that user code writes to `TPIDR_EL0` is lost.** The kernel
writes `stack_alloc_ptr` again at each context switch. The second test writes
the register, yields 20 times to another thread on the same CPU, and reads it
back:
```
x801tpw: old 0xc0006000 set 0xc0004020 read back 0xc0006000, helper ran 20
times
x801tpw: user value kept across context switches NO
```
This is fine for libc today, but native TLS needs a thread pointer that the
program sets, because each separately linked program has its own TLS block.
risc-v (`REG_TP`), xtensa (`REG_THREADPTR`) and x86_64 (`REG_FSBASE`) keep the
thread pointer in the context of each thread.
<details><summary>Test program x801kerr.c (kernel build program)</summary>
```c
#include <errno.h>
#include <fcntl.h>
#include <stdint.h>
#include <stdio.h>
#include <string.h>
static uint8_t g_copy[512] __attribute__((aligned(16)));
static uint8_t g_snap[512];
int main(int argc, char **argv)
{
uintptr_t old;
int changed = -1;
int fd;
int i;
errno = 0;
__asm__ volatile("mrs %0, tpidr_el0" : "=r"(old));
memcpy(g_copy, (void *)old, sizeof(g_copy));
memcpy(g_snap, g_copy, sizeof(g_copy));
__asm__ volatile("msr tpidr_el0, %0" : : "r"(g_copy) : "memory");
fd = open("/nonexistent/x801kerr", O_RDONLY);
__asm__ volatile("msr tpidr_el0, %0" : : "r"(old) : "memory");
for (i = 0; i < sizeof(g_copy); i++)
{
if (g_copy[i] != g_snap[i])
{
changed = i;
break;
}
}
printf("x801kerr: open() %d, errno of the thread %d, copy changed at %d\n",
fd, errno, changed);
printf("x801kerr: kernel wrote errno through the user TPIDR_EL0 %s\n",
changed >= 0 ? "YES" : "NO");
return 0;
}
```
</details>
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
