Skip to content
10 changes: 10 additions & 0 deletions src/device/pci/xhci.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::device::{
traits::PciDevice,
},
xhci::{
controller_reset::ResetCoordinator,
endpoint_launcher::EndpointLauncher,
port::{get_portli_id, get_portpmsc_id, get_portsc_id, HotplugControl, PortArray},
real_device::CompleteRealDevice,
Expand Down Expand Up @@ -60,6 +61,15 @@ impl<CRD: CompleteRealDevice> XhciController<CRD> {
slot_manager.create_slot_worker_handle(),
usbcmd.value_reference(),
);
ResetCoordinator::start(
usbcmd.clone(),
[
Box::new(command_ring.reset_sender()),
Box::new(interrupter.reset_sender()),
Box::new(slot_manager.reset_sender()),
],
&async_runtime,
);

Self {
config_space: Mutex::new(Self::build_config_space()),
Expand Down
45 changes: 43 additions & 2 deletions src/device/xhci/command_ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,27 +8,45 @@ use std::sync::{
use anyhow::anyhow;
use tokio::{
runtime,
sync::mpsc::{self, error::TryRecvError},
sync::{
mpsc::{self, error::TryRecvError},
oneshot,
},
};
use tracing::{debug, info, trace, warn};

use crate::device::{
bus::BusDeviceRef,
pci::constants::xhci::operational::{crcr, usbcmd},
xhci::{
controller_reset::ResetSender,
interrupter::EventSender,
linked_ring::LinkedRing,
slot_manager::SlotWorkerHandle,
trb::{CommandTrb, CommandTrbVariant, CompletionCode, EventTrb},
},
};

#[derive(Debug)]
#[derive(Debug, Clone)]
pub struct CommandRing {
running: Arc<AtomicBool>,
sender_to_worker: mpsc::UnboundedSender<WorkerMessage>,
}

#[derive(Debug, Clone)]
pub struct CommandRingResetSender {
sender_to_worker: mpsc::UnboundedSender<WorkerMessage>,
}

impl ResetSender for CommandRingResetSender {
fn send_reset(&self, completion_notifier: oneshot::Sender<()>) -> anyhow::Result<()> {
self.sender_to_worker
.send(WorkerMessage::Reset(completion_notifier))?;

Ok(())
}
}

#[derive(Debug)]
struct CommandWorker {
state: WorkerState,
Expand All @@ -54,6 +72,7 @@ enum WorkerMessage {
SetDequeuePointerAndCS(u64, bool),
Doorbell,
Stop,
Reset(oneshot::Sender<()>),
}

impl CommandRing {
Expand Down Expand Up @@ -144,6 +163,12 @@ impl CommandRing {
}
}

pub fn reset_sender(&self) -> CommandRingResetSender {
CommandRingResetSender {
sender_to_worker: self.sender_to_worker.clone(),
}
}

fn send_to_worker(&self, msg: WorkerMessage) -> anyhow::Result<()> {
self.sender_to_worker.send(msg)?;

Expand All @@ -166,6 +191,10 @@ impl CommandWorker {
loop {
match &self.state {
WorkerState::Stopped => match self.next_msg().await? {
WorkerMessage::Reset(completion) => {
self.commandring_running.store(false, Ordering::Relaxed);
completion.send(()).ok();
}
WorkerMessage::SetDequeuePointerAndCS(dp, cs) => {
debug!("Updating command ring parameters: dp={dp:#x}, cs={cs}");
self.ring.set_dequeue_pointer(dp, cs);
Expand All @@ -183,6 +212,11 @@ impl CommandWorker {
msg => warn!("Unexpected message: msg={msg:?}, state={:?}", self.state),
},
WorkerState::Idle => match self.next_msg().await? {
WorkerMessage::Reset(completion) => {
self.commandring_running.store(false, Ordering::Relaxed);
self.state = WorkerState::Stopped;
completion.send(()).ok();
}
WorkerMessage::Doorbell => {
self.state = WorkerState::LookingForNewCommand;
}
Expand All @@ -198,10 +232,17 @@ impl CommandWorker {
};

match msg {
WorkerMessage::Reset(completion) => {
self.commandring_running.store(false, Ordering::Relaxed);
self.state = WorkerState::Stopped;
completion.send(()).ok();
break;
}
WorkerMessage::Doorbell => {
// we are already active and running, silently consume
}
WorkerMessage::Stop => {
self.commandring_running.store(false, Ordering::Relaxed);
self.state = WorkerState::Stopping;
break;
}
Expand Down
69 changes: 69 additions & 0 deletions src/device/xhci/controller_reset.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
use tokio::{runtime, sync::oneshot};
use tracing::{error, info};

use crate::device::{pci::constants::xhci::operational::usbcmd, xhci::registers::UsbcmdRegister};

/// Sends reset requests to a component and reports when the reset finished.
///
/// The completion notifier must be triggered by the worker after it has applied
/// its reset side effects.
pub trait ResetSender: Send + Sync {
fn send_reset(&self, completion_notifier: oneshot::Sender<()>) -> anyhow::Result<()>;

fn reset(&self) -> anyhow::Result<oneshot::Receiver<()>> {
let (send, recv) = oneshot::channel();
self.send_reset(send)?;

Ok(recv)
}
}

/// Coordinates host controller reset across all resettable xHCI components.
///
/// The coordinator waits for `USBCMD.HCRST`, resets all registered components,
/// and clears `HCRST` after all reset completion signals were received.
pub struct ResetCoordinator {
usbcmd: UsbcmdRegister,
reset_senders: [Box<dyn ResetSender>; 3],
}

impl ResetCoordinator {
pub fn start(
usbcmd: UsbcmdRegister,
reset_senders: [Box<dyn ResetSender>; 3],
async_runtime: &runtime::Handle,
) {
let coordinator = Self {
usbcmd,
reset_senders,
};

async_runtime.spawn(coordinator.run_loop());
}

async fn run_loop(self) {
loop {
self.usbcmd.hcrst_notification().await;

if self.usbcmd.read() & usbcmd::HCRST == 0 {
continue;
}

self.reset()
.await
.map_err(|err| error!("failed to reset host controller: {err}"))
.ok();
}
}

async fn reset(&self) -> anyhow::Result<()> {
info!("Host Controller Reset requested");

for reset_sender in &self.reset_senders {
reset_sender.reset()?.await?;
}
self.usbcmd.clear_hcrst();

Ok(())
}
}
8 changes: 8 additions & 0 deletions src/device/xhci/event_ring.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,14 @@ impl EventRing {
}
}

pub const fn reset(&mut self) {
// The driver configures a fresh event ring after reset by writing ERSTBA.
self.enqueue_pointer = 0;
self.trb_count = 0;
self.erst_count = 0;
self.cycle_state = false;
}

/// Configure the Event Ring.
///
/// Call this function when the driver writes to the ERSTBA register (as
Expand Down
77 changes: 65 additions & 12 deletions src/device/xhci/interrupter.rs
Original file line number Diff line number Diff line change
@@ -1,23 +1,46 @@
use anyhow::{anyhow, Context};
use tokio::sync::mpsc;
use tokio::sync::{mpsc, oneshot};
use tokio::{runtime, select};
use tracing::{debug, info};

use crate::device::bus::BusDeviceRef;
use crate::device::interrupt_line::{DummyInterruptLine, InterruptLine};
use crate::device::pci::constants::xhci::runtime::IMOD_DEFAULT;
use crate::device::xhci::controller_reset::ResetSender;
use crate::device::xhci::event_ring::EventRing;
use crate::device::xhci::registers::{ErstbaRegister, GenericRwRegister};
use crate::device::xhci::trb::EventTrb;
use std::sync::Arc;

#[derive(Debug)]
#[derive(Debug, Clone)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this change? You never use the clone functionality, the code works perfectly without Clone.

pub struct Interrupter {
pub registers: InterrupterRegisters,
/// Transmits events to send to the worker
msg_sender: mpsc::UnboundedSender<InterrupterMessage>,
}

#[derive(Debug, Clone)]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is to create an InterrupterResetSender once and give it to the ResetCoordinator. There is no need for cloning intended, so I would rather not remove the Clone from here.

pub struct InterrupterResetSender {
registers: InterrupterRegisters,
msg_sender: mpsc::UnboundedSender<InterrupterMessage>,
}

impl ResetSender for InterrupterResetSender {
fn send_reset(&self, completion_notifier: oneshot::Sender<()>) -> anyhow::Result<()> {
self.registers.interrupt_management.write(0);
self.registers
.interrupt_moderation_interval
.write(IMOD_DEFAULT);
self.registers.erst_base_address.reset();
self.registers.erst_size.write(0);
self.registers.eventring_dequeue_pointer.write(0);
Comment on lines +30 to +36

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The interrupter worker should execute all this code once it receives the message. send_reset should really only do sending the reset message to the worker.

self.msg_sender
.send(InterrupterMessage::Reset(completion_notifier))?;

Ok(())
}
}

#[derive(Debug, Clone)]
pub struct InterrupterRegisters {
/// IMAN: Interrupt management register
Expand Down Expand Up @@ -58,6 +81,7 @@ struct EventWorker {
enum InterrupterMessage {
SendEvent(EventTrb),
UpdateInterruptLine(Arc<dyn InterruptLine>),
Reset(oneshot::Sender<()>),
}

#[derive(Debug, Clone)]
Expand Down Expand Up @@ -115,6 +139,13 @@ impl Interrupter {
sender: self.msg_sender.clone(),
}
}

pub fn reset_sender(&self) -> InterrupterResetSender {
InterrupterResetSender {
registers: self.registers.clone(),
msg_sender: self.msg_sender.clone(),
}
}
}

impl EventWorker {
Expand All @@ -134,29 +165,44 @@ impl EventWorker {
}
}

// function only returns on error, but cannot use ! in Result
async fn run_loop(&mut self) -> anyhow::Result<()> {
// first ERSTBA write starts the event ring.
// drop all events that happen before.
// interrupt line updates should be processed.
// Each ERSTBA write configures a new event ring. A host controller reset
// clears that configuration and sends us back to waiting for ERSTBA.
loop {
self.wait_for_event_ring_configuration().await?;
self.event_ring.configure(
self.registers.erst_base_address.erstba(),
self.registers.erst_size.read() as u32,
);

self.run_configured().await?;
}
Comment thread
ziyifu225 marked this conversation as resolved.
}

// The first ERSTBA write starts the event ring. Drop events that happen
// before configuration, but keep processing control messages.
async fn wait_for_event_ring_configuration(&mut self) -> anyhow::Result<()> {
loop {
select! {
_ = self.registers.erst_base_address.write_notification() => break,
_ = self.registers.erst_base_address.write_notification() => return Ok(()),
// we cannot use self.next_msg() here because it borrows self mutable, clashing
// with the borrow of self.registers above
msg = self.msg_recv.recv() => match msg.ok_or_else(|| anyhow!("event channel closed"))? {
InterrupterMessage::SendEvent(_) => {}
InterrupterMessage::UpdateInterruptLine(interrupt_line) => self.interrupt_line = interrupt_line,
InterrupterMessage::Reset(completion) => {
self.event_ring.reset();
completion.send(()).ok();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use completion.send_anyhow(())?; (needs use crate::oneshot_anyhow::SendWithAnyhowError;)

}
},
}
}
self.event_ring.configure(
self.registers.erst_base_address.erstba(),
self.registers.erst_size.read() as u32,
);
}

// Process messages while the event ring is configured. A reset clears the
// current configuration and returns to the outer loop to wait for ERSTBA.
async fn run_configured(&mut self) -> anyhow::Result<()> {
loop {
// process TRB
match self.next_msg().await? {
InterrupterMessage::SendEvent(event_trb) => {
self.event_ring.enqueue(
Expand All @@ -172,7 +218,14 @@ impl EventWorker {
self.interrupt_line = interrupt_line;
debug!("Updated interrupt line");
}
InterrupterMessage::Reset(completion) => {
self.event_ring.reset();
completion.send(()).ok();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use completion.send_anyhow(())?; (needs use crate::oneshot_anyhow::SendWithAnyhowError;)

break;
}
}
}

Ok(())
}
}
1 change: 1 addition & 0 deletions src/device/xhci/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
pub mod command_ring;
pub mod controller_reset;
pub mod endpoint;
pub mod endpoint_handle;
pub mod endpoint_launcher;
Expand Down
Loading