Re: [PATCH v14 14/16] cxl: Restore CXL state after PCI reset

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] cxl: PCI config space corruption if state not saved before reset
--

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

cxl: Restore CXL state after PCI reset

This commit restores the PCI configuration needed for HDM access, followed by
cached HDM decoder and CXL Device DVSEC state after a PCI reset. It stops at
the first restoration failure and preserves a disabled PCI command snapshot.

> diff --git a/drivers/cxl/core/resource.c b/drivers/cxl/core/resource.c
> index 679a605da24c4..0acc471d0d7e2 100644
> --- a/drivers/cxl/core/resource.c
> +++ b/drivers/cxl/core/resource.c

[ ... ]

> +static void cxl_restore_pci_state_for_hdm_restore(struct pci_dev *pdev,
> +						  u16 *command)
> +{
> +	u32 saved_config = pdev->saved_config_space[PCI_COMMAND / 4];
> +	u32 restore_config;
> +
> +	device_lock_assert(&pdev->dev);
> +	*command = saved_config & 0xffff;
> +
> +	/*
> +	 * pci_restore_state() is the available interface for restoring BARs and
> +	 * related PCI state needed to reach HDM MMIO. Temporarily alter its saved
> +	 * command value so that it enables MSE but cannot enable BME or INTx
> +	 * before CXL state restoration completes. Preserve the original saved
> +	 * value for the outer PCI reset path.
> +	 */
> +	restore_config = saved_config | PCI_COMMAND_MEMORY |
> +			 PCI_COMMAND_INTX_DISABLE;
> +	restore_config &= ~PCI_COMMAND_MASTER;
> +
> +	pdev->saved_config_space[PCI_COMMAND / 4] = restore_config;
> +	pci_restore_state(pdev);

[Severity: High]
Does this code corrupt the PCI configuration space if the state was not saved
before the reset?

cxl_restore_pci_state_for_hdm_restore() unconditionally calls
pci_restore_state() without verifying that pdev->state_saved is true.
pci_restore_state() writes the contents of pdev->saved_config_space back to
the hardware's PCI configuration registers. If the caller did not successfully
call pci_save_state() prior to the reset, this array contains uninitialized
zeroes (or stale data from a prior suspend cycle).

Writing this invalid state wipes the device's BARs and configuration,
permanently breaking access to the CXL device.

> +	pdev->saved_config_space[PCI_COMMAND / 4] = saved_config;
> +}

[ ... ]

> +static void cxl_reset_save_disabled_state(struct pci_dev *pdev)
> +{
> +	u32 *saved_command = &pdev->saved_config_space[PCI_COMMAND / 4];
> +	int rc;
> +
> +	rc = pci_write_config_word(pdev, PCI_COMMAND, PCI_COMMAND_INTX_DISABLE);
> +	if (rc)
> +		pci_err(pdev,
> +			"failed to disable device after CXL reset failure: %d\n",
> +			pcibios_err_to_errno(rc));
> +
> +	/* Preserve the pre-reset snapshot if PCI state has not been restored. */
> +	if (!pdev->state_saved) {
> +		rc = pci_save_state(pdev);

[Severity: High]
This error path explicitly handles the !pdev->state_saved case, which indicates
that a missing saved state is anticipated during a reset, yet it is
unconditionally restored on the success path.

> +		if (rc)
> +			pci_err(pdev,
> +				"failed to save disabled state after CXL reset failure: %d\n",
> +				rc);
> +	}

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