refactor: split OutBAction into OutBAction + VmAction, move hw_interrupts to x86_64/

- Split OutBAction enum: PvTimerConfig and Halt moved to new VmAction
  enum. VmAction ports are intercepted at the hypervisor level in
  run_vcpu and never reach the sandbox outb handler, so the split
  eliminates unreachable match arms.
- Move hw_interrupts.rs to virtual_machine/x86_64/hw_interrupts.rs
  since it contains x86-64 specific helpers (LAPIC, PIC, PIT).
- Remove halt() from hyperlight_guest::exit — all halt sites use
  inline assembly with options(noreturn) to avoid stack epilogue issues.
- Extract handle_pv_timer_config() method in KVM backend.
- Remove intermediate variable in WhpVm::new().
- Restore create_vm_with_args() comment in MSHV backend.
- Add memory fence after IDT writes in simpleguest for CoW safety.

Signed-off-by: danbugs <danilochiarlone@gmail.com>

Author: danbugs

src/hyperlight_common/src/outb.rs

@@ -86,6 +86,7 @@ impl TryFrom<u8> for Exception {
 }
 
 /// Supported actions when issuing an OUTB actions by Hyperlight.
+/// These are handled by the sandbox-level outb dispatcher.
 /// - Log: for logging,
 /// - CallFunction: makes a call to a host function,
 /// - Abort: aborts the execution of the guest,
@@ -104,6 +105,13 @@ pub enum OutBAction {
     TraceMemoryAlloc = 105,
     #[cfg(feature = "mem_profile")]
     TraceMemoryFree = 106,
+}
+
+/// IO-port actions intercepted at the hypervisor level (in `run_vcpu`)
+/// before they ever reach the sandbox outb handler.  These are split
+/// from [`OutBAction`] so the outb handler does not need unreachable
+/// match arms for ports it can never see.
+pub enum VmAction {
     /// IO port for PV timer configuration. The guest writes a 32-bit
     /// LE value representing the desired timer period in microseconds.
     /// A value of 0 disables the timer.
@@ -129,9 +137,18 @@ impl TryFrom<u16> for OutBAction {
             105 => Ok(OutBAction::TraceMemoryAlloc),
             #[cfg(feature = "mem_profile")]
             106 => Ok(OutBAction::TraceMemoryFree),
-            107 => Ok(OutBAction::PvTimerConfig),
-            108 => Ok(OutBAction::Halt),
             _ => Err(anyhow::anyhow!("Invalid OutBAction value: {}", val)),
         }
     }
 }
+
+impl TryFrom<u16> for VmAction {
+    type Error = anyhow::Error;
+    fn try_from(val: u16) -> anyhow::Result<Self> {
+        match val {
+            107 => Ok(VmAction::PvTimerConfig),
+            108 => Ok(VmAction::Halt),
+            _ => Err(anyhow::anyhow!("Invalid VmAction value: {}", val)),
+        }
+    }
+}

src/hyperlight_guest/src/exit.rs

@@ -18,34 +18,6 @@ use core::arch::asm;
 use core::ffi::{CStr, c_char};
 
 use hyperlight_common::outb::OutBAction;
-use tracing::instrument;
-
-/// Signal successful completion of the guest's work and return control
-/// to the host.  This replaces the previous `hlt`-based exit: under the
-/// `hw-interrupts` feature, `hlt` becomes a wait-for-interrupt (the
-/// in-kernel IRQ chip wakes the vCPU), so we use an explicit IO-port
-/// write (port 108) to trigger a VM exit that the host treats as a
-/// clean shutdown.
-///
-/// This function never returns — the host does not re-enter the guest
-/// after seeing the Halt port.
-#[inline(never)]
-#[instrument(skip_all, level = "Trace")]
-pub fn halt() {
-    #[cfg(all(feature = "trace_guest", target_arch = "x86_64"))]
-    {
-        // Send data before halting
-        // If there is no data, this doesn't do anything
-        // The reason we do this here instead of in `hlt` asm function
-        // is to avoid allocating before halting, which leaks memory
-        // because the guest is not expected to resume execution after halting.
-        hyperlight_guest_tracing::flush();
-    }
-
-    unsafe {
-        out32(OutBAction::Halt as u16, 0);
-    }
-}
 
 /// Exits the VM with an Abort OUT action and code 0.
 #[unsafe(no_mangle)]

src/hyperlight_host/src/hypervisor/virtual_machine/kvm/x86_64.rs

@@ -20,7 +20,7 @@ use std::sync::LazyLock;
 #[cfg(feature = "hw-interrupts")]
 use std::sync::atomic::{AtomicBool, Ordering};
 
-use hyperlight_common::outb::OutBAction;
+use hyperlight_common::outb::VmAction;
 #[cfg(gdb)]
 use kvm_bindings::kvm_guest_debug;
 use kvm_bindings::{
@@ -135,7 +135,7 @@ impl KvmVm {
         //
         // Instead of creating an in-kernel PIT (create_pit2), we use a
         // host-side timer thread + irqfd to inject IRQ0 at the rate
-        // requested by the guest via OutBAction::PvTimerConfig (port 107).
+        // requested by the guest via VmAction::PvTimerConfig (port 107).
         // This eliminates the in-kernel PIT device. Guest PIT port writes
         // (0x40, 0x43) become no-ops handled in the run loop.
         #[cfg(feature = "hw-interrupts")]
@@ -201,7 +201,7 @@ impl KvmVm {
     /// When hw-interrupts is enabled, the in-kernel PIC + LAPIC deliver
     /// interrupts triggered by the host-side timer thread via irqfd.
     /// There is no in-kernel PIT; guest PIT port writes are no-ops.
-    /// The guest signals "I'm done" by writing to OutBAction::Halt
+    /// The guest signals "I'm done" by writing to VmAction::Halt
     /// (an IO port exit) instead of using HLT, because the in-kernel
     /// LAPIC absorbs HLT (never returns VcpuExit::Hlt to userspace).
     #[cfg(feature = "hw-interrupts")]
@@ -214,47 +214,20 @@ impl KvmVm {
                     continue;
                 }
                 Ok(VcpuExit::IoOut(port, data)) => {
-                    if port == OutBAction::Halt as u16 {
+                    if port == VmAction::Halt as u16 {
                         // Stop the timer thread before returning.
                         self.timer_stop.store(true, Ordering::Relaxed);
                         if let Some(h) = self.timer_thread.take() {
                             let _ = h.join();
                         }
                         return Ok(VmExit::Halt());
                     }
-                    if port == OutBAction::PvTimerConfig as u16 {
-                        // The guest is configuring the timer period.
-                        // Extract the period in microseconds (LE u32).
-                        if let Some(bytes4) = data.get(..4)
-                            && let Ok(bytes) = bytes4.try_into()
-                        {
-                            let period_us = u32::from_le_bytes(bytes) as u64;
-                            if period_us == 0 {
-                                // Stop existing timer if any.
-                                if let Some(thread) = self.timer_thread.take() {
-                                    self.timer_stop.store(true, Ordering::Relaxed);
-                                    let _ = thread.join();
-                                }
-                            } else if self.timer_thread.is_none() {
-                                // Reset the stop flag — a previous halt (e.g. the
-                                // init halt during evolve()) may have set it.
-                                self.timer_stop.store(false, Ordering::Relaxed);
-                                let eventfd = self.timer_irq_eventfd.try_clone().map_err(|e| {
-                                    RunVcpuError::Unknown(HypervisorError::KvmError(e.into()))
-                                })?;
-                                let stop = self.timer_stop.clone();
-                                let period = std::time::Duration::from_micros(period_us);
-                                self.timer_thread = Some(std::thread::spawn(move || {
-                                    while !stop.load(Ordering::Relaxed) {
-                                        std::thread::sleep(period);
-                                        if stop.load(Ordering::Relaxed) {
-                                            break;
-                                        }
-                                        let _ = eventfd.write(1);
-                                    }
-                                }));
-                            }
-                        }
+                    if port == VmAction::PvTimerConfig as u16 {
+                        let data_copy: [u8; 4] = data
+                            .get(..4)
+                            .and_then(|s| s.try_into().ok())
+                            .unwrap_or([0; 4]);
+                        self.handle_pv_timer_config(&data_copy)?;
                         continue;
                     }
                     // PIT ports (0x40-0x43): no in-kernel PIT, so these
@@ -288,12 +261,44 @@ impl KvmVm {
         }
     }
 
+    #[cfg(feature = "hw-interrupts")]
+    fn handle_pv_timer_config(&mut self, data: &[u8; 4]) -> std::result::Result<(), RunVcpuError> {
+        let period_us = u32::from_le_bytes(*data) as u64;
+        if period_us == 0 {
+            // Stop existing timer if any.
+            if let Some(thread) = self.timer_thread.take() {
+                self.timer_stop.store(true, Ordering::Relaxed);
+                let _ = thread.join();
+            }
+        } else if self.timer_thread.is_none() {
+            // Reset the stop flag — a previous halt (e.g. the
+            // init halt during evolve()) may have set it.
+            self.timer_stop.store(false, Ordering::Relaxed);
+            let eventfd = self
+                .timer_irq_eventfd
+                .try_clone()
+                .map_err(|e| RunVcpuError::Unknown(HypervisorError::KvmError(e.into())))?;
+            let stop = self.timer_stop.clone();
+            let period = std::time::Duration::from_micros(period_us);
+            self.timer_thread = Some(std::thread::spawn(move || {
+                while !stop.load(Ordering::Relaxed) {
+                    std::thread::sleep(period);
+                    if stop.load(Ordering::Relaxed) {
+                        break;
+                    }
+                    let _ = eventfd.write(1);
+                }
+            }));
+        }
+        Ok(())
+    }
+
     /// Run the vCPU once without hardware interrupt support (default path).
     #[cfg(not(feature = "hw-interrupts"))]
     fn run_vcpu_default(&mut self) -> std::result::Result<VmExit, RunVcpuError> {
         match self.vcpu_fd.run() {
             Ok(VcpuExit::Hlt) => Ok(VmExit::Halt()),
-            Ok(VcpuExit::IoOut(port, _)) if port == OutBAction::Halt as u16 => Ok(VmExit::Halt()),
+            Ok(VcpuExit::IoOut(port, _)) if port == VmAction::Halt as u16 => Ok(VmExit::Halt()),
             Ok(VcpuExit::IoOut(port, data)) => Ok(VmExit::IoOut(port, data.to_vec())),
             Ok(VcpuExit::MmioRead(addr, _)) => Ok(VmExit::MmioRead(addr)),
             Ok(VcpuExit::MmioWrite(addr, _)) => Ok(VmExit::MmioWrite(addr)),
@@ -609,8 +614,8 @@ mod hw_interrupt_tests {
 
     #[test]
     fn halt_port_is_not_standard_device() {
-        // OutBAction::Halt port must not overlap in-kernel PIC/PIT/speaker ports
-        const HALT: u16 = OutBAction::Halt as u16;
+        // VmAction::Halt port must not overlap in-kernel PIC/PIT/speaker ports
+        const HALT: u16 = VmAction::Halt as u16;
         const _: () = assert!(HALT != 0x20 && HALT != 0x21);
         const _: () = assert!(HALT != 0xA0 && HALT != 0xA1);
         const _: () = assert!(HALT != 0x40 && HALT != 0x41 && HALT != 0x42 && HALT != 0x43);

src/hyperlight_host/src/hypervisor/virtual_machine/mod.rs

@@ -38,9 +38,9 @@ pub(crate) mod mshv;
 #[cfg(target_os = "windows")]
 pub(crate) mod whp;
 
-/// Shared helpers for hardware interrupt support (MSHV and WHP)
+/// Shared x86-64 helpers for hardware interrupt support (MSHV and WHP)
 #[cfg(feature = "hw-interrupts")]
-pub(crate) mod hw_interrupts;
+pub(crate) mod x86_64;
 
 static AVAILABLE_HYPERVISOR: OnceLock<Option<HypervisorType>> = OnceLock::new();
 

src/hyperlight_host/src/hypervisor/virtual_machine/mshv/x86_64.rs

@@ -22,7 +22,7 @@ use std::sync::LazyLock;
 #[cfg(feature = "hw-interrupts")]
 use std::sync::atomic::{AtomicBool, Ordering};
 
-use hyperlight_common::outb::OutBAction;
+use hyperlight_common::outb::VmAction;
 #[cfg(feature = "hw-interrupts")]
 use mshv_bindings::LapicState;
 #[cfg(gdb)]
@@ -128,6 +128,8 @@ impl MshvVm {
         {
             pr.pt_flags = 1u64; // LAPIC
         }
+        // It's important to use create_vm_with_args() (not create_vm()),
+        // because create_vm() sets up a SynIC partition by default.
         let vm_fd = mshv
             .create_vm_with_args(&pr)
             .map_err(|e| CreateVmError::CreateVmFd(e.into()))?;
@@ -156,7 +158,7 @@ impl MshvVm {
         // interrupts can be delivered (request_virtual_interrupt would fail).
         #[cfg(feature = "hw-interrupts")]
         {
-            use super::hw_interrupts::init_lapic_registers;
+            use super::super::x86_64::hw_interrupts::init_lapic_registers;
 
             let mut lapic: LapicState = vcpu_fd
                 .get_lapic()
@@ -261,9 +263,9 @@ impl VirtualMachine for MshvVm {
                                 }])
                                 .map_err(|e| RunVcpuError::IncrementRip(e.into()))?;
 
-                            // OutBAction::Halt always means "I'm done", regardless
+                            // VmAction::Halt always means "I'm done", regardless
                             // of whether a timer is active.
-                            if is_write && port_number == OutBAction::Halt as u16 {
+                            if is_write && port_number == VmAction::Halt as u16 {
                                 // Stop the timer thread before returning.
                                 #[cfg(feature = "hw-interrupts")]
                                 {
@@ -283,7 +285,7 @@ impl VirtualMachine for MshvVm {
                                         continue;
                                     }
                                 } else if let Some(val) =
-                                    super::hw_interrupts::handle_io_in(port_number)
+                                    super::super::x86_64::hw_interrupts::handle_io_in(port_number)
                                 {
                                     self.vcpu_fd
                                         .set_reg(&[hv_register_assoc {
@@ -629,13 +631,13 @@ impl MshvVm {
     /// acknowledges via PIC.
     fn do_lapic_eoi(&self) {
         if let Ok(mut lapic) = self.vcpu_fd.get_lapic() {
-            super::hw_interrupts::lapic_eoi(lapic_regs_as_u8_mut(&mut lapic.regs));
+            super::super::x86_64::hw_interrupts::lapic_eoi(lapic_regs_as_u8_mut(&mut lapic.regs));
             let _ = self.vcpu_fd.set_lapic(&lapic);
         }
     }
 
     fn handle_hw_io_out(&mut self, port: u16, data: &[u8]) -> bool {
-        if port == OutBAction::PvTimerConfig as u16 {
+        if port == VmAction::PvTimerConfig as u16 {
             if data.len() >= 4 {
                 let period_us = u32::from_le_bytes([data[0], data[1], data[2], data[3]]);
                 if period_us == 0 {
@@ -677,17 +679,19 @@ impl MshvVm {
                     // guest disabled the APIC globally)
                     if let Ok(mut lapic) = self.vcpu_fd.get_lapic() {
                         let regs = lapic_regs_as_u8(&lapic.regs);
-                        let svr = super::hw_interrupts::read_lapic_u32(regs, 0xF0);
+                        let svr = super::super::x86_64::hw_interrupts::read_lapic_u32(regs, 0xF0);
                         if svr & 0x100 == 0 {
                             let regs_mut = lapic_regs_as_u8_mut(&mut lapic.regs);
-                            super::hw_interrupts::write_lapic_u32(regs_mut, 0xF0, 0x1FF);
-                            super::hw_interrupts::write_lapic_u32(regs_mut, 0x80, 0); // TPR
+                            super::super::x86_64::hw_interrupts::write_lapic_u32(
+                                regs_mut, 0xF0, 0x1FF,
+                            );
+                            super::super::x86_64::hw_interrupts::write_lapic_u32(regs_mut, 0x80, 0); // TPR
                             let _ = self.vcpu_fd.set_lapic(&lapic);
                         }
                     }
 
                     let vm_fd = self.vm_fd.clone();
-                    let vector = super::hw_interrupts::TIMER_VECTOR;
+                    let vector = super::super::x86_64::hw_interrupts::TIMER_VECTOR;
                     let stop = self.timer_stop.clone();
                     let period = std::time::Duration::from_micros(period_us as u64);
                     self.timer_thread = Some(std::thread::spawn(move || {
@@ -711,7 +715,9 @@ impl MshvVm {
             return true;
         }
         let timer_active = self.timer_thread.is_some();
-        super::hw_interrupts::handle_common_io_out(port, data, timer_active, || self.do_lapic_eoi())
+        super::super::x86_64::hw_interrupts::handle_common_io_out(port, data, timer_active, || {
+            self.do_lapic_eoi()
+        })
     }
 }
 
@@ -734,10 +740,10 @@ mod hw_interrupt_tests {
     fn lapic_regs_conversion_roundtrip() {
         let mut regs = [0i8; 1024];
         let bytes = lapic_regs_as_u8_mut(&mut regs);
-        super::super::hw_interrupts::write_lapic_u32(bytes, 0xF0, 0xDEAD_BEEF);
+        super::super::super::x86_64::hw_interrupts::write_lapic_u32(bytes, 0xF0, 0xDEAD_BEEF);
         let bytes = lapic_regs_as_u8(&regs);
         assert_eq!(
-            super::super::hw_interrupts::read_lapic_u32(bytes, 0xF0),
+            super::super::super::x86_64::hw_interrupts::read_lapic_u32(bytes, 0xF0),
             0xDEAD_BEEF
         );
     }