From f8e9054d764eba7a0443916c6940058a7d512d8a Mon Sep 17 00:00:00 2001 From: Nuno Das Neves Date: Wed, 20 Mar 2024 22:50:27 +0000 Subject: [PATCH] ioctls/bindings: Implement new MSHV_GET/SET_VP_STATE ioctls Rework XSave and LapicState, replace their From implementations to convert to/from Buffer with TryFrom. Replace existing MSHV_GET/SET_VP_STATE implementations with the new kernel interface. Signed-off-by: Nuno Das Neves --- mshv-bindings/src/regs.rs | 222 ++++++++++--------------------- mshv-bindings/src/serializers.rs | 4 +- mshv-ioctls/src/ioctls/vcpu.rs | 58 ++++---- mshv-ioctls/src/ioctls/vm.rs | 7 +- mshv-ioctls/src/mshv_ioctls.rs | 4 +- 5 files changed, 107 insertions(+), 188 deletions(-) diff --git a/mshv-bindings/src/regs.rs b/mshv-bindings/src/regs.rs index ec844b83..2777f3cb 100644 --- a/mshv-bindings/src/regs.rs +++ b/mshv-bindings/src/regs.rs @@ -6,12 +6,15 @@ use crate::bindings::*; #[cfg(feature = "with-serde")] use serde_derive::{Deserialize, Serialize}; -use std::cmp; use std::fmt; use std::ptr; +use std::convert::TryFrom; use vmm_sys_util::errno; use zerocopy::{AsBytes, FromBytes, FromZeroes}; +const PAGE_SIZE: usize = 0x1000; +const PAGE_SHIFT: usize = 12; + #[repr(C)] #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, AsBytes, FromBytes, FromZeroes)] #[cfg_attr(feature = "with-serde", derive(Deserialize, Serialize))] @@ -443,7 +446,7 @@ impl Buffer { Ok(buf) } - pub fn dealloc(self) { + pub fn dealloc(&mut self) { // SAFETY: buf was allocated with layout unsafe { std::alloc::dealloc(self.buf, self.layout); @@ -457,42 +460,27 @@ impl Buffer { impl Drop for Buffer { fn drop(&mut self) { - // SAFETY: buf was allocated with layout - unsafe { - std::alloc::dealloc(self.buf, self.layout); - } + self.dealloc(); } } #[repr(C)] #[derive(Copy, Clone, Debug, AsBytes, FromBytes, FromZeroes)] +/// Fixed buffer for lapic state pub struct LapicState { - pub regs: [::std::os::raw::c_char; 1024usize], + pub regs: [u8; 1024usize], } impl Default for LapicState { fn default() -> Self { unsafe { ::std::mem::zeroed() } } } -/* -impl Default for hv_register_value { - fn default() -> Self { - unsafe { ::std::mem::zeroed() } - } -} */ + #[repr(C)] #[derive(Copy, Clone, Debug, AsBytes, FromBytes, FromZeroes)] -/// This struct normalizes the actual mhsv XSave structure -/// XSave only used in save and restore functionalities, serilization and -/// deserialization are needed. Putting all the fields into a single buffer makes -/// it easier to serialize and deserialize -/// Total buffer size: 4120 -/// flags: 8 bytes -/// states: 8 bytes -/// data_size: 8 bytes -/// Actual xsave buffer: 4096 bytes +/// Fixed buffer for xsave state pub struct XSave { - pub buffer: [::std::os::raw::c_char; 4120usize], + pub buffer: [u8; 4096usize], } impl Default for XSave { @@ -501,69 +489,45 @@ impl Default for XSave { } } -impl From for XSave { - fn from(reg: mshv_vp_state) -> Self { - let ret = XSave { +impl TryFrom for XSave { + type Error = errno::Error; + fn try_from(buf: Buffer) -> Result { + let mut ret = XSave { ..Default::default() }; - let mut bs = reg.xsave.flags.to_le_bytes(); - unsafe { - ptr::copy( - bs.as_ptr() as *mut u8, - ret.buffer.as_ptr().offset(0) as *mut u8, - 8, - ) - }; - bs = unsafe { reg.xsave.states.as_uint64.to_le_bytes() }; - unsafe { - ptr::copy( - bs.as_ptr() as *mut u8, - ret.buffer.as_ptr().offset(8) as *mut u8, - 8, - ) - }; - bs = reg.buf_size.to_le_bytes(); - unsafe { - ptr::copy( - bs.as_ptr() as *mut u8, - ret.buffer.as_ptr().offset(16) as *mut u8, - 8, - ) - }; - let min: usize = cmp::min(4096, reg.buf_size as u32) as usize; - unsafe { - ptr::copy( - reg.buf.bytes, - ret.buffer.as_ptr().offset(24) as *mut u8, - min, - ) - }; - ret + let ret_size = std::mem::size_of_val(&ret.buffer); + if ret_size < buf.size() { + return Err(errno::Error::new(libc::EINVAL)); + } + // SAFETY: ret is large enough to hold buffer + unsafe { ptr::copy(buf.buf, ret.buffer.as_mut_ptr(), buf.size()) }; + Ok(ret) } } -impl From for mshv_vp_state { - fn from(reg: XSave) -> Self { - let flags = reg.flags(); - let states = reg.states(); - let buffer_size = reg.data_size(); - let mut ret = mshv_vp_state { - type_: hv_get_set_vp_state_type_HV_GET_SET_VP_STATE_XSAVE, - buf_size: buffer_size, - ..Default::default() - }; - ret.xsave.flags = flags; - ret.xsave.states.as_uint64 = states; - ret.buf.bytes = reg.data_buffer() as *mut u8; - ret +impl TryFrom<&XSave> for Buffer { + type Error = errno::Error; + fn try_from(reg: &XSave) -> Result { + let reg_size = std::mem::size_of_val(®.buffer); + let num_pages = (reg_size + PAGE_SIZE - 1) >> PAGE_SHIFT; + let buffer = Buffer::new(num_pages * PAGE_SIZE, PAGE_SIZE)?; + // SAFETY: buffer is large enough to hold reg + unsafe { ptr::copy(reg.buffer.as_ptr(), buffer.buf, reg_size) }; + Ok(buffer) } } -impl From for LapicState { - fn from(reg: mshv_vp_state) -> Self { + +impl TryFrom for LapicState { + type Error = errno::Error; + fn try_from(buf: Buffer) -> Result { let mut ret: LapicState = LapicState::default(); let state = ret.regs.as_mut_ptr(); - let hv_state = unsafe { *reg.buf.lapic }; + if buf.size() < std::mem::size_of::() { + return Err(errno::Error::new(libc::EINVAL)); + } + // SAFETY: buf is large enough for hv_local_interrupt_controller_state unsafe { + let hv_state = &*(buf.buf as *const hv_local_interrupt_controller_state); *(state.offset(LOCAL_APIC_OFFSET_APIC_ID) as *mut u32) = hv_state.apic_id; *(state.offset(LOCAL_APIC_OFFSET_VERSION) as *mut u32) = hv_state.apic_version; *(state.offset(LOCAL_APIC_OFFSET_REMOTE_READ) as *mut u32) = hv_state.apic_remote_read; @@ -585,11 +549,9 @@ impl From for LapicState { hv_state.apic_counter_value; *(state.offset(LOCAL_APIC_OFFSET_DIVIDER) as *mut u32) = hv_state.apic_divide_configuration; - } - /* vectors ISR TMR IRR */ - for i in 0..8 { - unsafe { + /* vectors ISR TMR IRR */ + for i in 0..8 { *(state.offset(LOCAL_APIC_OFFSET_ISR + i * 16) as *mut u32) = hv_state.apic_isr[i as usize]; *(state.offset(LOCAL_APIC_OFFSET_TMR + i * 16) as *mut u32) = @@ -597,33 +559,36 @@ impl From for LapicState { *(state.offset(LOCAL_APIC_OFFSET_IRR + i * 16) as *mut u32) = hv_state.apic_irr[i as usize]; } - } - // Highest priority interrupt (isr = in service register) this is how WHP computes it - let mut isrv: u32 = 0; - for i in (0..8).rev() { - let val: u32 = hv_state.apic_isr[i as usize]; - if val != 0 { - isrv = 31 - val.leading_zeros(); // index of most significant set bit - isrv += i * 4 * 8; // i don't know - break; + // Highest priority interrupt (isr = in service register) this is how WHP computes it + let mut isrv: u32 = 0; + for i in (0..8).rev() { + let val: u32 = hv_state.apic_isr[i as usize]; + if val != 0 { + isrv = 31 - val.leading_zeros(); // index of most significant set bit + isrv += i * 4 * 8; // i don't know + break; + } } - } - // TODO This is meant to be max(tpr, isrv), but tpr is not populated! - unsafe { + // TODO This is meant to be max(tpr, isrv), but tpr is not populated! *(state.offset(LOCAL_APIC_OFFSET_PPR) as *mut u32) = isrv; } - ret + Ok(ret) } } -impl From for mshv_vp_state { - fn from(reg: LapicState) -> Self { - let state = reg.regs.as_ptr(); - let mut vp_state: mshv_vp_state = mshv_vp_state::default(); +impl TryFrom<&LapicState> for Buffer { + type Error = errno::Error; + fn try_from(reg: &LapicState) -> Result { + let hv_state_size = std::mem::size_of::(); + let num_pages = (hv_state_size + PAGE_SIZE - 1) >> PAGE_SHIFT; + let buffer = Buffer::new(num_pages * PAGE_SIZE, 0x1000)?; + // SAFETY: buf is large enough for hv_local_interrupt_controller_state unsafe { - let mut lapic_state = hv_local_interrupt_controller_state { + let state = reg.regs.as_ptr(); + let hv_state = &mut *(buffer.buf as *mut hv_local_interrupt_controller_state); + *hv_state = hv_local_interrupt_controller_state { apic_id: *(state.offset(LOCAL_APIC_OFFSET_APIC_ID) as *mut u32), apic_version: *(state.offset(LOCAL_APIC_OFFSET_VERSION) as *mut u32), apic_remote_read: *(state.offset(LOCAL_APIC_OFFSET_REMOTE_READ) as *mut u32), @@ -651,75 +616,30 @@ impl From for mshv_vp_state { /* vectors ISR TMR IRR */ for i in 0..8 { - lapic_state.apic_isr[i as usize] = + hv_state.apic_isr[i as usize] = *(state.offset(LOCAL_APIC_OFFSET_ISR + i * 16) as *mut u32); - lapic_state.apic_tmr[i as usize] = + hv_state.apic_tmr[i as usize] = *(state.offset(LOCAL_APIC_OFFSET_TMR + i * 16) as *mut u32); - lapic_state.apic_irr[i as usize] = + hv_state.apic_irr[i as usize] = *(state.offset(LOCAL_APIC_OFFSET_IRR + i * 16) as *mut u32); } - vp_state.type_ = - hv_get_set_vp_state_type_HV_GET_SET_VP_STATE_LOCAL_INTERRUPT_CONTROLLER_STATE; - vp_state.buf_size = 1024; - let boxed_obj = Box::new(lapic_state); - vp_state.buf.lapic = Box::into_raw(boxed_obj); } - vp_state + + Ok(buffer) } } + // implement `Display` for `XSave` impl fmt::Display for XSave { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, - "flags: {}, states: {}, buffer_size: {}, buffer: {:?}\n data: {:02X?}", - self.flags(), - self.states(), - self.data_size(), - self.data_buffer(), + "buffer: {:?}\n data: {:02X?}", + self.buffer.as_ptr(), self.buffer, ) } } -// Implement XSave to retrieve each field from the buffer -impl XSave { - pub fn flags(&self) -> u64 { - let array: [u8; 8] = [0, 0, 0, 0, 0, 0, 0, 0]; - unsafe { - ptr::copy( - self.buffer.as_ptr().offset(0) as *mut u8, - array.as_ptr() as *mut u8, - 8, - ) - }; - u64::from_le_bytes(array) - } - pub fn states(&self) -> u64 { - let array: [u8; 8] = [0, 0, 0, 0, 0, 0, 0, 0]; - unsafe { - ptr::copy( - self.buffer.as_ptr().offset(8) as *mut u8, - array.as_ptr() as *mut u8, - 8, - ) - }; - u64::from_le_bytes(array) - } - pub fn data_size(&self) -> u64 { - let array: [u8; 8] = [0, 0, 0, 0, 0, 0, 0, 0]; - unsafe { - ptr::copy( - self.buffer.as_ptr().offset(16) as *mut u8, - array.as_ptr() as *mut u8, - 8, - ) - }; - u64::from_le_bytes(array) - } - pub fn data_buffer(&self) -> *const u8 { - unsafe { self.buffer.as_ptr().offset(24) as *mut u8 } - } -} #[repr(C)] #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, AsBytes, FromBytes, FromZeroes)] diff --git a/mshv-bindings/src/serializers.rs b/mshv-bindings/src/serializers.rs index eb906bd0..d59939e1 100644 --- a/mshv-bindings/src/serializers.rs +++ b/mshv-bindings/src/serializers.rs @@ -14,7 +14,7 @@ impl<'de> Deserialize<'de> for LapicState { where D: Deserializer<'de>, { - let regs: Vec<::std::os::raw::c_char> = Vec::deserialize(deserializer)?; + let regs: Vec = Vec::deserialize(deserializer)?; let mut val = LapicState::default(); // This panics if the source and destination have different lengths. val.regs.copy_from_slice(®s[..]); @@ -36,7 +36,7 @@ impl<'de> Deserialize<'de> for XSave { where D: Deserializer<'de>, { - let data_buffer: Vec<::std::os::raw::c_char> = Vec::deserialize(deserializer)?; + let data_buffer: Vec = Vec::deserialize(deserializer)?; let mut val = XSave::default(); // This panics if the source and destination have different lengths. val.buffer.copy_from_slice(&data_buffer[..]); diff --git a/mshv-ioctls/src/ioctls/vcpu.rs b/mshv-ioctls/src/ioctls/vcpu.rs index dbb0d3ee..1e3f75ca 100644 --- a/mshv-ioctls/src/ioctls/vcpu.rs +++ b/mshv-ioctls/src/ioctls/vcpu.rs @@ -5,10 +5,9 @@ use crate::ioctls::Result; use crate::mshv_ioctls::*; use mshv_bindings::*; -use std::cmp; use std::fs::File; use std::os::unix::io::{AsRawFd, RawFd}; -use std::ptr; +use std::convert::TryFrom; #[cfg(test)] use std::slice; use vmm_sys_util::errno; @@ -816,7 +815,7 @@ impl VcpuFd { }]) } /// Returns the VCpu state. This IOCTLs can be used to get XSave and LAPIC state. - pub fn get_vp_state_ioctl(&self, state: &mut mshv_vp_state) -> Result<()> { + pub fn get_vp_state_ioctl(&self, state: &mut mshv_get_set_vp_state) -> Result<()> { // SAFETY: we know that our file is a vCPU fd and we verify the return result. let ret = unsafe { ioctl_with_mut_ref(self, MSHV_GET_VP_STATE(), state) }; if ret != 0 { @@ -826,7 +825,7 @@ impl VcpuFd { } /// Set vp states (LAPIC, XSave etc) /// Test code already covered by get/set_lapic/xsave - pub fn set_vp_state_ioctl(&self, state: &mshv_vp_state) -> Result<()> { + pub fn set_vp_state_ioctl(&self, state: &mshv_get_set_vp_state) -> Result<()> { // SAFETY: IOCTL call with correct types let ret = unsafe { ioctl_with_ref(self, MSHV_SET_VP_STATE(), state) }; if ret != 0 { @@ -837,46 +836,45 @@ impl VcpuFd { /// Get the state of the LAPIC (Local Advanced Programmable Interrupt Controller). pub fn get_lapic(&self) -> Result { let buffer = Buffer::new(0x1000, 0x1000)?; - let mut vp_state: mshv_vp_state = mshv_vp_state::default(); - vp_state.buf.bytes = buffer.buf; - vp_state.buf_size = buffer.size() as u64; - vp_state.type_ = - hv_get_set_vp_state_type_HV_GET_SET_VP_STATE_LOCAL_INTERRUPT_CONTROLLER_STATE; + let mut vp_state: mshv_get_set_vp_state = mshv_get_set_vp_state::default(); + vp_state.buf_ptr = buffer.buf as u64; + vp_state.buf_sz = buffer.size() as u32; + vp_state.type_ = MSHV_VP_STATE_LAPIC as u8; self.get_vp_state_ioctl(&mut vp_state)?; - Ok(LapicState::from(vp_state)) + LapicState::try_from(buffer) } /// Sets the state of the LAPIC (Local Advanced Programmable Interrupt Controller). pub fn set_lapic(&self, lapic_state: &LapicState) -> Result<()> { - let mut vp_state: mshv_vp_state = mshv_vp_state::from(*lapic_state); - let buffer = Buffer::new(0x1000, 0x1000)?; - let min: usize = cmp::min(buffer.size(), vp_state.buf_size as usize); - // SAFETY: src and dest are valid and properly aligned - unsafe { ptr::copy(vp_state.buf.bytes, buffer.buf, min) }; - vp_state.buf_size = buffer.size() as u64; - vp_state.buf.bytes = buffer.buf; + let buffer = Buffer::try_from(lapic_state)?; + let vp_state = mshv_get_set_vp_state { + type_: MSHV_VP_STATE_LAPIC as u8, + buf_sz: buffer.size() as u32, + buf_ptr: buffer.buf as u64, + ..Default::default() + }; self.set_vp_state_ioctl(&vp_state) } /// Returns the xsave data pub fn get_xsave(&self) -> Result { let buffer = Buffer::new(0x1000, 0x1000)?; - let mut vp_state: mshv_vp_state = mshv_vp_state::default(); - vp_state.buf.bytes = buffer.buf; - vp_state.buf_size = buffer.size() as u64; - vp_state.type_ = hv_get_set_vp_state_type_HV_GET_SET_VP_STATE_XSAVE; + let mut vp_state: mshv_get_set_vp_state = mshv_get_set_vp_state::default(); + vp_state.buf_ptr = buffer.buf as u64; + vp_state.buf_sz = buffer.size() as u32; + vp_state.type_ = MSHV_VP_STATE_XSAVE as u8; + self.get_vp_state_ioctl(&mut vp_state)?; - let ret = XSave::from(vp_state); - Ok(ret) + XSave::try_from(buffer) } /// Set the xsave data pub fn set_xsave(&self, data: &XSave) -> Result<()> { - let mut vp_state: mshv_vp_state = mshv_vp_state::from(*data); - let buffer = Buffer::new(0x1000, 0x1000)?; - let min: usize = cmp::min(buffer.size(), vp_state.buf_size as usize); - // SAFETY: src and dest are valid and properly aligned - unsafe { ptr::copy(data.buffer.as_ptr().offset(24) as *mut u8, buffer.buf, min) }; - vp_state.buf_size = buffer.size() as u64; - vp_state.buf.bytes = buffer.buf; + let buffer = Buffer::try_from(data)?; + let vp_state = mshv_get_set_vp_state { + type_: MSHV_VP_STATE_XSAVE as u8, + buf_sz: buffer.size() as u32, + buf_ptr: buffer.buf as u64, + ..Default::default() + }; self.set_vp_state_ioctl(&vp_state) } /// Translate guest virtual address to guest physical address diff --git a/mshv-ioctls/src/ioctls/vm.rs b/mshv-ioctls/src/ioctls/vm.rs index 7c676a06..0bccf915 100644 --- a/mshv-ioctls/src/ioctls/vm.rs +++ b/mshv-ioctls/src/ioctls/vm.rs @@ -223,6 +223,7 @@ impl VmFd { }, dest_addr: request.apic_id, vector: request.vector, + ..Default::default() }; // SAFETY: IOCTL with correct types let ret = unsafe { ioctl_with_ref(&self.vm, MSHV_ASSERT_INTERRUPT(), &interrupt_arg) }; @@ -762,11 +763,11 @@ mod tests { let vm = hv.create_vm().unwrap(); let vcpu = vm.create_vcpu(0).unwrap(); let state = vcpu.get_lapic().unwrap(); - let vp_state: mshv_vp_state = mshv_vp_state::from(state); - let lapic: hv_local_interrupt_controller_state = unsafe { *(vp_state.buf.lapic) }; + let buffer = Buffer::from(&state); + let hv_state = unsafe { &*(buffer.buf as *const hv_local_interrupt_controller_state) }; let cfg = InterruptRequest { interrupt_type: hv_interrupt_type_HV_X64_INTERRUPT_TYPE_EXTINT, - apic_id: lapic.apic_id as u64, + apic_id: hv_state.apic_id as u64, vector: 0, level_triggered: false, logical_destination_mode: false, diff --git a/mshv-ioctls/src/mshv_ioctls.rs b/mshv-ioctls/src/mshv_ioctls.rs index 12b0d716..d16578dc 100644 --- a/mshv-ioctls/src/mshv_ioctls.rs +++ b/mshv-ioctls/src/mshv_ioctls.rs @@ -7,8 +7,8 @@ ioctl_iow_nr!(MSHV_CREATE_VP, MSHV_IOCTL, 0x04, mshv_create_vp); ioctl_iowr_nr!(MSHV_GET_VP_REGISTERS, MSHV_IOCTL, 0x05, mshv_vp_registers); ioctl_iow_nr!(MSHV_SET_VP_REGISTERS, MSHV_IOCTL, 0x06, mshv_vp_registers); ioctl_ior_nr!(MSHV_RUN_VP, MSHV_IOCTL, 0x07, hv_message); -ioctl_iowr_nr!(MSHV_GET_VP_STATE, MSHV_IOCTL, 0x0A, mshv_vp_state); -ioctl_iowr_nr!(MSHV_SET_VP_STATE, MSHV_IOCTL, 0x0B, mshv_vp_state); +ioctl_iowr_nr!(MSHV_GET_VP_STATE, MSHV_IOCTL, 0x0A, mshv_get_set_vp_state); +ioctl_iowr_nr!(MSHV_SET_VP_STATE, MSHV_IOCTL, 0x0B, mshv_get_set_vp_state); ioctl_iow_nr!( MSHV_CREATE_PARTITION, MSHV_IOCTL,