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

Add support for bus lock detection (KVM_CAP_X86_BUS_LOCK_EXIT). #245

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ reg_size as a public method.
userspace MSR handling.
- [[#246](https://github.com/rust-vmm/kvm-ioctls/pull/246)] Add support for
userspace NMI injection (`KVM_NMI` ioctl).
- [[#245](https://github.com/rust-vmm/kvm-ioctls/pull/245)] x86: add support
for bus lock detection (`KVM_CAP_X86_BUS_LOCK_EXIT` capability).

# v0.15.0

Expand Down
2 changes: 2 additions & 0 deletions src/cap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -166,4 +166,6 @@ pub enum Cap {
ArmPtrAuthGeneric = KVM_CAP_ARM_PTRAUTH_GENERIC,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
X86UserSpaceMsr = KVM_CAP_X86_USER_SPACE_MSR,
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
X86BusLockExit = KVM_CAP_X86_BUS_LOCK_EXIT,
}
8 changes: 8 additions & 0 deletions src/ioctls/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,14 @@ impl KvmRunWrapper {
}
}

impl AsRef<kvm_run> for KvmRunWrapper {
fn as_ref(&self) -> &kvm_run {
// SAFETY: Safe because we know we mapped enough memory to hold the kvm_run struct because
// the kernel told us how large it was.
unsafe { &*(self.kvm_run_ptr as *const kvm_run) }
}
}

impl Drop for KvmRunWrapper {
fn drop(&mut self) {
// SAFETY: This is safe because we mmap the area at kvm_run_ptr ourselves,
Expand Down
69 changes: 60 additions & 9 deletions src/ioctls/vcpu.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,8 @@ pub enum VcpuExit<'a> {
X86Rdmsr(ReadMsrExit<'a>),
/// Corresponds to KVM_EXIT_X86_WRMSR.
X86Wrmsr(WriteMsrExit<'a>),
/// Corresponds to KVM_EXIT_X86_BUS_LOCK.
X86BusLock,
/// Corresponds to an exit reason that is unknown from the current version
/// of the kvm-ioctls crate. Let the consumer decide about what to do with
/// it.
Expand Down Expand Up @@ -1544,6 +1546,7 @@ impl VcpuFd {
Ok(VcpuExit::IoapicEoi(eoi.vector))
}
KVM_EXIT_HYPERV => Ok(VcpuExit::Hyperv),
KVM_EXIT_X86_BUS_LOCK => Ok(VcpuExit::X86BusLock),
r => Ok(VcpuExit::Unsupported(r)),
}
} else {
Expand Down Expand Up @@ -1764,7 +1767,7 @@ impl VcpuFd {
/// ```
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn sync_regs(&self) -> kvm_sync_regs {
let kvm_run: &mut kvm_run = self.kvm_run_ptr.as_mut_ref();
let kvm_run = self.kvm_run_ptr.as_ref();

// SAFETY: Accessing this union field could be out of bounds if the `kvm_run`
// allocation isn't large enough. The `kvm_run` region is set using
Expand Down Expand Up @@ -1849,6 +1852,19 @@ impl VcpuFd {
_ => Err(errno::Error::last()),
}
}

/// If [`Cap::X86BusLockExit`](crate::Cap::X86BusLockExit) was enabled,
/// checks whether a bus lock was detected on the last VM exit. This may
/// return `true` even if the corresponding exit was not
/// [`VcpuExit::X86BusLock`], as a different VM exit may have preempted
/// it.
///
/// See the API documentation for `KVM_CAP_X86_BUS_LOCK_EXIT`.
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
pub fn bus_lock_detected(&self) -> bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: I would prefix the name with is_ (is_bus_...) to match the naming conventions used for such functions.

Also, can we add a test for it?

let kvm_run = self.kvm_run_ptr.as_ref();
kvm_run.flags as u32 & KVM_RUN_X86_BUS_LOCK != 0
}
}

/// Helper function to create a new `VcpuFd`.
Expand Down Expand Up @@ -2689,9 +2705,9 @@ mod tests {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();
let vcpu = vm.create_vcpu(0).unwrap();
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().immediate_exit, 0);
assert_eq!(vcpu.kvm_run_ptr.as_ref().immediate_exit, 0);
vcpu.set_kvm_immediate_exit(1);
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().immediate_exit, 1);
assert_eq!(vcpu.kvm_run_ptr.as_ref().immediate_exit, 1);
}

#[test]
Expand Down Expand Up @@ -2763,17 +2779,17 @@ mod tests {
];
for reg in &sync_regs {
vcpu.set_sync_valid_reg(*reg);
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().kvm_valid_regs, *reg as u64);
assert_eq!(vcpu.kvm_run_ptr.as_ref().kvm_valid_regs, *reg as u64);
vcpu.clear_sync_valid_reg(*reg);
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().kvm_valid_regs, 0);
assert_eq!(vcpu.kvm_run_ptr.as_ref().kvm_valid_regs, 0);
}

// Test that multiple valid SyncRegs can be set at the same time
vcpu.set_sync_valid_reg(SyncReg::Register);
vcpu.set_sync_valid_reg(SyncReg::SystemRegister);
vcpu.set_sync_valid_reg(SyncReg::VcpuEvents);
assert_eq!(
vcpu.kvm_run_ptr.as_mut_ref().kvm_valid_regs,
vcpu.kvm_run_ptr.as_ref().kvm_valid_regs,
SyncReg::Register as u64 | SyncReg::SystemRegister as u64 | SyncReg::VcpuEvents as u64
);

Expand All @@ -2786,17 +2802,17 @@ mod tests {

for reg in &sync_regs {
vcpu.set_sync_dirty_reg(*reg);
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().kvm_dirty_regs, *reg as u64);
assert_eq!(vcpu.kvm_run_ptr.as_ref().kvm_dirty_regs, *reg as u64);
vcpu.clear_sync_dirty_reg(*reg);
assert_eq!(vcpu.kvm_run_ptr.as_mut_ref().kvm_dirty_regs, 0);
assert_eq!(vcpu.kvm_run_ptr.as_ref().kvm_dirty_regs, 0);
}

// Test that multiple dirty SyncRegs can be set at the same time
vcpu.set_sync_dirty_reg(SyncReg::Register);
vcpu.set_sync_dirty_reg(SyncReg::SystemRegister);
vcpu.set_sync_dirty_reg(SyncReg::VcpuEvents);
assert_eq!(
vcpu.kvm_run_ptr.as_mut_ref().kvm_dirty_regs,
vcpu.kvm_run_ptr.as_ref().kvm_dirty_regs,
SyncReg::Register as u64 | SyncReg::SystemRegister as u64 | SyncReg::VcpuEvents as u64
);
}
Expand Down Expand Up @@ -3075,4 +3091,39 @@ mod tests {
e => panic!("Unexpected exit: {:?}", e),
}
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
fn test_enable_bus_lock_detection() {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();
if !vm.check_extension(Cap::X86BusLockExit) {
return;
}
let args = KVM_BUS_LOCK_DETECTION_EXIT;
let cap = kvm_enable_cap {
cap: Cap::X86BusLockExit as u32,
args: [args as u64, 0, 0, 0],
..Default::default()
};
vm.enable_cap(&cap).unwrap();
}

#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
#[test]
fn test_enable_bus_lock_detection_invalid() {
let kvm = Kvm::new().unwrap();
let vm = kvm.create_vm().unwrap();
if !vm.check_extension(Cap::X86BusLockExit) {
return;
}
// These flags should be mutually exclusive
let args = KVM_BUS_LOCK_DETECTION_OFF | KVM_BUS_LOCK_DETECTION_EXIT;
let cap = kvm_enable_cap {
cap: Cap::X86BusLockExit as u32,
args: [args as u64, 0, 0, 0],
..Default::default()
};
vm.enable_cap(&cap).unwrap_err();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also check the inner error? Maybe using .errno() like in other tests.

}
}