Re: [PATCH 2/2] cxl/core: Recover from PM Init failure via cxl_reset_bus_function()

[Dave Jiang <dave.jiang@intel.com>]

On 4/28/26 11:24 AM, Fabio M. De Francesco wrote:
> CXL r4.0 sec 8.1.5.1 Implementation Note describes a scenario in which a
> Secondary Bus Reset, a Link Down, or Downstream Port Containment on a
> CXL Downstream Port prevents Port PM Init from completing when ACS
> Source Validation is enabled.
> 
> During CXL enumeration, for each CXL Downstream Port in a memdev's
> ancestry, check whether PM Init has completed. If it has not, invoke
> cxl_reset_bus_function() which is exported for use by CXL.
> 
> Signed-off-by: Fabio M. De Francesco <fabio.m.de.francesco@linux.intel.com>
> ---
>  drivers/cxl/core/pci.c        | 30 ++++++++++++++++++++++++++++++
>  drivers/cxl/core/port.c       | 22 ++++++++++++++++++++++
>  drivers/cxl/cxlpci.h          |  3 +++
>  drivers/pci/pci.c             |  3 ++-
>  include/linux/pci.h           |  1 +
>  include/uapi/linux/pci_regs.h |  2 ++
>  6 files changed, 60 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index d1f487b3d809..de6a317df650 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -926,3 +926,33 @@ int cxl_port_get_possible_dports(struct cxl_port *port)
>  
>  	return ctx.count;
>  }
> +
> +/**
> + * cxl_port_pm_init_is_complete - check the downstream port's PM Init Complete
> + * @pdev: downstream port
> + *
> + * Read the Port Power Management Initialization Complete bit in the
> + * Downstream Port's CXL DVSEC Port Extended Status register.
> + *
> + * Return: false only when the bit is observably clear. Return true when PM
> + * init is complete, when @pdev is not a CXL port (no Port DVSEC), or when
> + * the status register cannot be read.
> + */
> +bool cxl_port_pm_init_is_complete(struct pci_dev *pdev)
> +{
> +	u16 status;
> +	u16 dvsec;
> +	int rc;
> +
> +	dvsec = pci_find_dvsec_capability(pdev, PCI_VENDOR_ID_CXL,
> +					  PCI_DVSEC_CXL_PORT);
> +	if (!dvsec)
> +		return true;
> +
> +	rc = pci_read_config_word(pdev, dvsec + PCI_DVSEC_CXL_PORT_EXT_STATUS,
> +				  &status);
> +	if (rc || PCI_POSSIBLE_ERROR(status))
> +		return true;
> +
> +	return !!FIELD_GET(PCI_DVSEC_CXL_PORT_EXT_STATUS_PM_INIT_COMP, status);
> +}
> diff --git a/drivers/cxl/core/port.c b/drivers/cxl/core/port.c
> index c5aacd7054f1..a91841855d3b 100644
> --- a/drivers/cxl/core/port.c
> +++ b/drivers/cxl/core/port.c
> @@ -1825,6 +1825,28 @@ int devm_cxl_enumerate_ports(struct cxl_memdev *cxlmd)
>  		if (is_cxl_host_bridge(dport_dev))
>  			return 0;
>  
> +		/*
> +		 * Check the downstream port's PM init status, and if it has
> +		 * failed retry PM init according to CXL Spec. 4.0 Sect. 8.1.5.1
> +		 * - Implementation Note
> +		 */
> +		if (dev_is_pci(dport_dev) && dev_is_pci(iter->parent)) {
> +			struct pci_dev *dport_pdev = to_pci_dev(dport_dev);
> +
> +			if (!cxl_port_pm_init_is_complete(dport_pdev)) {
> +				dev_dbg(&cxlmd->dev,
> +					"PM init failed for %s, retrying PM init\n",
> +					dev_name(dport_dev));
> +
> +				cxl_reset_bus_function(to_pci_dev(iter->parent), false);
> +
> +				if (!cxl_port_pm_init_is_complete(dport_pdev))
> +					dev_dbg(&cxlmd->dev,
> +						"PM init failed retry for %s\n",
> +						dev_name(dport_dev));
> +			}
> +		}

Make this a helper function and move it to core/pci.c. Also pass 'struct device' and not 'struct pci_dev' to that function.

I do wonder if you reset it here, maybe you need to restart enumeration from the endpoint instead of just continuing as if nothing happened.

DJ

> +
>  		uport_dev = dport_dev->parent;
>  		if (!uport_dev) {
>  			dev_warn(dev, "at %s no parent for dport: %s\n",
> diff --git a/drivers/cxl/cxlpci.h b/drivers/cxl/cxlpci.h
> index b826eb53cf7b..c66ff2ce82a3 100644
> --- a/drivers/cxl/cxlpci.h
> +++ b/drivers/cxl/cxlpci.h
> @@ -114,4 +114,7 @@ static inline void devm_cxl_port_ras_setup(struct cxl_port *port)
>  
>  int cxl_pci_setup_regs(struct pci_dev *pdev, enum cxl_regloc_type type,
>  		       struct cxl_register_map *map);
> +
> +bool cxl_port_pm_init_is_complete(struct pci_dev *pdev);
> +
>  #endif /* __CXL_PCI_H__ */
> diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
> index 047d3b4508a5..ae30da22daf4 100644
> --- a/drivers/pci/pci.c
> +++ b/drivers/pci/pci.c
> @@ -4974,7 +4974,7 @@ static void cxl_restore_acs_sv_bme(struct pci_dev *bridge, u16 saved_cmd,
>   * Return: 0 on success, -ENOTTY if the reset cannot be issued, or an
>   * errno from the reset path.
>   */
> -static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
> +int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  {
>  	struct pci_dev *bridge;
>  	u16 dvsec, reg, val;
> @@ -5023,6 +5023,7 @@ static int cxl_reset_bus_function(struct pci_dev *dev, bool probe)
>  	pci_dev_reset_iommu_done(dev);
>  	return rc;
>  }
> +EXPORT_SYMBOL_NS_GPL(cxl_reset_bus_function, "CXL");
>  
>  void pci_dev_lock(struct pci_dev *dev)
>  {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c11..1fb1360d41e8 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -1477,6 +1477,7 @@ int pci_probe_reset_slot(struct pci_slot *slot);
>  int pci_probe_reset_bus(struct pci_bus *bus);
>  int pci_reset_bus(struct pci_dev *dev);
>  void pci_reset_secondary_bus(struct pci_dev *dev);
> +int cxl_reset_bus_function(struct pci_dev *dev, bool probe);
>  void pcibios_reset_secondary_bus(struct pci_dev *dev);
>  void pci_update_resource(struct pci_dev *dev, int resno);
>  int __must_check pci_assign_resource(struct pci_dev *dev, int i);
> diff --git a/include/uapi/linux/pci_regs.h b/include/uapi/linux/pci_regs.h
> index 14f634ab9350..7e2579f89041 100644
> --- a/include/uapi/linux/pci_regs.h
> +++ b/include/uapi/linux/pci_regs.h
> @@ -1369,6 +1369,8 @@
>  
>  /* CXL r4.0, 8.1.5: Extensions DVSEC for Ports */
>  #define PCI_DVSEC_CXL_PORT				3
> +#define  PCI_DVSEC_CXL_PORT_EXT_STATUS			0x0A
> +#define   PCI_DVSEC_CXL_PORT_EXT_STATUS_PM_INIT_COMP	_BITUL(0)
>  #define  PCI_DVSEC_CXL_PORT_CTL				0x0c
>  #define   PCI_DVSEC_CXL_PORT_CTL_UNMASK_SBR		0x00000001
>