Re: [PATCH v4 04/10] PCI: add CXL reset method

Dave Jiang
2026-01-21T00:08:05Z

On 1/20/26 3:26 PM, smadhavan@nvidia.com wrote:
> From: Srirangan Madhavan <smadhavan@nvidia.com>
>
> Add a PCI reset method "cxl_reset" that drives the CXL reset sequence using
> DVSEC controls and timeout encoding. The method is restricted to
> Type 2 devices, limiting the scope of the changes.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202601172246.rz4Orygn-lkp@intel.com/

Don't think this is needed if it's kbot issue found during the series postings.

> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
> drivers/pci/pci.c | 104 ++++++++++++++++++++++++++++++++++++++++++++
> include/linux/pci.h | 10 ++++-
> 2 files changed, 113 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 8bb07e253646..e2d5ff25ab67 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4892,6 +4892,109 @@ static int pci_reset_bus_function(struct pci_dev *dev, bool probe)
> return pci_parent_bus_reset(dev, probe);
> }
>
> +static int cxl_reset_init(struct pci_dev *dev, u16 dvsec)

cxl_dev_reset() to go with existing reset method naming?

> +{
> + /*
> + * Timeout values ref CXL Spec v3.2 Ch 8 Control and Status Registers,
> + * under section 8.1.3.1 DVSEC CXL Capability.
> + */
> + u32 reset_timeouts_ms[] = { 10, 100, 1000, 10000, 100000 };
Should this be const?

> + u16 reg;
> + u32 timeout_ms;
> + int rc, ind;
> +
> + /* Check if CXL Reset MEM CLR is supported. */
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CAP_OFFSET, &reg);
> + if (rc)
> + return rc;
> +
> + if (reg & CXL_DVSEC_CXL_RST_MEM_CLR_CAPABLE) {
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET,
> + &reg);
> + if (rc)
> + return rc;
> +
> + reg |= CXL_DVSEC_CXL_RST_MEM_CLR_ENABLE;
> + pci_write_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET, reg);
> + }
> +
> + /* Read timeout value. */
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CAP_OFFSET, &reg);
> + if (rc)
> + return rc;
> + ind = FIELD_GET(CXL_DVSEC_CXL_RST_TIMEOUT_MASK, reg);
> + timeout_ms = reset_timeouts_ms[ind];
> +
> + /* Write reset config. */
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET, &reg);
> + if (rc)
> + return rc;
> +
> + reg |= CXL_DVSEC_INIT_CXL_RESET;
> + pci_write_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET, reg);
> +
> + /* Wait till timeout and then check reset status is complete. */
> + msleep(timeout_ms);
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_STATUS2_OFFSET, &reg);
> + if (rc)
> + return rc;
> + if (reg & CXL_DVSEC_CXL_RESET_ERR ||
> + ~reg & CXL_DVSEC_CXL_RST_COMPLETE)
> + return -ETIMEDOUT;
> +
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET, &reg);
> + if (rc)
> + return rc;
> + reg &= (~CXL_DVSEC_DISABLE_CACHING);
> + pci_write_config_word(dev, dvsec + CXL_DVSEC_CTRL2_OFFSET, reg);
> +
> + return 0;
> +}
> +
> +/**
> + * cxl_reset - initiate a cxl reset
> + * @dev: device to reset
> + * @probe: if true, return 0 if device can be reset this way
> + *
> + * Initiate a cxl reset on @dev.
> + */
> +static int cxl_reset(struct pci_dev *dev, bool probe)
> +{
> + u16 dvsec, reg;
> + int rc;
> +
> + dvsec = pci_find_dvsec_capability(dev, PCI_VENDOR_ID_CXL,
> + CXL_DVSEC_PCIE_DEVICE);
> + if (!dvsec)
> + return -ENOTTY;
> +
> + /* Check if CXL Reset is supported. */
> + rc = pci_read_config_word(dev, dvsec + CXL_DVSEC_CAP_OFFSET, &reg);
> + if (rc)
> + return -ENOTTY;
> +
> + if ((reg & CXL_DVSEC_CXL_RST_CAPABLE) == 0)
> + return -ENOTTY;
> +
> +#if !IS_REACHABLE(CONFIG_CXL_PCI)

Does this not require CONFIG_CXL_PCI to be built in (not module) to evaluate to 'y'? Also, instead of adding ifdef in the C code, maybe create a helper function for the remaining code within the function below and add an ifdef in the header depending on the config?

Although I think you may want to define cxl_is_type2_device() differently rather than relying on cxl_pci driver data. It probably should check the device type via config space instead to lessen the complications.

DJ

> + return -ENOTTY;
> +#endif
> +
> + /*
> + * Expose CXL reset for Type 2 devices.
> + */
> + if (!cxl_is_type2_device(dev))
> + return -ENOTTY;
> +
> + if (probe)
> + return 0;
> +
> + if (!pci_wait_for_pending_transaction(dev))
> + pci_err(dev, "timed out waiting for pending transaction; performing function level reset anyway\n");
> +
> + return cxl_reset_init(dev, dvsec);
> +}
> +
> static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
> {
> struct pci_dev *bridge;
> @@ -5016,6 +5119,7 @@ const struct pci_reset_fn_method pci_reset_fn_methods[] = {
> { pci_dev_acpi_reset, .name = "acpi" },
> { pcie_reset_flr, .name = "flr" },
> { pci_af_flr, .name = "af_flr" },
> + { cxl_reset, .name = "cxl_reset" },
> { pci_pm_reset, .name = "pm" },
> { pci_reset_bus_function, .name = "bus" },
> { cxl_reset_bus_function, .name = "cxl_bus" },
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 864775651c6f..4a8c4767db6e 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -51,7 +51,7 @@
> PCI_STATUS_PARITY)
>
> /* Number of reset methods used in pci_reset_fn_methods array in pci.c */
> -#define PCI_NUM_RESET_METHODS 8
> +#define PCI_NUM_RESET_METHODS 9
>
> #define PCI_RESET_PROBE true
> #define PCI_RESET_DO_RESET false
> @@ -1464,6 +1464,14 @@ int __must_check pci_resize_resource(struct pci_dev *dev, int i, int size,
>
> int pci_select_bars(struct pci_dev *dev, unsigned long flags);
> bool pci_device_is_present(struct pci_dev *pdev);
> +#ifdef CONFIG_CXL_PCI
> +bool cxl_is_type2_device(struct pci_dev *dev);
> +#else
> +static inline bool cxl_is_type2_device(struct pci_dev *dev)
> +{
> + return false;
> +}
> +#endif
> void pci_ignore_hotplug(struct pci_dev *dev);
> struct pci_dev *pci_real_dma_dev(struct pci_dev *dev);
> int pci_status_get_and_clear_errors(struct pci_dev *pdev);
> --
> 2.34.1
>