From patchwork Thu Apr 10 15:18:23 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 880529 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) (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 DD6A31E5B62 for ; Thu, 10 Apr 2025 15:17:37 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298259; cv=none; b=VhIeIyKBxRwgv/IoZ2qVy2o2vy1wXjbFBPKVaQKBf9LqpKBzDS0cqIPBNP18mQwe+enPyQPxTMtDfNp3LpX0aP7dV7ZAzwF30wlipGjNaeCQb4fXEvzym9PdDd2hKfXuwO4BBiQKytfXXqT/HN7LiPjBkNXPcdC/14MgcqucvOg= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298259; c=relaxed/simple; bh=7X9K936NDdJI7VX8E6tVVtQM4qvXW1mkExk9bwY0gT8=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=hiKutCjGj1JKd/1OJbgDcTW9hy3wTQCqrYnAodEC3cN/zngTWOxrsiiT2lMzIGP4RZoME2tflGwLxLYW/nnyyv2Yg00b5Hw6At7VRguobJDLx6sp2RVlsWCWTxGBTQmNIXAHxpbRAzWFTJx3ZQMW6o7F1vCEYZaPO/a0UEQGeqE= 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=OLFD2fjH; arc=none smtp.client-ip=192.198.163.13 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="OLFD2fjH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1744298258; x=1775834258; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=7X9K936NDdJI7VX8E6tVVtQM4qvXW1mkExk9bwY0gT8=; b=OLFD2fjHsyVxD1cogN6bv5Fs17BI5KTSirJl7fhKfwpvYqvtcNMIJy9/ X+INXF4H2FJ6g1Liq+YFMBk8DMxfpNyAjZdqycgTOgY7yvp2DM+U0Pxg4 pF77KyAqM7+u44qZwIvdP8yLBi/xk0d0h7DnCfioQue1CH4AZ6BVybygu Drf87s4vgs4zoIKvGDIY288jXzzQAjQRbCUHZs5TKlaKJQah9Xv3uNUKB btPqmkI4ETLQ8IU4H2CnkOQulrqLhXDUk4/3dRkK3JSy8UG0z8ZjKljl0 u2Soti8Wna7NDI9E6BdUknNKFQwSYbun/j2nnciK+/zkk7DJmEtXBIiGA A==; X-CSE-ConnectionGUID: N65IwDDtTwqDY+457Lj/Pw== X-CSE-MsgGUID: l88NEllCT+G6z4aF3yrG+A== X-IronPort-AV: E=McAfee;i="6700,10204,11400"; a="48534962" X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="48534962" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2025 08:17:35 -0700 X-CSE-ConnectionGUID: H2R7GEmBSyKJ8vL09obsJA== X-CSE-MsgGUID: j9K0069pR8emPzPztjnwEQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="128913176" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by fmviesa007.fm.intel.com with ESMTP; 10 Apr 2025 08:17:33 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , Paul Menzel , Michal Pecio Subject: [PATCH 1/5] Revert "xhci: Avoid queuing redundant Stop Endpoint command for stalled endpoint" Date: Thu, 10 Apr 2025 18:18:23 +0300 Message-ID: <20250410151828.2868740-2-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250410151828.2868740-1-mathias.nyman@linux.intel.com> References: <20250410151828.2868740-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 This reverts commit 0c74d232578b1a7071e0312312811cb75b26b202. Paul Menzel reported that the two EP_STALLED patches in 6.15-rc1 cause regression. Turns out that the new flag may never get cleared after reset-resume, preventing xhci from restarting the endpoint. Revert this to take a proper look at it. Link: https://lore.kernel.org/linux-usb/84b400f8-2943-44e0-8803-f3aac3b670af@molgen.mpg.de cc: Paul Menzel cc: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 0452b8d65832..6370874bf265 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1770,8 +1770,8 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) goto done; } - /* In these cases no commands are pending but the endpoint is stopped */ - if (ep->ep_state & (EP_CLEARING_TT | EP_STALLED)) { + /* In this case no commands are pending but the endpoint is stopped */ + if (ep->ep_state & EP_CLEARING_TT) { /* 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); @@ -3208,12 +3208,10 @@ 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 Thu Apr 10 15:18:24 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 880112 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) (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 7FE83757F3 for ; Thu, 10 Apr 2025 15:17:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298260; cv=none; b=b/YQrNEFknSXA9OdxbeMbxQdEyHBmG9bxHZvjuk6qcrxhFEgpRzZtNMKyswSL9bWLXlehQO72jsRJlX9Vc6WZZmi2soZBC+yxDNKGeOopgnoEU6rUbl58pVFVRXgVmYBFAaKBR2Xawk2OSatDDBWtpuox+7l69gamVbcasqz+zU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298260; c=relaxed/simple; bh=5Yc21wueEndWZDDnB2dz3Zbd+I7CcrfZC6vxrfYhCsA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=fa6EDkPucL8990loXPya7odOM71wagj/SX8KMjedEXDpJAB7mx1Tt5vOkG4yX2rJNrGnnJoYhWNJrzQ8kY/kQV9g/3hXhHYpLcxW7Dy42idxYXV8CQzWC/Poj6N5qQ2Y+368Z5bNwfIkLnoKqcGj6sJ6KCujS5JkOLNkEruSEgk= 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=g79Yx1Sm; arc=none smtp.client-ip=192.198.163.13 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="g79Yx1Sm" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1744298258; x=1775834258; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=5Yc21wueEndWZDDnB2dz3Zbd+I7CcrfZC6vxrfYhCsA=; b=g79Yx1SmjCppcCb1745DWT7BOzDg9AWs3A/00ARAKbUsMm/IQkv7eq3r Lz3NqHxbylpYALLdIwl9iV2hYJo/JBlKMm/KKru74my23+cwlgfSc3zmK +Un3MjHBPtk/YZcDl7/JMfrbYyamXDPdThlp/5duoDGdYjk4uiH0pS88m y2s1RSyAGIkfZFplFFR3ttjgD5LOFaQvFVsBdh74QqRQsmiqD2ywUZDWh 7w060UKUS6xfD1XDQ7BuBXCAHUlbE6YdhmjbwOxYdVmjmx0nLxxAzphhM Z0ZITU+NriML7K5iJKFOJpa/SOi5KXuX3ywkCX6+yhibiKsRdKuy2L4jq A==; X-CSE-ConnectionGUID: 60bMseohTq2vJCWkwtPsIA== X-CSE-MsgGUID: PfT1ps4aRv6uUzgXVIs9wA== X-IronPort-AV: E=McAfee;i="6700,10204,11400"; a="48534970" X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="48534970" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2025 08:17:37 -0700 X-CSE-ConnectionGUID: MGc4wgZvQaSCqXiuDCB4Ew== X-CSE-MsgGUID: FyNEH+QMRVSqfzXxSmhgYA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="128913190" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by fmviesa007.fm.intel.com with ESMTP; 10 Apr 2025 08:17:35 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , Paul Menzel , Michal Pecio Subject: [PATCH 2/5] Revert "xhci: Prevent early endpoint restart when handling STALL errors." Date: Thu, 10 Apr 2025 18:18:24 +0300 Message-ID: <20250410151828.2868740-3-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250410151828.2868740-1-mathias.nyman@linux.intel.com> References: <20250410151828.2868740-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 This reverts commit 860f5d0d3594005d4588240028f42e8d2bfc725b. Paul Menzel reported that the two EP_STALLED patches in 6.15-rc1 cause regression. Turns out that the new flag may never get cleared after reset-resume, preventing xhci from restarting the endpoint. Revert this to take a proper look at it. Link: https://lore.kernel.org/linux-usb/84b400f8-2943-44e0-8803-f3aac3b670af@molgen.mpg.de cc: Paul Menzel cc: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 7 ++----- drivers/usb/host/xhci.c | 6 ------ drivers/usb/host/xhci.h | 3 +-- 3 files changed, 3 insertions(+), 13 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 5d64c297721c..94a2ae2c52e2 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -561,8 +561,8 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, * pointer command pending because the device can choose to start any * stream once the endpoint is on the HW schedule. */ - if (ep_state & (EP_STOP_CMD_PENDING | SET_DEQ_PENDING | EP_HALTED | - EP_CLEARING_TT | EP_STALLED)) + if ((ep_state & EP_STOP_CMD_PENDING) || (ep_state & SET_DEQ_PENDING) || + (ep_state & EP_HALTED) || (ep_state & EP_CLEARING_TT)) return; trace_xhci_ring_ep_doorbell(slot_id, DB_VALUE(ep_index, stream_id)); @@ -2573,9 +2573,6 @@ static void process_bulk_intr_td(struct xhci_hcd *xhci, struct xhci_virt_ep *ep, xhci_handle_halted_endpoint(xhci, ep, td, EP_SOFT_RESET); return; - case COMP_STALL_ERROR: - ep->ep_state |= EP_STALLED; - break; default: /* do nothing */ break; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 6370874bf265..ca390beda85b 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1605,11 +1605,6 @@ static int xhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag goto free_priv; } - /* Class driver might not be aware ep halted due to async URB giveback */ - if (*ep_state & EP_STALLED) - dev_dbg(&urb->dev->dev, "URB %p queued before clearing halt\n", - urb); - switch (usb_endpoint_type(&urb->ep->desc)) { case USB_ENDPOINT_XFER_CONTROL: @@ -3208,7 +3203,6 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd, return; ep = &vdev->eps[ep_index]; - ep->ep_state &= ~EP_STALLED; /* Bail out if toggle is already being cleared by a endpoint reset */ spin_lock_irqsave(&xhci->lock, flags); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 37860f1e3aba..28b6264f8b87 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -664,7 +664,7 @@ struct xhci_virt_ep { unsigned int err_count; unsigned int ep_state; #define SET_DEQ_PENDING (1 << 0) -#define EP_HALTED (1 << 1) /* Halted host ep handling */ +#define EP_HALTED (1 << 1) /* For stall handling */ #define EP_STOP_CMD_PENDING (1 << 2) /* For URB cancellation */ /* Transitioning the endpoint to using streams, don't enqueue URBs */ #define EP_GETTING_STREAMS (1 << 3) @@ -675,7 +675,6 @@ struct xhci_virt_ep { #define EP_SOFT_CLEAR_TOGGLE (1 << 7) /* usb_hub_clear_tt_buffer is in progress */ #define EP_CLEARING_TT (1 << 8) -#define EP_STALLED (1 << 9) /* For stall handling */ /* ---- Related to URB cancellation ---- */ struct list_head cancelled_td_list; struct xhci_hcd *xhci; From patchwork Thu Apr 10 15:18:25 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 880528 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) (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 E2C0B1E5B9F for ; Thu, 10 Apr 2025 15:17:39 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298262; cv=none; b=LmwVj7QA41LB5MlxiFUAnRbysEsRo3Tpu/uSTnj1y5oPkRcZy6RuvdVtx6EnyJSkKZRxnQWsCxIAzJX8rifv4mgMX5dIcbO/SFO/nx3kTKgiMAhdIGZE5YzIxzADb/76YIaA0+ZLglcXZ5zNN1UCbLWrD38+q94SkNulNKBR8+s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298262; c=relaxed/simple; bh=kWiYUW4oM5Uc9rLButl1omzs1blRdK1HKWAloBzuEL0=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=V9lf5DsS3+6XExyXaefXew+/2g8IBNCJyqWsThVrYDcZi0yaWmhRmCrLwIg52BWQrlRMk5ZMVUksD8BsQpBADjS9vm+d4unYi7DjfW+VUYc3OdZcNpNejUyKNAct0t78A5VDaefQDRWNGb/+jhjwzxfV3wm42niKwt+CQRvHL0Y= 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=nPR98IPj; arc=none smtp.client-ip=192.198.163.13 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="nPR98IPj" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1744298260; x=1775834260; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=kWiYUW4oM5Uc9rLButl1omzs1blRdK1HKWAloBzuEL0=; b=nPR98IPjCCMvaey7KUM0jYbFQJQORqY/InhXIakXJvNtL+xkmTl3qW+Y pPgYPpL0aQlN7n3j/XFJF9M3PVL6tToeHGpMZ73PwfaZqek1J0LS1Ir34 6ZWyc8AKam8u2J3TQnvF3Zqnn9Wfej11MAJtvxUghaScrRg7zZdyet9O7 OhGfBmXhaP0ka5xDRbHIGubWR3kWpEEhOQtPeh/o9zpRBt9xgcaGA4gDu +3lKuv99mQor5wqgYb11BYY42PpaEoDcak61rcc7OlEDNkGJiV58EmEgv xT+cmkJGNcCEHH3N8ODhWXw1QjxDFF3PjtEy5kVw2M3CFJ56WHuz/TlQI A==; X-CSE-ConnectionGUID: Qkk2wEVLRv6PymMXEwHXAQ== X-CSE-MsgGUID: p1CMuuduS5mBzMfv4++LFw== X-IronPort-AV: E=McAfee;i="6700,10204,11400"; a="48534979" X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="48534979" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2025 08:17:39 -0700 X-CSE-ConnectionGUID: OGnjUBg8T26ZIlrSJo928A== X-CSE-MsgGUID: 6R2hF353QZ2Yubex6cBb/g== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="128913202" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by fmviesa007.fm.intel.com with ESMTP; 10 Apr 2025 08:17:37 -0700 From: Mathias Nyman To: Cc: , Michal Pecio , Mathias Nyman Subject: [PATCH 3/5] usb: xhci: Fix Short Packet handling rework ignoring errors Date: Thu, 10 Apr 2025 18:18:25 +0300 Message-ID: <20250410151828.2868740-4-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250410151828.2868740-1-mathias.nyman@linux.intel.com> References: <20250410151828.2868740-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 A Short Packet event before the last TRB of a TD is followed by another event on the final TRB on spec-compliant HCs, which is most of them. A 'last_td_was_short' flag was added to know if a TD has just completed as Short Packet and another event is to come. The flag was cleared after seeing the event (unless no TDs are pending, but that's a separate bug) or seeing a new TD complete as something other than Short Packet. A rework replaced the flag with an 'old_trb_comp_code' variable. When an event doesn't match the pending TD and the previous event was Short Packet, the new event is silently ignored. To preserve old behavior, 'old_trb_comp_code' should be cleared at this point, but instead it is being set to current comp code, which is often Short Packet again. This can cause more events to be silently ignored, even though they are no longer connected with the old TD that completed short and indicate a serious problem with the driver or the xHC. Common device classes like UAC in async mode, UVC, serial or the UAS status pipe complete as Short Packet routinely and could be affected. Clear 'old_trb_comp_code' to zero, which is an invalid completion code and the same value the variable starts with. This restores original behavior on Short Packet and also works for illegal Etron events, which the code has been extended to cover too. Fixes: b331a3d8097f ("xhci: Handle spurious events on Etron host isoc enpoints") Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 94a2ae2c52e2..4e975caca235 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2913,7 +2913,7 @@ static int handle_tx_event(struct xhci_hcd *xhci, if (xhci_spurious_success_tx_event(xhci, ep_ring)) { xhci_dbg(xhci, "Spurious event dma %pad, comp_code %u after %u\n", &ep_trb_dma, trb_comp_code, ep_ring->old_trb_comp_code); - ep_ring->old_trb_comp_code = trb_comp_code; + ep_ring->old_trb_comp_code = 0; return 0; } From patchwork Thu Apr 10 15:18:26 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 880111 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) (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 50BC41E9B0F; Thu, 10 Apr 2025 15:17:41 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298262; cv=none; b=dvRXNqri5DXt0isrZL7/rmxjLd9U73sxJ1YAp2rg+UHTKF6fIenu0yo1lUskOM03l5Q8keRX3cAFTM9UnBcQ2h3owMLtt31MIG8fXOCFEkMnxAk0JFCJePY46tTAJ7QAP6lL8OXzED/elXfAQ1XvGh0Ps7XwrbxdtXYrDbaxWwI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298262; c=relaxed/simple; bh=ihgs8oQ5HV7KkFZRvPnbVcGuq5j4Sx5YNX3RvTm0kXs=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=Aq1EEcsNmpJGcwlaSakR/dXXg6L6C1bfGpuE5DhXZo+8JNjfrslMQw1BvgzmNk72a7vUCDn4Of1D5Jxv2mhq6PY9w+dwl95SgzI/92wEGGGfA0kXpvWFXLb2upTzaX2iQtr+2g75XtuQ9RzyV3l1RhC8buZt4LXAYLsVLIUHOgY= 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=QZEVxcV1; arc=none smtp.client-ip=192.198.163.13 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="QZEVxcV1" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1744298261; x=1775834261; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=ihgs8oQ5HV7KkFZRvPnbVcGuq5j4Sx5YNX3RvTm0kXs=; b=QZEVxcV13VRJ/9K8jUBWbslPeydjb3sQ27j9bqEjKG7NHdncDkHGTrfQ rffPz4afne8G6cXzqS96vKiABbDaJohXOsB105lgyGeDtLZwEf58ePxgF eEaCwc3rhfSPu030Ztt9eBxjrikQHDLIQsd8/MTYbqi5+Tnk2g57PnRIq 32isIWCCKGmr5HmfkBPv129CthEavaNO77qpGk9TtyzksPhcGeXKDmP6A qblu0dYqlmZmQl7kUcq05NLs/3BUx2L6LrqyjfLXaaRo3JQe8JBKiTaU3 T2G2++dbblXhicvP4j1+2p38mMYLmkVhh0Hu6rfk4xauSXaAkNloUYMWa A==; X-CSE-ConnectionGUID: H4XYRMzXTwaI5xyoOqoPow== X-CSE-MsgGUID: GNhFDYCOSE+UaIqTrm8DXw== X-IronPort-AV: E=McAfee;i="6700,10204,11400"; a="48534987" X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="48534987" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2025 08:17:40 -0700 X-CSE-ConnectionGUID: O69HrhR4Ssmcu14P6DdlvA== X-CSE-MsgGUID: UJ+nMm9zStydMmp2pb6oBQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="128913217" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by fmviesa007.fm.intel.com with ESMTP; 10 Apr 2025 08:17:39 -0700 From: Mathias Nyman To: Cc: , Michal Pecio , stable@vger.kernel.org, Mathias Nyman Subject: [PATCH 4/5] usb: xhci: Fix invalid pointer dereference in Etron workaround Date: Thu, 10 Apr 2025 18:18:26 +0300 Message-ID: <20250410151828.2868740-5-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250410151828.2868740-1-mathias.nyman@linux.intel.com> References: <20250410151828.2868740-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 This check is performed before prepare_transfer() and prepare_ring(), so enqueue can already point at the final link TRB of a segment. And indeed it will, some 0.4% of times this code is called. Then enqueue + 1 is an invalid pointer. It will crash the kernel right away or load some junk which may look like a link TRB and cause the real link TRB to be replaced with a NOOP. This wouldn't end well. Use a functionally equivalent test which doesn't dereference the pointer and always gives correct result. Something has crashed my machine twice in recent days while playing with an Etron HC, and a control transfer stress test ran for confirmation has just crashed it again. The same test passes with this patch applied. Fixes: 5e1c67abc930 ("xhci: Fix control transfer error on Etron xHCI host") Cc: stable@vger.kernel.org Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 4e975caca235..b906bc2eea5f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -3777,7 +3777,7 @@ int xhci_queue_ctrl_tx(struct xhci_hcd *xhci, gfp_t mem_flags, * enqueue a No Op TRB, this can prevent the Setup and Data Stage * TRB to be breaked by the Link TRB. */ - if (trb_is_link(ep_ring->enqueue + 1)) { + if (last_trb_on_seg(ep_ring->enq_seg, ep_ring->enqueue + 1)) { field = TRB_TYPE(TRB_TR_NOOP) | ep_ring->cycle_state; queue_trb(xhci, ep_ring, false, 0, 0, TRB_INTR_TARGET(0), field); From patchwork Thu Apr 10 15:18:27 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 880527 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.13]) (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 BEF341E9B2F; Thu, 10 Apr 2025 15:17:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.13 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298264; cv=none; b=tSIO8SreJ8bpzFR5VDKXiMUTB9anFbj3ZK6hTBKijpiH0HSUA0f0m/wywToRGgUePwzugrR3F72pMlP0mwSxl2RkITyiDtXQCJUWpXx2XWHe/SU11hiwITNfAWoIqA8x1jB2b3/soU12ghaLdNDr+BtIWjXzz6+IArhD59+WcrU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1744298264; c=relaxed/simple; bh=Jkt5Se38Pf/t3Pe9IkB7jcqZB9143Imd57ts1oxbTJY=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=K0ukA5iDcqlwtaGQZU42SO9mso4OftgQ8JjjutErueevQEZc+Dw7v+4Ha9+0TkKGTWXdthjsGmzX74eq2UjCCeNG2JRHetqGHbO6IelGgXrKeSLq7YhFyGdUBQhE1Ci82vWq9usXCa9W/g32Ub21L0vi/Izqk3tsTatdLQRkNRE= 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=Y7MWb65q; arc=none smtp.client-ip=192.198.163.13 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="Y7MWb65q" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1744298263; x=1775834263; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=Jkt5Se38Pf/t3Pe9IkB7jcqZB9143Imd57ts1oxbTJY=; b=Y7MWb65qGIC3L5yCF2XZKAecgBvUIAccXe/inFuBk9ZK+eXobeWrH9w1 0zEKWaFGymJMT1t6KNza4kQ7Y40ehg2tXmHY6wuuGnKrAO7diHRG5MA/E W/shaRnezN4+jaQg9HWfUsgy8xLguhWZTDkGm7CFfmCIx7VNxVKYvldhT 6ftE2kc7fMzLN8Qd/i2b1W8hpx3HSWe8DPEzBbE020AG92bWXfzv/9+dx cldrccZBUbB/pzuQ0f88GCcyYCEe+dReEWqCRKblIakDURNjMDMoHgvPC K24KAPy/mPsau+5n4acGu5fNSSZc4bmHOAXFOMsAZ48xUop8YPbKigSIm w==; X-CSE-ConnectionGUID: UZrGcBX+S0iqdpo3xrhE6g== X-CSE-MsgGUID: 9usRH7ctSrONTTGiVlHmjQ== X-IronPort-AV: E=McAfee;i="6700,10204,11400"; a="48534991" X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="48534991" Received: from fmviesa007.fm.intel.com ([10.60.135.147]) by fmvoesa107.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 10 Apr 2025 08:17:42 -0700 X-CSE-ConnectionGUID: j9Xu/5bnTHGGuYA58KhSRw== X-CSE-MsgGUID: i72DV4cdSrCsqtccQR/5dA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.15,202,1739865600"; d="scan'208";a="128913263" Received: from unknown (HELO mattu-haswell.fi.intel.com) ([10.237.72.199]) by fmviesa007.fm.intel.com with ESMTP; 10 Apr 2025 08:17:41 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , Devyn Liu , stable@vger.kernel.org Subject: [PATCH 5/5] xhci: Limit time spent with xHC interrupts disabled during bus resume Date: Thu, 10 Apr 2025 18:18:27 +0300 Message-ID: <20250410151828.2868740-6-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20250410151828.2868740-1-mathias.nyman@linux.intel.com> References: <20250410151828.2868740-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 Current xhci bus resume implementation prevents xHC host from generating interrupts during high-speed USB 2 and super-speed USB 3 bus resume. Only reason to disable interrupts during bus resume would be to prevent the interrupt handler from interfering with the resume process of USB 2 ports. Host initiated resume of USB 2 ports is done in two stages. The xhci driver first transitions the port from 'U3' to 'Resume' state, then wait in Resume for 20ms, and finally moves port to U0 state. xhci driver can't prevent interrupts by keeping the xhci spinlock due to this 20ms sleep. Limit interrupt disabling to the USB 2 port resume case only. resuming USB 2 ports in bus resume is only done in special cases where USB 2 ports had to be forced to suspend during bus suspend. The current way of preventing interrupts by clearing the 'Interrupt Enable' (INTE) bit in USBCMD register won't prevent the Interrupter registers 'Interrupt Pending' (IP), 'Event Handler Busy' (EHB) and USBSTS register Event Interrupt (EINT) bits from being set. New interrupts can't be issued before those bits are properly clered. Disable interrupts by clearing the interrupter register 'Interrupt Enable' (IE) bit instead. This way IP, EHB and INTE won't be set before IE is enabled again and a new interrupt is triggered. Reported-by: Devyn Liu Closes: https://lore.kernel.org/linux-usb/b1a9e2d51b4d4ff7a304f77c5be8164e@huawei.com/ Cc: stable@vger.kernel.org Tested-by: Devyn Liu Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-hub.c | 30 ++++++++++++++++-------------- drivers/usb/host/xhci.c | 4 ++-- drivers/usb/host/xhci.h | 2 ++ 3 files changed, 20 insertions(+), 16 deletions(-) diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c index c0f226584a40..486347776cb2 100644 --- a/drivers/usb/host/xhci-hub.c +++ b/drivers/usb/host/xhci-hub.c @@ -1878,9 +1878,10 @@ int xhci_bus_resume(struct usb_hcd *hcd) int max_ports, port_index; int sret; u32 next_state; - u32 temp, portsc; + u32 portsc; struct xhci_hub *rhub; struct xhci_port **ports; + bool disabled_irq = false; rhub = xhci_get_rhub(hcd); ports = rhub->ports; @@ -1896,17 +1897,20 @@ int xhci_bus_resume(struct usb_hcd *hcd) return -ESHUTDOWN; } - /* delay the irqs */ - temp = readl(&xhci->op_regs->command); - temp &= ~CMD_EIE; - writel(temp, &xhci->op_regs->command); - /* bus specific resume for ports we suspended at bus_suspend */ - if (hcd->speed >= HCD_USB3) + if (hcd->speed >= HCD_USB3) { next_state = XDEV_U0; - else + } else { next_state = XDEV_RESUME; - + if (bus_state->bus_suspended) { + /* + * prevent port event interrupts from interfering + * with usb2 port resume process + */ + xhci_disable_interrupter(xhci->interrupters[0]); + disabled_irq = true; + } + } port_index = max_ports; while (port_index--) { portsc = readl(ports[port_index]->addr); @@ -1974,11 +1978,9 @@ int xhci_bus_resume(struct usb_hcd *hcd) (void) readl(&xhci->op_regs->command); bus_state->next_statechange = jiffies + msecs_to_jiffies(5); - /* re-enable irqs */ - temp = readl(&xhci->op_regs->command); - temp |= CMD_EIE; - writel(temp, &xhci->op_regs->command); - temp = readl(&xhci->op_regs->command); + /* re-enable interrupter */ + if (disabled_irq) + xhci_enable_interrupter(xhci->interrupters[0]); spin_unlock_irqrestore(&xhci->lock, flags); return 0; diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index ca390beda85b..90eb491267b5 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -322,7 +322,7 @@ static void xhci_zero_64b_regs(struct xhci_hcd *xhci) xhci_info(xhci, "Fault detected\n"); } -static int xhci_enable_interrupter(struct xhci_interrupter *ir) +int xhci_enable_interrupter(struct xhci_interrupter *ir) { u32 iman; @@ -335,7 +335,7 @@ static int xhci_enable_interrupter(struct xhci_interrupter *ir) return 0; } -static int xhci_disable_interrupter(struct xhci_interrupter *ir) +int xhci_disable_interrupter(struct xhci_interrupter *ir) { u32 iman; diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 28b6264f8b87..242ab9fbc8ae 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1890,6 +1890,8 @@ int xhci_alloc_tt_info(struct xhci_hcd *xhci, struct usb_tt *tt, gfp_t mem_flags); int xhci_set_interrupter_moderation(struct xhci_interrupter *ir, u32 imod_interval); +int xhci_enable_interrupter(struct xhci_interrupter *ir); +int xhci_disable_interrupter(struct xhci_interrupter *ir); /* xHCI ring, segment, TRB, and TD functions */ dma_addr_t xhci_trb_virt_to_dma(struct xhci_segment *seg, union xhci_trb *trb);