Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Update get/set Standard Registers to use VP register page #183

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from
Draft
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
4 changes: 4 additions & 0 deletions mshv-bindings/src/arm64/regs.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
// Copyright © 2024, Microsoft Corporation
//
// SPDX-License-Identifier: Apache-2.0 OR BSD-3-Clause
//
12 changes: 12 additions & 0 deletions mshv-bindings/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,15 @@ pub use hvdef::*;

pub mod hvcall;
pub use hvcall::*;

#[derive(Debug)]
pub struct RegisterPage(pub *mut hv_vp_register_page);

// SAFETY: struct is based on register page in the hypervisor,
// safe to Send across threads
unsafe impl Send for RegisterPage {}

// SAFETY: struct is based on Register page in the hypervisor,
// safe to Sync across threads as this is only required for Vcpu trait
// functionally not used anyway
unsafe impl Sync for RegisterPage {}
28 changes: 28 additions & 0 deletions mshv-bindings/src/x86_64/regs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -759,6 +759,34 @@ impl AllVpStateComponents {
}
}

#[macro_export]
macro_rules! set_gp_regs_field_ptr {
($this: ident, $name: ident, $value: expr) => {
#[allow(clippy::macro_metavars_in_unsafe)]
// SAFETY: access union fields
unsafe {
(*$this)
.__bindgen_anon_1
.__bindgen_anon_1
.__bindgen_anon_1
.__bindgen_anon_1
.$name = $value;
}
};
}

#[macro_export]
macro_rules! get_gp_regs_field_ptr {
($this: ident, $name: ident) => {
(*$this)
.__bindgen_anon_1
.__bindgen_anon_1
.__bindgen_anon_1
.__bindgen_anon_1
.$name
};
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
12 changes: 8 additions & 4 deletions mshv-ioctls/src/ioctls/system.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,13 +61,17 @@ impl Mshv {
}

/// Creates a VM fd using the MSHV fd and prepared mshv partition.
pub fn create_vm_with_args(&self, args: &mshv_create_partition) -> Result<VmFd> {
pub fn create_vm_with_args(
&self,
args: &mshv_create_partition,
vm_type: VmType,
) -> Result<VmFd> {
// SAFETY: IOCTL call with the correct types.
let ret = unsafe { ioctl_with_ref(&self.hv, MSHV_CREATE_PARTITION(), args) };
if ret >= 0 {
// SAFETY: we verify the value of ret and we are the owners of the fd.
let vm_file = unsafe { File::from_raw_fd(ret) };
Ok(new_vmfd(vm_file))
Ok(new_vmfd(vm_file, vm_type))
} else {
Err(errno::Error::last().into())
}
Expand Down Expand Up @@ -111,7 +115,7 @@ impl Mshv {
pt_isolation,
};

let vm = self.create_vm_with_args(&create_args)?;
let vm = self.create_vm_with_args(&create_args, vm_type)?;

// This is an 'early' property that must be set between creation and initialization
vm.hvcall_set_partition_property(
Expand Down Expand Up @@ -236,7 +240,7 @@ mod tests {
fn test_create_vm_with_default_config() {
let pr: mshv_create_partition = Default::default();
let hv = Mshv::new().unwrap();
let vm = hv.create_vm_with_args(&pr);
let vm = hv.create_vm_with_args(&pr, VmType::Normal);
assert!(vm.is_ok());
}

Expand Down
80 changes: 78 additions & 2 deletions mshv-ioctls/src/ioctls/vcpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,15 +46,20 @@ macro_rules! set_registers_64 {
pub struct VcpuFd {
index: u32,
vcpu: File,
vp_page: Option<RegisterPage>,
}

/// Helper function to create a new `VcpuFd`.
///
/// This should not be exported as a public function because the preferred way is to use
/// `create_vcpu` from `VmFd`. The function cannot be part of the `VcpuFd` implementation because
/// then it would be exported with the public `VcpuFd` interface.
pub fn new_vcpu(index: u32, vcpu: File) -> VcpuFd {
VcpuFd { index, vcpu }
pub fn new_vcpu(index: u32, vcpu: File, vp_page: Option<RegisterPage>) -> VcpuFd {
VcpuFd {
index,
vcpu,
vp_page,
}
}

impl AsRawFd for VcpuFd {
Expand All @@ -64,6 +69,13 @@ impl AsRawFd for VcpuFd {
}

impl VcpuFd {
/// Get mut reference of VP register page
pub fn get_vp_reg_page(&self) -> *mut hv_vp_register_page {
Copy link
Member

Choose a reason for hiding this comment

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

If this can fail, you should return an Option. That's far better than expecting someone to read your comment in the function.

match self.vp_page { Some(p) => p, None => None }.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We can just return as is self.vp_page.

In your example two incompatible return type.

// This API should not be called if the VM is encrypted
assert!(self.vp_page.is_some());
self.vp_page.as_ref().unwrap().0
}

/// Get the register values by providing an array of register names
#[cfg(not(target_arch = "aarch64"))]
pub fn get_reg(&self, reg_names: &mut [hv_register_assoc]) -> Result<()> {
Expand Down Expand Up @@ -153,6 +165,7 @@ impl VcpuFd {

Ok(())
}

/// Sets the vCPU general purpose registers
#[cfg(not(target_arch = "aarch64"))]
pub fn set_regs(&self, regs: &StandardRegisters) -> Result<()> {
Expand Down Expand Up @@ -252,6 +265,68 @@ impl VcpuFd {
Ok(())
}

/// Sets the vCPU general purpose registers using VP register page
#[cfg(not(target_arch = "aarch64"))]
pub fn set_vp_page_standard_regs(&self, regs: &StandardRegisters) -> Result<()> {
let vp_reg_page = self.get_vp_reg_page();
set_gp_regs_field_ptr!(vp_reg_page, rax, regs.rax);
set_gp_regs_field_ptr!(vp_reg_page, rbx, regs.rbx);
set_gp_regs_field_ptr!(vp_reg_page, rcx, regs.rcx);
set_gp_regs_field_ptr!(vp_reg_page, rdx, regs.rdx);
set_gp_regs_field_ptr!(vp_reg_page, rsi, regs.rsi);
set_gp_regs_field_ptr!(vp_reg_page, rdi, regs.rdi);
set_gp_regs_field_ptr!(vp_reg_page, rsp, regs.rsp);
set_gp_regs_field_ptr!(vp_reg_page, rbp, regs.rbp);
set_gp_regs_field_ptr!(vp_reg_page, r8, regs.r8);
set_gp_regs_field_ptr!(vp_reg_page, r9, regs.r9);
set_gp_regs_field_ptr!(vp_reg_page, r10, regs.r10);
set_gp_regs_field_ptr!(vp_reg_page, r11, regs.r11);
set_gp_regs_field_ptr!(vp_reg_page, r12, regs.r12);
set_gp_regs_field_ptr!(vp_reg_page, r13, regs.r13);
set_gp_regs_field_ptr!(vp_reg_page, r14, regs.r14);
set_gp_regs_field_ptr!(vp_reg_page, r15, regs.r15);

// SAFETY: access union fields
unsafe {
(*vp_reg_page).dirty |= 1 << HV_X64_REGISTER_CLASS_GENERAL;
(*vp_reg_page).__bindgen_anon_1.__bindgen_anon_1.rip = regs.rip;
(*vp_reg_page).dirty |= 1 << HV_X64_REGISTER_CLASS_IP;
(*vp_reg_page).__bindgen_anon_1.__bindgen_anon_1.rflags = regs.rflags;
(*vp_reg_page).dirty |= 1 << HV_X64_REGISTER_CLASS_FLAGS;
}
Ok(())
}

/// Returns the vCPU general purpose registers using VP register page
#[cfg(not(target_arch = "aarch64"))]
pub fn get_vp_page_standard_regs(&self) -> Result<StandardRegisters> {
let vp_reg_page = self.get_vp_reg_page();
let mut ret_regs = StandardRegisters::default();
// SAFETY: access union fields
unsafe {
ret_regs.rax = get_gp_regs_field_ptr!(vp_reg_page, rax);
ret_regs.rbx = get_gp_regs_field_ptr!(vp_reg_page, rbx);
ret_regs.rcx = get_gp_regs_field_ptr!(vp_reg_page, rcx);
ret_regs.rdx = get_gp_regs_field_ptr!(vp_reg_page, rdx);
ret_regs.rsi = get_gp_regs_field_ptr!(vp_reg_page, rsi);
ret_regs.rdi = get_gp_regs_field_ptr!(vp_reg_page, rdi);
ret_regs.rsp = get_gp_regs_field_ptr!(vp_reg_page, rsp);
ret_regs.rbp = get_gp_regs_field_ptr!(vp_reg_page, rbp);
ret_regs.r8 = get_gp_regs_field_ptr!(vp_reg_page, r8);
ret_regs.r9 = get_gp_regs_field_ptr!(vp_reg_page, r9);
ret_regs.r10 = get_gp_regs_field_ptr!(vp_reg_page, r10);
ret_regs.r11 = get_gp_regs_field_ptr!(vp_reg_page, r11);
ret_regs.r12 = get_gp_regs_field_ptr!(vp_reg_page, r12);
ret_regs.r13 = get_gp_regs_field_ptr!(vp_reg_page, r13);
ret_regs.r14 = get_gp_regs_field_ptr!(vp_reg_page, r14);
ret_regs.r15 = get_gp_regs_field_ptr!(vp_reg_page, r15);
ret_regs.rip = (*vp_reg_page).__bindgen_anon_1.__bindgen_anon_1.rip;
ret_regs.rflags = (*vp_reg_page).__bindgen_anon_1.__bindgen_anon_1.rflags;
}

Ok(ret_regs)
}

/// Returns the vCPU general purpose registers.
#[cfg(not(target_arch = "aarch64"))]
pub fn get_regs(&self) -> Result<StandardRegisters> {
Expand Down Expand Up @@ -308,6 +383,7 @@ impl VcpuFd {

Ok(ret_regs)
}

/// Returns the vCPU special registers.
#[cfg(not(target_arch = "aarch64"))]
pub fn get_sregs(&self) -> Result<SpecialRegisters> {
Expand Down
29 changes: 26 additions & 3 deletions mshv-ioctls/src/ioctls/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ pub struct InterruptRequest {
#[derive(Debug)]
pub struct VmFd {
vm: File,
vm_type: VmType,
}

impl AsRawFd for VmFd {
Expand Down Expand Up @@ -225,7 +226,28 @@ impl VmFd {
// SAFETY: we're sure vcpu_fd is valid.
let vcpu = unsafe { File::from_raw_fd(vcpu_fd) };

Ok(new_vcpu(id as u32, vcpu))
let vp_page = if self.vm_type != VmType::Snp {
// SAFETY: Safe to call as VCPU has this map already available upon creation
let addr = unsafe {
libc::mmap(
std::ptr::null_mut(),
HV_PAGE_SIZE,
libc::PROT_READ | libc::PROT_WRITE,
libc::MAP_SHARED,
vcpu_fd,
MSHV_VP_MMAP_OFFSET_REGISTERS as i64 * libc::sysconf(libc::_SC_PAGE_SIZE),
)
};
if addr == libc::MAP_FAILED {
// No point of continuing, without this mmap VMGEXIT will fail anyway
// Return error
return Err(errno::Error::last().into());
}
Some(RegisterPage(addr as *mut hv_vp_register_page))
} else {
None
};
Ok(new_vcpu(id as u32, vcpu, vp_page))
}

/// Inject an interrupt into the guest..
Expand Down Expand Up @@ -749,13 +771,14 @@ impl VmFd {
}
}
}

/// Helper function to create a new `VmFd`.
///
/// This should not be exported as a public function because the preferred way is to use
/// `create_vm` from `Mshv`. The function cannot be part of the `VmFd` implementation because
/// then it would be exported with the public `VmFd` interface.
pub fn new_vmfd(vm: File) -> VmFd {
VmFd { vm }
pub fn new_vmfd(vm: File, vm_type: VmType) -> VmFd {
VmFd { vm, vm_type }
}
#[cfg(test)]
mod tests {
Expand Down
Loading