Re: [PATCH v15 10/16] PCI/CXL: Add CXL Device Reset sequencing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] pci: CXL reset timeout silently clamped to 20s, invalidating 100s capability
--

commit 40437b091280a3bac8200508563fab32b5d72e62
Author: Srirangan Madhavan <smadhavan@nvidia.com>

PCI/CXL: Add CXL Device Reset sequencing

This commit implements CXL Device Reset for eligible Type 2 devices. It
introduces functions to disable CXL.cache, initiate reset with Memory
Clear, and wait for completion.

Link: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?id=04f8f84816d724f8b47f4d47a77916ac37c63a49

> diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c
> index 471b57391593e..9eb13adf0daa3 100644
> --- a/drivers/pci/cxl.c
> +++ b/drivers/pci/cxl.c

[ ... ]

> +static int cxl_reset_wait_done(struct pci_dev *pdev, int dvsec, u16 cap)
> +{
> +	unsigned long deadline;
> +	u32 timeout_ms;
> +	u16 status2;
> +	int idx, rc;
> +
> +	idx = FIELD_GET(PCI_DVSEC_CXL_RST_TIMEOUT, cap);
> +	if (idx >= ARRAY_SIZE(cxl_reset_timeout_ms)) {
> +		pci_warn(pdev,
> +			 "unknown CXL reset timeout encoding %d; using %u ms\n",
> +			 idx, CXL_RESET_MAX_WAIT_MS);
> +		idx = ARRAY_SIZE(cxl_reset_timeout_ms) - 1;
> +	}
> +
> +	/* Allow one retry after the mandatory 100 ms quiet period. */
> +	timeout_ms = clamp_t(u32, cxl_reset_timeout_ms[idx],
> +			     CXL_RESET_RRS_WAIT_MS + CXL_RESET_STATUS_POLL_MS,
> +			     CXL_RESET_MAX_WAIT_MS);

[Severity: Medium]
Does this clamp_t() logic unintentionally override the 100-second 
capability?

The CXL 4.0 specification defines reset timeout encodings up to 100 seconds, 
and the array cxl_reset_timeout_ms correctly defines 100,000 ms at index 4. 
However, the timeout calculation explicitly applies clamp_t() with a 
hardcoded upper bound of CXL_RESET_MAX_WAIT_MS, which is defined as 20,000 
(20 seconds).

When a device legitimately advertises a 100-second wait requirement via 
capability encodings, this silent truncation to 20 seconds could cause 
reset operations for compliant devices to prematurely fail with -ETIMEDOUT.

> +	msleep(CXL_RESET_RRS_WAIT_MS);
> +	deadline = jiffies + msecs_to_jiffies(timeout_ms -
> +					      CXL_RESET_RRS_WAIT_MS);

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261011021422.3428136-1-smadhavan@nvidia.com?part=10