From 3d26ce9f8d772de1e43332628bdb16bf82e41f33 Mon Sep 17 00:00:00 2001 From: Stefan Kalkowski Date: Tue, 6 Sep 2022 17:36:23 +0200 Subject: [PATCH] usb_host: acknowledge all requests asynchronously Fix genodelabs/genode#4601 --- repos/dde_linux/src/lib/lx_emul/usb.c | 122 ++++++++++++++++---------- repos/os/include/genode_c_api/usb.h | 29 ++++-- repos/os/src/lib/genode_c_api/usb.cc | 33 ++++--- 3 files changed, 118 insertions(+), 66 deletions(-) diff --git a/repos/dde_linux/src/lib/lx_emul/usb.c b/repos/dde_linux/src/lib/lx_emul/usb.c index 8f2c777a33..76270835f9 100644 --- a/repos/dde_linux/src/lib/lx_emul/usb.c +++ b/repos/dde_linux/src/lib/lx_emul/usb.c @@ -333,10 +333,22 @@ struct genode_usb_rpc_callbacks lx_emul_usb_rpc_callbacks = { static genode_usb_request_ret_t +handle_return_code(struct genode_usb_request_urb req, void * data) +{ + return (genode_usb_request_ret_t)data; +}; + + +static void handle_string_request(struct genode_usb_request_string * req, - void * buf, unsigned long size, void * data) + genode_usb_session_handle_t session, + genode_usb_request_handle_t request, + void * buf, + unsigned long size, + void * data) { struct usb_device * udev = (struct usb_device *) data; + genode_usb_request_ret_t ret = UNKNOWN_ERROR; int length = usb_string(udev, req->index, buf, size); if (length < 0) { @@ -345,58 +357,73 @@ handle_string_request(struct genode_usb_request_string * req, } else { /* returned length is in bytes (char) */ req->length = length / 2; - return NO_ERROR; + ret = NO_ERROR; } - return UNKNOWN_ERROR; + genode_usb_ack_request(session, request, handle_return_code, (void*)ret); } -static genode_usb_request_ret_t -handle_altsetting_request(unsigned iface, unsigned alt_setting, void * data) +static void +handle_altsetting_request(unsigned iface, + unsigned alt_setting, + genode_usb_session_handle_t session, + genode_usb_request_handle_t request, + void * data) { struct usb_device * udev = (struct usb_device *) data; - int const ret = usb_set_interface(udev, iface, alt_setting); + genode_usb_request_ret_t ret = NO_ERROR; - if (ret < 0) - printk("Alt setting request (iface=%u alt_setting=%u) failed with %d\n", - iface, alt_setting, ret); - - return (ret == 0) ? NO_ERROR : UNKNOWN_ERROR; -} - - -static genode_usb_request_ret_t -handle_config_request(unsigned cfg_idx, void * data) -{ - struct usb_device * udev = (struct usb_device *) data; - - if (udev && udev->actconfig - && udev->actconfig->desc.bConfigurationValue == cfg_idx) { - /* - * Skip SET_CONFIGURATION requests if the device already has the - * selected config as active config. This workaround prevents issues - * with Linux guests in vbox and SDC-reader passthrough. - */ - return NO_ERROR; + if (usb_set_interface(udev, iface, alt_setting)) { + ret = UNKNOWN_ERROR; + printk("Alt setting request (iface=%u alt_setting=%u) failed\n", + iface, alt_setting); } - return (usb_set_configuration(udev, cfg_idx)) ? UNKNOWN_ERROR : NO_ERROR; + genode_usb_ack_request(session, request, handle_return_code, (void*)ret); } -static genode_usb_request_ret_t -handle_flush_request(unsigned char ep, void * data) +static void +handle_config_request(unsigned cfg_idx, + genode_usb_session_handle_t session, + genode_usb_request_handle_t request, + void * data) +{ + struct usb_device * udev = (struct usb_device *) data; + genode_usb_request_ret_t ret = NO_ERROR; + + /* + * Skip SET_CONFIGURATION requests if the device already has the + * selected config as active config. This workaround prevents issues + * with Linux guests in vbox and SDC-reader passthrough. + */ + if (!(udev && udev->actconfig && + udev->actconfig->desc.bConfigurationValue == cfg_idx)) + ret = (usb_set_configuration(udev, cfg_idx)) ? UNKNOWN_ERROR : NO_ERROR; + + genode_usb_ack_request(session, request, handle_return_code, (void*)ret); +} + + +static void +handle_flush_request(unsigned char ep, + genode_usb_session_handle_t session, + genode_usb_request_handle_t request, + void * data) { struct usb_device * udev = (struct usb_device *) data; struct usb_host_endpoint * endpoint = ep & USB_DIR_IN ? udev->ep_in[ep & 0xf] : udev->ep_out[ep & 0xf]; - if (!endpoint) - return INTERFACE_OR_ENDPOINT_ERROR; + genode_usb_request_ret_t ret = NO_ERROR; - usb_hcd_flush_endpoint(udev, endpoint); - return NO_ERROR; + if (!endpoint) + ret = INTERFACE_OR_ENDPOINT_ERROR; + else + usb_hcd_flush_endpoint(udev, endpoint); + + genode_usb_ack_request(session, request, handle_return_code, (void*)ret); } enum Timer_state { TIMER_OFF, TIMER_ACTIVE, TIMER_TRIGGERED }; @@ -625,7 +652,7 @@ static int fill_isoc_urb(struct usb_device * udev, } -static genode_usb_request_ret_t +static void handle_urb_request(struct genode_usb_request_urb req, genode_usb_session_handle_t session_handle, genode_usb_request_handle_t request_handle, @@ -637,6 +664,7 @@ handle_urb_request(struct genode_usb_request_urb req, genode_usb_get_request_control(&req); struct genode_usb_request_transfer * transfer = genode_usb_get_request_transfer(&req); + genode_usb_request_ret_t ret = UNKNOWN_ERROR; int err = 0; int read = transfer ? (transfer->ep & 0x80) : 0; @@ -644,8 +672,10 @@ handle_urb_request(struct genode_usb_request_urb req, struct usb_urb_context * context = kmalloc(sizeof(struct usb_urb_context), GFP_NOIO); - if (!context) - return MEMORY_ERROR; + if (!context) { + ret = MEMORY_ERROR; + goto error; + } context->session = session_handle; context->request = request_handle; @@ -666,7 +696,7 @@ handle_urb_request(struct genode_usb_request_urb req, break; default: printk("Unknown USB transfer request!\n"); - return UNKNOWN_ERROR; + goto error; }; if (err) @@ -684,7 +714,7 @@ handle_urb_request(struct genode_usb_request_urb req, mod_timer(&context->timeo, jiffies + msecs_to_jiffies(ctrl->timeout)); } - return 0; + return; free_urb: usb_unanchor_urb(urb); @@ -692,13 +722,15 @@ handle_urb_request(struct genode_usb_request_urb req, free_context: kfree(context); switch (err) { - case -ENOENT: return INTERFACE_OR_ENDPOINT_ERROR; - case -ENODEV: return NO_DEVICE_ERROR; - case -ESHUTDOWN: return NO_DEVICE_ERROR; - case -ENOSPC: return STALL_ERROR; - case -ENOMEM: return MEMORY_ERROR; + case -ENOENT: ret = INTERFACE_OR_ENDPOINT_ERROR; break; + case -ENODEV: ret = NO_DEVICE_ERROR; break; + case -ESHUTDOWN: ret = NO_DEVICE_ERROR; break; + case -ENOSPC: ret = STALL_ERROR; break; + case -ENOMEM: ret = MEMORY_ERROR; break; } - return UNKNOWN_ERROR; + error: + genode_usb_ack_request(context->session, context->request, + handle_return_code, (void*)ret); } diff --git a/repos/os/include/genode_c_api/usb.h b/repos/os/include/genode_c_api/usb.h index 1969e02481..e7335c3598 100644 --- a/repos/os/include/genode_c_api/usb.h +++ b/repos/os/include/genode_c_api/usb.h @@ -159,7 +159,7 @@ struct genode_usb_request_transfer unsigned long actual_packet_size[MAX_PACKETS]; }; -enum Urb_type { CTRL, BULK, IRQ, ISOC }; +enum Urb_type { CTRL, BULK, IRQ, ISOC, NONE }; typedef enum Urb_type genode_usb_urb_t; struct genode_usb_request_urb @@ -193,7 +193,7 @@ enum Request_return_error { }; typedef enum Request_return_error genode_usb_request_ret_t; -typedef genode_usb_request_ret_t (*genode_usb_req_urb_t) +typedef void (*genode_usb_req_urb_t) (struct genode_usb_request_urb req, genode_usb_session_handle_t session_handle, genode_usb_request_handle_t request_handle, @@ -201,20 +201,31 @@ typedef genode_usb_request_ret_t (*genode_usb_req_urb_t) unsigned long payload_size, void * opaque_data); -typedef genode_usb_request_ret_t (*genode_usb_req_string_t) +typedef void (*genode_usb_req_string_t) (struct genode_usb_request_string * req, + genode_usb_session_handle_t session_handle, + genode_usb_request_handle_t request_handle, void * payload, unsigned long payload_size, void * opaque_data); -typedef genode_usb_request_ret_t (*genode_usb_req_altsetting_t) - (unsigned iface, unsigned alt_setting, void * opaque_data); +typedef void (*genode_usb_req_altsetting_t) + (unsigned iface, unsigned alt_setting, + genode_usb_session_handle_t session_handle, + genode_usb_request_handle_t request_handle, + void * opaque_data); -typedef genode_usb_request_ret_t (*genode_usb_req_config_t) - (unsigned config_idx, void * opaque_data); +typedef void (*genode_usb_req_config_t) + (unsigned config_idx, + genode_usb_session_handle_t session_handle, + genode_usb_request_handle_t request_handle, + void * opaque_data); -typedef genode_usb_request_ret_t (*genode_usb_req_flush_t) - (unsigned char ep, void * opaque_data); +typedef void (*genode_usb_req_flush_t) + (unsigned char ep, + genode_usb_session_handle_t session_handle, + genode_usb_request_handle_t request_handle, + void * opaque_data); typedef genode_usb_request_ret_t (*genode_usb_response_t) (struct genode_usb_request_urb req, diff --git a/repos/os/src/lib/genode_c_api/usb.cc b/repos/os/src/lib/genode_c_api/usb.cc index 2a1f49ee29..6e421f6220 100644 --- a/repos/os/src/lib/genode_c_api/usb.cc +++ b/repos/os/src/lib/genode_c_api/usb.cc @@ -445,39 +445,38 @@ bool genode_usb_session::request(genode_usb_request_callbacks & req, void * data void * addr = (void*)(genode_shared_dataspace_local_address(_ds) + offset); + packets[idx].construct(p); + switch (p.type) { case Packet_descriptor::STRING: - _ack(req.string_fn((genode_usb_request_string*)&p.string, - addr, p.size(), data), p); + req.string_fn((genode_usb_request_string*)&p.string, + _id, idx, addr, p.size(), data); break; case Packet_descriptor::CTRL: - packets[idx].construct(p); req.urb_fn({ CTRL, &p.control }, _id, idx, addr, p.size(), data); break; case Packet_descriptor::BULK: - packets[idx].construct(p); req.urb_fn({ BULK, &p.transfer }, _id, idx, addr, p.size(), data); break; case Packet_descriptor::IRQ: - packets[idx].construct(p); req.urb_fn({ IRQ, &p.transfer }, _id, idx, addr, p.size(), data); break; case Packet_descriptor::ISOC: - packets[idx].construct(p); req.urb_fn({ ISOC, &p.transfer }, _id, idx, addr, p.size(), data); break; case Packet_descriptor::ALT_SETTING: - _ack(req.altsetting_fn(p.interface.number, - p.interface.alt_setting, data), p); + req.altsetting_fn(p.interface.number, p.interface.alt_setting, + _id, idx, data); break; case Packet_descriptor::CONFIG: - _ack(req.config_fn(p.number, data), p); + req.config_fn(p.number, _id, idx, data); break; case Packet_descriptor::RELEASE_IF: warning("Release interface gets ignored!"); + packets[idx].destruct(); break; case Packet_descriptor::FLUSH_TRANSFERS: - _ack(req.flush_fn(p.number, data), p); + req.flush_fn(p.number, _id, idx, data); break; }; @@ -506,7 +505,7 @@ void genode_usb_session::handle_response(genode_usb_request_handle_t id, _ack(callback({ ISOC, &p.transfer }, callback_data), p); break; default: - error("Invalid packet descriptor for asynchronous response"); + _ack(callback({ NONE, nullptr }, callback_data), p); }; packets[id].destruct(); } @@ -773,9 +772,19 @@ genode_usb_session_handle_t ::Root::session(genode_usb_bus_num_t bus, template void ::Root::session(genode_usb_session_handle_t id, FUNC const & fn) { + genode_usb_session * session = nullptr; + _for_each_session([&] (genode_usb_session & s) { - if (s._id == id) fn(s); + if (s._id == id) session = &s; }); + + /* + * We've to execute the functor outside the session iteration, + * because the functor might block and the actual session + * can be destroyed in the meantime, which will lead to + * a corrupted next() pointer. + */ + if (session) fn(*session); }