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 <liuwe@microsoft.com>

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",

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",

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"] }
 

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<T> = std::result::Result<T, HypervisorDeviceError>;
-
-///
-/// 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;
-}

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

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_bindings::kvm_run, kvm_bindings::kvm_vcpu_events as VcpuEvents, kvm_ioctls::VcpuExit,
-    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<DeviceFd> {
+    fn create_device(&self, device: &mut CreateDevice) -> vm::Result<vfio_ioctls::VfioDeviceFd> {
         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<Arc<dyn device::Device>> {
+    fn create_passthrough_device(&self) -> vm::Result<VfioDeviceFd> {
         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(
+        self.create_device(&mut vfio_dev)
-            |e| vm::HypervisorVmError::CreatePassthroughDevice(e.into()),
+            .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
-    }
-}

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,

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<DeviceFd> {
+    fn create_device(&self, device: &mut CreateDevice) -> vm::Result<VfioDeviceFd> {
         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<Arc<dyn device::Device>> {
+    fn create_passthrough_device(&self) -> vm::Result<VfioDeviceFd> {
         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(
+        self.create_device(&mut vfio_dev)
-            |e| vm::HypervisorVmError::CreatePassthroughDevice(e.into()),
+            .map_err(|e| vm::HypervisorVmError::CreatePassthroughDevice(e.into()))
-        )?))
     }
 
     ///

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<Arc<dyn Device>>;
+    fn create_passthrough_device(&self) -> Result<vfio_ioctls::VfioDeviceFd>;
     /// Start logging dirty pages
     fn start_dirty_log(&self) -> Result<()>;
     /// Stop logging dirty pages

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<Arc<dyn InterruptManager<GroupConfig = LegacyIrqGroupConfig>>>,
 
     // Passthrough device handle
-    passthrough_device: Option<Arc<dyn hypervisor::Device>>,
+    passthrough_device: Option<VfioDeviceFd>,
 
     // 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.
+        let dup = passthrough_device
-        //
+            .try_clone()
-        // This dup() is mandatory to be able to give full ownership of the
+            .map_err(DeviceManagerError::VfioCreate)?;
-        // 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);
-        }
 
-        assert!(passthrough_device.as_any().is::<DeviceFd>());
-
-        // 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) }))
+            VfioContainer::new(Some(Arc::new(dup))).map_err(DeviceManagerError::VfioCreate)?,
-                .map_err(DeviceManagerError::VfioCreate)?,
         ))
     }