Re: [PATCH v17 14/19] PCI/CXL: Reject reset with unsafe function scope

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] PCI/CXL: incomplete multifunction check in cxl_reset_function() and cxl_reset_bus_function()
--

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

PCI/CXL: Reject reset with unsafe function scope

This commit rejects cxl_reset and cxl_bus reset methods for virtual
functions, multifunction devices, and physical functions with enabled
virtual functions so per-device resets do not reset sibling functions.

Link: https://lore.kernel.org/linux-cxl/20260626160846.426e71b6@nvidia.com/
Link: https://lore.kernel.org/linux-cxl/6a50441e2bb5e_3cabcb1003e@djbw-dev.notmuch/

> diff --git a/drivers/pci/cxl.c b/drivers/pci/cxl.c
> index af8ec8db20f12..c07266ab01336 100644
> --- a/drivers/pci/cxl.c
> +++ b/drivers/pci/cxl.c
> @@ -501,6 +501,9 @@ int cxl_reset_function(struct pci_dev *pdev, bool probe)
>  	u16 dvsec, cap, ctrl;
>  	int rc;
>  
> +	if (pdev->is_virtfn || pdev->multifunction || pci_num_vf(pdev))
> +		return -ENOTTY;
> +
>  	rc = cxl_pci_device_dvsec(pdev, &dvsec, &cap);
>  	if (rc)
>  		return rc;
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index b2879a6be5f80..aef976d0c850e 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -5018,6 +5018,13 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  	u16 dvsec, reg, val;
>  	int rc;
>  
> +	/*
> +	 * Reject VFs and multifunction devices before walking the bridge's
> +	 * capabilities: every VF behind the same bridge would repeat the walk.
> +	 */
> +	if (dev->is_virtfn || dev->multifunction || pci_num_vf(dev))
> +		return -ENOTTY;

[Severity: High]
Is checking dev->multifunction sufficient to catch all multifunction CXL
devices during reset probing and execution in both cxl_reset_function()
and cxl_reset_bus_function()?

During enumeration in drivers/pci/probe.c:pci_scan_slot(),
pci_scan_single_device() runs pci_init_reset_methods() before
dev->multifunction = 1 is set for non-zero functions:

drivers/pci/probe.c:pci_scan_slot()
  pci_scan_single_device(bus, devfn + fn)
    pci_device_add()
      pci_init_capabilities()
        pci_init_reset_methods()
  if (fn > 0)
    dev->multifunction = 1;

Since non-zero functions of conventional multifunction devices are not
required to set PCI_HEADER_TYPE_MFD in their header type register, and ARI
devices must have PCI_HEADER_TYPE_MFD cleared in all functions, any
non-zero function with PCI_HEADER_TYPE_MFD == 0 still has
dev->multifunction == 0 when pci_init_reset_methods() probes
cxl_reset_function() and cxl_reset_bus_function(). As a result, probe does
not reject those functions early, walking the bridge capabilities and
recording cxl_reset and cxl_bus in dev->reset_methods[].

Furthermore, for Function 0 of an ARI multifunction device,
PCI_HEADER_TYPE_MFD is 0 by specification and pci_scan_slot() only sets
dev->multifunction = 1 when fn > 0, leaving Function 0 with
dev->multifunction == 0 even after sibling functions are enumerated. Would
resetting Function 0 of an ARI multifunction CXL device via
cxl_reset_function() (or via cxl_reset_bus_function() when the slot reset
path is used) bypass this check and reset active sibling functions?

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