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: 9256733 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 B7D6960754 for ; Tue, 2 Aug 2016 12:34:54 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A9372284D4 for ; Tue, 2 Aug 2016 12:34:54 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9DA5628527; Tue, 2 Aug 2016 12:34:54 +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=-6.9 required=2.0 tests=BAYES_00,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id D4ED0284D4 for ; Tue, 2 Aug 2016 12:34:53 +0000 (UTC) Received: from localhost ([::1]:55914 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUYuK-00080n-Fj for patchwork-qemu-devel@patchwork.kernel.org; Tue, 02 Aug 2016 08:34:52 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:55160) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUYaL-0005Ha-9A for qemu-devel@nongnu.org; Tue, 02 Aug 2016 08:14:18 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUYaI-0004Bx-FW for qemu-devel@nongnu.org; Tue, 02 Aug 2016 08:14:12 -0400 Received: from mx2.suse.de ([195.135.220.15]:60317) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUYaI-0004Bg-2t for qemu-devel@nongnu.org; Tue, 02 Aug 2016 08:14:10 -0400 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> X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x (no timestamps) [generic] X-Received-From: 195.135.220.15 Subject: [Qemu-devel] [PATCH v2 2/2] xen: drain submit queue in xen-usb before removing device X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: anthony.perard@citrix.com, Juergen Gross , sstabellini@kernel.org, kraxel@redhat.com Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-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");