From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga12.intel.com ([192.55.52.136]:47737 "EHLO mga12.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S933209AbeFUNRM (ORCPT ); Thu, 21 Jun 2018 09:17:12 -0400 From: Mathias Nyman To: Cc: , Mathias Nyman , stable@vger.kernel.org Subject: [PATCH 1/5] xhci: Fix perceived dead host due to runtime suspend race with event handler Date: Thu, 21 Jun 2018 16:19:41 +0300 Message-Id: <1529587185-2776-2-git-send-email-mathias.nyman@linux.intel.com> In-Reply-To: <1529587185-2776-1-git-send-email-mathias.nyman@linux.intel.com> References: <1529587185-2776-1-git-send-email-mathias.nyman@linux.intel.com> Sender: stable-owner@vger.kernel.org List-ID: Don't rely on event interrupt (EINT) bit alone to detect pending port change in resume. If no change event is detected the host may be suspended again, oterwise roothubs are resumed. There is a lag in xHC setting EINT. If we don't notice the pending change in resume, and the controller is runtime suspeded again, it causes the event handler to assume host is dead as it will fail to read xHC registers once PCI puts the controller to D3 state. [ 268.520969] xhci_hcd: xhci_resume: starting port polling. [ 268.520985] xhci_hcd: xhci_hub_status_data: stopping port polling. [ 268.521030] xhci_hcd: xhci_suspend: stopping port polling. [ 268.521040] xhci_hcd: // Setting command ring address to 0x349bd001 [ 268.521139] xhci_hcd: Port Status Change Event for port 3 [ 268.521149] xhci_hcd: resume root hub [ 268.521163] xhci_hcd: port resume event for port 3 [ 268.521168] xhci_hcd: xHC is not running. [ 268.521174] xhci_hcd: handle_port_status: starting port polling. [ 268.596322] xhci_hcd: xhci_hc_died: xHCI host controller not responding, assume dead The EINT lag is described in a additional note in xhci specs 4.19.2: "Due to internal xHC scheduling and system delays, there will be a lag between a change bit being set and the Port Status Change Event that it generated being written to the Event Ring. If SW reads the PORTSC and sees a change bit set, there is no guarantee that the corresponding Port Status Change Event has already been written into the Event Ring." Cc: Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 40 +++++++++++++++++++++++++++++++++++++--- drivers/usb/host/xhci.h | 4 ++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 8c8da2d..f11ec61 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -908,6 +908,41 @@ static void xhci_disable_port_wake_on_bits(struct xhci_hcd *xhci) spin_unlock_irqrestore(&xhci->lock, flags); } +static bool xhci_pending_portevent(struct xhci_hcd *xhci) +{ + struct xhci_port **ports; + int port_index; + u32 status; + u32 portsc; + + status = readl(&xhci->op_regs->status); + if (status & STS_EINT) + return true; + /* + * Checking STS_EINT is not enough as there is a lag between a change + * bit being set and the Port Status Change Event that it generated + * being written to the Event Ring. See note in xhci 1.1 section 4.19.2. + */ + + port_index = xhci->usb2_rhub.num_ports; + ports = xhci->usb2_rhub.ports; + while (port_index--) { + portsc = readl(ports[port_index]->addr); + if (portsc & PORT_CHANGE_MASK || + (portsc & PORT_PLS_MASK) == XDEV_RESUME) + return true; + } + port_index = xhci->usb3_rhub.num_ports; + ports = xhci->usb3_rhub.ports; + while (port_index--) { + portsc = readl(ports[port_index]->addr); + if (portsc & PORT_CHANGE_MASK || + (portsc & PORT_PLS_MASK) == XDEV_RESUME) + return true; + } + return false; +} + /* * Stop HC (not bus-specific) * @@ -1009,7 +1044,7 @@ EXPORT_SYMBOL_GPL(xhci_suspend); */ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) { - u32 command, temp = 0, status; + u32 command, temp = 0; struct usb_hcd *hcd = xhci_to_hcd(xhci); struct usb_hcd *secondary_hcd; int retval = 0; @@ -1134,8 +1169,7 @@ int xhci_resume(struct xhci_hcd *xhci, bool hibernated) done: if (retval == 0) { /* Resume root hubs only when have pending events. */ - status = readl(&xhci->op_regs->status); - if (status & STS_EINT) { + if (xhci_pending_portevent(xhci)) { usb_hcd_resume_root_hub(xhci->shared_hcd); usb_hcd_resume_root_hub(hcd); } diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 939e2f86..841e89f 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -382,6 +382,10 @@ struct xhci_op_regs { #define PORT_PLC (1 << 22) /* port configure error change - port failed to configure its link partner */ #define PORT_CEC (1 << 23) +#define PORT_CHANGE_MASK (PORT_CSC | PORT_PEC | PORT_WRC | PORT_OCC | \ + PORT_RC | PORT_PLC | PORT_CEC) + + /* Cold Attach Status - xHC can set this bit to report device attached during * Sx state. Warm port reset should be perfomed to clear this bit and move port * to connected state. -- 2.7.4 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: [1/5] xhci: Fix perceived dead host due to runtime suspend race with event handler From: Mathias Nyman Message-Id: <1529587185-2776-2-git-send-email-mathias.nyman@linux.intel.com> Date: Thu, 21 Jun 2018 16:19:41 +0300 To: gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org, Mathias Nyman , stable@vger.kernel.org List-ID: RG9uJ3QgcmVseSBvbiBldmVudCBpbnRlcnJ1cHQgKEVJTlQpIGJpdCBhbG9uZSB0byBkZXRlY3Qg cGVuZGluZyBwb3J0CmNoYW5nZSBpbiByZXN1bWUuIElmIG5vIGNoYW5nZSBldmVudCBpcyBkZXRl Y3RlZCB0aGUgaG9zdCBtYXkgYmUgc3VzcGVuZGVkCmFnYWluLCBvdGVyd2lzZSByb290aHVicyBh cmUgcmVzdW1lZC4KClRoZXJlIGlzIGEgbGFnIGluIHhIQyBzZXR0aW5nIEVJTlQuIElmIHdlIGRv bid0IG5vdGljZSB0aGUgcGVuZGluZyBjaGFuZ2UKaW4gcmVzdW1lLCBhbmQgdGhlIGNvbnRyb2xs ZXIgaXMgcnVudGltZSBzdXNwZWRlZCBhZ2FpbiwgaXQgY2F1c2VzIHRoZQpldmVudCBoYW5kbGVy IHRvIGFzc3VtZSBob3N0IGlzIGRlYWQgYXMgaXQgd2lsbCBmYWlsIHRvIHJlYWQgeEhDIHJlZ2lz dGVycwpvbmNlIFBDSSBwdXRzIHRoZSBjb250cm9sbGVyIHRvIEQzIHN0YXRlLgoKWyAgMjY4LjUy MDk2OV0geGhjaV9oY2Q6IHhoY2lfcmVzdW1lOiBzdGFydGluZyBwb3J0IHBvbGxpbmcuClsgIDI2 OC41MjA5ODVdIHhoY2lfaGNkOiB4aGNpX2h1Yl9zdGF0dXNfZGF0YTogc3RvcHBpbmcgcG9ydCBw b2xsaW5nLgpbICAyNjguNTIxMDMwXSB4aGNpX2hjZDogeGhjaV9zdXNwZW5kOiBzdG9wcGluZyBw b3J0IHBvbGxpbmcuClsgIDI2OC41MjEwNDBdIHhoY2lfaGNkOiAvLyBTZXR0aW5nIGNvbW1hbmQg cmluZyBhZGRyZXNzIHRvIDB4MzQ5YmQwMDEKWyAgMjY4LjUyMTEzOV0geGhjaV9oY2Q6IFBvcnQg U3RhdHVzIENoYW5nZSBFdmVudCBmb3IgcG9ydCAzClsgIDI2OC41MjExNDldIHhoY2lfaGNkOiBy ZXN1bWUgcm9vdCBodWIKWyAgMjY4LjUyMTE2M10geGhjaV9oY2Q6IHBvcnQgcmVzdW1lIGV2ZW50 IGZvciBwb3J0IDMKWyAgMjY4LjUyMTE2OF0geGhjaV9oY2Q6IHhIQyBpcyBub3QgcnVubmluZy4K WyAgMjY4LjUyMTE3NF0geGhjaV9oY2Q6IGhhbmRsZV9wb3J0X3N0YXR1czogc3RhcnRpbmcgcG9y dCBwb2xsaW5nLgpbICAyNjguNTk2MzIyXSB4aGNpX2hjZDogeGhjaV9oY19kaWVkOiB4SENJIGhv c3QgY29udHJvbGxlciBub3QgcmVzcG9uZGluZywgYXNzdW1lIGRlYWQKClRoZSBFSU5UIGxhZyBp cyBkZXNjcmliZWQgaW4gYSBhZGRpdGlvbmFsIG5vdGUgaW4geGhjaSBzcGVjcyA0LjE5LjI6Cgoi RHVlIHRvIGludGVybmFsIHhIQyBzY2hlZHVsaW5nIGFuZCBzeXN0ZW0gZGVsYXlzLCB0aGVyZSB3 aWxsIGJlIGEgbGFnCmJldHdlZW4gYSBjaGFuZ2UgYml0IGJlaW5nIHNldCBhbmQgdGhlIFBvcnQg U3RhdHVzIENoYW5nZSBFdmVudCB0aGF0IGl0CmdlbmVyYXRlZCBiZWluZyB3cml0dGVuIHRvIHRo ZSBFdmVudCBSaW5nLiBJZiBTVyByZWFkcyB0aGUgUE9SVFNDIGFuZApzZWVzIGEgY2hhbmdlIGJp dCBzZXQsIHRoZXJlIGlzIG5vIGd1YXJhbnRlZSB0aGF0IHRoZSBjb3JyZXNwb25kaW5nIFBvcnQK U3RhdHVzIENoYW5nZSBFdmVudCBoYXMgYWxyZWFkeSBiZWVuIHdyaXR0ZW4gaW50byB0aGUgRXZl bnQgUmluZy4iCgpDYzogPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ClNpZ25lZC1vZmYtYnk6IE1h dGhpYXMgTnltYW4gPG1hdGhpYXMubnltYW5AbGludXguaW50ZWwuY29tPgotLS0KIGRyaXZlcnMv dXNiL2hvc3QveGhjaS5jIHwgNDAgKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr Ky0tLQogZHJpdmVycy91c2IvaG9zdC94aGNpLmggfCAgNCArKysrCiAyIGZpbGVzIGNoYW5nZWQs IDQxIGluc2VydGlvbnMoKyksIDMgZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvZHJpdmVycy91 c2IvaG9zdC94aGNpLmMgYi9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuYwppbmRleCA4YzhkYTJkLi5m MTFlYzYxIDEwMDY0NAotLS0gYS9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuYworKysgYi9kcml2ZXJz L3VzYi9ob3N0L3hoY2kuYwpAQCAtOTA4LDYgKzkwOCw0MSBAQCBzdGF0aWMgdm9pZCB4aGNpX2Rp c2FibGVfcG9ydF93YWtlX29uX2JpdHMoc3RydWN0IHhoY2lfaGNkICp4aGNpKQogCXNwaW5fdW5s b2NrX2lycXJlc3RvcmUoJnhoY2ktPmxvY2ssIGZsYWdzKTsKIH0KIAorc3RhdGljIGJvb2wgeGhj aV9wZW5kaW5nX3BvcnRldmVudChzdHJ1Y3QgeGhjaV9oY2QgKnhoY2kpCit7CisJc3RydWN0IHho Y2lfcG9ydAkqKnBvcnRzOworCWludAkJCXBvcnRfaW5kZXg7CisJdTMyCQkJc3RhdHVzOworCXUz MgkJCXBvcnRzYzsKKworCXN0YXR1cyA9IHJlYWRsKCZ4aGNpLT5vcF9yZWdzLT5zdGF0dXMpOwor CWlmIChzdGF0dXMgJiBTVFNfRUlOVCkKKwkJcmV0dXJuIHRydWU7CisJLyoKKwkgKiBDaGVja2lu ZyBTVFNfRUlOVCBpcyBub3QgZW5vdWdoIGFzIHRoZXJlIGlzIGEgbGFnIGJldHdlZW4gYSBjaGFu Z2UKKwkgKiBiaXQgYmVpbmcgc2V0IGFuZCB0aGUgUG9ydCBTdGF0dXMgQ2hhbmdlIEV2ZW50IHRo YXQgaXQgZ2VuZXJhdGVkCisJICogYmVpbmcgd3JpdHRlbiB0byB0aGUgRXZlbnQgUmluZy4gU2Vl IG5vdGUgaW4geGhjaSAxLjEgc2VjdGlvbiA0LjE5LjIuCisJICovCisKKwlwb3J0X2luZGV4ID0g eGhjaS0+dXNiMl9yaHViLm51bV9wb3J0czsKKwlwb3J0cyA9IHhoY2ktPnVzYjJfcmh1Yi5wb3J0 czsKKwl3aGlsZSAocG9ydF9pbmRleC0tKSB7CisJCXBvcnRzYyA9IHJlYWRsKHBvcnRzW3BvcnRf aW5kZXhdLT5hZGRyKTsKKwkJaWYgKHBvcnRzYyAmIFBPUlRfQ0hBTkdFX01BU0sgfHwKKwkJICAg IChwb3J0c2MgJiBQT1JUX1BMU19NQVNLKSA9PSBYREVWX1JFU1VNRSkKKwkJCXJldHVybiB0cnVl OworCX0KKwlwb3J0X2luZGV4ID0geGhjaS0+dXNiM19yaHViLm51bV9wb3J0czsKKwlwb3J0cyA9 IHhoY2ktPnVzYjNfcmh1Yi5wb3J0czsKKwl3aGlsZSAocG9ydF9pbmRleC0tKSB7CisJCXBvcnRz YyA9IHJlYWRsKHBvcnRzW3BvcnRfaW5kZXhdLT5hZGRyKTsKKwkJaWYgKHBvcnRzYyAmIFBPUlRf Q0hBTkdFX01BU0sgfHwKKwkJICAgIChwb3J0c2MgJiBQT1JUX1BMU19NQVNLKSA9PSBYREVWX1JF U1VNRSkKKwkJCXJldHVybiB0cnVlOworCX0KKwlyZXR1cm4gZmFsc2U7Cit9CisKIC8qCiAgKiBT dG9wIEhDIChub3QgYnVzLXNwZWNpZmljKQogICoKQEAgLTEwMDksNyArMTA0NCw3IEBAIEVYUE9S VF9TWU1CT0xfR1BMKHhoY2lfc3VzcGVuZCk7CiAgKi8KIGludCB4aGNpX3Jlc3VtZShzdHJ1Y3Qg eGhjaV9oY2QgKnhoY2ksIGJvb2wgaGliZXJuYXRlZCkKIHsKLQl1MzIJCQljb21tYW5kLCB0ZW1w ID0gMCwgc3RhdHVzOworCXUzMgkJCWNvbW1hbmQsIHRlbXAgPSAwOwogCXN0cnVjdCB1c2JfaGNk CQkqaGNkID0geGhjaV90b19oY2QoeGhjaSk7CiAJc3RydWN0IHVzYl9oY2QJCSpzZWNvbmRhcnlf aGNkOwogCWludAkJCXJldHZhbCA9IDA7CkBAIC0xMTM0LDggKzExNjksNyBAQCBpbnQgeGhjaV9y ZXN1bWUoc3RydWN0IHhoY2lfaGNkICp4aGNpLCBib29sIGhpYmVybmF0ZWQpCiAgZG9uZToKIAlp ZiAocmV0dmFsID09IDApIHsKIAkJLyogUmVzdW1lIHJvb3QgaHVicyBvbmx5IHdoZW4gaGF2ZSBw ZW5kaW5nIGV2ZW50cy4gKi8KLQkJc3RhdHVzID0gcmVhZGwoJnhoY2ktPm9wX3JlZ3MtPnN0YXR1 cyk7Ci0JCWlmIChzdGF0dXMgJiBTVFNfRUlOVCkgeworCQlpZiAoeGhjaV9wZW5kaW5nX3BvcnRl dmVudCh4aGNpKSkgewogCQkJdXNiX2hjZF9yZXN1bWVfcm9vdF9odWIoeGhjaS0+c2hhcmVkX2hj ZCk7CiAJCQl1c2JfaGNkX3Jlc3VtZV9yb290X2h1YihoY2QpOwogCQl9CmRpZmYgLS1naXQgYS9k cml2ZXJzL3VzYi9ob3N0L3hoY2kuaCBiL2RyaXZlcnMvdXNiL2hvc3QveGhjaS5oCmluZGV4IDkz OWUyZjg2Li44NDFlODlmIDEwMDY0NAotLS0gYS9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuaAorKysg Yi9kcml2ZXJzL3VzYi9ob3N0L3hoY2kuaApAQCAtMzgyLDYgKzM4MiwxMCBAQCBzdHJ1Y3QgeGhj aV9vcF9yZWdzIHsKICNkZWZpbmUgUE9SVF9QTEMJKDEgPDwgMjIpCiAvKiBwb3J0IGNvbmZpZ3Vy ZSBlcnJvciBjaGFuZ2UgLSBwb3J0IGZhaWxlZCB0byBjb25maWd1cmUgaXRzIGxpbmsgcGFydG5l ciAqLwogI2RlZmluZSBQT1JUX0NFQwkoMSA8PCAyMykKKyNkZWZpbmUgUE9SVF9DSEFOR0VfTUFT SwkoUE9SVF9DU0MgfCBQT1JUX1BFQyB8IFBPUlRfV1JDIHwgUE9SVF9PQ0MgfCBcCisJCQkJIFBP UlRfUkMgfCBQT1JUX1BMQyB8IFBPUlRfQ0VDKQorCisKIC8qIENvbGQgQXR0YWNoIFN0YXR1cyAt IHhIQyBjYW4gc2V0IHRoaXMgYml0IHRvIHJlcG9ydCBkZXZpY2UgYXR0YWNoZWQgZHVyaW5nCiAg KiBTeCBzdGF0ZS4gV2FybSBwb3J0IHJlc2V0IHNob3VsZCBiZSBwZXJmb21lZCB0byBjbGVhciB0 aGlzIGJpdCBhbmQgbW92ZSBwb3J0CiAgKiB0byBjb25uZWN0ZWQgc3RhdGUuCg==