usb: separate trb data from XhciTransferType

The old XhciTransferType structure copied the memory and trb data
unnecessarily when we just wanted to match on the transfer type.
Depending on the transfer type, we need to allocate a scatter/gather
buffer structure or similar, but we don't need to do that every time we
match on the type itself.

This patch decouples the type and the contents, and after some tests it
shows a slight improvement in performance (2~3%) too.

BUG=b:294331964
TEST=`tools/dev_container tools/presubmit`
TEST=deployed code to DUT and verified USB functionality

Change-Id: I4f10021c76dad8942d1c2ce166e355836a42b194
Reviewed-on: https://chromium-review.googlesource.com/c/crosvm/crosvm/+/4739522
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Commit-Queue: Keiichi Watanabe <keiichiw@chromium.org>
Auto-Submit: Morg <morg@google.com>
This commit is contained in:
Federico 'Morg' Pareschi 2023-08-08 05:03:08 +00:00 committed by crosvm LUCI
parent f11e2d80c6
commit 18bd141f19
4 changed files with 56 additions and 48 deletions

View file

@ -26,12 +26,16 @@ pub enum Error {
BufferLen(BufferError),
#[error("failed to clear halt: {0}")]
ClearHalt(UsbUtilError),
#[error("failed to create contro tube: {0}")]
#[error("failed to create scatter gather buffer: {0}")]
CreateBuffer(XhciTransferError),
#[error("failed to create control tube: {0}")]
CreateControlTube(TubeError),
#[error("failed to create libusb context: {0}")]
CreateLibUsbContext(UsbUtilError),
#[error("failed to create transfer: {0}")]
CreateTransfer(UsbUtilError),
#[error("failed to create USB request setup: {0}")]
CreateUsbRequestSetup(XhciTransferError),
#[error("failed to free streams: {0}")]
FreeStreams(UsbUtilError),
#[error("failed to get active config: {0}")]

View file

@ -295,7 +295,10 @@ impl HostDevice {
.get_transfer_type()
.map_err(Error::GetXhciTransferType)?;
match transfer_type {
XhciTransferType::SetupStage(setup) => {
XhciTransferType::SetupStage => {
let setup = xhci_transfer
.create_usb_request_setup()
.map_err(Error::CreateUsbRequestSetup)?;
if self.ctl_ep_state != ControlEndpointState::SetupStage {
error!("Control endpoint is in an inconsistant state");
return Ok(());
@ -307,13 +310,14 @@ impl HostDevice {
.map_err(Error::TransferComplete)?;
self.ctl_ep_state = ControlEndpointState::DataStage;
}
XhciTransferType::DataStage(buffer) => {
XhciTransferType::DataStage => {
if self.ctl_ep_state != ControlEndpointState::DataStage {
error!("Control endpoint is in an inconsistant state");
return Ok(());
}
// Requests with a DataStage will be executed here.
// Requests without a DataStage will be executed in StatusStage.
let buffer = xhci_transfer.create_buffer().map_err(Error::CreateBuffer)?;
self.execute_control_transfer(xhci_transfer, Some(buffer))?;
self.executed = true;
self.ctl_ep_state = ControlEndpointState::StatusStage;

View file

@ -76,7 +76,7 @@ impl UsbEndpoint {
.get_transfer_type()
.map_err(Error::GetXhciTransferType)?
{
XhciTransferType::Normal(buffer) => buffer,
XhciTransferType::Normal => transfer.create_buffer().map_err(Error::CreateBuffer)?,
XhciTransferType::Noop => {
return transfer
.on_transfer_complete(&TransferStatus::Completed, 0)

View file

@ -115,14 +115,14 @@ impl XhciTransferState {
pub enum XhciTransferType {
// Normal means bulk transfer or interrupt transfer, depending on endpoint type.
// See spec 4.11.2.1.
Normal(ScatterGatherBuffer),
Normal,
// See usb spec for setup stage, data stage and status stage,
// see xHCI spec 4.11.2.2 for corresponding trbs.
SetupStage(UsbRequestSetup),
DataStage(ScatterGatherBuffer),
SetupStage,
DataStage,
StatusStage,
// See xHCI spec 4.11.2.3.
Isochronous(ScatterGatherBuffer),
Isochronous,
// See xHCI spec 6.4.1.4.
Noop,
}
@ -132,51 +132,16 @@ impl Display for XhciTransferType {
use self::XhciTransferType::*;
match self {
Normal(_) => write!(f, "Normal"),
SetupStage(_) => write!(f, "SetupStage"),
DataStage(_) => write!(f, "DataStage"),
Normal => write!(f, "Normal"),
SetupStage => write!(f, "SetupStage"),
DataStage => write!(f, "DataStage"),
StatusStage => write!(f, "StatusStage"),
Isochronous(_) => write!(f, "Isochronous"),
Isochronous => write!(f, "Isochronous"),
Noop => write!(f, "Noop"),
}
}
}
impl XhciTransferType {
/// Analyze transfer descriptor and return transfer type.
pub fn new(mem: GuestMemory, td: TransferDescriptor) -> Result<XhciTransferType> {
// We can figure out transfer type from the first trb.
// See transfer descriptor description in xhci spec for more details.
match td[0].trb.get_trb_type().map_err(Error::TrbType)? {
TrbType::Normal => {
let buffer = ScatterGatherBuffer::new(mem, td).map_err(Error::CreateBuffer)?;
Ok(XhciTransferType::Normal(buffer))
}
TrbType::SetupStage => {
let trb = td[0].trb.cast::<SetupStageTrb>().map_err(Error::CastTrb)?;
Ok(XhciTransferType::SetupStage(UsbRequestSetup::new(
trb.get_request_type(),
trb.get_request(),
trb.get_value(),
trb.get_index(),
trb.get_length(),
)))
}
TrbType::DataStage => {
let buffer = ScatterGatherBuffer::new(mem, td).map_err(Error::CreateBuffer)?;
Ok(XhciTransferType::DataStage(buffer))
}
TrbType::StatusStage => Ok(XhciTransferType::StatusStage),
TrbType::Isoch => {
let buffer = ScatterGatherBuffer::new(mem, td).map_err(Error::CreateBuffer)?;
Ok(XhciTransferType::Isochronous(buffer))
}
TrbType::Noop => Ok(XhciTransferType::Noop),
t => Err(Error::BadTrbType(t)),
}
}
}
/// Xhci Transfer manager holds reference to all ongoing transfers. Can cancel them all if
/// needed.
#[derive(Clone)]
@ -310,7 +275,42 @@ impl XhciTransfer {
/// Get transfer type.
pub fn get_transfer_type(&self) -> Result<XhciTransferType> {
XhciTransferType::new(self.mem.clone(), self.transfer_trbs.clone())
// We can figure out transfer type from the first trb.
// See transfer descriptor description in xhci spec for more details.
match self.transfer_trbs[0]
.trb
.get_trb_type()
.map_err(Error::TrbType)?
{
TrbType::Normal => Ok(XhciTransferType::Normal),
TrbType::SetupStage => Ok(XhciTransferType::SetupStage),
TrbType::DataStage => Ok(XhciTransferType::DataStage),
TrbType::StatusStage => Ok(XhciTransferType::StatusStage),
TrbType::Isoch => Ok(XhciTransferType::Isochronous),
TrbType::Noop => Ok(XhciTransferType::Noop),
t => Err(Error::BadTrbType(t)),
}
}
/// Create a scatter gather buffer for the given xhci transfer
pub fn create_buffer(&self) -> Result<ScatterGatherBuffer> {
ScatterGatherBuffer::new(self.mem.clone(), self.transfer_trbs.clone())
.map_err(Error::CreateBuffer)
}
/// Create a usb request setup for the control transfer buffer
pub fn create_usb_request_setup(&self) -> Result<UsbRequestSetup> {
let trb = self.transfer_trbs[0]
.trb
.checked_cast::<SetupStageTrb>()
.map_err(Error::CastTrb)?;
Ok(UsbRequestSetup::new(
trb.get_request_type(),
trb.get_request(),
trb.get_value(),
trb.get_index(),
trb.get_length(),
))
}
/// Get endpoint number.