Skip to content

Conversation

@pamaury
Copy link
Contributor

@pamaury pamaury commented Nov 4, 2025

This virtual host talks to the QEMU usbdev driver. At the moment, this host perform the initial handshake, turns on vbus, waits for a device connection and retrieve the device descriptor.

This host is not super useful in its current state and will be extended in future PRs but it is already quite a large piece of work and it's good state to stop.

@pamaury pamaury requested a review from a team as a code owner November 4, 2025 13:45
@pamaury pamaury requested review from AlexJones0, jwnrt and nbdd0121 and removed request for a team November 4, 2025 13:45
@pamaury pamaury force-pushed the qemu_ott_usb_backend branch from 72627ae to ab4ed95 Compare November 4, 2025 14:04
This virtual host talks to the QEMU usbdev driver. At the moment,
this host perform the initial handshake, turns on vbus, waits for
a device connection and retrieve the device descriptor.

Signed-off-by: Amaury Pouly <[email protected]>
@pamaury pamaury force-pushed the qemu_ott_usb_backend branch from ab4ed95 to 5fcbb86 Compare November 4, 2025 15:26
}

/// This structure records a complete event received from QEMU on the chardev.
/// The complete payload is parsed and the received data in stored as-is.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// The complete payload is parsed and the received data in stored as-is.
/// The complete payload is parsed and the received data is stored as-is.

#[allow(clippy::too_many_arguments)]
fn send_control_in(
&mut self,
addr: u8,
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: it's not very important, but I wonder if it would be possible to refactor to reduce the number of arguments being passed in here and to a couple of other functions. It's quite hard to follow from reading it.

Ok(QemuUsbdevEvent { cmd, id, data }) => {
ensure!(
cmd == QemuUsbdevCmd::Hello,
"Expected an HELLO event, got {cmd:?}"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Expected an HELLO event, got {cmd:?}"
"Expected a HELLO event, got {cmd:?}"

);
ensure!(
payload.magic == Self::USBDEV_HELLO_MAGIC,
"HELLO payload has the wrong magic bytes"
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe dump the incorrect magic bytes here to help debugging?

fn wait_connect(&mut self) -> anyhow::Result<WaitResult<u32>> {
Ok(match self.wait_qemu_event()? {
Ok(QemuUsbdevEvent { cmd, id, data }) => {
// If device was previously disconnect, the only valid event that the device
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// If device was previously disconnect, the only valid event that the device
// If the device was previously disconnected, the only valid event that the device


/// Perform a full enumeration sequence, retrieving the
/// device and config descriptors, as well as assigning
/// an address to the device. If the device fails to enumetate
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// an address to the device. If the device fails to enumetate
/// an address to the device. If the device fails to enumerate

Comment on lines +611 to +612
// HACK Wait for a while. This should really be done in QEMU to emulate the reset.
std::thread::sleep(std::time::Duration::from_millis(100));
Copy link
Contributor

Choose a reason for hiding this comment

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

Question on this hack (maybe it could be added to the comment) - why do we have to add this delay? Is this to give QEMU time to reset, or is it expected by the host, or is there some other reason?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

At the moment resets are handled as zero-time events on the QEMU side. This is problematic because if you send a reset followed by a SETUP then you could end up with the SW having both reset and rx interrupts set. The SW has no way to know in which order they arrived and has to assume that the reset happened afterwards (this is because a bus reset takes several milliseconds on a real bus) which messes up with the transfer.

The real solution is to emulate the reset properly in QEMU (which will be done eventually) but this hack avoids the issue above.

const USB_DEVICE_DESCRIPTOR_LENGTH: usize = 18;

// Location of the bMaxPacketSize field in the device descriptor.
const USV_DEV_DESC_MAX_PACKET_SIZE_OFFSET: usize = 7;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const USV_DEV_DESC_MAX_PACKET_SIZE_OFFSET: usize = 7;
const USB_DEV_DESC_MAX_PACKET_SIZE_OFFSET: usize = 7;

}

/// This thread simulates a USB host. It can react to events sent by otlib
/// other the channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// other the channel.
/// over the channel.

?

Comment on lines +679 to +680
// We spawn a thread just to listen to events send by QEMU USBDEV.
// Those will be sent to the same channel used otlib to gives us
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
// We spawn a thread just to listen to events send by QEMU USBDEV.
// Those will be sent to the same channel used otlib to gives us
// We spawn a thread just to listen to events sent by QEMU USBDEV.
// Those will be sent to the same channel otlib uses to gives us

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants