From afb12dc98b1ecf33f05070834faccabd4145ac0e Mon Sep 17 00:00:00 2001 From: Jeremy Soller Date: Thu, 16 Feb 2023 15:28:34 -0700 Subject: [PATCH] xhci: ensure doorbell is rung after irq reactor has state --- xhcid/src/xhci/irq_reactor.rs | 49 ++++++++++++++++++++++----- xhcid/src/xhci/mod.rs | 26 ++++++++++----- xhcid/src/xhci/scheme.rs | 63 +++++++++++++++++++++++++---------- 3 files changed, 105 insertions(+), 33 deletions(-) diff --git a/xhcid/src/xhci/irq_reactor.rs b/xhcid/src/xhci/irq_reactor.rs index 97f4585fd3..602cce288f 100644 --- a/xhcid/src/xhci/irq_reactor.rs +++ b/xhcid/src/xhci/irq_reactor.rs @@ -17,6 +17,7 @@ use syscall::Io; use event::{Event, EventQueue}; use super::Xhci; +use super::doorbell::Doorbell; use super::ring::Ring; use super::trb::{Trb, TrbCompletionCode, TrbType}; use super::event::EventRing; @@ -211,7 +212,7 @@ impl IrqReactor { self.hci.run.lock().unwrap().ints[0].erdp.write(dequeue_pointer); } fn handle_requests(&mut self) { - self.states.extend(self.receiver.try_iter().inspect(|req| trace!("Received request: {:?}", req))); + self.states.extend(self.receiver.try_iter().inspect(|req| trace!("Received request: {:X?}", req))); } fn acknowledge(&mut self, trb: Trb) { //TODO: handle TRBs without an attached state @@ -298,7 +299,7 @@ impl IrqReactor { index += 1; } - warn!("Lost event TRB type {}: {:?}", trb.trb_type(), trb); + warn!("Lost event TRB type {}, completion code: {}: {:X?}", trb.trb_type(), trb.completion_code(), trb); } fn acknowledge_failed_transfer_trbs(&mut self, trb: Trb) { let mut index = 0; @@ -351,8 +352,30 @@ struct FutureState { state_kind: StateKind, } +pub struct EventDoorbell { + dbs: Arc>, + index: usize, + data: u32, +} + +impl EventDoorbell { + pub fn new(hci: &Xhci, index: usize, data: u32) -> Self { + Self { + //TODO: simplify this logic, maybe just use a raw pointer? + dbs: hci.dbs.clone(), + index, + data, + } + } + + pub fn ring(self) { + trace!("Ring doorbell {} with data {}", self.index, self.data); + self.dbs.lock().unwrap()[self.index].write(self.data); + } +} + enum EventTrbFuture { - Pending { state: FutureState, sender: Sender, }, + Pending { state: FutureState, sender: Sender, doorbell_opt: Option }, Finished, } @@ -363,10 +386,12 @@ impl Future for EventTrbFuture { let this = self.get_mut(); let message = match this { - &mut Self::Pending { ref state, ref sender } => match state.message.lock().unwrap().take() { + &mut Self::Pending { ref state, ref sender, ref mut doorbell_opt } => match state.message.lock().unwrap().take() { Some(message) => message, None => { + // Register state with IRQ reactor + trace!("Send state {:X?}", state.state_kind); sender.send(State { message: Arc::clone(&state.message), is_isoch_or_vf: state.is_isoch_or_vf, @@ -374,6 +399,11 @@ impl Future for EventTrbFuture { waker: context.waker().clone(), }).expect("IRQ reactor thread unexpectedly stopped"); + // Doorbell must be rung after sending state + if let Some(doorbell) = doorbell_opt.take() { + doorbell.ring(); + } + return task::Poll::Pending; } } @@ -414,26 +444,27 @@ impl Xhci { Some(function(ring_ref)) } - pub fn next_transfer_event_trb(&self, ring_id: RingId, ring: &Ring, trb: &Trb) -> impl Future + Send + Sync + 'static { + pub fn next_transfer_event_trb(&self, ring_id: RingId, ring: &Ring, trb: &Trb, doorbell: EventDoorbell) -> impl Future + Send + Sync + 'static { if ! trb.is_transfer_trb() { panic!("Invalid TRB type given to next_transfer_event_trb(): {} (TRB {:?}. Expected transfer TRB.", trb.trb_type(), trb) } let is_isoch_or_vf = trb.trb_type() == TrbType::Isoch as u8; - + let phys_ptr = ring.trb_phys_ptr(self.cap.ac64(), trb); EventTrbFuture::Pending { state: FutureState { is_isoch_or_vf, state_kind: StateKind::Transfer { ring_id, - phys_ptr: ring.trb_phys_ptr(self.cap.ac64(), trb), + phys_ptr, }, message: Arc::new(Mutex::new(None)), }, sender: self.irq_reactor_sender.clone(), + doorbell_opt: Some(doorbell), } } - pub fn next_command_completion_event_trb(&self, command_ring: &Ring, trb: &Trb) -> impl Future + Send + Sync + 'static { + pub fn next_command_completion_event_trb(&self, command_ring: &Ring, trb: &Trb, doorbell: EventDoorbell) -> impl Future + Send + Sync + 'static { if ! trb.is_command_trb() { panic!("Invalid TRB type given to next_command_completion_event_trb(): {} (TRB {:?}. Expected command TRB.", trb.trb_type(), trb) } @@ -447,6 +478,7 @@ impl Xhci { message: Arc::new(Mutex::new(None)), }, sender: self.irq_reactor_sender.clone(), + doorbell_opt: Some(doorbell), } } pub fn next_misc_event_trb(&self, trb_type: TrbType) -> impl Future + Send + Sync + 'static { @@ -468,6 +500,7 @@ impl Xhci { message: Arc::new(Mutex::new(None)), }, sender: self.irq_reactor_sender.clone(), + doorbell_opt: None, } } diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index 4e081c7235..ef8d103016 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -38,7 +38,7 @@ mod trb; use self::capability::CapabilityRegs; use self::context::{DeviceContextList, InputContext, ScratchpadBufferArray, StreamContextArray}; use self::doorbell::Doorbell; -use self::irq_reactor::{IrqReactor, NewPendingTrb, RingId}; +use self::irq_reactor::{EventDoorbell, IrqReactor, NewPendingTrb, RingId}; use self::event::EventRing; use self::extended::{CapabilityId, ExtendedCapabilitiesIter, ProtocolSpeed, SupportedProtoCap}; use self::operational::OperationalRegs; @@ -113,13 +113,23 @@ impl Xhci { let last_index = ring.next_index(); let (cmd, cycle) = (&mut ring.trbs[last_index], ring.cycle); - cmd.status(0, false, true, false, false, cycle); - self.next_transfer_event_trb(RingId::default_control_pipe(port as u8), &ring, &ring.trbs[last_index]) + let interrupter = 0; + // When the data stage is in, the status stage must be out + let input = false; + let ioc = true; + let ch = false; + let ent = false; + cmd.status(interrupter, input, ioc, ch, ent, cycle); + + self.next_transfer_event_trb( + RingId::default_control_pipe(port as u8), + &ring, + &ring.trbs[last_index], + EventDoorbell::new(self, usize::from(slot), Self::def_control_endp_doorbell()) + ) }; - self.dbs.lock().unwrap()[usize::from(slot)].write(Self::def_control_endp_doorbell()); - let trbs = future.await; let event_trb = trbs.event_trb; let status_trb = trbs.src_trb.unwrap(); @@ -170,7 +180,7 @@ pub struct Xhci { // without having to wrap every element in a lock (which wouldn't work since they're packed). op: Mutex<&'static mut OperationalRegs>, ports: Mutex<&'static mut [Port]>, - dbs: Mutex<&'static mut [Doorbell]>, + dbs: Arc>, run: Mutex<&'static mut RuntimeRegs>, cmd: Mutex, primary_event_ring: Mutex, @@ -307,7 +317,7 @@ impl Xhci { op: Mutex::new(op), ports: Mutex::new(ports), - dbs: Mutex::new(dbs), + dbs: Arc::new(Mutex::new(dbs)), run: Mutex::new(run), dev_ctx: DeviceContextList::new(cap.ac64(), max_slots)?, @@ -404,7 +414,7 @@ impl Xhci { // Ring command doorbell debug!("Ringing command doorbell."); - self.dbs.get_mut().unwrap()[0].write(0); + self.dbs.lock().unwrap()[0].write(0); info!("XHCI initialized."); diff --git a/xhcid/src/xhci/scheme.rs b/xhcid/src/xhci/scheme.rs index 6352b0b694..0c0cf893fc 100644 --- a/xhcid/src/xhci/scheme.rs +++ b/xhcid/src/xhci/scheme.rs @@ -27,7 +27,7 @@ use super::context::{ }; use super::doorbell::Doorbell; use super::extended::ProtocolSpeed; -use super::irq_reactor::RingId; +use super::irq_reactor::{EventDoorbell, RingId}; use super::operational::OperationalRegs; use super::ring::Ring; use super::runtime::RuntimeRegs; @@ -274,11 +274,13 @@ impl Xhci { // get the future here before awaiting, to destroy the lock before deadlock let command_trb = &command_ring.trbs[cmd_index]; - self.next_command_completion_event_trb(&*command_ring, command_trb) + self.next_command_completion_event_trb( + &*command_ring, + command_trb, + EventDoorbell::new(self, 0, 0) + ) }; - self.dbs.lock().unwrap()[0].write(0); - let trbs = next_event.await; let event_trb = trbs.event_trb; let command_trb = trbs.src_trb.expect("Command completion event TRBs shall always have a valid pointer to a valid source command TRB"); @@ -298,7 +300,7 @@ impl Xhci { where D: FnMut(&mut Trb, bool) -> ControlFlow, { - let (future, slot) = { + let future = { let mut port_state = self.port_state_mut(port_num)?; let slot = port_state.slot; @@ -334,11 +336,14 @@ impl Xhci { let ent = false; cmd.status(interrupter, input, ioc, ch, ent, cycle); - (self.next_transfer_event_trb(RingId::default_control_pipe(port_num as u8), ring, &ring.trbs[last_index]), slot) + self.next_transfer_event_trb( + RingId::default_control_pipe(port_num as u8), + ring, + &ring.trbs[last_index], + EventDoorbell::new(self, usize::from(slot), Self::def_control_endp_doorbell()) + ) }; - self.dbs.lock().unwrap()[usize::from(slot)].write(Self::def_control_endp_doorbell()); - let trbs = future.await; let event_trb = trbs.event_trb; let status_trb = trbs.src_trb.unwrap(); @@ -374,6 +379,28 @@ impl Xhci { let slot = port_state.slot; + let (doorbell_data_stream, doorbell_data_no_stream) = { + let endp_desc = port_state + .dev_desc.as_ref().unwrap() + .config_descs.get(usize::from(cfg_idx)).ok_or(Error::new(EIO))? + .interface_descs.get(usize::from(if_idx)).ok_or(Error::new(EIO))? + .endpoints.get(usize::from(endp_idx)).ok_or(Error::new(EBADFD))?; + + //TODO: clean this up + ( + Self::endp_doorbell( + endp_num, + endp_desc, + stream_id, + ), + Self::endp_doorbell( + endp_num, + endp_desc, + 0, + ), + ) + }; + let endp_state = port_state .endpoint_states .get_mut(&endp_num) @@ -399,21 +426,23 @@ impl Xhci { match d(trb, cycle) { ControlFlow::Break => { - break self.next_transfer_event_trb(super::irq_reactor::RingId { port: port_num as u8, endpoint_num: endp_num, stream_id }, ring, &ring.trbs[last_index]); + break self.next_transfer_event_trb( + super::irq_reactor::RingId { port: port_num as u8, endpoint_num: endp_num, stream_id }, + ring, + &ring.trbs[last_index], + EventDoorbell::new(self, usize::from(slot), if has_streams { + doorbell_data_stream + } else { + doorbell_data_no_stream + }), + ); } ControlFlow::Continue => continue, } }; - let endp_desc = port_state.dev_desc.as_ref().unwrap().config_descs.get(usize::from(cfg_idx)).ok_or(Error::new(EIO))?.interface_descs.get(usize::from(if_idx)).ok_or(Error::new(EIO))?.endpoints.get(usize::from(endp_idx)).ok_or(Error::new(EBADFD))?; - - self.dbs.lock().unwrap()[usize::from(slot)].write(Self::endp_doorbell( - endp_num, - endp_desc, - if has_streams { stream_id } else { 0 }, - )); - drop(port_state); + let trbs = future.await; let event_trb = trbs.event_trb; let transfer_trb = trbs.src_trb.unwrap();