From e457f0393502bf9edc880112e29a76abe0873df0 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 18 Jul 2023 18:25:32 +1000 Subject: [PATCH 1/6] virtio-gpu: handling GPU resets Signed-off-by: Anhad Singh --- Cargo.lock | 1 + inputd/src/lib.rs | 35 +++++- inputd/src/main.rs | 79 ++++++++---- vesad/src/scheme.rs | 93 +++++--------- virtio-core/src/lib.rs | 2 +- virtio-core/src/probe.rs | 50 +++++--- virtio-core/src/transport.rs | 42 ++++++- virtio-gpud/src/main.rs | 24 +++- virtio-gpud/src/scheme.rs | 235 ++++++++++++++++++++++++----------- virtio-netd/Cargo.toml | 1 + virtio-netd/src/scheme.rs | 2 + 11 files changed, 371 insertions(+), 193 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 0e17d62f53..a0d8949010 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1908,6 +1908,7 @@ dependencies = [ name = "virtio-netd" version = "0.1.0" dependencies = [ + "common", "futures", "log", "netutils", diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index 9ad4ae5876..bd9cb6655d 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -1,11 +1,12 @@ use std::fs::File; use std::io::{Error, Read}; +use std::mem; pub struct Handle(File); impl Handle { pub fn new>(device_name: S) -> Result { - let path = format!("input:handle/{}", device_name.into()); + let path = format!("input:handle/display/{}", device_name.into()); Ok(Self(File::open(path)?)) } @@ -16,6 +17,38 @@ impl Handle { } } +#[derive(Debug, Copy, Clone, PartialEq)] +#[repr(u8)] +pub enum CommandTy { + Unknown = 0, + + Activate, + Deactivate, +} + +#[repr(C)] +pub struct Command { + pub ty: CommandTy, + pub value: usize, +} + +impl Command { + pub fn new(ty: CommandTy, value: usize) -> Self { + Self { ty, value } + } + + /// ## Panics + /// This function panics if the buffer is not `sizeof::()` bytes wide. + pub fn parse<'a>(buffer: &'a [u8]) -> &'a Command { + assert_eq!(buffer.len(), core::mem::size_of::()); + unsafe { &*(buffer.as_ptr() as *const Command) } + } + + pub fn into_bytes(self) -> [u8; mem::size_of::()] { + unsafe { core::mem::transmute(self) } + } +} + #[repr(packed)] pub struct Damage { pub x: i32, diff --git a/inputd/src/main.rs b/inputd/src/main.rs index b06a0c247d..0609cb1491 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -13,10 +13,11 @@ use std::collections::BTreeMap; use std::fs::File; -use std::io::{Read, Write}; +use std::io::{self, Read, Write}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; +use inputd::{Command, CommandTy}; use orbclient::{Event, EventOption}; use syscall::{Error as SysError, EventFlags, Packet, SchemeMut, EINVAL}; @@ -26,6 +27,7 @@ enum Handle { events: EventFlags, pending: Vec, notified: bool, + vt: usize, }, Device(Arc), } @@ -66,17 +68,30 @@ impl SchemeMut for InputScheme { let mut path_parts = path.split('/'); let command = path_parts.next().ok_or(SysError::new(EINVAL))?; + let fd = self.next_id.fetch_add(1, Ordering::SeqCst); let handle_ty = match command { "producer" => Handle::Producer, - "consumer" => Handle::Consumer { - events: EventFlags::empty(), - pending: Vec::new(), - notified: false, - }, + "consumer" => { + let vt = if let Some((_, _, vt)) = self.active_vt { + vt + } else { + // No one is currently set as active, so we assume that the last allocated + // one will be willing to take this consumer. + self.next_vt_id.load(Ordering::SeqCst) - 1 + }; + + dbg!(vt); + Handle::Consumer { + events: EventFlags::empty(), + pending: Vec::new(), + notified: false, + vt, + } + } "handle" => { - let value = path_parts.next().ok_or(SysError::new(EINVAL))?; - Handle::Device(Arc::new(value.to_string())) + let value = path_parts.collect::>().join("/"); + Handle::Device(Arc::new(value)) } _ => unreachable!("inputd: invalid path {path}"), @@ -84,9 +99,7 @@ impl SchemeMut for InputScheme { log::info!("inputd: {path} channel has been opened"); - let fd = self.next_id.fetch_add(1, Ordering::SeqCst); self.handles.insert(fd, handle_ty); - Ok(fd) } @@ -156,36 +169,34 @@ impl SchemeMut for InputScheme { _ => continue, } - let deactivate = |vt, current| { - // Drop the old active VT first. - // - // TODO(andypython): This can be drastically improved by introducting something - // like ioctl() to the display scheme. - let deactivate = format!("display/{}:{current}/deactivate", vt); - let _ = File::open(&deactivate); + let deactivate = |file: &mut File, id: usize| -> io::Result<()> { + let command = Command::new(CommandTy::Deactivate, id).into_bytes(); + file.write(&command)?; + Ok(()) }; if let Some(new_active) = new_active_opt { - if let Some(vt) = self.vts.get(&new_active) { - // If the VT is already active, don't do anything. - if let Some((vt, _, current)) = self.active_vt.as_ref() { + if let Some(vt) = self.vts.get(&new_active).cloned() { + if let Some((_, file, current)) = self.active_vt.as_mut() { + // If the VT is already active, don't do anything. if new_active == *current { continue; } - deactivate(vt, *current); + deactivate(file, *current).unwrap(); } else { let id = self.next_vt_id.load(Ordering::SeqCst) - 1; - let vt = self.vts.get(&id).unwrap(); + let mut vt = File::open(self.vts.get_mut(&id).unwrap().as_ref()).unwrap(); - deactivate(vt, id); + deactivate(&mut vt, id).unwrap(); } - // Result: display/virtio-gpu:3/acvtivate - let activate = format!("display/{vt}:{new_active}/activate"); - log::info!("inputd: switching to VT #{new_active} (`{activate}`)"); + log::info!("inputd: switching to VT #{new_active} (`{vt}`)"); + + let mut file = File::open(vt.as_ref()).unwrap(); + file.write(&Command::new(CommandTy::Activate, new_active).into_bytes()) + .unwrap(); - let file = File::open(&activate).unwrap(); self.active_vt = Some((vt.clone(), file, new_active)); } else { log::warn!("inputd: switch to non-existent VT #{new_active} was requested"); @@ -269,12 +280,26 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { events, pending, ref mut notified, + vt, } = handle { if pending.is_empty() || *notified || !events.contains(EventFlags::EVENT_READ) { continue; } + let should_notify = if let Some((_, _, active_vt)) = scheme.active_vt { + active_vt == *vt + } else { + let last_allocated = scheme.next_vt_id.load(Ordering::SeqCst) - 1; + last_allocated == *vt + }; + + // The activate VT is not the same as the VT that the consumer is listening to + // for events. + if !should_notify { + continue; + } + // Notify the consumer that we have some events to read. Yum yum. let mut event_packet = Packet::default(); event_packet.a = syscall::SYS_FEVENT; diff --git a/vesad/src/scheme.rs b/vesad/src/scheme.rs index c6a17b9509..d334c79a89 100644 --- a/vesad/src/scheme.rs +++ b/vesad/src/scheme.rs @@ -43,7 +43,7 @@ pub struct DisplayScheme { impl DisplayScheme { pub fn new(mut framebuffers: Vec, spec: &[bool]) -> DisplayScheme { - let mut inputd_handle = inputd::Handle::new("vesa").unwrap(); + let mut inputd_handle = inputd::Handle::new("vesa:handle").unwrap(); let mut onscreens = Vec::new(); for fb in framebuffers.iter_mut() { @@ -146,6 +146,18 @@ impl DisplayScheme { impl SchemeMut for DisplayScheme { fn open(&mut self, path_str: &str, flags: usize, _uid: u32, _gid: u32) -> Result { + if path_str == "handle" { + let id = self.next_id; + self.next_id += 1; + self.handles.insert(id, Handle { + kind: HandleKind::Input, + flags, + events: EventFlags::empty(), + notified_read: false + }); + return Ok(id); + } + let mut parts = path_str.split('/'); let mut vt_screen = parts.next().unwrap_or("").split('.'); let vt_i = VtIndex( @@ -156,16 +168,6 @@ impl SchemeMut for DisplayScheme { ); if let Some(screens) = self.vts.get_mut(&vt_i) { if let Some(screen) = screens.get_mut(&screen_i) { - for cmd in parts { - if cmd == "activate" { - self.active = vt_i; - screen.redraw( - self.onscreens[screen_i.0], - self.framebuffers[screen_i.0].stride - ); - } - } - let id = self.next_id; self.next_id += 1; @@ -303,51 +305,14 @@ impl SchemeMut for DisplayScheme { let handle = self.handles.get(&id).ok_or(Error::new(EBADF))?; match handle.kind { - HandleKind::Input => if buf.len() == 1 && buf[0] >= 0xF4 { - let new_active = VtIndex((buf[0] - 0xF4) as usize + 1); - if let Some(screens) = self.vts.get_mut(&new_active) { - self.active = new_active; - for (screen_i, screen) in screens.iter_mut() { - screen.redraw( - self.onscreens[screen_i.0], - self.framebuffers[screen_i.0].stride - ); - } - } - Ok(1) - } else { - let events = unsafe { slice::from_raw_parts(buf.as_ptr() as *const Event, buf.len()/mem::size_of::()) }; + HandleKind::Input => { + let command = inputd::Command::parse(buf); - for event in events.iter() { - let mut new_active_opt = None; - match event.to_option() { - EventOption::Key(key_event) => match key_event.scancode { - f @ 0x3B ..= 0x44 if self.super_key => { // F1 through F10 - new_active_opt = Some(VtIndex((f - 0x3A) as usize)); - }, - 0x57 if self.super_key => { // F11 - new_active_opt = Some(VtIndex(11)); - }, - 0x58 if self.super_key => { // F12 - new_active_opt = Some(VtIndex(12)); - }, - 0x5B => { // Super - self.super_key = key_event.pressed; - }, - _ => () - }, - EventOption::Resize(resize_event) => { - let width = resize_event.width as usize; - let height = resize_event.height as usize; - let stride = width; //TODO: get stride somehow - self.resize(width, height, stride); - }, - _ => () - }; + match command.ty { + inputd::CommandTy::Activate => { + let vt_i = VtIndex(command.value); - if let Some(new_active) = new_active_opt { - if let Some(screens) = self.vts.get_mut(&new_active) { - self.active = new_active; + if let Some(screens) = self.vts.get_mut(&vt_i) { for (screen_i, screen) in screens.iter_mut() { screen.redraw( self.onscreens[screen_i.0], @@ -355,18 +320,20 @@ impl SchemeMut for DisplayScheme { ); } } - } else { - if let Some(screens) = self.vts.get_mut(&self.active) { - //TODO: send input to other screens? Don't want extra events - if let Some(screen) = screens.get_mut(&ScreenIndex(0)) { - screen.input(event); - } - } - } + + self.active = vt_i; + }, + + inputd::CommandTy::Deactivate => { + // Nothing :) + }, + + _ => unreachable!() } - Ok(events.len() * mem::size_of::()) + Ok(buf.len()) }, + HandleKind::Screen(vt_i, screen_i) => if let Some(screens) = self.vts.get_mut(&vt_i) { if let Some(screen) = screens.get_mut(&screen_i) { let count = screen.write(buf)?; diff --git a/virtio-core/src/lib.rs b/virtio-core/src/lib.rs index 5d1d6d0436..54554679a1 100644 --- a/virtio-core/src/lib.rs +++ b/virtio-core/src/lib.rs @@ -6,4 +6,4 @@ pub mod utils; mod probe; -pub use probe::{probe_device, Device, MSIX_PRIMARY_VECTOR}; +pub use probe::{probe_device, Device, MSIX_PRIMARY_VECTOR, reinit}; diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index c9fad8427e..cdb029a60c 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -11,7 +11,7 @@ use pcid_interface::*; use syscall::Io; use crate::spec::*; -use crate::transport::{Error, StandardTransport}; +use crate::transport::{Error, StandardTransport, Queue}; use crate::utils::{align_down, VolatileCell}; pub struct Device<'a> { @@ -21,6 +21,9 @@ pub struct Device<'a> { pub isr: &'a VolatileCell, } +unsafe impl Send for Device<'_> {} +unsafe impl Sync for Device<'_> {} + struct MsixInfo { pub virt_table_base: NonNull, pub capability: MsixCapability, @@ -237,21 +240,7 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result let device_space = unsafe { &mut *(device_addr as *mut u8) }; let isr = unsafe { &*(isr_addr as *mut VolatileCell) }; - // Reset the device. - common.device_status.set(DeviceStatusFlags::empty()); - // Upon reset, the device must initialize device status to 0. - assert_eq!(common.device_status.get(), DeviceStatusFlags::empty()); - log::info!("virtio: successfully reseted the device"); - - // XXX: According to the virtio specification v1.2, setting the ACKNOWLEDGE and DRIVER bits - // in `device_status` is required to be done in two steps. - common - .device_status - .set(common.device_status.get() | DeviceStatusFlags::ACKNOWLEDGE); - - common - .device_status - .set(common.device_status.get() | DeviceStatusFlags::DRIVER); + let transport = StandardTransport::new(common, notify_addr as *const u8, notify_multiplier); // Setup interrupts. let all_pci_features = pcid_handle.fetch_all_features()?; @@ -265,12 +254,33 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result log::info!("virtio: using standard PCI transport"); - let transport = StandardTransport::new(common, notify_addr as *const u8, notify_multiplier); - - Ok(Device { + let device = Device { transport, device_space, irq_handle, isr, - }) + }; + + device.transport.reset(); + reinit(& device)?; + + Ok(device) +} + +pub fn reinit<'a>(device: & Device<'a>) -> Result<(), Error> { + // XXX: According to the virtio specification v1.2, setting the ACKNOWLEDGE and DRIVER bits + // in `device_status` is required to be done in two steps. + let mut common = device.transport.common.lock().unwrap(); + let old = common.device_status.get(); + + common + .device_status + .set(old | DeviceStatusFlags::ACKNOWLEDGE); + + let old = common.device_status.get(); + common.device_status.set(old | DeviceStatusFlags::DRIVER); + + + + Ok(()) } diff --git a/virtio-core/src/transport.rs b/virtio-core/src/transport.rs index 6048d14f34..7118adbf9d 100644 --- a/virtio-core/src/transport.rs +++ b/virtio-core/src/transport.rs @@ -131,6 +131,7 @@ pub struct Queue<'a> { pub descriptor: Dma<[Descriptor]>, pub available: Available<'a>, pub used_head: AtomicU16, + vector: u16, notification_bell: &'a mut AtomicU16, descriptor_stack: crossbeam_queue::SegQueue, @@ -145,6 +146,7 @@ impl<'a> Queue<'a> { notification_bell: &'a mut AtomicU16, queue_index: u16, + vector: u16, ) -> Arc { let descriptor_stack = crossbeam_queue::SegQueue::new(); (0..descriptor.len() as u16).for_each(|i| descriptor_stack.push(i)); @@ -159,9 +161,21 @@ impl<'a> Queue<'a> { descriptor_stack, used_head: AtomicU16::new(0), sref: sref.clone(), + vector }) } + fn reinit(&self) { + self.used_head.store(0, Ordering::SeqCst); + self.available.set_head_idx(0); + + // Drain all of the available descriptors. + while let Some(_) = self.descriptor_stack.pop() {} + + // Refill the descriptor stack. + (0..self.descriptor.len() as u16).for_each(|i| self.descriptor_stack.push(i)); + } + #[must_use = "The function returns a future that must be awaited to ensure the sent request is completed."] pub fn send(&self, chain: Vec) -> PendingRequest<'a> { let mut first_descriptor: Option = None; @@ -363,7 +377,7 @@ impl Drop for Used<'_> { } pub struct StandardTransport<'a> { - common: Mutex<&'a mut CommonCfg>, + pub(crate) common: Mutex<&'a mut CommonCfg>, notify: *const u8, notify_mul: u32, @@ -383,7 +397,12 @@ impl<'a> StandardTransport<'a> { pub fn reset(&self) { let mut common = self.common.lock().unwrap(); + common.device_status.set(DeviceStatusFlags::empty()); + // Upon reset, the device must initialize device status to 0. + assert_eq!(common.device_status.get(), DeviceStatusFlags::empty()); + + log::debug!("virtio: successfully reseted device"); } pub fn check_device_feature(&self, feature: u32) -> bool { @@ -474,7 +493,7 @@ impl<'a> StandardTransport<'a> { log::info!("virtio-core: enabled queue #{queue_index} (size={queue_size})"); - let queue = Queue::new(descriptor, avail, used, notification_bell, queue_index); + let queue = Queue::new(descriptor, avail, used, notification_bell, queue_index, vector); let queue_copy = queue.clone(); let irq_fd = irq_handle.as_raw_fd(); @@ -499,4 +518,23 @@ impl<'a> StandardTransport<'a> { Ok(queue) } + + /// Re-initializes a queue; usually done after a device reset. + pub fn reinit_queue(&self, queue: Arc) { + let mut common = self.common.lock().unwrap(); + queue.reinit(); + + common.queue_select.set(queue.queue_index); + + common.queue_desc.set(queue.descriptor.physical() as u64); + common.queue_driver.set(queue.available.phys_addr() as u64); + common.queue_device.set(queue.used.phys_addr() as u64); + + // Set the MSI-X vector. + common.queue_msix_vector.set(queue.vector); + assert!(common.queue_msix_vector.get() == queue.vector); + + // Enable the queue. + common.queue_enable.set(1); + } } diff --git a/virtio-gpud/src/main.rs b/virtio-gpud/src/main.rs index bbe5675688..f22ed521da 100644 --- a/virtio-gpud/src/main.rs +++ b/virtio-gpud/src/main.rs @@ -25,10 +25,12 @@ use std::cell::UnsafeCell; use std::fs::File; use std::io::{Read, Write}; +use std::sync::Arc; use pcid_interface::PcidServerHandle; use syscall::{Packet, SchemeMut}; +use virtio_core::transport::{Queue, self}; use virtio_core::utils::VolatileCell; use virtio_core::MSIX_PRIMARY_VECTOR; @@ -356,10 +358,7 @@ pub struct SetScanout { impl SetScanout { pub fn new(scanout_id: u32, resource_id: u32, rect: GpuRect) -> Self { Self { - header: ControlHeader { - ty: VolatileCell::new(CommandTy::SetScanout), - ..Default::default() - }, + header: ControlHeader::with_ty(CommandTy::SetScanout), rect, scanout_id, @@ -394,6 +393,21 @@ impl XferToHost2d { } } +static DEVICE: spin::Once = spin::Once::new(); + +fn reinit(control_queue: Arc, cursor_queue: Arc) -> Result<(), transport::Error> { + let device = DEVICE.get().unwrap(); + + virtio_core::reinit(device)?; + device.transport.finalize_features(); + + device.transport.reinit_queue(control_queue.clone()); + device.transport.reinit_queue(cursor_queue.clone()); + + device.transport.run_device(); + Ok(()) +} + fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { let mut pcid_handle = PcidServerHandle::connect_default()?; @@ -405,7 +419,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { assert_eq!(pci_config.func.devid, 0x1050); log::info!("virtio-gpu: initiating startup sequence :^)"); - let device = virtio_core::probe_device(&mut pcid_handle)?; + let device = DEVICE.try_call_once(|| virtio_core::probe_device(&mut pcid_handle))?; let config = unsafe { &mut *(device.device_space as *mut GpuConfig) }; // Negotiate features. diff --git a/virtio-gpud/src/scheme.rs b/virtio-gpud/src/scheme.rs index 4439f647be..a07b54a8a5 100644 --- a/virtio-gpud/src/scheme.rs +++ b/virtio-gpud/src/scheme.rs @@ -1,11 +1,12 @@ use std::collections::BTreeMap; -use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering}; use std::sync::Arc; -use common::dma::Dma; use inputd::Damage; -use syscall::{Error as SysError, SchemeMut, EINVAL}; + +use common::dma::Dma; +use syscall::{Error as SysError, SchemeMut, EINVAL, EAGAIN}; use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::{Error, Queue, StandardTransport}; @@ -40,6 +41,8 @@ pub struct Display<'a> { resource_id: u32, id: usize, + + is_reseted: AtomicBool, } impl<'a> Display<'a> { @@ -61,9 +64,27 @@ impl<'a> Display<'a> { id, resource_id: RESOURCE_ALLOC.fetch_add(1, Ordering::SeqCst), + + is_reseted: AtomicBool::new(false), } } + async fn init(&self) -> Result<(), Error> { + if !self.is_reseted.load(Ordering::SeqCst) { + // The device is already initialized. + return Ok(()); + } + + self.is_reseted.store(false, Ordering::SeqCst); + + log::info!("virtio-gpu: initializing GPU after a reset"); + + crate::reinit(self.control_queue.clone(), self.cursor_queue.clone())?; + self.remap_screen().await?; + + Ok(()) + } + async fn get_fpath(&self, buffer: &mut [u8]) -> Result { let path = format!("display/virtio-gpu:3.0/{}/{}", self.width, self.height); @@ -90,29 +111,26 @@ impl<'a> Display<'a> { Ok(()) } + async fn remap_screen(&self) -> Result { + let bpp = 32; + let fb_size = (self.width as usize * self.height as usize * bpp / 8) + .next_multiple_of(syscall::PAGE_SIZE); + + let mapped = *self.mapped.get().unwrap(); + let address = unsafe {syscall::virttophys(mapped)}?; + + self.map_screen_with(0, address, fb_size, mapped).await + } + async fn map_screen(&self, offset: usize) -> Result { if let Some(mapped) = self.mapped.get() { return Ok(mapped + offset); } - // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. - let mut request = Dma::new(ResourceCreate2d::default())?; - - request.set_width(self.width); - request.set_height(self.height); - request.set_format(ResourceFormat::Bgrx); - request.set_resource_id(self.resource_id); - - self.send_request(request).await?; - - // Allocate a framebuffer from guest ram, and attach it as backing storage to the - // resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. Scatter - // lists are supported, so the framebuffer doesn’t need to be contignous in guest - // physical memory. let bpp = 32; let fb_size = (self.width as usize * self.height as usize * bpp / 8) .next_multiple_of(syscall::PAGE_SIZE); - let address = unsafe { syscall::physalloc(fb_size) }? as u64; + let address = unsafe { syscall::physalloc(fb_size) }? ; let mapped = unsafe { common::physmap( address as usize, @@ -126,9 +144,28 @@ impl<'a> Display<'a> { core::ptr::write_bytes(mapped as *mut u8, 255, fb_size); } + self.map_screen_with(offset, address, fb_size, mapped).await + } + + async fn map_screen_with(&self, offset: usize, address: usize, size: usize, mapped: usize) -> Result { + // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. + let mut request = Dma::new(ResourceCreate2d::default())?; + + request.set_width(self.width); + request.set_height(self.height); + request.set_format(ResourceFormat::Bgrx); + request.set_resource_id(self.resource_id); + + self.send_request(request).await?; + + // Use the allocated framebuffer from tthe guest ram, and attach it as backing + // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. + // + // TODO(andypython): Scatter lists are supported, so the framebuffer doesn’t need to be + // contignous in guest physical memory. let entry = Dma::new(MemEntry { - address, - length: fb_size as u32, + address: address as u64, + length: size as u32, padding: 0, })?; @@ -200,12 +237,22 @@ impl<'a> Display<'a> { // Go back to legacy mode. self.transport.reset(); + self.is_reseted.store(true, Ordering::SeqCst); + Ok(()) } } +enum Handle { + Vt(usize /* VT index */), + Input, +} + pub struct Scheme<'a> { - handles: BTreeMap>>, + vts: BTreeMap>>, + handles: BTreeMap, + /// Counter used for file descriptor allocation. + next_id: AtomicUsize, inputd_handle: inputd::Handle, displays: Vec>>, } @@ -225,9 +272,16 @@ impl<'a> Scheme<'a> { ) .await?; + let mut inputd_handle = inputd::Handle::new("virtio-gpu:handle").unwrap(); + + let mut vts = BTreeMap::new(); + vts.insert(inputd_handle.register().unwrap(), displays[0].clone()); + Ok(Self { + vts, handles: BTreeMap::new(), - inputd_handle: inputd::Handle::new("virtio-gpu").unwrap(), + next_id: AtomicUsize::new(0), + inputd_handle, displays, }) } @@ -284,56 +338,42 @@ impl<'a> Scheme<'a> { impl<'a> SchemeMut for Scheme<'a> { fn open(&mut self, path: &str, _flags: usize, _uid: u32, _gid: u32) -> syscall::Result { - dbg!(&path); + if path == "handle" { + let fd = self.next_id.fetch_add(1, Ordering::SeqCst); + self.handles.insert(fd, Handle::Input); + + return Ok(fd); + } let mut parts = path.split('/'); let mut screen = parts.next().unwrap_or("").split('.'); - static YES: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(true); - if YES.load(Ordering::SeqCst) { - YES.store(false, Ordering::SeqCst); - } - - let fd = screen.next().unwrap_or("").parse::().unwrap(); - - if path.contains("deactivate") { - log::info!("virtio-gpu: deactivating display at file #{}", fd); - - let handle = self.handles.get(&fd).unwrap(); - futures::executor::block_on(handle.detach()).unwrap(); - - return Ok(0); - } - + let vt = screen.next().unwrap_or("").parse::().unwrap(); let id = screen.next().unwrap_or("").parse::().unwrap_or(0); - dbg!(&id); + dbg!(vt, id); - let display = self.displays.get(id).ok_or(SysError::new(EINVAL))?; + if self.displays.get(id).is_none() { + return Err(SysError::new(EINVAL)); + } - let fd = self.inputd_handle.register().unwrap(); - self.handles.insert(fd, display.clone()); + let fd = self.next_id.fetch_add(1, Ordering::SeqCst); + // FIXME: The +1 is a hack. vesad smh + self.handles.insert(fd, Handle::Vt(vt + 1)); Ok(fd) } - fn dup(&mut self, _old_id: usize, _buf: &[u8]) -> syscall::Result { - todo!() - } - - fn fevent( - &mut self, - _id: usize, - _flags: syscall::EventFlags, - ) -> syscall::Result { - log::warn!("fevent is a stub!"); - Ok(syscall::EventFlags::empty()) - } - fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { - let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; - let bytes_copied = futures::executor::block_on(handle.get_fpath(buf)).unwrap(); + match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { + Handle::Vt(id) => { + let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + let bytes_copied = futures::executor::block_on(handle.get_fpath(buf)).unwrap(); - Ok(bytes_copied) + Ok(bytes_copied) + } + + Handle::Input => unreachable!(), + } } fn fmap_old(&mut self, id: usize, map: &syscall::OldMap) -> syscall::Result { @@ -349,14 +389,25 @@ impl<'a> SchemeMut for Scheme<'a> { } fn fmap(&mut self, id: usize, map: &syscall::Map) -> syscall::Result { - let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; - Ok(futures::executor::block_on(handle.map_screen(map.offset)).unwrap()) + match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { + Handle::Vt(id) => { + let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + Ok(futures::executor::block_on(handle.map_screen(map.offset)).unwrap()) + } + _ => unreachable!(), + } } fn fsync(&mut self, id: usize) -> syscall::Result { - let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; - futures::executor::block_on(handle.flush(None)).unwrap(); - Ok(0) + match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { + Handle::Vt(id) => { + let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + futures::executor::block_on(handle.flush(None)).unwrap(); + Ok(0) + } + + _ => unreachable!(), + } } fn read(&mut self, _id: usize, _buf: &mut [u8]) -> syscall::Result { @@ -366,18 +417,54 @@ impl<'a> SchemeMut for Scheme<'a> { } fn write(&mut self, id: usize, buf: &[u8]) -> syscall::Result { - let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; - let damages = unsafe { - core::slice::from_raw_parts( - buf.as_ptr() as *const Damage, - buf.len() / core::mem::size_of::(), - ) - }; + match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { + Handle::Vt(id) => { + let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; - for damage in damages { - futures::executor::block_on(handle.flush(Some(damage))).unwrap(); + // The VT is not active and the device is reseted. Ask them to try + // again later. + if handle.is_reseted.load(Ordering::SeqCst) { + return Err(SysError::new(EAGAIN)); + } + + let damages = unsafe { + core::slice::from_raw_parts( + buf.as_ptr() as *const Damage, + buf.len() / core::mem::size_of::(), + ) + }; + + for damage in damages { + futures::executor::block_on(handle.flush(Some(damage))).unwrap(); + } + + Ok(buf.len()) + } + + Handle::Input => { + let command = inputd::Command::parse(buf); + + match command.ty { + inputd::CommandTy::Activate => { + let vt = command.value; + + let display = self.vts.get(&vt).unwrap(); + futures::executor::block_on(display.init()).unwrap(); + }, + + inputd::CommandTy::Deactivate => { + let vt = command.value; + + let display = self.vts.get(&vt).ok_or(SysError::new(EINVAL))?; + futures::executor::block_on(display.detach()).unwrap(); + } + + _ => unreachable!(), + } + + Ok(buf.len()) + } } - Ok(buf.len()) } fn seek(&mut self, _id: usize, _pos: isize, _whence: usize) -> syscall::Result { diff --git a/virtio-netd/Cargo.toml b/virtio-netd/Cargo.toml index f4a116f10c..3458d7b046 100644 --- a/virtio-netd/Cargo.toml +++ b/virtio-netd/Cargo.toml @@ -10,6 +10,7 @@ futures = { version = "0.3.28", features = ["executor"] } virtio-core = { path = "../virtio-core" } pcid = { path = "../pcid" } +common = { path = "../common" } redox-daemon = "0.1" redox_syscall = "0.3" diff --git a/virtio-netd/src/scheme.rs b/virtio-netd/src/scheme.rs index 251a501ce0..829a3002e6 100644 --- a/virtio-netd/src/scheme.rs +++ b/virtio-netd/src/scheme.rs @@ -5,6 +5,8 @@ use std::sync::Arc; use syscall::Error as SysError; use syscall::*; +use common::dma::Dma; + use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::Queue; From a153c8bc1c15d76d05a70eec8f6499d041a910c0 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Thu, 20 Jul 2023 19:20:56 +1000 Subject: [PATCH 2/6] inputd: correctly pass on resize events Signed-off-by: Anhad Singh --- bgad/src/main.rs | 4 +- inputd/src/lib.rs | 107 +++++++++++++++++++++++++++++------ inputd/src/main.rs | 36 +++++++----- vboxd/src/main.rs | 2 +- vesad/src/scheme.rs | 26 ++++----- virtio-core/src/lib.rs | 2 +- virtio-core/src/probe.rs | 15 +++-- virtio-core/src/transport.rs | 22 ++++--- virtio-gpud/src/main.rs | 2 +- virtio-gpud/src/scheme.rs | 42 ++++++++------ 10 files changed, 178 insertions(+), 80 deletions(-) diff --git a/bgad/src/main.rs b/bgad/src/main.rs index 781e68ae8f..cc7570efab 100644 --- a/bgad/src/main.rs +++ b/bgad/src/main.rs @@ -37,8 +37,8 @@ fn main() { print!("{}", format!(" - BGA {}x{}\n", bga.width(), bga.height())); let mut scheme = BgaScheme { - bga: bga, - display: File::open("display/vesa:input").ok() + bga, + display: File::open("input:producer").ok() }; scheme.update_size(); diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index bd9cb6655d..3c2eb329d4 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -1,6 +1,7 @@ +#![feature(iter_next_chunk)] + use std::fs::File; use std::io::{Error, Read}; -use std::mem; pub struct Handle(File); @@ -19,33 +20,103 @@ impl Handle { #[derive(Debug, Copy, Clone, PartialEq)] #[repr(u8)] -pub enum CommandTy { +enum CmdTy { Unknown = 0, Activate, Deactivate, + Resize, } -#[repr(C)] -pub struct Command { - pub ty: CommandTy, - pub value: usize, +impl From for CmdTy { + fn from(value: u8) -> Self { + match value { + 1 => CmdTy::Activate, + 2 => CmdTy::Deactivate, + 3 => CmdTy::Resize, + _ => CmdTy::Unknown, + } + } } -impl Command { - pub fn new(ty: CommandTy, value: usize) -> Self { - Self { ty, value } - } +pub enum Cmd { + // TODO(andypython): #VT should really need to be a `u8`. + Activate(usize /* #VT */), + Deactivate(usize /* #VT */), + Resize { + // TODO(andypython): do we really need to pass the VT here? + vt: usize, - /// ## Panics - /// This function panics if the buffer is not `sizeof::()` bytes wide. - pub fn parse<'a>(buffer: &'a [u8]) -> &'a Command { - assert_eq!(buffer.len(), core::mem::size_of::()); - unsafe { &*(buffer.as_ptr() as *const Command) } - } + width: u32, + height: u32, + stride: u32, + }, +} - pub fn into_bytes(self) -> [u8; mem::size_of::()] { - unsafe { core::mem::transmute(self) } +impl Cmd { + fn ty(&self) -> CmdTy { + match self { + Cmd::Activate(_) => CmdTy::Activate, + Cmd::Deactivate(_) => CmdTy::Deactivate, + Cmd::Resize { .. } => CmdTy::Resize, + } + } +} + +pub fn send_comand(file: &mut File, command: Cmd) -> Result<(), syscall::Error> { + use std::os::fd::AsRawFd; + + let mut result = vec![]; + result.push(command.ty() as u8); + + match command { + Cmd::Activate(vt) | Cmd::Deactivate(vt) => result.extend_from_slice(&vt.to_le_bytes()), + Cmd::Resize { + vt, + width, + height, + stride, + } => { + result.extend_from_slice(&vt.to_le_bytes()); + result.extend(width.to_le_bytes()); + result.extend(height.to_le_bytes()); + result.extend(stride.to_le_bytes()); + } + }; + + let written = syscall::write(file.as_raw_fd() as usize, &result)?; + + // XXX: Ensure all of the data is written. + assert_eq!(written, result.len()); + Ok(()) +} + +pub fn parse_command(buffer: &[u8]) -> Option { + const U32_SIZE: usize = core::mem::size_of::(); + const USIZE_SIZE: usize = core::mem::size_of::(); + + let mut parser = buffer.iter().cloned(); + + let command = CmdTy::from(parser.next()?); + let vt = usize::from_le_bytes(parser.next_chunk::().ok()?); + + match command { + CmdTy::Activate => Some(Cmd::Activate(vt)), + CmdTy::Deactivate => Some(Cmd::Deactivate(vt)), + CmdTy::Resize => { + let width = parser.next_chunk::().ok()?; + let height = parser.next_chunk::().ok()?; + let stride = parser.next_chunk::().ok()?; + + Some(Cmd::Resize { + vt, + width: u32::from_le_bytes(width), + height: u32::from_le_bytes(height), + stride: u32::from_le_bytes(stride), + }) + } + + CmdTy::Unknown => None, } } diff --git a/inputd/src/main.rs b/inputd/src/main.rs index 0609cb1491..da9efd6619 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -13,11 +13,12 @@ use std::collections::BTreeMap; use std::fs::File; -use std::io::{self, Read, Write}; +use std::io::{Read, Write}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; -use inputd::{Command, CommandTy}; +use inputd::Cmd; + use orbclient::{Event, EventOption}; use syscall::{Error as SysError, EventFlags, Packet, SchemeMut, EINVAL}; @@ -166,15 +167,25 @@ impl SchemeMut for InputScheme { _ => (), }, + EventOption::Resize(resize_event) => { + if let Some((_, file, current)) = self.active_vt.as_mut() { + inputd::send_comand( + file, + Cmd::Resize { + vt: current.clone(), + width: resize_event.width, + height: resize_event.height, + + // TODO(andypython): Figure out how to get the stride. + stride: resize_event.width, + }, + )?; + } + } + _ => continue, } - let deactivate = |file: &mut File, id: usize| -> io::Result<()> { - let command = Command::new(CommandTy::Deactivate, id).into_bytes(); - file.write(&command)?; - Ok(()) - }; - if let Some(new_active) = new_active_opt { if let Some(vt) = self.vts.get(&new_active).cloned() { if let Some((_, file, current)) = self.active_vt.as_mut() { @@ -183,20 +194,19 @@ impl SchemeMut for InputScheme { continue; } - deactivate(file, *current).unwrap(); + inputd::send_comand(file, Cmd::Deactivate(*current))?; } else { let id = self.next_vt_id.load(Ordering::SeqCst) - 1; let mut vt = File::open(self.vts.get_mut(&id).unwrap().as_ref()).unwrap(); - deactivate(&mut vt, id).unwrap(); + inputd::send_comand(&mut vt, Cmd::Deactivate(id))?; } log::info!("inputd: switching to VT #{new_active} (`{vt}`)"); let mut file = File::open(vt.as_ref()).unwrap(); - file.write(&Command::new(CommandTy::Activate, new_active).into_bytes()) - .unwrap(); + inputd::send_comand(&mut file, Cmd::Activate(new_active))?; self.active_vt = Some((vt.clone(), file, new_active)); } else { log::warn!("inputd: switch to non-existent VT #{new_active} was requested"); @@ -294,7 +304,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { last_allocated == *vt }; - // The activate VT is not the same as the VT that the consumer is listening to + // The activate VT is not the same as the VT that the consumer is listening to // for events. if !should_notify { continue; diff --git a/vboxd/src/main.rs b/vboxd/src/main.rs index cb3bba6035..845464d5e4 100644 --- a/vboxd/src/main.rs +++ b/vboxd/src/main.rs @@ -208,7 +208,7 @@ fn main() { let mut width = 0; let mut height = 0; - let mut display_opt = File::open("display/vesa:input").ok(); + let mut display_opt = File::open("inputd:producer").ok(); if let Some(ref display) = display_opt { let mut buf: [u8; 4096] = [0; 4096]; if let Ok(count) = syscall::fpath(display.as_raw_fd() as usize, &mut buf) { diff --git a/vesad/src/scheme.rs b/vesad/src/scheme.rs index d334c79a89..86f29b7c9e 100644 --- a/vesad/src/scheme.rs +++ b/vesad/src/scheme.rs @@ -1,8 +1,7 @@ use std::collections::BTreeMap; use std::{mem, ptr, slice, str}; -use orbclient::{Event, EventOption}; -use syscall::{Error, EventFlags, EACCES, EBADF, EINVAL, ENOENT, Map, O_NONBLOCK, Result, SchemeMut, PAGE_SIZE}; +use syscall::{Error, EventFlags, EBADF, EINVAL, ENOENT, Map, O_NONBLOCK, Result, SchemeMut, PAGE_SIZE}; use crate::{ display::Display, @@ -37,7 +36,6 @@ pub struct DisplayScheme { pub vts: BTreeMap>>, next_id: usize, pub handles: BTreeMap, - super_key: bool, inputd_handle: inputd::Handle, } @@ -74,7 +72,6 @@ impl DisplayScheme { vts, next_id: 0, handles: BTreeMap::new(), - super_key: false, inputd_handle } } @@ -306,11 +303,13 @@ impl SchemeMut for DisplayScheme { match handle.kind { HandleKind::Input => { - let command = inputd::Command::parse(buf); + use inputd::Cmd as DisplayCommand; + + let command = inputd::parse_command(buf).unwrap(); - match command.ty { - inputd::CommandTy::Activate => { - let vt_i = VtIndex(command.value); + match command { + DisplayCommand::Activate(vt) => { + let vt_i = VtIndex(vt); if let Some(screens) = self.vts.get_mut(&vt_i) { for (screen_i, screen) in screens.iter_mut() { @@ -323,12 +322,13 @@ impl SchemeMut for DisplayScheme { self.active = vt_i; }, - - inputd::CommandTy::Deactivate => { - // Nothing :) - }, - _ => unreachable!() + DisplayCommand::Resize { width, height, stride, .. } => { + self.resize(width as usize, height as usize, stride as usize); + } + + // Nothing to do for deactivate :) + DisplayCommand::Deactivate(_) => {}, } Ok(buf.len()) diff --git a/virtio-core/src/lib.rs b/virtio-core/src/lib.rs index 54554679a1..87b737f124 100644 --- a/virtio-core/src/lib.rs +++ b/virtio-core/src/lib.rs @@ -6,4 +6,4 @@ pub mod utils; mod probe; -pub use probe::{probe_device, Device, MSIX_PRIMARY_VECTOR, reinit}; +pub use probe::{probe_device, reinit, Device, MSIX_PRIMARY_VECTOR}; diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index cdb029a60c..f97ae30f5a 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -11,7 +11,7 @@ use pcid_interface::*; use syscall::Io; use crate::spec::*; -use crate::transport::{Error, StandardTransport, Queue}; +use crate::transport::{Error, Queue, StandardTransport}; use crate::utils::{align_down, VolatileCell}; pub struct Device<'a> { @@ -190,7 +190,12 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result let size = offset + capability.length as usize; - let addr = common::physmap(aligned_addr, size, common::Prot::RW, common::MemoryType::Uncacheable)? as usize; + let addr = common::physmap( + aligned_addr, + size, + common::Prot::RW, + common::MemoryType::Uncacheable, + )? as usize; addr + offset }; @@ -262,12 +267,12 @@ pub fn probe_device<'a>(pcid_handle: &mut PcidServerHandle) -> Result }; device.transport.reset(); - reinit(& device)?; + reinit(&device)?; Ok(device) } -pub fn reinit<'a>(device: & Device<'a>) -> Result<(), Error> { +pub fn reinit<'a>(device: &Device<'a>) -> Result<(), Error> { // XXX: According to the virtio specification v1.2, setting the ACKNOWLEDGE and DRIVER bits // in `device_status` is required to be done in two steps. let mut common = device.transport.common.lock().unwrap(); @@ -280,7 +285,5 @@ pub fn reinit<'a>(device: & Device<'a>) -> Result<(), Error> { let old = common.device_status.get(); common.device_status.set(old | DeviceStatusFlags::DRIVER); - - Ok(()) } diff --git a/virtio-core/src/transport.rs b/virtio-core/src/transport.rs index 7118adbf9d..9d72b4fab0 100644 --- a/virtio-core/src/transport.rs +++ b/virtio-core/src/transport.rs @@ -161,7 +161,7 @@ impl<'a> Queue<'a> { descriptor_stack, used_head: AtomicU16::new(0), sref: sref.clone(), - vector + vector, }) } @@ -171,7 +171,7 @@ impl<'a> Queue<'a> { // Drain all of the available descriptors. while let Some(_) = self.descriptor_stack.pop() {} - + // Refill the descriptor stack. (0..self.descriptor.len() as u16).for_each(|i| self.descriptor_stack.push(i)); } @@ -244,9 +244,9 @@ impl<'a> Available<'a> { let size = size.next_multiple_of(syscall::PAGE_SIZE); // align to page size let addr = unsafe { syscall::physalloc(size) }.map_err(Error::SyscallError)?; - let virt = unsafe { - common::physmap(addr, size, common::Prot::RW, common::MemoryType::default()) - }.map_err(Error::SyscallError)?; + let virt = + unsafe { common::physmap(addr, size, common::Prot::RW, common::MemoryType::default()) } + .map_err(Error::SyscallError)?; let ring = unsafe { &mut *(virt as *mut AvailableRing) }; @@ -312,7 +312,8 @@ impl<'a> Used<'a> { let addr = unsafe { syscall::physalloc(size) }.map_err(Error::SyscallError)?; let virt = - unsafe { common::physmap(addr, size, common::Prot::RW, common::MemoryType::default()) }.map_err(Error::SyscallError)?; + unsafe { common::physmap(addr, size, common::Prot::RW, common::MemoryType::default()) } + .map_err(Error::SyscallError)?; let ring = unsafe { &mut *(virt as *mut UsedRing) }; @@ -493,7 +494,14 @@ impl<'a> StandardTransport<'a> { log::info!("virtio-core: enabled queue #{queue_index} (size={queue_size})"); - let queue = Queue::new(descriptor, avail, used, notification_bell, queue_index, vector); + let queue = Queue::new( + descriptor, + avail, + used, + notification_bell, + queue_index, + vector, + ); let queue_copy = queue.clone(); let irq_fd = irq_handle.as_raw_fd(); diff --git a/virtio-gpud/src/main.rs b/virtio-gpud/src/main.rs index f22ed521da..3fdc3eb347 100644 --- a/virtio-gpud/src/main.rs +++ b/virtio-gpud/src/main.rs @@ -30,7 +30,7 @@ use std::sync::Arc; use pcid_interface::PcidServerHandle; use syscall::{Packet, SchemeMut}; -use virtio_core::transport::{Queue, self}; +use virtio_core::transport::{self, Queue}; use virtio_core::utils::VolatileCell; use virtio_core::MSIX_PRIMARY_VECTOR; diff --git a/virtio-gpud/src/scheme.rs b/virtio-gpud/src/scheme.rs index a07b54a8a5..a42a81c51e 100644 --- a/virtio-gpud/src/scheme.rs +++ b/virtio-gpud/src/scheme.rs @@ -6,7 +6,7 @@ use std::sync::Arc; use inputd::Damage; use common::dma::Dma; -use syscall::{Error as SysError, SchemeMut, EINVAL, EAGAIN}; +use syscall::{Error as SysError, SchemeMut, EAGAIN, EINVAL}; use virtio_core::spec::{Buffer, ChainBuilder, DescriptorFlags}; use virtio_core::transport::{Error, Queue, StandardTransport}; @@ -117,8 +117,8 @@ impl<'a> Display<'a> { .next_multiple_of(syscall::PAGE_SIZE); let mapped = *self.mapped.get().unwrap(); - let address = unsafe {syscall::virttophys(mapped)}?; - + let address = unsafe { syscall::virttophys(mapped) }?; + self.map_screen_with(0, address, fb_size, mapped).await } @@ -130,7 +130,7 @@ impl<'a> Display<'a> { let bpp = 32; let fb_size = (self.width as usize * self.height as usize * bpp / 8) .next_multiple_of(syscall::PAGE_SIZE); - let address = unsafe { syscall::physalloc(fb_size) }? ; + let address = unsafe { syscall::physalloc(fb_size) }?; let mapped = unsafe { common::physmap( address as usize, @@ -147,7 +147,13 @@ impl<'a> Display<'a> { self.map_screen_with(offset, address, fb_size, mapped).await } - async fn map_screen_with(&self, offset: usize, address: usize, size: usize, mapped: usize) -> Result { + async fn map_screen_with( + &self, + offset: usize, + address: usize, + size: usize, + mapped: usize, + ) -> Result { // Create a host resource using `VIRTIO_GPU_CMD_RESOURCE_CREATE_2D`. let mut request = Dma::new(ResourceCreate2d::default())?; @@ -158,10 +164,10 @@ impl<'a> Display<'a> { self.send_request(request).await?; - // Use the allocated framebuffer from tthe guest ram, and attach it as backing - // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. + // Use the allocated framebuffer from tthe guest ram, and attach it as backing + // storage to the resource just created, using `VIRTIO_GPU_CMD_RESOURCE_ATTACH_BACKING`. // - // TODO(andypython): Scatter lists are supported, so the framebuffer doesn’t need to be + // TODO(andypython): Scatter lists are supported, so the framebuffer doesn’t need to be // contignous in guest physical memory. let entry = Dma::new(MemEntry { address: address as u64, @@ -421,7 +427,7 @@ impl<'a> SchemeMut for Scheme<'a> { Handle::Vt(id) => { let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; - // The VT is not active and the device is reseted. Ask them to try + // The VT is not active and the device is reseted. Ask them to try // again later. if handle.is_reseted.load(Ordering::SeqCst) { return Err(SysError::new(EAGAIN)); @@ -442,24 +448,24 @@ impl<'a> SchemeMut for Scheme<'a> { } Handle::Input => { - let command = inputd::Command::parse(buf); + use inputd::Cmd as DisplayCommand; - match command.ty { - inputd::CommandTy::Activate => { - let vt = command.value; + let command = inputd::parse_command(buf).unwrap(); + match command { + DisplayCommand::Activate(vt) => { let display = self.vts.get(&vt).unwrap(); futures::executor::block_on(display.init()).unwrap(); - }, - - inputd::CommandTy::Deactivate => { - let vt = command.value; + } + DisplayCommand::Deactivate(vt) => { let display = self.vts.get(&vt).ok_or(SysError::new(EINVAL))?; futures::executor::block_on(display.detach()).unwrap(); } - _ => unreachable!(), + DisplayCommand::Resize { .. } => { + log::warn!("virtio-gpu: resize is not implemented yet") + } } Ok(buf.len()) From 440d6381f9423995b5c2cb37a48e11a53b87ab0d Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Thu, 20 Jul 2023 20:04:43 +1000 Subject: [PATCH 3/6] inputd: rework display events Signed-off-by: Anhad Singh --- Cargo.lock | 1 + inputd/Cargo.toml | 1 + inputd/src/lib.rs | 6 +- inputd/src/main.rs | 197 +++++++++++++++++++++++++++----------- vesad/src/main.rs | 2 + vesad/src/scheme.rs | 6 +- virtio-gpud/src/main.rs | 2 + virtio-gpud/src/scheme.rs | 18 ++-- 8 files changed, 166 insertions(+), 67 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0d8949010..67b819364a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -640,6 +640,7 @@ dependencies = [ "redox-daemon", "redox-log", "redox_syscall 0.3.5", + "spin", ] [[package]] diff --git a/inputd/Cargo.toml b/inputd/Cargo.toml index dfc537236e..78057535f6 100644 --- a/inputd/Cargo.toml +++ b/inputd/Cargo.toml @@ -11,3 +11,4 @@ redox-daemon = "0.1.0" redox-log = "0.1.1" redox_syscall = "0.3.5" orbclient = "0.3.27" +spin = "0.9.8" diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index 3c2eb329d4..0f71bdd213 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -1,7 +1,7 @@ #![feature(iter_next_chunk)] use std::fs::File; -use std::io::{Error, Read}; +use std::io::{Error, Read, Write}; pub struct Handle(File); @@ -16,6 +16,10 @@ impl Handle { pub fn register(&mut self) -> Result { Ok(dbg!(self.0.read(&mut [])?)) } + + pub fn activate(&mut self, vt: usize) -> Result { + Ok(dbg!(self.0.write(&vt.to_le_bytes())?)) + } } #[derive(Debug, Copy, Clone, PartialEq)] diff --git a/inputd/src/main.rs b/inputd/src/main.rs index da9efd6619..91cd76c92a 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -19,6 +19,8 @@ use std::sync::Arc; use inputd::Cmd; +use spin::Mutex; + use orbclient::{Event, EventOption}; use syscall::{Error as SysError, EventFlags, Packet, SchemeMut, EINVAL}; @@ -30,7 +32,9 @@ enum Handle { notified: bool, vt: usize, }, - Device(Arc), + Device { + device: String, + }, } impl Handle { @@ -39,15 +43,52 @@ impl Handle { } } +/// VT Inner State +/// +/// This is *required* to be lazily initialized since opening the handle to the display +/// requires the system call to return first. Otherwise, it will block indefinitely. +struct VtInner { + handle_file: File, +} + +struct Vt { + display: String, + index: usize, + inner: spin::Once>, +} + +impl Vt { + pub fn new(display: D, index: usize) -> Arc + where + D: Into, + { + Arc::new(Self { + display: display.into(), + inner: spin::Once::new(), + index, + }) + } + + pub fn inner(&self) -> &Mutex { + self.inner.call_once(|| { + let handle_file = File::open(format!("{}:handle", self.display)).unwrap(); + + Mutex::new(VtInner { handle_file }) + }) + } +} + struct InputScheme { handles: BTreeMap, next_id: AtomicUsize, next_vt_id: AtomicUsize, - vts: BTreeMap>, + vts: BTreeMap>, super_key: bool, - active_vt: Option<(Arc, File, usize /* VT index */)>, + active_vt: Option>, + + todo: Vec, } impl InputScheme { @@ -60,6 +101,8 @@ impl InputScheme { vts: BTreeMap::new(), super_key: false, active_vt: None, + + todo: vec![], } } } @@ -74,25 +117,18 @@ impl SchemeMut for InputScheme { let handle_ty = match command { "producer" => Handle::Producer, "consumer" => { - let vt = if let Some((_, _, vt)) = self.active_vt { - vt - } else { - // No one is currently set as active, so we assume that the last allocated - // one will be willing to take this consumer. - self.next_vt_id.load(Ordering::SeqCst) - 1 - }; + let target = path_parts.next().unwrap().parse::().unwrap(); - dbg!(vt); Handle::Consumer { events: EventFlags::empty(), pending: Vec::new(), notified: false, - vt, + vt: target, } } "handle" => { - let value = path_parts.collect::>().join("/"); - Handle::Device(Arc::new(value)) + let display = path_parts.collect::>().join("/"); + Handle::Device { device: display } } _ => unreachable!("inputd: invalid path {path}"), @@ -104,6 +140,22 @@ impl SchemeMut for InputScheme { Ok(fd) } + fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { + let handle = self.handles.get(&id).ok_or(SysError::new(EINVAL))?; + + if let Handle::Consumer { vt, .. } = handle { + let display = self.vts.get(vt).ok_or(SysError::new(EINVAL))?; + let vt = format!("{}:{vt}", display.display); + + let size = core::cmp::min(vt.len(), buf.len()); + buf[..size].copy_from_slice(&vt.as_bytes()[..size]); + + Ok(size) + } else { + Err(SysError::new(EINVAL)) + } + } + fn read(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; @@ -118,9 +170,11 @@ impl SchemeMut for InputScheme { Ok(copy) } - Handle::Device(device) => { + Handle::Device { device } => { + assert!(buf.is_empty()); + let vt = self.next_vt_id.fetch_add(1, Ordering::SeqCst); - self.vts.insert(vt, device.clone()); + self.vts.insert(vt, Vt::new(device.clone(), vt)); Ok(vt) } @@ -129,6 +183,25 @@ impl SchemeMut for InputScheme { } fn write(&mut self, id: usize, buf: &[u8]) -> syscall::Result { + let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; + + match handle { + Handle::Device { .. } | Handle::Consumer { .. } => { + if buf.len() == core::mem::size_of::() { + // The device is requesting to activate a VT. + let vt = + usize::from_le_bytes(buf.try_into().map_err(|_| SysError::new(EINVAL))?); + + self.todo.push(vt); + return Ok(buf.len()); + } else { + unreachable!() + } + } + + _ => {} + } + if buf.len() == 1 && buf[0] > 0xf4 { return Ok(1); } @@ -168,62 +241,65 @@ impl SchemeMut for InputScheme { }, EventOption::Resize(resize_event) => { - if let Some((_, file, current)) = self.active_vt.as_mut() { - inputd::send_comand( - file, - Cmd::Resize { - vt: current.clone(), - width: resize_event.width, - height: resize_event.height, + let active_vt = self.active_vt.as_ref().unwrap(); + let mut vt_inner = active_vt.inner().lock(); - // TODO(andypython): Figure out how to get the stride. - stride: resize_event.width, - }, - )?; - } + inputd::send_comand( + &mut vt_inner.handle_file, + Cmd::Resize { + vt: active_vt.index, + width: resize_event.width, + height: resize_event.height, + + // TODO(andypython): Figure out how to get the stride. + stride: resize_event.width, + }, + )?; } _ => continue, } if let Some(new_active) = new_active_opt { - if let Some(vt) = self.vts.get(&new_active).cloned() { - if let Some((_, file, current)) = self.active_vt.as_mut() { - // If the VT is already active, don't do anything. - if new_active == *current { - continue; - } + if let Some(new_active) = self.vts.get(&new_active).cloned() { + { + let active_vt = self.active_vt.as_ref().unwrap(); + let mut vt_inner = active_vt.inner().lock(); - inputd::send_comand(file, Cmd::Deactivate(*current))?; - } else { - let id = self.next_vt_id.load(Ordering::SeqCst) - 1; - let mut vt = File::open(self.vts.get_mut(&id).unwrap().as_ref()).unwrap(); - - inputd::send_comand(&mut vt, Cmd::Deactivate(id))?; + inputd::send_comand( + &mut vt_inner.handle_file, + Cmd::Deactivate(active_vt.index), + )?; } - log::info!("inputd: switching to VT #{new_active} (`{vt}`)"); + let mut vt_inner = new_active.inner().lock(); - let mut file = File::open(vt.as_ref()).unwrap(); - - inputd::send_comand(&mut file, Cmd::Activate(new_active))?; - self.active_vt = Some((vt.clone(), file, new_active)); + inputd::send_comand( + &mut vt_inner.handle_file, + Cmd::Activate(new_active.index), + )?; + self.active_vt = Some(new_active.clone()); } else { log::warn!("inputd: switch to non-existent VT #{new_active} was requested"); } } } - let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; assert!(handle.is_producer()); + let active_vt = self.active_vt.as_ref().unwrap(); for handle in self.handles.values_mut() { match handle { Handle::Consumer { - ref mut pending, - ref mut notified, + pending, + notified, + vt, .. } => { + if *vt != active_vt.index { + continue; + } + pending.extend_from_slice(buf); *notified = false; } @@ -257,7 +333,7 @@ impl SchemeMut for InputScheme { } fn close(&mut self, _id: usize) -> syscall::Result { - todo!() + Ok(0) } } @@ -281,6 +357,20 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { scheme.handle(&mut packet); socket_file.write(&packet)?; + while let Some(vt) = scheme.todo.pop() { + let vt = scheme.vts.get_mut(&vt).unwrap(); + let mut vt_inner = vt.inner().lock(); + + // Failing to activate a VT is not a fatal error. + if let Err(err) = + inputd::send_comand(&mut vt_inner.handle_file, Cmd::Activate(vt.index)) + { + log::error!("inputd: failed to activate VT #{}: {err}", vt.index) + } + + scheme.active_vt = Some(vt.clone()); + } + if !should_handle { continue; } @@ -297,16 +387,11 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { continue; } - let should_notify = if let Some((_, _, active_vt)) = scheme.active_vt { - active_vt == *vt - } else { - let last_allocated = scheme.next_vt_id.load(Ordering::SeqCst) - 1; - last_allocated == *vt - }; + let active_vt = scheme.active_vt.as_ref().unwrap(); // The activate VT is not the same as the VT that the consumer is listening to // for events. - if !should_notify { + if active_vt.index != *vt { continue; } diff --git a/vesad/src/main.rs b/vesad/src/main.rs index 3d7cf98f4c..505eb0d95b 100644 --- a/vesad/src/main.rs +++ b/vesad/src/main.rs @@ -98,6 +98,8 @@ fn inner(daemon: redox_daemon::Daemon, framebuffers: Vec, spec: &[b daemon.ready().expect("failed to notify parent"); + scheme.inputd_handle.activate(1).unwrap(); + let mut blocked = Vec::new(); loop { let mut packet = Packet::default(); diff --git a/vesad/src/scheme.rs b/vesad/src/scheme.rs index 86f29b7c9e..ee781eb1e5 100644 --- a/vesad/src/scheme.rs +++ b/vesad/src/scheme.rs @@ -36,12 +36,12 @@ pub struct DisplayScheme { pub vts: BTreeMap>>, next_id: usize, pub handles: BTreeMap, - inputd_handle: inputd::Handle, + pub inputd_handle: inputd::Handle, } impl DisplayScheme { pub fn new(mut framebuffers: Vec, spec: &[bool]) -> DisplayScheme { - let mut inputd_handle = inputd::Handle::new("vesa:handle").unwrap(); + let mut inputd_handle = inputd::Handle::new("vesa").unwrap(); let mut onscreens = Vec::new(); for fb in framebuffers.iter_mut() { @@ -185,7 +185,7 @@ impl SchemeMut for DisplayScheme { } fn dup(&mut self, id: usize, buf: &[u8]) -> Result { - if ! buf.is_empty() { + if !buf.is_empty() { return Err(Error::new(EINVAL)); } diff --git a/virtio-gpud/src/main.rs b/virtio-gpud/src/main.rs index 3fdc3eb347..73434398e3 100644 --- a/virtio-gpud/src/main.rs +++ b/virtio-gpud/src/main.rs @@ -446,6 +446,8 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { device.transport.clone(), ))?; + scheme.inputd_handle.activate(scheme.main_vt)?; + loop { let mut packet = Packet::default(); socket_file diff --git a/virtio-gpud/src/scheme.rs b/virtio-gpud/src/scheme.rs index a42a81c51e..5495218cbb 100644 --- a/virtio-gpud/src/scheme.rs +++ b/virtio-gpud/src/scheme.rs @@ -259,8 +259,10 @@ pub struct Scheme<'a> { handles: BTreeMap, /// Counter used for file descriptor allocation. next_id: AtomicUsize, - inputd_handle: inputd::Handle, displays: Vec>>, + + pub(crate) inputd_handle: inputd::Handle, + pub(crate) main_vt: usize, } impl<'a> Scheme<'a> { @@ -278,10 +280,11 @@ impl<'a> Scheme<'a> { ) .await?; - let mut inputd_handle = inputd::Handle::new("virtio-gpu:handle").unwrap(); + let mut inputd_handle = inputd::Handle::new("virtio-gpu").unwrap(); let mut vts = BTreeMap::new(); - vts.insert(inputd_handle.register().unwrap(), displays[0].clone()); + let main_vt = inputd_handle.register().unwrap(); + vts.insert(main_vt, displays[0].clone()); Ok(Self { vts, @@ -289,6 +292,7 @@ impl<'a> Scheme<'a> { next_id: AtomicUsize::new(0), inputd_handle, displays, + main_vt, }) } @@ -370,9 +374,9 @@ impl<'a> SchemeMut for Scheme<'a> { } fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { - match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { + match self.handles.get(&id).unwrap() { Handle::Vt(id) => { - let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; let bytes_copied = futures::executor::block_on(handle.get_fpath(buf)).unwrap(); Ok(bytes_copied) @@ -397,7 +401,7 @@ impl<'a> SchemeMut for Scheme<'a> { fn fmap(&mut self, id: usize, map: &syscall::Map) -> syscall::Result { match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { Handle::Vt(id) => { - let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; Ok(futures::executor::block_on(handle.map_screen(map.offset)).unwrap()) } _ => unreachable!(), @@ -425,7 +429,7 @@ impl<'a> SchemeMut for Scheme<'a> { fn write(&mut self, id: usize, buf: &[u8]) -> syscall::Result { match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { Handle::Vt(id) => { - let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; + let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; // The VT is not active and the device is reseted. Ask them to try // again later. From 820b8370ae76d2f9e7987a3e7ad074d8435ca36b Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 25 Jul 2023 16:34:25 +1000 Subject: [PATCH 4/6] virtio-gpu: remove the `+1` workaround Signed-off-by: Anhad Singh --- inputd/src/lib.rs | 53 ++++++++++++++++--- inputd/src/main.rs | 103 ++++++++++++++++++++++++++++-------- vesad/src/main.rs | 2 +- vesad/src/scheme.rs | 28 +++++++--- vesad/src/screen/graphic.rs | 2 +- virtio-core/src/probe.rs | 4 +- virtio-gpud/src/main.rs | 2 +- virtio-gpud/src/scheme.rs | 86 +++++++++++++++--------------- 8 files changed, 197 insertions(+), 83 deletions(-) diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index 0f71bdd213..c8f9e7e532 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -3,6 +3,25 @@ use std::fs::File; use std::io::{Error, Read, Write}; +unsafe fn any_as_u8_slice(p: &T) -> &[u8] { + ::core::slice::from_raw_parts((p as *const T) as *const u8, ::core::mem::size_of::()) +} + +#[derive(Debug, Copy, Clone, PartialEq)] +#[repr(usize)] +pub enum VtMode { + Text = 0, + Graphic = 1, + Default = 2, +} + +#[derive(Debug, Copy, Clone)] +#[repr(C)] +pub struct VtActivate { + pub vt: usize, + pub mode: VtMode, +} + pub struct Handle(File); impl Handle { @@ -14,11 +33,12 @@ impl Handle { // The return value is the display identifier. It will be used to uniquely // identify the display on activation events. pub fn register(&mut self) -> Result { - Ok(dbg!(self.0.read(&mut [])?)) + Ok(self.0.read(&mut [])?) } - pub fn activate(&mut self, vt: usize) -> Result { - Ok(dbg!(self.0.write(&vt.to_le_bytes())?)) + pub fn activate(&mut self, vt: usize, mode: VtMode) -> Result { + let cmd = VtActivate { vt, mode }; + Ok(self.0.write(unsafe { any_as_u8_slice(&cmd) })?) } } @@ -43,9 +63,14 @@ impl From for CmdTy { } } +#[derive(Debug)] pub enum Cmd { // TODO(andypython): #VT should really need to be a `u8`. - Activate(usize /* #VT */), + Activate { + vt: usize, + mode: VtMode, + }, + Deactivate(usize /* #VT */), Resize { // TODO(andypython): do we really need to pass the VT here? @@ -60,7 +85,7 @@ pub enum Cmd { impl Cmd { fn ty(&self) -> CmdTy { match self { - Cmd::Activate(_) => CmdTy::Activate, + Cmd::Activate { .. } => CmdTy::Activate, Cmd::Deactivate(_) => CmdTy::Deactivate, Cmd::Resize { .. } => CmdTy::Resize, } @@ -74,7 +99,14 @@ pub fn send_comand(file: &mut File, command: Cmd) -> Result<(), syscall::Error> result.push(command.ty() as u8); match command { - Cmd::Activate(vt) | Cmd::Deactivate(vt) => result.extend_from_slice(&vt.to_le_bytes()), + Cmd::Activate { vt, mode } => { + let cmd = VtActivate { vt, mode }; + let bytes = unsafe { any_as_u8_slice(&cmd) }; + + result.extend_from_slice(bytes); + } + + Cmd::Deactivate(vt) => result.extend_from_slice(&vt.to_le_bytes()), Cmd::Resize { vt, width, @@ -105,7 +137,14 @@ pub fn parse_command(buffer: &[u8]) -> Option { let vt = usize::from_le_bytes(parser.next_chunk::().ok()?); match command { - CmdTy::Activate => Some(Cmd::Activate(vt)), + CmdTy::Activate => { + let cmd = unsafe { &*buffer.as_ptr().offset(1).cast::() }; + Some(Cmd::Activate { + vt: cmd.vt, + mode: VtMode::from(cmd.mode), + }) + } + CmdTy::Deactivate => Some(Cmd::Deactivate(vt)), CmdTy::Resize => { let width = parser.next_chunk::().ok()?; diff --git a/inputd/src/main.rs b/inputd/src/main.rs index 91cd76c92a..f2d6f1fd0f 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -17,7 +17,7 @@ use std::io::{Read, Write}; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; -use inputd::Cmd; +use inputd::{Cmd, VtActivate, VtMode}; use spin::Mutex; @@ -49,6 +49,7 @@ impl Handle { /// requires the system call to return first. Otherwise, it will block indefinitely. struct VtInner { handle_file: File, + mode: VtMode, } struct Vt { @@ -72,8 +73,10 @@ impl Vt { pub fn inner(&self) -> &Mutex { self.inner.call_once(|| { let handle_file = File::open(format!("{}:handle", self.display)).unwrap(); - - Mutex::new(VtInner { handle_file }) + Mutex::new(VtInner { + handle_file, + mode: VtMode::Default, + }) }) } } @@ -88,7 +91,7 @@ struct InputScheme { super_key: bool, active_vt: Option>, - todo: Vec, + todo: Vec, } impl InputScheme { @@ -186,19 +189,19 @@ impl SchemeMut for InputScheme { let handle = self.handles.get_mut(&id).ok_or(SysError::new(EINVAL))?; match handle { - Handle::Device { .. } | Handle::Consumer { .. } => { - if buf.len() == core::mem::size_of::() { - // The device is requesting to activate a VT. - let vt = - usize::from_le_bytes(buf.try_into().map_err(|_| SysError::new(EINVAL))?); + Handle::Device { device } => { + assert!(buf.len() == core::mem::size_of::()); - self.todo.push(vt); - return Ok(buf.len()); - } else { - unreachable!() - } + // SAFETY: We have verified the size of the buffer above. + let cmd = unsafe { &*buf.as_ptr().cast::() }; + + self.vts.insert(cmd.vt, Vt::new(device.clone(), cmd.vt)); + self.todo.push(cmd.clone()); + + return Ok(buf.len()); } + Handle::Consumer { .. } => unreachable!(), _ => {} } @@ -276,7 +279,10 @@ impl SchemeMut for InputScheme { inputd::send_comand( &mut vt_inner.handle_file, - Cmd::Activate(new_active.index), + Cmd::Activate { + vt: new_active.index, + mode: VtMode::Default, + }, )?; self.active_vt = Some(new_active.clone()); } else { @@ -287,7 +293,7 @@ impl SchemeMut for InputScheme { assert!(handle.is_producer()); - let active_vt = self.active_vt.as_ref().unwrap(); + let active_vt = self.active_vt.as_ref().unwrap(); for handle in self.handles.values_mut() { match handle { Handle::Consumer { @@ -357,17 +363,22 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { scheme.handle(&mut packet); socket_file.write(&packet)?; - while let Some(vt) = scheme.todo.pop() { - let vt = scheme.vts.get_mut(&vt).unwrap(); + while let Some(cmd) = scheme.todo.pop() { + let vt = scheme.vts.get_mut(&cmd.vt).unwrap(); let mut vt_inner = vt.inner().lock(); // Failing to activate a VT is not a fatal error. - if let Err(err) = - inputd::send_comand(&mut vt_inner.handle_file, Cmd::Activate(vt.index)) - { + if let Err(err) = inputd::send_comand( + &mut vt_inner.handle_file, + Cmd::Activate { + vt: vt.index, + mode: VtMode::from(cmd.mode), + }, + ) { log::error!("inputd: failed to activate VT #{}: {err}", vt.index) } + vt_inner.mode = cmd.mode; scheme.active_vt = Some(vt.clone()); } @@ -454,5 +465,53 @@ pub fn setup_logging(level: log::LevelFilter, name: &str) { pub fn main() { #[cfg(target_os = "redox")] setup_logging(log::LevelFilter::Trace, "inputd"); - redox_daemon::Daemon::new(daemon_runner).expect("virtio-core: failed to daemonize"); + + let mut args = std::env::args().skip(1); + + if let Some(val) = args.next() { + if val != "-G" { + panic!("inputd: invalid argument: {}", val); + } + + let vt = args.next().unwrap().parse::().unwrap(); + + // On startup, the VESA display driver is started which basically makes use of the framebuffer + // provided by the firmware. The GPU device are latter started by `pcid` (such as `virtio-gpu`). + let mut devices = vec![]; + let schemes = std::fs::read_dir(":").unwrap(); + + for entry in schemes { + let path = entry.unwrap().path(); + let path_str = path + .into_os_string() + .into_string() + .expect("inputd: failed to convert path to string"); + + if path_str.contains("display") { + println!("inputd: found display scheme {}", path_str); + devices.push(path_str); + } + } + + let device = devices + .iter() + .filter(|d| !d.contains("vesa")) + .collect::>(); + let device = if device.is_empty() { + "vesa" + } else { + // TODO: What should we do when there are multiple display devices? + device[0].split("/").nth(2).unwrap() + }; + + dbg!(&device); + let mut handle = + inputd::Handle::new(device).expect("inputd: failed to open display handle"); + + handle + .activate(vt, VtMode::Graphic) + .expect("inputd: failed to activate VT in graphic mode"); + } else { + redox_daemon::Daemon::new(daemon_runner).expect("virtio-core: failed to daemonize"); + } } diff --git a/vesad/src/main.rs b/vesad/src/main.rs index 505eb0d95b..08046147ca 100644 --- a/vesad/src/main.rs +++ b/vesad/src/main.rs @@ -98,7 +98,7 @@ fn inner(daemon: redox_daemon::Daemon, framebuffers: Vec, spec: &[b daemon.ready().expect("failed to notify parent"); - scheme.inputd_handle.activate(1).unwrap(); + scheme.inputd_handle.activate(1, inputd::VtMode::Default).unwrap(); let mut blocked = Vec::new(); loop { diff --git a/vesad/src/scheme.rs b/vesad/src/scheme.rs index ee781eb1e5..fe9db07eba 100644 --- a/vesad/src/scheme.rs +++ b/vesad/src/scheme.rs @@ -37,6 +37,7 @@ pub struct DisplayScheme { next_id: usize, pub handles: BTreeMap, pub inputd_handle: inputd::Handle, + } impl DisplayScheme { @@ -72,7 +73,7 @@ impl DisplayScheme { vts, next_id: 0, handles: BTreeMap::new(), - inputd_handle + inputd_handle, } } @@ -303,20 +304,33 @@ impl SchemeMut for DisplayScheme { match handle.kind { HandleKind::Input => { - use inputd::Cmd as DisplayCommand; + use inputd::{Cmd as DisplayCommand, VtMode}; let command = inputd::parse_command(buf).unwrap(); + dbg!(&command); match command { - DisplayCommand::Activate(vt) => { + DisplayCommand::Activate { vt, mode } => { let vt_i = VtIndex(vt); if let Some(screens) = self.vts.get_mut(&vt_i) { for (screen_i, screen) in screens.iter_mut() { - screen.redraw( - self.onscreens[screen_i.0], - self.framebuffers[screen_i.0].stride - ); + match mode { + VtMode::Graphic => { + dbg!("yes"); + *screen = Box::new(GraphicScreen::new(Display::new(screen.width(), screen.height()))); + } + + VtMode::Default => { + dbg!("x", &mode); + screen.redraw( + self.onscreens[screen_i.0], + self.framebuffers[screen_i.0].stride + ); + } + + VtMode::Text => todo!() + } } } diff --git a/vesad/src/screen/graphic.rs b/vesad/src/screen/graphic.rs index e0bf22ca35..ca96be28a8 100644 --- a/vesad/src/screen/graphic.rs +++ b/vesad/src/screen/graphic.rs @@ -27,7 +27,7 @@ pub struct GraphicScreen { impl GraphicScreen { pub fn new(display: Display) -> GraphicScreen { GraphicScreen { - display: display, + display, input: VecDeque::new(), sync_rects: Vec::new(), } diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index f97ae30f5a..3e4ca9e1e8 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -11,7 +11,7 @@ use pcid_interface::*; use syscall::Io; use crate::spec::*; -use crate::transport::{Error, Queue, StandardTransport}; +use crate::transport::{Error, StandardTransport}; use crate::utils::{align_down, VolatileCell}; pub struct Device<'a> { @@ -21,6 +21,8 @@ pub struct Device<'a> { pub isr: &'a VolatileCell, } +// FIXME(andypython): `device_space` should not be `Send` nor `Sync`. Take +// it out of `Device`. unsafe impl Send for Device<'_> {} unsafe impl Sync for Device<'_> {} diff --git a/virtio-gpud/src/main.rs b/virtio-gpud/src/main.rs index 73434398e3..4f1f08eb80 100644 --- a/virtio-gpud/src/main.rs +++ b/virtio-gpud/src/main.rs @@ -446,7 +446,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { device.transport.clone(), ))?; - scheme.inputd_handle.activate(scheme.main_vt)?; + // scheme.inputd_handle.activate(scheme.main_vt)?; loop { let mut packet = Packet::default(); diff --git a/virtio-gpud/src/scheme.rs b/virtio-gpud/src/scheme.rs index 5495218cbb..7fe379a24c 100644 --- a/virtio-gpud/src/scheme.rs +++ b/virtio-gpud/src/scheme.rs @@ -249,20 +249,16 @@ impl<'a> Display<'a> { } } -enum Handle { - Vt(usize /* VT index */), +enum Handle<'a> { + Vt { display: Arc>, vt: usize }, Input, } pub struct Scheme<'a> { - vts: BTreeMap>>, - handles: BTreeMap, + handles: BTreeMap>, /// Counter used for file descriptor allocation. next_id: AtomicUsize, displays: Vec>>, - - pub(crate) inputd_handle: inputd::Handle, - pub(crate) main_vt: usize, } impl<'a> Scheme<'a> { @@ -280,19 +276,10 @@ impl<'a> Scheme<'a> { ) .await?; - let mut inputd_handle = inputd::Handle::new("virtio-gpu").unwrap(); - - let mut vts = BTreeMap::new(); - let main_vt = inputd_handle.register().unwrap(); - vts.insert(main_vt, displays[0].clone()); - Ok(Self { - vts, handles: BTreeMap::new(), next_id: AtomicUsize::new(0), - inputd_handle, displays, - main_vt, }) } @@ -363,22 +350,17 @@ impl<'a> SchemeMut for Scheme<'a> { dbg!(vt, id); - if self.displays.get(id).is_none() { - return Err(SysError::new(EINVAL)); - } + let display = self.displays.get(id).ok_or(SysError::new(EINVAL))?; let fd = self.next_id.fetch_add(1, Ordering::SeqCst); - // FIXME: The +1 is a hack. vesad smh - self.handles.insert(fd, Handle::Vt(vt + 1)); + self.handles.insert(fd, Handle::Vt {display: display.clone(), vt }); Ok(fd) } fn fpath(&mut self, id: usize, buf: &mut [u8]) -> syscall::Result { match self.handles.get(&id).unwrap() { - Handle::Vt(id) => { - let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; - let bytes_copied = futures::executor::block_on(handle.get_fpath(buf)).unwrap(); - + Handle::Vt { display, .. } => { + let bytes_copied = futures::executor::block_on(display.get_fpath(buf)).unwrap(); Ok(bytes_copied) } @@ -400,9 +382,8 @@ impl<'a> SchemeMut for Scheme<'a> { fn fmap(&mut self, id: usize, map: &syscall::Map) -> syscall::Result { match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { - Handle::Vt(id) => { - let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; - Ok(futures::executor::block_on(handle.map_screen(map.offset)).unwrap()) + Handle::Vt { display, .. } => { + Ok(futures::executor::block_on(display.map_screen(map.offset)).unwrap()) } _ => unreachable!(), } @@ -410,9 +391,8 @@ impl<'a> SchemeMut for Scheme<'a> { fn fsync(&mut self, id: usize) -> syscall::Result { match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { - Handle::Vt(id) => { - let handle = self.vts.get_mut(&id).ok_or(SysError::new(EINVAL))?; - futures::executor::block_on(handle.flush(None)).unwrap(); + Handle::Vt { display, .. } => { + futures::executor::block_on(display.flush(None)).unwrap(); Ok(0) } @@ -428,12 +408,10 @@ impl<'a> SchemeMut for Scheme<'a> { fn write(&mut self, id: usize, buf: &[u8]) -> syscall::Result { match self.handles.get(&id).ok_or(SysError::new(EINVAL))? { - Handle::Vt(id) => { - let handle = self.vts.get_mut(&(id - 1)).ok_or(SysError::new(EINVAL))?; - + Handle::Vt { display, .. } => { // The VT is not active and the device is reseted. Ask them to try // again later. - if handle.is_reseted.load(Ordering::SeqCst) { + if display.is_reseted.load(Ordering::SeqCst) { return Err(SysError::new(EAGAIN)); } @@ -445,26 +423,48 @@ impl<'a> SchemeMut for Scheme<'a> { }; for damage in damages { - futures::executor::block_on(handle.flush(Some(damage))).unwrap(); + futures::executor::block_on(display.flush(Some(damage))).unwrap(); } Ok(buf.len()) } Handle::Input => { - use inputd::Cmd as DisplayCommand; + use inputd::{Cmd as DisplayCommand, VtMode}; let command = inputd::parse_command(buf).unwrap(); match command { - DisplayCommand::Activate(vt) => { - let display = self.vts.get(&vt).unwrap(); - futures::executor::block_on(display.init()).unwrap(); + DisplayCommand::Activate { mode, vt } => { + assert!(mode == VtMode::Graphic || mode == VtMode::Default); + let target_vt = vt; + + for handle in self.handles.values() { + if let Handle::Vt { display , vt } = handle { + if *vt != target_vt { + continue; + } + + futures::executor::block_on(display.init()).unwrap(); + } + } } - DisplayCommand::Deactivate(vt) => { - let display = self.vts.get(&vt).ok_or(SysError::new(EINVAL))?; - futures::executor::block_on(display.detach()).unwrap(); + DisplayCommand::Deactivate(target_vt) => { + for handle in self.handles.values() { + if let Handle::Vt { display , vt } = handle { + if *vt != target_vt { + continue; + } + + futures::executor::block_on(display.detach()).unwrap(); + break; + } + } + + // for display in self.displays.iter() { + // futures::executor::block_on(display.detach()).unwrap(); + // } } DisplayCommand::Resize { .. } => { From ed995306bd051fc8a28331deff7dee18c7f31c42 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 25 Jul 2023 16:36:46 +1000 Subject: [PATCH 5/6] inputd: minor clippy fixes Signed-off-by: Anhad Singh --- inputd/src/lib.rs | 8 ++++---- inputd/src/main.rs | 10 +++++++--- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/inputd/src/lib.rs b/inputd/src/lib.rs index c8f9e7e532..a3d1020678 100644 --- a/inputd/src/lib.rs +++ b/inputd/src/lib.rs @@ -15,7 +15,7 @@ pub enum VtMode { Default = 2, } -#[derive(Debug, Copy, Clone)] +#[derive(Debug, Clone)] #[repr(C)] pub struct VtActivate { pub vt: usize, @@ -33,12 +33,12 @@ impl Handle { // The return value is the display identifier. It will be used to uniquely // identify the display on activation events. pub fn register(&mut self) -> Result { - Ok(self.0.read(&mut [])?) + self.0.read(&mut []) } pub fn activate(&mut self, vt: usize, mode: VtMode) -> Result { let cmd = VtActivate { vt, mode }; - Ok(self.0.write(unsafe { any_as_u8_slice(&cmd) })?) + self.0.write(unsafe { any_as_u8_slice(&cmd) }) } } @@ -141,7 +141,7 @@ pub fn parse_command(buffer: &[u8]) -> Option { let cmd = unsafe { &*buffer.as_ptr().offset(1).cast::() }; Some(Cmd::Activate { vt: cmd.vt, - mode: VtMode::from(cmd.mode), + mode: cmd.mode, }) } diff --git a/inputd/src/main.rs b/inputd/src/main.rs index f2d6f1fd0f..2ca582eba6 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -216,7 +216,7 @@ impl SchemeMut for InputScheme { ) }; - for event in events.iter() { + 'out: for event in events.iter() { let mut new_active_opt = None; match event.to_option() { EventOption::Key(key_event) => match key_event.scancode { @@ -264,6 +264,10 @@ impl SchemeMut for InputScheme { } if let Some(new_active) = new_active_opt { + if new_active == self.active_vt.as_ref().unwrap().index { + continue 'out; + } + if let Some(new_active) = self.vts.get(&new_active).cloned() { { let active_vt = self.active_vt.as_ref().unwrap(); @@ -372,7 +376,7 @@ fn deamon(deamon: redox_daemon::Daemon) -> anyhow::Result<()> { &mut vt_inner.handle_file, Cmd::Activate { vt: vt.index, - mode: VtMode::from(cmd.mode), + mode: cmd.mode, }, ) { log::error!("inputd: failed to activate VT #{}: {err}", vt.index) @@ -501,7 +505,7 @@ pub fn main() { "vesa" } else { // TODO: What should we do when there are multiple display devices? - device[0].split("/").nth(2).unwrap() + device[0].split('/').nth(2).unwrap() }; dbg!(&device); From 71a6eb1bb284feb109b29b529b8e247c8fe07223 Mon Sep 17 00:00:00 2001 From: Anhad Singh Date: Tue, 25 Jul 2023 19:20:04 +1000 Subject: [PATCH 6/6] inputd: add `-A` command to activate a VT Signed-off-by: Anhad Singh --- inputd/src/main.rs | 101 +++++++++++++++++++++++++++----------------- vesad/src/scheme.rs | 3 -- 2 files changed, 62 insertions(+), 42 deletions(-) diff --git a/inputd/src/main.rs b/inputd/src/main.rs index 2ca582eba6..fa37f58b2c 100644 --- a/inputd/src/main.rs +++ b/inputd/src/main.rs @@ -14,6 +14,7 @@ use std::collections::BTreeMap; use std::fs::File; use std::io::{Read, Write}; +use std::os::fd::AsRawFd; use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; @@ -473,48 +474,70 @@ pub fn main() { let mut args = std::env::args().skip(1); if let Some(val) = args.next() { - if val != "-G" { - panic!("inputd: invalid argument: {}", val); - } + match val.as_ref() { + // Sets the VT mode of the specified VT to [`VtMode::Graphic`]. + "-G" => { + let vt = args.next().unwrap().parse::().unwrap(); - let vt = args.next().unwrap().parse::().unwrap(); - - // On startup, the VESA display driver is started which basically makes use of the framebuffer - // provided by the firmware. The GPU device are latter started by `pcid` (such as `virtio-gpu`). - let mut devices = vec![]; - let schemes = std::fs::read_dir(":").unwrap(); - - for entry in schemes { - let path = entry.unwrap().path(); - let path_str = path - .into_os_string() - .into_string() - .expect("inputd: failed to convert path to string"); - - if path_str.contains("display") { - println!("inputd: found display scheme {}", path_str); - devices.push(path_str); + // On startup, the VESA display driver is started which basically makes use of the framebuffer + // provided by the firmware. The GPU devices are latter started by `pcid` (such as `virtio-gpu`). + let mut devices = vec![]; + let schemes = std::fs::read_dir(":").unwrap(); + + for entry in schemes { + let path = entry.unwrap().path(); + let path_str = path + .into_os_string() + .into_string() + .expect("inputd: failed to convert path to string"); + + if path_str.contains("display") { + println!("inputd: found display scheme {}", path_str); + devices.push(path_str); + } + } + + let device = devices + .iter() + .filter(|d| !d.contains("vesa")) + .collect::>(); + let device = if device.is_empty() { + "vesa" + } else { + // TODO: What should we do when there are multiple display devices? + device[0].split('/').nth(2).unwrap() + }; + + let mut handle = + inputd::Handle::new(device).expect("inputd: failed to open display handle"); + + handle + .activate(vt, VtMode::Graphic) + .expect("inputd: failed to activate VT in graphic mode"); } + + // Activates a VT. + "-A" => { + let vt = args.next().unwrap().parse::().unwrap(); + + let handle = File::open(format!("input:consumer/{vt}")).expect("inputd: failed to open consumer handle"); + let mut display_path = [0; 4096]; + + let written = syscall::fpath(handle.as_raw_fd() as usize, &mut display_path).expect("inputd: fpath() failed"); + + assert!(written <= display_path.len()); + drop(handle); + + let display_path = std::str::from_utf8(&display_path[..written]).expect("inputd: display path UTF-8 validation failed"); + let display_name = display_path.split('/').skip(1).next().expect("inputd: invalid display path"); + let display_scheme = display_name.split(':').next().expect("inputd: invalid display path"); + + let mut handle = inputd::Handle::new(display_scheme).expect("inputd: failed to open display handle"); + handle.activate(vt, VtMode::Default).expect("inputd: failed to activate VT"); + } + + _ => panic!("inputd: invalid argument: {}", val), } - - let device = devices - .iter() - .filter(|d| !d.contains("vesa")) - .collect::>(); - let device = if device.is_empty() { - "vesa" - } else { - // TODO: What should we do when there are multiple display devices? - device[0].split('/').nth(2).unwrap() - }; - - dbg!(&device); - let mut handle = - inputd::Handle::new(device).expect("inputd: failed to open display handle"); - - handle - .activate(vt, VtMode::Graphic) - .expect("inputd: failed to activate VT in graphic mode"); } else { redox_daemon::Daemon::new(daemon_runner).expect("virtio-core: failed to daemonize"); } diff --git a/vesad/src/scheme.rs b/vesad/src/scheme.rs index fe9db07eba..20d1b45719 100644 --- a/vesad/src/scheme.rs +++ b/vesad/src/scheme.rs @@ -307,7 +307,6 @@ impl SchemeMut for DisplayScheme { use inputd::{Cmd as DisplayCommand, VtMode}; let command = inputd::parse_command(buf).unwrap(); - dbg!(&command); match command { DisplayCommand::Activate { vt, mode } => { @@ -317,12 +316,10 @@ impl SchemeMut for DisplayScheme { for (screen_i, screen) in screens.iter_mut() { match mode { VtMode::Graphic => { - dbg!("yes"); *screen = Box::new(GraphicScreen::new(Display::new(screen.width(), screen.height()))); } VtMode::Default => { - dbg!("x", &mode); screen.redraw( self.onscreens[screen_i.0], self.framebuffers[screen_i.0].stride