Re: [PATCH v16 09/18] cxl: Refresh cached PCI HDM decoder settings

[Richard Cheng <icheng@nvidia.com>]

On Thu, Oct 29, 2026 at 02:56:54PM +0800, Srirangan Madhavan wrote:
> Early PCI discovery creates the HDM cache, while later CXL enumeration and
> decoder operations provide updated programming state.
> 
> Refresh the PCI snapshot when decoders are enumerated, committed, or reset
> so reset recovery need not walk the CXL topology. Keep cache access and
> locking in the built-in HDM state helpers. Ignore updates when no cache
> exists and reject decoder-count mismatches.
> 
> Use CXL_RESET for the PCI HDM cache. Shared HDM register helpers do not
> require it.
> 
> Skip cache checks and updates for DVSEC-emulated decoders, which do not
> use HDM registers. Keep cached global control when MMIO reads return all
> ones.
> 
> Non-PCI mock ports have no upstream PCI device, so the cache helpers
> ignore them. This avoids test wrappers and a circular dependency between
> cxl_core and cxl_mock.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> Assisted-by: LLM
> 

Reviewed-by: Richard Cheng <icheng@nvidia.com>

> ---
> Reference branch: https://git.kernel.org/pub/scm/linux/kernel/git/djiang/linux.git/commit/?id=50a88f5194e5df38590d413f925d8d5afa8a8488
> ---
>  drivers/cxl/Kconfig          |  5 ++++
>  drivers/cxl/core/core.h      |  1 +
>  drivers/cxl/core/hdm.c       | 34 +++++++++++++++++++--
>  drivers/cxl/core/hdm_state.c | 57 ++++++++++++++++++++++++++++++++++++
>  drivers/cxl/core/pci.c       | 24 +++++++++++++++
>  include/cxl/cxl.h            |  1 +
>  include/cxl/hdm.h            | 32 ++++++++++++++++++++
>  include/linux/pci.h          |  6 ++++
>  8 files changed, 158 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/cxl/Kconfig b/drivers/cxl/Kconfig
> index 80aeb0d556bd..8b08bfdc4982 100644
> --- a/drivers/cxl/Kconfig
> +++ b/drivers/cxl/Kconfig
> @@ -6,6 +6,7 @@ menuconfig CXL_BUS
>  	select FW_UPLOAD
>  	select PCI_DOE
>  	select FIRMWARE_TABLE
> +	select CXL_RESET
>  	select NUMA_KEEP_MEMINFO if NUMA_MEMBLKS
>  	select FWCTL if CXL_FEATURES
>  	help
> @@ -244,3 +245,7 @@ config CXL_ATL
>  	depends on ACPI_PRMT && AMD_NB
>  
>  endif
> +
> +# CXL_BUS requires the PCI HDM cache and built-in state helpers, even when modular.
> +config CXL_RESET
> +	bool
> diff --git a/drivers/cxl/core/core.h b/drivers/cxl/core/core.h
> index ea539f00e55d..e34d574f6594 100644
> --- a/drivers/cxl/core/core.h
> +++ b/drivers/cxl/core/core.h
> @@ -224,6 +224,7 @@ static inline void devm_cxl_dport_ras_setup(struct cxl_dport *dport) { }
>  int cxl_gpf_port_setup(struct cxl_dport *dport);
>  
>  struct cxl_hdm;
> +int cxl_hdm_check_decoder_count(struct cxl_hdm *cxlhdm);
>  int cxl_hdm_decode_init(struct cxl_dev_state *cxlds, struct cxl_hdm *cxlhdm,
>  			struct cxl_endpoint_dvsec_info *info);
>  int cxl_port_get_possible_dports(struct cxl_port *port);
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 85f9abf58a42..ec387070d7ad 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -725,6 +725,24 @@ static void cxlsd_set_targets(struct cxl_switch_decoder *cxlsd, u64 *tgt)
>  		*tgt |= FIELD_PREP(GENMASK_ULL(63, 56), t[7]->port_id);
>  }
>  
> +static void cxl_hdm_refresh_decoder(struct cxl_hdm *cxlhdm,
> +				    struct cxl_decoder *cxld)
> +{
> +	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
> +	struct pci_dev *pdev __free(pci_dev_put) = NULL;
> +
> +	if (!hdm)
> +		return;
> +	if (!is_endpoint_decoder(&cxld->dev))
> +		return;
> +
> +	pdev = cxl_port_get_uport_pci_dev(cxlhdm->port);
> +	if (!pdev)
> +		return;
> +	cxl_hdm_cache_refresh_decoder(pdev, hdm,
> +				      to_cxl_endpoint_decoder(&cxld->dev));
> +}
> +
>  static void
>  cxl_endpoint_decoder_snapshot(struct cxl_endpoint_decoder *cxled,
>  			      struct cxl_endpoint_decoder_settings *settings)
> @@ -805,6 +823,7 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
>  	}
>  	port->commit_end++;
>  	cxld->config.flags |= CXL_DECODER_F_ENABLE;
> +	cxl_hdm_refresh_decoder(cxlhdm, cxld);
>  
>  	return 0;
>  }
> @@ -877,6 +896,7 @@ static void cxl_decoder_reset(struct cxl_decoder *cxld)
>  	writel(0, hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(id));
>  
>  	cxld->config.flags &= ~CXL_DECODER_F_ENABLE;
> +	cxl_hdm_refresh_decoder(cxlhdm, cxld);
>  
>  	/* Userspace is now responsible for reconfiguring this decoder */
>  	if (is_endpoint_decoder(&cxld->dev)) {
> @@ -1079,13 +1099,21 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  {
>  	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
>  	struct cxl_port *port = cxlhdm->port;
> -	int i;
> +	/* DVSEC-emulated decoders do not describe the cached HDM registers. */
> +	bool emulated = should_emulate_decoders(info);
>  	u64 dpa_base = 0;
> +	int i, rc;
> +
> +	if (is_cxl_endpoint(port) && hdm && !emulated) {
> +		rc = cxl_hdm_check_decoder_count(cxlhdm);
> +		if (rc)
> +			return rc;
> +	}
>  
>  	cxl_settle_decoders(cxlhdm);
>  
>  	for (i = 0; i < cxlhdm->decoder_count; i++) {
> -		int rc, target_count = cxlhdm->target_count;
> +		int target_count = cxlhdm->target_count;
>  		struct cxl_decoder *cxld;
>  
>  		if (is_cxl_endpoint(port)) {
> @@ -1120,6 +1148,8 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  			put_device(&cxld->dev);
>  			return rc;
>  		}
> +		if (!emulated)
> +			cxl_hdm_refresh_decoder(cxlhdm, cxld);
>  		rc = add_hdm_decoder(port, cxld);
>  		if (rc) {
>  			dev_warn(&port->dev,
> diff --git a/drivers/cxl/core/hdm_state.c b/drivers/cxl/core/hdm_state.c
> index c9cc019902ef..d4c2be1ef833 100644
> --- a/drivers/cxl/core/hdm_state.c
> +++ b/drivers/cxl/core/hdm_state.c
> @@ -1,6 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /* Copyright (c) 2026 NVIDIA Corporation & Affiliates */
>  #include <linux/export.h>
> +#include <linux/io.h>
> +#include <linux/pci.h>
>  #include <linux/rwsem.h>
>  
>  #include <cxl/hdm.h>
> @@ -13,3 +15,58 @@ struct cxl_rwsem cxl_rwsem = {
>  	.dpa = __RWSEM_INITIALIZER(cxl_rwsem.dpa),
>  };
>  EXPORT_SYMBOL_FOR_MODULES(cxl_rwsem, "cxl_core");
> +
> +void cxl_hdm_cache_refresh_decoder(struct pci_dev *pdev, void __iomem *hdm,
> +				   struct cxl_endpoint_decoder *cxled)
> +{
> +	struct cxl_endpoint_decoder_settings *settings;
> +	struct cxl_decoder *cxld = &cxled->cxld;
> +	struct cxl_hdm_info *info;
> +	u32 global_ctrl;
> +	int id;
> +
> +	guard(rwsem_write)(&cxl_rwsem.dpa);
> +	id = cxld->config.id;
> +	info = pdev->hdm;
> +	if (!info)
> +		return;
> +	if (id < 0 || id >= info->decoder_count) {
> +		pci_warn(pdev, "CXL HDM decoder %d exceeds cached count %d\n",
> +			 id, info->decoder_count);
> +		return;
> +	}
> +
> +	/* Preserve the last good value when the device does not respond. */
> +	global_ctrl = readl(hdm + CXL_HDM_DECODER_CTRL_OFFSET);
> +	if (global_ctrl != U32_MAX)
> +		info->global_ctrl = global_ctrl;
> +	settings = &info->settings[id];
> +
> +	/*
> +	 * A disabled decoder's software object may retain its old range and
> +	 * skip state. Leave only the decoder id in the cached settings so stale
> +	 * state is not restored as an enabled decode.
> +	 */
> +	*settings = (struct cxl_endpoint_decoder_settings) {
> +		.config.id = id,
> +	};
> +	if (cxld->config.flags & CXL_DECODER_F_ENABLE) {
> +		settings->config = cxld->config;
> +		settings->skip = cxled->skip;
> +	}
> +}
> +EXPORT_SYMBOL_FOR_MODULES(cxl_hdm_cache_refresh_decoder, "cxl_core");
> +
> +int cxl_hdm_cache_check_decoder_count(struct pci_dev *pdev, int count)
> +{
> +	guard(rwsem_read)(&cxl_rwsem.dpa);
> +	if (pdev->hdm && pdev->hdm->decoder_count != count) {
> +		pci_warn(pdev,
> +			 "CXL HDM cache decoder count mismatch: cached=%d hdm=%d\n",
> +			 pdev->hdm->decoder_count, count);
> +		return -ENXIO;
> +	}
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_FOR_MODULES(cxl_hdm_cache_check_decoder_count, "cxl_core");
> diff --git a/drivers/cxl/core/pci.c b/drivers/cxl/core/pci.c
> index 33b21c8dcb21..e2034a2179b4 100644
> --- a/drivers/cxl/core/pci.c
> +++ b/drivers/cxl/core/pci.c
> @@ -6,6 +6,7 @@
>  #include <linux/delay.h>
>  #include <linux/pci.h>
>  #include <linux/pci-doe.h>
> +#include <cxl/hdm.h>
>  #include <cxl/pci.h>
>  #include <linux/aer.h>
>  #include <cxlpci.h>
> @@ -180,6 +181,29 @@ int cxl_await_media_ready(struct cxl_dev_state *cxlds)
>  }
>  EXPORT_SYMBOL_NS_GPL(cxl_await_media_ready, "CXL");
>  
> +/**
> + * cxl_hdm_check_decoder_count() - check enumeration against the PCI HDM cache
> + * @cxlhdm: HDM capability containing the decoder count to check
> + *
> + * Take cxl_rwsem.dpa for read while comparing the cached count. Call only
> + * for hardware HDM decoders, since DVSEC-emulated ranges do not describe
> + * the HDM registers represented by the cache.
> + *
> + * Return: 0 if the counts match or no upstream PCI device or cache exists,
> + * or -ENXIO if the cached and enumerated decoder counts differ.
> + */
> +int cxl_hdm_check_decoder_count(struct cxl_hdm *cxlhdm)
> +{
> +	struct pci_dev *pdev __free(pci_dev_put) =
> +		cxl_port_get_uport_pci_dev(cxlhdm->port);
> +
> +	if (!pdev)
> +		return 0;
> +
> +	return cxl_hdm_cache_check_decoder_count(pdev, cxlhdm->decoder_count);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_hdm_check_decoder_count, "CXL");
> +
>  static int cxl_set_mem_enable(struct cxl_dev_state *cxlds, u16 val)
>  {
>  	struct pci_dev *pdev = to_pci_dev(cxlds->dev);
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 6d6e564be27a..341a7c52894a 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -26,6 +26,7 @@ enum cxl_devtype {
>  };
>  
>  struct cxl_region;
> +struct cxl_hdm_info;
>  
>  enum cxl_decoder_type {
>  	CXL_DECODER_DEVMEM = 2,
> diff --git a/include/cxl/hdm.h b/include/cxl/hdm.h
> index 219b9bb67440..06e9aa4479c2 100644
> --- a/include/cxl/hdm.h
> +++ b/include/cxl/hdm.h
> @@ -6,6 +6,9 @@
>  #include <linux/types.h>
>  #include <cxl/cxl.h>
>  
> +struct pci_dev;
> +struct cxl_endpoint_decoder;
> +
>  /**
>   * struct cxl_endpoint_decoder_settings - endpoint HDM decoder programming
>   * @config: common decoder configuration
> @@ -16,4 +19,33 @@ struct cxl_endpoint_decoder_settings {
>  	u64 skip;
>  };
>  
> +/**
> + * struct cxl_hdm_info - cached CXL HDM state for a PCI device
> + * @decoder_count: number of entries in @settings
> + * @hdm_bar: PCI BAR containing the HDM decoder capability
> + * @hdm_offset: offset of the HDM decoder capability in @hdm_bar
> + * @hdm_size: size of the HDM decoder register block
> + * @global_ctrl: HDM decoder global control register
> + * @dvsec_ctrl: CXL DVSEC control register
> + * @dvsec_ctrl_valid: whether @dvsec_ctrl contains valid state
> + * @settings: per-endpoint-decoder programming state
> + *
> + * Publication through pci_dev.hdm and all mutable fields are protected by
> + * cxl_rwsem.dpa.
> + */
> +struct cxl_hdm_info {
> +	int decoder_count;
> +	int hdm_bar;
> +	resource_size_t hdm_offset;
> +	resource_size_t hdm_size;
> +	u32 global_ctrl;
> +	u16 dvsec_ctrl;
> +	bool dvsec_ctrl_valid;
> +	struct cxl_endpoint_decoder_settings settings[] __counted_by(decoder_count);
> +};
> +
> +void cxl_hdm_cache_refresh_decoder(struct pci_dev *pdev, void __iomem *hdm,
> +				   struct cxl_endpoint_decoder *cxled);
> +int cxl_hdm_cache_check_decoder_count(struct pci_dev *pdev, int count);
> +
>  #endif /* __CXL_HDM_H__ */
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index d31a8d107b1e..7bb37fcb556d 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -339,6 +339,9 @@ struct pcie_link_state;
>  struct pci_sriov;
>  struct pci_p2pdma;
>  struct rcec_ea;
> +#ifdef CONFIG_CXL_RESET
> +struct cxl_hdm_info;
> +#endif
>  
>  /* struct pci_dev - describes a PCI device
>   *
> @@ -566,6 +569,9 @@ struct pci_dev {
>  #ifdef CONFIG_PCI_DOE
>  	struct xarray	doe_mbs;	/* Data Object Exchange mailboxes */
>  #endif
> +#ifdef CONFIG_CXL_RESET
> +	struct cxl_hdm_info *hdm;	/* CXL HDM decoder state */
> +#endif
>  #ifdef CONFIG_PCI_NPEM
>  	struct npem	*npem;		/* Native PCIe Enclosure Management */
>  #endif
> -- 
> 2.43.0
>