From patchwork Wed Oct 16 13:59:57 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 836614 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) (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 B5235209F29; Wed, 16 Oct 2024 13:57:59 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.15 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087081; cv=none; b=tSWz4PBlePjh6k1BJJ6oWYnAHyM889dcSXL7AyV8tgU7tHI62vbHoHKonP/2aL0XDPkCIESJD02CX7BMdjMkbL7RDHS0U9R+bTMn+ABg9wiHuFHvXOx+o5KaN4rDe9ZngmYlsB9haDrXzFC416Xqr/Wk4+FwFkrwoHhAYjT2fFs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087081; c=relaxed/simple; bh=n1n94pBrX90e8RweJy1ueHuQ21nejD4b5AoXSQNpog4=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=rhIfEExQEoZXszfmIRXQbS8nSQNhbubCqu6EIvaUM9CPYf8HQW6C5Fa2a610fNzumq5tKq/8JQuFpJeKxVZOqIsMzfFQs2zWaazBIyvyqZ8d+iT1zYUqUyppWwrx86Q4TM2QEbAHdvJsmQ7hszRI11eOSKoYLdYmqs4Xrp11wHI= 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=QU/GlYuJ; arc=none smtp.client-ip=192.198.163.15 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="QU/GlYuJ" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729087080; x=1760623080; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=n1n94pBrX90e8RweJy1ueHuQ21nejD4b5AoXSQNpog4=; b=QU/GlYuJ+8BgamiIw1WpsqmDn2JpH7ewwTT4Vmk8k2vjNFdlkeAk6xdI Z+bYPAOFMHPpEQVqZNriWgcCo/e0g7asgQhN6jTtrgiBArffYeBHH1dvh ZjplJt5vyCp9vnu6EruOUoKdp2xyfvmHyLXurmpCE67aKKHxcSXD3Qa5/ sCs2NS/Qjkp2KWaQE8tKhTo9GHSEVsxuH5Et89+ZuX5oxlnAtdv/FWoDe ealPafQ9vYLWUxkdmePbi0Psr8wviB8gKleG4BmZn/RCDJrlzPQErvBmb yQ6tiGc8UwBJB3PwGrpX6ZC4N0deno2dkM+54qonw0KWPrOQq4kdRoQLJ A==; X-CSE-ConnectionGUID: 70aVMG2lQxCvGECIjdHN0A== X-CSE-MsgGUID: +v7HcHs3RN+HLuBaoOM5VQ== X-IronPort-AV: E=McAfee;i="6700,10204,11226"; a="28664019" X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="28664019" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2024 06:57:59 -0700 X-CSE-ConnectionGUID: yCcYBNNeRZmWj7G8nKLbrg== X-CSE-MsgGUID: 0Uoz+i7dTAKQfxmDghbbtg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="82776216" Received: from mattu-haswell.fi.intel.com ([10.237.72.199]) by fmviesa005.fm.intel.com with ESMTP; 16 Oct 2024 06:57:58 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , stable@vger.kernel.org Subject: [PATCH 1/4] xhci: Fix incorrect stream context type macro Date: Wed, 16 Oct 2024 16:59:57 +0300 Message-Id: <20241016140000.783905-2-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20241016140000.783905-1-mathias.nyman@linux.intel.com> References: <20241016140000.783905-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 The stream contex type (SCT) bitfield is used both in the stream context data structure, and in the 'Set TR Dequeue pointer' command TRB. In both cases it uses bits 3:1 The SCT_FOR_TRB(p) macro used to set the stream context type (SCT) field for the 'Set TR Dequeue pointer' command TRB incorrectly shifts the value 1 bit left before masking the three bits. Fix this by first masking and rshifting, just like the similar SCT_FOR_CTX(p) macro does This issue has not been visibile as the lost bit 3 is only used with secondary stream arrays (SSA). Xhci driver currently only supports using a primary stream array with Linear stream addressing. Fixes: 95241dbdf828 ("xhci: Set SCT field for Set TR dequeue on streams") Cc: stable@vger.kernel.org Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 620502de971a..f0fb696d5619 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -1001,7 +1001,7 @@ enum xhci_setup_dev { /* Set TR Dequeue Pointer command TRB fields, 6.4.3.9 */ #define TRB_TO_STREAM_ID(p) ((((p) & (0xffff << 16)) >> 16)) #define STREAM_ID_FOR_TRB(p) ((((p)) & 0xffff) << 16) -#define SCT_FOR_TRB(p) (((p) << 1) & 0x7) +#define SCT_FOR_TRB(p) (((p) & 0x7) << 1) /* Link TRB specific fields */ #define TRB_TC (1<<1) From patchwork Wed Oct 16 13:59:58 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 836115 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) (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 04D59207206; Wed, 16 Oct 2024 13:58:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.15 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087082; cv=none; b=ls/ZPUHyLE4nwp7sMXTPT1GwV0pMro23cHcflRH3AhvlGkP7YawB8GWBOpDu32ybConcULhoGgUnSvRwjArsfpMsQTURUBx0AupXtEhA+j/ZvsLrgrTtoP6Hu8leGq79MDHehrxegMl7z1t3YAdAKTrPIFbIR1+l/nr0dwhzXeM= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087082; c=relaxed/simple; bh=lu1/EiX2eFn2V18PLdd/sCqnm3qRuo9tYRRBCnxq8vY=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=LUIuk6Oep+Wcbb/8INqV7A8+ANuEtYlZHPyCUOa+bRuKcLdI02cTS/juMnTwuSt49HBFk9JY7QzZwNJu95hxAyPdjsRaB64z8hqkI46SuvvSl/lwqBbW70uNbKdPAXAAmyduUuzvHwbycqoJX9UV8tHo07qFXngGXK9hhZsGpLM= 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=FWL1oUAA; arc=none smtp.client-ip=192.198.163.15 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="FWL1oUAA" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729087081; x=1760623081; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=lu1/EiX2eFn2V18PLdd/sCqnm3qRuo9tYRRBCnxq8vY=; b=FWL1oUAA9Y6BIevY2ANxZawtJ1v1AHbx+UbnpcVUqLcyp2MVIeZUEM2U 2A7Xex1iCCWAu8+Rk5CAlQGU2mo2s6r3yDXcKEWVH0N0KeExMNgJbrJsb nZFq4L3KDiTRB5Sj9UQMkL2Mgj41SqJ0+vxniRuClbH+hEI91cPkQkz3i 54FUXj0w97fuu8ZpHUtgc2OzVkrjaXIrvqq6sdsrM8Rl2lI7r7nBVhIG3 7Yej7qcXrmrLWhYyhr+TqA9PxhaPbZ8yF9a+/hX81nePhqXOOrUbSDiWQ D/ztiJ5qQWSdaO3YEJ2cnM2fHhC7E74NvmZ9fJ3JUJuEuKYisn7T03KVx w==; X-CSE-ConnectionGUID: MNGT44DrQTKm+xJNve/eGA== X-CSE-MsgGUID: vmTu3H/ITPyncVZMrgh/Rw== X-IronPort-AV: E=McAfee;i="6700,10204,11226"; a="28664023" X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="28664023" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2024 06:58:01 -0700 X-CSE-ConnectionGUID: BkwzXJvVTiKVDy8DCQwRTA== X-CSE-MsgGUID: PqvBrWqHSe+0RRzwnDMltA== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="82776218" Received: from mattu-haswell.fi.intel.com ([10.237.72.199]) by fmviesa005.fm.intel.com with ESMTP; 16 Oct 2024 06:57:59 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , stable@vger.kernel.org Subject: [PATCH 2/4] xhci: Mitigate failed set dequeue pointer commands Date: Wed, 16 Oct 2024 16:59:58 +0300 Message-Id: <20241016140000.783905-3-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20241016140000.783905-1-mathias.nyman@linux.intel.com> References: <20241016140000.783905-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 Avoid xHC host from processing a cancelled URB by always turning cancelled URB TDs into no-op TRBs before queuing a 'Set TR Deq' command. If the command fails then xHC will start processing the cancelled TD instead of skipping it once endpoint is restarted, causing issues like Babble error. This is not a complete solution as a failed 'Set TR Deq' command does not guarantee xHC TRB caches are cleared. Fixes: 4db356924a50 ("xhci: turn cancelled td cleanup to its own function") Cc: stable@vger.kernel.org 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 4d664ba53fe9..7dedf31bbddd 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1023,7 +1023,7 @@ static int xhci_invalidate_cancelled_tds(struct xhci_virt_ep *ep) td_to_noop(xhci, ring, cached_td, false); cached_td->cancel_status = TD_CLEARED; } - + td_to_noop(xhci, ring, td, false); td->cancel_status = TD_CLEARING_CACHE; cached_td = td; break; From patchwork Wed Oct 16 13:59:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 836613 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) (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 4A87120C025 for ; Wed, 16 Oct 2024 13:58:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.15 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087084; cv=none; b=h/aEgjx3+96TfHQt9NotsHo7eW7oz6OrRLJoF1KCsek9RcZj9Fm1fVVJIO4WlwqQqM4xgLyKJwyb4DLRJfWZe91beSy//8ZKhJM98sgvHp9IDznZyRwad3PTmIMUGSOawENZ20ifAotNgWUWIGix+4ysRIjUTbjF4wGCfgn5QYc= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087084; c=relaxed/simple; bh=PpLVJHCs/Lsa4JXqIq9mfDwKCRL9WHKJbDQ+XqqtlOY=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=Pl0nWKXgtXYiJUc9wlGSr71mcnQEGeK6ITflbsaRFCUplCpvrFGXRuNHls+wLatK9uUG0QeFe9LKC/rqPgBLNZeoAB6IQ3j9kdWuUPjYnXQ8Xj0yZ4wvGMQRKEOGOFyUggrpwo2HFLDYfTIYRUgeCZwoIJ+88UdMiQkTb/AxC3M= 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=QPaJ75P9; arc=none smtp.client-ip=192.198.163.15 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="QPaJ75P9" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729087083; x=1760623083; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=PpLVJHCs/Lsa4JXqIq9mfDwKCRL9WHKJbDQ+XqqtlOY=; b=QPaJ75P9jHSFAIx72I33x/Btvju06ARrddC4eveZplC/xr62SRgllXW6 bikpjqKdizTIpljR9qOZnNbG/Xb56R3Kii4w+nnwGIVUDyBui4cgUuuKf mKUy8H6QOZoyNn1uDGEoiuTOojFvuyL3hhJU1uiYwgCR8esFQx2e6VG41 S3ABMhHUAB/V2sKNHOMfC+1mMux7udE+fmfTpz1uDxxAJLan6fN6wtSt4 qPRGa9Tzr2PHhZFzAQ6KL0TKxp6P3a2lE3QL7DslpbNSspk4fe809dg4b op2DbYsBpqnwGYgNKG4KxqZueiHtRDeKj/Ca4mbh1IGp9Vezoe3xkTK0o g==; X-CSE-ConnectionGUID: XsYf4EgjSuqmelg+7cuIBg== X-CSE-MsgGUID: r5yo7d8hT4SSH7EASAd9gg== X-IronPort-AV: E=McAfee;i="6700,10204,11226"; a="28664028" X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="28664028" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2024 06:58:02 -0700 X-CSE-ConnectionGUID: y0YE+u3YTgqA0qYJePdoMg== X-CSE-MsgGUID: F5zpf+4GQ+udnP5Cqe1kjw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="82776224" Received: from mattu-haswell.fi.intel.com ([10.237.72.199]) by fmviesa005.fm.intel.com with ESMTP; 16 Oct 2024 06:58:01 -0700 From: Mathias Nyman To: Cc: , Michal Pecio , Mathias Nyman Subject: [PATCH 3/4] usb: xhci: Fix handling errors mid TD followed by other errors Date: Wed, 16 Oct 2024 16:59:59 +0300 Message-Id: <20241016140000.783905-4-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20241016140000.783905-1-mathias.nyman@linux.intel.com> References: <20241016140000.783905-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 Some host controllers fail to produce the final completion event on an isochronous TD which experienced an error mid TD. We deal with it by flagging such TDs and checking if the next event points at the flagged TD or at the next one, and giving back the flagged TD if the latter. This is not enough, because the next TD may be missed by the xHC. Or there may be no next TD but a ring underrun. We also need to get such TD quickly out of the way, or errors on later TDs may be handled wrong. If the next TD experiences a Missed Service Error, we will set the skip flag on the endpoint and then attempt skipping TDs when yet another event arrives. In such scenario, we ought to report the 'error mid TD' transfer as such rather than skip it. Another problem case are Stopped events. If we see one after an error mid TD, we naively assume that it's a Force Stopped Event because it doesn't match the pending TD, but in reality it might be an ordinary Stopped event for the next TD, which we fail to recognize and handle. Fix this by moving error mid TD handling before the whole TD skipping loop. Remove unnecessary conditions, always give back the TD if the new event points to any TRB outside it or if the pointer is NULL, as may be the case in Ring Underrun and Overrun events on 1st gen hardware. Only if the pending TD isn't flagged, consider other actions like skipping. As a side effect of reordering with skip and FSE cases, error mid TD is reordered with last_td_was_short check. This is harmless, because the two cases are mutually exclusive - only one can happen in any given run of handle_tx_event(). Tested on the NEC host and a USB camera with flaky cable. Dynamic debug confirmed that Transaction Errors are sometimes seen, sometimes mid-TD, sometimes followed by Missed Service. In such cases, they were finished properly before skipping began. [Rebase on 6.12-rc1 -Mathias] Signed-off-by: Michal Pecio Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-ring.c | 66 ++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 37 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 7dedf31bbddd..b6eb928e260f 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -2775,6 +2775,29 @@ static int handle_tx_event(struct xhci_hcd *xhci, return 0; } + /* + * xhci 4.10.2 states isoc endpoints should continue + * processing the next TD if there was an error mid TD. + * So host like NEC don't generate an event for the last + * isoc TRB even if the IOC flag is set. + * xhci 4.9.1 states that if there are errors in mult-TRB + * TDs xHC should generate an error for that TRB, and if xHC + * proceeds to the next TD it should genete an event for + * any TRB with IOC flag on the way. Other host follow this. + * + * We wait for the final IOC event, but if we get an event + * anywhere outside this TD, just give it back already. + */ + td = list_first_entry_or_null(&ep_ring->td_list, struct xhci_td, td_list); + + if (td && td->error_mid_td && !trb_in_td(xhci, td, ep_trb_dma, false)) { + xhci_dbg(xhci, "Missing TD completion event after mid TD error\n"); + ep_ring->dequeue = td->last_trb; + ep_ring->deq_seg = td->last_trb_seg; + inc_deq(xhci, ep_ring); + xhci_td_cleanup(xhci, td, ep_ring, td->status); + } + if (list_empty(&ep_ring->td_list)) { /* * Don't print wanings if ring is empty due to a stopped endpoint generating an @@ -2836,44 +2859,13 @@ static int handle_tx_event(struct xhci_hcd *xhci, return 0; } - /* - * xhci 4.10.2 states isoc endpoints should continue - * processing the next TD if there was an error mid TD. - * So host like NEC don't generate an event for the last - * isoc TRB even if the IOC flag is set. - * xhci 4.9.1 states that if there are errors in mult-TRB - * TDs xHC should generate an error for that TRB, and if xHC - * proceeds to the next TD it should genete an event for - * any TRB with IOC flag on the way. Other host follow this. - * So this event might be for the next TD. - */ - if (td->error_mid_td && - !list_is_last(&td->td_list, &ep_ring->td_list)) { - struct xhci_td *td_next = list_next_entry(td, td_list); - - ep_seg = trb_in_td(xhci, td_next, ep_trb_dma, false); - if (ep_seg) { - /* give back previous TD, start handling new */ - xhci_dbg(xhci, "Missing TD completion event after mid TD error\n"); - ep_ring->dequeue = td->last_trb; - ep_ring->deq_seg = td->last_trb_seg; - inc_deq(xhci, ep_ring); - xhci_td_cleanup(xhci, td, ep_ring, td->status); - td = td_next; - } - } - - if (!ep_seg) { - /* HC is busted, give up! */ - xhci_err(xhci, - "ERROR Transfer event TRB DMA ptr not " - "part of current TD ep_index %d " - "comp_code %u\n", ep_index, - trb_comp_code); - trb_in_td(xhci, td, ep_trb_dma, true); + /* HC is busted, give up! */ + xhci_err(xhci, + "ERROR Transfer event TRB DMA ptr not part of current TD ep_index %d comp_code %u\n", + ep_index, trb_comp_code); + trb_in_td(xhci, td, ep_trb_dma, true); - return -ESHUTDOWN; - } + return -ESHUTDOWN; } if (ep->skip) { From patchwork Wed Oct 16 14:00:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Mathias Nyman X-Patchwork-Id: 836114 Received: from mgamail.intel.com (mgamail.intel.com [192.198.163.15]) (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 1097D20C000; Wed, 16 Oct 2024 13:58:04 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=192.198.163.15 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087086; cv=none; b=IisAmP10QM76Sn3KrkVJV5U/YOcmas185Dv6m9yPXdba8/V/9zDWR6YzHZrHouVYh+hmEebxeyF9iMsK1OGWhx9b47tZxP45OZMysaYrUz8D7sU+44zs6HFGxD1vbzkq/59Z7xmI6hNJ3mKxwfuovCsqTrU4eSTFswpVxyA0O1I= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1729087086; c=relaxed/simple; bh=CNhKFwL3hrGpnciMqwCTYZJcQC1POF4XejApSmQzBg4=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version:Content-Type; b=WpBYsoviG1BQiVPTIZHf5C+/Qix9803UbBaLkV3FBpzocLuEz8r1XHL1adKf9trFTrVvGoWDk33i14qbwNipsGJfpqGoM1Sg29V/ctzLOgm0/7SINNvEy2e3ByGzAqOhMXUQ/PrKKjnj9G3NHWHzWChOWAsCOk43QgSVKqdjzlU= 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=Ti4ZmQVl; arc=none smtp.client-ip=192.198.163.15 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="Ti4ZmQVl" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1729087085; x=1760623085; h=from:to:cc:subject:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=CNhKFwL3hrGpnciMqwCTYZJcQC1POF4XejApSmQzBg4=; b=Ti4ZmQVlbOGWLNF2meVcjTeLREwDtYG6VRW6ah5uLk1ybFsvvfKAFvJj eXwbg0sCzHAofQ5D9pZEgpYdbqG9Idy59zgbPOOy3Dd5qpI67cIqB6qXC MK7mpIXt1rM8GGB2U23AfInYFTySpSGKVnUdbStLqxdbzz6d4Ss8iOqcN w+KLgCeS8azZmCR2Izom0tFkvzg2mgCv88feXW6jwp7j+/wxTMZYqUpsE XHRKBPVA0H8SMeAJimhYXpXkenr202ojLsdIIIlwGiuAcUtkNRb/T80iE GkVFWLh5hd74I9VqYR199Fatv+LikNLQ4Snq6VvcrBU5o/ICyLbFpph+6 w==; X-CSE-ConnectionGUID: +xFbyDECRNOG0FJnkbaDug== X-CSE-MsgGUID: M+47JckCQkehGUiRe/UmPQ== X-IronPort-AV: E=McAfee;i="6700,10204,11226"; a="28664031" X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="28664031" Received: from fmviesa005.fm.intel.com ([10.60.135.145]) by fmvoesa109.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 16 Oct 2024 06:58:04 -0700 X-CSE-ConnectionGUID: 8fqjvmOfT06QbzPHiOM3bA== X-CSE-MsgGUID: xxuISZWUTZ+WQRp/S8w9gw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.11,208,1725346800"; d="scan'208";a="82776245" Received: from mattu-haswell.fi.intel.com ([10.237.72.199]) by fmviesa005.fm.intel.com with ESMTP; 16 Oct 2024 06:58:02 -0700 From: Mathias Nyman To: Cc: , Mathias Nyman , Uday M Bhat , =?utf-8?q?=C5=81ukasz_Bartosik?= , stable@vger.kernel.org Subject: [PATCH 4/4] xhci: dbc: honor usb transfer size boundaries. Date: Wed, 16 Oct 2024 17:00:00 +0300 Message-Id: <20241016140000.783905-5-mathias.nyman@linux.intel.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20241016140000.783905-1-mathias.nyman@linux.intel.com> References: <20241016140000.783905-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 Treat each completed full size write to /dev/ttyDBC0 as a separate usb transfer. Make sure the size of the TRBs matches the size of the tty write by first queuing as many max packet size TRBs as possible up to the last TRB which will be cut short to match the size of the tty write. This solves an issue where userspace writes several transfers back to back via /dev/ttyDBC0 into a kfifo before dbgtty can find available request to turn that kfifo data into TRBs on the transfer ring. The boundary between transfer was lost as xhci-dbgtty then turned everyting in the kfifo into as many 'max packet size' TRBs as possible. DbC would then send more data to the host than intended for that transfer, causing host to issue a babble error. Refuse to write more data to kfifo until previous tty write data is turned into properly sized TRBs with data size boundaries matching tty write size Tested-by: Uday M Bhat Tested-by: Łukasz Bartosik Cc: Signed-off-by: Mathias Nyman --- drivers/usb/host/xhci-dbgcap.h | 1 + drivers/usb/host/xhci-dbgtty.c | 55 ++++++++++++++++++++++++++++++---- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/drivers/usb/host/xhci-dbgcap.h b/drivers/usb/host/xhci-dbgcap.h index 8ec813b6e9fd..9dc8f4d8077c 100644 --- a/drivers/usb/host/xhci-dbgcap.h +++ b/drivers/usb/host/xhci-dbgcap.h @@ -110,6 +110,7 @@ struct dbc_port { struct tasklet_struct push; struct list_head write_pool; + unsigned int tx_boundary; bool registered; }; diff --git a/drivers/usb/host/xhci-dbgtty.c b/drivers/usb/host/xhci-dbgtty.c index b8e78867e25a..d719c16ea30b 100644 --- a/drivers/usb/host/xhci-dbgtty.c +++ b/drivers/usb/host/xhci-dbgtty.c @@ -24,6 +24,29 @@ static inline struct dbc_port *dbc_to_port(struct xhci_dbc *dbc) return dbc->priv; } +static unsigned int +dbc_kfifo_to_req(struct dbc_port *port, char *packet) +{ + unsigned int len; + + len = kfifo_len(&port->port.xmit_fifo); + + if (len == 0) + return 0; + + len = min(len, DBC_MAX_PACKET); + + if (port->tx_boundary) + len = min(port->tx_boundary, len); + + len = kfifo_out(&port->port.xmit_fifo, packet, len); + + if (port->tx_boundary) + port->tx_boundary -= len; + + return len; +} + static int dbc_start_tx(struct dbc_port *port) __releases(&port->port_lock) __acquires(&port->port_lock) @@ -36,7 +59,7 @@ static int dbc_start_tx(struct dbc_port *port) while (!list_empty(pool)) { req = list_entry(pool->next, struct dbc_request, list_pool); - len = kfifo_out(&port->port.xmit_fifo, req->buf, DBC_MAX_PACKET); + len = dbc_kfifo_to_req(port, req->buf); if (len == 0) break; do_tty_wake = true; @@ -200,14 +223,32 @@ static ssize_t dbc_tty_write(struct tty_struct *tty, const u8 *buf, { struct dbc_port *port = tty->driver_data; unsigned long flags; + unsigned int written = 0; spin_lock_irqsave(&port->port_lock, flags); - if (count) - count = kfifo_in(&port->port.xmit_fifo, buf, count); - dbc_start_tx(port); + + /* + * Treat tty write as one usb transfer. Make sure the writes are turned + * into TRB request having the same size boundaries as the tty writes. + * Don't add data to kfifo before previous write is turned into TRBs + */ + if (port->tx_boundary) { + spin_unlock_irqrestore(&port->port_lock, flags); + return 0; + } + + if (count) { + written = kfifo_in(&port->port.xmit_fifo, buf, count); + + if (written == count) + port->tx_boundary = kfifo_len(&port->port.xmit_fifo); + + dbc_start_tx(port); + } + spin_unlock_irqrestore(&port->port_lock, flags); - return count; + return written; } static int dbc_tty_put_char(struct tty_struct *tty, u8 ch) @@ -241,6 +282,10 @@ static unsigned int dbc_tty_write_room(struct tty_struct *tty) spin_lock_irqsave(&port->port_lock, flags); room = kfifo_avail(&port->port.xmit_fifo); + + if (port->tx_boundary) + room = 0; + spin_unlock_irqrestore(&port->port_lock, flags); return room;