From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sudip Mukherjee Subject: Re: usb HC busted? Date: Tue, 17 Jul 2018 18:01:08 +0100 Message-ID: <20180717170108.5bv22bfmqbjktifs@debian> References: <20180717120411.GB28592@kroah.com> <20180717155259.GB2416@kroah.com> <20180717155901.5csifwp5ak2ixfk5@debian> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="irlgiq2damz5zlpp" Return-path: Content-Disposition: inline In-Reply-To: <20180717155901.5csifwp5ak2ixfk5@debian> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org Errors-To: iommu-bounces-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org To: Greg KH Cc: Mathias Nyman , Mathias Nyman , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org, Christoph Hellwig , Andy Shevchenko , Alan Stern , Andy Shevchenko , lukaszx.szulc-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org List-Id: iommu@lists.linux-foundation.org --irlgiq2damz5zlpp Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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 --irlgiq2damz5zlpp Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename=patch 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); --irlgiq2damz5zlpp Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline --irlgiq2damz5zlpp-- From mboxrd@z Thu Jan 1 00:00:00 1970 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: base64 Subject: usb HC busted? From: Sudip Mukherjee Message-Id: <20180717170108.5bv22bfmqbjktifs@debian> Date: Tue, 17 Jul 2018 18:01:08 +0100 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 List-ID: T24gVHVlLCBKdWwgMTcsIDIwMTggYXQgMDQ6NTk6MDFQTSArMDEwMCwgU3VkaXAgTXVraGVyamVl IHdyb3RlOgo+IE9uIFR1ZSwgSnVsIDE3LCAyMDE4IGF0IDA1OjUyOjU5UE0gKzAyMDAsIEdyZWcg S0ggd3JvdGU6Cj4gPiBPbiBUdWUsIEp1bCAxNywgMjAxOCBhdCAxMDozMTozOEFNIC0wNDAwLCBB bGFuIFN0ZXJuIHdyb3RlOgo+ID4gPiBPbiBUdWUsIDE3IEp1bCAyMDE4LCBHcmVnIEtIIHdyb3Rl Ogo+ID4gPiAKPiA+ID4gPiA+IEZyb206IFN1ZGlwIE11a2hlcmplZSA8c3VkaXBtLm11a2hlcmpl ZUBnbWFpbC5jb20+Cj4gPiA+ID4gPiBEYXRlOiBUdWUsIDEwIEp1bCAyMDE4IDA5OjUwOjAwICsw MTAwCj4gPiA+ID4gPiBTdWJqZWN0OiBbUEFUQ0hdIGhhY2t5IHNvbHV0aW9uIHRvIG1lbS1jb3Jy dXB0aW9uCj4gPiA+ID4gPiAKPiA+ID4gPiA+IFNpZ25lZC1vZmYtYnk6IFN1ZGlwIE11a2hlcmpl ZSA8c3VkaXBtLm11a2hlcmplZUBnbWFpbC5jb20+Cj4gPiA+ID4gPiAtLS0KPiA8c25pcD4KPiA+ ID4gCj4gPiA+IE5vLCBuZWl0aGVyIG9mIHRoZXNlIGlzIHJpZ2h0LiAgSXQncyBwb3NzaWJsZSB0 byB1c2UgCj4gPiA+IHVzYl9zZXRfaW50ZXJmYWNlKCkgYXMgYSBraW5kIG9mICJzb2Z0IiByZXNl dC4gIEV2ZW4gd2hlbiB0aGUgbmV3IAo+ID4gPiBhbHRzZXR0aW5nIGlzIHNwZWNpZmllZCB0byBi ZSB0aGUgc2FtZSBhcyB0aGUgY3VycmVudCBvbmUsIHdlIHN0aWxsIAo+ID4gPiBoYXZlIHRvIHRl bGwgdGhlIGxvd2VyLWxheWVyIGRyaXZlcnMgYW5kIGhhcmR3YXJlIGFib3V0IGl0Lgo+ID4gCj4g PiBZb3UgYXJlIHJpZ2h0LCBpdCdzIGEgaGFja3kgc29mdCByZXNldCwgSSB3YXMganVzdCB0cnlp bmcgdG8gZmlndXJlIG91dAo+ID4gd2hhdCB0aGUgYmx1ZXRvb3RoIGRyaXZlciB3YXMgdHJ5aW5n IHRvIGRvLiAgSSB3b3VsZG4ndCBleHBlY3QgaXQgdG8gYmUKPiA+IGNhbGxpbmcgdGhhdCBmdW5j dGlvbiBhIGxvdCwgYnV0IEkgZ3Vlc3MgaXQgZG9lcyA6KAo+IAo+IHVzYl9zZXRfaW50ZXJmYWNl KCkgaXMgYmVpbmcgY2FsbGVkIHR3byB0aW1lcyBmcm9tIGJsdWV0b290aCBldmVudC4gQnV0Cj4g SSBhbSBub3cgYWRkaW5nIG1vcmUgZGVidWdzIHRvIHNlZSB3aHkgeW91ciBwYXRjaCBkaWQgbm90 IHdvcmsuCgpTbywgYSB2ZXJ5IHNpbXBsZSBkZWJ1ZyB0byBzZWUgdGhlIHNlcXVlbmNlIG9mIGZ1 bmN0aW9ucyBiZWluZyBjYWxsZWQuCkkgaGF2ZSBhdHRhY2hlZCB0aGUgcGF0Y2ggSSB1c2VkLgoK SW4gYSBnb29kIGNhc2U6ClsgIDEyNC4yODc5OTFdIHN1ZGlwOiB4aGNpX3VyYl9kZXF1ZXVlClsg IDEyNC4yODc5OTddIHN1ZGlwOiB4aGNpX3F1ZXVlX3N0b3BfZW5kcG9pbnQgY21kPWVlMDMyOTUw ClsgIDEyNC4yODgwMTZdIHN1ZGlwOiBoYW5kbGVfY21kX2NvbXBsZXRpb24gY21kPWVlMDMyOTUw ClsgIDEyNC4yODgxNzNdIHN1ZGlwOiB4aGNpX3VyYl9kZXF1ZXVlClsgIDEyNC4yODgxNzZdIHN1 ZGlwOiB4aGNpX3F1ZXVlX3N0b3BfZW5kcG9pbnQgY21kPWVlMDMyOTUwClsgIDEyNC4yODgxODld IHN1ZGlwOiBoYW5kbGVfY21kX2NvbXBsZXRpb24gY21kPWVlMDMyOTUwClsgIDEyNC4yOTA2NDdd IHN1ZGlwOiB1c2JfaGNkX2ZsdXNoX2VuZHBvaW50ClsgIDEyNC4yOTA2NTJdIHN1ZGlwOiB1c2Jf aGNkX2ZsdXNoX2VuZHBvaW50CgpCdXQgaW4gYSBiYWQgY2FzZToKWyAgMTg2Ljc4NjkwMF0gc3Vk aXA6IHhoY2lfdXJiX2RlcXVldWUKWyAgMTg2Ljc4NjkwNV0gc3VkaXA6IHhoY2lfcXVldWVfc3Rv cF9lbmRwb2ludCBjbWQ9ZWJlNDdjYjAKWyAgMTg2Ljc4NjkyM10gc3VkaXA6IGhhbmRsZV9jbWRf Y29tcGxldGlvbiBjbWQ9ZWJlNDdjYjAKWyAgMTg2Ljc4OTA0MF0gc3VkaXA6IHhoY2lfdXJiX2Rl cXVldWUKWyAgMTg2Ljc4OTA0N10gc3VkaXA6IHhoY2lfcXVldWVfc3RvcF9lbmRwb2ludCBjbWQ9 ZWJlNDdjYjAKWyAgMTg2Ljc4OTA2OV0gc3VkaXA6IGhhbmRsZV9jbWRfY29tcGxldGlvbiBjbWQ9 ZWJlNDdjYjAKWyAgMTg2Ljc5MDA4Ml0gc3VkaXA6IHVzYl9oY2RfZmx1c2hfZW5kcG9pbnQKWyAg MTg2Ljc5MDA5NF0gc3VkaXA6IHhoY2lfdXJiX2RlcXVldWUKWyAgMTg2Ljc5MDA5N10gc3VkaXA6 IHhoY2lfcXVldWVfc3RvcF9lbmRwb2ludCBjbWQ9ZWJlNDcyOTAKWyAgMTg2Ljc5MDE1MF0gc3Vk aXA6IGhhbmRsZV9jbWRfY29tcGxldGlvbiBjbWQ9ZWJlNDcyOTAKWyAgMTg2Ljc5MDIwMl0gc3Vk aXA6IHVzYl9oY2RfZmx1c2hfZW5kcG9pbnQKClNvLCB3aGVuIHVzYl9oY2RfZmx1c2hfZW5kcG9p bnQoKSBpcyBjYWxsZWQgYnkgdXNiX2Rpc2FibGVfZW5kcG9pbnQoKSBpdApmaW5kcyB1cmJzIHN0 aWxsIG9uIHRoZSB1cmJfbGlzdCBvZiB0aGUgZXAuIEFuZCBpbiB0aGUgcHJvY2VzcyBvZiB1bmxp bmtpbmcKdGhlbSwgaXQgYWdhaW4gc2VuZHMgdGhlIGNvbW1hbmQgdG8gc3RvcCB0aGUgZW5kcG9p bnQsIGFsdGhvdWdoIHRoYXQgZW5kcG9pbnQKaGFzIGFscmVhZHkgYmVlbiBzdG9wcGVkLgpTbyBH cmVnJ3MgcGF0Y2ggZGlkIG5vdCB3b3JrIGFzIHRoZSBtZW1vcnkgZ290IGNvcnJ1cHRlZCBvbiB0 aGUgZmlyc3QgY2FsbAp0byB1c2Jfc2V0X2ludGVyZmFjZSgpLCB3aGVyZWFzIHRoYXQgcGF0Y2gg d2FzIHByZXZlbnRpbmcgdGhlIHNlY29uZCBjYWxsCnRvIHVzYl9zZXRfaW50ZXJmYWNlKCkuCi0t LQpSZWdhcmRzClN1ZGlwCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvY29yZS9oY2QuYyBiL2Ry aXZlcnMvdXNiL2NvcmUvaGNkLmMKaW5kZXggNDY3YmVkZWI1NDJhLi44ZDI4ZjEyMGVjMGEgMTAw NjQ0Ci0tLSBhL2RyaXZlcnMvdXNiL2NvcmUvaGNkLmMKKysrIGIvZHJpdmVycy91c2IvY29yZS9o Y2QuYwpAQCAtMTg4NSw2ICsxODg1LDcgQEAgdm9pZCB1c2JfaGNkX2ZsdXNoX2VuZHBvaW50KHN0 cnVjdCB1c2JfZGV2aWNlICp1ZGV2LAogCW1pZ2h0X3NsZWVwKCk7CiAJaGNkID0gYnVzX3RvX2hj ZCh1ZGV2LT5idXMpOwogCisJcHJfZXJyKCJzdWRpcDogJXNcbiIsIF9fZnVuY19fKTsKIAkvKiBO byBtb3JlIHN1Ym1pdHMgY2FuIG9jY3VyICovCiAJc3Bpbl9sb2NrX2lycSgmaGNkX3VyYl9saXN0 X2xvY2spOwogcmVzY2FuOgpkaWZmIC0tZ2l0IGEvZHJpdmVycy91c2IvaG9zdC94aGNpLXJpbmcu YyBiL2RyaXZlcnMvdXNiL2hvc3QveGhjaS1yaW5nLmMKaW5kZXggNjk5NjIzNWUzNGE5Li40Zjgw NzkxZmRmYzUgMTAwNjQ0Ci0tLSBhL2RyaXZlcnMvdXNiL2hvc3QveGhjaS1yaW5nLmMKKysrIGIv ZHJpdmVycy91c2IvaG9zdC94aGNpLXJpbmcuYwpAQCAtMTQ1MCw2ICsxNDUwLDcgQEAgc3RhdGlj IHZvaWQgaGFuZGxlX2NtZF9jb21wbGV0aW9uKHN0cnVjdCB4aGNpX2hjZCAqeGhjaSwKIAljYXNl IFRSQl9TVE9QX1JJTkc6CiAJCVdBUk5fT04oc2xvdF9pZCAhPSBUUkJfVE9fU0xPVF9JRCgKIAkJ CQlsZTMyX3RvX2NwdShjbWRfdHJiLT5nZW5lcmljLmZpZWxkWzNdKSkpOworCQlwcl9lcnIoInN1 ZGlwOiAlcyBjbWQ9JXBcbiIsIF9fZnVuY19fLCBjbWQpOwogCQl4aGNpX2hhbmRsZV9jbWRfc3Rv cF9lcCh4aGNpLCBzbG90X2lkLCBjbWRfdHJiLCBldmVudCk7CiAJCWJyZWFrOwogCWNhc2UgVFJC X1NFVF9ERVE6CkBAIC00MDA5LDYgKzQwMTAsNyBAQCBpbnQgeGhjaV9xdWV1ZV9zdG9wX2VuZHBv aW50KHN0cnVjdCB4aGNpX2hjZCAqeGhjaSwgc3RydWN0IHhoY2lfY29tbWFuZCAqY21kLAogCXUz MiB0eXBlID0gVFJCX1RZUEUoVFJCX1NUT1BfUklORyk7CiAJdTMyIHRyYl9zdXNwZW5kID0gU1VT UEVORF9QT1JUX0ZPUl9UUkIoc3VzcGVuZCk7CiAKKwlwcl9lcnIoInN1ZGlwOiAlcyBjbWQ9JXBc biIsIF9fZnVuY19fLCBjbWQpOwogCXJldHVybiBxdWV1ZV9jb21tYW5kKHhoY2ksIGNtZCwgMCwg MCwgMCwKIAkJCXRyYl9zbG90X2lkIHwgdHJiX2VwX2luZGV4IHwgdHlwZSB8IHRyYl9zdXNwZW5k LCBmYWxzZSk7CiB9CmRpZmYgLS1naXQgYS9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuYyBiL2RyaXZl cnMvdXNiL2hvc3QveGhjaS5jCmluZGV4IGRiMWRlNjExM2RiMi4uMzgzMjEyODEwN2ZmIDEwMDY0 NAotLS0gYS9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuYworKysgYi9kcml2ZXJzL3VzYi9ob3N0L3ho Y2kuYwpAQCAtMTUxNiw2ICsxNTE2LDcgQEAgc3RhdGljIGludCB4aGNpX3VyYl9kZXF1ZXVlKHN0 cnVjdCB1c2JfaGNkICpoY2QsIHN0cnVjdCB1cmIgKnVyYiwgaW50IHN0YXR1cykKIAkJZXAtPnN0 b3BfY21kX3RpbWVyLmV4cGlyZXMgPSBqaWZmaWVzICsKIAkJCVhIQ0lfU1RPUF9FUF9DTURfVElN RU9VVCAqIEhaOwogCQlhZGRfdGltZXIoJmVwLT5zdG9wX2NtZF90aW1lcik7CisJCXByX2Vycigi c3VkaXA6ICVzXG4iLCBfX2Z1bmNfXyk7CiAJCXhoY2lfcXVldWVfc3RvcF9lbmRwb2ludCh4aGNp LCBjb21tYW5kLCB1cmItPmRldi0+c2xvdF9pZCwKIAkJCQkJIGVwX2luZGV4LCAwKTsKIAkJeGhj aV9yaW5nX2NtZF9kYih4aGNpKTsK