Re: [PATCH v13 08/15] cxl: Refresh cached PCI HDM decoder settings

[Cheatham, Benjamin]

On 9/22/2026 3:39 AM, 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. Ignore updates when no
> cache exists and reject decoder-count mismatches.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@xxxxxxxxxx>
> ---
>  drivers/cxl/core/hdm.c | 66 ++++++++++++++++++++++++++++++++++++++++++
>  include/cxl/cxl.h      | 22 ++++++++++++++
>  include/linux/pci.h    |  6 ++++
>  3 files changed, 94 insertions(+)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index 98268e8e15b2..0ae250ea4ce3 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -753,6 +753,51 @@ static void cxl_decoder_snapshot(struct cxl_decoder *cxld,
>  	}
>  }
>  
> +#ifdef CONFIG_CXL_RESET

The CXL maintainers are normally pretty against #ifdefs in c code. I was going to suggest
moving it to resource.c, but it looks like cxl_decoder_snapshot() needs to also be in this
file. So, I think the fix is to put:

if (!IS_ENABLED(CONFIG_CXL_RESET))
	return;

immediately after the variable declarations.

> +static void cxl_hdm_refresh_decoder(struct cxl_hdm *cxlhdm,
> +				    struct cxl_decoder *cxld)
> +{
> +	struct cxl_port *port = cxlhdm->port;
> +	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
> +	struct pci_dev *pdev __free(pci_dev_put) =
> +		cxl_port_get_uport_pci_dev(port);

This line needs to be at the first usage, i.e. right before "if (!pdev || !hdm)" below, to prevent
lifetime bugs.

> +	struct cxl_decoder_settings *settings;
> +	struct cxl_hdm_info *info;
> +
> +	if (!pdev || !hdm)
> +		return;
> +
> +	guard(rwsem_write)(&cxl_rwsem.dpa);
> +	info = pdev->hdm;
> +	if (!info)
> +		return;
> +	if (cxld->id < 0 || cxld->id >= info->decoder_count) {
> +		pci_warn(pdev, "CXL HDM decoder %d exceeds cached count %d\n",
> +			 cxld->id, info->decoder_count);
> +		return;
> +	}
> +
> +	info->global_ctrl = readl(hdm + CXL_HDM_DECODER_CTRL_OFFSET);
> +	settings = &info->settings[cxld->id];
> +
> +	/*
> +	 * A disabled decoder's software object may retain its old range and
> +	 * target state. Leave only the decoder id in the cached settings so stale
> +	 * state is not restored as an enabled decode.
> +	 */
> +	*settings = (struct cxl_decoder_settings) {
> +		.config.id = cxld->id,
> +	};
> +	if (cxld->flags & CXL_DECODER_F_ENABLE)
> +		cxl_decoder_snapshot(cxld, settings);
> +}
> +#else
> +static void cxl_hdm_refresh_decoder(struct cxl_hdm *cxlhdm,
> +				    struct cxl_decoder *cxld)
> +{
> +}
> +#endif
> +
>  static int cxl_decoder_commit(struct cxl_decoder *cxld)
>  {
>  	struct cxl_port *port = to_cxl_port(cxld->dev.parent);
> @@ -804,6 +849,7 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
>  	}
>  	port->commit_end++;
>  	cxld->flags |= CXL_DECODER_F_ENABLE;
> +	cxl_hdm_refresh_decoder(cxlhdm, cxld);
>  
>  	return 0;
>  }
> @@ -876,6 +922,7 @@ static void cxl_decoder_reset(struct cxl_decoder *cxld)
>  	writel(0, hdm + CXL_HDM_DECODER0_BASE_LOW_OFFSET(id));
>  
>  	cxld->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)) {
> @@ -1089,9 +1136,27 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  {
>  	void __iomem *hdm = cxlhdm->regs.hdm_decoder;
>  	struct cxl_port *port = cxlhdm->port;
> +#ifdef CONFIG_CXL_RESET
> +	struct pci_dev *pdev __free(pci_dev_put) =
> +		cxl_port_get_uport_pci_dev(port);
> +#endif

This shouldn't be here, and if you move it to be inside the below if statement
then you won't need the #ifdef as well.

>  	int i;
>  	u64 dpa_base = 0;
>  
> +#ifdef CONFIG_CXL_RESET
> +	if (is_cxl_endpoint(port) && pdev && hdm) {
> +		guard(rwsem_read)(&cxl_rwsem.dpa);
> +
> +		if (pdev->hdm &&
> +		    pdev->hdm->decoder_count != cxlhdm->decoder_count) {
> +			pci_warn(pdev,
> +				 "CXL HDM cache decoder count mismatch: cached=%d hdm=%d\n",
> +				 pdev->hdm->decoder_count, cxlhdm->decoder_count);
> +			return -ENXIO;
> +		}
> +	}
> +#endif
> +

Add IS_ENABLED(CONFIG_CXL_RESET) to the above if condition, like above.

>  	cxl_settle_decoders(cxlhdm);
>  
>  	for (i = 0; i < cxlhdm->decoder_count; i++) {
> @@ -1130,6 +1195,7 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  			put_device(&cxld->dev);
>  			return rc;
>  		}
> +		cxl_hdm_refresh_decoder(cxlhdm, cxld);
>  		rc = add_hdm_decoder(port, cxld);
>  		if (rc) {
>  			dev_warn(&port->dev,
> diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
> index 1acdf537e4e9..005259e38f4b 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -77,6 +77,28 @@ struct cxl_decoder_settings {
>  	u64 target_or_skip_reg_val;
>  };
>  
> +/**
> + * 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-decoder programming state
> + */
> +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_decoder_settings settings[];
> +};
> +
>  /*
>   * Using struct_group() allows for per register-block-type helper routines,
>   * without requiring block-type agnostic code to include the prefix.
> 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