Re: [PATCH v4 08/10] cxl: add DVSEC config save/restore

[Jonathan Cameron <jonathan.cameron@huawei.com>]

On Tue, 20 Jan 2026 22:26:08 +0000
smadhavan@nvidia.com wrote:

> From: Srirangan Madhavan <smadhavan@nvidia.com>
> 
> Save and restore CXL DVSEC control registers across reset with
> CONFIG_LOCK handling so RWL fields are preserved when locked. This
> maintains device policy and capability state across cxl_reset while
> avoiding writes to locked fields.

Add some more info here on what is protected by the lock and
preserved across reset.  This is odd enough I think that detail is needed.
There are other RWL fields outside those registers you are restoring here.
e.g. the range registers, various things in extended meta data,

> 
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>
> ---
>  drivers/cxl/pci.c | 107 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/cxl/pci.h |  15 +++++++
>  2 files changed, 122 insertions(+)
> 
> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index f9cc452ccb8a..7d6a0ef70b2d 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c
> @@ -1154,6 +1154,113 @@ static int cxl_region_flush_host_cpu_caches(struct device *dev, void *data)
>  	return 0;
>  }
> 
> +/*
> + * CXL DVSEC register save/restore
> + */
> +static int cxl_save_dvsec_state(struct pci_dev *pdev,
> +				struct cxl_type2_saved_state *state, int dvsec)
> +{
> +	int rc;
> +
> +	rc = pci_read_config_word(pdev, dvsec + CXL_DVSEC_CTRL_OFFSET,
> +				  &state->dvsec_ctrl);
> +	if (rc)
> +		return rc;
> +
> +	rc = pci_read_config_word(pdev, dvsec + CXL_DVSEC_CTRL2_OFFSET,
> +				  &state->dvsec_ctrl2);
> +	return rc;
Similar to below. Just combine 2 lines into 1.
> +}
> +
> +static int cxl_restore_dvsec_state(struct pci_dev *pdev,
> +				   const struct cxl_type2_saved_state *state,
> +				   int dvsec, bool config_locked)
> +{
> +	int rc;
> +	u16 val_to_restore;
> +
> +	if (config_locked) {
> +		u16 current_val;
> +
> +		rc = pci_read_config_word(pdev, dvsec + CXL_DVSEC_CTRL_OFFSET,
> +					  &current_val);
> +		if (rc)
> +			return rc;
> +
> +		val_to_restore = (current_val & CXL_DVSEC_CTRL_RWL_MASK) |
> +				 (state->dvsec_ctrl & ~CXL_DVSEC_CTRL_RWL_MASK);
> +	} else {
> +		val_to_restore = state->dvsec_ctrl;
> +	}
> +
> +	rc = pci_write_config_word(pdev, dvsec + CXL_DVSEC_CTRL_OFFSET,
> +				   val_to_restore);
> +	if (rc)
> +		return rc;
> +
> +	rc = pci_write_config_word(pdev, dvsec + CXL_DVSEC_CTRL2_OFFSET,
> +				   state->dvsec_ctrl2);
> +	return rc;

If this isn't expected to get more complex (I haven't checked) then
return pci_write_config()

> +}
> +
> +/**
> + * cxl_config_save_state - Save CXL configuration state
> + * @pdev: PCI device
> + * @state: Structure to store saved state
> + *
> + * Saves CXL DVSEC state before reset.
> + */
> +int cxl_config_save_state(struct pci_dev *pdev,
> +			  struct cxl_type2_saved_state *state)
> +{
> +	struct cxl_dev_state *cxlds = pci_get_drvdata(pdev);
> +	int dvsec;
> +
> +	if (!cxlds || !state)
> +		return -EINVAL;
> +
> +	memset(state, 0, sizeof(*state));
We need to make sure all registers are read, so why is zeroing helpful? 

> +
> +	dvsec = cxlds->cxl_dvsec;
> +	if (!dvsec)
> +		return -ENODEV;
> +
> +	return cxl_save_dvsec_state(pdev, state, dvsec);
> +}
> +EXPORT_SYMBOL_NS_GPL(cxl_config_save_state, "CXL");