Skip to content

platform: set up syscall trap on SMP application processors#2370

Open
uadhran wants to merge 2 commits into
includeos:mainfrom
uadhran:fix/smp-syscall-trap
Open

platform: set up syscall trap on SMP application processors#2370
uadhran wants to merge 2 commits into
includeos:mainfrom
uadhran:fix/smp-syscall-trap

Conversation

@uadhran

@uadhran uadhran commented Jul 7, 2026

Copy link
Copy Markdown

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 and revenant_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() under src/arch/x86_64/ in a follow-up if preferred before merge.

Related: #2358

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().
@uadhran

uadhran commented Jul 7, 2026

Copy link
Copy Markdown
Author

Moved init_syscall_trap() into src/arch/x86_64/syscall_trap.cpp with declaration in api/arch/x86/syscall.hpp, per review. Platform code only calls it now.

Comment thread api/arch/x86/syscall.hpp
@@ -0,0 +1,5 @@
#pragma once

namespace x86 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

@mazunki

mazunki commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

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.

@mazunki

mazunki commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Btw, would merging this resolve #2358 (cc @torgeiru)? If so, you can add Closes: #2358 to the PR :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants