Re: [PATCH v13 14/15] 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] PCI/CXL: unsafe fallback to FLR on cxl_reset safety check failure
- [High] PCI/CXL: cxl_reset rejected during probe if features disabled
--

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

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

This commit registers cxl_reset with the PCI reset-method interface, placing
it before FLR. It adds capability and state checks to ensure the device
supports and is configured for CXL cache and memory protocols. It also
returns -ENOTTY when the device capabilities or required cache
synchronization do not permit a safe reset.

> 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
> @@ ... @@
>  int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  {
>  	int dvsec, rc;
>  	u16 cap, ctrl;
> 
> [ ... ]
> 
>  	if (!(cap & PCI_DVSEC_CXL_CACHE_CAPABLE) ||
>  	    !(cap & PCI_DVSEC_CXL_MEM_CAPABLE) ||
>  	    !(cap & PCI_DVSEC_CXL_RST_CAPABLE) ||
>  	    !(cap & PCI_DVSEC_CXL_RST_MEM_CLR_CAPABLE))
>  		return -ENOTTY;

[Severity: High]
Does returning -ENOTTY here cause the PCI core to silently fall back to FLR
during an actual reset request? 

Type 3 devices (memory expanders) lack CXL.cache capabilities, so they would
fail this check. If they fall back to FLR, it resets the PCIe state but not
the CXL protocol state. For devices with active CXL memory, this might lead
to Machine Check Exceptions or memory corruption when the CPU interacts with
the reset device. 

Would returning a hard error like -EOPNOTSUPP during execution safely abort
the unsafe reset?

>  
>  	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]
Does evaluating this dynamic state during probe permanently exclude the
reset method before the driver can enable the features?

When pci_init_reset_methods() calls cxl_reset_function() with probe == true
during early device discovery, returning -ENOTTY causes the PCI core to omit
this method entirely from the supported reset methods.

If this check evaluates dynamic state before the early-return for probe, it
might cause hot-plugged devices to silently fall back to FLR when a reset
is later requested. Could this check be moved after the if (probe)
early-return?

> 
> [ ... ]
> 
>  	if (probe)
>  		return 0;
> 
> [ ... ]
> 
>  	if (!cpu_cache_has_invalidate_memregion())
>  		return -ENOTTY;

[Severity: High]
Similar to the capability check above, does returning -ENOTTY here fail to
safely abort the reset? 

Since the PCI framework interprets -ENOTTY as "method not applicable, try
next", this causes a fallback to the unsafe FLR method. Would returning a
hard error prevent this unsafe fallback?

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