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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.