From patchwork Tue Mar 11 15:45:49 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 14012011 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1EA3C25E807 for ; Tue, 11 Mar 2025 15:44:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707893; cv=none; b=FLkVqsiRV43gk6uSqJpvCGvROwCZiVk2K1eGwgqSpsFos1r+OcjemuveB2PEAfSaKnC5ArnkNlFfkjjXvhbItWHcm5KoJW4Mdb/cF8YfRYos42fxA+xvAWTUiI0/YobHcekQaDpT3FRaSbC+fs0dVtzjxcfM+4/hVRltw99Xs24= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707893; c=relaxed/simple; bh=ZK8Z/ztxQnTD9D147oktvTcS8Xjx1IlJk/oVJ2QW0nc=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=sZ74JvBPBBpIOuUrXihaubOJseiCvKbFq6h1DB3odyZ6YhVfrpmbkTIhJynNWGt5FCQQ14qwzPbP1XqM+KV6qlgCmhqKhxZvOpVRBpBMk1gFkwE98gvqPuPqEz4b2lweTqzZVth57VB0IeaKYc+R0R3ffVLizx5r0Eu2LxdAj2U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=TTY0YxM7; arc=none smtp.client-ip=192.198.163.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="TTY0YxM7" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1741707892; x=1773243892; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=ZK8Z/ztxQnTD9D147oktvTcS8Xjx1IlJk/oVJ2QW0nc=; b=TTY0YxM7QssLwA3z+ulc+xLNxCFWJtQmlDAWxj6L5ZQuHkvDbC2me46W VwS9l3cqldMn26GAKgRdeAY13blPYZWQbfVOScdVJ6ZCts3lnfl2Iy6mF ZNtL3mbt578BURvFMj3UyqhmzgSsl7rq2GvQl5xzXdnEoOi19HSnRE8Oa f45G2KgYQwz5Tj47vwnGuOchafn1IOaQYe173KtXRPIOaP7vtrd71oaPE d9KYFCI07iAWnkqIG3XNdcJZDU3hyTtomDCErXjxk0H3zsKtSdlKHOxnH 64z0S8xBOKduJ5uir2NpaEg74DnWH7+IzYk7cUZlbf6PnK5ht1bebZRQ6 w==; X-CSE-ConnectionGUID: Va/hMT8cTWCjMqqR8RYuRQ== X-CSE-MsgGUID: banRDN/BQZu4vtca3qDTog== X-IronPort-AV: E=McAfee;i="6700,10204,11370"; a="30327898" X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="30327898" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Mar 2025 08:44:51 -0700 X-CSE-ConnectionGUID: rBrg1wtSR9aDsNuwiDGOGw== X-CSE-MsgGUID: 7ILL9i4ITQOVLoPwzYFrxg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="125396697" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by orviesa004.jf.intel.com with ESMTP; 11 Mar 2025 08:44:50 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , Michal Pecio Subject: [PATCH 1/3] xhci: Avoid queuing redundant Stop Endpoint command for stalled endpoint Date: Tue, 11 Mar 2025 17:45:49 +0200 Message-ID: <20250311154551.4035726-2-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> References: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 If EP_STALLED flag is set in xhci_urb_dequeue(), without EP_HALTED or SET_DEQ_PENDING flags, then the endpoint is in stopped state and the cancelled URB can be given back immediately withouth queueing a 'stop endpoint' command. Without this change the cancelled URB would eventually be given back in the 'context state error' completion path of the 'stop endpoint' command. This is not optimal. For this improvement to work the EP_STALLED flag must be cleared with xhci lock held. Suggested-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 1852175f48c1..19e308f4fc06 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1773,8 +1773,8 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } - /* In this case no commands are pending but the endpoint is stopped */ - if (ep->ep_state & EP_CLEARING_TT) { + /* In these cases no commands are pending but the endpoint is stopped */ + if (ep->ep_state & (EP_CLEARING_TT | EP_STALLED)) { /* and cancelled TDs can be given back right away */ xhci_dbg(xhci, "Invalidating TDs instantly on slot %d ep %d in state 0x%x\n", urb->dev->slot_id, ep_index, ep->ep_state); @@ -3211,10 +3211,12 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd, return; ep = &vdev->eps[ep_index]; + + spin_lock_irqsave(&xhci->lock, flags); + ep->ep_state &= ~EP_STALLED; /* Bail out if toggle is already being cleared by a endpoint reset */ - spin_lock_irqsave(&xhci->lock, flags); if (ep->ep_state & EP_HARD_CLEAR_TOGGLE) { ep->ep_state &= ~EP_HARD_CLEAR_TOGGLE; spin_unlock_irqrestore(&xhci->lock, flags); From patchwork Tue Mar 11 15:45:50 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 14012012 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 574E425E83A for ; Tue, 11 Mar 2025 15:44:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707895; cv=none; b=h4N7BKWmfo4F54PoR7EB6WLjHxeT+7qZDUQ4QqRQ8RwqXbqtjVXXLMNpXRfUXcIbhlo70eh7S8y5L1PAv1lTjvh59/OvG8oHQS7qva3JJD3FC+jBOE/ZI3jzMLbL8cDXKbjPGI/4NRZ1Sp7brWvO8CZ7JE/VuNgtPLwxHFhLrZs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707895; c=relaxed/simple; bh=wU0o/mz4ba52ZJLJ1FH+/Uo1Zt8koAcEWfIlZwhnimo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=ZRuw5Gs2MUUkYpAR9chzeJ5WdfIyS9ytryPs1nJaKohLytloxB3u3p68RJsYmVwgOY11zup8u9Diuut/wge1CFC3fZv0wipdKHIn2iUe94e1Fjds9rahGgqi/sXSYFZbW+dtx98c16kCBVOpzZJ/KU5CZ0DIeB9oajxQw/x23kI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=AT0ZOOGr; arc=none smtp.client-ip=192.198.163.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="AT0ZOOGr" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1741707893; x=1773243893; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=wU0o/mz4ba52ZJLJ1FH+/Uo1Zt8koAcEWfIlZwhnimo=; b=AT0ZOOGrJ2t6l0fkaX5VnWN1P2dhl9qfu5MlXxdz3qRWhVKXeu1X0a0P zTIFxbrs65dxtAC9mJtUn0lB7l0z+2gDNKtNJa77Z0d8wntLBVCSGaCnv yT9p3zocfYTrrTWnk10ItOxVb9/rxQq1AVdo8Bq9cwshubW7xwrcQrGDh Cnu35ruMzUviBjNe++ZLXxv+fCnZWd7tdAdm9iDaOfSXqjmkcf9PjQ21S oI3wDcnyjPSKNVeck6kRFvvmkLnfK5AoWi5ut/u7OftyBUR6NIjBvCzQ5 HJ5Ta4wZdfOJDPpac5n4jnrDN63LRG8kgfTc5XPU00c2VpdWz3OD9hufA g==; X-CSE-ConnectionGUID: k8f9psKdQxiqYdPk9cxAeg== X-CSE-MsgGUID: F31/Vl8nTres7iE8s+ssxQ== X-IronPort-AV: E=McAfee;i="6700,10204,11370"; a="30327902" X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="30327902" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Mar 2025 08:44:53 -0700 X-CSE-ConnectionGUID: gCQhknSqR7Gy6h9D45ltrA== X-CSE-MsgGUID: e/WGlIebSW+ttFX92oWkNw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="125396701" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by orviesa004.jf.intel.com with ESMTP; 11 Mar 2025 08:44:52 -0700 From: Mathias Nyman To: Cc: , Michal Pecio , Mathias Nyman Subject: [PATCH 2/3] usb: xhci: Don't change the status of stalled TDs on failed Stop EP Date: Tue, 11 Mar 2025 17:45:50 +0200 Message-ID: <20250311154551.4035726-3-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> References: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Michal Pecio When the device stalls an endpoint, current TD is assigned -EPIPE status and Reset Endpoint is queued. If a Stop Endpoint is pending at the time, it will run before Reset Endpoint and fail due to the stall. Its handler will change TD's status to -EPROTO before Reset Endpoint handler runs and initiates giveback. Check if the stall has already been handled and don't try to do it again. Since xhci_handle_halted_endpoint() performs this check too, not overwriting td->status is the only difference. I haven't seen this case yet, but I have seen a related one where the xHC has already executed Reset Endpoint, EP Context state is now Stopped and EP_HALTED is set. If the xHC took a bit longer to execute Reset Endpoint, said case would become this one. Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0f8acbb9cd21..303d66df271e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1220,7 +1220,14 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, */ switch (GET_EP_CTX_STATE(ep_ctx)) { case EP_STATE_HALTED: - xhci_dbg(xhci, "Stop ep completion raced with stall, reset ep\n"); + xhci_dbg(xhci, "Stop ep completion raced with stall\n"); + /* + * If the halt happened before Stop Endpoint failed, its transfer event + * should have already been handled and Reset Endpoint should be pending. + */ + if (ep->ep_state & EP_HALTED) + goto reset_done; + if (ep->ep_state & EP_HAS_STREAMS) { reset_type = EP_SOFT_RESET; } else { @@ -1231,8 +1238,11 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, } /* reset ep, reset handler cleans up cancelled tds */ err = xhci_handle_halted_endpoint(xhci, ep, td, reset_type); + xhci_dbg(xhci, "Stop ep completion resetting ep, status %d\n", err); if (err) break; +reset_done: + /* Reset EP handler will clean up cancelled TDs */ ep->ep_state &= ~EP_STOP_CMD_PENDING; return; case EP_STATE_STOPPED: From patchwork Tue Mar 11 15:45:51 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 14012013 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 02F9525E82C for ; Tue, 11 Mar 2025 15:44:54 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.16 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707896; cv=none; b=o5HGvDopYF/sTh4q5N0WYzAdfHNAfM879U1MXAbKgC8LiSdrE0auStlPD9t8ZTgBODrZYQqpB2snMFhItFsj6FLXybjftDJgJ1BKn6yHRSBKCvkdIe1ctzpmKIK/ztuUCukXramOeoCYap/rsb7K47/82pdyxuXd9wt+f1eCVzY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741707896; c=relaxed/simple; bh=gGLGqJ7cUb/dd9OgBck1p3Qpbvuhqs4s8+m6HjQPNDE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Qs1JuRiq9DZqk/Lj7ISPRygCMKF2cGQqiWmuLVE+0JviTxUjgE8xJnrsrQfhWaWjLZhQM8dJZ3xCl2nxBr728WK5SGzhMrJVtIf/r1g74tlmxYsR7knEjYw3YqqykpQSlyR3R1ZwmlkmsXQ+UKzBo6XS32+O1LHD4Uzki71+9nM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=none smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=h3bC0hG+; arc=none smtp.client-ip=192.198.163.16 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="h3bC0hG+" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1741707895; x=1773243895; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=gGLGqJ7cUb/dd9OgBck1p3Qpbvuhqs4s8+m6HjQPNDE=; b=h3bC0hG+ootOGtjwVJyOOJtMoK3H7Qr7fKXBDuwKOa3kX8aMBMSiapJ7 Q+A4pdLMxycgG6Cw7+oZYZ6SgOSVzvvETju2AGPDw/ZsKRMRM2swLkTOA BHT1YfMOgtQCHq9tjmoSgDLqunVclCOC6dlaYCw5sEfglcO/KQI1yAHkA x0tumExK3pshCSfeNifXc6woAOqSOtGdsmFmAgOAVJvRkMkhV16zfiVMG 1ZMfQJGj0QUbLbVAyIaLPxmK0QlcnQoYRma3sp1P9FQJE5lfEOxMbNnd7 bkHRNpjrsiA1RNQPXo106LPmrgM3Kyoc58t173sVVK80QKj8VPWyfNytl w==; X-CSE-ConnectionGUID: mHccSzCuSKaWyLiaoSFxxQ== X-CSE-MsgGUID: Oly+fOeWSpu/hNzltYwodw== X-IronPort-AV: E=McAfee;i="6700,10204,11370"; a="30327908" X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="30327908" Received: from orviesa004.jf.intel.com ([10.64.159.144]) by fmvoesa110.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Mar 2025 08:44:55 -0700 X-CSE-ConnectionGUID: CxoGEE06T0Or4oOyVe9yfw== X-CSE-MsgGUID: 8k1BvkLRRGu1vYtUNXWiVg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.14,239,1736841600"; d="scan'208";a="125396707" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by orviesa004.jf.intel.com with ESMTP; 11 Mar 2025 08:44:54 -0700 From: Mathias Nyman To: Cc: , Michal Pecio , Mathias Nyman Subject: [PATCH 3/3] usb: xhci: Avoid Stop Endpoint retry loop if the endpoint seems Running Date: Tue, 11 Mar 2025 17:45:51 +0200 Message-ID: <20250311154551.4035726-4-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> References: <20250311154551.4035726-1-mathias.nyman@linux.intel.com> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 From: Michal Pecio Nothing prevents a broken HC from claiming that an endpoint is Running and repeatedly rejecting Stop Endpoint with Context State Error. Avoid infinite retries and give back cancelled TDs. No such cases known so far, but HCs have bugs. Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 303d66df271e..5d64c297721c 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1264,16 +1264,19 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, * Stopped state, but it will soon change to Running. * * Assume this bug on unexpected Stop Endpoint failures. - * Keep retrying until the EP starts and stops again, on - * chips where this is known to help. Wait for 100ms. + * Keep retrying until the EP starts and stops again. */ - if (time_is_before_jiffies(ep->stop_time + msecs_to_jiffies(100))) - break; fallthrough; case EP_STATE_RUNNING: /* Race, HW handled stop ep cmd before ep was running */ xhci_dbg(xhci, "Stop ep completion ctx error, ctx_state %d\n", GET_EP_CTX_STATE(ep_ctx)); + /* + * Don't retry forever if we guessed wrong or a defective HC never starts + * the EP or says 'Running' but fails the command. We must give back TDs. + */ + if (time_is_before_jiffies(ep->stop_time + msecs_to_jiffies(100))) + break; command = xhci_alloc_command(xhci, false, GFP_ATOMIC); if (!command) {