Re: [PATCH v8 02/10] cxl: Cache decoder settings on PCI devices

[Dave Jiang]

On 7/3/26 3:05 PM, Srirangan Madhavan wrote:
> Cache CXL core's HDM decoder settings in pci_dev->hdm as decoders are
> enumerated, committed, or reset. PCI reset paths can use this snapshot to
> restore HDM programming without walking CXL topology during reset recovery.
> 
> Signed-off-by: Srirangan Madhavan <smadhavan@xxxxxxxxxx>
> ---
>  drivers/cxl/core/hdm.c | 81 +++++++++++++++++++++++++++++++++++++++++-
>  include/cxl/cxl.h      | 12 +++++++
>  include/linux/pci.h    |  6 ++++
>  3 files changed, 98 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
> index fa978c297546..83cda63f76a5 100644
> --- a/drivers/cxl/core/hdm.c
> +++ b/drivers/cxl/core/hdm.c
> @@ -84,6 +84,76 @@ static void parse_hdm_decoder_caps(struct cxl_hdm *cxlhdm)
>  		cxlhdm->iw_cap_mask |= BIT(16);
>  }
>  
> +static void clear_hdm_info(void *data)
> +{
> +	struct pci_dev *pdev = data;
> +
> +	WRITE_ONCE(pdev->hdm, NULL);
> +}
> +
> +static int devm_cxl_pci_setup_hdm_info(struct cxl_hdm *cxlhdm)
> +{
> +	struct cxl_port *port = cxlhdm->port;
> +	struct cxl_hdm_info *info;
> +	struct pci_dev *pdev;
> +	struct device *uport;
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +
> +		uport = cxlmd->dev.parent;
> +	} else {
> +		uport = port->uport_dev;
> +	}

This wants a helper given it happens open coded couple times in this patch. Maybe 'cxl_port_get_uport_dev()'? Also probably should take a device reference when the helper returns the device ptr.

DJ

> +
> +	if (!dev_is_pci(uport))
> +		return 0;
> +
> +	pdev = to_pci_dev(uport);
> +	info = devm_kzalloc(&pdev->dev,
> +			    struct_size(info, settings, cxlhdm->decoder_count),
> +			    GFP_KERNEL);
> +	if (!info)
> +		return -ENOMEM;
> +
> +	info->decoder_count = cxlhdm->decoder_count;
> +	WRITE_ONCE(pdev->hdm, info);
> +
> +	return devm_add_action_or_reset(&pdev->dev, clear_hdm_info, pdev);
> +}
> +
> +static void cxl_hdm_info_set_decoder(struct cxl_hdm *cxlhdm,
> +				     struct cxl_decoder *cxld)
> +{
> +	struct cxl_port *port = cxlhdm->port;
> +	struct cxl_hdm_info *info;
> +	struct pci_dev *pdev;
> +	struct device *uport;
> +
> +	if (is_cxl_endpoint(port)) {
> +		struct cxl_memdev *cxlmd = to_cxl_memdev(port->uport_dev);
> +
> +		uport = cxlmd->dev.parent;
> +	} else {
> +		uport = port->uport_dev;
> +	}
> +
> +	if (!dev_is_pci(uport))
> +		return;
> +
> +	pdev = to_pci_dev(uport);
> +	info = READ_ONCE(pdev->hdm);
> +	if (!info || cxld->id >= info->decoder_count)
> +		return;
> +
> +	if (cxld->flags & CXL_DECODER_F_ENABLE)
> +		info->settings[cxld->id] = cxld->settings;
> +	else
> +		info->settings[cxld->id] = (struct cxl_decoder_settings) {
> +			.id = cxld->id,
> +		};
> +}
> +
>  static bool should_emulate_decoders(struct cxl_endpoint_dvsec_info *info)
>  {
>  	struct cxl_hdm *cxlhdm;
> @@ -747,6 +817,7 @@ static int cxl_decoder_commit(struct cxl_decoder *cxld)
>  		return rc;
>  	}
>  	port->commit_end++;
> +	cxl_hdm_info_set_decoder(cxlhdm, cxld);
>  
>  	return 0;
>  }
> @@ -819,6 +890,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_info_set_decoder(cxlhdm, cxld);
>  
>  	/* Userspace is now responsible for reconfiguring this decoder */
>  	if (is_endpoint_decoder(&cxld->dev)) {
> @@ -989,6 +1061,7 @@ static int init_hdm_decoder(struct cxl_port *port, struct cxl_decoder *cxld,
>  		lo = readl(hdm + CXL_HDM_DECODER0_TL_LOW(which));
>  		hi = readl(hdm + CXL_HDM_DECODER0_TL_HIGH(which));
>  		target_list.value = (hi << 32) + lo;
> +		cxld->targets = target_list.value;
>  		for (i = 0; i < cxld->interleave_ways; i++)
>  			cxld->target_map[i] = target_list.target_id[i];
>  
> @@ -1062,11 +1135,16 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  	struct cxl_port *port = cxlhdm->port;
>  	int i;
>  	u64 dpa_base = 0;
> +	int rc;
>  
>  	cxl_settle_decoders(cxlhdm);
>  
> +	rc = devm_cxl_pci_setup_hdm_info(cxlhdm);
> +	if (rc)
> +		return rc;
> +
>  	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)) {
> @@ -1101,6 +1179,7 @@ static int devm_cxl_enumerate_decoders(struct cxl_hdm *cxlhdm,
>  			put_device(&cxld->dev);
>  			return rc;
>  		}
> +		cxl_hdm_info_set_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 757d916a6330..84924ca06e52 100644
> --- a/include/cxl/cxl.h
> +++ b/include/cxl/cxl.h
> @@ -121,6 +121,18 @@ struct cxl_regs {
>  	);
>  };
>  
> +/**
> + * struct cxl_hdm_info - PCI device HDM decoder programming cache
> + * @decoder_count: number of decoder settings entries
> + * @regs: mapped CXL component registers for this HDM decoder block
> + * @settings: cached per-decoder programming state
> + */
> +struct cxl_hdm_info {
> +	int decoder_count;
> +	struct cxl_component_regs regs;
> +	struct cxl_decoder_settings settings[] __counted_by(decoder_count);
> +};
> +
>  int cxl_commit(struct cxl_decoder_settings *settings, void __iomem *hdm);
>  
>  struct cxl_reg_map {
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 2c4454583c11..7db2daf8597c 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -334,6 +334,9 @@ struct pcie_link_state;
>  struct pci_sriov;
>  struct pci_p2pdma;
>  struct rcec_ea;
> +#ifdef CONFIG_CXL_HDM
> +struct cxl_hdm_info;
> +#endif
>  
>  /* struct pci_dev - describes a PCI device
>   *
> @@ -563,6 +566,9 @@ struct pci_dev {
>  #ifdef CONFIG_PCI_DOE
>  	struct xarray	doe_mbs;	/* Data Object Exchange mailboxes */
>  #endif
> +#ifdef CONFIG_CXL_HDM
> +	struct cxl_hdm_info *hdm;	/* CXL HDM decoder reset state */
> +#endif
>  #ifdef CONFIG_PCI_NPEM
>  	struct npem	*npem;		/* Native PCIe Enclosure Management */
>  #endif