From patchwork Tue Aug 2 12:14:04 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?b?SsO8cmdlbiBHcm/Dnw==?= X-Patchwork-Id: 9256455 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 0BABA6077C for ; Tue, 2 Aug 2016 12:16:56 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F065028512 for ; Tue, 2 Aug 2016 12:16:55 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E520328522; Tue, 2 Aug 2016 12:16:55 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-4.2 required=2.0 tests=BAYES_00, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from lists.xenproject.org (lists.xenproject.org [192.237.175.120]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 464D828512 for ; Tue, 2 Aug 2016 12:16:55 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=lists.xenproject.org) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bUYaL-0004v1-7j; Tue, 02 Aug 2016 12:14:13 +0000 Received: from mail6.bemta5.messagelabs.com ([195.245.231.135]) by lists.xenproject.org with esmtp (Exim 4.84_2) (envelope-from ) id 1bUYaJ-0004uP-Jx for xen-devel@lists.xensource.com; Tue, 02 Aug 2016 12:14:11 +0000 Received: from [85.158.139.211] by server-7.bemta-5.messagelabs.com id 9B/72-05127-29E80A75; Tue, 02 Aug 2016 12:14:10 +0000 X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFrrMLMWRWlGSWpSXmKPExsVyuP0Ov+6kvgX hBv/38Vvcm/Ke3YHRY3vfLvYAxijWzLyk/IoE1oy17+cxFlwxrVgzu5mlgfGJZhcjJ4eEgJHE 24n/mLoYuTiEBBYySrw6vIkRJMEmoCqx4fopVhBbRMBSYtb6NjCbWSBH4vzhDvYuRg4OYYFAi ebz7CBhFqDyqbcWg7XyCthLzJ27kQ1ivpzE9ZnTmUDKOQUcJM7d8QAJCwGVTFrQwz6BkXsBI8 MqRvXi1KKy1CJdS72kosz0jJLcxMwcXUMDU73c1OLixPTUnMSkYr3k/NxNjEDPMgDBDsa1rc6 HGCU5mJREeV2+zA8X4kvKT6nMSCzOiC8qzUktPsQow8GhJMFr37sgXEiwKDU9tSItMwcYYjBp CQ4eJRFeG5A0b3FBYm5xZjpE6hSjopQ4Lw9IQgAkkVGaB9cGC+tLjLJSwryMQIcI8RSkFuVml qDKv2IU52BUEuZ91gM0hSczrwRu+iugxUxAi08YgC0uSURISTUwyt2bLrT/S7e2U8Sip3U3+1 Jvnt5ybPH+P3KdfQfX2LAcb40o2xNW7vfJXdOCbZ/Iy909fML/PpS/bF/5trPlY5bv+k8Hr+8 6K3Df9nr4O+kDO7OuHHvPK+C+oPgOw2EXzZ93rhuYZ2jtO79NOFJwY/DDD9X1Sx7Jvcm5kWdT 5SCsl3HbzUB5ixJLcUaioRZzUXEiAHMERtRmAgAA X-Env-Sender: jgross@suse.com X-Msg-Ref: server-16.tower-206.messagelabs.com!1470140049!36610854!1 X-Originating-IP: [195.135.220.15] X-SpamReason: No, hits=0.0 required=7.0 tests= X-StarScan-Received: X-StarScan-Version: 8.77; banners=-,-,- X-VirusChecked: Checked Received: (qmail 2722 invoked from network); 2 Aug 2016 12:14:10 -0000 Received: from mx2.suse.de (HELO mx2.suse.de) (195.135.220.15) by server-16.tower-206.messagelabs.com with DHE-RSA-CAMELLIA256-SHA encrypted SMTP; 2 Aug 2016 12:14:10 -0000 X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay1.suse.de (charybdis-ext.suse.de [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 6E1F4AC1E; Tue, 2 Aug 2016 12:14:09 +0000 (UTC) From: Juergen Gross To: qemu-devel@nongnu.org, xen-devel@lists.xensource.com Date: Tue, 2 Aug 2016 14:14:04 +0200 Message-Id: <1470140044-16492-3-git-send-email-jgross@suse.com> X-Mailer: git-send-email 2.6.6 In-Reply-To: <1470140044-16492-1-git-send-email-jgross@suse.com> References: <1470140044-16492-1-git-send-email-jgross@suse.com> Cc: anthony.perard@citrix.com, Juergen Gross , sstabellini@kernel.org, kraxel@redhat.com Subject: [Xen-devel] [PATCH v2 2/2] xen: drain submit queue in xen-usb before removing device X-BeenThere: xen-devel@lists.xen.org X-Mailman-Version: 2.1.18 Precedence: list List-Id: Xen developer discussion List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: xen-devel-bounces@lists.xen.org Sender: "Xen-devel" X-Virus-Scanned: ClamAV using ClamSMTP When unplugging a device in the Xen pvusb backend drain the submit queue before deallocation of the control structures. Otherwise there will be bogus memory accesses when I/O contracts are finished. Correlated to this issue is the handling of cancel requests: a packet cancelled will still lead to the call of complete, so add a flag to the request indicating it should be just dropped on complete. Signed-off-by: Juergen Gross Acked-by: Anthony PERARD --- hw/usb/xen-usb.c | 94 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 62 insertions(+), 32 deletions(-) diff --git a/hw/usb/xen-usb.c b/hw/usb/xen-usb.c index 7992456..174d715 100644 --- a/hw/usb/xen-usb.c +++ b/hw/usb/xen-usb.c @@ -90,6 +90,8 @@ struct usbback_req { void *buffer; void *isoc_buffer; struct libusb_transfer *xfer; + + bool cancelled; }; struct usbback_hotplug { @@ -301,20 +303,23 @@ static void usbback_do_response(struct usbback_req *usbback_req, int32_t status, usbback_req->isoc_buffer = NULL; } - res = RING_GET_RESPONSE(&usbif->urb_ring, usbif->urb_ring.rsp_prod_pvt); - res->id = usbback_req->req.id; - res->status = status; - res->actual_length = actual_length; - res->error_count = error_count; - res->start_frame = 0; - usbif->urb_ring.rsp_prod_pvt++; - RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&usbif->urb_ring, notify); + if (usbif->urb_sring) { + res = RING_GET_RESPONSE(&usbif->urb_ring, usbif->urb_ring.rsp_prod_pvt); + res->id = usbback_req->req.id; + res->status = status; + res->actual_length = actual_length; + res->error_count = error_count; + res->start_frame = 0; + usbif->urb_ring.rsp_prod_pvt++; + RING_PUSH_RESPONSES_AND_CHECK_NOTIFY(&usbif->urb_ring, notify); - if (notify) { - xen_be_send_notify(xendev); + if (notify) { + xen_be_send_notify(xendev); + } } - usbback_put_req(usbback_req); + if (!usbback_req->cancelled) + usbback_put_req(usbback_req); } static void usbback_do_response_ret(struct usbback_req *usbback_req, @@ -366,15 +371,14 @@ static void usbback_set_address(struct usbback_info *usbif, } } -static bool usbback_cancel_req(struct usbback_req *usbback_req) +static void usbback_cancel_req(struct usbback_req *usbback_req) { - bool ret = false; - if (usb_packet_is_inflight(&usbback_req->packet)) { usb_cancel_packet(&usbback_req->packet); - ret = true; + QTAILQ_REMOVE(&usbback_req->stub->submit_q, usbback_req, q); + usbback_req->cancelled = true; + usbback_do_response_ret(usbback_req, -EPROTO); } - return ret; } static void usbback_process_unlink_req(struct usbback_req *usbback_req) @@ -391,7 +395,7 @@ static void usbback_process_unlink_req(struct usbback_req *usbback_req) devnum = usbif_pipedevice(usbback_req->req.pipe); if (unlikely(devnum == 0)) { usbback_req->stub = usbif->ports + - usbif_pipeportnum(usbback_req->req.pipe); + usbif_pipeportnum(usbback_req->req.pipe) - 1; if (unlikely(!usbback_req->stub)) { ret = -ENODEV; goto fail_response; @@ -406,9 +410,7 @@ static void usbback_process_unlink_req(struct usbback_req *usbback_req) QTAILQ_FOREACH(unlink_req, &usbback_req->stub->submit_q, q) { if (unlink_req->req.id == id) { - if (usbback_cancel_req(unlink_req)) { - usbback_do_response_ret(unlink_req, -EPROTO); - } + usbback_cancel_req(unlink_req); break; } } @@ -681,6 +683,33 @@ static void usbback_hotplug_enq(struct usbback_info *usbif, unsigned port) usbback_hotplug_notify(usbif); } +static void usbback_portid_drain(struct usbback_info *usbif, unsigned port) +{ + struct usbback_req *req, *tmp; + bool sched = false; + + QTAILQ_FOREACH_SAFE(req, &usbif->ports[port - 1].submit_q, q, tmp) { + usbback_cancel_req(req); + sched = true; + } + + if (sched) { + qemu_bh_schedule(usbif->bh); + } +} + +static void usbback_portid_detach(struct usbback_info *usbif, unsigned port) +{ + if (!usbif->ports[port - 1].attached) { + return; + } + + usbif->ports[port - 1].speed = USBIF_SPEED_NONE; + usbif->ports[port - 1].attached = false; + usbback_portid_drain(usbif, port); + usbback_hotplug_enq(usbif, port); +} + static void usbback_portid_remove(struct usbback_info *usbif, unsigned port) { USBPort *p; @@ -694,9 +723,7 @@ static void usbback_portid_remove(struct usbback_info *usbif, unsigned port) object_unparent(OBJECT(usbif->ports[port - 1].dev)); usbif->ports[port - 1].dev = NULL; - usbif->ports[port - 1].speed = USBIF_SPEED_NONE; - usbif->ports[port - 1].attached = false; - usbback_hotplug_enq(usbif, port); + usbback_portid_detach(usbif, port); TR_BUS(&usbif->xendev, "port %d removed\n", port); } @@ -801,7 +828,6 @@ static void usbback_process_port(struct usbback_info *usbif, unsigned port) static void usbback_disconnect(struct XenDevice *xendev) { struct usbback_info *usbif; - struct usbback_req *req, *tmp; unsigned int i; TR_BUS(xendev, "start\n"); @@ -820,11 +846,8 @@ static void usbback_disconnect(struct XenDevice *xendev) } for (i = 0; i < usbif->num_ports; i++) { - if (!usbif->ports[i].dev) { - continue; - } - QTAILQ_FOREACH_SAFE(req, &usbif->ports[i].submit_q, q, tmp) { - usbback_cancel_req(req); + if (usbif->ports[i].dev) { + usbback_portid_drain(usbif, i + 1); } } @@ -944,8 +967,7 @@ static void xen_bus_detach(USBPort *port) usbif = port->opaque; TR_BUS(&usbif->xendev, "\n"); - usbif->ports[port->index].attached = false; - usbback_hotplug_enq(usbif, port->index + 1); + usbback_portid_detach(usbif, port->index + 1); } static void xen_bus_child_detach(USBPort *port, USBDevice *child) @@ -958,9 +980,16 @@ static void xen_bus_child_detach(USBPort *port, USBDevice *child) static void xen_bus_complete(USBPort *port, USBPacket *packet) { + struct usbback_req *usbback_req; struct usbback_info *usbif; - usbif = port->opaque; + usbback_req = container_of(packet, struct usbback_req, packet); + if (usbback_req->cancelled) { + g_free(usbback_req); + return; + } + + usbif = usbback_req->usbif; TR_REQ(&usbif->xendev, "\n"); usbback_packet_complete(packet); } @@ -1037,6 +1066,7 @@ static int usbback_free(struct XenDevice *xendev) } usb_bus_release(&usbif->bus); + object_unparent(OBJECT(&usbif->bus)); TR_BUS(xendev, "finished\n");