From patchwork Mon Mar 10 08:36:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009397 Received: from mail-ej1-f44.google.com (mail-ej1-f44.google.com [209.85.218.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 370F5E545; Mon, 10 Mar 2025 08:37:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595825; cv=none; b=PVAr+p5aDCoI97iZLluXQiODHv+YxSS3K2VcohXjpd+yDhfSP8HQfhB2pvt0LT9O22LENcTqHSafcl3d0StE5jrjaFjvUa3lTaI0xVQLeVJMhoC/yw/MHNaFEc/P4lZbdM94LTdHolCHT9llstDKIZkYpvFNisqTZ1DIUAw+B8U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595825; c=relaxed/simple; bh=tpAaiAtCdaamxXM04C4iQNK1HkFBf7XzgFPkCYxU8IY=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=eREbeccTTOdVzIELnkuKzz6yQZB7noi0GLm7cQRgBZHflF90MS1WCkZTntA4nVF06k/tvnZl3Zeu94NurmW1Z+60Bckr1125DV32tBXV6h9x7jCXwumCA6YlnyzjCJLYjNNdqDXznEdniBs36IMoS4yj9jfNyV8Jry+domXFOlU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=ZXlv9ctn; arc=none smtp.client-ip=209.85.218.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="ZXlv9ctn" Received: by mail-ej1-f44.google.com with SMTP id a640c23a62f3a-abbd96bef64so633829066b.3; Mon, 10 Mar 2025 01:37:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741595822; x=1742200622; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=Iac6UoWKVrOdOtUS/jSWoBYCwqGBA+BOqutr7ei4GnQ=; b=ZXlv9ctn/qRL3X5ST/pezluQQ8lsdk9fK4PQwFiS6P45KeQbZUv7NkNJk9NpLeXkDk WMMo9oImmn3DBuefgZMLIpWHxo9o16IS80N8rPKQZVJZIZNlZA9AD7sm1vLJ+1CxlfwR NCshHP3+n4k5BqETn4GaC6HAkIYwg5YpP+VtoMKVm5t9TaS/HSvlM2BlXTj8cF8lOOBj 6xlEfZVGIgoMOtkMGo9ZdvZQ7tspm/DB3CGyQDzLbGZgt4TNzUxeMsO6E1kesjVsQC/i EyCGVxd4+bIkdtRjTvO/JPMT/4VRnySTYdZALN2v0jSVxLwRnvwEJksVThlf+8bpsokM ra2A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741595822; x=1742200622; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Iac6UoWKVrOdOtUS/jSWoBYCwqGBA+BOqutr7ei4GnQ=; b=X79/3gXTo4+cCeb4fSblyuIhoJgrx9tuupPylc+3l2kYEhWP1RJ0ZhMFaGSw2XLhuG lYLHrqHaoLzV21hOkdh6AfqNuSzsTePuUiG7iKC4OwJHj+nwa5h5dPM8iMCnrDBREgY+ uRl+HxGOP5J+NNuscm9e5rS2iKsqGnyrnkK/+1+9EABNJaxbq+29vj0HhClCKBeQRYk2 kQAF4owgdMDyftSrE5a//JFULyVfMTo8Js+VBBcxuvTGMo07lBJJ8eUQfe/Gm1Y3lHWx BlTYuK3Mxd31oZM2D0N6XVCJXj5xMFrivg2f7UTa4tqZ3Ed7zm5UKQtzl22o8qudl9qt 3N8w== X-Forwarded-Encrypted: i=1; AJvYcCU1etMTlvqDzMmJoLie3m7IaakrdPmHfZ/3+n1vnSxgfzk17QPL01cVRIR5GcJTu4CdoCrNhYHPWLa+VUY=@vger.kernel.org X-Gm-Message-State: AOJu0Yw7L/3vTtxhR0XHJVR/ugDCNgHVtQBMfIFyRHpJzZQrEe/Hl+A7 X7iwGs0HnG8N0UQrrzcY0zFkyk2SKEnWe32kkxzDFo6vhhJ7gElA X-Gm-Gg: ASbGncskTOt6ZDi7X0qC2Qc5pics/o23nf1dfgFTRSKSM3zhorYSKNpjSDRE8zVr5IA +dB18zmTI8Mz5+WQypEzQak3SSIgCyfYM0+HiK5DBH5E7YUtxr0fUUqYZyfE9REpOipNyDEU137 SfNzzHrW824dzNuzdSV5CpDGOxUVekVeNTv8D3TriZoN3wYqavJz1BWHmhnq6p48hhYVHkFYDL1 m9bX46U++4bTADIt616hyQ9L7ZUAOZV9VDveu+dMf8SWxGj8Yg09CvMcfcEGJvepy0eIpQ0W2bf 5n7daGKQ1h68+95OFjx3zPwFHIUx4ludt9PWcRfS4uw2ROx6/q4E6L/U8lAwNQ== X-Google-Smtp-Source: AGHT+IEe1+RUwbFp++B+BlmoIf71yYL2vv7OX/QVFzIa2N4mUXSJMiqcnjqOOWMXxM/K0VCsS7sDzw== X-Received: by 2002:a17:906:f587:b0:ac2:50fd:bbb5 with SMTP id a640c23a62f3a-ac2526264f0mr1128342866b.16.1741595822293; Mon, 10 Mar 2025 01:37:02 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac29dd5a75bsm162910866b.8.2025.03.10.01.37.01 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:37:01 -0700 (PDT) Date: Mon, 10 Mar 2025 09:36:59 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 1/6] usb: xhci: Document endpoint state management Message-ID: <20250310093659.04b082e3@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Add systematic comments describing xhci_virt_ep.ep_state flags and their relation to xHC endpoint state. Add a few paragraphs about how they are used to track and manage the state of endpoints. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci.h | 44 +++++++++++++++++++++++++++++------------ 1 file changed, 31 insertions(+), 13 deletions(-) diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 56ed1b817f91..46bbdc97cc4b 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -662,20 +662,18 @@ struct xhci_virt_ep { */ struct xhci_ring *new_ring; unsigned int err_count; + /* Endpoint state, state transitions, pending operations. See also notes below. */ unsigned int ep_state; -#define SET_DEQ_PENDING (1 << 0) -#define EP_HALTED (1 << 1) /* Halted host ep 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) -#define EP_HAS_STREAMS (1 << 4) -/* Transitioning the endpoint to not using streams, don't enqueue URBs */ -#define EP_GETTING_NO_STREAMS (1 << 5) -#define EP_HARD_CLEAR_TOGGLE (1 << 6) -#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 */ +#define SET_DEQ_PENDING BIT(0) /* EP Stopped, Set TR Dequeue pending */ +#define EP_HALTED BIT(1) /* EP Halted -> Stopped, Reset Endpoint pending */ +#define EP_STOP_CMD_PENDING BIT(2) /* EP Running -> Stopped, Stop Endpoint pending */ +#define EP_GETTING_STREAMS BIT(3) /* Transitioning to streams, no URBs allowed */ +#define EP_HAS_STREAMS BIT(4) /* Streams are enabled */ +#define EP_GETTING_NO_STREAMS BIT(5) /* Transitioning to no streams, no URBs allowed */ +#define EP_HARD_CLEAR_TOGGLE BIT(6) /* Toggle is being or has been cleared by reset */ +#define EP_SOFT_CLEAR_TOGGLE BIT(7) /* Software toggle clearing, no URBs allowed */ +#define EP_CLEARING_TT BIT(8) /* EP not Running, Transaction Translator reset */ +#define EP_STALLED BIT(9) /* EP not Running, waiting for usb_clear_halt() */ /* ---- Related to URB cancellation ---- */ struct list_head cancelled_td_list; struct xhci_hcd *xhci; @@ -703,6 +701,26 @@ struct xhci_virt_ep { bool use_extended_tbc; }; +/* + * Endpoint state notes. + * xHCI 4.8.3 defines three basic states of an enabled endpoint: Running, Halted, Stopped. + * The fourth Error state is avoided by not queuing invalid TRBs. This seems to work so far. + * + * 4.8.3 warns that Endpoint Context EP State field may be stale, and it doesn't indicate ongoing + * transitions anyway. Some xhci_virt_ep.ep_state flags are used to keep track of known transitions + * and various operations which imply or require the endpoint to be in a particular state. + * + * Notably missing is the Stopped -> Running transition started by a doorbell ring. 4.8.3 suggests + * that it is instantenous, but on some common HCs it may not be. There is no universal way to know + * when it completes. Transfer events imply completion, but don't arrive if the device NAKs/NRDYs. + * Stop Endpoint fails if the transition isn't complete. Polling the Endpoint Context is an option. + * + * xhci_ring_ep_doorbell() inspects the flags to decide if the endpoint can be restarted. Another + * user is xhci_urb_dequeue(), which must not attempt to stop a Stopped endpoint, due to HW bugs. + * An endpoint with pending URBs and no flags preventing restart must be Running for this to work. + * Call xhci_ring_doorbell_for_active_rings() or similar after clearing any such flag. + */ + enum xhci_overhead_type { LS_OVERHEAD_TYPE = 0, FS_OVERHEAD_TYPE, From patchwork Mon Mar 10 08:37:48 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009398 Received: from mail-ed1-f47.google.com (mail-ed1-f47.google.com [209.85.208.47]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54F50223704; Mon, 10 Mar 2025 08:37:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.47 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595875; cv=none; b=GbQ015g3zIz50V55NcV55cYb4vzQvxurv5B1TGCT7BtLSN0mkRGzIQz9rJDBvkvHAj13yV0V1rUXV9b4XJnDLJnEE/UbDzz6eey/NvTls50EDrjljs8YoGasXXTMhegfytTLlHTfNSAEpAdaQWfZ2ewrhUbBk2YVDTY7OpYMylw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595875; c=relaxed/simple; bh=WwzChQUR0/Orj8v4gVEr8SzL6nweKeVJJMunMAUDTNg=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=fYZgBWIS9fmZs8E7gGx25T3561w1xt6qUTR4NaXFwJ7hn+PcNbh3Hi1BHJwdr3BTmKmPFJdlVV95utO9zKgpSzlkGWGagi8cMWPBvlXqxKkEs1xUWV6F2dk1ce6U8wKBj1ZOx1aFs49FHmKMTiKTkVFOjd6WvuyvxyN6MXDIt0g= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=i7yiwgjh; arc=none smtp.client-ip=209.85.208.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="i7yiwgjh" Received: by mail-ed1-f47.google.com with SMTP id 4fb4d7f45d1cf-5e535e6739bso6161670a12.1; Mon, 10 Mar 2025 01:37:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741595871; x=1742200671; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=PzM2FI7xwyYh3FQWLyCsyYca8N8fg915Jvc3iX3DzCA=; b=i7yiwgjhTt7ondUNwHxCzVo3xo6GRfb3Rz55x2aTVfHAzs/3kvxw+hXOLwm7gd94ap S41em9IMVKe8gJ6ud93iKaEiUcqWIjz7THxTfpFcuCJwQ9gu/clA7AL0avjTysRrSGmZ hKBXPG5xYbt5HzIRbqOILJujaUTkrhLAJ/TY13mWpAlwR1LGyhZW8XDpAfLYOEWwAS62 hUv3FeejQhp4tjAX9yBldG1Yqcc5E9eBUigUqJ5wD/mTB8EdLZyheP6yzPPP/OfhE9kv 0oEF4/udt9X3hulIiB3yDRm+hZGiPRlFqYjjqjt8NecJn8XeuKL4fLhivdM+5KvQdIuM WK/w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741595871; x=1742200671; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=PzM2FI7xwyYh3FQWLyCsyYca8N8fg915Jvc3iX3DzCA=; b=K4Oqdup5ut1p4zIc223Cx06mpy7AwttBA4VCPkoQpDs+hUO5bgEs2k+0uduKWuI6Ud JUZuBEmFxb6NXaJA+oI03Q1OQl09hfdsX7jPI4DnakOVVUxWkm/fovDnkcxrbpYt8Ixr QFZRmNJDuREVukZzvlTFVk/6eGOg2/xg+P37KpECoQ6m//MvBo2Z0IeOhVloABzOBujP 1SyMKf3iRXJIFsuTWkl61kP/2jWMaNuj/Dxktq2qLs+hm3YEUWvflTdy8iE1KViXy5jQ /36IvIBsqNYjRWYxue1eQmoyN99tK3qE9n74JhtIADIQPL9GFQ/gC19hNK6NxFzSKTR/ CIIw== X-Forwarded-Encrypted: i=1; AJvYcCUhtMDufje62WAhfB+9yCvZFTGNz122Ki6jCnZTW/UuH25330+pGiER9i4+/R5CH5mYGo7l2scr/exfbKM=@vger.kernel.org X-Gm-Message-State: AOJu0YzRZjjhuHUpe0PIM8/imepFINB7eNM8tHZOFDBKR95t0wMyhD32 6idXfFUZ64I9ZUB396C64l87nppwQaYBqAiogE9/1+xnJn77oJ4O X-Gm-Gg: ASbGncvB9GAnxoVRPdH35oNlJlPTBK5asd4kOV9DUSY9gxMHuW1tHhfaSjV45u/V5SE LFWERwSXdEWMjbRTduNfmMrg3EcY6aC9/k7Z7MQLaJK+eJ0jKRhn2Ai/s3dcZCTPOyBq9ZkMW1y KE52eWwsEdMuPNqVBlFpl5x3sdWB8BUBMgMrG4F/56Weg3zemqNRU1jZrIUfl9iKKBmliRz6gah MLxGf0FW+63cH5IEzWWCFxf4KZ/MCgOErENzzcQCN0yRNYKoMj65VE8F63aldK552u7v0HX3ILb 0S29VdIv845OQLpbcYUeEGBUzW3ASD+IEtptYLDZZJ537MiA3o0Nadr443LTRg== X-Google-Smtp-Source: AGHT+IEA88nbsP/2GVWNBm0DAGnRT9RKYSapr/mIao77wh/OXRQAhQ2TnBS6HICb+VzXnLu1ToJwhQ== X-Received: by 2002:a05:6402:3547:b0:5e4:d2c9:455c with SMTP id 4fb4d7f45d1cf-5e5e22b7935mr12418985a12.10.1741595871370; Mon, 10 Mar 2025 01:37:51 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5e5c7472169sm6564505a12.24.2025.03.10.01.37.50 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:37:50 -0700 (PDT) Date: Mon, 10 Mar 2025 09:37:48 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 2/6] usb: xhci: Deduplicate some endpoint state flag lists Message-ID: <20250310093748.201e87cd@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 xhci_ring_endpoint_doorbell() needs a list of flags which prohibit running the endpoint. xhci_urb_dequeue() needs the same list, split in two parts, to know whether the endpoint is running and how to cancel TDs. Define the two partial lists in xhci.h and use them in both functions. Add a comment about the AMD Stop Endpoint bug, see commit 28a2369f7d72 ("usb: xhci: Issue stop EP command only when the EP state is running") Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 10 ++-------- drivers/usb/host/xhci.c | 16 +++++++++++----- drivers/usb/host/xhci.h | 16 +++++++++++++++- 3 files changed, 28 insertions(+), 14 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 0f8acbb9cd21..8aab077d6183 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -555,14 +555,8 @@ void xhci_ring_ep_doorbell(struct xhci_hcd *xhci, struct xhci_virt_ep *ep = &xhci->devs[slot_id]->eps[ep_index]; unsigned int ep_state = ep->ep_state; - /* Don't ring the doorbell for this endpoint if there are pending - * cancellations because we don't want to interrupt processing. - * We don't want to restart any stream rings if there's a set dequeue - * 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)) + /* Don't start yet if certain endpoint operations are ongoing */ + if (ep_state & (EP_CANCEL_PENDING | EP_MISC_OPS_PENDING)) return; trace_xhci_ring_ep_doorbell(slot_id, DB_VALUE(ep_index, stream_id)); diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 7492090fad5f..c33134a3003a 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1762,16 +1762,22 @@ static int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) } } - /* These completion handlers will sort out cancelled TDs for us */ - if (ep->ep_state & (EP_STOP_CMD_PENDING | EP_HALTED | SET_DEQ_PENDING)) { + /* + * We have a few strategies to give back the cancelled TDs. If the endpoint is running, + * no other choice - it must be stopped. But if it's not, we avoid queuing Stop Endpoint + * because this triggers a bug in "AMD SNPS 3.1 xHC" and because our completion handler + * is complex enough already without having to worry about such things. + */ + + /* If cancellation is already running, giveback of all cancelled TDs is guaranteed */ + if (ep->ep_state & EP_CANCEL_PENDING) { xhci_dbg(xhci, "Not queuing Stop Endpoint on slot %d ep %d in state 0x%x\n", urb->dev->slot_id, ep_index, ep->ep_state); goto done; } - /* In this case 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 */ + /* Cancel immediately if no commands are pending but the endpoint is held stopped */ + if (ep->ep_state & EP_MISC_OPS_PENDING) { 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); xhci_process_cancelled_tds(ep); diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h index 46bbdc97cc4b..87d87ed08b8b 100644 --- a/drivers/usb/host/xhci.h +++ b/drivers/usb/host/xhci.h @@ -718,9 +718,23 @@ struct xhci_virt_ep { * xhci_ring_ep_doorbell() inspects the flags to decide if the endpoint can be restarted. Another * user is xhci_urb_dequeue(), which must not attempt to stop a Stopped endpoint, due to HW bugs. * An endpoint with pending URBs and no flags preventing restart must be Running for this to work. - * Call xhci_ring_doorbell_for_active_rings() or similar after clearing any such flag. + * Call xhci_ring_doorbell_for_active_rings() or similar after clearing flags on the lists below. */ +/* + * TD cancellation is in progress. New TDs can be marked as cancelled without further action and + * indeed no such action is possible until these commands complete. Their handlers must check for + * more cancelled TDs and continue until all are given back. The endpoint must not be restarted. + */ +#define EP_CANCEL_PENDING (SET_DEQ_PENDING | EP_HALTED | EP_STOP_CMD_PENDING) + +/* + * Some other operations are pending which preclude restarting the endpoint. If the endpoint isn't + * transitioning to the Stopped state, it has already reached this state and stays in it. + */ +#define EP_MISC_OPS_PENDING (EP_CLEARING_TT | EP_STALLED) + + enum xhci_overhead_type { LS_OVERHEAD_TYPE = 0, FS_OVERHEAD_TYPE, From patchwork Mon Mar 10 08:38:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009399 Received: from mail-ej1-f46.google.com (mail-ej1-f46.google.com [209.85.218.46]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9DA2F22331C; Mon, 10 Mar 2025 08:38:49 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.218.46 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595931; cv=none; b=NWf/vUpGHGj2bQ8hixDLA/iF4QRxhEPEfeil//A69Nle5bJ3rDNmFGhZ4doh8u7nAbxFmDv0EbeA0M7P6CH+o3bybkrLyIjCZ3DxWFyb//pfK+lmElVGHo2Fl9DnK0fxDdVgugx/q7KvBjkGLfFRmIxz4SBgMnYIJXZ4g8NLjJU= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741595931; c=relaxed/simple; bh=X/GmOW/hNTjDdVCoJzXZunwFoXnbdmHANH3kWmdFLtQ=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=o4Wqvs1fahCewj1SetNKxVlR/bv/YSkZ5vPso65Yy+0JY5q+cEetm/HfA0jTtiAfRqDG0zny01+fv8uj2HNgugKQjAPdXTEDHcBtyev0xuQubOywwu8I+YUY7obsR9LJ16mDMcemrRs1JftYkM7byhvJAdzSPHMhqO74WG4gBDc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=MsRYYhbj; arc=none smtp.client-ip=209.85.218.46 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="MsRYYhbj" Received: by mail-ej1-f46.google.com with SMTP id a640c23a62f3a-aaf0f1adef8so690560066b.3; Mon, 10 Mar 2025 01:38:49 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741595928; x=1742200728; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=Mc4o3nge4FTwi3uakH4SDvIBS7mKNo+if4JgTye23cA=; b=MsRYYhbjr6++41TqfLsUQMaOKmWdCUTT9BsNWIMDAt91jRlIbdcgDIUJVrMoTjHdqc rNfzJsyxTKdlbQTxo63pE6RZc4A5lJjKIIEKg2NXzpNY8DSMxDCrB2hDnL8LsfM4I2eu mKDsf6usYy7rbTJXbUaSMmOJCgmbTknVmSHSJQ/aUdjVIedMzRqlH4DhSiuOr/74MesB iMYQQvqwv6iHTqtsiMvNdbNKTTjmRV2jeocjl15i1+/A+lpBEsEER0sryDtVO+vUiddp 8L75rhpgfBGeXidytc0FOHL8JKRg9/zolxfNJho779PQrskMeP1M5et0eXUEphLXixFH lROg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741595928; x=1742200728; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Mc4o3nge4FTwi3uakH4SDvIBS7mKNo+if4JgTye23cA=; b=Rrjq6K5ALfN/BxeYS8CSEor7YGEBSN24m6sJYdYOGXiAVzfV90t3zzHVP33/ninges r75f2x3bVypGVi2qRBk/p0zJzwZpQzHUxGOz4L/5ZbNJuGOvMfPOPDtfPg9GvFP+oXt3 9gKFLCVhtGNr6b5WrucY3d8Wd8lRStLj958MJcXnAGUSrEoNNgd//zjMa3wlGYoa38Mg JLNpEJ/r09qBEqcHsf8TUahdIov1qLBsjayj2J8lR05XNJhjATGlToc9C4iiQ+YYWA0G kvlvN3E7AmcofWTM96IDad06A73GI3UPBSlvFw0K93gEaxF9S6CuRKZKayHnRDtp0skB oozg== X-Forwarded-Encrypted: i=1; AJvYcCUzJKpU8pYEnw5geYW1VjvytpgGOCjP40WFgd4zmMCC4xczwf2GW+0/qbeYsjsD5pnYAOLhKGx70ctLVLk=@vger.kernel.org X-Gm-Message-State: AOJu0YxwMoIEAiHtUJuMMFNkc6Y0GyTqsF/19viOOfrrQQolg0sYHbHV QcYqe2Hvnn428D5rCoGmlMFHYlIy0bkeY/Z26HE7sXTTpva4cxOWz6lyWQ== X-Gm-Gg: ASbGncuvkqgCY99koP81/b3w9uJaCtp76mJNr7g0SpoKpA81ox+rql7d7lz5FNqMmhB FI97RINcOAfGCOcausSdifsIBbYl8oiEBGQ8h3m9DxyZky1DJp1JVUJ7hE+PyXZZzz+1GlxaX/P MvvcOaPzT5bki5VHk9vsKzAg3DY7AtTU70yVxVmWcy+ioOyc7+vIjRxHarRkbPi66iUgJGPcEnG Bh6OgKzjYQXoGJ5oa3aBf1XWaiR4Uv6PW18rXh8xw1HTOkPJJHI8sFdmCaJ63Gj6A6ZkLfcRx5T sCCzAiuH4WaUg4Zhy61ho9rgz9njOTtDNiRuhTtiRH2az6KKblvz/LzvXANAxQ== X-Google-Smtp-Source: AGHT+IEc16nb0VGL5o8sirLNt5/Ky3SosovsNBGOGfy/haFuQaDfONZlrOGAj3TmCCvKy8gZcDROWA== X-Received: by 2002:a17:907:3f96:b0:abf:62a4:14ef with SMTP id a640c23a62f3a-ac25274a051mr1533256066b.9.1741595927450; Mon, 10 Mar 2025 01:38:47 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac2a4541360sm111134466b.6.2025.03.10.01.38.46 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:38:46 -0700 (PDT) Date: Mon, 10 Mar 2025 09:38:44 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 3/6] usb: xhci: Only set EP_HARD_CLEAR_TOGGLE after queuing Reset Endpoint Message-ID: <20250310093844.19e0dbdd@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The flag tells xhci_endpoint_reset() that toggle/sequence state is being cleared or has been cleared by Reset Endpoint. This only works if we actually queue the Reset Endpoint command. Impact should be minimal, because the endpoint can't start running with wrong toggle state if it's still halted, and class driver is unlikely to usb_clear_halt() if the halted TD isn't given back (it should normally unlink all URBs first before calling that). But it looks wrong and could cause problems if the code changes. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 8aab077d6183..9e4940220252 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -988,7 +988,6 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci, /* add td to cancelled list and let reset ep handler take care of it */ if (reset_type == EP_HARD_RESET) { - ep->ep_state |= EP_HARD_CLEAR_TOGGLE; if (td && list_empty(&td->cancelled_td_list)) { list_add_tail(&td->cancelled_td_list, &ep->cancelled_td_list); td->cancel_status = TD_HALTED; @@ -1006,6 +1005,8 @@ static int xhci_handle_halted_endpoint(struct xhci_hcd *xhci, return err; ep->ep_state |= EP_HALTED; + if (reset_type == EP_HARD_RESET) + ep->ep_state |= EP_HARD_CLEAR_TOGGLE; xhci_ring_cmd_db(xhci); From patchwork Mon Mar 10 08:40:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009401 Received: from mail-ed1-f44.google.com (mail-ed1-f44.google.com [209.85.208.44]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4017CF9CB; Mon, 10 Mar 2025 08:40:42 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.44 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596044; cv=none; b=Ix81n2IRn9NdWW8+L/CDCYaUj87Za21/nZkUFo+91NJ5JDfALSnYzBql82DUq817snPS+l1E0+B86zLHNThBSrTPMEwkEv3s/gSnMrv4PernXmfZdOLr1B6nWBgDQ+qkFqYwsiccYxjFnxQxeZN+UqkFYGHcSfZ19m+4O5iqFpQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596044; c=relaxed/simple; bh=Ens4mW/VOnDxf+/m0m0sz7eh4cSEe/GjTqbYLbm48l0=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=PhK8fNY3esap+c+ASL/1ArxTNALqf0eXMDcV6kYUzhALXgaETebfRsnJY2xXPAfJuzbwR8vIkP0mkGd/ifOkrtM981uwRS8n1qRihVDtZe4oDmFra1hSFpBGZDHe6OxzL1zL3yVjK/GnJ+u6w/SyrmZaT9ibS3Ob9IaOVH6K2h4= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=VB++FgA7; arc=none smtp.client-ip=209.85.208.44 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="VB++FgA7" Received: by mail-ed1-f44.google.com with SMTP id 4fb4d7f45d1cf-5e6167d0536so3488385a12.1; Mon, 10 Mar 2025 01:40:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741596041; x=1742200841; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=uKRxHFwd/2vW4jmL6ANEf5eoCq9mv6fT9Ju/M9j0wwM=; b=VB++FgA7KGDhbSqk+FXpVOCPONz2zfScacKmO2NajMV1RbhdW1R/gX/0dmHn6FXxYA v2qafwCSK9Adw+wAiJfOeyk8JPfV37xngPrIBEIpl6RCQcx69GBnL01jM5gf9sVsaKls tnSaDckPLnFFMcN7Nu1t0nMBiF2qnCUJDD6J8Jyowoxc4FOmHhD+YFQlJNSv7UI+3LTi qLTMf0BMNq+LN2LAhthCq0/SkFJT8pmn6rVQESWxSyscYcwuX+eg83kmce/0zBxSx8DH 5rEbJpmDZSrDYKR7MYe8L9z7kVkWqsN909vfaKKZRBMO+xutLwAYIP1o8tPCL+qIZmxk 4Mcw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741596041; x=1742200841; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=uKRxHFwd/2vW4jmL6ANEf5eoCq9mv6fT9Ju/M9j0wwM=; b=ho2iT4KaSr60hsfMRlKCkgiFbTQrSqBBALe2k0xYV9jeSUqn0NwiMX0dCIRaG/Tsou p3F0bqmEQuz4ZoPbaHpJKct93A8MPHeO+JzZXbzF6yw6cLiOYf/OhdsLlVnqNh8ZNNlG ycd5vEP/iW1aQc2/IZ5OM17YE5HcvNf7dG2N8VnlwMv7clMTQ8mbgCjzLBu9HTTiZzcM Lom86W1wqvlqVUYPzxMv+mrfQAPwO2PqNhOlC0pkrFyEbpiIaQTAEvzbF9U0/wncGRWn 4jHZZrFRH3LDiSs/0WT27EhVXjtCGLmSLc36oSUXmsUI+YqLnHN5ZtiAEpnHwlKsHLgA tPNg== X-Forwarded-Encrypted: i=1; AJvYcCWP1w9FQOt4boaIDrIYTTUBEIJZmyiZhrKQrjAMz1/0E0X7jJXF+N3lMMwhID50P6CLsaNTaibXZ7t5ww0=@vger.kernel.org X-Gm-Message-State: AOJu0YzJVf1FJlusqVJQXLsmansAt545fxpxMltVetA1LncnwxOUQHlT movufSXoATaS8GDZmHM+1VjW6DCjc53LwNpnc12WFftGHf7QZ4zN3a7whA== X-Gm-Gg: ASbGncvt2P7n7mp5AWkDp1WvrXi7NrPJR2I6zrhTlunVUNk1NNJWzu1DrsuXonbQipy IRCASb68i5Tm3U2C1ykktingO8w5KQUjt4uiX4ufHJ5TwFPVjl2Be9RfdA0EQpUn+7Eetcrmg7K r2t4xh9aa+jLQbN+ZCLnKvT31BsiYtW5vlmJQj5WWT0l53d3Hr5HJZIgzGHfRyB746FaKwtlXdj usocfSvbYm8Z44fi5BuK7+eTllNfwGoqXDsTqju7pqQPlL3vejbxkFr9Irlz/paw5RlnWyhxbEm hmqPf5+0FLHwlNiPoKCd/NG10hDN26QDM/f9YKEPAMFmJbNrjJ0rZwqs1tliJkBKVEm+/48e X-Google-Smtp-Source: AGHT+IFhw9TBYLkR/DFN/0ojmMM3DShRsh6XKMKPnlE4NPHejTsc3h+RLUH4BZOKSBp6+0MdTdQRrw== X-Received: by 2002:a17:907:938a:b0:ac2:92df:fab3 with SMTP id a640c23a62f3a-ac292e003e5mr325618066b.16.1741596041358; Mon, 10 Mar 2025 01:40:41 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac2394825adsm743622666b.56.2025.03.10.01.40.40 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:40:40 -0700 (PDT) Date: Mon, 10 Mar 2025 09:40:37 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 4/6] usb: xhci: Don't change the status of stalled TDs on failed Stop EP Message-ID: <20250310094037.52625e24@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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 --- 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 9e4940220252..28ebc0fc5bc2 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1215,7 +1215,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 { @@ -1226,8 +1233,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 Mon Mar 10 08:41:29 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009402 Received: from mail-ed1-f42.google.com (mail-ed1-f42.google.com [209.85.208.42]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8980822258E; Mon, 10 Mar 2025 08:41:34 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.42 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596096; cv=none; b=sTqBvhFJT1R46JgiKwCTriB1vBv/hX3Zmm0It4Xgv+mUa2WevfMcBTc6EpgKgh17o1rRM8/br3Nf/AXadKrtq386nKtYVOK4pYaDfDM4mC+M8JtAXbTWJP4qLENDuWJCtT/jOnCkGRjRo2zQS7xFbtL2+u6XwbpyLd9P6dYWm9Q= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596096; c=relaxed/simple; bh=sZJpqurhRMS2QPh3cwePIaisIvFQbiS5sBipqSnor+o=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=G2LZoqZwDLeaC7FgrJG3BC2Ca4BGypSnmjCnJU5oZLuZ9bkSntjPHG0kSJwPMD95hDPlHQgEANew0irUEodVQx6XX9To7J4Fnhae00pkeyLb3RpYigjq++56I0JF1+ZWwcRgIgNZ2ytl/eYhkNj13YOeOpHis8Di+hv+sjz6KIU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=CjbincBd; arc=none smtp.client-ip=209.85.208.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="CjbincBd" Received: by mail-ed1-f42.google.com with SMTP id 4fb4d7f45d1cf-5e5bc066283so5994640a12.0; Mon, 10 Mar 2025 01:41:34 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741596093; x=1742200893; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=QZUNeINjdqSJ79E/oEIhLa9NAr++krOxCOFKaWQ3/OY=; b=CjbincBdIrgVzTdHzhRN5Z4BsDMszNfTeBkRXoNVWDvO38Yk853cDBfROLnZqJFQ/5 2DH9kYA/Chx3vN6BHmsgAX681HS92dhskIjpndhiCZcXSs2Y3R3eGCUgsc8ZcclYRIal COtoFAUhMA4FQDPD4U0jDU7eCo5L971Kf2thfNQl/lM8JW9R69x685qTwJDHlFUmr9ba /yDpo3bbaXXmxRrJpCBNq1HvwxtadvEnnglQZfonTXWhm/sNz3LEaDftdIisDphJVgWI Ad1e+qTlaF/aw+Db6Mh6ajNONdEtoj4nHmHO7g+B3T5Ej05nkSlQGNRCxgZwO0s8WALJ FyqA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741596093; x=1742200893; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=QZUNeINjdqSJ79E/oEIhLa9NAr++krOxCOFKaWQ3/OY=; b=jutWKIlKK5OM8xqCbPsV2yOCVJM18YCHU2GdPDdIhorSPtGenczskABKOZ4ShZKXPM cPbrkRvqPMI5xza2imLorM1yHix0TM5qgs87wLVrG8/BuTnq2HERH3wGceo2p1p0MwX2 BZPkecgeH4g4O1xnj6JYOhxAzPpFcAbBOH9RwohMZphLvH4lmhlBDwOXska5mHQTE8Ds PgF1cqQinRjyOTBNkS9X5d6AQD4b8t5ZZVb7YRZ35scuoycGwUbYocNgJkoUJfHnOp42 zRfCf3SD+xPp69xyZRfACuIPqUkvayE0T/Sw235IQcnJOq4T6kuu6e4Oc/boSuGH0SOx NPiQ== X-Forwarded-Encrypted: i=1; AJvYcCVHGBXaJgKRFWYK3l8ycycEQQg+SOOd73Os1qKdD4F2D/9NZw6aJRiCG1sIQrux54ywqE62S8rpKueN/Ho=@vger.kernel.org X-Gm-Message-State: AOJu0YyJkQkZS284mmN/qZBQPJr68l2TS30GjolKbCpnxcEkShcnp/As muywpmhwbOts/9x150nDFGQTFp3PmTo1BUs6/IGAJShirlYQ7+q9 X-Gm-Gg: ASbGncuAorjcOFLd2dDA5rO+1hscoK3n64yuJBFsuRId1qi06g7VUim8PLcXWGTJiDu uMO18k1ehZXn0LvYA/QQ96BGr+6RMXsMekZfDSPcBXmLyGw+PRPfJOHDtQL7pjOGBMM6UqkKaKf q6bumc9NhT377udbXY3YYHlowa5ZesQd29PYGBLx7NfIBapLsb5vKTJgU6wJr6hvsGE61VfhBf1 0syInTlCi6+jZ4xu1WHOkgc18l/jd6lCqRN1fXYx1teJvZ4N76j4D1B8LVjmyTEgBDzOkyND7Z9 UUZf7b7apVhnIMKNB/rO4fh9PTkqXCA54a2DBood3nlRocCiZmGNSGbP6B5k8g== X-Google-Smtp-Source: AGHT+IELDJI+VIDcVn7Yq5RFxY6w/z428UF4fp4K88TLicDPu3CdberaWXBF66KZJcQjbTu2huEF6w== X-Received: by 2002:a05:6402:234c:b0:5e4:ce6e:3885 with SMTP id 4fb4d7f45d1cf-5e5e22b22aamr13600345a12.2.1741596092564; Mon, 10 Mar 2025 01:41:32 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id 4fb4d7f45d1cf-5e5c7473ea1sm6619073a12.29.2025.03.10.01.41.31 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:41:32 -0700 (PDT) Date: Mon, 10 Mar 2025 09:41:29 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 5/6] usb: xhci: Avoid Stop Endpoint retry loop if the endpoint seems Running Message-ID: <20250310094129.20273c14@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 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 --- 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 28ebc0fc5bc2..241cd82672a6 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1259,16 +1259,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) { From patchwork Mon Mar 10 08:42:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: =?utf-8?q?Micha=C5=82_Pecio?= X-Patchwork-Id: 14009403 Received: from mail-ed1-f50.google.com (mail-ed1-f50.google.com [209.85.208.50]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2A02B22370A; Mon, 10 Mar 2025 08:42:18 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.208.50 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596141; cv=none; b=hbgnpQ4XAUx9gKnzqS5Ley7i7MJw1oP+7dMyBWw4Eyx+RIVek77nSJpsVlCfpGNYVFx0phAg4AYEhsRZr4N+RwGnrFuLlVjslGMYtb8KPT+bp5aleFTlgQrK/EqzXbTqo05Y88sGOAP5qIgZPhR4v6VTNNQ1folvn7x8dmtnOJs= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1741596141; c=relaxed/simple; bh=PXuQmvsVxN3m72BnR6ST/tTb9xlntmcfVC+/hzMx5Xg=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=IzgnUSB0aPy6m32RmklRnO5miSFwbLxBsx80+Zs+KG4TIQfuDKCzugiota2P1HZ9pQZdIOnGYolKwRQmKnl0zoF66je2RVQRUV8WE3yiMSyQsoh4uS991Ci6gAKF2fOTUzORFhuqBdkXWfCZnBAXSZeMCCOBSASrkgS5GIoGRsg= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=CeYDNCM5; arc=none smtp.client-ip=209.85.208.50 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="CeYDNCM5" Received: by mail-ed1-f50.google.com with SMTP id 4fb4d7f45d1cf-5e56b229d60so9140607a12.0; Mon, 10 Mar 2025 01:42:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1741596137; x=1742200937; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=YfIpDSCu41Ig3X26NPNtdponl0+k33/p8Qk35er3Ujs=; b=CeYDNCM5Vt7llbEY6iJDs1C0aQ4tTSFaDeq3bM2m0cxtHm5WXuNZn9UE0G+BLqE5Mg vnPPPZhXb4qXTpijCJ+h0DZf7+BXc5sq6CWJNXASWAwoG/1aIwBWjuHeFJHxAEcNBFlu spkkvHQYXmbBrtiiTzKp7JoS6QCH/Ojk6TUcbAZvGx60Tf0NwgqcPZiIp6kE/3/IPxf+ 8IiLFz5ejrYLk4IlzwiWDREmEPzAQxl0d/jJZtFUJaemgAJbw/HtQvoVLWsRRYEMemYx GyU/o9xVGueNcYx1mXxugz8nj6ZAZywdcfkZF88ui7ama+fgm0vdkM/GGiCXXwZl2zhr n85g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1741596137; x=1742200937; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=YfIpDSCu41Ig3X26NPNtdponl0+k33/p8Qk35er3Ujs=; b=tdTpf6nYDY0X3f0jPBxi/K1M5CQZNxUx/ITcPryZoJDic5h5a9d9innvVb7iR95ypv +H/WVNcgQAwzfIcQ/8+LOAFfyWHSDRaWOcmiPb19Pis2wa7tBeOMnzUI9Dn9DkHxdkGP KuZ/h4rNJqGAjRiXrpvaTfQ2FYtf1xa/h+1sQ3BRhc7ZT43RAzK8q4ftfdguCTY9+oS6 I+9tNbea1sKfcn+SiQIRjG7vOeRlEj+InLkylk2xGDeARv3eaTp7CiteRJEPzGCFZ250 bfqujd1qfU7mDGrqNKl1kaIgeXThE1lpV8bmcCchFyXck1UmFe0Y0muD8b4jBQv0+UNb ZQfg== X-Forwarded-Encrypted: i=1; AJvYcCUsQROZA55sn9vaCmfnrWuvx7SJrZgq7DStOV2UL1s7D6p0dVm2YliKXPUGs5VQbfMlGBB/Dzc1G3Ei9io=@vger.kernel.org X-Gm-Message-State: AOJu0YyJsrM5/eXWFZGRYl/8C6WNRnAO2MzcrktFlVQjuycA/jeB0cCX 6qAsO56BmM3SitbOEfqH8uQXsYelaqXq3QytI/prgrgTL/is434T X-Gm-Gg: ASbGncvB1yyiBhYtNZdk5Q+poni8QyKAykZrQEqZFIPeWH8jEB56GI2pxRYnClMeZvW 1Sqn5aWUCvCBmLAuEsA/4E4X4jpCUa4RRp58OBbLsgWdl2lDTdY0LOL0QvrQ7tNrtp8dQxPp9hv nUU0B4OrI1VasbXPGHYDtX7ggsbZQsaDWwC7HPHc5HYCft7YqXk0wMWdbq+q3ugcU95X5qr/UuD qC6HQnnzwq5NIvzIRnIkTw8Ooo8ndidcTpurDOCOEh8lzawpjwvHOsuSGRdP9tDHQ0QnTgMgq9e gG6enNCerHSTwpSQZ5DMAARswb4PUVA7/wQ2Nf9qLfHXFTgFG9PgaCxTxrIXSQ== X-Google-Smtp-Source: AGHT+IG4/ADwP8W171t4vUpwm7B2Fco1Sd49nJ2HQ6NR6fVUcMZufAZlzTFmYs6O+Ku0Nps74N6p1w== X-Received: by 2002:a17:907:3906:b0:abf:6e9:3732 with SMTP id a640c23a62f3a-ac26c9d4a9dmr875613866b.3.1741596137279; Mon, 10 Mar 2025 01:42:17 -0700 (PDT) Received: from foxbook (adts246.neoplus.adsl.tpnet.pl. [79.185.230.246]) by smtp.gmail.com with ESMTPSA id a640c23a62f3a-ac23943897esm737671766b.13.2025.03.10.01.42.16 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Mon, 10 Mar 2025 01:42:16 -0700 (PDT) Date: Mon, 10 Mar 2025 09:42:13 +0100 From: Michal Pecio To: Mathias Nyman , Greg Kroah-Hartman Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH 6/6] usb: xhci: Update comments about Stop Endpoint races Message-ID: <20250310094213.3afca51c@foxbook> In-Reply-To: <20250310093605.2b3d0425@foxbook> References: <20250310093605.2b3d0425@foxbook> Precedence: bulk X-Mailing-List: linux-usb@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 The switch statement has grown beyond its original EP_STATE_HALTED case, so move related comments inside this case and shorten them somewhat. Shorten EP_STATE_STOPPED comments, add some common context at the top. Signed-off-by: Michal Pecio --- drivers/usb/host/xhci-ring.c | 70 ++++++++++++++++++++---------------- 1 file changed, 39 insertions(+), 31 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index 241cd82672a6..759e8f612b4d 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -1199,20 +1199,21 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, trace_xhci_handle_cmd_stop_ep(ep_ctx); if (comp_code == COMP_CONTEXT_STATE_ERROR) { - /* - * If stop endpoint command raced with a halting endpoint we need to - * reset the host side endpoint first. - * If the TD we halted on isn't cancelled the TD should be given back - * with a proper error code, and the ring dequeue moved past the TD. - * If streams case we can't find hw_deq, or the TD we halted on so do a - * soft reset. - * - * Proper error code is unknown here, it would be -EPIPE if device side - * of enadpoit halted (aka STALL), and -EPROTO if not (transaction error) - * We use -EPROTO, if device is stalled it should return a stall error on - * next transfer, which then will return -EPIPE, and device side stall is - * noted and cleared by class driver. - */ + + /* + * This happens if the endpoint was not in the Running state. Its state now may + * be other than at the time the command failed. We need to look at the Endpoint + * Context and driver state to figure out what went wrong and what to do next. + * + * Many HCs are kind enough to hide their internal state transitions from us and + * never fail Stop Endpoint after a doorbell ring. Others, including vendors like + * NEC, ASMedia or Intel, may fail the command and begin running microseconds or + * even milliseconds later (up to an ESIT on NEC periodic endpoints). + * + * We may see Running or Halted state after the command really failed on Stopped. + * We may see Stopped after it failed on Halted, if somebody resets the endpoint. + */ + switch (GET_EP_CTX_STATE(ep_ctx)) { case EP_STATE_HALTED: xhci_dbg(xhci, "Stop ep completion raced with stall\n"); @@ -1222,8 +1223,23 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, */ if (ep->ep_state & EP_HALTED) goto reset_done; - + /* + * Maybe reset failed with -ENOMEM. We are paranoid that a buggy HC may + * mishandle the race and produce no transfer event before this command + * completion. Or the endpoint actually was restarting, rejected Stop + * Endpoint, then started and halted. The transfer event may only come + * after this completion and some ASMedia HCs don't report such events. + * + * Try to reset the host endpoint now. Locate the halted TD, update its + * status and cancel it. Reset EP completion takes care of the rest. + * + * Proper status is unknown here, it would be -EPIPE if the device side + * endpoint stalled and -EPROTO otherwise (Transaction Error, etc). + * We use -EPROTO, if device is stalled it should keep returning STALL + * on future transfers which will hopefully receive normal handling. + */ if (ep->ep_state & EP_HAS_STREAMS) { + /* We don't know which stream failed, attempt Soft Retry */ reset_type = EP_SOFT_RESET; } else { reset_type = EP_HARD_RESET; @@ -1231,39 +1247,31 @@ static void xhci_handle_cmd_stop_ep(struct xhci_hcd *xhci, int slot_id, if (td) td->status = -EPROTO; } - /* 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) + /* persistent problem or disconnection, we must give back TDs */ break; reset_done: /* Reset EP handler will clean up cancelled TDs */ ep->ep_state &= ~EP_STOP_CMD_PENDING; return; + case EP_STATE_STOPPED: /* - * Per xHCI 4.6.9, Stop Endpoint command on a Stopped - * EP is a Context State Error, and EP stays Stopped. - * - * But maybe it failed on Halted, and somebody ran Reset - * Endpoint later. EP state is now Stopped and EP_HALTED - * still set because Reset EP handler will run after us. + * Maybe the command failed in Halted state and the transfer event queued + * Reset Endpoint. The endpoint is now Stopped and EP_HALTED is still set, + * because Reset Endpoint completion handler will run after this one. */ if (ep->ep_state & EP_HALTED) break; /* - * On some HCs EP state remains Stopped for some tens of - * us to a few ms or more after a doorbell ring, and any - * new Stop Endpoint fails without aborting the restart. - * This handler may run quickly enough to still see this - * 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. + * We don't queue Stop Endpoint on Stopped endpoints. Assume the pending + * restart case and keep retrying until it starts and stops again. */ 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)); /*