rutabaga_gfx: convert to SafeDescriptor
To be truly OS-agnostic, we need an OS-agnostic Rust wrapper over
the OS-specific handle type. SafeDescriptor seems to be the best
option, and I hope it on crates.io in the future.
This converts virtio_gpu/rutabaga to use the SafeDescriptor handle
when practical. minigbm still uses File in some places, since it
needs to SeekFrom(..), but minigbm is a Linux only thing anyways.
BUG=b:173630595
TEST=boot VM in 2D/3D mode
Change-Id: I18d735844d479f52d82d7976bf9b4e383b2e2252
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2779492
Commit-Queue: Gurchetan Singh <gurchetansingh@chromium.org>
Commit-Queue: Zach Reizner <zachr@chromium.org>
Tested-by: Gurchetan Singh <gurchetansingh@chromium.org>
Reviewed-by: Michael Hoyle <mikehoyle@google.com>
Reviewed-by: Zach Reizner <zachr@chromium.org>
diff --git a/base/src/shm.rs b/base/src/shm.rs
index 36c6f7e..fa7355a 100644
--- a/base/src/shm.rs
+++ b/base/src/shm.rs
@@ -8,7 +8,7 @@
};
use std::ffi::CStr;
use std::fs::File;
-use std::os::unix::io::AsRawFd;
+use std::os::unix::io::{AsRawFd, IntoRawFd};
use sys_util::SharedMemory as SysUtilSharedMemory;
/// See [SharedMemory](sys_util::SharedMemory) for struct- and method-level
@@ -77,8 +77,15 @@
}
}
-impl Into<File> for SharedMemory {
- fn into(self) -> File {
- self.0.into()
+impl IntoRawDescriptor for SharedMemory {
+ fn into_raw_descriptor(self) -> RawDescriptor {
+ self.0.into_raw_fd()
+ }
+}
+
+impl Into<SafeDescriptor> for SharedMemory {
+ fn into(self) -> SafeDescriptor {
+ // Safe because we own the SharedMemory at this point.
+ unsafe { SafeDescriptor::from_raw_descriptor(self.into_raw_descriptor()) }
}
}
diff --git a/devices/src/virtio/gpu/virtio_gpu.rs b/devices/src/virtio/gpu/virtio_gpu.rs
index 0e23c6a..58d3dcf 100644
--- a/devices/src/virtio/gpu/virtio_gpu.rs
+++ b/devices/src/virtio/gpu/virtio_gpu.rs
@@ -427,7 +427,7 @@
/// If supported, export the resource with the given `resource_id` to a file.
pub fn export_resource(&mut self, resource_id: u32) -> ResourceResponse {
let file = match self.rutabaga.export_blob(resource_id) {
- Ok(handle) => handle.os_handle,
+ Ok(handle) => handle.os_handle.into(),
Err(_) => return ResourceResponse::Invalid,
};
@@ -463,7 +463,7 @@
pub fn export_fence(&self, fence_id: u32) -> ResourceResponse {
match self.rutabaga.export_fence(fence_id) {
Ok(handle) => ResourceResponse::Resource(ResourceInfo::Fence {
- file: handle.os_handle,
+ file: handle.os_handle.into(),
}),
Err(_) => ResourceResponse::Invalid,
}
diff --git a/rutabaga_gfx/src/rutabaga_gralloc/minigbm.rs b/rutabaga_gfx/src/rutabaga_gralloc/minigbm.rs
index bdf62e7..c20438d 100644
--- a/rutabaga_gfx/src/rutabaga_gralloc/minigbm.rs
+++ b/rutabaga_gfx/src/rutabaga_gralloc/minigbm.rs
@@ -143,7 +143,7 @@
return Err(RutabagaError::SpecViolation);
}
- let dmabuf = gbm_buffer.export()?;
+ let dmabuf = gbm_buffer.export()?.into();
return Ok(RutabagaHandle {
os_handle: dmabuf,
handle_type: RUTABAGA_MEM_HANDLE_TYPE_DMABUF,
@@ -165,7 +165,7 @@
}
let gbm_buffer = MinigbmBuffer(bo, self.clone());
- let dmabuf = gbm_buffer.export()?;
+ let dmabuf = gbm_buffer.export()?.into();
Ok(RutabagaHandle {
os_handle: dmabuf,
handle_type: RUTABAGA_MEM_HANDLE_TYPE_DMABUF,
diff --git a/rutabaga_gfx/src/rutabaga_gralloc/vulkano_gralloc.rs b/rutabaga_gfx/src/rutabaga_gralloc/vulkano_gralloc.rs
index b74908a..c122224 100644
--- a/rutabaga_gfx/src/rutabaga_gralloc/vulkano_gralloc.rs
+++ b/rutabaga_gfx/src/rutabaga_gralloc/vulkano_gralloc.rs
@@ -269,10 +269,10 @@
.export_info(handle_type)
.build()?;
- let file = device_memory.export_fd(handle_type)?;
+ let descriptor = device_memory.export_fd(handle_type)?.into();
Ok(RutabagaHandle {
- os_handle: file,
+ os_handle: descriptor,
handle_type: rutabaga_type,
})
}
diff --git a/rutabaga_gfx/src/rutabaga_utils.rs b/rutabaga_gfx/src/rutabaga_utils.rs
index 943f36c..b81989d 100644
--- a/rutabaga_gfx/src/rutabaga_utils.rs
+++ b/rutabaga_gfx/src/rutabaga_utils.rs
@@ -5,13 +5,12 @@
//! rutabaga_utils: Utility enums, structs, and implementations needed by the rest of the crate.
use std::fmt::{self, Display};
-use std::fs::File;
use std::io::Error as IoError;
use std::os::raw::c_void;
use std::path::PathBuf;
use std::str::Utf8Error;
-use base::{Error as SysError, ExternalMappingError};
+use base::{Error as SysError, ExternalMappingError, SafeDescriptor};
use data_model::VolatileMemoryError;
#[cfg(feature = "vulkano")]
@@ -463,7 +462,7 @@
/// Handle to OS-specific memory or synchronization objects.
pub struct RutabagaHandle {
- pub os_handle: File,
+ pub os_handle: SafeDescriptor,
pub handle_type: u32,
}
diff --git a/rutabaga_gfx/src/virgl_renderer.rs b/rutabaga_gfx/src/virgl_renderer.rs
index 55c5cda..fcd2156 100644
--- a/rutabaga_gfx/src/virgl_renderer.rs
+++ b/rutabaga_gfx/src/virgl_renderer.rs
@@ -9,7 +9,6 @@
use std::cell::RefCell;
use std::ffi::CString;
-use std::fs::File;
use std::mem::{size_of, transmute};
use std::os::raw::{c_char, c_void};
use std::ptr::null_mut;
@@ -19,7 +18,7 @@
use base::{
warn, Error as SysError, ExternalMapping, ExternalMappingError, ExternalMappingResult,
- FromRawDescriptor,
+ FromRawDescriptor, SafeDescriptor,
};
use crate::generated::virgl_renderer_bindings::*;
@@ -269,7 +268,7 @@
return Err(RutabagaError::Unsupported);
}
- let dmabuf = unsafe { File::from_raw_descriptor(fd) };
+ let dmabuf = unsafe { SafeDescriptor::from_raw_descriptor(fd) };
Ok(Arc::new(RutabagaHandle {
os_handle: dmabuf,
handle_type: RUTABAGA_MEM_HANDLE_TYPE_DMABUF,
@@ -539,7 +538,7 @@
// Safe because the FD was just returned by a successful virglrenderer call so it must
// be valid and owned by us.
- let fence = unsafe { File::from_raw_descriptor(fd) };
+ let fence = unsafe { SafeDescriptor::from_raw_descriptor(fd) };
Ok(RutabagaHandle {
os_handle: fence,
handle_type: RUTABAGA_FENCE_HANDLE_TYPE_SYNC_FD,
diff --git a/sys_util/src/descriptor.rs b/sys_util/src/descriptor.rs
index 5ee075a..4c86ca2 100644
--- a/sys_util/src/descriptor.rs
+++ b/sys_util/src/descriptor.rs
@@ -113,6 +113,20 @@
}
}
+impl Into<File> for SafeDescriptor {
+ fn into(self) -> File {
+ // Safe because we own the SafeDescriptor at this point.
+ unsafe { File::from_raw_fd(self.into_raw_descriptor()) }
+ }
+}
+
+impl Into<SafeDescriptor> for File {
+ fn into(self) -> SafeDescriptor {
+ // Safe because we own the File at this point.
+ unsafe { SafeDescriptor::from_raw_descriptor(self.into_raw_descriptor()) }
+ }
+}
+
/// For use cases where a simple wrapper around a RawDescriptor is needed.
/// This is a simply a wrapper and does not manage the lifetime of the descriptor.
/// Most usages should prefer SafeDescriptor or using a RawDescriptor directly
diff --git a/sys_util/src/shm.rs b/sys_util/src/shm.rs
index d9a5839..6bfe206 100644
--- a/sys_util/src/shm.rs
+++ b/sys_util/src/shm.rs
@@ -5,7 +5,7 @@
use std::ffi::{CStr, CString};
use std::fs::{read_link, File};
use std::io::{self, Read, Seek, SeekFrom, Write};
-use std::os::unix::io::{AsRawFd, FromRawFd, RawFd};
+use std::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
use libc::{
self, c_char, c_int, c_long, c_uint, close, fcntl, ftruncate64, off64_t, syscall, EINVAL,
@@ -267,6 +267,12 @@
}
}
+impl IntoRawFd for SharedMemory {
+ fn into_raw_fd(self) -> RawFd {
+ self.fd.into_raw_fd()
+ }
+}
+
impl Into<File> for SharedMemory {
fn into(self) -> File {
self.fd
