Use simple virtio_input_events where possible.

Previously, all input events in CrosVM were required to be linux
input_events, which have a timestamp field that is actually unused by
when we send/receive from the guest which are of type
virtio_input_event. This CL allows CrosVM to understand both types of input
events in a first class manner. It is a follow up on
https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1930405.

This CL also addresses some bugs with window driven input:
1. attach_event_device was being called before the surface was
created, so the devices were never attached.
2. The default touchpad size was not being set to the display window
size.

Additionally, it removes the unused event "filter" feature on event
sources.

Breaking change: from this point forward, CrosVM will treat input events sent
via a socket (e.g. SocketEventSource) to be virtio_input_events.

BUG=None
TEST=builds + manual

Change-Id: I7fec07c582e5a071a6f116975ba70d6e621bb483
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2034046
Reviewed-by: Zach Reizner <zachr@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Noah Gold <nkgold@google.com>
This commit is contained in:
Noah Gold 2020-02-01 13:01:58 -08:00 committed by Commit Bot
parent d2a862b41f
commit dc7f52bdb7
8 changed files with 157 additions and 163 deletions

View file

@ -59,6 +59,7 @@ impl VirtioBackend {
pub fn import_event_device(&mut self, event_device: EventDevice, scanout: u32) {
// TODO(zachr): support more than one scanout.
if scanout != 0 {
error!("got nonzero scanout: {:}, but only support zero.", scanout);
return;
}
@ -105,6 +106,9 @@ impl VirtioBackend {
match display.create_surface(None, self.display_width, self.display_height) {
Ok(surface_id) => {
self.scanout_surface_id = Some(surface_id);
for (event_device_id, _) in &self.event_devices {
display.attach_event_device(surface_id, *event_device_id);
}
}
Err(e) => error!("failed to create display surface: {}", e),
}

View file

@ -4,32 +4,15 @@
use super::constants::*;
use super::evdev::{grab_evdev, ungrab_evdev};
use super::virtio_input_event;
use super::InputError;
use super::Result;
use data_model::DataInit;
use linux_input_sys::input_event;
use linux_input_sys::{input_event, virtio_input_event, InputEventDecoder};
use std::collections::VecDeque;
use std::io::Read;
use std::io::Write;
use std::mem::size_of;
use std::os::unix::io::{AsRawFd, RawFd};
use sys_util::{error, warn};
trait ConvertFromVirtioInputEvent {
fn from_virtio_input_event(other: &virtio_input_event) -> input_event;
}
impl ConvertFromVirtioInputEvent for input_event {
fn from_virtio_input_event(other: &virtio_input_event) -> input_event {
input_event {
timestamp_fields: [0, 0],
type_: other.type_.into(),
code: other.code.into(),
value: other.value.into(),
}
}
}
use sys_util::warn;
/// Encapsulates a socket or device node into an abstract event source, providing a common
/// interface.
@ -58,21 +41,11 @@ pub trait EventSource: AsRawFd {
fn send_event(&mut self, vio_evt: &virtio_input_event) -> Result<()>;
}
// Try to read 16 events at a time to match what the linux guest driver does.
const READ_BUFFER_SIZE: usize = 16 * size_of::<input_event>();
// The read buffer needs to be aligned to the alignment of input_event, which is aligned as u64
#[repr(align(8))]
pub struct ReadBuffer {
buffer: [u8; READ_BUFFER_SIZE],
}
/// Encapsulates implementation details common to all kinds of event sources.
pub struct EventSourceImpl<T> {
source: T,
queue: VecDeque<virtio_input_event>,
read_buffer: ReadBuffer,
// The read index accounts for incomplete events read previously.
read_buffer: Vec<u8>,
read_idx: usize,
}
@ -86,38 +59,19 @@ impl<T> EventSourceImpl<T>
where
T: Read + Write,
{
// Receive events from the source and store them in a queue, unless they should be filtered out.
fn receive_events<F: Fn(&input_event) -> bool>(&mut self, event_filter: F) -> Result<usize> {
// Receive events from the source and store them in a queue.
fn receive_events<E: InputEventDecoder>(&mut self) -> Result<usize> {
let read = self
.source
.read(&mut self.read_buffer.buffer[self.read_idx..])
.read(&mut self.read_buffer[self.read_idx..])
.map_err(InputError::EventsReadError)?;
let buff_size = read + self.read_idx;
for evt_slice in self.read_buffer.buffer[..buff_size].chunks_exact(input_event::EVENT_SIZE)
{
let input_evt = match input_event::from_slice(evt_slice) {
Some(x) => x,
None => {
// This shouldn't happen because all slices (even the last one) are guaranteed
// to have the correct size and be properly aligned.
error!(
"Failed converting a slice of sice {} to input_event",
evt_slice.len()
);
// Skipping the event here effectively means no events will be received, because
// if from_slice fails once it will fail always.
continue;
}
};
if !event_filter(&input_evt) {
continue;
}
let vio_evt = virtio_input_event::from_input_event(input_evt);
self.queue.push_back(vio_evt);
for evt_slice in self.read_buffer[..buff_size].chunks_exact(E::SIZE) {
self.queue.push_back(E::decode(evt_slice));
}
let remainder = buff_size % input_event::EVENT_SIZE;
let remainder = buff_size % E::SIZE;
// If there is an incomplete event at the end of the buffer, it needs to be moved to the
// beginning and the next read operation must write right after it.
if remainder != 0 {
@ -125,13 +79,13 @@ where
// The copy should only happen if there is at least one complete event in the buffer,
// otherwise source and destination would be the same.
if buff_size != remainder {
let (des, src) = self.read_buffer.buffer.split_at_mut(buff_size - remainder);
let (des, src) = self.read_buffer.split_at_mut(buff_size - remainder);
des[..remainder].copy_from_slice(&src[..remainder]);
}
}
self.read_idx = remainder;
let received_events = buff_size / input_event::EVENT_SIZE;
let received_events = buff_size / E::SIZE;
Ok(received_events)
}
@ -144,32 +98,42 @@ where
self.queue.pop_front()
}
fn send_event(&mut self, vio_evt: &virtio_input_event) -> Result<()> {
let evt = input_event::from_virtio_input_event(vio_evt);
fn send_event(&mut self, vio_evt: &virtio_input_event, encoding: EventType) -> Result<()> {
// Miscellaneous events produced by the device are sent back to it by the kernel input
// subsystem, but because these events are handled by the host kernel as well as the
// guest the device would get them twice. Which would prompt the device to send the
// event to the guest again entering an infinite loop.
if evt.type_ != EV_MSC {
if vio_evt.type_ != EV_MSC {
let evt;
let event_bytes = match encoding {
EventType::InputEvent => {
evt = input_event::from_virtio_input_event(vio_evt);
evt.as_slice()
}
EventType::VirtioInputEvent => vio_evt.as_slice(),
};
self.source
.write_all(evt.as_slice())
.write_all(event_bytes)
.map_err(InputError::EventsWriteError)?;
}
Ok(())
}
fn new(source: T) -> EventSourceImpl<T> {
fn new(source: T, capacity: usize) -> EventSourceImpl<T> {
EventSourceImpl {
source,
queue: VecDeque::new(),
read_buffer: ReadBuffer {
buffer: [0u8; READ_BUFFER_SIZE],
},
read_buffer: vec![0; capacity],
read_idx: 0,
}
}
}
enum EventType {
VirtioInputEvent,
InputEvent,
}
/// Encapsulates a (unix) socket as an event source.
pub struct SocketEventSource<T> {
evt_source_impl: EventSourceImpl<T>,
@ -181,7 +145,7 @@ where
{
pub fn new(source: T) -> SocketEventSource<T> {
SocketEventSource {
evt_source_impl: EventSourceImpl::new(source),
evt_source_impl: EventSourceImpl::new(source, 16 * virtio_input_event::SIZE),
}
}
}
@ -205,7 +169,7 @@ where
}
fn receive_events(&mut self) -> Result<usize> {
self.evt_source_impl.receive_events(|_evt| true)
self.evt_source_impl.receive_events::<virtio_input_event>()
}
fn available_events_count(&self) -> usize {
@ -217,7 +181,8 @@ where
}
fn send_event(&mut self, vio_evt: &virtio_input_event) -> Result<()> {
self.evt_source_impl.send_event(vio_evt)
self.evt_source_impl
.send_event(vio_evt, EventType::VirtioInputEvent)
}
}
@ -232,7 +197,7 @@ where
{
pub fn new(source: T) -> EvdevEventSource<T> {
EvdevEventSource {
evt_source_impl: EventSourceImpl::new(source),
evt_source_impl: EventSourceImpl::new(source, 16 * input_event::SIZE),
}
}
}
@ -256,7 +221,7 @@ where
}
fn receive_events(&mut self) -> Result<usize> {
self.evt_source_impl.receive_events(|_evt| true)
self.evt_source_impl.receive_events::<input_event>()
}
fn available_events_count(&self) -> usize {
@ -268,19 +233,20 @@ where
}
fn send_event(&mut self, vio_evt: &virtio_input_event) -> Result<()> {
self.evt_source_impl.send_event(vio_evt)
self.evt_source_impl
.send_event(vio_evt, EventType::InputEvent)
}
}
#[cfg(test)]
mod tests {
use crate::virtio::input::event_source::input_event;
use crate::virtio::input::event_source::EventSourceImpl;
use crate::virtio::input::virtio_input_event;
use data_model::{DataInit, Le16, Le32};
use std::cmp::min;
use std::io::Read;
use std::io::Write;
use std::io::{Read, Write};
use data_model::{DataInit, Le16, Le32};
use linux_input_sys::InputEventDecoder;
use crate::virtio::input::event_source::{input_event, virtio_input_event, EventSourceImpl};
struct SourceMock {
events: Vec<u8>,
@ -317,7 +283,7 @@ mod tests {
#[test]
fn empty_new() {
let mut source = EventSourceImpl::new(SourceMock::new(&vec![]));
let mut source = EventSourceImpl::new(SourceMock::new(&vec![]), 128);
assert_eq!(
source.available_events(),
0,
@ -332,9 +298,9 @@ mod tests {
#[test]
fn empty_receive() {
let mut source = EventSourceImpl::new(SourceMock::new(&vec![]));
let mut source = EventSourceImpl::new(SourceMock::new(&vec![]), 128);
assert_eq!(
source.receive_events(|_| true).unwrap(),
source.receive_events::<input_event>().unwrap(),
0,
"zero events should be received"
);
@ -367,9 +333,9 @@ mod tests {
#[test]
fn partial_pop() {
let evts = instantiate_input_events(4usize);
let mut source = EventSourceImpl::new(SourceMock::new(&evts));
let mut source = EventSourceImpl::new(SourceMock::new(&evts), input_event::SIZE * 4);
assert_eq!(
source.receive_events(|_| true).unwrap(),
source.receive_events::<input_event>().unwrap(),
evts.len(),
"should receive all events"
);
@ -383,9 +349,9 @@ mod tests {
fn total_pop() {
const EVENT_COUNT: usize = 4;
let evts = instantiate_input_events(EVENT_COUNT);
let mut source = EventSourceImpl::new(SourceMock::new(&evts));
let mut source = EventSourceImpl::new(SourceMock::new(&evts), input_event::SIZE * 4);
assert_eq!(
source.receive_events(|_| true).unwrap(),
source.receive_events::<input_event>().unwrap(),
evts.len(),
"should receive all events"
);

View file

@ -20,12 +20,11 @@ use super::{
copy_config, DescriptorChain, DescriptorError, Interrupt, Queue, Reader, VirtioDevice, Writer,
TYPE_INPUT,
};
use linux_input_sys::input_event;
use linux_input_sys::{virtio_input_event, InputEventDecoder};
use std::collections::BTreeMap;
use std::fmt::{self, Display};
use std::io::Read;
use std::io::Write;
use std::mem::size_of;
use std::thread;
const EVENT_QUEUE_SIZE: u16 = 64;
@ -341,29 +340,6 @@ impl VirtioInputConfig {
}
}
#[derive(Copy, Clone, Debug, Default)]
#[repr(C)]
pub struct virtio_input_event {
type_: Le16,
code: Le16,
value: Le32,
}
// Safe because it only has data and has no implicit padding.
unsafe impl DataInit for virtio_input_event {}
impl virtio_input_event {
const EVENT_SIZE: usize = size_of::<virtio_input_event>();
fn from_input_event(other: &input_event) -> virtio_input_event {
virtio_input_event {
type_: Le16::from(other.type_),
code: Le16::from(other.code),
value: Le32::from(other.value),
}
}
}
struct Worker<T: EventSource> {
interrupt: Interrupt,
event_source: T,
@ -381,7 +357,7 @@ impl<T: EventSource> Worker<T> {
) -> Result<usize> {
let mut writer = Writer::new(mem, avail_desc).map_err(InputError::Descriptor)?;
while writer.available_bytes() >= virtio_input_event::EVENT_SIZE {
while writer.available_bytes() >= virtio_input_event::SIZE {
if let Some(evt) = event_source.pop_available_event() {
writer.write_obj(evt).map_err(InputError::WriteQueue)?;
} else {
@ -437,7 +413,7 @@ impl<T: EventSource> Worker<T> {
mem: &GuestMemory,
) -> Result<usize> {
let mut reader = Reader::new(mem, avail_desc).map_err(InputError::Descriptor)?;
while reader.available_bytes() >= virtio_input_event::EVENT_SIZE {
while reader.available_bytes() >= virtio_input_event::SIZE {
let evt: virtio_input_event = reader.read_obj().map_err(InputError::ReadQueue)?;
event_source.send_event(&evt)?;
}

View file

@ -3,14 +3,14 @@
// found in the LICENSE file.
use data_model::DataInit;
use linux_input_sys::input_event;
use linux_input_sys::{virtio_input_event, InputEventDecoder};
use std::collections::VecDeque;
use std::io::{self, Error, ErrorKind, Read, Write};
use std::iter::ExactSizeIterator;
use std::os::unix::io::{AsRawFd, RawFd};
use std::os::unix::net::UnixStream;
const EVENT_SIZE: usize = input_event::EVENT_SIZE;
const EVENT_SIZE: usize = virtio_input_event::SIZE;
const EVENT_BUFFER_LEN_MAX: usize = 16 * EVENT_SIZE;
// /// Half-way build `EventDevice` with only the `event_socket` defined. Finish building the
@ -93,7 +93,7 @@ impl EventDevice {
self.event_buffer.is_empty()
}
pub fn send_report<E: IntoIterator<Item = input_event>>(
pub fn send_report<E: IntoIterator<Item = virtio_input_event>>(
&mut self,
events: E,
) -> io::Result<bool>
@ -111,14 +111,14 @@ impl EventDevice {
}
self.event_buffer
.extend(input_event::syn().as_slice().iter());
.extend(virtio_input_event::syn().as_slice().iter());
self.flush_buffered_events()
}
/// Sends the given `event`, returning `Ok(true)` if, after this function returns, there are no
/// buffered events remaining.
pub fn send_event_encoded(&mut self, event: input_event) -> io::Result<bool> {
pub fn send_event_encoded(&mut self, event: virtio_input_event) -> io::Result<bool> {
if !self.flush_buffered_events()? {
return Ok(false);
}
@ -137,14 +137,14 @@ impl EventDevice {
Ok(false)
}
pub fn recv_event_encoded(&self) -> io::Result<input_event> {
pub fn recv_event_encoded(&self) -> io::Result<virtio_input_event> {
let mut event_bytes = [0u8; 24];
(&self.event_socket).read_exact(&mut event_bytes)?;
match input_event::from_slice(&event_bytes) {
match virtio_input_event::from_slice(&event_bytes) {
Some(event) => Ok(*event),
None => Err(Error::new(
ErrorKind::InvalidInput,
"failed to read input_event",
"failed to read virtio_input_event",
)),
}
}

View file

@ -11,7 +11,7 @@
)]
mod xlib;
use linux_input_sys::input_event;
use linux_input_sys::virtio_input_event;
use std::cmp::max;
use std::collections::BTreeMap;
use std::ffi::{c_void, CStr, CString};
@ -333,7 +333,11 @@ impl Surface {
}
}
fn dispatch_to_event_devices(&mut self, events: &[input_event], device_type: EventDeviceKind) {
fn dispatch_to_event_devices(
&mut self,
events: &[virtio_input_event],
device_type: EventDeviceKind,
) {
for event_device in self.event_devices.values_mut() {
if event_device.kind() != device_type {
continue;
@ -348,7 +352,7 @@ impl Surface {
match ev.as_enum(self.buffer_completion_type) {
XEventEnum::KeyEvent(key) => {
if let Some(linux_keycode) = self.keycode_translator.translate(key.keycode) {
let events = &[input_event::key(
let events = &[virtio_input_event::key(
linux_keycode,
key.type_ == xlib::KeyPress as i32,
)];
@ -363,9 +367,9 @@ impl Surface {
if button_event.button & xlib::Button1 != 0 {
// The touch event *must* be first per the Linux input subsystem's guidance.
let events = &[
input_event::touch(pressed),
input_event::absolute_x(max(0, button_event.x) as u32),
input_event::absolute_y(max(0, button_event.y) as u32),
virtio_input_event::touch(pressed),
virtio_input_event::absolute_x(max(0, button_event.x) as u32),
virtio_input_event::absolute_y(max(0, button_event.y) as u32),
];
self.dispatch_to_event_devices(events, EventDeviceKind::Touchscreen);
}
@ -373,9 +377,9 @@ impl Surface {
XEventEnum::Motion(motion) => {
if motion.state & xlib::Button1Mask != 0 {
let events = &[
input_event::touch(true),
input_event::absolute_x(max(0, motion.x) as u32),
input_event::absolute_y(max(0, motion.y) as u32),
virtio_input_event::touch(true),
virtio_input_event::absolute_x(max(0, motion.x) as u32),
virtio_input_event::absolute_y(max(0, motion.y) as u32),
];
self.dispatch_to_event_devices(events, EventDeviceKind::Touchscreen);
}

View file

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
use data_model::DataInit;
use data_model::{DataInit, Le16, Le32};
use std::mem::size_of;
const EV_SYN: u16 = 0x00;
@ -20,6 +20,13 @@ const ABS_Y: u16 = 0x01;
const BTN_TOUCH: u16 = 0x14a;
const BTN_TOOL_FINGER: u16 = 0x145;
/// Allows a raw input event of the implementor's type to be decoded into
/// a virtio_input_event.
pub trait InputEventDecoder {
const SIZE: usize;
fn decode(data: &[u8]) -> virtio_input_event;
}
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
#[repr(C)]
pub struct input_event {
@ -32,55 +39,99 @@ pub struct input_event {
unsafe impl DataInit for input_event {}
impl input_event {
pub const EVENT_SIZE: usize = size_of::<input_event>();
#[inline]
pub fn syn() -> input_event {
pub fn from_virtio_input_event(other: &virtio_input_event) -> input_event {
input_event {
timestamp_fields: [0, 0],
type_: EV_SYN,
code: SYN_REPORT,
value: 0,
type_: other.type_.into(),
code: other.code.into(),
value: other.value.into(),
}
}
}
impl InputEventDecoder for input_event {
const SIZE: usize = size_of::<Self>();
fn decode(data: &[u8]) -> virtio_input_event {
#[repr(align(8))]
struct Aligner([u8; input_event::SIZE]);
let data_aligned = Aligner(*<[u8; input_event::SIZE]>::from_slice(data).unwrap());
let e = Self::from_slice(&data_aligned.0).unwrap();
virtio_input_event {
type_: Le16::from(e.type_),
code: Le16::from(e.code),
value: Le32::from(e.value),
}
}
}
#[derive(Copy, Clone, Debug, Default, Eq, PartialEq)]
#[repr(C)]
pub struct virtio_input_event {
pub type_: Le16,
pub code: Le16,
pub value: Le32,
}
// Safe because it only has data and has no implicit padding.
unsafe impl DataInit for virtio_input_event {}
impl InputEventDecoder for virtio_input_event {
const SIZE: usize = size_of::<Self>();
fn decode(data: &[u8]) -> virtio_input_event {
#[repr(align(4))]
struct Aligner([u8; virtio_input_event::SIZE]);
let data_aligned = Aligner(*<[u8; virtio_input_event::SIZE]>::from_slice(data).unwrap());
*Self::from_slice(&data_aligned.0).unwrap()
}
}
impl virtio_input_event {
#[inline]
pub fn syn() -> virtio_input_event {
virtio_input_event {
type_: Le16::from(EV_SYN),
code: Le16::from(SYN_REPORT),
value: Le32::from(0),
}
}
#[inline]
pub fn absolute(code: u16, value: u32) -> input_event {
input_event {
timestamp_fields: [0, 0],
type_: EV_ABS,
code,
value,
pub fn absolute(code: u16, value: u32) -> virtio_input_event {
virtio_input_event {
type_: Le16::from(EV_ABS),
code: Le16::from(code),
value: Le32::from(value),
}
}
#[inline]
pub fn absolute_x(x: u32) -> input_event {
pub fn absolute_x(x: u32) -> virtio_input_event {
Self::absolute(ABS_X, x)
}
#[inline]
pub fn absolute_y(y: u32) -> input_event {
pub fn absolute_y(y: u32) -> virtio_input_event {
Self::absolute(ABS_Y, y)
}
#[inline]
pub fn touch(has_contact: bool) -> input_event {
pub fn touch(has_contact: bool) -> virtio_input_event {
Self::key(BTN_TOUCH, has_contact)
}
#[inline]
pub fn finger_tool(active: bool) -> input_event {
pub fn finger_tool(active: bool) -> virtio_input_event {
Self::key(BTN_TOOL_FINGER, active)
}
#[inline]
pub fn key(code: u16, pressed: bool) -> input_event {
input_event {
timestamp_fields: [0, 0],
type_: EV_KEY,
code,
value: if pressed { 1 } else { 0 },
pub fn key(code: u16, pressed: bool) -> virtio_input_event {
virtio_input_event {
type_: Le16::from(EV_KEY),
code: Le16::from(code),
value: Le32::from(if pressed { 1 } else { 0 }),
}
}
}

View file

@ -57,8 +57,8 @@ pub struct GidMap {
pub count: u32,
}
pub const DEFAULT_TOUCH_DEVICE_WIDTH: u32 = 800;
pub const DEFAULT_TOUCH_DEVICE_HEIGHT: u32 = 1280;
pub const DEFAULT_TOUCH_DEVICE_HEIGHT: u32 = 1024;
pub const DEFAULT_TOUCH_DEVICE_WIDTH: u32 = 1280;
pub struct TouchDeviceOption {
path: PathBuf,

View file

@ -63,10 +63,6 @@ use vm_control::{
};
use crate::{Config, DiskOption, Executable, SharedDir, SharedDirKind, TouchDeviceOption};
#[cfg(feature = "gpu")]
use crate::{DEFAULT_TOUCH_DEVICE_HEIGHT, DEFAULT_TOUCH_DEVICE_WIDTH};
use arch::{self, LinuxArch, RunnableLinuxVm, VirtioDeviceStub, VmComponents, VmImage};
#[cfg(any(target_arch = "arm", target_arch = "aarch64"))]
@ -1048,19 +1044,16 @@ fn create_virtio_devices(
#[cfg(feature = "gpu")]
{
if cfg.gpu_parameters.is_some() {
if let Some(gpu_parameters) = &cfg.gpu_parameters {
let mut event_devices = Vec::new();
if cfg.display_window_mouse {
let (event_device_socket, virtio_dev_socket) =
UnixStream::pair().map_err(Error::CreateSocket)?;
// TODO(nkgold): the width/height here should match the display's height/width. When
// those settings are available as CLI options, we should use the CLI options here
// as well.
let (single_touch_width, single_touch_height) = cfg
.virtio_single_touch
.as_ref()
.map(|single_touch_spec| single_touch_spec.get_size())
.unwrap_or((DEFAULT_TOUCH_DEVICE_WIDTH, DEFAULT_TOUCH_DEVICE_HEIGHT));
.unwrap_or((gpu_parameters.display_width, gpu_parameters.display_height));
let dev = virtio::new_single_touch(
virtio_dev_socket,
single_touch_width,