From 5a11cb05e597a59fc5c15a3a696c962bd6a37fcc Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 23 Sep 2025 20:53:47 +0000 Subject: [PATCH 1/3] Set mxcsr explicity on kvm and add tests Signed-off-by: James Sturtevant --- src/hyperlight_host/src/hypervisor/kvm.rs | 21 ++++- .../src/sandbox/initialized_multi_use.rs | 27 ++++++ src/tests/rust_guests/simpleguest/src/main.rs | 84 +++++++++++++++++++ 3 files changed, 131 insertions(+), 1 deletion(-) diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index a69b51f55..29f2652f4 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -579,7 +579,8 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_regs(®s)?; - // reset fpu state + // note kvm set_fpu doesn't actually set or read the mxcsr value + // https://elixir.bootlin.com/linux/v6.16/source/arch/x86/kvm/x86.c#L12229 let fpu = kvm_fpu { fcw: FP_CONTROL_WORD_DEFAULT, ftwx: FP_TAG_WORD_DEFAULT, @@ -588,6 +589,24 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_fpu(&fpu)?; + // Set MXCSR from XSAVE (MXCSR is at byte offset 24 -> u32 index 6) + // Locations are from + // AMD64 Architecture Programmer's Manual, Volume 2 + // 11.5.10 Mode-Specific XSAVE/XRSTOR State Management + let mut xsave = match self.vcpu_fd.get_xsave() { + Ok(xsave) => xsave, + Err(e) => { + return Err(new_error!("Could not write guest registers: {:?}", e)); + } + }; + + xsave.region[6] = MXCSR_DEFAULT; + unsafe { + self.vcpu_fd + .set_xsave(&xsave) + .map_err(|e| new_error!("Could not write guest registers: {:?}", e))? + }; + // run VirtualCPU::run( self.as_mut_hypervisor(), diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index fc860caf6..89088c8d4 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -486,6 +486,7 @@ mod tests { use hyperlight_common::flatbuffer_wrappers::guest_error::ErrorCode; use hyperlight_testing::simple_guest_as_string; + use crate::hypervisor::fpu; #[cfg(target_os = "linux")] use crate::mem::memory_region::{MemoryRegion, MemoryRegionFlags, MemoryRegionType}; #[cfg(target_os = "linux")] @@ -1021,4 +1022,30 @@ mod tests { }; assert_ne!(sandbox3.id, sandbox_id); } + + /// Test that FPU control word and MXCSR register are properly reset between guest calls + #[test] + fn test_fpu_mxcsr_register_reset() { + let path = simple_guest_as_string().unwrap(); + let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); + let mut sandbox = u_sbox.evolve().unwrap(); + + // First, read the initial state of FPU and MXCSR registers + let initial_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); + // Verify the initial state matches expected default values + let expected = format!("fcw:{:04X},mxcsr:{:08X}", fpu::FP_CONTROL_WORD_DEFAULT, fpu::MXCSR_DEFAULT); + assert_eq!(initial_state, expected, + "Initial FPU/MXCSR state does not match expected defaults"); + + // Modify the FPU and MXCSR registers to non-default values + let modify_result: String = sandbox.call("ModifyFpuMxcsr", ()).unwrap(); + let expected = format!("fcw:{:04X},mxcsr:{:08X}", 0x027F, 0x1F00); + assert_eq!(modify_result, expected, + "Modified state does not match"); + + // A new call should reset these + let reset_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); + assert_eq!(initial_state, reset_state, + "FPU/MXCSR registers were not properly reset between calls"); + } } diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index aabe74dc5..f7b8ef97a 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -930,6 +930,73 @@ fn exec_mapped_buffer(function_call: &FunctionCall) -> Result> { } } +/// Simple function to modify FPU control word and MXCSR register +#[hyperlight_guest_tracing::trace_function] +fn modify_fpu_mxcsr(_function_call: &FunctionCall) -> Result> { + unsafe { + // Set FPU control word to a non-default value (default is usually 0x037F) + // We'll set it to 0x027F (change precision control and rounding) + let new_fcw: u16 = 0x027F; + core::arch::asm!( + "fldcw [{fcw_ptr}]", + fcw_ptr = in(reg) &new_fcw, + ); + + // Set MXCSR to a non-default value (default is usually 0x1F80) + // We'll set it to 0x1F00 (disable some exception masks) + let new_mxcsr: u32 = 0x1F00; + core::arch::asm!( + "ldmxcsr [{mxcsr_ptr}]", + mxcsr_ptr = in(reg) &new_mxcsr, + ); + } + + // now read them and return those values + let mut fcw: u16 = 0; + let mut mxcsr: u32 = 0; + + unsafe { + // Read FPU control word + core::arch::asm!( + "fnstcw [{fcw_ptr}]", + fcw_ptr = in(reg) &mut fcw, + ); + + // Read MXCSR + core::arch::asm!( + "stmxcsr [{mxcsr_ptr}]", + mxcsr_ptr = in(reg) &mut mxcsr, + ); + } + + let result = format!("fcw:{:04X},mxcsr:{:08X}", fcw, mxcsr); + Ok(get_flatbuffer_result(result.as_str())) +} + +/// Simple function to read current FPU control word and MXCSR values +#[hyperlight_guest_tracing::trace_function] +fn read_fpu_mxcsr(_function_call: &FunctionCall) -> Result> { + let mut fcw: u16 = 0; + let mut mxcsr: u32 = 0; + + unsafe { + // Read FPU control word + core::arch::asm!( + "fnstcw [{fcw_ptr}]", + fcw_ptr = in(reg) &mut fcw, + ); + + // Read MXCSR + core::arch::asm!( + "stmxcsr [{mxcsr_ptr}]", + mxcsr_ptr = in(reg) &mut mxcsr, + ); + } + + let result = format!("fcw:{:04X},mxcsr:{:08X}", fcw, mxcsr); + Ok(get_flatbuffer_result(result.as_str())) +} + #[no_mangle] #[hyperlight_guest_tracing::trace_function] pub extern "C" fn hyperlight_main() { @@ -1477,6 +1544,23 @@ pub extern "C" fn hyperlight_main() { call_given_paramless_hostfunc_that_returns_i64 as usize, ); register_function(call_given_hostfunc_def); + + // Register FPU and MXCSR test functions + let modify_fpu_mxcsr_def = GuestFunctionDefinition::new( + "ModifyFpuMxcsr".to_string(), + Vec::new(), + ReturnType::String, + modify_fpu_mxcsr as usize, + ); + register_function(modify_fpu_mxcsr_def); + + let read_fpu_mxcsr_def = GuestFunctionDefinition::new( + "ReadFpuMxcsr".to_string(), + Vec::new(), + ReturnType::String, + read_fpu_mxcsr as usize, + ); + register_function(read_fpu_mxcsr_def); } #[hyperlight_guest_tracing::trace_function] From 9242091d90a25bfa91e650606500ff427dc66d47 Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 23 Sep 2025 20:45:09 +0000 Subject: [PATCH 2/3] Update FPU control tag for xsave fmt and test Signed-off-by: James Sturtevant --- src/hyperlight_host/src/hypervisor/fpu.rs | 2 +- src/hyperlight_host/src/hypervisor/kvm.rs | 8 ++- .../src/sandbox/initialized_multi_use.rs | 40 +++++++++---- src/hyperlight_host/tests/integration_test.rs | 33 +++++++++++ src/tests/rust_guests/dummyguest/Cargo.lock | 4 +- src/tests/rust_guests/simpleguest/Cargo.lock | 4 +- src/tests/rust_guests/simpleguest/src/main.rs | 59 +++++++++++++++---- src/tests/rust_guests/witguest/Cargo.lock | 4 +- 8 files changed, 120 insertions(+), 34 deletions(-) diff --git a/src/hyperlight_host/src/hypervisor/fpu.rs b/src/hyperlight_host/src/hypervisor/fpu.rs index f0b6cb6a2..a02e62b2f 100644 --- a/src/hyperlight_host/src/hypervisor/fpu.rs +++ b/src/hyperlight_host/src/hypervisor/fpu.rs @@ -15,5 +15,5 @@ limitations under the License. */ pub(crate) const FP_CONTROL_WORD_DEFAULT: u16 = 0x37f; // mask all fp-exception, set rounding to nearest, set precision to 64-bit -pub(crate) const FP_TAG_WORD_DEFAULT: u8 = 0xff; // each 8 of x87 fpu registers is empty +pub(crate) const FP_TAG_WORD_DEFAULT: u8 = 0x00; // 11.5.10.1 FXSAVE Format for x87 Tag Word: A two-bit value of 11 is encoded as a 0, indicating the corresponding x87 FPRn is empty. pub(crate) const MXCSR_DEFAULT: u32 = 0x1f80; // mask simd fp-exceptions, clear exception flags, set rounding to nearest, disable flush-to-zero mode, disable denormals-are-zero mode diff --git a/src/hyperlight_host/src/hypervisor/kvm.rs b/src/hyperlight_host/src/hypervisor/kvm.rs index 29f2652f4..7b173713a 100644 --- a/src/hyperlight_host/src/hypervisor/kvm.rs +++ b/src/hyperlight_host/src/hypervisor/kvm.rs @@ -579,7 +579,8 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_regs(®s)?; - // note kvm set_fpu doesn't actually set or read the mxcsr value + // Note kvm set_fpu doesn't actually set or read the mxcsr value + // We reset it in a call below // https://elixir.bootlin.com/linux/v6.16/source/arch/x86/kvm/x86.c#L12229 let fpu = kvm_fpu { fcw: FP_CONTROL_WORD_DEFAULT, @@ -589,10 +590,11 @@ impl Hypervisor for KVMDriver { }; self.vcpu_fd.set_fpu(&fpu)?; - // Set MXCSR from XSAVE (MXCSR is at byte offset 24 -> u32 index 6) - // Locations are from + // Set MXCSR from XSAVE + // Locations are from // AMD64 Architecture Programmer's Manual, Volume 2 // 11.5.10 Mode-Specific XSAVE/XRSTOR State Management + // MXCSR is at byte offset 24 -> u32 index 6 let mut xsave = match self.vcpu_fd.get_xsave() { Ok(xsave) => xsave, Err(e) => { diff --git a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs index 89088c8d4..5679bc746 100644 --- a/src/hyperlight_host/src/sandbox/initialized_multi_use.rs +++ b/src/hyperlight_host/src/sandbox/initialized_multi_use.rs @@ -1023,29 +1023,43 @@ mod tests { assert_ne!(sandbox3.id, sandbox_id); } - /// Test that FPU control word and MXCSR register are properly reset between guest calls #[test] fn test_fpu_mxcsr_register_reset() { let path = simple_guest_as_string().unwrap(); let u_sbox = UninitializedSandbox::new(GuestBinary::FilePath(path), None).unwrap(); let mut sandbox = u_sbox.evolve().unwrap(); - // First, read the initial state of FPU and MXCSR registers + // The x87 Tag Word Register (FTW) all ones means empty state but we are setting it with xsave + // where the value should be all 00's for empty state + // https://github.com/hyperlight-dev/hyperlight/issues/904 + let expected = format!( + "fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", + fpu::FP_CONTROL_WORD_DEFAULT, + 0xFFFF, + fpu::MXCSR_DEFAULT + ); + + // First, read the initial state of FPU control word, tag word, and MXCSR registers let initial_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); - // Verify the initial state matches expected default values - let expected = format!("fcw:{:04X},mxcsr:{:08X}", fpu::FP_CONTROL_WORD_DEFAULT, fpu::MXCSR_DEFAULT); - assert_eq!(initial_state, expected, - "Initial FPU/MXCSR state does not match expected defaults"); - + assert_eq!( + initial_state, expected, + "Initial FPU/tag word/MXCSR state does not match expected defaults" + ); + // Modify the FPU and MXCSR registers to non-default values let modify_result: String = sandbox.call("ModifyFpuMxcsr", ()).unwrap(); - let expected = format!("fcw:{:04X},mxcsr:{:08X}", 0x027F, 0x1F00); - assert_eq!(modify_result, expected, - "Modified state does not match"); - + let modified_expected = + format!("fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", 0x027F, 0x1113, 0x1F00); + assert_eq!( + modify_result, modified_expected, + "Modified state does not match" + ); + // A new call should reset these let reset_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); - assert_eq!(initial_state, reset_state, - "FPU/MXCSR registers were not properly reset between calls"); + assert_eq!( + reset_state, expected, + "FPU/tag word/MXCSR registers were not properly reset between calls" + ); } } diff --git a/src/hyperlight_host/tests/integration_test.rs b/src/hyperlight_host/tests/integration_test.rs index 8b2077a19..d51095ffa 100644 --- a/src/hyperlight_host/tests/integration_test.rs +++ b/src/hyperlight_host/tests/integration_test.rs @@ -748,6 +748,39 @@ fn log_message() { assert_eq!(1, LOGGER.num_log_calls()); } +#[test] +fn test_fpu_mxcsr_snapshot_restore() { + let mut sandbox = new_uninit_rust().unwrap().evolve().unwrap(); + + // The x87 Tag Word Register (FTW) all ones means empty state but we are setting it with xsave + // where the value should be all 00's for empty state + // https://github.com/hyperlight-dev/hyperlight/issues/904 + let expected = format!("fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", 0x37F, 0xFFFF, 0x1F80); + + let initial_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); + assert_eq!( + initial_state, expected, + "Initial FPU/tag word/MXCSR state does not match expected defaults" + ); + + let snapshot = sandbox.snapshot().unwrap(); + + let modify_result: String = sandbox.call("ModifyFpuMxcsr", ()).unwrap(); + let modified_expected = format!("fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", 0x027F, 0x1113, 0x1F00); + assert_eq!( + modify_result, modified_expected, + "Modified state does not match expected values" + ); + + sandbox.restore(&snapshot).unwrap(); + + let restored_state: String = sandbox.call("ReadFpuMxcsr", ()).unwrap(); + assert_eq!( + restored_state, expected, + "FPU/tag word/MXCSR registers should be restored to defaults after snapshot restore" + ); +} + fn log_test_messages(levelfilter: Option) { LOGGER.clear_log_calls(); assert_eq!(0, LOGGER.num_log_calls()); diff --git a/src/tests/rust_guests/dummyguest/Cargo.lock b/src/tests/rust_guests/dummyguest/Cargo.lock index fa3de189b..dcdc90569 100644 --- a/src/tests/rust_guests/dummyguest/Cargo.lock +++ b/src/tests/rust_guests/dummyguest/Cargo.lock @@ -4,9 +4,9 @@ version = 4 [[package]] name = "anyhow" -version = "1.0.99" +version = "1.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100" +checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" [[package]] name = "autocfg" diff --git a/src/tests/rust_guests/simpleguest/Cargo.lock b/src/tests/rust_guests/simpleguest/Cargo.lock index 9081da46b..a327588e9 100644 --- a/src/tests/rust_guests/simpleguest/Cargo.lock +++ b/src/tests/rust_guests/simpleguest/Cargo.lock @@ -4,9 +4,9 @@ version = 4 [[package]] name = "anyhow" -version = "1.0.99" +version = "1.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100" +checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" [[package]] name = "autocfg" diff --git a/src/tests/rust_guests/simpleguest/src/main.rs b/src/tests/rust_guests/simpleguest/src/main.rs index f7b8ef97a..326fb5a29 100644 --- a/src/tests/rust_guests/simpleguest/src/main.rs +++ b/src/tests/rust_guests/simpleguest/src/main.rs @@ -941,7 +941,20 @@ fn modify_fpu_mxcsr(_function_call: &FunctionCall) -> Result> { "fldcw [{fcw_ptr}]", fcw_ptr = in(reg) &new_fcw, ); - + + // Load some FPU registers to change the tag word from default 0xFF + // This will mark some registers as valid, changing the tag word + // Sets up FPR with: 00 01 00 01 00 01 00 11 which is 0x1113; + core::arch::asm!( + "fld1", // ST(6) + "fldz", // ST(5) + "fld1", // ST(4) + "fldz", // ST(3) + "fld1", // ST(2) + "fldz", // ST(1) + "fld1", // ST(0) + ); + // Set MXCSR to a non-default value (default is usually 0x1F80) // We'll set it to 0x1F00 (disable some exception masks) let new_mxcsr: u32 = 0x1F00; @@ -951,49 +964,73 @@ fn modify_fpu_mxcsr(_function_call: &FunctionCall) -> Result> { ); } - // now read them and return those values let mut fcw: u16 = 0; + let ftw: u16; let mut mxcsr: u32 = 0; - + unsafe { // Read FPU control word core::arch::asm!( "fnstcw [{fcw_ptr}]", fcw_ptr = in(reg) &mut fcw, ); - + + // Read FPU status word and tag word using fnstenv + // fnstenv stores the complete FPU environment (28 bytes) + // The format is the the FTW format where 11 == empty + // This is different from the FXSAVE format + // https://github.com/hyperlight-dev/hyperlight/issues/904 + let mut fpu_env: [u8; 28] = [0; 28]; + core::arch::asm!( + "fnstenv [{env_ptr}]", + env_ptr = in(reg) &mut fpu_env, + ); + ftw = u16::from_le_bytes([fpu_env[8], fpu_env[9]]); + // Read MXCSR core::arch::asm!( "stmxcsr [{mxcsr_ptr}]", mxcsr_ptr = in(reg) &mut mxcsr, ); } - - let result = format!("fcw:{:04X},mxcsr:{:08X}", fcw, mxcsr); + + let result = format!("fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", fcw, ftw, mxcsr); Ok(get_flatbuffer_result(result.as_str())) } -/// Simple function to read current FPU control word and MXCSR values #[hyperlight_guest_tracing::trace_function] fn read_fpu_mxcsr(_function_call: &FunctionCall) -> Result> { let mut fcw: u16 = 0; + let ftw: u16; let mut mxcsr: u32 = 0; - + unsafe { // Read FPU control word core::arch::asm!( "fnstcw [{fcw_ptr}]", fcw_ptr = in(reg) &mut fcw, ); - + + // Read FPU status word and tag word using fnstenv + // fnstenv stores the complete FPU environment (28 bytes) + // The format is the the FTW format where 11 == empty + // This is different from the FXSAVE format + // https://github.com/hyperlight-dev/hyperlight/issues/904 + let mut fpu_env: [u8; 28] = [0; 28]; + core::arch::asm!( + "fnstenv [{env_ptr}]", + env_ptr = in(reg) &mut fpu_env, + ); + ftw = u16::from_le_bytes([fpu_env[8], fpu_env[9]]); + // Read MXCSR core::arch::asm!( "stmxcsr [{mxcsr_ptr}]", mxcsr_ptr = in(reg) &mut mxcsr, ); } - - let result = format!("fcw:{:04X},mxcsr:{:08X}", fcw, mxcsr); + + let result = format!("fcw:{:04X},ftw:{:02X},mxcsr:{:08X}", fcw, ftw, mxcsr); Ok(get_flatbuffer_result(result.as_str())) } diff --git a/src/tests/rust_guests/witguest/Cargo.lock b/src/tests/rust_guests/witguest/Cargo.lock index 6a33cb503..cc904b193 100644 --- a/src/tests/rust_guests/witguest/Cargo.lock +++ b/src/tests/rust_guests/witguest/Cargo.lock @@ -63,9 +63,9 @@ dependencies = [ [[package]] name = "anyhow" -version = "1.0.99" +version = "1.0.100" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b0674a1ddeecb70197781e945de4b3b8ffb61fa939a5597bcf48503737663100" +checksum = "a23eb6b1614318a8071c9b2521f36b424b2c83db5eb3a0fead4a6c0809af6e61" [[package]] name = "autocfg" From 9b82922355a7ed2c339affb6c6c855cfaaa3cddb Mon Sep 17 00:00:00 2001 From: James Sturtevant Date: Tue, 23 Sep 2025 21:00:27 +0000 Subject: [PATCH 3/3] Update typo word with FPR (floating point register) Signed-off-by: James Sturtevant --- typos.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/typos.toml b/typos.toml index c05ec808d..21a5d2e5a 100644 --- a/typos.toml +++ b/typos.toml @@ -8,3 +8,4 @@ extend-exclude = ["**/*.patch", "src/hyperlight_guest_bin/third_party/**/*", "NO # typ is used for field name as type is a reserved keyword typ="typ" mmaped="mmapped" +FPR="FPR"