Re: [PATCH v14 15/16] PCI/CXL: Expose CXL Reset as a PCI reset method

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] cxl: cxl_reset_function() incorrectly skips CXL reset for Type 3 devices
- [Medium] cxl: cxl_reset_function() incorrectly evaluates probe condition preventing registration
--

commit 75de83aed9edbce7d35622dfb455ec27e90edf7c
Author: Srirangan Madhavan <smadhavan@nvidia.com>

PCI/CXL: Expose CXL Reset as a PCI reset method

Register cxl_reset with the PCI reset-method interface and place it before
FLR, which does not reset CXL.cache or CXL.mem protocol state. Return
-ENOTTY when device capabilities, function scope, cached HDM state, or
required cache synchronization do not permit a safe reset.

> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> index 0acc471d0d7e2..efbab341b30f6 100644
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c
> @@ -1264,9 +1264,9 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  
>  	rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CAP, &cap);
>  	if (rc)
> -		return pcibios_err_to_errno(rc);
> +		return probe ? -ENOTTY : pcibios_err_to_errno(rc);
>  	if (PCI_POSSIBLE_ERROR(cap))
> -		return -ENODEV;
> +		return probe ? -ENOTTY : -ENODEV;
>  	if (!(cap & PCI_DVSEC_CXL_CACHE_CAPABLE) ||
>  	    !(cap & PCI_DVSEC_CXL_MEM_CAPABLE) ||
>  	    !(cap & PCI_DVSEC_CXL_RST_CAPABLE) ||

[Severity: High]
Does this logic correctly handle CXL Type 1 and Type 3 devices?

Type 3 devices (memory expanders) only use the CXL.mem protocol and do not
use CXL.cache. The bitwise AND requirements force the device to support BOTH
protocols. This causes standard Type 3 devices to fail the capability check
and fall back to standard FLR, which leaves the CXL.mem protocol state
un-reset.

> @@ -1275,9 +1275,9 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  
>  	rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_CTRL, &ctrl);
>  	if (rc)
> -		return pcibios_err_to_errno(rc);
> +		return probe ? -ENOTTY : pcibios_err_to_errno(rc);
>  	if (PCI_POSSIBLE_ERROR(ctrl))
> -		return -ENODEV;
> +		return probe ? -ENOTTY : -ENODEV;
>  	if (!(ctrl & PCI_DVSEC_CXL_CACHE_ENABLE) ||
>  	    !(ctrl & PCI_DVSEC_CXL_MEM_ENABLE))
>  		return -ENOTTY;

[Severity: High]
Similarly, does this incorrectly require both CXL.cache and CXL.mem to be
actively enabled? This will again exclude Type 1 and Type 3 CXL devices
which only enable one protocol.

[Severity: Medium]
Will this reset method fail to register at boot due to the evaluation of
the transient control state before the probe check?

PCI enumeration calls pci_init_reset_methods(), which evaluates the reset
methods with probe=true. Because PCI_DVSEC_CXL_CTRL enable bits are a
transient state typically set later by the CXL driver, the ctrl check here
fails and returns -ENOTTY during boot, silently omitting the reset method.

The probe early return occurs later in the function:

	if (!(ctrl & PCI_DVSEC_CXL_CACHE_ENABLE) ||
	    !(ctrl & PCI_DVSEC_CXL_MEM_ENABLE))
		return -ENOTTY;

	if (pdev->multifunction || pci_num_vf(pdev))
		return -ENOTTY;

	if (probe)
		return 0;

Because the transient driver state is evaluated before the probe check, it
prevents registration. Conversely, static capabilities like
cpu_cache_has_invalidate_memregion() are evaluated after the probe check,
bypassing them during probe but enforcing them at runtime. This can lead to
inconsistent reset availability.

Could the probe check be reordered to skip transient driver states while
still enforcing static architecture capability checks?

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