From patchwork Tue Jul 17 17:01:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sudip Mukherjee X-Patchwork-Id: 10530151 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 58228603ED for ; Tue, 17 Jul 2018 17:01:26 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 352CB29691 for ; Tue, 17 Jul 2018 17:01:26 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 291E32969B; Tue, 17 Jul 2018 17:01:26 +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=-7.8 required=2.0 tests=BAYES_00, DKIM_ADSP_CUSTOM_MED, DKIM_SIGNED, FREEMAIL_FROM, MAILING_LIST_MULTI, RCVD_IN_DNSWL_HI, T_DKIM_INVALID, T_TVD_MIME_EPI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9AE2129691 for ; Tue, 17 Jul 2018 17:01:25 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1729683AbeGQRe6 (ORCPT ); Tue, 17 Jul 2018 13:34:58 -0400 Received: from mail-wr1-f66.google.com ([209.85.221.66]:38499 "EHLO mail-wr1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729641AbeGQRe6 (ORCPT ); Tue, 17 Jul 2018 13:34:58 -0400 Received: by mail-wr1-f66.google.com with SMTP id v14-v6so1961143wro.5 for ; Tue, 17 Jul 2018 10:01:23 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=UhpuhBjiylDPJC5HLZHklrEwnXyxtm5xeNwidH29rNA=; b=lwO2a+KGvmVGaWBwkcPWNAydKnQPjhXk2/iry7taRCpSN/2QpPYoriw3BaNRPyu42S Q3kDmaJlcWy4vwos8TeAFSA5f4qwj0XdsME76kYmSgVJwarOpJbJ17WS7TKvWmfNB3je 5wxczUspuzoqd4RqeqGj78312OFIf0qAU0xBlGOXkM2GfMOywotgIZwvkvktbABmMNNo RMNmvhJqomMF/yrQ5jwoiSXFaE0R5spUDDSZhvBqnFw7XyJb8MzAMkJdbNaWuA0aQSBE OkUY9yxwi5Vjl0majqyYPHekdylCt2oWziVaWwEhxOX3SuYr1rIGouUSqVQB6aIlOCcx t2pw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=UhpuhBjiylDPJC5HLZHklrEwnXyxtm5xeNwidH29rNA=; b=c2icx2f4m6v247xVGB6AF/oHQ00qxfYv03z9RMaxk7axk/DqwLDIbs/BoFYkRy+6NC 62eehAkDM39+h2wWW/gejFAvHdqzP2Z8b9ogpSj6zXV62QlscyH47d+xJAruhxo8Vxc9 OtLwS83XcPmrKVLgkwVTAJPP/w3J/L8jMlDQPxufe6VLSdh2ax8x4mNnj2fV2X1sw/Tp hluQX7w1NhPsYsuMfNmXXwPrasvL6/BqWg1OIozOrDM10CC8p6vZW/j20Yx+JD4QKsyP Xv6DAQUdAgWjwCK94RV9vGv9D1fNDMQEr3UCjedaClmmPOAA8HG/z97L/W990VwpZLU6 a0lQ== X-Gm-Message-State: AOUpUlGw/P/GI2XNEry0gWvD7E0Zm9t6lK+R1lr0fSq5UAhfEOl8TyZq E5C0IAeJDffKgg9rqKvp+O4= X-Google-Smtp-Source: AAOMgpffnQ2CurSUVyRcGTSo+ta72vbnC0zjE19KEI3kRRGTNvrL+9GumhsiNoHE1I5lT2aQ/IhW/Q== X-Received: by 2002:adf:e78d:: with SMTP id n13-v6mr2013753wrm.136.1531846882446; Tue, 17 Jul 2018 10:01:22 -0700 (PDT) Received: from debian ([78.40.148.180]) by smtp.gmail.com with ESMTPSA id q140-v6sm27236wmb.35.2018.07.17.10.01.20 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 17 Jul 2018 10:01:21 -0700 (PDT) Date: Tue, 17 Jul 2018 18:01:08 +0100 From: Sudip Mukherjee To: Greg KH Cc: Alan Stern , Mathias Nyman , Andy Shevchenko , Andy Shevchenko , Mathias Nyman , linux-usb@vger.kernel.org, lukaszx.szulc@intel.com, Christoph Hellwig , Marek Szyprowski , iommu@lists.linux-foundation.org Subject: Re: usb HC busted? Message-ID: <20180717170108.5bv22bfmqbjktifs@debian> References: <20180717120411.GB28592@kroah.com> <20180717155259.GB2416@kroah.com> <20180717155901.5csifwp5ak2ixfk5@debian> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180717155901.5csifwp5ak2ixfk5@debian> User-Agent: NeoMutt/20170113 (1.7.2) Sender: linux-usb-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-usb@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Tue, Jul 17, 2018 at 04:59:01PM +0100, Sudip Mukherjee wrote: > On Tue, Jul 17, 2018 at 05:52:59PM +0200, Greg KH wrote: > > On Tue, Jul 17, 2018 at 10:31:38AM -0400, Alan Stern wrote: > > > On Tue, 17 Jul 2018, Greg KH wrote: > > > > > > > > From: Sudip Mukherjee > > > > > Date: Tue, 10 Jul 2018 09:50:00 +0100 > > > > > Subject: [PATCH] hacky solution to mem-corruption > > > > > > > > > > Signed-off-by: Sudip Mukherjee > > > > > --- > > > > > > > No, neither of these is right. It's possible to use > > > usb_set_interface() as a kind of "soft" reset. Even when the new > > > altsetting is specified to be the same as the current one, we still > > > have to tell the lower-layer drivers and hardware about it. > > > > You are right, it's a hacky soft reset, I was just trying to figure out > > what the bluetooth driver was trying to do. I wouldn't expect it to be > > calling that function a lot, but I guess it does :( > > usb_set_interface() is being called two times from bluetooth event. But > I am now adding more debugs to see why your patch did not work. So, a very simple debug to see the sequence of functions being called. I have attached the patch I used. In a good case: [ 124.287991] sudip: xhci_urb_dequeue [ 124.287997] sudip: xhci_queue_stop_endpoint cmd=ee032950 [ 124.288016] sudip: handle_cmd_completion cmd=ee032950 [ 124.288173] sudip: xhci_urb_dequeue [ 124.288176] sudip: xhci_queue_stop_endpoint cmd=ee032950 [ 124.288189] sudip: handle_cmd_completion cmd=ee032950 [ 124.290647] sudip: usb_hcd_flush_endpoint [ 124.290652] sudip: usb_hcd_flush_endpoint But in a bad case: [ 186.786900] sudip: xhci_urb_dequeue [ 186.786905] sudip: xhci_queue_stop_endpoint cmd=ebe47cb0 [ 186.786923] sudip: handle_cmd_completion cmd=ebe47cb0 [ 186.789040] sudip: xhci_urb_dequeue [ 186.789047] sudip: xhci_queue_stop_endpoint cmd=ebe47cb0 [ 186.789069] sudip: handle_cmd_completion cmd=ebe47cb0 [ 186.790082] sudip: usb_hcd_flush_endpoint [ 186.790094] sudip: xhci_urb_dequeue [ 186.790097] sudip: xhci_queue_stop_endpoint cmd=ebe47290 [ 186.790150] sudip: handle_cmd_completion cmd=ebe47290 [ 186.790202] sudip: usb_hcd_flush_endpoint So, when usb_hcd_flush_endpoint() is called by usb_disable_endpoint() it finds urbs still on the urb_list of the ep. And in the process of unlinking them, it again sends the command to stop the endpoint, although that endpoint has already been stopped. So Greg's patch did not work as the memory got corrupted on the first call to usb_set_interface(), whereas that patch was preventing the second call to usb_set_interface(). --- Regards Sudip diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c index 467bedeb542a..8d28f120ec0a 100644 --- a/drivers/usb/core/hcd.c +++ b/drivers/usb/core/hcd.c @@ -1885,6 +1885,7 @@ void usb_hcd_flush_endpoint(struct usb_device *udev, might_sleep(); hcd = bus_to_hcd(udev->bus); + pr_err("sudip: %s\n", __func__); /* No more submits can occur */ spin_lock_irq(&hcd_urb_list_lock); rescan: diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 6996235e34a9..4f80791fdfc5 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1450,6 +1450,7 @@ static void handle_cmd_completion(struct xhci_hcd *xhci, case TRB_STOP_RING: WARN_ON(slot_id != TRB_TO_SLOT_ID( le32_to_cpu(cmd_trb->generic.field[3]))); + pr_err("sudip: %s cmd=%p\n", __func__, cmd); xhci_handle_cmd_stop_ep(xhci, slot_id, cmd_trb, event); break; case TRB_SET_DEQ: @@ -4009,6 +4010,7 @@ int xhci_queue_stop_endpoint(struct xhci_hcd *xhci, struct xhci_command *cmd, u32 type = TRB_TYPE(TRB_STOP_RING); u32 trb_suspend = SUSPEND_PORT_FOR_TRB(suspend); + pr_err("sudip: %s cmd=%p\n", __func__, cmd); return queue_command(xhci, cmd, 0, 0, 0, trb_slot_id | trb_ep_index | type | trb_suspend, false); } diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index db1de6113db2..3832128107ff 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1516,6 +1516,7 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) ep->stop_cmd_timer.expires = jiffies + XHCI_STOP_EP_CMD_TIMEOUT * HZ; add_timer(&ep->stop_cmd_timer); + pr_err("sudip: %s\n", __func__); xhci_queue_stop_endpoint(xhci, command, urb->dev->slot_id, ep_index, 0); xhci_ring_cmd_db(xhci);