From a4f484bc5e72cf649c1bdfad1c91848acd9b8be9 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Fri, 3 Jul 2020 16:27:53 +0200 Subject: [PATCH] hypervisor: Define a VM-Exit abstraction In order to move the hypervisor specific parts of the VM exit handling path, we're defining a generic, hypervisor agnostic VM exit enum. This is what the hypervisor's Vcpu run() call should return when the VM exit can not be completely handled through the hypervisor specific bits. For KVM based hypervisors, this means directly forwarding the IO related exits back to the VMM itself. For other hypervisors that e.g. rely on the VMM to decode and emulate instructions, this means the decoding itself would happen in the hypervisor crate exclusively, and the rest of the VM exit handling would be handled through the VMM device model implementation. Signed-off-by: Samuel Ortiz Fix test_vm unit test by using the new abstraction and dropping some dead code. Signed-off-by: Wei Liu --- hypervisor/src/cpu.rs | 26 +++++++++++++++----- hypervisor/src/kvm/mod.rs | 47 +++++++++++++++++++++++++++++++++-- hypervisor/src/lib.rs | 4 ++- vmm/src/cpu.rs | 52 +++++++++------------------------------ vmm/src/vm.rs | 42 +++---------------------------- 5 files changed, 83 insertions(+), 88 deletions(-) diff --git a/hypervisor/src/cpu.rs b/hypervisor/src/cpu.rs index 37c37a8ae..16b5ed062 100644 --- a/hypervisor/src/cpu.rs +++ b/hypervisor/src/cpu.rs @@ -10,7 +10,7 @@ #[cfg(target_arch = "aarch64")] use crate::aarch64::VcpuInit; -use crate::{CpuState, MpState, VcpuExit}; +use crate::{CpuState, MpState}; #[cfg(target_arch = "x86_64")] use crate::x86_64::{ @@ -18,7 +18,6 @@ use crate::x86_64::{ StandardRegisters, VcpuEvents, Xsave, }; use thiserror::Error; -use vmm_sys_util::errno::Error as RunError; #[derive(Error, Debug)] /// @@ -151,6 +150,20 @@ pub enum HypervisorCpuError { NotifyGuestClockPaused(#[source] anyhow::Error), } +#[derive(Debug)] +pub enum VmExit<'a> { + #[cfg(target_arch = "x86_64")] + IoOut(u16 /* port */, &'a [u8] /* data */), + #[cfg(target_arch = "x86_64")] + IoIn(u16 /* port */, &'a mut [u8] /* data */), + #[cfg(target_arch = "x86_64")] + IoapicEoi(u8 /* vector */), + MmioRead(u64 /* address */, &'a mut [u8]), + MmioWrite(u64 /* address */, &'a [u8]), + Ignore, + Reset, +} + /// /// Result type for returning from a function /// @@ -248,10 +261,6 @@ pub trait Vcpu: Send + Sync { /// X86 specific call that sets the vcpu's current "xcrs". /// fn set_xcrs(&self, xcrs: &ExtendedControlRegisters) -> Result<()>; - /// - /// Triggers the running of the current virtual CPU returning an exit reason. - /// - fn run(&self) -> std::result::Result; #[cfg(target_arch = "x86_64")] /// /// Returns currently pending exceptions, interrupts, and NMIs as well as related @@ -295,4 +304,9 @@ pub trait Vcpu: Send + Sync { /// This function is required when restoring the VM /// fn set_state(&self, state: &CpuState) -> Result<()>; + + /// + /// Triggers the running of the current virtual CPU returning an exit reason. + /// + fn run(&self) -> std::result::Result; } diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 278f69004..b99ddc339 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -564,8 +564,51 @@ impl cpu::Vcpu for KvmVcpu { /// /// Triggers the running of the current virtual CPU returning an exit reason. /// - fn run(&self) -> std::result::Result { - self.fd.run() + fn run(&self) -> std::result::Result { + match self.fd.run() { + Ok(run) => match run { + #[cfg(target_arch = "x86_64")] + VcpuExit::IoIn(addr, data) => Ok(cpu::VmExit::IoIn(addr, data)), + #[cfg(target_arch = "x86_64")] + VcpuExit::IoOut(addr, data) => Ok(cpu::VmExit::IoOut(addr, data)), + #[cfg(target_arch = "x86_64")] + VcpuExit::IoapicEoi(vector) => Ok(cpu::VmExit::IoapicEoi(vector)), + #[cfg(target_arch = "x86_64")] + VcpuExit::Shutdown | VcpuExit::Hlt => Ok(cpu::VmExit::Reset), + + #[cfg(target_arch = "aarch64")] + VcpuExit::SystemEvent(event_type, flags) => { + use kvm_bindings::KVM_SYSTEM_EVENT_SHUTDOWN; + // On Aarch64, when the VM is shutdown, run() returns + // VcpuExit::SystemEvent with reason KVM_SYSTEM_EVENT_SHUTDOWN + if event_type == KVM_SYSTEM_EVENT_SHUTDOWN { + Ok(cpu::VmExit::Reset) + } else { + Err(cpu::HypervisorCpuError::RunVcpu(anyhow!( + "Unexpected system event with type 0x{:x}, flags 0x{:x}", + event_type, + flags + ))) + } + } + + VcpuExit::MmioRead(addr, data) => Ok(cpu::VmExit::MmioRead(addr, data)), + VcpuExit::MmioWrite(addr, data) => Ok(cpu::VmExit::MmioWrite(addr, data)), + + r => Err(cpu::HypervisorCpuError::RunVcpu(anyhow!( + "Unexpected exit reason on vcpu run: {:?}", + r + ))), + }, + + Err(ref e) => match e.errno() { + libc::EAGAIN | libc::EINTR => Ok(cpu::VmExit::Ignore), + _ => Err(cpu::HypervisorCpuError::RunVcpu(anyhow!( + "VCPU error {:?}", + e + ))), + }, + } } #[cfg(target_arch = "x86_64")] /// diff --git a/hypervisor/src/lib.rs b/hypervisor/src/lib.rs index 1af03aeea..66402f412 100644 --- a/hypervisor/src/lib.rs +++ b/hypervisor/src/lib.rs @@ -22,6 +22,8 @@ extern crate serde; extern crate serde_derive; extern crate serde_json; extern crate thiserror; +#[macro_use] +extern crate anyhow; /// KVM implementation module pub mod kvm; @@ -39,6 +41,6 @@ pub mod arch; mod cpu; pub use crate::hypervisor::{Hypervisor, HypervisorError}; -pub use cpu::{HypervisorCpuError, Vcpu}; +pub use cpu::{HypervisorCpuError, Vcpu, VmExit}; pub use kvm::*; pub use vm::{DataMatch, HypervisorVmError, Vm}; diff --git a/vmm/src/cpu.rs b/vmm/src/cpu.rs index 341fd474e..60f18e34e 100644 --- a/vmm/src/cpu.rs +++ b/vmm/src/cpu.rs @@ -25,11 +25,9 @@ use arch::EntryPoint; #[cfg(target_arch = "x86_64")] use arch::{CpuidPatch, CpuidReg}; use devices::{interrupt_controller::InterruptController, BusDevice}; -#[cfg(target_arch = "aarch64")] -use hypervisor::kvm::kvm_bindings::KVM_SYSTEM_EVENT_SHUTDOWN; #[cfg(target_arch = "x86_64")] use hypervisor::CpuId; -use hypervisor::{CpuState, VcpuExit}; +use hypervisor::{CpuState, VmExit}; use libc::{c_void, siginfo_t}; @@ -124,9 +122,6 @@ pub enum Error { /// Error configuring VCPU VcpuConfiguration(arch::Error), - /// Unexpected KVM_RUN exit reason - VcpuUnhandledKvmExit, - /// Failed to join on vCPU threads ThreadCleanup(std::boxed::Box), @@ -322,28 +317,28 @@ impl Vcpu { match self.vcpu.run() { Ok(run) => match run { #[cfg(target_arch = "x86_64")] - VcpuExit::IoIn(addr, data) => { + VmExit::IoIn(addr, data) => { self.io_bus.read(u64::from(addr), data); Ok(true) } #[cfg(target_arch = "x86_64")] - VcpuExit::IoOut(addr, data) => { + VmExit::IoOut(addr, data) => { if addr == DEBUG_IOPORT && data.len() == 1 { self.log_debug_ioport(data[0]); } self.io_bus.write(u64::from(addr), data); Ok(true) } - VcpuExit::MmioRead(addr, data) => { + VmExit::MmioRead(addr, data) => { self.mmio_bus.read(addr as u64, data); Ok(true) } - VcpuExit::MmioWrite(addr, data) => { + VmExit::MmioWrite(addr, data) => { self.mmio_bus.write(addr as u64, data); Ok(true) } #[cfg(target_arch = "x86_64")] - VcpuExit::IoapicEoi(vector) => { + VmExit::IoapicEoi(vector) => { if let Some(interrupt_controller) = &self.interrupt_controller { interrupt_controller .lock() @@ -352,37 +347,12 @@ impl Vcpu { } Ok(true) } - VcpuExit::Shutdown => { - // Triple fault to trigger a reboot - Ok(false) - } - #[cfg(target_arch = "aarch64")] - VcpuExit::SystemEvent(event_type, flags) => { - // On Aarch64, when the VM is shutdown, run() returns - // VcpuExit::SystemEvent with reason KVM_SYSTEM_EVENT_SHUTDOWN - if event_type == KVM_SYSTEM_EVENT_SHUTDOWN { - Ok(false) - } else { - error!( - "Unexpected system event with type 0x{:x}, flags 0x{:x}", - event_type, flags - ); - Err(Error::VcpuUnhandledKvmExit) - } - } - r => { - error!("Unexpected exit reason on vcpu run: {:?}", r); - Err(Error::VcpuUnhandledKvmExit) - } + + VmExit::Ignore => Ok(true), + VmExit::Reset => Ok(false), }, - Err(ref e) => match e.errno() { - libc::EAGAIN | libc::EINTR => Ok(true), - _ => { - error!("VCPU {:?} error {:?}", self.id, e); - Err(Error::VcpuUnhandledKvmExit) - } - }, + Err(e) => Err(Error::VcpuRun(e.into())), } } @@ -846,7 +816,7 @@ impl CpuManager { break; } - // vcpu.run() returns false on a KVM_EXIT_SHUTDOWN (triple-fault) so trigger a reset + // vcpu.run() returns false on a triple-fault so trigger a reset match vcpu.lock().unwrap().run() { Err(e) => { error!("VCPU generated error: {:?}", e); diff --git a/vmm/src/vm.rs b/vmm/src/vm.rs index b8833d9ed..5a55288e9 100644 --- a/vmm/src/vm.rs +++ b/vmm/src/vm.rs @@ -1429,7 +1429,7 @@ mod tests { #[cfg(target_arch = "x86_64")] #[test] pub fn test_vm() { - use hypervisor::VcpuExit; + use hypervisor::VmExit; use vm_memory::{GuestMemory, GuestMemoryRegion}; // This example based on https://lwn.net/Articles/658511/ let code = [ @@ -1481,52 +1481,18 @@ pub fn test_vm() { loop { match vcpu.run().expect("run failed") { - VcpuExit::IoIn(addr, data) => { - println!( - "IO in -- addr: {:#x} data [{:?}]", - addr, - str::from_utf8(&data).unwrap() - ); - } - VcpuExit::IoOut(addr, data) => { + VmExit::IoOut(addr, data) => { println!( "IO out -- addr: {:#x} data [{:?}]", addr, str::from_utf8(&data).unwrap() ); } - VcpuExit::MmioRead(_addr, _data) => {} - VcpuExit::MmioWrite(_addr, _data) => {} - VcpuExit::Unknown => {} - VcpuExit::Exception => {} - VcpuExit::Hypercall => {} - VcpuExit::Debug => {} - VcpuExit::Hlt => { + VmExit::Reset => { println!("HLT"); break; } - VcpuExit::IrqWindowOpen => {} - VcpuExit::Shutdown => {} - VcpuExit::FailEntry => {} - VcpuExit::Intr => {} - VcpuExit::SetTpr => {} - VcpuExit::TprAccess => {} - VcpuExit::S390Sieic => {} - VcpuExit::S390Reset => {} - VcpuExit::Dcr => {} - VcpuExit::Nmi => {} - VcpuExit::InternalError => {} - VcpuExit::Osi => {} - VcpuExit::PaprHcall => {} - VcpuExit::S390Ucontrol => {} - VcpuExit::Watchdog => {} - VcpuExit::S390Tsch => {} - VcpuExit::Epr => {} - VcpuExit::SystemEvent(_, _) => {} - VcpuExit::S390Stsi => {} - VcpuExit::IoapicEoi(_vector) => {} - VcpuExit::Hyperv => {} + r => panic!("unexpected exit reason: {:?}", r), } - // r => panic!("unexpected exit reason: {:?}", r), } }