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

Jonathan Cameron
2026-01-21T11:31:29Z

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");