Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
8 changes: 3 additions & 5 deletions src/hyperlight_host/src/hypervisor/gdb/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ use crate::hypervisor::virtual_machine::{HypervisorError, RegisterError, Virtual
use crate::mem::layout::SandboxMemoryLayout;
use crate::mem::memory_region::MemoryRegion;
use crate::mem::mgr::SandboxMemoryManager;
use crate::mem::shared_mem::{HostSharedMemory, SharedMemory};
use crate::mem::shared_mem::HostSharedMemory;

#[derive(Debug, Error)]
pub enum GdbTargetError {
Expand Down Expand Up @@ -166,8 +166,7 @@ impl DebugMemoryAccess {
.dbg_mem_access_fn
.try_lock()
.map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?;
let scratch_base =
hyperlight_common::layout::scratch_base_gpa(mgr.scratch_mem.mem_size());
let scratch_base = mgr.layout.get_scratch_base_gpa();
let (mem, offset, name): (&mut HostSharedMemory, _, _) = if gpa >= scratch_base {
(
&mut mgr.scratch_mem,
Expand Down Expand Up @@ -249,8 +248,7 @@ impl DebugMemoryAccess {
.dbg_mem_access_fn
.try_lock()
.map_err(|e| DebugMemoryAccessError::LockFailed(file!(), line!(), e.to_string()))?;
let scratch_base =
hyperlight_common::layout::scratch_base_gpa(mgr.scratch_mem.mem_size());
let scratch_base = mgr.layout.get_scratch_base_gpa();
let (mem, offset, name): (&mut HostSharedMemory, _, _) = if gpa >= scratch_base {
(
&mut mgr.scratch_mem,
Expand Down
32 changes: 24 additions & 8 deletions src/hyperlight_host/src/hypervisor/hyperlight_vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ pub(crate) struct HyperlightVm {
// The current scratch region, used to keep it alive as long as it
// is used & when unmapping
scratch_memory: Option<GuestSharedMemory>,
// The resolved base GPA of the scratch region
scratch_base_gpa: u64,

mmap_regions: Vec<(u32, MemoryRegion)>, // Later mapped regions (slot number, region)

Expand Down Expand Up @@ -374,6 +376,12 @@ impl HyperlightVm {
vm.set_sregs(&CommonSpecialRegisters::standard_real_mode_defaults())
.map_err(VmError::Register)?;

// Initialize XSAVE state with proper FPU defaults (FCW, MXCSR).
// MSHV (unlike KVM) does not provide sane defaults for the XSAVE
// area on a freshly created vCPU, and will reject the VP state
// on the first run if it is left uninitialized.
vm.reset_xsave().map_err(VmError::Register)?;
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm curious how this worked before

Copy link
Member

Choose a reason for hiding this comment

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

It looks like at issue here is the difference between bringing up the vcpu in real vs long mode. Does the xsave reset call need to be there in both, or should it just be in real mode? I know we had tests of the long mode mshv stuff passing before.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the real vs long is the issue here. When I did some investigation in mxcsr on kvm awhile back it did work fine in long mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree that if we don't need this for some targets it would be nice to not waste cycles here


#[cfg(any(kvm, mshv3))]
let interrupt_handle: Arc<dyn InterruptHandleImpl> = Arc::new(LinuxInterruptHandle {
state: AtomicU8::new(0),
Expand Down Expand Up @@ -407,6 +415,9 @@ impl HyperlightVm {

let snapshot_slot = 0u32;
let scratch_slot = 1u32;
let scratch_base_gpa = config
.get_scratch_base_gpa()
.unwrap_or_else(|| hyperlight_common::layout::scratch_base_gpa(scratch_mem.mem_size()));
#[cfg_attr(not(gdb), allow(unused_mut))]
let mut ret = Self {
vm,
Expand All @@ -422,6 +433,7 @@ impl HyperlightVm {
snapshot_memory: None,
scratch_slot,
scratch_memory: None,
scratch_base_gpa,

mmap_regions: Vec::new(),

Expand Down Expand Up @@ -605,11 +617,11 @@ impl HyperlightVm {
&mut self,
scratch: GuestSharedMemory,
) -> Result<(), UpdateRegionError> {
let guest_base = hyperlight_common::layout::scratch_base_gpa(scratch.mem_size());
let guest_base = self.scratch_base_gpa;
let rgn = scratch.mapping_at(guest_base, MemoryRegionType::Scratch);

if let Some(old_scratch) = self.scratch_memory.replace(scratch) {
let old_base = hyperlight_common::layout::scratch_base_gpa(old_scratch.mem_size());
let old_base = self.scratch_base_gpa;
let old_rgn = old_scratch.mapping_at(old_base, MemoryRegionType::Scratch);
self.vm.unmap_memory((self.scratch_slot, &old_rgn))?;
}
Expand Down Expand Up @@ -2093,7 +2105,9 @@ mod tests {

// Map the scratch region at the top of the address space
let scratch_size = config.get_scratch_size();
let scratch_gpa = hyperlight_common::layout::scratch_base_gpa(scratch_size);
let scratch_gpa = config
.get_scratch_base_gpa()
.unwrap_or_else(|| hyperlight_common::layout::scratch_base_gpa(scratch_size));
let scratch_gva = hyperlight_common::layout::scratch_base_gva(scratch_size);
let scratch_mapping = Mapping {
phys_base: scratch_gpa,
Expand Down Expand Up @@ -2129,9 +2143,10 @@ mod tests {
let (mut hshm, gshm) = mem_mgr.build().unwrap();

let peb_address = gshm.layout.peb_address;
let stack_top_gva = hyperlight_common::layout::MAX_GVA as u64
- hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET
+ 1;
let scratch_base_gva =
hyperlight_common::layout::scratch_base_gva(config.get_scratch_size());
let stack_top_gva = scratch_base_gva + config.get_scratch_size() as u64
- hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET;
let mut vm = set_up_hypervisor_partition(
gshm,
&config,
Expand Down Expand Up @@ -2735,9 +2750,10 @@ mod tests {

/// Get the stack top GVA, same as the regular codepath.
fn stack_top_gva(&self) -> u64 {
hyperlight_common::layout::MAX_GVA as u64
let scratch_size = crate::sandbox::SandboxConfiguration::DEFAULT_SCRATCH_SIZE;
let scratch_base_gva = hyperlight_common::layout::scratch_base_gva(scratch_size);
scratch_base_gva + scratch_size as u64
- hyperlight_common::layout::SCRATCH_TOP_EXN_STACK_OFFSET
+ 1
}
}

Expand Down
37 changes: 29 additions & 8 deletions src/hyperlight_host/src/hypervisor/regs/special_regs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,34 +106,55 @@ impl CommonSpecialRegisters {

#[cfg(not(feature = "init-paging"))]
pub(crate) fn standard_real_mode_defaults() -> Self {
// In real mode, all data/code segment registers must have valid
// limit, present, type_, and s fields. MSHV (unlike KVM) rejects
// the vCPU state if any segment has limit=0 / present=0.
let data_seg = CommonSegmentRegister {
base: 0,
selector: 0,
limit: 0xFFFF,
type_: 3, // data, read/write, accessed
present: 1,
s: 1, // non-system
..Default::default()
};

CommonSpecialRegisters {
cs: CommonSegmentRegister {
base: 0,
selector: 0,
limit: 0xFFFF,
type_: 11,
type_: 11, // code, readable, accessed
present: 1,
s: 1,
s: 1, // non-system
..Default::default()
},
ds: CommonSegmentRegister {
ds: data_seg,
es: data_seg,
ss: data_seg,
fs: data_seg,
gs: data_seg,
tr: CommonSegmentRegister {
base: 0,
selector: 0,
limit: 0xFFFF,
type_: 3,
type_: 11, // 32-bit busy TSS
present: 1,
s: 1,
s: 0, // system segment
..Default::default()
},
tr: CommonSegmentRegister {
ldt: CommonSegmentRegister {
base: 0,
selector: 0,
limit: 0xFFFF,
type_: 11,
type_: 2, // LDT descriptor
present: 1,
s: 0,
s: 0, // system segment
..Default::default()
},
// CR0.ET (bit 4) is hardwired to 1 on modern x86 CPUs.
// MSHV rejects the vCPU state if ET is not set.
Copy link
Contributor

Choose a reason for hiding this comment

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

How hasn't mshv failed so far? was this scenario not tested before?

Copy link
Member

Choose a reason for hiding this comment

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

I don't believe there are many tests for the 16/32-bit guests.

cr0: 1 << 4,
..Default::default()
}
}
Expand Down
53 changes: 48 additions & 5 deletions src/hyperlight_host/src/mem/layout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,15 @@ pub(crate) struct SandboxMemoryLayout {
// The size of the scratch region in physical memory; note that
// this will appear under the top of physical memory.
scratch_size: usize,
// The resolved base GPA of the scratch region. Computed from the
// optional override in SandboxConfiguration, or defaults to
// MAX_GPA - scratch_size + 1.
scratch_base_gpa: u64,
// The resolved base GVA of the scratch region. For non-init-paging
// (no page tables), GVA = GPA (identity mapped). For init-paging
// (64-bit page tables), GVA = MAX_GVA - scratch_size + 1 (high
// canonical address).
scratch_base_gva: u64,
}

impl Debug for SandboxMemoryLayout {
Expand Down Expand Up @@ -158,6 +167,14 @@ impl Debug for SandboxMemoryLayout {
"Scratch region size",
&format_args!("{:#x}", self.scratch_size),
)
.field(
"Scratch base GPA",
&format_args!("{:#x}", self.scratch_base_gpa),
)
.field(
"Scratch base GVA",
&format_args!("{:#x}", self.scratch_base_gva),
)
.finish()
}
}
Expand Down Expand Up @@ -204,6 +221,18 @@ impl SandboxMemoryLayout {
return Err(MemoryRequestTooSmall(scratch_size, min_scratch_size));
}

let scratch_base_gpa = cfg
.get_scratch_base_gpa()
.unwrap_or_else(|| hyperlight_common::layout::scratch_base_gpa(scratch_size));

// For init-paging guests, GVA comes from the high canonical address
// (MAX_GVA - scratch_size + 1). For non-init-paging guests, GVA = GPA
// (identity mapped, no hypervisor-managed page tables).
#[cfg(feature = "init-paging")]
let scratch_base_gva = hyperlight_common::layout::scratch_base_gva(scratch_size);
#[cfg(not(feature = "init-paging"))]
let scratch_base_gva = scratch_base_gpa;

let guest_code_offset = 0;
// The following offsets are to the fields of the PEB struct itself!
let peb_offset = code_size.next_multiple_of(PAGE_SIZE_USIZE);
Expand Down Expand Up @@ -240,6 +269,8 @@ impl SandboxMemoryLayout {
init_data_permissions,
pt_size: None,
scratch_size,
scratch_base_gpa,
scratch_base_gva,
})
}

Expand All @@ -262,6 +293,20 @@ impl SandboxMemoryLayout {
self.scratch_size
}

/// Get the resolved base GPA of the scratch region.
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
pub(crate) fn get_scratch_base_gpa(&self) -> u64 {
self.scratch_base_gpa
}

/// Get the base GVA of the scratch region.
/// For init-paging guests (feature = "init-paging"), this is the high canonical address
/// (MAX_GVA - scratch_size + 1). For non-init-paging guests, GVA = GPA (identity mapped).
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
pub(crate) fn get_scratch_base_gva(&self) -> u64 {
self.scratch_base_gva
}

/// Get the offset in guest memory to the output data pointer.
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
fn get_output_data_pointer_offset(&self) -> usize {
Expand All @@ -281,8 +326,7 @@ impl SandboxMemoryLayout {
/// Get the guest virtual address of the start of output data.
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
pub(crate) fn get_output_data_buffer_gva(&self) -> u64 {
hyperlight_common::layout::scratch_base_gva(self.scratch_size)
+ self.sandbox_memory_config.get_input_data_size() as u64
self.get_scratch_base_gva() + self.sandbox_memory_config.get_input_data_size() as u64
}

/// Get the offset into the host scratch buffer of the start of
Expand Down Expand Up @@ -310,7 +354,7 @@ impl SandboxMemoryLayout {
/// Get the guest virtual address of the start of input data
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
fn get_input_data_buffer_gva(&self) -> u64 {
hyperlight_common::layout::scratch_base_gva(self.scratch_size)
self.get_scratch_base_gva()
}

/// Get the offset into the host scratch buffer of the start of
Expand All @@ -333,8 +377,7 @@ impl SandboxMemoryLayout {
/// copied on restore
#[instrument(skip_all, parent = Span::current(), level= "Trace")]
pub(crate) fn get_pt_base_gpa(&self) -> u64 {
hyperlight_common::layout::scratch_base_gpa(self.scratch_size)
+ self.get_pt_base_scratch_offset() as u64
self.scratch_base_gpa + self.get_pt_base_scratch_offset() as u64
}

/// Get the first GPA of the scratch region that the host hasn't
Expand Down
6 changes: 3 additions & 3 deletions src/hyperlight_host/src/mem/mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,11 +435,11 @@ impl SandboxMemoryManager<HostSharedMemory> {

use crate::sandbox::snapshot::{SharedMemoryPageTableBuffer, access_gpa};

let scratch_size = self.scratch_mem.mem_size();
let scratch_base_gpa = self.layout.get_scratch_base_gpa();

self.shared_mem.with_exclusivity(|snap| {
self.scratch_mem.with_exclusivity(|scratch| {
let pt_buf = SharedMemoryPageTableBuffer::new(snap, scratch, scratch_size, root_pt);
let pt_buf = SharedMemoryPageTableBuffer::new(snap, scratch, scratch_base_gpa, root_pt);

// Walk page tables to get all mappings that cover the GVA range
let mappings: Vec<_> = unsafe {
Expand Down Expand Up @@ -479,7 +479,7 @@ impl SandboxMemoryManager<HostSharedMemory> {

// Translate the GPA to host memory
let gpa = mapping.phys_base + page_offset as u64;
let (mem, offset) = access_gpa(snap, scratch, scratch_size, gpa)
let (mem, offset) = access_gpa(snap, scratch, scratch_base_gpa, gpa)
.ok_or_else(|| {
new_error!(
"Failed to resolve GPA {:#x} to host memory (GVA {:#x})",
Expand Down
17 changes: 16 additions & 1 deletion src/hyperlight_host/src/mem/shared_mem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,22 @@ impl GuestSharedMemory {
MemoryRegionType::Scratch => {
MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE
}
MemoryRegionType::Snapshot => MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE,
// For init-paging, the snapshot is read-only because guest page
// tables provide CoW semantics for writable pages. For
// non-init-paging there are no guest page tables, so the snapshot
// must be writable — otherwise writes (including the CPU setting
// the "Accessed" bit in GDT descriptors during segment loads)
// cause EPT violations that KVM retries forever.
MemoryRegionType::Snapshot => {
#[cfg(feature = "init-paging")]
{
MemoryRegionFlags::READ | MemoryRegionFlags::EXECUTE
}
#[cfg(not(feature = "init-paging"))]
{
MemoryRegionFlags::READ | MemoryRegionFlags::WRITE | MemoryRegionFlags::EXECUTE
}
}
#[allow(clippy::panic)]
// In the future, all the host side knowledge about memory
// region types should collapse down to Snapshot vs
Expand Down
Loading