From 691a089db1d9591563f07d2f29636c40098a4c60 Mon Sep 17 00:00:00 2001 From: goriainovstanislav Date: Wed, 29 May 2024 12:54:12 +0300 Subject: [PATCH] usbhost: Move usb host reset into a separate thread USB Host reset calls libusb_reset_device, which is a rather costly operation and can hold the VM lock for too long. Factor out reset into a separate thread to avoid that. In case the device receives USB packets during a reset, they get remembered and are processed later, after the reset is complete. Signed-off-by: goriainovstanislav --- devices/src/usb/usbhost/mod.rs | 116 +++++++++++++++++++++++++++++---- 1 file changed, 104 insertions(+), 12 deletions(-) diff --git a/devices/src/usb/usbhost/mod.rs b/devices/src/usb/usbhost/mod.rs index 4dcade11f..9d17ad193 100644 --- a/devices/src/usb/usbhost/mod.rs +++ b/devices/src/usb/usbhost/mod.rs @@ -15,16 +15,18 @@ mod host_usblib; mod ohusb; use std::{ - collections::LinkedList, - os::unix::io::RawFd, + collections::{LinkedList, VecDeque}, + ffi::CStr, + os::unix::io::{AsRawFd, RawFd}, rc::Rc, sync::{Arc, Mutex, Weak}, + thread, time::Duration, }; #[cfg(not(all(target_arch = "aarch64", target_env = "ohos")))] use anyhow::Context as anyhowContext; -use anyhow::{anyhow, Result}; +use anyhow::{anyhow, bail, Result}; use clap::Parser; use libc::c_int; use libusb1_sys::{ @@ -35,6 +37,7 @@ use rusb::{ constants::LIBUSB_CLASS_HUB, Context, Device, DeviceDescriptor, DeviceHandle, Direction, Error, TransferType, UsbContext, }; +use vmm_sys_util::{epoll::EventSet, eventfd::EventFd}; use crate::usb::{ config::{ @@ -59,7 +62,9 @@ use ohusb::OhUsbDev; use util::byte_code::ByteCode; use util::gen_base_func; use util::link_list::{List, Node}; -use util::loop_context::{EventNotifier, EventNotifierHelper, NotifierCallback}; +use util::loop_context::{ + read_fd, EventNotifier, EventNotifierHelper, NotifierCallback, NotifierOperation, +}; use util::num_ops::str_to_num; const NON_ISO_PACKETS_NUMS: c_int = 0; @@ -409,6 +414,11 @@ pub struct UsbHost { iso_urb_count: u32, #[cfg(all(target_arch = "aarch64", target_env = "ohos"))] oh_dev: OhUsbDev, + is_resetting: bool, + /// USB packets delayed during a reset. + delayed_transfers: VecDeque<(*mut libusb_transfer, Arc>)>, + /// Event to trigger reset callback. + reset_cb_evt: Arc, } // SAFETY: Send and Sync is not auto-implemented for util::link_list::List. @@ -443,6 +453,9 @@ impl UsbHost { iso_urb_count, #[cfg(all(target_arch = "aarch64", target_env = "ohos"))] oh_dev, + is_resetting: false, + delayed_transfers: VecDeque::new(), + reset_cb_evt: Arc::new(EventFd::new(libc::EFD_NONBLOCK)?), }) } @@ -1015,9 +1028,30 @@ impl Drop for UsbHost { impl EventNotifierHelper for UsbHost { fn internal_notifiers(usbhost: Arc>) -> Vec { + // Register event notifier for reset_cb_evt. let cloned_usbhost = usbhost.clone(); - let mut notifiers = Vec::new(); + let handler: Rc = Rc::new(move |_, fd: RawFd| { + read_fd(fd); + let mut locked_usbhost = cloned_usbhost.lock().unwrap(); + let xfers: Vec<_> = locked_usbhost.delayed_transfers.drain(0..).collect(); + locked_usbhost.is_resetting = false; + + for (host_transfer, ref packet) in xfers { + locked_usbhost.submit_host_transfer(host_transfer, packet); + } + None + }); + let mut notifiers = vec![EventNotifier::new( + NotifierOperation::AddShared, + usbhost.lock().unwrap().reset_cb_evt.as_raw_fd(), + None, + EventSet::IN, + vec![handler], + )]; + + // Register event notifier libusb pollfds. + let cloned_usbhost = usbhost.clone(); let timeout = Some(Duration::new(0, 0)); let handler: Rc = Rc::new(move |_, _fd: RawFd| { cloned_usbhost @@ -1047,6 +1081,30 @@ fn register_exit(usbhost: Arc>) { exit_notifier, ); } +struct DeviceHandleWrapper(*mut libusb1_sys::libusb_device_handle); + +// SAFETY: Send is not auto-implemented for raw pointers, implementing it is safe because the device +// handle is managed by libusb and is known to be valid during UsbHost device lifetime. +unsafe impl Send for DeviceHandleWrapper {} + +impl DeviceHandleWrapper { + fn reset(&self) -> Result<()> { + if self.0.is_null() { + bail!("invalid device handle pointer"); + } + + // SAFETY: Handle is guaranteed to be not null. + match unsafe { libusb1_sys::libusb_reset_device(self.0) } { + 0 => Ok(()), + err => { + // SAFETY: The memory referenced by the pointer is not mutated here, it is also + // guaranteed by libusb to be a valid c string. + let err_name = unsafe { CStr::from_ptr(libusb1_sys::libusb_error_name(err)) }; + Err(anyhow!(err_name.to_str().unwrap())) + } + } + } +} impl UsbDevice for UsbHost { gen_base_func!(usb_device_base, usb_device_base_mut, UsbDeviceBase, base); @@ -1082,13 +1140,34 @@ impl UsbDevice for UsbHost { self.clear_iso_queues(); + if self.is_resetting { + self.delayed_transfers.clear(); + return; + } + trace::usb_host_reset(self.config.hostbus, self.config.hostaddr); - self.handle - .as_mut() - .unwrap() - .reset() - .unwrap_or_else(|e| error!("Failed to reset the usb host device {:?}", e)); + self.is_resetting = true; + let handle = DeviceHandleWrapper(self.handle.as_ref().unwrap().as_raw()); + let reset_cb_evt = self.reset_cb_evt.clone(); + + let res = thread::Builder::new() + .name("usb host reset".to_string()) + .spawn(move || match handle.reset() { + Ok(()) => { + if let Err(err) = reset_cb_evt.write(1) { + error!( + "USB Host device failed to trigger reset callback event: {:?}", + err + ) + } + } + Err(err) => error!("USB Host device reset error: {:?}", err), + }); + + if let Err(err) = res { + error!("USB Host device failed to start reset thread: {:?}", err); + } } fn set_controller(&mut self, _cntlr: std::sync::Weak>) {} @@ -1189,7 +1268,13 @@ impl UsbDevice for UsbHost { self.requests.lock().unwrap().add_tail(node); - self.submit_host_transfer(host_transfer, packet); + if self.is_resetting { + packet.lock().unwrap().is_async = true; + self.delayed_transfers + .push_back((host_transfer, packet.clone())); + } else { + self.submit_host_transfer(host_transfer, packet); + } } fn handle_data(&mut self, packet: &Arc>) { @@ -1313,7 +1398,14 @@ impl UsbDevice for UsbHost { return; } }; - self.submit_host_transfer(host_transfer, packet); + + if self.is_resetting { + packet.lock().unwrap().is_async = true; + self.delayed_transfers + .push_back((host_transfer, packet.clone())); + } else { + self.submit_host_transfer(host_transfer, packet); + } } } -- Gitee