diff --git a/usbscsid/src/protocol/bot.rs b/usbscsid/src/protocol/bot.rs index 589dfa367f..62c78f4fd8 100644 --- a/usbscsid/src/protocol/bot.rs +++ b/usbscsid/src/protocol/bot.rs @@ -167,27 +167,23 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { self.current_tag += 1; let tag = self.current_tag; - println!("{}", base64::encode(cb)); - println!(); - let mut cbw_bytes = [0u8; 31]; let cbw = plain::from_mut_bytes::(&mut cbw_bytes).unwrap(); *cbw = CommandBlockWrapper::new(tag, data.len() as u32, data.direction().into(), 0, cb)?; let cbw = *cbw; - println!("{}", base64::encode(&cbw_bytes)); - - dbg!(self.bulk_in.status()?, self.bulk_out.status()?); + // TODO: Is this needed? bulk_only_mass_storage_reset(&self.handle, self.interface_num.into())?; match self.bulk_out.transfer_write(&cbw_bytes)? { PortTransferStatus::Stalled => { - println!("bulk out endpoint stalled when sending CBW"); - self.clear_stall_out()?; - dbg!(self.bulk_in.status()?, self.bulk_out.status()?); + // TODO: Error handling + panic!("bulk out endpoint stalled when sending CBW {:?}", cbw); + //self.clear_stall_out()?; + //dbg!(self.bulk_in.status()?, self.bulk_out.status()?); } PortTransferStatus::ShortPacket(n) if n != 31 => { - panic!("received short packet when sending CBW ({} != 31)", n); + //panic!("received short packet when sending CBW ({} != 31)", n); } _ => (), } @@ -200,8 +196,8 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { panic!("received short packed (len {}) when transferring data", len) } PortTransferStatus::Stalled => { - println!("bulk in endpoint stalled when reading data"); - self.clear_stall_in()?; + panic!("bulk in endpoint stalled when reading data"); + //self.clear_stall_in()?; } PortTransferStatus::Unknown => { return Err(ProtocolError::XhciError( @@ -211,16 +207,15 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { )) } }; - println!("{}", base64::encode(&buffer[..])); } DeviceReqData::Out(buffer) => match self.bulk_out.transfer_write(buffer)? { PortTransferStatus::Success => (), PortTransferStatus::ShortPacket(len) => { - panic!("received short packed (len {}) when transferring data", len) + panic!("received short packet (len {}) when transferring data", len) } PortTransferStatus::Stalled => { - println!("bulk out endpoint stalled when reading data"); - self.clear_stall_out()?; + panic!("bulk out endpoint stalled when reading data"); + //self.clear_stall_out()?; } PortTransferStatus::Unknown => { return Err(ProtocolError::XhciError( @@ -235,28 +230,27 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { match self.bulk_in.transfer_read(&mut csw_buffer)? { PortTransferStatus::Stalled => { - println!("bulk in endpoint stalled when reading CSW"); - self.clear_stall_in()?; + panic!("bulk in endpoint stalled when reading CSW"); + //self.clear_stall_in()?; } PortTransferStatus::ShortPacket(n) if n != 13 => { panic!("received a short packet when reading CSW ({} != 13)", n) } _ => (), }; - println!("{}", base64::encode(&csw_buffer)); let csw = plain::from_bytes::(&csw_buffer).unwrap(); - dbg!(csw); if !csw.is_valid() || csw.tag != cbw.tag { - self.reset_recovery()?; + panic!("Invald CSW {:?} (for CBW {:?})", csw, cbw); + //self.reset_recovery()?; } - if self.bulk_in.status()? == EndpointStatus::Halted + /*if self.bulk_in.status()? == EndpointStatus::Halted || self.bulk_out.status()? == EndpointStatus::Halted { println!("Trying to recover from stall"); dbg!(self.bulk_in.status()?, self.bulk_out.status()?); - } + }*/ Ok(SendCommandStatus { kind: if csw.status == CswStatus::Passed as u8 { diff --git a/usbscsid/src/scsi/mod.rs b/usbscsid/src/scsi/mod.rs index a4b1f3e243..5d4bd2d798 100644 --- a/usbscsid/src/scsi/mod.rs +++ b/usbscsid/src/scsi/mod.rs @@ -25,8 +25,12 @@ const REQUEST_SENSE_CMD_LEN: u8 = 6; const MIN_INQUIRY_ALLOC_LEN: u16 = 5; const MIN_REPORT_SUPP_OPCODES_ALLOC_LEN: u32 = 4; +type Result = Result; + #[derive(Debug, Error)] pub enum ScsiError { + // TODO: Add some kind of context here, since it's very useful indeed to be able to see which + // command returned the protocol error. #[error("protocol error when sending command: {0}")] ProtocolError(#[from] ProtocolError), @@ -35,10 +39,13 @@ pub enum ScsiError { } impl Scsi { - pub fn new(protocol: &mut dyn Protocol) -> Self { + pub fn new(protocol: &mut dyn Protocol) -> Result { assert_eq!(std::mem::size_of::(), 96); + let mut this = Self { command_buffer: [0u8; 16], + // separate buffer since the inquiry data is most likely going to be used in the + // future. inquiry_buffer: [0u8; 259], // additional_len = 255 max data_buffer: Vec::new(), block_size: 0, @@ -46,13 +53,15 @@ impl Scsi { }; // Get the max length that the device supports, of the Standard Inquiry Data. - let max_inquiry_len = this.get_inquiry_alloc_len(protocol); + let max_inquiry_len = this.get_inquiry_alloc_len(protocol)?; // Get the Standard Inquiry Data. - this.get_standard_inquiry_data(protocol, max_inquiry_len); - this.res_standard_inquiry_data(); + this.get_standard_inquiry_data(protocol, max_inquiry_len)?; + + let version = this.res_standard_inquiry_data().version(); + println!("Inquiry version: {}", version); let (block_size, block_count) = { - let (_, blkdescs, mode_page_iter) = this.get_mode_sense10(protocol).unwrap(); + let (_, blkdescs, mode_page_iter) = this.get_mode_sense10(protocol)?; // TODO: Can there be multiple disks at all? let only_blkdesc = blkdescs.get(0).unwrap(); @@ -62,14 +71,14 @@ impl Scsi { this.block_size = block_size; this.block_count = block_count; - this + Ok(this) } - pub fn get_inquiry_alloc_len(&mut self, protocol: &mut dyn Protocol) -> u16 { - self.get_standard_inquiry_data(protocol, MIN_INQUIRY_ALLOC_LEN); + pub fn get_inquiry_alloc_len(&mut self, protocol: &mut dyn Protocol) -> Result { + self.get_standard_inquiry_data(protocol, MIN_INQUIRY_ALLOC_LEN)?; let standard_inquiry_data = self.res_standard_inquiry_data(); - 4 + u16::from(standard_inquiry_data.additional_len) + Ok(4 + u16::from(standard_inquiry_data.additional_len)) } - pub fn get_standard_inquiry_data(&mut self, protocol: &mut dyn Protocol, max_inquiry_len: u16) { + pub fn get_standard_inquiry_data(&mut self, protocol: &mut dyn Protocol, max_inquiry_len: u16) -> Result<()> { let inquiry = self.cmd_inquiry(); *inquiry = cmds::Inquiry::new(false, 0, max_inquiry_len, 0); @@ -77,10 +86,10 @@ impl Scsi { .send_command( &self.command_buffer[..INQUIRY_CMD_LEN as usize], DeviceReqData::In(&mut self.inquiry_buffer[..max_inquiry_len as usize]), - ) - .expect("Failed to send INQUIRY command"); + )?; + Ok(()) } - pub fn get_ff_sense(&mut self, protocol: &mut dyn Protocol, alloc_len: u8) { + pub fn get_ff_sense(&mut self, protocol: &mut dyn Protocol, alloc_len: u8) -> Result<()> { let request_sense = self.cmd_request_sense(); *request_sense = cmds::RequestSense::new(false, alloc_len, 0); self.data_buffer.resize(alloc_len.into(), 0); @@ -88,8 +97,7 @@ impl Scsi { .send_command( &self.command_buffer[..REQUEST_SENSE_CMD_LEN as usize], DeviceReqData::In(&mut self.data_buffer[..alloc_len as usize]), - ) - .expect("Failed to send REQUEST_SENSE command"); + )?; } pub fn get_mode_sense10( &mut self, diff --git a/xhcid/src/driver_interface.rs b/xhcid/src/driver_interface.rs index 055bf3096f..8d448edc84 100644 --- a/xhcid/src/driver_interface.rs +++ b/xhcid/src/driver_interface.rs @@ -4,6 +4,7 @@ pub extern crate smallvec; use std::convert::TryFrom; use std::fs::{File, OpenOptions}; use std::io::prelude::*; +use std::num::NonZeroU8; use std::{io, result, str}; use serde::{Deserialize, Serialize}; @@ -176,17 +177,17 @@ impl EndpDesc { self.is_isoch() } } - pub fn max_streams(&self) -> u8 { + pub fn log_max_streams(&self) -> Option { self.ssc .as_ref() .map(|ssc| { if self.is_bulk() { - 1 << (ssc.attributes & 0x1F) + let raw = ssc.attributes & 0x1F; + NonZeroU8::new(raw) } else { - 0 + None } - }) - .unwrap_or(0) + }).flatten() } pub fn isoch_mult(&self, lec: bool) -> u8 { if !lec && self.is_isoch() { @@ -597,9 +598,9 @@ pub enum XhciEndpCtlRes { impl XhciEndpHandle { fn ctl_req(&mut self, ctl_req: &XhciEndpCtlReq) -> result::Result<(), XhciClientHandleError> { - let ctl_buffer = serde_json::to_vec(ctl_req).expect("serde"); + let ctl_buffer = serde_json::to_vec(ctl_req)?; - let ctl_bytes_written = self.ctl.write(&ctl_buffer).expect("ctlwrite"); + let ctl_bytes_written = self.ctl.write(&ctl_buffer)?; if ctl_bytes_written != ctl_buffer.len() { return Err(Invalid("xhcid didn't process all of the ctl bytes").into()); } @@ -609,7 +610,7 @@ impl XhciEndpHandle { fn ctl_res(&mut self) -> result::Result { // a response must never exceed 256 bytes let mut ctl_buffer = [0u8; 256]; - let ctl_bytes_read = self.ctl.read(&mut ctl_buffer).expect("ctlread"); + let ctl_bytes_read = self.ctl.read(&mut ctl_buffer)?; let ctl_res = serde_json::from_slice(&ctl_buffer[..ctl_bytes_read as usize])?; Ok(ctl_res) @@ -618,9 +619,8 @@ impl XhciEndpHandle { self.ctl_req(&XhciEndpCtlReq::Reset { no_clear_feature }) } pub fn status(&mut self) -> result::Result { - self.ctl_req(&XhciEndpCtlReq::Status) - .expect("status ctlreq"); - match self.ctl_res().expect("status ctlres") { + self.ctl_req(&XhciEndpCtlReq::Status)?; + match self.ctl_res()? { XhciEndpCtlRes::Status(s) => Ok(s), _ => Err(Invalid("expected status response").into()), } @@ -635,10 +635,10 @@ impl XhciEndpHandle { direction, count: expected_len, }; - self.ctl_req(&req).expect("ctl_req"); + self.ctl_req(&req)?; - let bytes_read = f(&mut self.data).expect("f"); - let res = self.ctl_res().expect("ctl_res"); + let bytes_read = f(&mut self.data)?; + let res = self.ctl_res()?; match res { XhciEndpCtlRes::TransferResult(PortTransferStatus::Success) diff --git a/xhcid/src/xhci/command.rs b/xhcid/src/xhci/command.rs index 41c539fe2e..ca78d592fe 100644 --- a/xhcid/src/xhci/command.rs +++ b/xhcid/src/xhci/command.rs @@ -12,7 +12,7 @@ pub struct CommandRing { impl CommandRing { pub fn new() -> Result { Ok(CommandRing { - ring: Ring::new(true)?, + ring: Ring::new(16, true)?, events: EventRing::new()?, }) } diff --git a/xhcid/src/xhci/context.rs b/xhcid/src/xhci/context.rs index 9403fa0ac5..74944da174 100644 --- a/xhcid/src/xhci/context.rs +++ b/xhcid/src/xhci/context.rs @@ -141,7 +141,7 @@ impl StreamContextArray { // NOTE: stream_id 0 is reserved assert_ne!(stream_id, 0); - let ring = Ring::new(link)?; + let ring = Ring::new(16, link)?; let pointer = ring.register(); let sct = StreamContextType::PrimaryRing; diff --git a/xhcid/src/xhci/event.rs b/xhcid/src/xhci/event.rs index 903b34cbb6..1ee8367d75 100644 --- a/xhcid/src/xhci/event.rs +++ b/xhcid/src/xhci/event.rs @@ -13,19 +13,19 @@ pub struct EventRingSte { } pub struct EventRing { - pub ste: Dma, + pub ste: Dma<[EventRingSte]>, pub ring: Ring, } impl EventRing { pub fn new() -> Result { let mut ring = EventRing { - ste: Dma::zeroed()?, - ring: Ring::new(false)?, + ste: unsafe { Dma::zeroed_unsized(1)? }, + ring: Ring::new(32, false)?, }; - ring.ste.address.write(ring.ring.trbs.physical() as u64); - ring.ste.size.write(ring.ring.trbs.len() as u16); + ring.ste[0].address.write(ring.ring.trbs.physical() as u64); + ring.ste[0].size.write(ring.ring.trbs.len() as u16); Ok(ring) } diff --git a/xhcid/src/xhci/mod.rs b/xhcid/src/xhci/mod.rs index d8f27df065..6ccc58281a 100644 --- a/xhcid/src/xhci/mod.rs +++ b/xhcid/src/xhci/mod.rs @@ -417,7 +417,7 @@ impl Xhci { slot: u8, speed: u8, ) -> Result { - let mut ring = Ring::new(true)?; + let mut ring = Ring::new(16, true)?; { input_context.add_context.write(1 << 1 | 1); // Enable the slot (zeroth bit) and the control endpoint (first bit). @@ -505,12 +505,6 @@ impl Xhci { Ok(ring) } - pub fn ring_command_doorbell(&mut self) { - self.dbs[0].write(0); - } - pub fn ring_port_doorbell(&mut self, slot: u8, endpoint: u8, stream_id: u16) { - self.dbs[slot as usize].write(u32::from(endpoint) | (u32::from(stream_id) << 16)); - } pub fn trigger_irq(&mut self) -> bool { // Read the Interrupter Pending bit. diff --git a/xhcid/src/xhci/ring.rs b/xhcid/src/xhci/ring.rs index b1fae6ffdb..62aff3a2c6 100644 --- a/xhcid/src/xhci/ring.rs +++ b/xhcid/src/xhci/ring.rs @@ -5,16 +5,16 @@ use super::trb::Trb; pub struct Ring { pub link: bool, - pub trbs: Dma<[Trb; 16]>, + pub trbs: Dma<[Trb]>, pub i: usize, pub cycle: bool, } impl Ring { - pub fn new(link: bool) -> Result { + pub fn new(length: usize, link: bool) -> Result { Ok(Ring { link: link, - trbs: Dma::zeroed()?, + trbs: unsafe { Dma::zeroed_unsized(length)? }, i: 0, cycle: link, }) diff --git a/xhcid/src/xhci/scheme.rs b/xhcid/src/xhci/scheme.rs index f8c35f877b..72dcd19638 100644 --- a/xhcid/src/xhci/scheme.rs +++ b/xhcid/src/xhci/scheme.rs @@ -322,18 +322,18 @@ impl Xhci { .get_mut(&endp_num) .ok_or(Error::new(EBADF))?; - let ring: &mut Ring = match endp_state { + let (has_streams, ring) = match endp_state { EndpointState { transfer: super::RingOrStreams::Ring(ref mut ring), .. - } => ring, + } => (false, ring), EndpointState { transfer: super::RingOrStreams::Streams(stream_ctx_array), .. - } => stream_ctx_array + } => (true, stream_ctx_array .rings .get_mut(&1) - .ok_or(Error::new(EBADF))?, + .ok_or(Error::new(EBADF))?), }; loop { @@ -347,7 +347,7 @@ impl Xhci { self.dbs[usize::from(slot)].write(Self::endp_doorbell( endp_num, self.endp_desc(port_num, endp_num)?, - stream_id, + if has_streams { stream_id } else { 0 }, )); let cloned_trb = { @@ -586,7 +586,7 @@ impl Xhci { ) }; let lec = self.cap.lec(); - let max_psa_size = self.cap.max_psa_size(); + let log_max_psa_size = self.cap.max_psa_size(); let port_speed_id = self.ports[port].speed(); let speed_id: &ProtocolSpeed = self @@ -627,11 +627,16 @@ impl Xhci { let endp_num_xhc = Self::endp_num_to_dci(endp_num, endp_desc); - let max_streams = endp_desc.max_streams(); + let usb_log_max_streams = endp_desc.log_max_streams(); // TODO: Secondary streams. - let primary_streams = if max_streams != 0 { - cmp::min(max_streams, max_psa_size) + let primary_streams = if let Some(log_max_streams) = usb_log_max_streams { + // TODO: Can streams-capable be configured to not use streams? + if log_max_psa_size != 0 { + cmp::min(u8::from(log_max_streams), log_max_psa_size + 1) - 1 + } else { + 0 + } } else { 0 }; @@ -677,8 +682,8 @@ impl Xhci { let port_state = self.port_state_mut(port)?; - let ring_ptr = if max_streams != 0 { - let mut array = StreamContextArray::new(1 << (max_streams + 1))?; + let ring_ptr = if usb_log_max_streams.is_some() { + let mut array = StreamContextArray::new(1 << (primary_streams + 1))?; // TODO: Use as many stream rings as needed. array.add_ring(1, true)?; @@ -699,7 +704,7 @@ impl Xhci { array_ptr } else { - let ring = Ring::new(true)?; + let ring = Ring::new(16, true)?; let ring_ptr = ring.register(); assert_eq!( @@ -750,7 +755,7 @@ impl Xhci { let input_context_physical = self.input_context(port)?.physical(); self.execute_command("CONFIGURE_ENDPOINT", |trb, cycle| { trb.configure_endpoint(slot, input_context_physical, cycle) - }); + })?; // Tell the device about this configuration. let configuration_value = self.config_desc(port, req.config_desc)?.configuration_value; @@ -1663,7 +1668,6 @@ impl Xhci { endp_num: u8, clear_feature: bool, ) -> Result<()> { - dbg!(); if self.get_endp_status(port_num, endp_num)? != EndpointStatus::Halted { return Err(Error::new(EPROTO)); } @@ -1711,10 +1715,10 @@ impl Xhci { .endpoint_states .get_mut(&endp_num) .ok_or(Error::new(EBADFD))?; - let ring = match &mut endpoint_state.transfer { - &mut super::RingOrStreams::Ring(ref mut ring) => ring, + let (has_streams, ring) = match &mut endpoint_state.transfer { + &mut super::RingOrStreams::Ring(ref mut ring) => (false, ring), &mut super::RingOrStreams::Streams(ref mut arr) => { - arr.rings.get_mut(&1).ok_or(Error::new(EBADFD))? + (true, arr.rings.get_mut(&1).ok_or(Error::new(EBADFD))?) } }; @@ -1730,7 +1734,7 @@ impl Xhci { self.dbs[slot as usize].write(Self::endp_doorbell( endp_num, self.endp_desc(port_num, endp_num)?, - stream_id, + if has_streams { stream_id } else { 0 }, )); Ok(()) } @@ -1782,7 +1786,6 @@ impl Xhci { endp_num: u8, buf: &[u8], ) -> Result { - dbg!(); let ep_if_state = &mut self .port_states .get_mut(&port_num) @@ -1791,14 +1794,11 @@ impl Xhci { .get_mut(&endp_num) .ok_or(Error::new(EBADF))? .driver_if_state; - dbg!(); let req = serde_json::from_slice::(buf).or(Err(Error::new(EBADMSG)))?; - dbg!(); match req { XhciEndpCtlReq::Status => match ep_if_state { state @ EndpIfState::Init => *state = EndpIfState::WaitingForStatus, other => { - dbg!(other); return Err(Error::new(EBADF)); } }, @@ -1807,14 +1807,12 @@ impl Xhci { self.on_req_reset_device(port_num, endp_num, !no_clear_feature)? } other => { - dbg!(other); return Err(Error::new(EBADF)); } }, XhciEndpCtlReq::Transfer { direction, count } => match ep_if_state { state @ EndpIfState::Init => { if direction == XhciEndpCtlDirection::NoData { - dbg!(); // Yield the result directly because no bytes have to be sent or received // beforehand. let (completion_code, bytes_transferred) = @@ -1823,7 +1821,6 @@ impl Xhci { return Err(Error::new(EIO)); } let result = Self::transfer_result(completion_code, 0); - dbg!(); let new_state = &mut self .port_states .get_mut(&port_num) @@ -1834,7 +1831,6 @@ impl Xhci { .driver_if_state; *new_state = EndpIfState::WaitingForTransferResult(result) } else { - dbg!(); *state = EndpIfState::WaitingForDataPipe { direction, bytes_to_transfer: count, @@ -1843,12 +1839,10 @@ impl Xhci { } } other => { - dbg!(other); return Err(Error::new(EBADF)); } }, other => { - dbg!(other); return Err(Error::new(EBADF)); } } @@ -1885,6 +1879,10 @@ impl Xhci { self.transfer_write(port_num, endp_num - 1, buf)?; let result = Self::transfer_result(completion_code, some_bytes_transferred); + // To avoid having to read from the Ctl interface file, the client should stop + // invoking further data transfer calls if any single transfer returns fewer bytes + // than requested. + let ep_if_state = &mut self.endpoint_state_mut(port_num, endp_num)?.driver_if_state; if let &mut EndpIfState::WaitingForDataPipe { @@ -1893,7 +1891,8 @@ impl Xhci { ref mut bytes_transferred, } = ep_if_state { - if *bytes_transferred + some_bytes_transferred == bytes_to_transfer { + if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code == TrbCompletionCode::ShortPacket as u8 { + // TODO: Add an error flag to WaitingForTransferResult. *ep_if_state = EndpIfState::WaitingForTransferResult(result); } else { *bytes_transferred += some_bytes_transferred; @@ -1965,6 +1964,10 @@ impl Xhci { let (completion_code, some_bytes_transferred) = self.transfer_read(port_num, endp_num - 1, buf)?; + + // Just as with on_write_endp_data, a client issuing multiple reads must always + // stop reading if one read returns fewer bytes than expected. + let result = Self::transfer_result(completion_code, some_bytes_transferred); let ep_if_state = &mut self @@ -1981,7 +1984,7 @@ impl Xhci { ref mut bytes_transferred, } = ep_if_state { - if *bytes_transferred + some_bytes_transferred == bytes_to_transfer { + if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code == TrbCompletionCode::ShortPacket as u8 { *ep_if_state = EndpIfState::WaitingForTransferResult(result); } else { *bytes_transferred += some_bytes_transferred;