From 22272dd4b26cf1e3797ea8f7ea0331d71cbd399a Mon Sep 17 00:00:00 2001 From: David Reveman Date: Wed, 23 May 2018 06:41:30 -0400 Subject: [PATCH] virtwl: implement dmabuf sync support This provides proper synchronization for guest access to DMABufs. Virtio wayland device is given access to the DMA_BUF_SYNC ioctl in order to implement this. Being able to use this directly in the virtio wayland device process is important as these calls can sometimes be relatively expensive and they are frequent enough that avoiding another context switch is useful for good performance. TEST=cache-line artifacts no longer noticeable BUG=chromium:837209 Change-Id: Ibb8d7c01f70ed5b74afd69288015a65186fec52a Reviewed-on: https://chromium-review.googlesource.com/1076928 Commit-Ready: David Reveman Tested-by: David Reveman Reviewed-by: Zach Reizner --- devices/src/virtio/wl.rs | 87 +++++++++++++++++++++++++++++++- seccomp/aarch64/wl_device.policy | 4 +- seccomp/x86_64/wl_device.policy | 8 ++- 3 files changed, 94 insertions(+), 5 deletions(-) diff --git a/devices/src/virtio/wl.rs b/devices/src/virtio/wl.rs index 956163ef0c..dc736defd2 100644 --- a/devices/src/virtio/wl.rs +++ b/devices/src/virtio/wl.rs @@ -38,6 +38,8 @@ use std::fmt; use std::fs::File; use std::io::{self, Seek, SeekFrom, Read}; use std::mem::{size_of, size_of_val}; +#[cfg(feature = "wl-dmabuf")] +use std::os::raw::{c_uint, c_ulonglong}; use std::os::unix::io::{AsRawFd, RawFd}; #[cfg(feature = "wl-dmabuf")] use std::os::unix::io::FromRawFd; @@ -51,7 +53,7 @@ use std::thread; use std::time::Duration; #[cfg(feature = "wl-dmabuf")] -use libc::dup; +use libc::{dup, EBADF, EINVAL}; use data_model::*; use data_model::VolatileMemoryError; @@ -59,6 +61,9 @@ use data_model::VolatileMemoryError; use sys_util::{Error, Result, EventFd, Scm, SharedMemory, GuestAddress, GuestMemory, GuestMemoryError, PollContext, PollToken, FileFlags, pipe}; +#[cfg(feature = "wl-dmabuf")] +use sys_util::ioctl_with_ref; + use vm_control::{VmControlError, VmRequest, VmResponse, MaybeOwnedFd, GpuMemoryDesc}; use super::{VirtioDevice, Queue, DescriptorChain, INTERRUPT_STATUS_USED_RING, TYPE_WL}; @@ -72,6 +77,8 @@ const VIRTIO_WL_CMD_VFD_NEW_PIPE: u32 = 261; const VIRTIO_WL_CMD_VFD_HUP: u32 = 262; #[cfg(feature = "wl-dmabuf")] const VIRTIO_WL_CMD_VFD_NEW_DMABUF: u32 = 263; +#[cfg(feature = "wl-dmabuf")] +const VIRTIO_WL_CMD_VFD_DMABUF_SYNC: u32 = 264; const VIRTIO_WL_RESP_OK: u32 = 4096; const VIRTIO_WL_RESP_VFD_NEW: u32 = 4097; #[cfg(feature = "wl-dmabuf")] @@ -95,6 +102,22 @@ const NEXT_VFD_ID_BASE: u32 = 0x40000000; const VFD_ID_HOST_MASK: u32 = NEXT_VFD_ID_BASE; const IN_BUFFER_LEN: usize = 4080; +#[cfg(feature = "wl-dmabuf")] +const VIRTIO_WL_VFD_DMABUF_SYNC_VALID_FLAG_MASK: u32 = 0x7; + +#[cfg(feature = "wl-dmabuf")] +const DMA_BUF_IOCTL_BASE: c_uint = 0x62; + +#[cfg(feature = "wl-dmabuf")] +#[repr(C)] +#[derive(Copy, Clone)] +struct dma_buf_sync { + flags: c_ulonglong, +} + +#[cfg(feature = "wl-dmabuf")] +ioctl_iow_nr!(DMA_BUF_IOCTL_SYNC, DMA_BUF_IOCTL_BASE, 0, dma_buf_sync); + const PAGE_MASK: u64 = 0x0fff; fn round_to_page_size(v: u64) -> u64 { @@ -158,6 +181,20 @@ fn parse_new_dmabuf(addr: GuestAddress, mem: &GuestMemory) -> WlResult { }) } +#[cfg(feature = "wl-dmabuf")] +fn parse_dmabuf_sync(addr: GuestAddress, mem: &GuestMemory) -> WlResult { + const ID_OFFSET: u64 = 8; + const FLAGS_OFFSET: u64 = 12; + let id: Le32 = mem.read_obj_from_addr(mem.checked_offset(addr, ID_OFFSET) + .ok_or(WlError::CheckedOffset)?)?; + let flags: Le32 = + mem.read_obj_from_addr(mem.checked_offset(addr, FLAGS_OFFSET) + .ok_or(WlError::CheckedOffset)?)?; + Ok(WlOp::DmabufSync { + id: id.into(), + flags: flags.into(), + }) +} fn parse_send(addr: GuestAddress, len: u32, mem: &GuestMemory) -> WlResult { const ID_OFFSET: u64 = 8; @@ -200,6 +237,8 @@ fn parse_desc(desc: &DescriptorChain, mem: &GuestMemory) -> WlResult { VIRTIO_WL_CMD_VFD_NEW_PIPE => parse_new_pipe(desc.addr, mem), #[cfg(feature = "wl-dmabuf")] VIRTIO_WL_CMD_VFD_NEW_DMABUF => parse_new_dmabuf(desc.addr, mem), + #[cfg(feature = "wl-dmabuf")] + VIRTIO_WL_CMD_VFD_DMABUF_SYNC => parse_dmabuf_sync(desc.addr, mem), v => Ok(WlOp::InvalidCommand { op_type: v }), } } @@ -350,6 +389,7 @@ enum WlError { RecvVfd(Error), ReadPipe(io::Error), PollContextAdd(Error), + DmabufSync(io::Error), } impl fmt::Display for WlError { @@ -376,6 +416,7 @@ impl error::Error for WlError { WlError::RecvVfd(_) => "Failed to recv on a socket", WlError::ReadPipe(_) => "Failed to read a pipe", WlError::PollContextAdd(_) => "Failed to listen to FD on poll context", + WlError::DmabufSync(_) => "Failed to synchronize DMABuf access", } } } @@ -490,6 +531,8 @@ enum WlOp { NewPipe { id: u32, flags: u32 }, #[cfg(feature = "wl-dmabuf")] NewDmabuf { id: u32, width: u32, height: u32, format: u32 }, + #[cfg(feature = "wl-dmabuf")] + DmabufSync { id: u32, flags: u32 }, InvalidCommand { op_type: u32 }, } @@ -560,6 +603,8 @@ struct WlVfd { remote_pipe: Option, local_pipe: Option<(u32 /* flags */, File)>, slot: Option<(u32 /* slot */, u64 /* pfn */, VmRequester)>, + #[cfg(feature = "wl-dmabuf")] + is_dmabuf: bool, } impl fmt::Debug for WlVfd { @@ -630,12 +675,35 @@ impl WlVfd { let vfd_shm = SharedMemory::from_raw_fd(raw_fd).map_err(WlError::NewAlloc)?; vfd.guest_shared_memory = Some((vfd_shm.size(), vfd_shm.into())); vfd.slot = Some((slot, pfn, vm)); + vfd.is_dmabuf = true; Ok((vfd, desc)) } _ => Err(WlError::VmBadResponse), } } + #[cfg(feature = "wl-dmabuf")] + fn dmabuf_sync(&self, flags: u32) -> WlResult<()> { + if !self.is_dmabuf { + return Err(WlError::DmabufSync(io::Error::from_raw_os_error(EINVAL))); + } + + match self.guest_shared_memory.as_ref() { + Some(&(_, ref fd)) => { + let sync = dma_buf_sync { + flags: flags as u64, + }; + // Safe as fd is a valid dmabuf and incorrect flags will return an error. + if unsafe { ioctl_with_ref(fd, DMA_BUF_IOCTL_SYNC(), &sync) } < 0 { + Err(WlError::DmabufSync(io::Error::last_os_error())) + } else { + Ok(()) + } + } + None => Err(WlError::DmabufSync(io::Error::from_raw_os_error(EBADF))) + } + } + fn pipe_remote_read_local_write() -> WlResult { let (read_pipe, write_pipe) = pipe(true).map_err(WlError::NewPipe)?; let mut vfd = WlVfd::default(); @@ -949,6 +1017,21 @@ impl WlState { } } + #[cfg(feature = "wl-dmabuf")] + fn dmabuf_sync(&mut self, vfd_id: u32, flags: u32) -> WlResult { + if flags & !(VIRTIO_WL_VFD_DMABUF_SYNC_VALID_FLAG_MASK) != 0 { + return Ok(WlResp::InvalidFlags); + } + + match self.vfds.get_mut(&vfd_id) { + Some(vfd) => { + vfd.dmabuf_sync(flags)?; + Ok(WlResp::Ok) + } + None => Ok(WlResp::InvalidId), + } + } + fn new_context(&mut self, id: u32) -> WlResult { if id & VFD_ID_HOST_MASK != 0 { return Ok(WlResp::InvalidId); @@ -1112,6 +1195,8 @@ impl WlState { WlOp::NewPipe { id, flags } => self.new_pipe(id, flags), #[cfg(feature = "wl-dmabuf")] WlOp::NewDmabuf { id, width, height, format } => self.new_dmabuf(id, width, height, format), + #[cfg(feature = "wl-dmabuf")] + WlOp::DmabufSync { id, flags } => self.dmabuf_sync(id, flags), WlOp::InvalidCommand { op_type } => { warn!("unexpected command {}", op_type); Ok(WlResp::InvalidCommand) diff --git a/seccomp/aarch64/wl_device.policy b/seccomp/aarch64/wl_device.policy index 9f211695bc..52bf028e85 100644 --- a/seccomp/aarch64/wl_device.policy +++ b/seccomp/aarch64/wl_device.policy @@ -30,8 +30,8 @@ write: 1 eventfd2: 1 # Used to connect to wayland. arg0 == AF_UNIX && arg1 == SOCK_STREAM|SOCK_CLOEXEC socket: arg0 == 1 && arg1 == 0x80001 && arg2 == 0 -# arg1 == FIONBIO -ioctl: arg1 == 0x5421 +# arg1 == FIONBIO || arg1 == DMA_BUF_IOCTL_SYNC +ioctl: arg1 == 0x5421 || arg1 == 0x40086200 connect: arg2 == 13 # Used to communicate with wayland recvmsg: 1 diff --git a/seccomp/x86_64/wl_device.policy b/seccomp/x86_64/wl_device.policy index 045e40fc1a..cb8d6e43ba 100644 --- a/seccomp/x86_64/wl_device.policy +++ b/seccomp/x86_64/wl_device.policy @@ -1,3 +1,7 @@ +# Copyright 2018 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. + close: 1 dup: 1 dup2: 1 @@ -25,8 +29,8 @@ write: 1 eventfd2: 1 # Used to connect to wayland. arg0 == AF_UNIX && arg1 == SOCK_STREAM|SOCK_CLOEXEC socket: arg0 == 1 && arg1 == 0x80001 && arg2 == 0 -# arg1 == FIONBIO -ioctl: arg1 == 0x5421 +# arg1 == FIONBIO || arg1 == DMA_BUF_IOCTL_SYNC +ioctl: arg1 == 0x5421 || arg1 == 0x40086200 connect: arg2 == 13 # Used to communicate with wayland recvmsg: 1