Skip to content

Support multi-threading in hypervisors - #16

Merged
AdvDebug merged 2 commits into
mainfrom
hypervisor-smp
Sep 6, 2026
Merged

AdvDebug merged 2 commits into
mainfrom
hypervisor-smp

Conversation

@AdvDebug

@AdvDebug AdvDebug commented Sep 6, 2026

Copy link
Copy Markdown
Owner

Guest threads now run on several host threads at once on a hypervisor backend. A kernel lock covers every host thread that touches emulator, OS layer or backend state, and the backend releases it only for the time a processor spends in the guest. Scheduler state that used to sit on the emulator moved to a per host thread worker, so each worker holds its own current thread, context flag and register scratch. A dispatched thread is hidden from every scan and given back on every route out of a slice. Idle workers wait on the kernel lock and a sweep thread pulses one at the idle cadence, because a timed wake has no producer. --no-smp runs the single thread scheduler unchanged and --cores=<n> sets how many threads run together, both forwarded to a guest process the emulator launches.

The backend interface gains ProcessorLimit, UseRunLock, StopThread and StopAllProcessors. The WHP and KVM bindings give each guest thread its own virtual processor, so a thread switch moves no register state, and a thread that cannot get one waits. Per processor state moved out of the binding and into the processor object, including the register cache, the stepped completion, the stop request and the GDT, TSS and interrupt stack. A slice deadline is named by generation so a late cancel cannot end the slice that replaced it. A rebuild keeps a mapping whose pages still map as installed instead of replacing it, because a processor delivering an exception through the gap cannot recover. KVM offsets the counter of a later virtual processor onto the first one, and RDTSC is answered from the real counter when the backend has one.

NtGetContextThread, NtSetContextThread and NtTerminateThread park the target thread before they read or change it, and the context calls answer STATUS_UNSUCCESSFUL when it never reaches a safe point. NtSuspendThread and NtQueueApcThread end the target's slice instead of the calling one. A thread that has ended releases its TEB, its initial stack and its initial context, and its thread object stays only while a handle refers to it. NtLockFile retries a conflicting lock through a slice rather than sleeping on the host thread.

The 64 bit fields of KUSER_SHARED_DATA are written with the high half first, so a reader that retries while High1Time differs from High2Time reads a whole value, and the hypervisor shared page stores its offset before the sequence that publishes it. The module list poll reads the PEB_LDR_DATA list head instead of watching writes to the page it shares with the loader lock.

On Linux the syscall helper takes the current thread id from the emulator and keeps CpuidEnabled on the thread rather than on the helper.

Guest threads now run on several host threads at once on a hypervisor
backend. A kernel lock covers every host thread that touches emulator, OS
layer or backend state, and the backend releases it only for the time a
processor spends in the guest. Scheduler state that used to sit on the
emulator moved to a per host thread worker, so each worker holds its own
current thread, context flag and register scratch. A dispatched thread is
hidden from every scan and given back on every route out of a slice. Idle
workers wait on the kernel lock and a sweep thread pulses one at the idle
cadence, because a timed wake has no producer. `--no-smp` runs the single
thread scheduler unchanged and `--cores=<n>` sets how many threads run
together, both forwarded to a guest process the emulator launches.

The backend interface gains ProcessorLimit, UseRunLock, StopThread and
StopAllProcessors. The WHP and KVM bindings give each guest thread its own
virtual processor, so a thread switch moves no register state, and a thread
that cannot get one waits. Per processor state moved out of the binding and
into the processor object, including the register cache, the stepped
completion, the stop request and the GDT, TSS and interrupt stack. A slice
deadline is named by generation so a late cancel cannot end the slice that
replaced it. A rebuild keeps a mapping whose pages still map as installed
instead of replacing it, because a processor delivering an exception through
the gap cannot recover. KVM offsets the counter of a later virtual processor
onto the first one, and RDTSC is answered from the real counter when the
backend has one.

NtGetContextThread, NtSetContextThread and NtTerminateThread park the target
thread before they read or change it, and the context calls answer
STATUS_UNSUCCESSFUL when it never reaches a safe point. NtSuspendThread and
NtQueueApcThread end the target's slice instead of the calling one. A thread
that has ended releases its TEB, its initial stack and its initial context,
and its thread object stays only while a handle refers to it. NtLockFile
retries a conflicting lock through a slice rather than sleeping on the host
thread.

The 64 bit fields of KUSER_SHARED_DATA are written with the high half first,
so a reader that retries while High1Time differs from High2Time reads a whole
value, and the hypervisor shared page stores its offset before the sequence
that publishes it. The module list poll reads the PEB_LDR_DATA list head
instead of watching writes to the page it shares with the loader lock.

On Linux the syscall helper takes the current thread id from the emulator and
keeps CpuidEnabled on the thread rather than on the helper.
@AdvDebug

AdvDebug commented Sep 6, 2026

Copy link
Copy Markdown
Owner Author

@claude review

@claude

claude Bot commented Sep 6, 2026 •

Copy link
Copy Markdown

Claude finished @AdvDebug's task in 7m 55s —— View job


Review: Support multi-threading in hypervisors

Todo list

  • Gather context (PR body, changed files)
  • Read core scheduler/kernel-lock changes (BinaryEmulator.cs)
  • Read KVM backend changes (KvmBinding/Kvm.cs)
  • Read WHP backend changes (WhpBinding/Whp.cs)
  • Read Windows NT syscall changes (context/suspend/terminate/lock file)
  • Read KUSER_SHARED_DATA / shared-page write changes
  • Read Linux syscall helper / smaller files (Program.cs, guests, RuntimeTypes.cs)
  • Post review

This is a large, well-thought-out change (the kernel-lock discipline, per-processor state split, and generation-tagged slice deadlines are all sound design choices), but I found a few concrete correctness/concurrency issues that are worth fixing before merge, plus a couple of lower-confidence items worth double-checking.

High confidence

  1. NtTerminateThread ignores the result of WaitUntilParked — Brovan/Core/Emulation/OS/Windows/Process/NtTerminateThread.cs:28
    WaitUntilParked returns false when the target thread never reaches a safe point within its ~250ms deadline (still actively running on another host worker). NtGetContextThread and NtSetContextThread both check this and bail with STATUS_UNSUCCESSFUL, but NtTerminateThread discards the return value and proceeds to overwrite TargetThread.ExitCode/State/termination state anyway — while the target may still genuinely be executing on another vCPU. STATUS_SUCCESS is returned to the caller even though the termination raced the target's own execution.
    Fix this →

  2. WHP stepped-completion window is not safe under concurrent vCPUs — BeginSteppedCompletion/ArmSteppedCompletion in Brovan/Core/Emulation/WhpBinding/Whp.cs (~lines 2658–2684)
    When a vCPU traps on a write-protected/MMIO page, this arms a one-instruction grace window by mapping the page fully RWX via WHvMapGpaRange/WHvUnmapGpaRange. Those APIs are partition-wide, not per-vCPU, but CompletionActive is tracked only on the faulting VirtualProcessor. Since other vCPUs now keep running concurrently on other host threads (the kernel lock is released during WHvRunVirtualProcessor), a second vCPU can read/write the same page during that window and silently bypass the write-trap/MMIO hook entirely — this only became possible once SMP allowed genuinely concurrent vCPU execution. Relatedly, UnbindThread recycles a VirtualProcessor into _idleProcessors without resetting CompletionActive/CompletionPageGpa, and the UnrecoverableException/InvalidVpRegisterValue/UnsupportedFeature exit paths in Emulate throw without draining a pending completion first, so stale completion state can leak into a reused vCPU.
    Fix this →

  3. KUSER_SHARED_DATA's live update path lacks the ordering its sibling helper has — compare WriteKsystemTimeToSpan (~line 616) and WriteKsystemTimeToMemory (~line 797) in Brovan/Core/Emulation/OS/Windows/WinInternalHelper.cs
    Both are meant to implement the NT KSYSTEM_TIME torn-read trick this PR calls out (write High2Time, then Low, then High1Time last, so a reader retries while the two highs differ). WriteKsystemTimeToSpan correctly finishes with Volatile.Write on the final High1Time write. WriteKsystemTimeToMemory — the one actually used by the live, per-tick update path (UpdateDynamicFields → RefreshIfUnhooked, called from the scheduler loop every slice) — instead does three plain Emulator._emulator.WriteMemory calls, which on the KVM/WHP backends bottom out in an unordered Unsafe.CopyBlockUnaligned/memcpy with no release semantics. On a host with a weaker memory model than x86 (e.g. ARM64), a concurrently running vCPU on another core could observe the new High1Time before the new Low/High2Time are visible — exactly the torn read this mechanism exists to prevent under SMP.
    Fix this →

  4. KVM Dispose() doesn't join the maintenance thread before freeing native resources — Brovan/Core/Emulation/KvmBinding/Kvm.cs:1417-1436 vs. MaintenanceLoop/EnforceSliceDeadlines/KickProcessor (~1140-1194)
    Dispose() sets the disposing flag and immediately munmaps/closes every vCPU's run page and fd. The maintenance thread only checks that flag once per ~1ms sweep, so if it's already inside KickProcessor iterating the (now N-vCPU) processor list when Dispose runs concurrently, it can dereference a run pointer/fd that's already been unmapped/closed — a native crash on teardown. This existed in spirit pre-SMP but is now exercised across every vCPU on every sweep instead of a single one, making it meaningfully more likely to hit.
    Fix this →

Worth double-checking (lower confidence)

  1. KVM KickProcessor may signal a recycled OS thread id — Kvm.cs. SMP scheduler worker threads are created/joined per RunScheduler call, so a vCPU's recorded HostTid can outlive the OS thread that set it; if the OS recycles that tid, EnforceSliceDeadlines/StopThread could tgkill an unrelated thread instead of the intended vCPU.
  2. vp.Binding (KVM's generation counter) is a plain, non-atomic long whose correctness depends entirely on every caller going through KernelLock. That's true today, but nothing enforces it, and at least one debug-menu path (EmulationMenu.cs:1199, Emulator._emulator.Emulate(...) called directly from the "run" command for GenericGuest) and the "threads switch" command (EmulationMenu/Handlers.cs:1432 → BinaryEmulator.TrySwitchToThread) invoke emulator/backend state without visibly acquiring KernelLock. It's possible these only ever run while the scheduler is genuinely stopped, but it'd be good to confirm that invariant explicitly (e.g. an assert or a comment), since it's exactly the kind of lock-discipline requirement this PR introduces everywhere else.

Minor

  1. Whp.cs CreateProcessor: if InitializeVirtualProcessorState throws after AllocateProcessorTables succeeded, the catch block frees ExitContextPtr and deletes the vCPU but leaves the already-inserted GDT/TSS/exception-stack pages in _mappedPages/_pageTableViews and doesn't roll back the internal GPA pool offset — a one-time leak on a rare failure path.

What looks solid

Per-processor register cache/GDT/TSS/interrupt-stack isolation, the deadline-generation pairing in EnforceSliceDeadlines/Emulate (correctly rejects a stale cancel against a reused vCPU slot), the KVM TSC-offset alignment across virtual processors, and the rebuild-keeps-installed-mapping logic for the remap-gap crash all checked out.

@AdvDebug

AdvDebug commented Sep 6, 2026

Copy link
Copy Markdown
Owner Author

For the rest of the issues they are tested locally and didn't trigger anything, so that should be okay.

@AdvDebug
AdvDebug merged commit ded2575 into main Sep 6, 2026
2 checks passed
@AdvDebug
AdvDebug deleted the hypervisor-smp branch September 6, 2026 10:54
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.

1 participant