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: Michal Pecio X-Patchwork-Id: 872401 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:41:29 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michal Pecio X-Patchwork-Id: 872399 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) {