From: sashiko-bot@kernel.org
To: "Srirangan Madhavan" <smadhavan@nvidia.com>
Cc: linux-pci@vger.kernel.org, linux-cxl@vger.kernel.org
Subject: Re: [PATCH v10 06/12] cxl: Add CXL Device Reset helper
Date: Tue, 04 Aug 2026 19:42:20 +0000 [thread overview]
Message-ID: <20260804194221.4C42B1F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260804192958.1823952-7-smadhavan@nvidia.com>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The polling loop in cxl_reset_wait_cache_wbi() misses a final state check upon timeout and is vulnerable to returning a false timeout (-ETIMEDOUT) if the thread is preempted.
- [Low] usleep_range() is called with an overly tight 1-microsecond delta, preventing timer slack batching.
--
--- Patch [6]: [PATCH v10 06/12] cxl: Add CXL Device Reset helper ---
commit 7129cc4ec4e3e4cb936b9c61b556eecc1cfb46fa
Author: Srirangan Madhavan <smadhavan@nvidia.com>
cxl: Add CXL Device Reset helper
Add an internal CXL Device Reset helper for Type 2 functions that advertise
CXL Reset in the CXL Device DVSEC. The helper disables CXL.cache, performs
cache writeback, initiates reset, waits for completion, and re-enables
CXL.cache.
> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c
[ ... ]
> +static int cxl_reset_wait_cache_wbi(struct pci_dev *pdev, int dvsec)
> +{
> + unsigned long deadline;
> + u16 status2;
> + int rc;
> +
> + rc = cxl_reset_set_ctrl2(pdev, dvsec, PCI_DVSEC_CXL_INIT_CACHE_WBI);
> + if (rc)
> + return rc;
> +
> + deadline = jiffies + usecs_to_jiffies(CXL_CACHE_WBI_TIMEOUT_US);
> + do {
> + usleep_range(CXL_CACHE_WBI_POLL_US, CXL_CACHE_WBI_POLL_US + 1);
[Severity: Low]
This isn't a bug, but does this 1-microsecond delta defeat the high-resolution
timer subsystem's ability to coalesce timer wakeups?
Could we use a wider range here to allow timer slack batching and reduce CPU
overhead?
> +
> + rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_STATUS2,
> + &status2);
> + if (rc)
> + return pcibios_err_to_errno(rc);
> + if (status2 != U16_MAX && (status2 & PCI_DVSEC_CXL_CACHE_INV))
> + return 0;
> + } while (time_before(jiffies, deadline));
> +
> + return -ETIMEDOUT;
> +}
[Severity: Medium]
Is this loop missing a final state check upon timeout?
If the thread is preempted or experiences a scheduling delay immediately
after reading the hardware status but before evaluating time_before(), and the
delay pushes jiffies past the deadline, the loop will exit and return
-ETIMEDOUT.
Could this return a false timeout if the hardware actually completed the
operation during that scheduling delay? Unlike cxl_reset_wait_done(), which
correctly handles this with a final check, this loop appears vulnerable.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260804192958.1823952-1-smadhavan@nvidia.com?part=6
next prev parent reply other threads:[~2026-08-04 19:42 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-04 19:29 [PATCH v10 00/12] PCI/CXL: Add CXL reset support for Type 2 devices Srirangan Madhavan
2026-08-04 19:29 ` [PATCH v10 01/12] cxl: Move HDM decoder programming helpers Srirangan Madhavan
2026-08-04 19:46 ` sashiko-bot
2026-08-05 2:13 ` Alison Schofield
2026-08-04 19:29 ` [PATCH v10 02/12] cxl: Pass decoder settings to HDM commit helpers Srirangan Madhavan
2026-08-04 19:49 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 03/12] cxl: Share HDM decoder decode logic Srirangan Madhavan
2026-08-04 19:45 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 04/12] cxl: Cache decoder settings on PCI devices Srirangan Madhavan
2026-08-04 19:40 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 05/12] cxl: Cache endpoint decoder settings during PCI enumeration Srirangan Madhavan
2026-08-04 19:51 ` sashiko-bot
2026-08-05 2:28 ` Alison Schofield
2026-08-04 19:29 ` [PATCH v10 06/12] cxl: Add CXL Device Reset helper Srirangan Madhavan
2026-08-04 19:42 ` sashiko-bot [this message]
2026-08-04 19:29 ` [PATCH v10 07/12] cxl: Validate HDM ranges before CXL reset Srirangan Madhavan
2026-08-04 19:38 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 08/12] cxl: Reject CXL Reset on multifunction devices Srirangan Madhavan
2026-08-04 19:40 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 09/12] cxl: Restore CXL HDM state after PCI reset Srirangan Madhavan
2026-08-04 19:44 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 10/12] PCI/CXL: Expose CXL Reset as a PCI reset method Srirangan Madhavan
2026-08-04 20:00 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 11/12] Documentation/ABI: Document CXL Reset " Srirangan Madhavan
2026-08-04 19:41 ` sashiko-bot
2026-08-04 19:29 ` [PATCH v10 12/12] PCI/CXL: Restore HDM state after CXL bus reset Srirangan Madhavan
2026-08-04 19:59 ` sashiko-bot
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260804194221.4C42B1F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=linux-cxl@vger.kernel.org \
--cc=linux-pci@vger.kernel.org \
--cc=sashiko-reviews@lists.linux.dev \
--cc=smadhavan@nvidia.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox