kvm: allow registering memory from current address space

This is good for enabling non-exportable Vulkan host coherent memory,
along with anything else where an exportable OS object isn't supported.

This CL introduces:

1. ExternalMapping, which wraps an external library mapping using
function callbacks, for purposes of sharing device memory to the guest
in the case where the device memory is not compatible with the mmap interface.
This is common in Vulkan when VkDeviceMemory is host visible but not
external, or external but based on an opaque fd.

The lifetime of the library mapping is tied to the lifetime of the
ExternalMapping.

2. Usually, we would send such memory requests over a socket to the main
thread. However, since these new objects require more metadata than
other requests that are sent over the wire (because there's information
about inheritance and refcounts), we also plumb the "map_request" field,
which wraps a single ExternalMapping.

Note that this ExternalMapping will not work in the sandbox case. In the
sandbox case, we will then have to figure out how to serialize/deserialize
ExternalMapping requests over a socket.

BUG=b/146066070, b/153580313
TEST=compile and test

Change-Id: I3b099b308aec45a313a8278ed6274f9dec66c30b
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2034029
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

Cargo.lock

@@ -790,6 +790,7 @@ dependencies = [
  "libc 0.2.65 (registry+https://github.com/rust-lang/crates.io-index)",
  "msg_socket 0.1.0",
  "resources 0.1.0",
+ "sync 0.1.0",
  "sys_util 0.1.0",
 ]
 

src/linux.rs

@@ -49,9 +49,9 @@ use sys_util::net::{UnixSeqpacket, UnixSeqpacketListener, UnlinkUnixSeqpacketLis
 use sys_util::{
     self, block_signal, clear_signal, drop_capabilities, error, flock, get_blocked_signals,
     get_group_id, get_user_id, getegid, geteuid, info, register_rt_signal_handler,
-    set_cpu_affinity, validate_raw_fd, warn, EventFd, FlockOperation, GuestAddress, GuestMemory,
-    Killable, MemoryMappingArena, PollContext, PollToken, Protection, ScopedEvent, SignalFd,
-    Terminal, TimerFd, WatchingEvents, SIGRTMIN,
+    set_cpu_affinity, validate_raw_fd, warn, EventFd, ExternalMapping, FlockOperation,
+    GuestAddress, GuestMemory, Killable, MemoryMappingArena, PollContext, PollToken, Protection,
+    ScopedEvent, SignalFd, Terminal, TimerFd, WatchingEvents, SIGRTMIN,
 };
 use vm_control::{
     BalloonControlCommand, BalloonControlRequestSocket, BalloonControlResponseSocket,
@@ -656,6 +656,7 @@ fn create_gpu_device(
     wayland_socket_path: Option<&PathBuf>,
     x_display: Option<String>,
     event_devices: Vec<EventDevice>,
+    _map_request: Arc<Mutex<Option<ExternalMapping>>>,
 ) -> DeviceResult {
     let jailed_wayland_path = Path::new("/wayland-0");
 
@@ -1087,6 +1088,7 @@ fn create_virtio_devices(
     balloon_device_socket: BalloonControlResponseSocket,
     disk_device_sockets: &mut Vec<DiskControlResponseSocket>,
     pmem_device_sockets: &mut Vec<VmMsyncRequestSocket>,
+    map_request: Arc<Mutex<Option<ExternalMapping>>>,
 ) -> DeviceResult<Vec<VirtioDeviceStub>> {
     let mut devs = Vec::new();
 
@@ -1249,6 +1251,7 @@ fn create_virtio_devices(
                 cfg.wayland_socket_paths.get(""),
                 cfg.x_display.clone(),
                 event_devices,
+                map_request,
             )?);
         }
     }
@@ -1290,6 +1293,7 @@ fn create_devices(
     disk_device_sockets: &mut Vec<DiskControlResponseSocket>,
     pmem_device_sockets: &mut Vec<VmMsyncRequestSocket>,
     usb_provider: HostBackendDeviceProvider,
+    map_request: Arc<Mutex<Option<ExternalMapping>>>,
 ) -> DeviceResult<Vec<(Box<dyn PciDevice>, Option<Minijail>)>> {
     let stubs = create_virtio_devices(
         &cfg,
@@ -1302,6 +1306,7 @@ fn create_devices(
         balloon_device_socket,
         disk_device_sockets,
         pmem_device_sockets,
+        map_request,
     )?;
 
     let mut pci_devices = Vec::new();
@@ -1801,6 +1806,8 @@ pub fn run_config(cfg: Config) -> Result<()> {
         msg_socket::pair::<VmIrqResponse, VmIrqRequest>().map_err(Error::CreateSocket)?;
     control_sockets.push(TaggedControlSocket::VmIrq(ioapic_host_socket));
 
+    let map_request: Arc<Mutex<Option<ExternalMapping>>> = Arc::new(Mutex::new(None));
+
     let sandbox = cfg.sandbox;
     let linux = Arch::build_vm(
         components,
@@ -1822,6 +1829,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
                 &mut disk_device_sockets,
                 &mut pmem_device_sockets,
                 usb_provider,
+                Arc::clone(&map_request),
             )
         },
     )
@@ -1836,6 +1844,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
         usb_control_socket,
         sigchld_fd,
         sandbox,
+        Arc::clone(&map_request),
     )
 }
 
@@ -1848,6 +1857,7 @@ fn run_control(
     usb_control_socket: UsbControlSocket,
     sigchld_fd: SignalFd,
     sandbox: bool,
+    map_request: Arc<Mutex<Option<ExternalMapping>>>,
 ) -> Result<()> {
     const LOWMEM_AVAILABLE: &str = "/sys/kernel/mm/chromeos-low_mem/available";
 
@@ -2189,8 +2199,11 @@ fn run_control(
                             },
                             TaggedControlSocket::VmMemory(socket) => match socket.recv() {
                                 Ok(request) => {
-                                    let response =
-                                        request.execute(&mut linux.vm, &mut linux.resources);
+                                    let response = request.execute(
+                                        &mut linux.vm,
+                                        &mut linux.resources,
+                                        Arc::clone(&map_request),
+                                    );
                                     if let Err(e) = socket.send(&response) {
                                         error!("failed to send VmMemoryControlResponse: {}", e);
                                     }

sys_util/src/external_mapping.rs

@@ -0,0 +1,140 @@
+// Copyright 2020 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+use crate::MappedRegion;
+use std::fmt::{self, Display};
+
+#[derive(Debug, Eq, PartialEq)]
+pub enum Error {
+    // A null address is typically bad.  mmap allows it, but not external libraries
+    NullAddress,
+    // For external mappings that have weird sizes
+    InvalidSize,
+    // External library failed to map
+    LibraryError(i32),
+    // If external mapping is unsupported.
+    Unsupported,
+}
+
+impl Display for Error {
+    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
+        use self::Error::*;
+
+        match self {
+            NullAddress => write!(f, "null address returned"),
+            InvalidSize => write!(f, "invalid size returned"),
+            LibraryError(ret) => write!(f, "library failed to map with {}", ret),
+            Unsupported => write!(f, "external mapping unsupported"),
+        }
+    }
+}
+
+pub type Result<T> = std::result::Result<T, Error>;
+
+// Maps a external library resource given an id, returning address and size upon success
+pub type Map = fn(u32) -> Result<(u64, usize)>;
+// Unmaps the resource given a resource id.
+pub type Unmap = fn(u32) -> ();
+
+/// ExternalMapping wraps an external library mapping.  This is useful in cases where where the
+/// device memory is not compatible with the mmap interface, such as Vulkan VkDeviceMemory in the
+/// non-exportable case or when exported as an opaque fd.
+#[derive(Debug, PartialEq)]
+pub struct ExternalMapping {
+    resource_id: u32,
+    ptr: u64,
+    size: usize,
+    unmap: Unmap,
+}
+
+unsafe impl Send for ExternalMapping {}
+unsafe impl Sync for ExternalMapping {}
+impl ExternalMapping {
+    /// Creates an ExternalMapping given a library-specific resource id and map/unmap functions.
+    /// The map function must return a valid host memory region.  In addition, callers of the
+    /// function must guarantee that the map and unmap functions are thread-safe, never return a
+    /// region overlapping already Rust referenced-data, and the backing store of the resource
+    /// doesn't disappear before the unmap function is called.
+    pub unsafe fn new(resource_id: u32, map: Map, unmap: Unmap) -> Result<ExternalMapping> {
+        let (ptr, size) = map(resource_id)?;
+
+        if ptr as *mut u8 == std::ptr::null_mut() {
+            return Err(Error::NullAddress);
+        }
+        if size == 0 {
+            return Err(Error::InvalidSize);
+        }
+
+        Ok(ExternalMapping {
+            resource_id,
+            ptr,
+            size,
+            unmap,
+        })
+    }
+}
+
+unsafe impl MappedRegion for ExternalMapping {
+    /// used for passing this region to ioctls for setting guest memory.
+    fn as_ptr(&self) -> *mut u8 {
+        self.ptr as *mut u8
+    }
+
+    /// Returns the size of the memory region in bytes.
+    fn size(&self) -> usize {
+        self.size
+    }
+}
+
+impl Drop for ExternalMapping {
+    fn drop(&mut self) {
+        // This is safe because we own this memory range, and nobody else is holding a reference to
+        // it.
+        (self.unmap)(self.resource_id)
+    }
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn check_valid_external_map() {
+        let map1: Map = |_resource_id| Ok((0xAAAABBBB, 500));
+        let map2: Map = |_resource_id| Ok((0xBBBBAAAA, 1000));
+        let unmap: Unmap = |_resource_id| {};
+        let external_map1 = unsafe { ExternalMapping::new(0, map1, unmap).unwrap() };
+        let external_map2 = unsafe { ExternalMapping::new(0, map2, unmap).unwrap() };
+
+        assert_eq!(external_map1.as_ptr(), 0xAAAABBBB as *mut u8);
+        assert_eq!(external_map1.size(), 500);
+        assert_eq!(external_map2.as_ptr(), 0xBBBBAAAA as *mut u8);
+        assert_eq!(external_map2.size(), 1000);
+    }
+
+    #[test]
+    fn check_invalid_external_map() {
+        let map1: Map = |_resource_id| Ok((0xAAAABBBB, 0));
+        let map2: Map = |_resource_id| Ok((0, 500));
+        let unmap: Unmap = |_resource_id| {};
+
+        assert_eq!(
+            unsafe { ExternalMapping::new(0, map1, unmap) },
+            Err(Error::InvalidSize)
+        );
+
+        assert_eq!(
+            unsafe { ExternalMapping::new(0, map2, unmap) },
+            Err(Error::NullAddress)
+        );
+    }
+
+    #[test]
+    #[should_panic]
+    fn check_external_map_drop() {
+        let map = |_resource_id| Ok((0xAAAABBBB, 500));
+        let unmap = |_resource_id| panic!();
+        let _external_map = unsafe { ExternalMapping::new(0, map, unmap) };
+    }
+}

sys_util/src/lib.rs

@@ -16,6 +16,7 @@ mod clock;
 mod descriptor;
 mod errno;
 mod eventfd;
+mod external_mapping;
 mod file_flags;
 pub mod file_traits;
 mod fork;
@@ -44,6 +45,7 @@ pub use crate::clock::{Clock, FakeClock};
 pub use crate::descriptor::*;
 pub use crate::errno::{errno_result, Error, Result};
 pub use crate::eventfd::*;
+pub use crate::external_mapping::*;
 pub use crate::file_flags::*;
 pub use crate::fork::*;
 pub use crate::guest_address::*;
@@ -64,6 +66,8 @@ pub use crate::terminal::*;
 pub use crate::timerfd::*;
 pub use poll_token_derive::*;
 
+pub use crate::external_mapping::Error as ExternalMappingError;
+pub use crate::external_mapping::Result as ExternalMappingResult;
 pub use crate::file_traits::{
     AsRawFds, FileAllocate, FileGetLen, FileReadWriteAtVolatile, FileReadWriteVolatile, FileSetLen,
     FileSync,

vm_control/Cargo.toml

@@ -10,4 +10,5 @@ kvm = { path = "../kvm" }
 libc = "*"
 msg_socket = { path = "../msg_socket" }
 resources = { path = "../resources" }
+sync = { path = "../sync" }
 sys_util = { path = "../sys_util" }

vm_control/src/lib.rs

@@ -15,13 +15,18 @@ use std::fs::File;
 use std::io::{Seek, SeekFrom};
 use std::mem::ManuallyDrop;
 use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
+use std::sync::Arc;
 
 use libc::{EINVAL, EIO, ENODEV};
 
 use kvm::{IrqRoute, IrqSource, Vm};
 use msg_socket::{MsgError, MsgOnSocket, MsgReceiver, MsgResult, MsgSender, MsgSocket};
 use resources::{Alloc, GpuMemoryDesc, MmioType, SystemAllocator};
-use sys_util::{error, Error as SysError, EventFd, GuestAddress, MemoryMapping, MmapError, Result};
+use sync::Mutex;
+use sys_util::{
+    error, Error as SysError, EventFd, ExternalMapping, GuestAddress, MappedRegion, MemoryMapping,
+    MmapError, Result,
+};
 
 /// A file descriptor either borrowed or owned by this.
 #[derive(Debug)]
@@ -262,7 +267,9 @@ pub enum VmMemoryRequest {
     /// Similiar to `VmMemoryRequest::RegisterMemory`, but doesn't allocate new address space.
     /// Useful for cases where the address space is already allocated (PCI regions).
     RegisterFdAtPciBarOffset(Alloc, MaybeOwnedFd, usize, u64),
-    /// Unregister the given memory slot that was previously registereed with `RegisterMemory`.
+    /// Similar to RegisterFdAtPciBarOffset, but is for buffers in the current address space.
+    RegisterHostPointerAtPciBarOffset(Alloc, u64),
+    /// Unregister the given memory slot that was previously registered with `RegisterMemory*`.
     UnregisterMemory(u32),
     /// Allocate GPU buffer of a given size/format and register the memory into guest address space.
     /// The response variant is `VmResponse::AllocateAndRegisterGpuMemory`
@@ -290,7 +297,12 @@ impl VmMemoryRequest {
     /// This does not return a result, instead encapsulating the success or failure in a
     /// `VmMemoryResponse` with the intended purpose of sending the response back over the socket
     /// that received this `VmMemoryResponse`.
-    pub fn execute(&self, vm: &mut Vm, sys_allocator: &mut SystemAllocator) -> VmMemoryResponse {
+    pub fn execute(
+        &self,
+        vm: &mut Vm,
+        sys_allocator: &mut SystemAllocator,
+        map_request: Arc<Mutex<Option<ExternalMapping>>>,
+    ) -> VmMemoryResponse {
         use self::VmMemoryRequest::*;
         match *self {
             RegisterMemory(ref fd, size) => {
@@ -309,6 +321,18 @@ impl VmMemoryRequest {
                 Ok(_) => VmMemoryResponse::Ok,
                 Err(e) => VmMemoryResponse::Err(e),
             },
+            RegisterHostPointerAtPciBarOffset(alloc, offset) => {
+                let mem = map_request
+                    .lock()
+                    .take()
+                    .ok_or(VmMemoryResponse::Err(SysError::new(EINVAL)))
+                    .unwrap();
+
+                match register_memory_hva(vm, sys_allocator, Box::new(mem), (alloc, offset)) {
+                    Ok((pfn, slot)) => VmMemoryResponse::RegisterMemory { pfn, slot },
+                    Err(e) => VmMemoryResponse::Err(e),
+                }
+            }
             AllocateAndRegisterGpuMemory {
                 width,
                 height,
@@ -562,6 +586,21 @@ fn register_memory(
     Ok((addr >> 12, slot))
 }
 
+fn register_memory_hva(
+    vm: &mut Vm,
+    allocator: &mut SystemAllocator,
+    mem: Box<dyn MappedRegion>,
+    pci_allocation: (Alloc, u64),
+) -> Result<(u64, u32)> {
+    let addr = allocator
+        .mmio_allocator(MmioType::High)
+        .address_from_pci_offset(pci_allocation.0, pci_allocation.1, mem.size() as u64)
+        .map_err(|_e| SysError::new(EINVAL))?;
+
+    let slot = vm.add_memory_region(GuestAddress(addr), mem, false, false)?;
+    Ok((addr >> 12, slot))
+}
+
 impl VmRequest {
     /// Executes this request on the given Vm and other mutable state.
     ///