platform: set up syscall trap on SMP application processors#2370
platform: set up syscall trap on SMP application processors#2370uadhran wants to merge 2 commits into
Conversation
Pull syscall MSR setup into init_syscall_trap() and call it from both the BSP libc init path and revenant_main so AP cores get a handler instead of nothing. Related: includeos#2358
MSR setup for the syscall entry point lives next to __syscall_entry and syscall_entry.cpp. Platform code only calls x86::init_syscall_trap().
|
Moved |
| @@ -0,0 +1,5 @@ | |||
| #pragma once | |||
|
|
|||
| namespace x86 { | |||
There was a problem hiding this comment.
Do we want this under x86 only? I think syscalls can exist on any architecture, and thus it would make sense to declare this for all architectures, and require architectures for which it makes no sense to initialize syscalls to stub an empty implementation.
Maybe @elstr-512 has an opinion on this?
| #warning Classical syscall interface missing for 32-bit | ||
| #endif | ||
| KDEBUG("* Initialize syscall trap\n"); | ||
| x86::init_syscall_trap(); |
There was a problem hiding this comment.
I'm still wondering if this should be part of libc initialization. I wouldn't do such a change in this PR fwiw, but I would consider opening an issue to discuss it further, maybe.
|
This is much cleaner than what it used to be, thanks! All integration tests are passing too. I kinda wish we could add some integration tests to verify syscalls fail to trap before they're initialized, and make sure they're trapped after initialization. Otherwise, see inlined comments. |
Only CPU 0 had the syscall MSRs configured. This pulls that setup into
init_syscall_trap()and calls it from both the BSP libc init path andrevenant_main, so application processors get a trap instead of nothing. The handler still panics — syscalls aren't implemented.Tested:
nix-build unittests.nix— 85/85 passed (Nix 2.34.7, Linux).Split from #2367 per review feedback. Happy to move
init_syscall_trap()undersrc/arch/x86_64/in a follow-up if preferred before merge.Related: #2358