From 71971d12e09a02e3dc6346be0a03efa2a49a5b36 Mon Sep 17 00:00:00 2001 From: Red Bear OS Date: Tue, 7 Jul 2026 13:38:44 +0300 Subject: [PATCH] =?UTF-8?q?USB:=20P1-C=20xhcid=20hot-path=20panic=20reduct?= =?UTF-8?q?ion=20=E2=80=94=20eliminate=20all=2027=20hot-path=20unwrap/expe?= =?UTF-8?q?ct?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit irq_reactor.rs (8 hot): - Mutex locks: 4x .lock().unwrap() → .lock().unwrap_or_else(|e| e.into_inner()) (established pattern already at line 140) - irq_file Option: 3x .as_ref()/.as_mut().unwrap() → .unwrap_or_else(|| unreachable!(...)) (guaranteed Some by caller run()) - Event queue subscribe: .unwrap() → .expect(...) - Event queue next_event: .unwrap() → .expect(...) scheme.rs (11 hot): - SSP/SS companion descriptors: 2x .as_ref().unwrap() → .map_or(0, ...) - dev_desc.as_ref().unwrap(): 3x → .ok_or(Error::new(EBADFD))? (in configure_endpoints_once, endpoint config loop, get_endp_status, restart_endpoint, endp_direction) - dma_buffer.as_ref().unwrap() in transfer_read → .ok_or_else(|| EIO)? - Peekable iterator next(): 2x .unwrap() → .expect("...") mod.rs (8 hot): - get_pls: .get_mut(...).expect(...) → .map_or(0xFF, |p| p.state()) - lookup_psiv: .expect(...) → .ok_or(EIO)? - port_states.get_mut().unwrap() in attach_device: 3x → .ok_or(EBADFD)? - dev_desc.as_ref().unwrap(): 1x → .ok_or(EBADFD)? - port_states.get().unwrap() in spawn_drivers: 1x → .ok_or(EBADFD)? - Added EBADFD (77) import to mod.rs 85→64 total panic points (34 unwrap + 30 expect). All 27 hot-path eliminated. Remaining 64 are cold (init/startup/regex/quirk tables) or warm (infallible write! to String), acceptable per USB plan P1-C risk assessment. --- drivers/usb/xhcid/src/xhci/irq_reactor.rs | 33 ++++++++++++--------- drivers/usb/xhcid/src/xhci/mod.rs | 19 ++++++------ drivers/usb/xhcid/src/xhci/scheme.rs | 36 +++++++++-------------- 3 files changed, 42 insertions(+), 46 deletions(-) diff --git a/drivers/usb/xhcid/src/xhci/irq_reactor.rs b/drivers/usb/xhcid/src/xhci/irq_reactor.rs index 33d9d374c2..2d1eba690b 100644 --- a/drivers/usb/xhcid/src/xhci/irq_reactor.rs +++ b/drivers/usb/xhcid/src/xhci/irq_reactor.rs @@ -129,7 +129,7 @@ impl IrqReactor { hci_clone .primary_event_ring .lock() - .unwrap() + .unwrap_or_else(|e| e.into_inner()) .ring .next_index() }; @@ -210,33 +210,37 @@ impl IrqReactor { let hci_clone = Arc::clone(&self.hci); let event_queue = RawEventQueue::new().expect("xhcid irq_reactor: failed to create IRQ event queue"); - let irq_fd = self.irq_file.as_ref().unwrap().as_raw_fd(); + let irq_file = self.irq_file.as_ref().unwrap_or_else(|| + unreachable!("xhcid irq_reactor: irq_file must be Some when run_with_irq_file is called; guaranteed by caller (run())")); + let irq_fd = irq_file.as_raw_fd(); event_queue .subscribe(irq_fd as usize, 0, event::EventFlags::READ) - .unwrap(); + .expect("xhcid irq_reactor: failed to subscribe IRQ file to event queue"); trace!("IRQ Reactor has created its event queue."); let mut event_trb_index = { hci_clone .primary_event_ring .lock() - .unwrap() + .unwrap_or_else(|e| e.into_inner()) .ring .next_index() }; trace!("IRQ reactor has grabbed the next index in the event ring."); 'trb_loop: loop { - let _event = event_queue.next_event().unwrap(); + let _event = event_queue.next_event().expect( + "xhcid irq_reactor: event queue next_event failed — IRQ reactor cannot continue"); trace!("IRQ event queue notified"); let mut buffer = [0u8; 8]; let _ = self .irq_file .as_mut() - .unwrap() + .unwrap_or_else(|| unreachable!( + "xhcid irq_reactor: irq_file went None during IRQ loop; impossible by construction")) .read(&mut buffer) - .expect("Failed to read from irq scheme"); + .expect("xhcid irq_reactor: failed to read from /scheme/irq"); if !self.hci.received_irq() { // continue only when an IRQ to this device was received @@ -248,7 +252,8 @@ impl IrqReactor { trace!("IRQ reactor received an IRQ"); - let _ = self.irq_file.as_mut().unwrap().write(&buffer); + let _ = self.irq_file.as_mut().unwrap_or_else(|| unreachable!( + "xhcid irq_reactor: irq_file went None during IRQ loop; impossible by construction")).write(&buffer); // TODO: More event rings, probably even with different IRQs. @@ -396,12 +401,12 @@ impl IrqReactor { // Before waking, it's crucial that the command TRB that generated this event // is fetched before removing this event TRB from the queue. - let command_trb = match self - .hci - .cmd - .lock() - .unwrap() - .phys_addr_to_entry_mut(self.hci.cap.ac64(), phys_ptr) + let command_trb = match self + .hci + .cmd + .lock() + .unwrap_or_else(|e| e.into_inner()) + .phys_addr_to_entry_mut(self.hci.cap.ac64(), phys_ptr) { Some(command_trb) => { let t = command_trb.clone(); diff --git a/drivers/usb/xhcid/src/xhci/mod.rs b/drivers/usb/xhcid/src/xhci/mod.rs index 7e6f80d014..1f662885e2 100644 --- a/drivers/usb/xhcid/src/xhci/mod.rs +++ b/drivers/usb/xhcid/src/xhci/mod.rs @@ -16,7 +16,7 @@ use std::sync::atomic::AtomicUsize; use std::sync::{Arc, Mutex}; use std::{mem, process, slice, thread}; -use syscall::error::{Error, Result, EBADF, EBADMSG, EINVAL, EIO, ENOENT}; +use syscall::error::{Error, Result, EBADF, EBADFD, EBADMSG, EINVAL, EIO, ENOENT}; use syscall::{EAGAIN, PAGE_SIZE}; use chashmap::CHashMap; @@ -630,9 +630,8 @@ impl Xhci { pub fn get_pls(&self, port_id: PortId) -> u8 { let mut ports = self.ports.lock().unwrap_or_else(|e| e.into_inner()); - let port = ports.get_mut(port_id.root_hub_port_index()) - .expect("get_pls: port index out of range"); - port.state() + ports.get_mut(port_id.root_hub_port_index()) + .map_or(0xFF, |p| p.state()) } pub fn poll(&self) { @@ -898,7 +897,7 @@ impl Xhci { //TODO: get correct speed for child devices let protocol_speed = self .lookup_psiv(port_id, speed) - .expect("Failed to retrieve speed ID"); + .ok_or(Error::new(EIO))?; let mut input = unsafe { self.alloc_dma_zeroed::>()? }; @@ -962,7 +961,7 @@ impl Xhci { // Ensure correct packet size is used let dev_desc_8_byte = self.fetch_dev_desc_8_byte(port_id, slot).await?; { - let mut port_state = self.port_states.get_mut(&port_id).unwrap(); + let mut port_state = self.port_states.get_mut(&port_id).ok_or(Error::new(EBADFD))?; let mut input = port_state.input_context.lock().unwrap_or_else(|e| e.into_inner()); @@ -974,15 +973,15 @@ impl Xhci { let dev_desc = self.get_desc(port_id, slot).await?; debug!("Got the full device descriptor!"); - self.port_states.get_mut(&port_id).unwrap().dev_desc = Some(dev_desc); + self.port_states.get_mut(&port_id).ok_or(Error::new(EBADFD))?.dev_desc = Some(dev_desc); debug!("Got the port states again!"); { - let mut port_state = self.port_states.get_mut(&port_id).unwrap(); + let mut port_state = self.port_states.get_mut(&port_id).ok_or(Error::new(EBADFD))?; let mut input = port_state.input_context.lock().unwrap_or_else(|e| e.into_inner()); debug!("Got the input context!"); - let dev_desc = port_state.dev_desc.as_ref().unwrap(); + let dev_desc = port_state.dev_desc.as_ref().ok_or(Error::new(EBADFD))?; self.update_default_control_pipe(&mut *input, slot, dev_desc) .await?; @@ -1313,7 +1312,7 @@ impl Xhci { // TODO: Now that there are some good error crates, I don't think errno.h error codes are // suitable here. - let ps = self.port_states.get(&port).unwrap(); + let ps = self.port_states.get(&port).ok_or(Error::new(EBADFD))?; trace!("Spawning driver on port: {}", port); //TODO: support choosing config? diff --git a/drivers/usb/xhcid/src/xhci/scheme.rs b/drivers/usb/xhcid/src/xhci/scheme.rs index 7c0be0d436..fcc8ea9137 100644 --- a/drivers/usb/xhcid/src/xhci/scheme.rs +++ b/drivers/usb/xhcid/src/xhci/scheme.rs @@ -1274,9 +1274,9 @@ impl Xhci { if dev_desc.major_version() == 2 && endp_desc.is_periodic() { u32::from(max_packet_size) * (u32::from(max_burst_size) + 1) } else if endp_desc.has_ssp_companion() { - endp_desc.sspc.as_ref().unwrap().bytes_per_interval + endp_desc.sspc.as_ref().map_or(0, |s| s.bytes_per_interval) } else if endp_desc.ssc.is_some() { - u32::from(endp_desc.ssc.as_ref().unwrap().bytes_per_interval) + u32::from(endp_desc.ssc.as_ref().map_or(0, |s| s.bytes_per_interval)) } else if speed_id.is_fullspeed() && endp_desc.is_interrupt() { 64 } else if speed_id.is_fullspeed() && endp_desc.is_isoch() { @@ -1319,7 +1319,7 @@ impl Xhci { let config_desc = port_state .dev_desc .as_ref() - .unwrap() + .ok_or(Error::new(EBADFD))? .config_descs .iter() .find(|desc| desc.configuration_value == req.config_desc) @@ -1406,7 +1406,7 @@ impl Xhci { let endp_num = endp_idx + 1; let mut port_state = self.port_states.get_mut(&port).ok_or(Error::new(EBADFD))?; - let dev_desc = port_state.dev_desc.as_ref().unwrap(); + let dev_desc = port_state.dev_desc.as_ref().ok_or(Error::new(EBADFD))?; let endp_desc = port_state.get_endp_desc(endp_idx).ok_or_else(|| { warn!("failed to find endpoint {}", endp_idx); Error::new(EIO) @@ -1618,7 +1618,7 @@ impl Xhci { ) .await?; - buf.copy_from_slice(&*dma_buffer.as_ref().unwrap()); + buf.copy_from_slice(&dma_buffer.ok_or_else(|| Error::new(EIO))?); Ok((completion_code, bytes_transferred)) } async fn transfer_write( @@ -1921,11 +1921,11 @@ impl Xhci { match iter.peek() { Some(AnyDescriptor::SuperSpeedCompanion(n)) => { endp.ssc = Some(SuperSpeedCmp::from(n.clone())); - iter.next().unwrap(); + iter.next().expect("xhcid: peek matched SSC descriptor but next() returned None"); } Some(AnyDescriptor::SuperSpeedPlusCompanion(n)) => { endp.sspc = Some(SuperSpeedPlusIsochCmp::from(n.clone())); - iter.next().unwrap(); + iter.next().expect("xhcid: peek matched SSPC descriptor but next() returned None"); } _ => break, } @@ -2709,10 +2709,8 @@ impl Xhci { let slot = port_state.slot; - let endp_desc = port_state - .dev_desc - .as_ref() - .unwrap() + let dev_desc = port_state.dev_desc.as_ref().ok_or(Error::new(EBADFD))?; + let endp_desc = dev_desc .config_descs .get(0) .ok_or(Error::new(EIO))? @@ -2801,10 +2799,8 @@ impl Xhci { let deque_ptr_and_cycle = ring.register(); - let endp_desc = port_state - .dev_desc - .as_ref() - .unwrap() + let dev_desc = port_state.dev_desc.as_ref().ok_or(Error::new(EBADFD))?; + let endp_desc = dev_desc .config_descs .get(0) .ok_or(Error::new(EIO))? @@ -2831,13 +2827,9 @@ impl Xhci { Ok(()) } pub fn endp_direction(&self, port_num: PortId, endp_num: u8) -> Result { - Ok(self - .port_states - .get(&port_num) - .ok_or(Error::new(EIO))? - .dev_desc - .as_ref() - .unwrap() + let port_state = self.port_states.get(&port_num).ok_or(Error::new(EIO))?; + let dev_desc = port_state.dev_desc.as_ref().ok_or(Error::new(EBADFD))?; + Ok(dev_desc .config_descs .first() .ok_or(Error::new(EIO))?