Re: [PATCH v4 10/10] cxl: add HDM decoder and IDE save/restore

Jonathan Cameron
2026-01-21T11:42:09Z

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

> From: Srirangan Madhavan <smadhavan@nvidia.com>
>
> Extend state save/restore to HDM decoder and IDE registers for Type 2
> devices. The HDM/IDE blocks are located via the component register map,
> then preserved across reset to retain decoder configuration and IDE
> policy. This avoids losing HDM/IDE programming when cxl_reset is issued.
>
> Signed-off-by: Srirangan Madhavan <smadhavan@nvidia.com>

Split the two up. One patch for HDM, one for IDE.

Been a while since I read the IDE stuff.

Isn't reset going to trip up the encryption to the extent that we need
to redo the key exchange etc?

> ---
> drivers/cxl/core/regs.c | 7 ++
> drivers/cxl/cxl.h | 4 ++
> drivers/cxl/pci.c | 153 ++++++++++++++++++++++++++++++++++++++--
> include/cxl/pci.h | 43 +++++++++++
> 4 files changed, 201 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cxl/core/regs.c b/drivers/cxl/core/regs.c
> index ecdb22ae6952..76d6869d82ea 100644
> --- a/drivers/cxl/core/regs.c
> +++ b/drivers/cxl/core/regs.c
> @@ -93,6 +93,12 @@ void cxl_probe_component_regs(struct device *dev, void __iomem *base,
> length = CXL_RAS_CAPABILITY_LENGTH;
> rmap = &map->ras;
> break;
> + case CXL_CM_CAP_CAP_ID_IDE:
> + dev_dbg(dev, "found IDE capability (0x%x)\n",
> + offset);

Trivial: Fits on one line under 80 chars.

> + length = CXL_IDE_CAPABILITY_LENGTH;
> + rmap = &map->ide;
> + break;

> diff --git a/drivers/cxl/pci.c b/drivers/cxl/pci.c
> index 7d6a0ef70b2d..eb735d0ae175 100644
> --- a/drivers/cxl/pci.c
> +++ b/drivers/cxl/pci.c

> +/*
> + * CXL HDM Decoder register save/restore
> + */
> +static int cxl_save_hdm_state(struct cxl_dev_state *cxlds,
> + struct cxl_type2_saved_state *state)
> +{
> + void __iomem *hdm = cxlds->regs.hdm_decoder;
> + u32 cap, ctrl;
> + int i, count;
> +
> + if (!hdm)
> + return 0;
> +
> + cap = readl(hdm + CXL_HDM_DECODER_CAP_OFFSET);
> + count = cap & CXL_HDM_DECODER_COUNT_MASK;

Use FIELD_GET() for this so we don't need to go check the definition to be
sure there isn't a shift.

> + count = min(count, CXL_MAX_DECODERS);
> +
> + state->hdm_decoder_count = count;
> + state->hdm_global_ctrl = readl(hdm + CXL_HDM_DECODER_GLOBAL_CTRL_OFFSET);
> +
> + for (i = 0; i < count; i++) {
> + struct cxl_hdm_decoder_state *d = &state->decoders[i];
> + u32 base_low, base_high, size_low, size_high;
> + u32 dpa_skip_low, dpa_skip_high;
> +
> + base_low = readl(hdm + CXL_HDM_DECODER_BASE_LOW(i));
> + base_high = readl(hdm + CXL_HDM_DECODER_BASE_HIGH(i));
> + size_low = readl(hdm + CXL_HDM_DECODER_SIZE_LOW(i));
> + size_high = readl(hdm + CXL_HDM_DECODER_SIZE_HIGH(i));
> + ctrl = readl(hdm + CXL_HDM_DECODER_CTRL(i));
> + dpa_skip_low = readl(hdm + CXL_HDM_DECODER_DPA_SKIP_LOW(i));
> + dpa_skip_high = readl(hdm + CXL_HDM_DECODER_DPA_SKIP_HIGH(i));
> +
> + d->base = ((u64)base_high << 32) | base_low;
> + d->size = ((u64)size_high << 32) | size_low;
> + d->ctrl = ctrl;
> + d->dpa_skip = ((u64)dpa_skip_high << 32) | dpa_skip_low;
> + d->enabled = !!(ctrl & CXL_HDM_DECODER_ENABLE);

I'm lazy. Why not just stash the register values as a raw block and
put them back? Do we want to put them back if they aren't locked?
We ripped down the region so I'd be kind of expecting the type 2
driver for the specific hardware to need to put this stuff back
as part of it's own reinit.

> + }
> +
> + return 0;
> +}