From fab13d6404ea5cc1f85e76c06dc89d7b88bc4ead Mon Sep 17 00:00:00 2001 From: 4lDO2 <4lDO2@protonmail.com> Date: Sun, 16 Feb 2020 21:27:41 +0100 Subject: [PATCH] Fix get_mode_sense10 and some bbb error handling. --- usbscsid/src/main.rs | 2 +- usbscsid/src/protocol/bot.rs | 141 +++++++++++++--------- usbscsid/src/scsi/mod.rs | 22 ++-- xhcid/{config.toml => config.toml.unused} | 0 xhcid/src/driver_interface.rs | 17 ++- xhcid/src/xhci/scheme.rs | 19 +-- 6 files changed, 121 insertions(+), 80 deletions(-) rename xhcid/{config.toml => config.toml.unused} (100%) diff --git a/usbscsid/src/main.rs b/usbscsid/src/main.rs index d6691d47c5..840dfb44bd 100644 --- a/usbscsid/src/main.rs +++ b/usbscsid/src/main.rs @@ -90,7 +90,7 @@ fn main() { let mut socket_file = unsafe { File::from_raw_fd(socket_fd as RawFd) }; //syscall::setrens(0, 0).expect("scsid: failed to enter null namespace"); - let mut scsi = Scsi::new(&mut *protocol); + let mut scsi = Scsi::new(&mut *protocol).expect("usbscsid: failed to setup SCSI"); let mut buffer = [0u8; 512]; scsi.read(&mut *protocol, 0, &mut buffer).unwrap(); println!("DISK CONTENT: {}", base64::encode(&buffer[..])); diff --git a/usbscsid/src/protocol/bot.rs b/usbscsid/src/protocol/bot.rs index 62c78f4fd8..17e2432b9f 100644 --- a/usbscsid/src/protocol/bot.rs +++ b/usbscsid/src/protocol/bot.rs @@ -3,7 +3,7 @@ use std::slice; use xhcid_interface::{ ConfDesc, DeviceReqData, EndpBinaryDirection, EndpDirection, EndpointStatus, IfDesc, Invalid, - PortReqRecipient, PortReqTy, PortTransferStatus, XhciClientHandle, XhciClientHandleError, + PortReqRecipient, PortReqTy, PortTransferStatus, PortTransferStatusKind, XhciClientHandle, XhciClientHandleError, XhciEndpHandle, }; @@ -129,20 +129,26 @@ impl<'a> BulkOnlyTransport<'a> { }) } fn clear_stall_in(&mut self) -> Result<(), XhciClientHandleError> { - self.bulk_in.reset(false)?; - self.handle.clear_feature( - PortReqRecipient::Endpoint, - u16::from(self.bulk_in_num), - FEATURE_ENDPOINT_HALT, - ) + if self.bulk_in.status()? == EndpointStatus::Halted { + self.bulk_in.reset(false)?; + self.handle.clear_feature( + PortReqRecipient::Endpoint, + u16::from(self.bulk_in_num), + FEATURE_ENDPOINT_HALT, + )?; + } + Ok(()) } fn clear_stall_out(&mut self) -> Result<(), XhciClientHandleError> { - self.bulk_out.reset(false)?; - self.handle.clear_feature( - PortReqRecipient::Endpoint, - u16::from(self.bulk_out_num), - FEATURE_ENDPOINT_HALT, - ) + if self.bulk_out.status()? == EndpointStatus::Halted { + self.bulk_out.reset(false)?; + self.handle.clear_feature( + PortReqRecipient::Endpoint, + u16::from(self.bulk_out_num), + FEATURE_ENDPOINT_HALT, + )?; + } + Ok(()) } fn reset_recovery(&mut self) -> Result<(), ProtocolError> { bulk_only_mass_storage_reset(self.handle, self.interface_num.into())?; @@ -156,6 +162,26 @@ impl<'a> BulkOnlyTransport<'a> { } Ok(()) } + fn read_csw_raw(&mut self, csw_buffer: &mut [u8; 13], already: bool) -> Result<(), ProtocolError> { + match self.bulk_in.transfer_read(&mut csw_buffer[..])? { + PortTransferStatus { kind: PortTransferStatusKind::Stalled, .. } => { + if already { + self.reset_recovery()?; + } + println!("bulk in endpoint stalled when reading CSW"); + self.clear_stall_in()?; + self.read_csw_raw(csw_buffer, true)?; + } + PortTransferStatus { kind: PortTransferStatusKind::ShortPacket, bytes_transferred } if bytes_transferred != 13 => { + panic!("received a short packet when reading CSW ({} != 13)", bytes_transferred) + } + _ => (), + } + Ok(()) + } + fn read_csw(&mut self, csw_buffer: &mut [u8; 13]) -> Result<(), ProtocolError> { + self.read_csw_raw(csw_buffer, false) + } } impl<'a> Protocol for BulkOnlyTransport<'a> { @@ -172,77 +198,78 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { *cbw = CommandBlockWrapper::new(tag, data.len() as u32, data.direction().into(), 0, cb)?; let cbw = *cbw; - // 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 => { + PortTransferStatus { kind: PortTransferStatusKind::Stalled, .. } => { // 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); + PortTransferStatus { bytes_transferred, .. } if bytes_transferred != 31 => { + panic!("received short packet when sending CBW ({} != 31)", bytes_transferred); } _ => (), } - match data { - DeviceReqData::In(buffer) => { - match self.bulk_in.transfer_read(buffer)? { - PortTransferStatus::Success => (), - PortTransferStatus::ShortPacket(len) => { - panic!("received short packed (len {}) when transferring data", len) + let early_residue: Option = match data { + DeviceReqData::In(buffer) => match self.bulk_in.transfer_read(buffer)? { + PortTransferStatus { kind, bytes_transferred } => match kind { + PortTransferStatusKind::Success => None, + PortTransferStatusKind::ShortPacket => { + println!("received short packet (len {}) when transferring data", bytes_transferred); + NonZeroU32::new(bytes_transferred) } - PortTransferStatus::Stalled => { + PortTransferStatusKind::Stalled => { panic!("bulk in endpoint stalled when reading data"); //self.clear_stall_in()?; } - PortTransferStatus::Unknown => { + PortTransferStatusKind::Unknown => { return Err(ProtocolError::XhciError( XhciClientHandleError::InvalidResponse(Invalid( "unknown transfer status", )), - )) + )); } - }; + } } DeviceReqData::Out(buffer) => match self.bulk_out.transfer_write(buffer)? { - PortTransferStatus::Success => (), - PortTransferStatus::ShortPacket(len) => { - panic!("received short packet (len {}) when transferring data", len) - } - PortTransferStatus::Stalled => { - panic!("bulk out endpoint stalled when reading data"); - //self.clear_stall_out()?; - } - PortTransferStatus::Unknown => { - return Err(ProtocolError::XhciError( - XhciClientHandleError::InvalidResponse(Invalid("unknown transfer status")), - )) + PortTransferStatus { kind, bytes_transferred } => match kind { + PortTransferStatusKind::Success => None, + PortTransferStatusKind::ShortPacket => { + println!("received short packet (len {}) when transferring data", bytes_transferred); + NonZeroU32::new(bytes_transferred) + } + PortTransferStatusKind::Stalled => { + panic!("bulk out endpoint stalled when reading data"); + //self.clear_stall_out()?; + } + PortTransferStatusKind::Unknown => { + return Err(ProtocolError::XhciError( + XhciClientHandleError::InvalidResponse(Invalid("unknown transfer status")), + )); + } } }, - DeviceReqData::NoData => (), - } + DeviceReqData::NoData => None, + }; let mut csw_buffer = [0u8; 13]; - - match self.bulk_in.transfer_read(&mut csw_buffer)? { - PortTransferStatus::Stalled => { - 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) - } - _ => (), - }; + self.read_csw(&mut csw_buffer)?; let csw = plain::from_bytes::(&csw_buffer).unwrap(); + let residue = early_residue.or(NonZeroU32::new(csw.data_residue)); + + if csw.status == CswStatus::Failed as u8 { + println!("CSW indicated failure (CSW {:?}, CBW {:?})", csw, cbw); + } + if !csw.is_valid() || csw.tag != cbw.tag { - panic!("Invald CSW {:?} (for CBW {:?})", csw, cbw); - //self.reset_recovery()?; + println!("Invald CSW {:?} (for CBW {:?})", csw, cbw); + self.reset_recovery()?; + if self.bulk_in.status()? == EndpointStatus::Halted || self.bulk_out.status()? == EndpointStatus::Halted { + return Err(ProtocolError::ProtocolError("Reset Recovery didn't reset endpoints")); + } + return Err(ProtocolError::ProtocolError("CSW invalid, but a recover was successful")); } /*if self.bulk_in.status()? == EndpointStatus::Halted @@ -262,7 +289,7 @@ impl<'a> Protocol for BulkOnlyTransport<'a> { "bulk-only transport phase error, or other", )); }, - residue: NonZeroU32::new(csw.data_residue), + residue, }) } } diff --git a/usbscsid/src/scsi/mod.rs b/usbscsid/src/scsi/mod.rs index 5d4bd2d798..0bbf289d2a 100644 --- a/usbscsid/src/scsi/mod.rs +++ b/usbscsid/src/scsi/mod.rs @@ -25,7 +25,7 @@ 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; +type Result = std::result::Result; #[derive(Debug, Error)] pub enum ScsiError { @@ -57,7 +57,7 @@ impl Scsi { // Get the Standard Inquiry Data. this.get_standard_inquiry_data(protocol, max_inquiry_len)?; - let version = this.res_standard_inquiry_data().version(); + let version = dbg!(this.res_standard_inquiry_data()).version(); println!("Inquiry version: {}", version); let (block_size, block_count) = { @@ -98,6 +98,7 @@ impl Scsi { &self.command_buffer[..REQUEST_SENSE_CMD_LEN as usize], DeviceReqData::In(&mut self.data_buffer[..alloc_len as usize]), )?; + Ok(()) } pub fn get_mode_sense10( &mut self, @@ -107,10 +108,9 @@ impl Scsi { &cmds::ModeParamHeader10, BlkDescSlice, impl Iterator, - ), - ScsiError, + ) > { - let initial_alloc_len = 4; // covers both mode_data_len and blk_desc_len. + let initial_alloc_len = mem::size_of::() as u16; // covers both mode_data_len and blk_desc_len. let mode_sense10 = self.cmd_mode_sense10(); *mode_sense10 = cmds::ModeSense10::get_block_desc(initial_alloc_len, 0); self.data_buffer @@ -122,13 +122,13 @@ impl Scsi { &self.command_buffer[..10], DeviceReqData::In(&mut self.data_buffer[..initial_alloc_len as usize]), )? { - self.get_ff_sense(protocol, 252); + self.get_ff_sense(protocol, 252)?; panic!("{:?}", self.res_ff_sense_data()); } - let optimal_alloc_len = self.res_mode_param_header10().block_desc_len() - + self.res_mode_param_header10().mode_data_len() - + mem::size_of::() as u16; + let optimal_alloc_len = + self.res_mode_param_header10().mode_data_len() + + 2; // the length of the mode data field itself let mode_sense10 = self.cmd_mode_sense10(); *mode_sense10 = cmds::ModeSense10::get_block_desc(optimal_alloc_len, 0); @@ -229,7 +229,7 @@ impl Scsi { protocol: &mut dyn Protocol, lba: u64, buffer: &mut [u8], - ) -> Result { + ) -> Result { let blocks_to_read = buffer.len() as u64 / u64::from(self.block_size); let bytes_to_read = blocks_to_read as usize * self.block_size as usize; let transfer_len = u32::try_from(blocks_to_read).or(Err(ScsiError::Overflow( @@ -252,7 +252,7 @@ impl Scsi { protocol: &mut dyn Protocol, lba: u64, buffer: &[u8], - ) -> Result { + ) -> Result { let blocks_to_write = buffer.len() as u64 / u64::from(self.block_size); let bytes_to_write = blocks_to_write as usize * self.block_size as usize; let transfer_len = u32::try_from(blocks_to_write).or(Err(ScsiError::Overflow( diff --git a/xhcid/config.toml b/xhcid/config.toml.unused similarity index 100% rename from xhcid/config.toml rename to xhcid/config.toml.unused diff --git a/xhcid/src/driver_interface.rs b/xhcid/src/driver_interface.rs index 8d448edc84..1d0968f402 100644 --- a/xhcid/src/driver_interface.rs +++ b/xhcid/src/driver_interface.rs @@ -329,13 +329,24 @@ pub enum EndpointStatus { Error, } +#[derive(Clone, Copy, Debug, Default, Eq, Hash, PartialEq, Serialize, Deserialize)] +pub struct PortTransferStatus { + pub kind: PortTransferStatusKind, + pub bytes_transferred: u32, +} + #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Serialize, Deserialize)] -pub enum PortTransferStatus { +pub enum PortTransferStatusKind { Success, - ShortPacket(u16), + ShortPacket, Stalled, Unknown, } +impl Default for PortTransferStatusKind { + fn default() -> Self { + Self::Success + } +} impl EndpointStatus { pub fn as_str(&self) -> &'static str { @@ -641,7 +652,7 @@ impl XhciEndpHandle { let res = self.ctl_res()?; match res { - XhciEndpCtlRes::TransferResult(PortTransferStatus::Success) + XhciEndpCtlRes::TransferResult(PortTransferStatus { kind: PortTransferStatusKind::Success, .. }) if bytes_read != expected_len as usize => { Err(Invalid("no short packet, but fewer bytes were read/written").into()) diff --git a/xhcid/src/xhci/scheme.rs b/xhcid/src/xhci/scheme.rs index 72dcd19638..02a604429f 100644 --- a/xhcid/src/xhci/scheme.rs +++ b/xhcid/src/xhci/scheme.rs @@ -1849,14 +1849,18 @@ impl Xhci { Ok(buf.len()) } fn transfer_result(completion_code: u8, bytes_transferred: u32) -> PortTransferStatus { - if completion_code == TrbCompletionCode::Success as u8 { - PortTransferStatus::Success + let kind = if completion_code == TrbCompletionCode::Success as u8 { + PortTransferStatusKind::Success } else if completion_code == TrbCompletionCode::ShortPacket as u8 { - PortTransferStatus::ShortPacket(bytes_transferred as u16) + PortTransferStatusKind::ShortPacket } else if completion_code == TrbCompletionCode::Stall as u8 { - PortTransferStatus::Stalled + PortTransferStatusKind::Stalled } else { - PortTransferStatus::Unknown + PortTransferStatusKind::Unknown + }; + PortTransferStatus { + kind, + bytes_transferred, } } pub fn on_write_endp_data( @@ -1891,8 +1895,7 @@ impl Xhci { ref mut bytes_transferred, } = ep_if_state { - if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code == TrbCompletionCode::ShortPacket as u8 { - // TODO: Add an error flag to WaitingForTransferResult. + if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code != TrbCompletionCode::Success as u8 { *ep_if_state = EndpIfState::WaitingForTransferResult(result); } else { *bytes_transferred += some_bytes_transferred; @@ -1984,7 +1987,7 @@ impl Xhci { ref mut bytes_transferred, } = ep_if_state { - if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code == TrbCompletionCode::ShortPacket as u8 { + if *bytes_transferred + some_bytes_transferred == bytes_to_transfer || completion_code != TrbCompletionCode::Success as u8 { *ep_if_state = EndpIfState::WaitingForTransferResult(result); } else { *bytes_transferred += some_bytes_transferred;