From patchwork Wed Aug 3 13:05:48 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Gerd Hoffmann X-Patchwork-Id: 9261233 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 A77FD6048B for ; Wed, 3 Aug 2016 13:11:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9749127FA9 for ; Wed, 3 Aug 2016 13:11:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8BEF2284FB; Wed, 3 Aug 2016 13:11:28 +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 4CE1B27FA9 for ; Wed, 3 Aug 2016 13:11:27 +0000 (UTC) Received: from localhost ([::1]:34429 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUvxG-00051G-9a for patchwork-qemu-devel@patchwork.kernel.org; Wed, 03 Aug 2016 09:11:26 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39991) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUvrz-0008Du-NV for qemu-devel@nongnu.org; Wed, 03 Aug 2016 09:06:01 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bUvrv-00085d-4o for qemu-devel@nongnu.org; Wed, 03 Aug 2016 09:05:58 -0400 Received: from mx1.redhat.com ([209.132.183.28]:37568) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bUvru-00085B-Sp for qemu-devel@nongnu.org; Wed, 03 Aug 2016 09:05:55 -0400 Received: from int-mx10.intmail.prod.int.phx2.redhat.com (int-mx10.intmail.prod.int.phx2.redhat.com [10.5.11.23]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 83890C0AA369; Wed, 3 Aug 2016 13:05:54 +0000 (UTC) Received: from nilsson.home.kraxel.org (ovpn-116-18.ams2.redhat.com [10.36.116.18]) by int-mx10.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id u73D5rZp015010; Wed, 3 Aug 2016 09:05:53 -0400 Received: by nilsson.home.kraxel.org (Postfix, from userid 500) id B7B3280FF7; Wed, 3 Aug 2016 15:05:51 +0200 (CEST) From: Gerd Hoffmann To: qemu-devel@nongnu.org Date: Wed, 3 Aug 2016 15:05:48 +0200 Message-Id: <1470229549-32477-6-git-send-email-kraxel@redhat.com> In-Reply-To: <1470229549-32477-1-git-send-email-kraxel@redhat.com> References: <1470229549-32477-1-git-send-email-kraxel@redhat.com> X-Scanned-By: MIMEDefang 2.68 on 10.5.11.23 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Wed, 03 Aug 2016 13:05:54 +0000 (UTC) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.132.183.28 Subject: [Qemu-devel] [PULL 5/6] 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: Juergen Gross , Gerd Hoffmann Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP From: Juergen Gross 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 Message-id: 1470140044-16492-3-git-send-email-jgross@suse.com Signed-off-by: Gerd Hoffmann --- 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");