Re: [PATCH v12 04/12] cxl: Cache decoder settings on PCI devices
From: Srirangan Madhavan
Date: Tue Sep 22 2026 - 20:01:37 EST
On 9/11/26 5:22 PM, Jonathan Cameron wrote:
External email: Use caution opening links or attachments
On Thu, 10 Sep 2026 07:08:00 +0000
Srirangan Madhavan <smadhavan@xxxxxxxxxx> wrote:
Add CXL core plumbing to refresh a PCI device HDM decoder cache whenVarious comments inline
decoders are enumerated, committed, or reset. PCI reset paths can use
this snapshot to restore HDM programming without walking CXL topology
during reset recovery.
The cache is populated by PCI-side discovery in a follow-on patch. Until
then, the CXL core update path is a no-op when no PCI HDM cache is
present.
Signed-off-by: Srirangan Madhavan <smadhavan@xxxxxxxxxx>
Thanks,
Jonathan
---
drivers/cxl/core/hdm.c | 68 +++++++++++++++++++++++++++++++++++++++++-
include/cxl/cxl.h | 12 ++++++++
include/linux/pci.h | 6 ++++
3 files changed, 85 insertions(+), 1 deletion(-)
diff --git a/drivers/cxl/core/hdm.c b/drivers/cxl/core/hdm.c
index d621d827f59f..0927036aed27 100644
--- a/drivers/cxl/core/hdm.c
+++ b/drivers/cxl/core/hdm.c
@@ -16,6 +16,9 @@
* for enumerating these registers and capabilities.
+static bool __cxl_pci_hdm_decoder_count_match(struct pci_dev *pdev,
+ int decoder_count)
+{
+ struct cxl_hdm_info *info;
+ bool match = true;
+
+ down_read(&cxl_rwsem.dpa);
guard()
+ info = pdev->hdm;
+ if (info) {
If !info fails the number of decoders definitely didn't match - so maybe
print something in that path too.
so far, the local info variable is less readable than pdev->hdm.
Maybe it becomes more useful later in series.
+ if (info->decoder_count != decoder_count) {
+ pci_warn(pdev,
+ "CXL HDM cache decoder count mismatch: cached=%d hdm=%d\n",
+ info->decoder_count, decoder_count);
+ match = false;
+ }
+ }
+ up_read(&cxl_rwsem.dpa);
+
+ return match;
+}
+
+static bool cxl_pci_hdm_decoder_count_match(struct cxl_hdm *cxlhdm)
+{
+ struct pci_dev *pdev __free(pci_dev_put) =
+ cxl_port_get_uport_pci_dev(cxlhdm->port);
+
+ if (!pdev)
+ return true;
+
+ return __cxl_pci_hdm_decoder_count_match(pdev, cxlhdm->decoder_count);
I went looking and seems like this is the only call. Just bring the implementation
inline here.
+}
+
+static void cxl_hdm_save_decoder_info(struct cxl_hdm *cxlhdm,
+ struct cxl_decoder *cxld)
+{
+ struct pci_dev *pdev __free(pci_dev_put) =
+ cxl_port_get_uport_pci_dev(cxlhdm->port);
+ struct cxl_decoder_settings *settings;
+ struct cxl_hdm_info *info;
+
+ if (!pdev)
+ return;
+
+ guard(rwsem_write)(&cxl_rwsem.dpa);
+ info = pdev->hdm;
+ if (!info || cxld->id >= info->decoder_count)
+ return;
+
+ settings = &info->settings[cxld->id];
+ *settings = (struct cxl_decoder_settings) {
+ .id = cxld->id,
+ };
+ if (cxld->flags & CXL_DECODER_F_ENABLE)
Why is it bad to snapshot a non enabled decoder? Is it pointless
or harmful. Add a comment.
+ cxl_decoder_snapshot(cxld, settings);
+}
diff --git a/include/cxl/cxl.h b/include/cxl/cxl.h
index c09492af8fbd..ed5237df510f 100644
--- a/include/cxl/cxl.h
+++ b/include/cxl/cxl.h
@@ -133,6 +133,18 @@ struct cxl_regs {
);
};
+#define CXL_HDM_DECODER_MAX_COUNT 32
+
+/**
+ * struct cxl_hdm_info - PCI device HDM decoder programming cache
+ * @decoder_count: number of decoder settings entries
+ * @settings: cached per-decoder programming state
+ */
+struct cxl_hdm_info {
+ int decoder_count;
+ struct cxl_decoder_settings settings[CXL_HDM_DECODER_MAX_COUNT];
+};
This is quite a big structure. Can't we do
int decoder_count;
struct cxl_decodet setting settings[] __counted_by(decoder_count);
and allocate the necessary size using struct_size()?
I doubt that many devices will do all 32 decoders - particularly as
the spec used to allow a smaller number IIRC.
+
struct cxl_reg_map {
bool valid;
int id;
diff --git a/include/linux/pci.h b/include/linux/pci.h
index d31a8d107b1e..84d058b1b492 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 reset state */
+#endif
#ifdef CONFIG_PCI_NPEM
struct npem *npem; /* Native PCIe Enclosure Management */
#endif
I addressed these comments in v13 patch 8:
- replaced the manual locking with guard();
- removed the single-use count-match wrapper;
- added the explanation for not snapshotting stale state from a disabled decoder; and
- changed cxl_hdm_info to use a flexible array allocated with struct_size().
For the missing pdev->hdm case, I did not report a decoder-count mismatch. With the revised flow, no cache means there is no state to refresh; reset will subsequently be rejected because no snapshot is available. A mismatch is reported only when a cache exists and its decoder count differs.
https://lore.kernel.org/linux-cxl/20260922083924.2451158-9-smadhavan@xxxxxxxxxx/
--
Regards,
Srirangan