Linux PCI subsystem development
 help / color / mirror / Atom feed
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

  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