Skip to content

Commit f7c0224

Browse files
committed
Make set_xsave take slice
Signed-off-by: Ludvig Liljenberg <4257730+ludfjig@users.noreply.github.com>
1 parent 5c7145d commit f7c0224

File tree

6 files changed

+176
-40
lines changed

6 files changed

+176
-40
lines changed

.github/workflows/ValidatePullRequest.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ jobs:
6767
- docs-pr
6868
- build-guests
6969
strategy:
70-
fail-fast: true
70+
fail-fast: false
7171
matrix:
7272
hypervisor: [hyperv, 'hyperv-ws2025', mshv3, kvm]
7373
cpu: [amd, intel]

src/hyperlight_host/src/hypervisor/hyperlight_vm.rs

Lines changed: 142 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -1118,6 +1118,7 @@ mod debug {
11181118

11191119
#[cfg(test)]
11201120
#[cfg(feature = "init-paging")]
1121+
#[allow(clippy::needless_range_loop)]
11211122
mod tests {
11221123
use std::sync::{Arc, Mutex};
11231124

@@ -1294,24 +1295,98 @@ mod tests {
12941295
}
12951296
}
12961297

1297-
/// Build a dirty XSAVE area for testing reset_vcpu.
1298-
/// Can't use all 1s because XSAVE has reserved fields that must be zero
1299-
/// (header at 512-575, MXCSR reserved bits, etc.)
1298+
/// Query CPUID.0DH.n for XSAVE component info.
1299+
/// Returns (size, offset, align_64) for the given component:
1300+
/// - size: CPUID.0DH.n:EAX - size in bytes
1301+
/// - offset: CPUID.0DH.n:EBX - offset from XSAVE base (standard format only)
1302+
/// - align_64: CPUID.0DH.n:ECX bit 1 - true if 64-byte aligned (compacted format)
1303+
fn xsave_component_info(comp_id: u32) -> (usize, usize, bool) {
1304+
let result = unsafe { std::arch::x86_64::__cpuid_count(0xD, comp_id) };
1305+
let size = result.eax as usize;
1306+
let offset = result.ebx as usize;
1307+
let align_64 = (result.ecx & 0b10) != 0;
1308+
(size, offset, align_64)
1309+
}
1310+
1311+
/// Query CPUID.0DH.00H for the bitmap of supported user state components.
1312+
/// EDX:EAX forms a 64-bit bitmap where bit i indicates support for component i.
1313+
fn xsave_supported_components() -> u64 {
1314+
let result = unsafe { std::arch::x86_64::__cpuid_count(0xD, 0) };
1315+
(result.edx as u64) << 32 | (result.eax as u64)
1316+
}
1317+
1318+
/// Dirty extended state components using compacted XSAVE format (MSHV/WHP).
1319+
/// Components are stored contiguously starting at byte 576, with alignment
1320+
/// requirements from CPUID.0DH.n:ECX[1].
1321+
fn dirty_xsave_extended_compacted(xsave: &mut [u32], xcomp_bv: u64, supported_components: u64) {
1322+
let mut offset = 576usize;
1323+
1324+
for comp_id in 2..63u32 {
1325+
// Skip if component not supported by CPU or not enabled in XCOMP_BV
1326+
if (supported_components & (1u64 << comp_id)) == 0 {
1327+
continue;
1328+
}
1329+
if (xcomp_bv & (1u64 << comp_id)) == 0 {
1330+
continue;
1331+
}
1332+
1333+
let (size, _, align_64) = xsave_component_info(comp_id);
1334+
1335+
// ECX[1]=1 means 64-byte aligned; ECX[1]=0 means immediately after previous
1336+
if align_64 {
1337+
offset = offset.next_multiple_of(64);
1338+
}
1339+
1340+
// Dirty this component's data area
1341+
let start_idx = offset / 4;
1342+
let end_idx = (offset + size) / 4;
1343+
if end_idx <= xsave.len() {
1344+
for i in start_idx..end_idx {
1345+
xsave[i] = 0x12345678 ^ comp_id.wrapping_mul(0x11111111);
1346+
}
1347+
}
1348+
1349+
offset += size;
1350+
}
1351+
}
1352+
1353+
/// Dirty extended state components using standard XSAVE format (KVM).
1354+
/// Components are at fixed offsets from CPUID.0DH.n:EBX.
13001355
///
1301-
/// Takes the current xsave state to preserve hypervisor-specific header fields
1302-
/// like XCOMP_BV which MSHV/WHP require to be set correctly.
1356+
/// In standard format, space is always allocated at the fixed offset for ALL
1357+
/// CPU-supported components, regardless of XSTATE_BV or XCOMP_BV values.
1358+
/// XSTATE_BV only tells the CPU which components to restore on XRSTOR, not
1359+
/// which have allocated space in the buffer.
1360+
fn dirty_xsave_extended_standard(xsave: &mut [u32], supported_components: u64) {
1361+
for comp_id in 2..63u32 {
1362+
// Skip if component not supported by CPU
1363+
if (supported_components & (1u64 << comp_id)) == 0 {
1364+
continue;
1365+
}
1366+
1367+
let (size, fixed_offset, _) = xsave_component_info(comp_id);
1368+
1369+
let start_idx = fixed_offset / 4;
1370+
let end_idx = (fixed_offset + size) / 4;
1371+
if end_idx <= xsave.len() {
1372+
for i in start_idx..end_idx {
1373+
xsave[i] = 0x12345678 ^ comp_id.wrapping_mul(0x11111111);
1374+
}
1375+
}
1376+
}
1377+
}
1378+
1379+
/// Dirty the legacy XSAVE region (bytes 0-511) for testing reset_vcpu.
1380+
/// This includes FPU/x87 state, SSE state, and reserved areas.
13031381
///
13041382
/// Layout (from Intel SDM Table 13-1):
13051383
/// Bytes 0-1: FCW, 2-3: FSW, 4: FTW, 5: reserved, 6-7: FOP
13061384
/// Bytes 8-15: FIP, 16-23: FDP
1307-
/// Bytes 24-27: MXCSR, 28-31: MXCSR_MASK (don't touch)
1308-
/// Bytes 32-159: ST0-ST7/MM0-MM7 (8 regs × 16 bytes, only 10 bytes used per reg)
1385+
/// Bytes 24-27: MXCSR, 28-31: MXCSR_MASK (preserve - hardware defined)
1386+
/// Bytes 32-159: ST0-ST7/MM0-MM7 (8 regs × 16 bytes)
13091387
/// Bytes 160-415: XMM0-XMM15 (16 regs × 16 bytes)
13101388
/// Bytes 416-511: Reserved
1311-
/// Bytes 512-575: XSAVE header (preserve XCOMP_BV at 520-527)
1312-
fn dirty_xsave(current_xsave: &[u8]) -> [u32; 1024] {
1313-
let mut xsave = [0u32; 1024];
1314-
1389+
fn dirty_xsave_legacy(xsave: &mut [u32], current_xsave: &[u8]) {
13151390
// FCW (bytes 0-1) + FSW (bytes 2-3) - pack into xsave[0]
13161391
// FCW = 0x0F7F (different from default 0x037F), FSW = 0x1234
13171392
xsave[0] = 0x0F7F | (0x1234 << 16);
@@ -1326,34 +1401,71 @@ mod tests {
13261401
xsave[5] = 0xBABE0004;
13271402
// MXCSR (bytes 24-27) - xsave[6], use valid value different from default
13281403
xsave[6] = 0x3F80;
1329-
// xsave[7] is MXCSR_MASK - preserve from current
1404+
// xsave[7] is MXCSR_MASK - preserve from current (hardware defined, read-only)
13301405
if current_xsave.len() >= 32 {
13311406
xsave[7] = u32::from_le_bytes(current_xsave[28..32].try_into().unwrap());
13321407
}
13331408

13341409
// ST0-ST7/MM0-MM7 (bytes 32-159, indices 8-39)
1335-
for item in xsave.iter_mut().take(40).skip(8) {
1336-
*item = 0xCAFEBABE;
1410+
for i in 8..40 {
1411+
xsave[i] = 0xCAFEBABE;
13371412
}
13381413
// XMM0-XMM15 (bytes 160-415, indices 40-103)
1339-
for item in xsave.iter_mut().take(104).skip(40) {
1340-
*item = 0xDEADBEEF;
1414+
for i in 40..104 {
1415+
xsave[i] = 0xDEADBEEF;
13411416
}
13421417

1343-
// Preserve entire XSAVE header (bytes 512-575, indices 128-143) from current state
1344-
// This includes XSTATE_BV (512-519) and XCOMP_BV (520-527) which
1345-
// MSHV/WHP require to be set correctly for compacted format.
1346-
// Failure to do this will cause set_xsave to fail on MSHV.
1347-
if current_xsave.len() >= 576 {
1348-
#[allow(clippy::needless_range_loop)]
1349-
for i in 128..144 {
1350-
let byte_offset = i * 4;
1351-
xsave[i] = u32::from_le_bytes(
1352-
current_xsave[byte_offset..byte_offset + 4]
1353-
.try_into()
1354-
.unwrap(),
1355-
);
1356-
}
1418+
// Reserved area (bytes 416-511, indices 104-127)
1419+
for i in 104..128 {
1420+
xsave[i] = 0xABCDEF12;
1421+
}
1422+
}
1423+
1424+
/// Preserve XSAVE header (bytes 512-575) from current state.
1425+
/// This includes XSTATE_BV and XCOMP_BV which hypervisors require.
1426+
fn preserve_xsave_header(xsave: &mut [u32], current_xsave: &[u8]) {
1427+
for i in 128..144 {
1428+
let byte_offset = i * 4;
1429+
xsave[i] = u32::from_le_bytes(
1430+
current_xsave[byte_offset..byte_offset + 4]
1431+
.try_into()
1432+
.unwrap(),
1433+
);
1434+
}
1435+
}
1436+
1437+
/// Build a dirty XSAVE area for testing reset_vcpu.
1438+
fn dirty_xsave(current_xsave: &[u8]) -> Vec<u32> {
1439+
let mut xsave = vec![0u32; current_xsave.len() / 4];
1440+
1441+
dirty_xsave_legacy(&mut xsave, current_xsave);
1442+
preserve_xsave_header(&mut xsave, current_xsave);
1443+
1444+
// Dirty extended state areas (bytes 576+)
1445+
// Parse XCOMP_BV (bytes 520-527) to determine format and enabled components.
1446+
// Bit 63 = compacted format, bits 0-62 = enabled components.
1447+
let xcomp_bv = u64::from_le_bytes(current_xsave[520..528].try_into().unwrap());
1448+
let supported_components = xsave_supported_components();
1449+
1450+
if (xcomp_bv & (1u64 << 63)) != 0 {
1451+
// Compacted format (MSHV/WHP)
1452+
dirty_xsave_extended_compacted(&mut xsave, xcomp_bv, supported_components);
1453+
1454+
// UPDATE XSTATE_BV to indicate dirtied components have valid data.
1455+
// WHP validates consistency between XSTATE_BV and actual data in the buffer.
1456+
// Bits 0,1 = legacy x87/SSE (always set after dirty_xsave_legacy)
1457+
// Bits 2+ = extended components from XCOMP_BV that we dirtied
1458+
let extended_mask = xcomp_bv & !((1u64 << 63) | 0x3);
1459+
let xstate_bv = 0x3 | extended_mask;
1460+
1461+
// Write XSTATE_BV to bytes 512-519 (u32 indices 128-129)
1462+
xsave[128] = (xstate_bv & 0xFFFFFFFF) as u32;
1463+
xsave[129] = (xstate_bv >> 32) as u32;
1464+
} else {
1465+
// Standard format (KVM)
1466+
// KVM doesn't validate XSTATE_BV vs data consistency, and rejects XSTATE_BV
1467+
// with bits set for components that don't fit in the buffer. Keep original.
1468+
dirty_xsave_extended_standard(&mut xsave, supported_components);
13571469
}
13581470

13591471
xsave

src/hyperlight_host/src/hypervisor/hyperv_linux.rs

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -252,14 +252,26 @@ impl Hypervisor for MshvVm {
252252

253253
#[cfg(test)]
254254
#[cfg(feature = "init-paging")]
255-
fn set_xsave(&self, xsave: &[u32; 1024]) -> Result<()> {
256-
let mut buf = XSave::default();
255+
fn set_xsave(&self, xsave: &[u32]) -> Result<()> {
256+
const MSHV_XSAVE_SIZE: usize = 4096;
257+
if std::mem::size_of_val(xsave) != MSHV_XSAVE_SIZE {
258+
return Err(new_error!(
259+
"Provided xsave size {} does not match MSHV supported size {}",
260+
std::mem::size_of_val(xsave),
261+
MSHV_XSAVE_SIZE
262+
));
263+
}
264+
257265
// Safety: all valid u32 values are 4 valid u8 values
258266
let (prefix, bytes, suffix) = unsafe { xsave.align_to() };
259267
if !prefix.is_empty() || !suffix.is_empty() {
260268
return Err(new_error!("Invalid xsave buffer alignment"));
261269
}
262-
buf.buffer.copy_from_slice(bytes);
270+
let buf = XSave {
271+
buffer: bytes
272+
.try_into()
273+
.map_err(|_| new_error!("mshv xsave must be 4096 u8s"))?,
274+
};
263275
self.vcpu_fd.set_xsave(&buf)?;
264276
Ok(())
265277
}

src/hyperlight_host/src/hypervisor/hyperv_windows.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,7 @@ impl Hypervisor for WhpVm {
556556

557557
#[cfg(test)]
558558
#[cfg(feature = "init-paging")]
559-
fn set_xsave(&self, xsave: &[u32; 1024]) -> Result<()> {
559+
fn set_xsave(&self, xsave: &[u32]) -> Result<()> {
560560
use crate::HyperlightError;
561561

562562
// Get the required buffer size by calling with NULL buffer.

src/hyperlight_host/src/hypervisor/kvm.rs

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,10 +207,21 @@ impl Hypervisor for KvmVm {
207207

208208
#[cfg(test)]
209209
#[cfg(feature = "init-paging")]
210-
fn set_xsave(&self, xsave: &[u32; 1024]) -> Result<()> {
210+
fn set_xsave(&self, xsave: &[u32]) -> Result<()> {
211+
const KVM_XSAVE_SIZE: usize = 4096;
212+
213+
if std::mem::size_of_val(xsave) != KVM_XSAVE_SIZE {
214+
return Err(new_error!(
215+
"Provided xsave size {} does not match KVM supported size {}",
216+
std::mem::size_of_val(xsave),
217+
KVM_XSAVE_SIZE
218+
));
219+
}
211220
let xsave = kvm_xsave {
212-
region: *xsave,
213-
..Default::default() // zeroed 4KB buffer with no FAM
221+
region: xsave
222+
.try_into()
223+
.map_err(|_| new_error!("kvm xsave must be 1024 u32s"))?,
224+
..Default::default()
214225
};
215226
// Safety: Safe because we only copy 4096 bytes
216227
// and have not enabled any dynamic xsave features

src/hyperlight_host/src/hypervisor/mod.rs

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,11 @@ pub(crate) trait Hypervisor: Debug + Send {
131131
fn xsave(&self) -> Result<Vec<u8>>;
132132
/// Reset xsave to default state
133133
fn reset_xsave(&self) -> Result<()>;
134-
/// Set xsave - only used for tests
134+
/// Set xsave - only used for tests.
135+
/// Note: On MSHV/WHP, XCOMP_BV must be preserved.
135136
#[cfg(test)]
136137
#[cfg(feature = "init-paging")]
137-
fn set_xsave(&self, xsave: &[u32; 1024]) -> Result<()>;
138+
fn set_xsave(&self, xsave: &[u32]) -> Result<()>;
138139

139140
/// Get the debug registers of the vCPU
140141
#[allow(dead_code)]

0 commit comments

Comments
 (0)