From 3c45f5c7ab6e2c15e22dcaef05dbee0fc81edc49 Mon Sep 17 00:00:00 2001 From: Christian Helmuth Date: Thu, 23 Mar 2023 16:45:12 +0100 Subject: [PATCH] usb: support 32 in-flight packets - move metadata specific to isochronous transfers from the descriptor into the content of USB-session packets - restore support for 32 in-flight packets in the USB C API Fixes #4749 --- repos/dde_linux/src/drivers/usb_host/raw.cc | 48 ++++++----- repos/dde_linux/src/lib/lx_emul/usb.c | 23 ++--- .../libports/src/lib/libusb/genode_usb_raw.cc | 28 ++++--- repos/libports/src/lib/qemu-usb/host.cc | 84 +++++++++++-------- repos/os/include/genode_c_api/usb.h | 15 ++-- repos/os/include/usb_session/usb_session.h | 25 +++++- repos/os/src/lib/genode_c_api/usb.cc | 12 +-- 7 files changed, 142 insertions(+), 93 deletions(-) diff --git a/repos/dde_linux/src/drivers/usb_host/raw.cc b/repos/dde_linux/src/drivers/usb_host/raw.cc index dfc27c6ee3..e87ec92728 100644 --- a/repos/dde_linux/src/drivers/usb_host/raw.cc +++ b/repos/dde_linux/src/drivers/usb_host/raw.cc @@ -246,22 +246,26 @@ class Device : public List::Element p.transfer.actual_size = urb->actual_length; p.succeded = true; - if (read) { - /* make sure the client sees the actual amount of data */ - for (int i = 0; i < urb->number_of_packets; i++) { - p.transfer.actual_packet_size[i] = urb->iso_frame_desc[i].actual_length; + /* + * We have to copy the whole transfer buffer. In case of + * isochronous transfers, the controller used offsets into the + * original buffer to store the data of multiple packets. + */ + if (read) _with_packet_stream([&](Tx::Sink &sink) { + void * payload = sink.packet_content(p); + if (usb_pipeisoc(urb->pipe)) { + /* make sure the client sees the actual amount of data */ + Usb::Isoc_transfer &isoc = *(Usb::Isoc_transfer *)payload; + for (int i = 0; i < urb->number_of_packets; i++) + isoc.actual_packet_size[i] = urb->iso_frame_desc[i].actual_length; + + /* actual data begins after metdata */ + payload = isoc.data(); } - /* - * We have to copy the whole transfer buffer because the - * controller used the offsets into the original buffer to - * store the data. - */ - _with_packet_stream([&](Tx::Sink &sink) { - Genode::memcpy(sink.packet_content(p), urb->transfer_buffer, + Genode::memcpy(payload, urb->transfer_buffer, urb->transfer_buffer_length); - }); - } + }); } else if (urb->status == -ESHUTDOWN) { p.error = Packet_descriptor::NO_DEVICE_ERROR; } else if ((urb->status == -EPROTO) || (urb->status == -EILSEQ)) { @@ -426,12 +430,16 @@ class Device : public List::Element usb_host_endpoint *ep = _host_ep(p.transfer.ep); void *buf = dma_malloc(p.size()); + Usb::Isoc_transfer &isoc = *(Usb::Isoc_transfer *)_sink->packet_content(p); + void * const payload = isoc.data(); + unsigned const payload_size = p.size() - sizeof(isoc); + if (read) { pipe = usb_rcvisocpipe(&_udev, p.transfer.ep); } else { pipe = usb_sndisocpipe(&_udev, p.transfer.ep); - Genode::memcpy(buf, _sink->packet_content(p), p.size()); + Genode::memcpy(buf, payload, payload_size); } if (!ep) { @@ -441,7 +449,7 @@ class Device : public List::Element return false; } - urb *urb = usb_alloc_urb(p.transfer.number_of_packets, GFP_KERNEL); + urb *urb = usb_alloc_urb(isoc.number_of_packets, GFP_KERNEL); if (!urb) { error("Failed to allocate isochronous URB"); dma_free(buf); @@ -455,18 +463,18 @@ class Device : public List::Element urb->start_frame = -1; urb->stream_id = 0; urb->transfer_buffer = buf; - urb->transfer_buffer_length = p.size(); - urb->number_of_packets = p.transfer.number_of_packets; + urb->transfer_buffer_length = payload_size; + urb->number_of_packets = isoc.number_of_packets; urb->interval = 1 << min(15, ep->desc.bInterval - 1); urb->context = (void *)data; urb->transfer_flags = URB_ISO_ASAP | (read ? URB_DIR_IN : URB_DIR_OUT); urb->complete = _async_complete; unsigned offset = 0; - for (int i = 0; i < p.transfer.number_of_packets; i++) { + for (unsigned i = 0; i < isoc.number_of_packets; i++) { urb->iso_frame_desc[i].offset = offset; - urb->iso_frame_desc[i].length = p.transfer.packet_size[i]; - offset += p.transfer.packet_size[i]; + urb->iso_frame_desc[i].length = isoc.packet_size[i]; + offset += isoc.packet_size[i]; } int ret = usb_submit_urb(urb, GFP_KERNEL); diff --git a/repos/dde_linux/src/lib/lx_emul/usb.c b/repos/dde_linux/src/lib/lx_emul/usb.c index 98e7c402a4..12fb82e818 100644 --- a/repos/dde_linux/src/lib/lx_emul/usb.c +++ b/repos/dde_linux/src/lib/lx_emul/usb.c @@ -542,8 +542,10 @@ handle_transfer_response(struct genode_usb_request_urb req, transfer->actual_size = urb->actual_length; if (usb_pipeisoc(urb->pipe) && usb_pipein(urb->pipe)) { - for (i = 0; i < urb->number_of_packets; i++) - transfer->actual_packet_size[i] = + struct genode_usb_isoc_transfer *isoc = + (struct genode_usb_isoc_transfer *)payload.addr; + for (i = 0; i < isoc->number_of_packets; i++) + isoc->actual_packet_size[i] = urb->iso_frame_desc[i].actual_length; } } @@ -700,13 +702,14 @@ static int fill_isoc_urb(struct usb_device * udev, req->ep & USB_DIR_IN ? udev->ep_in[req->ep & 0xf] : udev->ep_out[req->ep & 0xf]; + struct genode_usb_isoc_transfer *isoc = (struct genode_usb_isoc_transfer *)buf.addr; - if (!buf.addr) + if (!buf.addr || isoc->number_of_packets > MAX_PACKETS) return PACKET_INVALID_ERROR; if (!ep) return -ENOENT; - *urb = usb_alloc_urb(req->number_of_packets, GFP_KERNEL); + *urb = usb_alloc_urb(isoc->number_of_packets, GFP_KERNEL); if (!*urb) return -ENOMEM; @@ -714,18 +717,18 @@ static int fill_isoc_urb(struct usb_device * udev, (*urb)->pipe = pipe; (*urb)->start_frame = -1; (*urb)->stream_id = 0; - (*urb)->transfer_buffer = buf.addr; - (*urb)->transfer_buffer_length = buf.size; - (*urb)->number_of_packets = req->number_of_packets; + (*urb)->transfer_buffer = isoc->data; + (*urb)->transfer_buffer_length = buf.size - sizeof(*isoc); + (*urb)->number_of_packets = isoc->number_of_packets; (*urb)->interval = 1 << min(15, ep->desc.bInterval - 1); (*urb)->context = handle; (*urb)->transfer_flags = URB_ISO_ASAP | (read ? URB_DIR_IN : URB_DIR_OUT); (*urb)->complete = async_complete; - for (i = 0; i < req->number_of_packets; i++) { + for (i = 0; i < isoc->number_of_packets; i++) { (*urb)->iso_frame_desc[i].offset = offset; - (*urb)->iso_frame_desc[i].length = req->packet_size[i]; - offset += req->packet_size[i]; + (*urb)->iso_frame_desc[i].length = isoc->packet_size[i]; + offset += isoc->packet_size[i]; } return 0; diff --git a/repos/libports/src/lib/libusb/genode_usb_raw.cc b/repos/libports/src/lib/libusb/genode_usb_raw.cc index e7eda6c4ea..9a4a361d34 100644 --- a/repos/libports/src/lib/libusb/genode_usb_raw.cc +++ b/repos/libports/src/lib/libusb/genode_usb_raw.cc @@ -227,16 +227,18 @@ struct Usb_device if (IS_XFERIN(transfer)) { + Usb::Isoc_transfer &isoc = *(Usb::Isoc_transfer *)packet_content; + unsigned out_offset = 0; - for (int i = 0; i < p.transfer.number_of_packets; i++) { - size_t const actual_length = p.transfer.actual_packet_size[i]; + for (unsigned i = 0; i < isoc.number_of_packets; i++) { + size_t const actual_length = isoc.actual_packet_size[i]; /* * Copy the data from the proper offsets within the buffer as * a short read is still stored at this location. */ unsigned char * dst = transfer->buffer + out_offset; - char const * src = packet_content + out_offset; + char const * src = isoc.data() + out_offset; Genode::memcpy(dst, src, actual_length); out_offset += transfer->iso_packet_desc[i].length; @@ -244,7 +246,7 @@ struct Usb_device transfer->iso_packet_desc[i].actual_length = actual_length; transfer->iso_packet_desc[i].status = LIBUSB_TRANSFER_COMPLETED; } - transfer->num_iso_packets = p.transfer.number_of_packets; + transfer->num_iso_packets = isoc.number_of_packets; } @@ -611,8 +613,9 @@ static int genode_submit_transfer(struct usbi_transfer * itransfer) } Usb::Packet_descriptor p; + size_t p_size = Usb::Isoc_transfer::size(total_length); try { - p = usb_device->usb_connection->alloc_packet(total_length); + p = usb_device->usb_connection->alloc_packet(p_size); } catch (Usb::Session::Tx::Source::Packet_alloc_failed) { return LIBUSB_ERROR_BUSY; } @@ -624,17 +627,16 @@ static int genode_submit_transfer(struct usbi_transfer * itransfer) p.completion = new (libc_alloc) Completion(itransfer); p.transfer.ep = transfer->endpoint; + Usb::Isoc_transfer &isoc = + *(Usb::Isoc_transfer *) usb_device->usb_connection->source()->packet_content(p); + isoc.number_of_packets = transfer->num_iso_packets; for (int i = 0; i < transfer->num_iso_packets; i++) { - p.transfer.packet_size[i] = transfer->iso_packet_desc[i].length; + isoc.packet_size[i] = transfer->iso_packet_desc[i].length; + isoc.actual_packet_size[i] = 0; } - p.transfer.number_of_packets = transfer->num_iso_packets; - if (IS_XFEROUT(transfer)) { - char *packet_content = - usb_device->usb_connection->source()->packet_content(p); - Genode::memcpy(packet_content, transfer->buffer, - transfer->length); - } + if (IS_XFEROUT(transfer)) + Genode::memcpy(isoc.data(), transfer->buffer, transfer->length); usb_device->usb_connection->source()->submit_packet(p); diff --git a/repos/libports/src/lib/qemu-usb/host.cc b/repos/libports/src/lib/qemu-usb/host.cc index 023d9f1dc8..4d8a8fed02 100644 --- a/repos/libports/src/lib/qemu-usb/host.cc +++ b/repos/libports/src/lib/qemu-usb/host.cc @@ -55,11 +55,13 @@ class Isoc_packet : Fifo::Element int _packet_index { 0 }; unsigned _packet_in_offset { 0 }; + Usb::Isoc_transfer *_isoc { (Usb::Isoc_transfer *)_content }; public: Isoc_packet(Usb::Packet_descriptor packet, char *content) - : _packet(packet), _content(content), + : + _packet(packet), _content(content), _size (_packet.read_transfer() ? _packet.transfer.actual_size : _packet.size()) { } @@ -74,13 +76,13 @@ class Isoc_packet : Fifo::Element unsigned remaining = _size - _offset; int copy_size = min(usb_packet->iov.size, remaining); - usb_packet_copy(usb_packet, _content + _offset, copy_size); + usb_packet_copy(usb_packet, _isoc->data() + _offset, copy_size); _offset += copy_size; if (!_packet.read_transfer()) { - _packet.transfer.packet_size[_packet.transfer.number_of_packets] = copy_size; - _packet.transfer.number_of_packets++; + _isoc->packet_size[_isoc->number_of_packets] = copy_size; + _isoc->number_of_packets++; } return remaining <= usb_packet->iov.size; @@ -90,24 +92,30 @@ class Isoc_packet : Fifo::Element { if (!valid()) return false; - unsigned remaining = _packet.transfer.actual_packet_size[_packet_index]; + unsigned remaining = _isoc->actual_packet_size[_packet_index]; /* this should not happen as there asserts in the qemu code */ if (remaining > usb_packet->iov.size) { Genode::error("iov too small, ignoring packet content"); } int copy_size = min(usb_packet->iov.size, remaining); - char *src = _content + _packet_in_offset; + char *src = _isoc->data() + _packet_in_offset; usb_packet_copy(usb_packet, src, copy_size); - _packet_in_offset += _packet.transfer.packet_size[_packet_index]; + _packet_in_offset += _isoc->packet_size[_packet_index]; ++_packet_index; - return _packet_index == _packet.transfer.number_of_packets; + return _packet_index == _isoc->number_of_packets; } - bool valid() const { return _content != nullptr; } - unsigned packet_count() const { return _packet.transfer.number_of_packets; } + bool valid() const { return _content != nullptr; } + + unsigned packet_count() const + { + if (!valid()) return 0; + + return _isoc->number_of_packets; + } Usb::Packet_descriptor& packet() { return _packet; } }; @@ -253,7 +261,7 @@ struct Usb_host_device : List::Element Completion completion[NUM_COMPLETIONS]; Fifo isoc_read_queue { }; - Reconstructible isoc_write_packet { Usb::Packet_descriptor(), nullptr }; + Reconstructible isoc_write_packet { }; Genode::Ring_buffer _isoch_out_queue { }; unsigned _isoch_out_pending { 0 }; @@ -376,9 +384,8 @@ struct Usb_host_device : List::Element bool isoc_read(USBPacket *packet) { - if (isoc_read_queue.empty()) { + if (isoc_read_queue.empty()) return false; - } isoc_read_queue.head([&] (Isoc_packet &head) { if (head.copy_read(packet)) { @@ -406,16 +413,19 @@ struct Usb_host_device : List::Element return; } - size_t size = usb_packet->ep->max_packet_size * NUMBER_OF_PACKETS; + size_t const payload_size = usb_packet->ep->max_packet_size * NUMBER_OF_PACKETS; + size_t const packet_size = Usb::Isoc_transfer::size(payload_size); try { - Usb::Packet_descriptor packet = alloc_packet(size); + Usb::Packet_descriptor packet = alloc_packet(packet_size, true); packet.type = Packet_type::ISOC; packet.transfer.ep = usb_packet->ep->nr | USB_DIR_IN; packet.transfer.polling_interval = Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL; - packet.transfer.number_of_packets = NUMBER_OF_PACKETS; - for (unsigned i = 0; i < NUMBER_OF_PACKETS; i++) { - packet.transfer.packet_size[i] = usb_packet->ep->max_packet_size; - } + + Usb::Isoc_transfer &isoc = + *(Usb::Isoc_transfer *)usb_raw.source()->packet_content(packet); + isoc.number_of_packets = NUMBER_OF_PACKETS; + for (unsigned i = 0; i < NUMBER_OF_PACKETS; i++) + isoc.packet_size[i] = usb_packet->ep->max_packet_size; Completion *c = dynamic_cast(packet.completion); c->p = nullptr; @@ -427,7 +437,7 @@ struct Usb_host_device : List::Element submit(packet); } catch (Packet_alloc_failed) { if (verbose_warnings) - warning("xHCI: packet allocation failed (size ", Hex(size), "in ", __func__, ")"); + warning("xHCI: packet allocation failed (size ", Hex(packet_size), "in ", __func__, ")"); } } @@ -451,37 +461,41 @@ struct Usb_host_device : List::Element { enum { NUMBER_OF_PACKETS = 32 }; - bool valid = isoc_write_packet->valid(); + bool const valid = isoc_write_packet->valid(); if (valid) { isoc_write_packet->copy(usb_packet); if (isoc_write_packet->packet_count() < NUMBER_OF_PACKETS) return; - _isoch_out_queue.add(*&*isoc_write_packet); + _isoch_out_queue.add(*isoc_write_packet); } - size_t size = usb_packet->ep->max_packet_size * NUMBER_OF_PACKETS; + size_t const payload_size = usb_packet->ep->max_packet_size * NUMBER_OF_PACKETS; + size_t const packet_size = Usb::Isoc_transfer::size(payload_size); try { - Usb::Packet_descriptor packet = alloc_packet(size, false); + Usb::Packet_descriptor packet = alloc_packet(packet_size, false); packet.type = Packet_type::ISOC; packet.transfer.ep = usb_packet->ep->nr; packet.transfer.polling_interval = Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL; - packet.transfer.number_of_packets = 0; - isoc_write_packet.construct(packet, usb_raw.source()->packet_content(packet)); + char *packet_content = usb_raw.source()->packet_content(packet); + + Usb::Isoc_transfer &isoc = *(Usb::Isoc_transfer *)packet_content; + isoc.number_of_packets = 0; + + isoc_write_packet.construct(packet, packet_content); if (!valid) isoc_write_packet->copy(usb_packet); } catch (Packet_alloc_failed) { if (verbose_warnings) - warning("xHCI: packet allocation failed (size ", Hex(size), "in ", __func__, ")"); - isoc_write_packet.construct(Usb::Packet_descriptor(), nullptr); + warning("xHCI: packet allocation failed (size ", Hex(packet_size), "in ", __func__, ")"); + isoc_write_packet.construct(); return; } - if (_isoch_out_pending == 0 && _isoch_out_queue.avail_capacity() > 1) { + if (_isoch_out_pending == 0 && _isoch_out_queue.avail_capacity() > 1) return; - } while (!_isoch_out_queue.empty()) { Isoc_packet i = _isoch_out_queue.get(); @@ -528,7 +542,7 @@ struct Usb_host_device : List::Element ** Packet stream ** *******************/ - Usb::Packet_descriptor alloc_packet(int length, bool completion = true) + Usb::Packet_descriptor alloc_packet(int length, bool completion) { if (!usb_raw.source()->ready_to_submit()) { throw Packet_alloc_failed(); @@ -621,7 +635,7 @@ struct Usb_host_device : List::Element _release_interfaces(); try { - Usb::Packet_descriptor packet = alloc_packet(0); + Usb::Packet_descriptor packet = alloc_packet(0, true); packet.type = Usb::Packet_descriptor::CONFIG; packet.number = value; @@ -641,7 +655,7 @@ struct Usb_host_device : List::Element void set_interface(int index, uint8_t value, USBPacket *p) { try { - Usb::Packet_descriptor packet = alloc_packet(0); + Usb::Packet_descriptor packet = alloc_packet(0, true); packet.type = Usb::Packet_descriptor::ALT_SETTING; packet.interface.number = index; packet.interface.alt_setting = value; @@ -828,7 +842,7 @@ static void usb_host_handle_data(USBDevice *udev, USBPacket *p) } try { - Usb::Packet_descriptor packet = dev->alloc_packet(size); + Usb::Packet_descriptor packet = dev->alloc_packet(size, true); packet.type = type; packet.transfer.ep = p->ep->nr | (in ? USB_DIR_IN : 0); packet.transfer.polling_interval = Usb::Packet_descriptor::DEFAULT_POLLING_INTERVAL; @@ -883,7 +897,7 @@ static void usb_host_handle_control(USBDevice *udev, USBPacket *p, Usb::Packet_descriptor packet; try { - packet = dev->alloc_packet(length); + packet = dev->alloc_packet(length, true); } catch (...) { if (verbose_warnings) warning("Packet allocation failed"); diff --git a/repos/os/include/genode_c_api/usb.h b/repos/os/include/genode_c_api/usb.h index 8c46545a47..ef1a611dbe 100644 --- a/repos/os/include/genode_c_api/usb.h +++ b/repos/os/include/genode_c_api/usb.h @@ -147,16 +147,21 @@ struct genode_usb_request_control int timeout; }; -enum Iso { MAX_PACKETS = 32 }; - struct genode_usb_request_transfer { unsigned char ep; int actual_size; int polling_interval; - int number_of_packets; - unsigned long packet_size[MAX_PACKETS]; - unsigned long actual_packet_size[MAX_PACKETS]; +}; + +enum Isoc { MAX_PACKETS = 32 }; + +struct genode_usb_isoc_transfer +{ + unsigned number_of_packets; + unsigned packet_size[MAX_PACKETS]; + unsigned actual_packet_size[MAX_PACKETS]; + char data[]; }; enum Urb_type { CTRL, BULK, IRQ, ISOC, NONE }; diff --git a/repos/os/include/usb_session/usb_session.h b/repos/os/include/usb_session/usb_session.h index 48ea6f8d94..4a5ffebf75 100644 --- a/repos/os/include/usb_session/usb_session.h +++ b/repos/os/include/usb_session/usb_session.h @@ -24,6 +24,7 @@ namespace Usb { using namespace Genode; class Session; struct Packet_descriptor; + struct Isoc_transfer; struct Completion; } @@ -34,7 +35,6 @@ namespace Usb { struct Usb::Packet_descriptor : Genode::Packet_descriptor { enum Type { STRING, CTRL, BULK, IRQ, ISOC, ALT_SETTING, CONFIG, RELEASE_IF, FLUSH_TRANSFERS }; - enum Iso { MAX_PACKETS = 32 }; /* use the polling interval stated in the endpoint descriptor */ enum { DEFAULT_POLLING_INTERVAL = -1 }; @@ -66,9 +66,6 @@ struct Usb::Packet_descriptor : Genode::Packet_descriptor uint8_t ep; int actual_size; /* returned */ int polling_interval; /* for interrupt transfers */ - int number_of_packets; - size_t packet_size[MAX_PACKETS]; - size_t actual_packet_size[MAX_PACKETS]; } transfer; struct @@ -110,6 +107,26 @@ struct Usb::Packet_descriptor : Genode::Packet_descriptor }; +/** + * Isochronous transfer metadata (located at start of stream packet) + */ +struct Usb::Isoc_transfer +{ + enum { MAX_PACKETS = 32 }; + + unsigned number_of_packets; + unsigned packet_size[MAX_PACKETS]; + unsigned actual_packet_size[MAX_PACKETS]; + + char *data() { return (char *)(this + 1); } + + static size_t size(unsigned data_size) + { + return sizeof(Isoc_transfer) + data_size; + } +}; + + /** * Completion for asynchronous communication */ diff --git a/repos/os/src/lib/genode_c_api/usb.cc b/repos/os/src/lib/genode_c_api/usb.cc index 04b396b5d1..c5510ca64a 100644 --- a/repos/os/src/lib/genode_c_api/usb.cc +++ b/repos/os/src/lib/genode_c_api/usb.cc @@ -73,9 +73,9 @@ class genode_usb_session : public Usb::Session_rpc_object friend class ::Root; - enum { MAX_PACKETS_IN_FLY = 10 }; + enum { MAX_PACKETS_IN_FLIGHT = 32 }; - Constructible packets[MAX_PACKETS_IN_FLY] { }; + Constructible packets[MAX_PACKETS_IN_FLIGHT] { }; ::Root & _root; genode_shared_dataspace * _ds; @@ -84,7 +84,7 @@ class genode_usb_session : public Usb::Session_rpc_object Session_label const _label; List_element _le { this }; - unsigned _packets_in_fly(); + unsigned _packets_in_flight(); void _ack(int err, Usb::Packet_descriptor p); genode_usb_buffer _buffer_of_packet(Usb::Packet_descriptor p); @@ -414,7 +414,7 @@ void genode_usb_session::release_interface(unsigned iface) } -unsigned genode_usb_session::_packets_in_fly() +unsigned genode_usb_session::_packets_in_flight() { unsigned ret = 0; for (auto const &p : packets) @@ -463,7 +463,7 @@ bool genode_usb_session::request(genode_usb_request_callbacks & req, void * data break; ++idx; } - if (idx == MAX_PACKETS_IN_FLY) + if (idx == MAX_PACKETS_IN_FLIGHT) return false; Packet_descriptor p; @@ -471,7 +471,7 @@ bool genode_usb_session::request(genode_usb_request_callbacks & req, void * data /* get next packet from request stream */ while (true) { if (!sink()->packet_avail() || - (sink()->ack_slots_free() <= _packets_in_fly())) + (sink()->ack_slots_free() <= _packets_in_flight())) return false; p = sink()->get_packet();