From a96a5d7816be680e5e895509e968c34d73afe2d2 Mon Sep 17 00:00:00 2001 From: Wei Liu Date: Thu, 21 Jul 2022 13:15:15 +0000 Subject: [PATCH] hypervisor, vmm: use new vfio-ioctls Use the new vfio-ioctls APIs. Drop Cloud Hypervisor's Device trait since it is no longer needed. Signed-off-by: Wei Liu --- Cargo.lock | 3 +- fuzz/Cargo.lock | 3 +- hypervisor/Cargo.toml | 1 + hypervisor/src/device.rs | 22 -------------- hypervisor/src/kvm/aarch64/gic/mod.rs | 10 ++++++- hypervisor/src/kvm/mod.rs | 42 +++++---------------------- hypervisor/src/lib.rs | 7 +---- hypervisor/src/mshv/mod.rs | 39 ++++--------------------- hypervisor/src/vm.rs | 3 +- vmm/src/device_manager.rs | 34 +++++----------------- 10 files changed, 36 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 85396ba0c..39eb58665 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -355,6 +355,7 @@ dependencies = [ "serde", "serde_json", "thiserror", + "vfio-ioctls", "vm-memory", "vmm-sys-util", ] @@ -1141,7 +1142,7 @@ dependencies = [ [[package]] name = "vfio-ioctls" version = "0.1.0" -source = "git+https://github.com/rust-vmm/vfio?branch=main#0e0e115551d7c3c894d42d8351e3d44f133e9056" +source = "git+https://github.com/rust-vmm/vfio?branch=main#1ff2324e5cb0fccab825f9c96bb9387a1a01863f" dependencies = [ "byteorder", "kvm-bindings", diff --git a/fuzz/Cargo.lock b/fuzz/Cargo.lock index 52e3f0875..85b8d4cf2 100644 --- a/fuzz/Cargo.lock +++ b/fuzz/Cargo.lock @@ -293,6 +293,7 @@ dependencies = [ "serde", "serde_json", "thiserror", + "vfio-ioctls", "vm-memory", "vmm-sys-util", ] @@ -723,7 +724,7 @@ dependencies = [ [[package]] name = "vfio-ioctls" version = "0.1.0" -source = "git+https://github.com/rust-vmm/vfio?branch=main#4630612f2ff300e78b6d55be324fb5481aa84661" +source = "git+https://github.com/rust-vmm/vfio?branch=main#1ff2324e5cb0fccab825f9c96bb9387a1a01863f" dependencies = [ "byteorder", "kvm-bindings", diff --git a/hypervisor/Cargo.toml b/hypervisor/Cargo.toml index 6972bdf63..be13a5662 100644 --- a/hypervisor/Cargo.toml +++ b/hypervisor/Cargo.toml @@ -23,6 +23,7 @@ mshv-bindings = { git = "https://github.com/rust-vmm/mshv", branch = "main", fea mshv-ioctls = { git = "https://github.com/rust-vmm/mshv", branch = "main", optional = true} serde = { version = "1.0.140", features = ["rc", "derive"] } serde_json = "1.0.82" +vfio-ioctls = { git = "https://github.com/rust-vmm/vfio", branch = "main", default-features = false } vm-memory = { version = "0.8.0", features = ["backend-mmap", "backend-atomic"] } vmm-sys-util = { version = "0.10.0", features = ["with-serde"] } diff --git a/hypervisor/src/device.rs b/hypervisor/src/device.rs index b25e42102..561e11043 100644 --- a/hypervisor/src/device.rs +++ b/hypervisor/src/device.rs @@ -9,9 +9,6 @@ // Copyright 2020, ARM Limited // -use crate::DeviceAttr; -use std::any::Any; -use std::os::unix::io::AsRawFd; use thiserror::Error; #[derive(Error, Debug)] @@ -29,22 +26,3 @@ pub enum HypervisorDeviceError { #[error("Failed to get device attribute: {0}")] GetDeviceAttribute(#[source] anyhow::Error), } - -/// -/// Result type for returning from a function -/// -pub type Result = std::result::Result; - -/// -/// Trait to represent a device -/// -/// This crate provides a hypervisor-agnostic interfaces for device -/// -pub trait Device: Send + Sync + AsRawFd { - /// Set device attribute. - fn set_device_attr(&self, attr: &DeviceAttr) -> Result<()>; - /// Get device attribute. - fn get_device_attr(&self, attr: &mut DeviceAttr) -> Result<()>; - /// Provide a way to downcast to the device fd. - fn as_any(&self) -> &dyn Any; -} diff --git a/hypervisor/src/kvm/aarch64/gic/mod.rs b/hypervisor/src/kvm/aarch64/gic/mod.rs index 20f90abe2..3e7e3bd45 100644 --- a/hypervisor/src/kvm/aarch64/gic/mod.rs +++ b/hypervisor/src/kvm/aarch64/gic/mod.rs @@ -166,6 +166,9 @@ impl KvmGicV3Its { .create_device(&mut its_device) .map_err(Error::CreateGic)?; + // We know vm is KvmVm + let its_fd = its_fd.to_kvm().unwrap(); + Self::set_device_attribute( &its_fd, kvm_bindings::KVM_DEV_ARM_VGIC_GRP_ADDR, @@ -216,7 +219,12 @@ impl KvmGicV3Its { flags: 0, }; - vm.create_device(&mut gic_device).map_err(Error::CreateGic) + let device_fd = vm + .create_device(&mut gic_device) + .map_err(Error::CreateGic)?; + + // We know for sure this is a KVM fd + Ok(device_fd.to_kvm().unwrap()) } /// Set a GIC device attribute diff --git a/hypervisor/src/kvm/mod.rs b/hypervisor/src/kvm/mod.rs index 6f9cb7a41..fbbc53cb5 100644 --- a/hypervisor/src/kvm/mod.rs +++ b/hypervisor/src/kvm/mod.rs @@ -18,7 +18,6 @@ pub use crate::aarch64::{ #[cfg(target_arch = "aarch64")] use crate::arch::aarch64::gic::Vgic; use crate::cpu; -use crate::device; use crate::hypervisor; use crate::vec_with_array_field; use crate::vm::{self, InterruptSourceConfig, VmOps}; @@ -88,6 +87,7 @@ pub use kvm_ioctls::{Cap, Kvm}; #[cfg(target_arch = "aarch64")] use std::mem; use thiserror::Error; +use vfio_ioctls::VfioDeviceFd; #[cfg(feature = "tdx")] use vmm_sys_util::{ioctl::ioctl_with_val, ioctl_ioc_nr, ioctl_iowr_nr}; /// @@ -95,8 +95,7 @@ use vmm_sys_util::{ioctl::ioctl_with_val, ioctl_ioc_nr, ioctl_iowr_nr}; /// pub use { kvm_bindings::kvm_create_device as CreateDevice, kvm_bindings::kvm_device_attr as DeviceAttr, - kvm_bindings::kvm_run, kvm_bindings::kvm_vcpu_events as VcpuEvents, kvm_ioctls::DeviceFd, - kvm_ioctls::VcpuExit, + kvm_bindings::kvm_run, kvm_bindings::kvm_vcpu_events as VcpuEvents, kvm_ioctls::VcpuExit, }; #[cfg(target_arch = "x86_64")] @@ -319,12 +318,12 @@ impl KvmVm { /// Creates an emulated device in the kernel. /// /// See the documentation for `KVM_CREATE_DEVICE`. - fn create_device(&self, device: &mut CreateDevice) -> vm::Result { + fn create_device(&self, device: &mut CreateDevice) -> vm::Result { let device_fd = self .fd .create_device(device) .map_err(|e| vm::HypervisorVmError::CreateDevice(e.into()))?; - Ok(device_fd) + Ok(VfioDeviceFd::new_from_kvm(device_fd)) } } @@ -687,16 +686,15 @@ impl vm::Vm for KvmVm { self.fd.check_extension(c) } /// Create a device that is used for passthrough - fn create_passthrough_device(&self) -> vm::Result> { + fn create_passthrough_device(&self) -> vm::Result { let mut vfio_dev = kvm_create_device { type_: kvm_device_type_KVM_DEV_TYPE_VFIO, fd: 0, flags: 0, }; - Ok(Arc::new(self.create_device(&mut vfio_dev).map_err( - |e| vm::HypervisorVmError::CreatePassthroughDevice(e.into()), - )?)) + self.create_device(&mut vfio_dev) + .map_err(|e| vm::HypervisorVmError::CreatePassthroughDevice(e.into())) } /// /// Start logging dirty pages @@ -2142,29 +2140,3 @@ impl KvmVcpu { .map_err(|e| cpu::HypervisorCpuError::SetVcpuEvents(e.into())) } } - -/// Device struct for KVM -pub type KvmDevice = DeviceFd; - -impl device::Device for KvmDevice { - /// - /// Set device attribute - /// - fn set_device_attr(&self, attr: &DeviceAttr) -> device::Result<()> { - self.set_device_attr(attr) - .map_err(|e| device::HypervisorDeviceError::SetDeviceAttribute(e.into())) - } - /// - /// Get device attribute - /// - fn get_device_attr(&self, attr: &mut DeviceAttr) -> device::Result<()> { - self.get_device_attr(attr) - .map_err(|e| device::HypervisorDeviceError::GetDeviceAttribute(e.into())) - } - /// - /// Cast to the underlying KVM device fd - /// - fn as_any(&self) -> &dyn Any { - self - } -} diff --git a/hypervisor/src/lib.rs b/hypervisor/src/lib.rs index c227231bb..4eb41f6ee 100644 --- a/hypervisor/src/lib.rs +++ b/hypervisor/src/lib.rs @@ -51,20 +51,15 @@ mod cpu; mod device; pub use cpu::{HypervisorCpuError, Vcpu, VmExit}; -pub use device::{Device, HypervisorDeviceError}; +pub use device::HypervisorDeviceError; pub use hypervisor::{Hypervisor, HypervisorError}; #[cfg(all(feature = "kvm", target_arch = "x86_64"))] pub use kvm::x86_64; #[cfg(all(feature = "kvm", target_arch = "aarch64"))] pub use kvm::{aarch64, GicState}; // Aliased types exposed from both hypervisors -#[cfg(feature = "kvm")] -pub use kvm::{CreateDevice, DeviceAttr, DeviceFd}; #[cfg(all(feature = "mshv", target_arch = "x86_64"))] pub use mshv::x86_64; -// Aliased types exposed from both hypervisors -#[cfg(all(feature = "mshv", target_arch = "x86_64"))] -pub use mshv::{CreateDevice, DeviceAttr, DeviceFd}; use std::sync::Arc; pub use vm::{ DataMatch, HypervisorVmError, InterruptSourceConfig, LegacyIrqSourceConfig, MsiIrqSourceConfig, diff --git a/hypervisor/src/mshv/mod.rs b/hypervisor/src/mshv/mod.rs index bea441977..dc823063c 100644 --- a/hypervisor/src/mshv/mod.rs +++ b/hypervisor/src/mshv/mod.rs @@ -17,11 +17,11 @@ use mshv_ioctls::{set_registers_64, Mshv, NoDatamatch, VcpuFd, VmFd}; use std::any::Any; use std::collections::HashMap; use std::sync::{Arc, RwLock}; +use vfio_ioctls::VfioDeviceFd; use vm::DataMatch; // x86_64 dependencies #[cfg(target_arch = "x86_64")] pub mod x86_64; -use crate::device; use crate::{ ClockData, CpuState, IoEventAddress, IrqRoutingEntry, MpState, UserMemoryRegion, USER_MEMORY_REGION_EXECUTE, USER_MEMORY_REGION_READ, USER_MEMORY_REGION_WRITE, @@ -753,32 +753,6 @@ impl MshvVcpu { } } -/// Device struct for MSHV -pub type MshvDevice = DeviceFd; - -impl device::Device for MshvDevice { - /// - /// Set device attribute - /// - fn set_device_attr(&self, attr: &DeviceAttr) -> device::Result<()> { - self.set_device_attr(attr) - .map_err(|e| device::HypervisorDeviceError::SetDeviceAttribute(e.into())) - } - /// - /// Get device attribute - /// - fn get_device_attr(&self, attr: &mut DeviceAttr) -> device::Result<()> { - self.get_device_attr(attr) - .map_err(|e| device::HypervisorDeviceError::GetDeviceAttribute(e.into())) - } - /// - /// Cast to the underlying MSHV device fd - /// - fn as_any(&self) -> &dyn Any { - self - } -} - struct MshvEmulatorContext<'a> { vcpu: &'a MshvVcpu, map: (u64, u64), // Initial GVA to GPA mapping provided by the hypervisor @@ -918,12 +892,12 @@ impl MshvVm { /// Creates an in-kernel device. /// /// See the documentation for `MSHV_CREATE_DEVICE`. - fn create_device(&self, device: &mut CreateDevice) -> vm::Result { + fn create_device(&self, device: &mut CreateDevice) -> vm::Result { let device_fd = self .fd .create_device(device) .map_err(|e| vm::HypervisorVmError::CreateDevice(e.into()))?; - Ok(device_fd) + Ok(VfioDeviceFd::new_from_mshv(device_fd)) } } @@ -1110,16 +1084,15 @@ impl vm::Vm for MshvVm { .into() } - fn create_passthrough_device(&self) -> vm::Result> { + fn create_passthrough_device(&self) -> vm::Result { let mut vfio_dev = mshv_create_device { type_: mshv_device_type_MSHV_DEV_TYPE_VFIO, fd: 0, flags: 0, }; - Ok(Arc::new(self.create_device(&mut vfio_dev).map_err( - |e| vm::HypervisorVmError::CreatePassthroughDevice(e.into()), - )?)) + self.create_device(&mut vfio_dev) + .map_err(|e| vm::HypervisorVmError::CreatePassthroughDevice(e.into())) } /// diff --git a/hypervisor/src/vm.rs b/hypervisor/src/vm.rs index 1873d2bae..055ee5f77 100644 --- a/hypervisor/src/vm.rs +++ b/hypervisor/src/vm.rs @@ -15,7 +15,6 @@ use crate::arch::aarch64::gic::Vgic; #[cfg(feature = "tdx")] use crate::arch::x86::CpuIdEntry; use crate::cpu::Vcpu; -use crate::device::Device; #[cfg(target_arch = "x86_64")] use crate::ClockData; use crate::UserMemoryRegion; @@ -331,7 +330,7 @@ pub trait Vm: Send + Sync + Any { /// Checks if a particular `Cap` is available. fn check_extension(&self, c: Cap) -> bool; /// Create a device that is used for passthrough - fn create_passthrough_device(&self) -> Result>; + fn create_passthrough_device(&self) -> Result; /// Start logging dirty pages fn start_dirty_log(&self) -> Result<()>; /// Stop logging dirty pages diff --git a/vmm/src/device_manager.rs b/vmm/src/device_manager.rs index ff92d81be..b2d405893 100644 --- a/vmm/src/device_manager.rs +++ b/vmm/src/device_manager.rs @@ -50,7 +50,7 @@ use devices::legacy::Serial; use devices::{ interrupt_controller, interrupt_controller::InterruptController, AcpiNotificationFlags, }; -use hypervisor::{DeviceFd, HypervisorVmError, IoEventAddress}; +use hypervisor::{HypervisorVmError, IoEventAddress}; use libc::{ cfmakeraw, isatty, tcgetattr, tcsetattr, termios, MAP_NORESERVE, MAP_PRIVATE, MAP_SHARED, O_TMPFILE, PROT_READ, PROT_WRITE, TCSANOW, @@ -75,7 +75,7 @@ use std::path::PathBuf; use std::result; use std::sync::{Arc, Mutex}; use std::time::Instant; -use vfio_ioctls::{VfioContainer, VfioDevice}; +use vfio_ioctls::{VfioContainer, VfioDevice, VfioDeviceFd}; use virtio_devices::transport::VirtioTransport; use virtio_devices::transport::{VirtioPciDevice, VirtioPciDeviceActivator}; use virtio_devices::vhost_user::VhostUserConfig; @@ -867,7 +867,7 @@ pub struct DeviceManager { legacy_interrupt_manager: Option>>, // Passthrough device handle - passthrough_device: Option>, + passthrough_device: Option, // VFIO container // Only one container can be created, therefore it is stored as part of the @@ -2942,32 +2942,12 @@ impl DeviceManager { .as_ref() .ok_or(DeviceManagerError::NoDevicePassthroughSupport)?; - // Safe because we know the RawFd is valid. - // - // This dup() is mandatory to be able to give full ownership of the - // file descriptor to the DeviceFd::from_raw_fd() function later in - // the code. - // - // This is particularly needed so that VfioContainer will still have - // a valid file descriptor even if DeviceManager, and therefore the - // passthrough_device are dropped. In case of Drop, the file descriptor - // would be closed, but Linux would still have the duplicated file - // descriptor opened from DeviceFd, preventing from unexpected behavior - // where the VfioContainer would try to use a closed file descriptor. - let dup_device_fd = unsafe { libc::dup(passthrough_device.as_raw_fd()) }; - if dup_device_fd == -1 { - return vmm_sys_util::errno::errno_result().map_err(DeviceManagerError::DupFd); - } + let dup = passthrough_device + .try_clone() + .map_err(DeviceManagerError::VfioCreate)?; - assert!(passthrough_device.as_any().is::()); - - // SAFETY the raw fd conversion here is safe because: - // 1. When running on KVM or MSHV, passthrough_device wraps around DeviceFd. - // 2. The conversion here extracts the raw fd and then turns the raw fd into a DeviceFd - // of the same (correct) type. Ok(Arc::new( - VfioContainer::new(Arc::new(unsafe { DeviceFd::from_raw_fd(dup_device_fd) })) - .map_err(DeviceManagerError::VfioCreate)?, + VfioContainer::new(Some(Arc::new(dup))).map_err(DeviceManagerError::VfioCreate)?, )) }