Skip to content
Draft
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
2 changes: 1 addition & 1 deletion src/hyperlight_host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ tracing-core = "0.1.36"
tracing-opentelemetry = { version = "0.33.0", optional = true }
hyperlight-common = { workspace = true, default-features = true, features = [ "std" ] }
hyperlight-guest-tracing = { workspace = true, default-features = true, optional = true }
vmm-sys-util = "0.15.0"
crossbeam-channel = "0.5.15"
thiserror = "2.0.18"
chrono = { version = "0.4", optional = true }
Expand Down Expand Up @@ -83,6 +82,7 @@ kvm-bindings = { version = "0.14", features = ["fam-wrappers"], optional = true
kvm-ioctls = { version = "0.25", optional = true }
mshv-bindings = { version = "0.6", optional = true }
mshv-ioctls = { version = "0.6", optional = true}
vmm-sys-util = "0.15.0"

[dev-dependencies]
uuid = { version = "1.23.3", features = ["v4"] }
Expand Down
34 changes: 28 additions & 6 deletions src/hyperlight_host/src/hypervisor/hyperlight_vm/aarch64.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ limitations under the License.
// TODO(aarch64): implement arch-specific HyperlightVm methods

use std::sync::Arc;
use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU64};
use std::sync::atomic::AtomicU8;
#[cfg(target_os = "linux")]
use std::sync::atomic::{AtomicBool, AtomicU64};

use super::{
AccessPageTableError, CreateHyperlightVmError, DispatchGuestCallError, HyperlightVm,
Expand All @@ -29,12 +31,17 @@ use crate::hypervisor::hyperlight_vm::get_guest_log_filter;
use crate::hypervisor::regs::{CommonFpu, CommonRegisters, CommonSpecialRegisters};
#[cfg(kvm)]
use crate::hypervisor::virtual_machine::kvm::KvmVm;
#[cfg(kvm)]
#[cfg(target_os = "windows")]
use crate::hypervisor::virtual_machine::whp::WhpVm;
#[cfg(any(kvm, mshv3, target_os = "windows"))]
use crate::hypervisor::virtual_machine::{HypervisorType, VmError};
use crate::hypervisor::virtual_machine::{
RegisterError, ResetVcpuError, VirtualMachine, get_available_hypervisor,
};
#[cfg(target_os = "linux")]
use crate::hypervisor::{InterruptHandleImpl, LinuxInterruptHandle};
#[cfg(target_os = "windows")]
use crate::hypervisor::{InterruptHandleImpl, PartitionState, WindowsInterruptHandle};
use crate::mem::mgr::{SandboxMemoryManager, SnapshotSharedMemory};
use crate::mem::shared_mem::{GuestSharedMemory, HostSharedMemory};
use crate::sandbox::SandboxConfiguration;
Expand All @@ -54,7 +61,7 @@ impl HyperlightVm {
next_action: NextAction,
rsp_gva: u64,
page_size: usize,
config: &SandboxConfiguration,
#[cfg_attr(target_os = "windows", allow(unused_variables))] config: &SandboxConfiguration,
#[cfg(gdb)] _gdb_conn: Option<DebugCommChannel<DebugResponse, DebugMsg>>,
#[cfg(crashdump)] _rt_cfg: SandboxRuntimeConfig,
#[cfg(feature = "mem_profile")] _trace_info: MemTraceInfo,
Expand All @@ -64,13 +71,19 @@ impl HyperlightVm {
let vm: VmType = match get_available_hypervisor() {
#[cfg(kvm)]
Some(HypervisorType::Kvm) => Box::new(KvmVm::new().map_err(VmError::CreateVm)?),
// TODO: mshv support
#[cfg(mshv3)]
Some(HypervisorType::Mshv) => return Err(CreateHyperlightVmError::NoHypervisorFound),
Some(HypervisorType::Mshv) => {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This line looks like it's the same before/after?

// MSHV aarch64 VirtualMachine impl not yet available on this branch
return Err(CreateHyperlightVmError::NoHypervisorFound);
}
#[cfg(target_os = "windows")]
Some(HypervisorType::Whp) => Box::new(WhpVm::new().map_err(VmError::CreateVm)?),
None => return Err(CreateHyperlightVmError::NoHypervisorFound),
};
vm.set_sregs(&CommonSpecialRegisters::defaults(root_pt_addr))
.map_err(VmError::Register)?;

#[cfg(target_os = "linux")]
let interrupt_handle: Arc<dyn InterruptHandleImpl> = Arc::new(LinuxInterruptHandle {
state: AtomicU8::new(0),
tid: AtomicU64::new(unsafe { libc::pthread_self() as u64 }),
Expand All @@ -79,6 +92,15 @@ impl HyperlightVm {
dropped: AtomicBool::new(false),
});

#[cfg(target_os = "windows")]
let interrupt_handle: Arc<dyn InterruptHandleImpl> = Arc::new(WindowsInterruptHandle {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just a heads up that there is a big refactor/simplification to the interrupt handle machinery in #1674 that you may want to take a look at.

state: AtomicU8::new(0),
partition_state: std::sync::RwLock::new(PartitionState {
handle: vm.partition_handle(),
dropped: false,
}),
});

let snapshot_slot = 0u32;
let scratch_slot = 1u32;
let vm_can_reset_vcpu = vm.can_reset_vcpu();
Expand Down Expand Up @@ -214,7 +236,7 @@ impl HyperlightVm {
self.vm_can_reset_vcpu,
"No fallback path for vcpu reset on aarch64"
);
self.vm.reset_vcpu()?;
self.interrupt_handle.reset_vcpu(self.vm.as_mut())?;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's this change about? It seems like the wrong place for this.

If it's related to the locking that happens in the interrupt handle: that's required because the interrupt handle can outlive the WhpVm object that it is created from (and the partition is destroyed when the latter is dropped). Here, you have a reference to the WhpVm on hand, so you can be sure that it hasn't been dropped (and then destroyed).

self.apply_sregs(cr3, sregs)?;
Ok(())
}
Expand Down
23 changes: 23 additions & 0 deletions src/hyperlight_host/src/hypervisor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,9 @@ use std::sync::atomic::{AtomicU8, Ordering};
#[cfg(any(kvm, mshv3))]
use std::time::Duration;

#[cfg(target_arch = "aarch64")]
use self::virtual_machine::{ResetVcpuError, VirtualMachine};

/// A trait for platform-specific interrupt handle implementation details
pub(crate) trait InterruptHandleImpl: InterruptHandle {
/// Set the thread ID for the vcpu thread
Expand All @@ -67,6 +70,12 @@ pub(crate) trait InterruptHandleImpl: InterruptHandle {
/// Clear the cancellation request flag
fn clear_cancel(&self);

/// Reset the vCPU while honoring platform lifecycle synchronization.
#[cfg(target_arch = "aarch64")]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As mentioned above, this seems like the wrong place for this.

fn reset_vcpu(&self, vm: &mut dyn VirtualMachine) -> Result<(), ResetVcpuError> {
vm.reset_vcpu()
}

/// Check if debug interrupt was requested (always returns false when gdb feature is disabled)
fn is_debug_interrupted(&self) -> bool;

Expand Down Expand Up @@ -347,6 +356,20 @@ impl InterruptHandleImpl for WindowsInterruptHandle {
self.state.fetch_and(!Self::RUNNING_BIT, Ordering::Release);
}

#[cfg(target_arch = "aarch64")]
fn reset_vcpu(&self, vm: &mut dyn VirtualMachine) -> Result<(), ResetVcpuError> {
let guard = self
.partition_state
.write()
.map_err(|e| ResetVcpuError::Unknown(e.to_string()))?;
if guard.dropped {
return Err(ResetVcpuError::Unknown(
"cannot reset a dropped partition".to_string(),
));
}
vm.reset_vcpu()
}

fn is_debug_interrupted(&self) -> bool {
#[cfg(gdb)]
{
Expand Down
4 changes: 2 additions & 2 deletions src/hyperlight_host/src/hypervisor/regs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,13 @@ pub(crate) use x86_64::*;

#[cfg(target_arch = "aarch64")]
mod aarch64;
#[cfg(target_os = "windows")]
#[cfg(all(target_arch = "x86_64", target_os = "windows"))]
use std::collections::HashSet;

#[cfg(target_arch = "aarch64")]
pub(crate) use aarch64::*;

#[cfg(target_os = "windows")]
#[cfg(all(target_arch = "x86_64", target_os = "windows"))]
#[derive(Debug, PartialEq)]
pub(crate) enum FromWhpRegisterError {
MissingRegister(HashSet<i32>),
Expand Down
Loading
Loading