Skip to content

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

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

Open
uadhran wants to merge 2 commits into
base: main
Choose a base branch
from
+28 −12
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e7ef29b
platform: set up syscall trap on SMP application processors
uadhran Jul 7, 2026
536d3e1
arch: move init_syscall_trap into src/arch/x86_64
uadhran Jul 7, 2026
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
5 changes: 5 additions & 0 deletions api/arch/x86/syscall.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
#pragma once

namespace x86 {

@mazunki mazunki Jul 7, 2026

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?

@uadhran uadhran Jul 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fair question. I kept this under x86:: for this PR because the implementation is x86_64-specific (IA32_STAR / IA32_LSTAR) and IncludeOS only runs the syscall path on x86 PC today.

A broader shape would be something like os::init_syscall_trap() in a neutral header with arch stubs (no-op on aarch64, real impl on x86_64). That's a nice cleanup but a separate refactor from "APs also get the MSRs."

I've left it as-is here to keep the diff small. Happy to follow up with an arch-neutral API if that's the direction you and @elstr-512 prefer — would you want that as a follow-up PR after this merges?

void init_syscall_trap();
}
1 change: 1 addition & 0 deletions src/arch/x86_64/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ set(ARCH_OBJECTS
fiber_asm.asm
__syscall_entry.asm
syscall_entry.cpp
syscall_trap.cpp
ist.cpp
paging.cpp
init_paging.cpp
Expand Down
17 changes: 17 additions & 0 deletions src/arch/x86_64/syscall_trap.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
#include <arch/x86/cpu.hpp>
#include <arch/x86/syscall.hpp>

extern "C" void __syscall_entry();

namespace x86 {

void init_syscall_trap()
{
uint64_t star_kernel_cs = 8ull << 32;
uint64_t star_user_cs = 8ull << 48;
uint64_t star = star_kernel_cs | star_user_cs;
CPU::write_msr(IA32_STAR, star);
CPU::write_msr(IA32_LSTAR, (uintptr_t)&__syscall_entry);
}

}
2 changes: 2 additions & 0 deletions src/platform/x86_pc/apic_revenant.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include "apic_timer.hpp"
#include "clocks.hpp"
#include "idt.hpp"
#include <arch/x86/syscall.hpp>
#include <kernel/events.hpp>
//#include <kernel/os.hpp>
#include <os.hpp>
Expand Down Expand Up @@ -90,6 +91,7 @@ void revenant_main(int cpu)
SMP::global_unlock();
// initialize exceptions before asserts
x86::idt_initialize_for_cpu(cpu);
x86::init_syscall_trap();
assert(cpu == SMP::cpu_id());
assert(stack >= this_stack_end && stack < this_stack); // relates to [[maybe_unused]]

Expand Down
15 changes: 3 additions & 12 deletions src/platform/x86_pc/init_libc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#include "init_libc.hpp"

#include <arch/x86/cpu.hpp>
#include <arch/x86/syscall.hpp>
#include <kernel.hpp>
#include <kernel/auxvec.h>
#include <kernel/cpuid.hpp>
Expand Down Expand Up @@ -153,18 +154,8 @@ namespace x86
aux[i++].set_long(AT_NULL, 0);

#ifdef PLATFORM_x86_pc
// SYSCALL instruction
#if defined(__x86_64__)
KDEBUG("* Initialize syscall MSR (64-bit)\n");
uint64_t star_kernel_cs = 8ull << 32;
uint64_t star_user_cs = 8ull << 48;
uint64_t star = star_kernel_cs | star_user_cs;
x86::CPU::write_msr(IA32_STAR, star);
x86::CPU::write_msr(IA32_LSTAR, (uintptr_t)&__syscall_entry);
#elif defined(__i386__)
KDEBUG("Initialize syscall intr (32-bit)\n");
#warning Classical syscall interface missing for 32-bit
#endif
KDEBUG("* Initialize syscall trap\n");
x86::init_syscall_trap();

@mazunki mazunki Jul 7, 2026

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.

@uadhran uadhran Jul 8, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Agree — hooking syscall MSR setup off the libc init path is a bit odd, and I wouldn't want to reshuffle boot ordering inside this PR.

I'll open an issue to discuss when/where syscall trap init should live (e.g. earlier platform bring-up vs libc/runtime init, and how that interacts with AP startup). This PR only extracts the MSR programming and ensures BSP + APs both call the same helper; it doesn't change when the BSP first calls it.

Thanks for calling that out.

#endif

// GDB_ENTRY;
Expand Down