[PATCH v5 00/16] Add ACPI-based PMT discovery support for Intel PMC

[PATCH v5 00/16] Add ACPI-based PMT discovery support for Intel PMC

[David E. Box]

This series adds ACPI-based PMT discovery support and wires it into the
Intel PMC telemetry stack for Nova Lake S.

Today, PMT discovery flow is primarily PCI-oriented. Some platforms expose
PMT discovery via ACPI _DSD, and PMC telemetry needs to consume that path
for both standalone and companion-device cases. This series addresses that
in three logical steps:

    1. Prepare intel_vsec for ACPI-backed discovery
    2. Refactor PMT class handling so discovery source is
       abstracted (PCI or ACPI)
    3. Add/enable PMC consumers using ACPI discovery, including NVL
       SSRAM support

Patches for step 1 were previously accepted. V4 includes the remaining
integration patches.

V5 changes:
- Addressed Ilpo Järvinen's review feedback on patches 08 and 14:
  - Added #include <linux/limits.h> for U16_MAX in pwrm_telemetry.c.
  - Split acpi_handle declaration from ACPI_HANDLE() assignment in both
    pmc_pwrm_acpi_probe() and pmc_ssram_telemetry_acpi_init(). The
    assignment now sits immediately above its !handle check.
  - Reordered local variables in those two functions in reverse-xmas-tree
    order.
- Patch 14: fixed a __free(pmc_acpi_free) ordering bug that caused
  buf.pointer to leak. The cleanup variable is now declared after
  acpi_evaluate_object() populates buf.pointer, and the GUID search
  operates on the cleanup variable so its current value tracks the
  actual allocation.
- Patches 05 and 06: bounded the new discovery-header memcpy_fromio()
  to resource_size() of the mapped DVSEC entry. The cache introduced in
  patch 05 (and the unified PCI header fetch in patch 06) would otherwise
  read 16 bytes from namespaces whose DVSEC entry_size is only 3 dwords
  (telemetry).
- Patch 06: documented in the ACPI branch that entry->disc_table = NULL
  is intentional, so consumers that dereference disc_table must only be
  wired to INTEL_VSEC_DISC_PCI namespaces.
- Remaining patches unchanged from v4.

V4 changes:
- Bound PMT discovery-header sizing to shared canonical definitions:
  - Added INTEL_VSEC_ACPI_DISC_DWORDS in intel_vsec API.
  - Added PMT_DISC_HEADER_DWORDS in PMT class, derived from the shared 
    definition.
  - Replaced literal header copy sizes in PMT header fetch with size bound 
    to entry->disc_header.
- Updated PMC ACPI discovery typing to use shared acpi_disc_t consistently:
  - acpi_disc_t now references INTEL_VSEC_ACPI_DISC_DWORDS.
  - pmc_parse_telem_dsd() uses acpi_disc_t return type and 
    declaration-site allocation with __free(kfree).
  - Included intel_vsec header in PMC core declarations to align type 
    ownership.
- In SSRAM ACPI scaffolding, switched temporary discovery pointer from raw 
  u32 (*)[4] to acpi_disc_t for consistency with exported PMC helper types.
- Remaining patches unchanged from v3.

V3 changes:
- Dropped "platform/x86/intel/pmt: Move header decode into common helper"
- The dropped patch caused a crashlog regression because disc_res was
  accessed before assignment
- Updated the surrounding PMT patches to keep the needed ACPI header-fetch
  work without the dropped helper
- Added "platform/x86/intel/pmt: Cache the telemetry discovery header"
  to carry the post-v3 bug fix cleanly
- Dropped the standalone cleanup-pattern patch to keep the simpler
  ssram pointer flow requested in review

David E. Box (14):
  platform/x86/intel/pmt: Add pre/post decode hooks around header
    parsing
  platform/x86/intel/pmt/crashlog: Split init into pre-decode
  platform/x86/intel/pmt/telemetry: Move overlap check to post-decode
    hook
  platform/x86/intel/pmt: Pass discovery index instead of resource
  platform/x86/intel/pmt: Cache the telemetry discovery header
  platform/x86/intel/pmt: Unify header fetch and add ACPI source
  platform/x86/intel/pmc: Add PMC SSRAM Kconfig description
  platform/x86/intel/pmc: Add ACPI PWRM telemetry driver for Nova Lake S
  platform/x86/intel/pmc/ssram: Rename probe and PCI ID table for
    consistency
  platform/x86/intel/pmc/ssram: Refactor DEVID/PWRMBASE extraction into
    helper
  platform/x86/intel/pmc/ssram: Add PCI platform data
  platform/x86/intel/pmc/ssram: Add ACPI discovery scaffolding
  platform/x86/intel/pmc/ssram: Make PMT registration optional
  platform/x86/intel/pmc: Add NVL PCI IDs for SSRAM telemetry discovery

Xi Pardee (2):
  platform/x86/intel/pmc/ssram: Use fixed-size static pmc array
  platform/x86/intel/pmc/ssram: Refactor memory barrier for reentrant
    probe

 drivers/platform/x86/intel/pmc/Kconfig        |  25 ++
 drivers/platform/x86/intel/pmc/Makefile       |   2 +
 drivers/platform/x86/intel/pmc/core.h         |  21 ++
 .../platform/x86/intel/pmc/pwrm_telemetry.c   | 216 +++++++++++++++
 .../platform/x86/intel/pmc/ssram_telemetry.c  | 253 +++++++++++++-----
 drivers/platform/x86/intel/pmt/class.c        | 160 ++++++++++-
 drivers/platform/x86/intel/pmt/class.h        |   7 +
 drivers/platform/x86/intel/pmt/crashlog.c     |  19 +-
 drivers/platform/x86/intel/pmt/telemetry.c    |  34 ++-
 include/linux/intel_vsec.h                    |   5 +-
 10 files changed, 649 insertions(+), 93 deletions(-)
 create mode 100644 drivers/platform/x86/intel/pmc/pwrm_telemetry.c


base-commit: 5d6919055dec134de3c40167a490f33c74c12581
-- 
2.43.0

[PATCH v5 01/16] platform/x86/intel/pmt: Add pre/post decode hooks around header parsing

[David E. Box]

Add optional pre- and post-decode callbacks to the PMT class so namespaces
can perform setup and cleanup steps around header parsing.

  - Add pmt_pre_decode() and pmt_post_decode() to struct
    intel_pmt_namespace.
  - Update intel_pmt_dev_create() to invoke, in order:
      pre → header_decode() → post.
  - Keep the existing pmt_header_decode() callback unchanged.

No functional changes. This adds flexibility for upcoming decoders while
preserving current behavior.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmt/class.c | 12 ++++++++++++
 drivers/platform/x86/intel/pmt/class.h |  4 ++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index b4c9964df807..9b315334a69b 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -381,10 +381,22 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
 	if (IS_ERR(entry->disc_table))
 		return PTR_ERR(entry->disc_table);
 
+	if (ns->pmt_pre_decode) {
+		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
+		if (ret)
+			return ret;
+	}
+
 	ret = ns->pmt_header_decode(entry, dev);
 	if (ret)
 		return ret;
 
+	if (ns->pmt_post_decode) {
+		ret = ns->pmt_post_decode(intel_vsec_dev, entry);
+		if (ret)
+			return ret;
+	}
+
 	ret = intel_pmt_populate_entry(entry, intel_vsec_dev, disc_res);
 	if (ret)
 		return ret;
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index 1ae56a5baad2..ff39014b208c 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -62,6 +62,10 @@ struct intel_pmt_namespace {
 	struct xarray *xa;
 	int (*pmt_header_decode)(struct intel_pmt_entry *entry,
 				 struct device *dev);
+	int (*pmt_pre_decode)(struct intel_vsec_device *ivdev,
+			      struct intel_pmt_entry *entry);
+	int (*pmt_post_decode)(struct intel_vsec_device *ivdev,
+			       struct intel_pmt_entry *entry);
 	int (*pmt_add_endpoint)(struct intel_vsec_device *ivdev,
 				struct intel_pmt_entry *entry);
 };
-- 
2.43.0

[PATCH v5 02/16] platform/x86/intel/pmt/crashlog: Split init into pre-decode

[David E. Box]

Refactor crashlog initialization to use the PMT namespace pre-decode hook:

  - Add pmt_crashlog_pre_decode() to parse type/version, select the
    crashlog_info, initialize the control mutex, and set entry->attr_grp.
  - Simplify pmt_crashlog_header_decode() to only read header fields from
    the discovery table.
  - Wire the namespace with .pmt_pre_decode = pmt_crashlog_pre_decode.

This separates structural initialization from header parsing, aligning
crashlog with the PMT class pre/post decode flow.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmt/crashlog.c | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/crashlog.c b/drivers/platform/x86/intel/pmt/crashlog.c
index b0393c9c5b4b..f936daf99e4d 100644
--- a/drivers/platform/x86/intel/pmt/crashlog.c
+++ b/drivers/platform/x86/intel/pmt/crashlog.c
@@ -496,11 +496,9 @@ static const struct crashlog_info *select_crashlog_info(u32 type, u32 version)
 	return &crashlog_type1_ver2;
 }
 
-static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
-				      struct device *dev)
+static int pmt_crashlog_pre_decode(struct intel_vsec_device *ivdev,
+				   struct intel_pmt_entry *entry)
 {
-	void __iomem *disc_table = entry->disc_table;
-	struct intel_pmt_header *header = &entry->header;
 	struct crashlog_entry *crashlog;
 	u32 version;
 	u32 type;
@@ -513,6 +511,16 @@ static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
 	mutex_init(&crashlog->control_mutex);
 
 	crashlog->info = select_crashlog_info(type, version);
+	entry->attr_grp = crashlog->info->attr_grp;
+
+	return 0;
+}
+
+static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
+				      struct device *dev)
+{
+	void __iomem *disc_table = entry->disc_table;
+	struct intel_pmt_header *header = &entry->header;
 
 	header->access_type = GET_ACCESS(readl(disc_table));
 	header->guid = readl(disc_table + GUID_OFFSET);
@@ -521,8 +529,6 @@ static int pmt_crashlog_header_decode(struct intel_pmt_entry *entry,
 	/* Size is measured in DWORDS, but accessor returns bytes */
 	header->size = GET_SIZE(readl(disc_table + SIZE_OFFSET));
 
-	entry->attr_grp = crashlog->info->attr_grp;
-
 	return 0;
 }
 
@@ -530,6 +536,7 @@ static DEFINE_XARRAY_ALLOC(crashlog_array);
 static struct intel_pmt_namespace pmt_crashlog_ns = {
 	.name = "crashlog",
 	.xa = &crashlog_array,
+	.pmt_pre_decode = pmt_crashlog_pre_decode,
 	.pmt_header_decode = pmt_crashlog_header_decode,
 };
 
-- 
2.43.0

[PATCH v5 03/16] platform/x86/intel/pmt/telemetry: Move overlap check to post-decode hook

[David E. Box]

Update the telemetry namespace to use the new PMT class pre/post decode
interface. The overlap check, which previously occurred during header
decode, is now performed in the post-decode hook once header fields are
populated. This preserves existing behavior while reusing the same header
decode logic across PMT drivers.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmt/class.h     |  1 +
 drivers/platform/x86/intel/pmt/telemetry.c | 24 ++++++++++++++--------
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index ff39014b208c..8a0db0ef58c1 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -37,6 +37,7 @@ struct intel_pmt_header {
 	u32	size;
 	u32	guid;
 	u8	access_type;
+	u8	telem_type;
 };
 
 struct intel_pmt_entry {
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index bdc7c24a3678..d22f633638be 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -58,14 +58,9 @@ struct pmt_telem_priv {
 	struct intel_pmt_entry		entry[];
 };
 
-static bool pmt_telem_region_overlaps(struct intel_pmt_entry *entry,
-				      struct device *dev)
+static bool pmt_telem_region_overlaps(struct device *dev, u32 guid, u32 type)
 {
-	u32 guid = readl(entry->disc_table + TELEM_GUID_OFFSET);
-
 	if (intel_pmt_is_early_client_hw(dev)) {
-		u32 type = TELEM_TYPE(readl(entry->disc_table));
-
 		if ((type == TELEM_TYPE_PUNIT_FIXED) ||
 		    (guid == TELEM_CLIENT_FIXED_BLOCK_GUID))
 			return true;
@@ -80,15 +75,25 @@ static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
 	void __iomem *disc_table = entry->disc_table;
 	struct intel_pmt_header *header = &entry->header;
 
-	if (pmt_telem_region_overlaps(entry, dev))
-		return 1;
-
 	header->access_type = TELEM_ACCESS(readl(disc_table));
 	header->guid = readl(disc_table + TELEM_GUID_OFFSET);
 	header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
 
 	/* Size is measured in DWORDS, but accessor returns bytes */
 	header->size = TELEM_SIZE(readl(disc_table));
+	header->telem_type = TELEM_TYPE(readl(entry->disc_table));
+
+	return 0;
+}
+
+static int pmt_telem_post_decode(struct intel_vsec_device *ivdev,
+				 struct intel_pmt_entry *entry)
+{
+	struct intel_pmt_header *header = &entry->header;
+	struct device *dev = &ivdev->auxdev.dev;
+
+	if (pmt_telem_region_overlaps(dev, header->guid, header->telem_type))
+		return 1;
 
 	/*
 	 * Some devices may expose non-functioning entries that are
@@ -131,6 +136,7 @@ static struct intel_pmt_namespace pmt_telem_ns = {
 	.name = "telem",
 	.xa = &telem_array,
 	.pmt_header_decode = pmt_telem_header_decode,
+	.pmt_post_decode = pmt_telem_post_decode,
 	.pmt_add_endpoint = pmt_telem_add_endpoint,
 };
 
-- 
2.43.0

[PATCH v5 04/16] platform/x86/intel/pmt: Pass discovery index instead of resource

[David E. Box]

Change PMT class code to pass a discovery index rather than a direct struct
resource when creating entries. This allows the class to identify the
discovery source generically without assuming PCI BAR resources. For PCI
devices, the index still resolves to a resource in the intel_vsec_device.
Other discovery sources, such as ACPI, can use the same index without
needing a struct resource.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 changes:
  - Rebased after dropping the previous "Move header decode into common
    helper" patch
  - Adjusted the intel_pmt_populate_entry() call path to match the restored
    intel_pmt_dev_create() flow
  - Did not apply Ilpo V2 signoff due to these changes.

V2 - No changes

 drivers/platform/x86/intel/pmt/class.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 9b315334a69b..7da8279b54f8 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -206,11 +206,12 @@ EXPORT_SYMBOL_GPL(intel_pmt_class);
 
 static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
 				    struct intel_vsec_device *ivdev,
-				    struct resource *disc_res)
+				    int idx)
 {
 	struct pci_dev *pci_dev = to_pci_dev(ivdev->dev);
 	struct device *dev = &ivdev->auxdev.dev;
 	struct intel_pmt_header *header = &entry->header;
+	struct resource *disc_res;
 	u8 bir;
 
 	/*
@@ -235,6 +236,7 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
 		 * For access_type LOCAL, the base address is as follows:
 		 * base address = end of discovery region + base offset
 		 */
+		disc_res = &ivdev->resource[idx];
 		entry->base_addr = disc_res->end + 1 + header->base_offset;
 
 		/*
@@ -397,7 +399,7 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
 			return ret;
 	}
 
-	ret = intel_pmt_populate_entry(entry, intel_vsec_dev, disc_res);
+	ret = intel_pmt_populate_entry(entry, intel_vsec_dev, idx);
 	if (ret)
 		return ret;
 
-- 
2.43.0

[PATCH v5 05/16] platform/x86/intel/pmt: Cache the telemetry discovery header

[David E. Box]

pmt_telem_header_decode() only needs the discovery header dwords, but it
currently decodes them by reading directly from entry->disc_table.

Cache the discovery header in intel_pmt_entry when the device is created
and have telemetry decode use the cached values instead of performing MMIO
reads at decode time.

The DVSEC discovery resource for a namespace is sized by its per-entry
entry_size (in dwords), which can be less than the 4-dword cache (e.g.
telemetry uses entry_size = 3, i.e. 12 bytes). Cap the memcpy_fromio()
to resource_size(disc_res) so the new cache does not read past the
mapped region. Any unread dwords stay zero from the zero-initialized
allocation of the containing struct.

This keeps the telemetry header decode path independent of how the
discovery data is backed and avoids baking a direct MMIO assumption into
the feature-specific decode logic.

Assisted-by: GitHub-Copilot:claude-opus-4.7
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 changes:
  - Cap memcpy_fromio() of the cached discovery header to
    resource_size(disc_res) so the newly introduced cache does not
    over-read namespaces whose DVSEC entry_size is smaller than the
    cache (e.g. telemetry has entry_size = 3, 12 bytes).

V4 - No changes

V3 changes:
  - New patch split out from PMT header-fetch rework to cache discovery
    header data before downstream decode/population.
  - Added to carry the post-v3 bug fix while preserving the original series
    ordering intent.

 drivers/platform/x86/intel/pmt/class.c     | 11 +++++++++++
 drivers/platform/x86/intel/pmt/class.h     |  1 +
 drivers/platform/x86/intel/pmt/telemetry.c | 12 ++++++------
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 7da8279b54f8..246e11837800 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -383,6 +383,17 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
 	if (IS_ERR(entry->disc_table))
 		return PTR_ERR(entry->disc_table);
 
+	/*
+	 * The mapped discovery resource may be smaller than disc_header (its
+	 * size is the namespace's DVSEC entry_size in dwords, which can be
+	 * less than 4). Cap the copy to the actual resource size to avoid
+	 * reading past the mapped region; any unread dwords stay zero from
+	 * the zero-initialized allocation of the containing struct.
+	 */
+	memcpy_fromio(entry->disc_header, entry->disc_table,
+		      min_t(size_t, sizeof(entry->disc_header),
+			    resource_size(disc_res)));
+
 	if (ns->pmt_pre_decode) {
 		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
 		if (ret)
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index 8a0db0ef58c1..84202fc7920c 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -44,6 +44,7 @@ struct intel_pmt_entry {
 	struct telem_endpoint	*ep;
 	struct pci_dev		*pcidev;
 	struct intel_pmt_header	header;
+	u32			disc_header[4];
 	struct bin_attribute	pmt_bin_attr;
 	const struct attribute_group *attr_grp;
 	struct kobject		*kobj;
diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
index d22f633638be..953f35b6daec 100644
--- a/drivers/platform/x86/intel/pmt/telemetry.c
+++ b/drivers/platform/x86/intel/pmt/telemetry.c
@@ -72,16 +72,16 @@ static bool pmt_telem_region_overlaps(struct device *dev, u32 guid, u32 type)
 static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
 				   struct device *dev)
 {
-	void __iomem *disc_table = entry->disc_table;
 	struct intel_pmt_header *header = &entry->header;
+	u32 *disc_header = entry->disc_header;
 
-	header->access_type = TELEM_ACCESS(readl(disc_table));
-	header->guid = readl(disc_table + TELEM_GUID_OFFSET);
-	header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
+	header->access_type = TELEM_ACCESS(disc_header[0]);
+	header->guid = disc_header[1];
+	header->base_offset = disc_header[2];
 
 	/* Size is measured in DWORDS, but accessor returns bytes */
-	header->size = TELEM_SIZE(readl(disc_table));
-	header->telem_type = TELEM_TYPE(readl(entry->disc_table));
+	header->size = TELEM_SIZE(disc_header[0]);
+	header->telem_type = TELEM_TYPE(disc_header[0]);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v5 06/16] platform/x86/intel/pmt: Unify header fetch and add ACPI source

[David E. Box]

Allow the PMT class to read discovery headers from either PCI MMIO or
ACPI-provided entries, depending on the discovery source. The new
source-aware fetch helper retrieves the first two QWORDs for both paths
while keeping the mapped discovery table available for users such as
crashlog.

Split intel_pmt_populate_entry() into source-specific resolvers:
  - pmt_resolve_access_pci(): handles both ACCESS_LOCAL and ACCESS_BARID
    for PCI-backed devices and sets entry->pcidev. Same existing
    functionality.
  - pmt_resolve_access_acpi(): handles only ACCESS_BARID for ACPI-backed
    devices, rejecting ACCESS_LOCAL which has no valid semantics without
    a physical discovery resource.

Also, when copying discovery headers, bind the copy size to the canonical
discovery header definition instead of relying on separate literals.

This maintains existing PCI behavior and makes no functional changes
for PCI devices.

Assisted-by: GitHub-Copilot:claude-opus-4.7
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 changes:
  - Cap memcpy_fromio() in pmt_get_headers() PCI branch to
    resource_size() of the discovery resource, matching the cap
    added in the cache-introducing patch.
  - Documented in the ACPI branch that disc_table is intentionally
    NULL on that source, so consumers that dereference disc_table
    must only be wired to INTEL_VSEC_DISC_PCI namespaces.

V4 changes:
  - Added discovery header width macro, INTEL_VSEC_ACPI_DISC_DWORDS, in
    shared intel_vsec header and in definitions.
  - Aliased to PMT_DISC_HEADER_DWORDS and converted PMT discovery header
    arrays to use it.
  - Replaced literal header copy sizes with entry->disc_header size in
    pmt_get_headers() for PCI and ACPI paths.

V3 changes:
  - Folded the header fetch rework back into intel_pmt_dev_create() after
    dropping the previous common header decode helper patch
  - Cleaned up line wrapping/indentation

V2 changes:
  - In pmt_resolve_access_acpi(), moved dev_err() call to single line
    instead of split across two lines
  - Restructured error handling in intel_pmt_populate_entry(), moving error
    returns from after switch/case into each case statement for better
    readability
  - Addressed Ilpo's feedback on error message formatting and error
    handling patterns

 drivers/platform/x86/intel/pmt/class.c | 157 +++++++++++++++++++++----
 drivers/platform/x86/intel/pmt/class.h |   3 +-
 include/linux/intel_vsec.h             |   5 +-
 3 files changed, 142 insertions(+), 23 deletions(-)

diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
index 246e11837800..1a77709edc6a 100644
--- a/drivers/platform/x86/intel/pmt/class.c
+++ b/drivers/platform/x86/intel/pmt/class.c
@@ -204,9 +204,9 @@ struct class intel_pmt_class = {
 };
 EXPORT_SYMBOL_GPL(intel_pmt_class);
 
-static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
-				    struct intel_vsec_device *ivdev,
-				    int idx)
+static int pmt_resolve_access_pci(struct intel_pmt_entry *entry,
+				  struct intel_vsec_device *ivdev,
+				  int idx)
 {
 	struct pci_dev *pci_dev = to_pci_dev(ivdev->dev);
 	struct device *dev = &ivdev->auxdev.dev;
@@ -286,6 +286,81 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
 	}
 
 	entry->pcidev = pci_dev;
+
+	return 0;
+}
+
+static int pmt_resolve_access_acpi(struct intel_pmt_entry *entry,
+				   struct intel_vsec_device *ivdev)
+{
+	struct pci_dev *pci_dev = NULL;
+	struct device *dev = &ivdev->auxdev.dev;
+	struct intel_pmt_header *header = &entry->header;
+	u8 bir;
+
+	if (dev_is_pci(ivdev->dev))
+		pci_dev = to_pci_dev(ivdev->dev);
+
+	/*
+	 * The base offset should always be 8 byte aligned.
+	 *
+	 * For non-local access types the lower 3 bits of base offset
+	 * contains the index of the base address register where the
+	 * telemetry can be found.
+	 */
+	bir = GET_BIR(header->base_offset);
+
+	switch (header->access_type) {
+	case ACCESS_BARID:
+		/* ACPI platform drivers use base_addr */
+		if (ivdev->base_addr) {
+			entry->base_addr = ivdev->base_addr +
+					   GET_ADDRESS(header->base_offset);
+			break;
+		}
+
+		/* If base_addr is not provided, then this is an ACPI companion device */
+		if (!pci_dev) {
+			dev_err(dev, "ACCESS_BARID requires PCI BAR resources or base_addr\n");
+			return -EINVAL;
+		}
+
+		entry->base_addr = pci_resource_start(pci_dev, bir) +
+			GET_ADDRESS(header->base_offset);
+		break;
+	default:
+		dev_err(dev, "Unsupported access type %d for ACPI based PMT\n",
+			header->access_type);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
+				    struct intel_vsec_device *ivdev,
+				    int idx)
+{
+	struct intel_pmt_header *header = &entry->header;
+	struct device *dev = &ivdev->auxdev.dev;
+	int ret;
+
+	switch (ivdev->src) {
+	case INTEL_VSEC_DISC_PCI:
+		ret = pmt_resolve_access_pci(entry, ivdev, idx);
+		if (ret)
+			return ret;
+		break;
+	case INTEL_VSEC_DISC_ACPI:
+		ret = pmt_resolve_access_acpi(entry, ivdev);
+		if (ret)
+			return ret;
+		break;
+	default:
+		dev_err(dev, "Unknown discovery source: %d\n", ivdev->src);
+		return -EINVAL;
+	}
+
 	entry->guid = header->guid;
 	entry->size = header->size;
 	entry->cb = ivdev->priv_data;
@@ -370,29 +445,71 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
 	return ret;
 }
 
+static int pmt_get_headers(struct intel_vsec_device *ivdev, int idx,
+			   struct intel_pmt_entry *entry,
+			   u32 headers[PMT_DISC_HEADER_DWORDS])
+{
+	struct device *dev = &ivdev->auxdev.dev;
+	size_t header_bytes = sizeof(entry->disc_header);
+
+	switch (ivdev->src) {
+	case INTEL_VSEC_DISC_PCI: {
+		struct resource *disc_res = &ivdev->resource[idx];
+		void __iomem *disc_table;
+
+		disc_table = devm_ioremap_resource(dev, disc_res);
+		if (IS_ERR(disc_table))
+			return PTR_ERR(disc_table);
+
+		/*
+		 * The mapped resource is sized by the namespace's DVSEC
+		 * entry_size (in dwords), which can be less than
+		 * PMT_DISC_HEADER_DWORDS (e.g. telemetry uses entry_size = 3,
+		 * 12 bytes). Cap the copy to resource_size() to avoid reading
+		 * past the mapped region; any unread dwords stay zero from
+		 * the zero-initialized allocation of the containing struct.
+		 */
+		memcpy_fromio(headers, disc_table,
+			      min_t(size_t, header_bytes,
+				    resource_size(disc_res)));
+		memcpy(entry->disc_header, headers, header_bytes);
+
+		/* Used by crashlog driver */
+		entry->disc_table = disc_table;
+
+		return 0;
+	}
+	case INTEL_VSEC_DISC_ACPI: {
+		memcpy(headers, &ivdev->acpi_disc[idx][0], header_bytes);
+		memcpy(entry->disc_header, headers, header_bytes);
+		/*
+		 * No MMIO mapping exists on the ACPI source path; the cached
+		 * headers are the only view of the discovery record. Consumers
+		 * that dereference disc_table (e.g. crashlog) must therefore
+		 * only be wired to namespaces backed by INTEL_VSEC_DISC_PCI.
+		 */
+		entry->disc_table = NULL;
+
+		return 0;
+	}
+	default:
+		dev_err(dev, "Unknown discovery source type: %d\n", ivdev->src);
+		break;
+	}
+
+	return -EINVAL;
+}
+
 int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespace *ns,
 			 struct intel_vsec_device *intel_vsec_dev, int idx)
 {
 	struct device *dev = &intel_vsec_dev->auxdev.dev;
-	struct resource	*disc_res;
+	u32 headers[PMT_DISC_HEADER_DWORDS];
 	int ret;
 
-	disc_res = &intel_vsec_dev->resource[idx];
-
-	entry->disc_table = devm_ioremap_resource(dev, disc_res);
-	if (IS_ERR(entry->disc_table))
-		return PTR_ERR(entry->disc_table);
-
-	/*
-	 * The mapped discovery resource may be smaller than disc_header (its
-	 * size is the namespace's DVSEC entry_size in dwords, which can be
-	 * less than 4). Cap the copy to the actual resource size to avoid
-	 * reading past the mapped region; any unread dwords stay zero from
-	 * the zero-initialized allocation of the containing struct.
-	 */
-	memcpy_fromio(entry->disc_header, entry->disc_table,
-		      min_t(size_t, sizeof(entry->disc_header),
-			    resource_size(disc_res)));
+	ret = pmt_get_headers(intel_vsec_dev, idx, entry, headers);
+	if (ret)
+		return ret;
 
 	if (ns->pmt_pre_decode) {
 		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
index 84202fc7920c..950fa4ee300d 100644
--- a/drivers/platform/x86/intel/pmt/class.h
+++ b/drivers/platform/x86/intel/pmt/class.h
@@ -18,6 +18,7 @@
 /* PMT discovery base address/offset register layout */
 #define GET_BIR(v)		((v) & GENMASK(2, 0))
 #define GET_ADDRESS(v)		((v) & GENMASK(31, 3))
+#define PMT_DISC_HEADER_DWORDS INTEL_VSEC_ACPI_DISC_DWORDS
 
 struct device;
 struct pci_dev;
@@ -44,7 +45,7 @@ struct intel_pmt_entry {
 	struct telem_endpoint	*ep;
 	struct pci_dev		*pcidev;
 	struct intel_pmt_header	header;
-	u32			disc_header[4];
+	u32			disc_header[PMT_DISC_HEADER_DWORDS];
 	struct bin_attribute	pmt_bin_attr;
 	const struct attribute_group *attr_grp;
 	struct kobject		*kobj;
diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
index 1fe5665a9d02..4c58a7f5031e 100644
--- a/include/linux/intel_vsec.h
+++ b/include/linux/intel_vsec.h
@@ -28,6 +28,7 @@
 #define INTEL_DVSEC_TABLE_BAR(x)	((x) & GENMASK(2, 0))
 #define INTEL_DVSEC_TABLE_OFFSET(x)	((x) & GENMASK(31, 3))
 #define TABLE_OFFSET_SHIFT		3
+#define INTEL_VSEC_ACPI_DISC_DWORDS	4
 
 struct device;
 struct pci_dev;
@@ -122,7 +123,7 @@ struct intel_vsec_platform_info {
 	struct device *parent;
 	struct intel_vsec_header **headers;
 	const struct vsec_feature_dependency *deps;
-	u32 (*acpi_disc)[4];
+	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
 	enum intel_vsec_disc_source src;
 	void *priv_data;
 	unsigned long caps;
@@ -154,7 +155,7 @@ struct intel_vsec_device {
 	struct auxiliary_device auxdev;
 	struct device *dev;
 	struct resource *resource;
-	u32 (*acpi_disc)[4];
+	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
 	enum intel_vsec_disc_source src;
 	struct ida *ida;
 	int num_resources;
-- 
2.43.0

[PATCH v5 07/16] platform/x86/intel/pmc: Add PMC SSRAM Kconfig description

[David E. Box]

Add a proper description for the intel_pmc_ssram driver.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmc/Kconfig | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig
index c6ef0bcf76af..0f19dc7edcf9 100644
--- a/drivers/platform/x86/intel/pmc/Kconfig
+++ b/drivers/platform/x86/intel/pmc/Kconfig
@@ -28,3 +28,14 @@ config INTEL_PMC_CORE
 
 config INTEL_PMC_SSRAM_TELEMETRY
 	tristate
+	help
+	  This PCI driver discovers PMC SSRAM telemetry regions through the
+	  PMC's MMIO interface and registers them with the Intel VSEC framework
+	  as Intel PMT telemetry devices.
+
+	  It probes the PMC SSRAM device, extracts DVSEC information from MMIO,
+	  reads device IDs and base addresses for multiple PMCs (main, IOE, PCH),
+	  and exposes the discovered telemetry through Intel PMT interfaces
+	  (including sysfs).
+
+	  This option is selected by INTEL_PMC_CORE.
-- 
2.43.0

[PATCH v5 08/16] platform/x86/intel/pmc: Add ACPI PWRM telemetry driver for Nova Lake S

[David E. Box]

Add an ACPI-based PMC PWRM telemetry driver for Nova Lake S. The driver
locates PMT discovery data in _DSD under the Intel VSEC UUID, parses it,
and registers telemetry regions with the PMT/VSEC framework so PMC
telemetry is exposed via existing PMT interfaces.

Export pmc_parse_telem_dsd() and pmc_find_telem_guid() to support ACPI
discovery in other PMC drivers (e.g., ssram_telemetry) without duplicating
ACPI parsing logic. Also export acpi_disc_t typedef from core.h for callers
to properly declare discovery table arrays.

Selected by INTEL_PMC_CORE. Existing PCI functionality is preserved.

Assisted-by: GitHub-Copilot:claude-opus-4.7
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 changes:
  - Added #include <linux/limits.h> for U16_MAX (Ilpo).
  - Split acpi_handle declaration from ACPI_HANDLE() assignment
    and placed the assignment immediately before the !handle
    check (Ilpo).
  - Reordered local variables in pmc_pwrm_acpi_probe() in
    reverse-xmas-tree order (Ilpo).

V4 changes:
  - These changes were supposed to be in V3
  - Updated pmc_parse_telem_dsd() in pwrm_telemetry.c to use acpi_disc_t
    in the function return type for consistency with the exported typedef
  - In pmc_parse_telem_dsd(), change acpi_disc declaration to happen at the
    allocation site as specified by cleanup.h
  - Style, readability and cleanup-path refinement based on review
    feedback

V2 changes:
  - Added explicit <linux/uuid.h> include for guid_t type availability in
    core.h
  - Added explicit <linux/bits.h> include in pwrm_telemetry.c for GENMASK()
  - Added <linux/cleanup.h> and converted goto based cleanup to __free()
    attributes per Ilpo's feedback
  - Combined u64 hdr0 and u64 hdr1 into single declaration
  - Converted pmc_parse_telem_dsd() to return acpi_disc directly with
    ERR_PTR() for failures
  - Added braces around _DSD evaluation failure path

 drivers/platform/x86/intel/pmc/Kconfig        |  14 ++
 drivers/platform/x86/intel/pmc/Makefile       |   2 +
 drivers/platform/x86/intel/pmc/core.h         |  16 ++
 .../platform/x86/intel/pmc/pwrm_telemetry.c   | 216 ++++++++++++++++++
 4 files changed, 248 insertions(+)
 create mode 100644 drivers/platform/x86/intel/pmc/pwrm_telemetry.c

diff --git a/drivers/platform/x86/intel/pmc/Kconfig b/drivers/platform/x86/intel/pmc/Kconfig
index 0f19dc7edcf9..937186b0b5dd 100644
--- a/drivers/platform/x86/intel/pmc/Kconfig
+++ b/drivers/platform/x86/intel/pmc/Kconfig
@@ -9,6 +9,7 @@ config INTEL_PMC_CORE
 	depends on ACPI
 	depends on INTEL_PMT_TELEMETRY
 	select INTEL_PMC_SSRAM_TELEMETRY
+	select INTEL_PMC_PWRM_TELEMETRY
 	help
 	  The Intel Platform Controller Hub for Intel Core SoCs provides access
 	  to Power Management Controller registers via various interfaces. This
@@ -39,3 +40,16 @@ config INTEL_PMC_SSRAM_TELEMETRY
 	  (including sysfs).
 
 	  This option is selected by INTEL_PMC_CORE.
+
+config INTEL_PMC_PWRM_TELEMETRY
+	tristate
+	help
+	  This driver discovers PMC PWRM telemetry regions described in ACPI
+	  _DSD and registers them with the Intel VSEC framework as Intel PMT
+	  telemetry devices.
+
+	  It validates the ACPI discovery data and publishes the discovered
+	  regions so they can be accessed through the Intel PMT telemetry
+	  interfaces (including sysfs).
+
+	  This option is selected by INTEL_PMC_CORE.
diff --git a/drivers/platform/x86/intel/pmc/Makefile b/drivers/platform/x86/intel/pmc/Makefile
index bb960c8721d7..fdbb768f7b09 100644
--- a/drivers/platform/x86/intel/pmc/Makefile
+++ b/drivers/platform/x86/intel/pmc/Makefile
@@ -12,3 +12,5 @@ obj-$(CONFIG_INTEL_PMC_CORE)		+= intel_pmc_core_pltdrv.o
 # Intel PMC SSRAM driver
 intel_pmc_ssram_telemetry-y		+= ssram_telemetry.o
 obj-$(CONFIG_INTEL_PMC_SSRAM_TELEMETRY)	+= intel_pmc_ssram_telemetry.o
+intel_pmc_pwrm_telemetry-y		+= pwrm_telemetry.o
+obj-$(CONFIG_INTEL_PMC_PWRM_TELEMETRY)	+= intel_pmc_pwrm_telemetry.o
diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index 118c8740ad3a..f458eb908c07 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -14,10 +14,15 @@
 
 #include <linux/acpi.h>
 #include <linux/bits.h>
+#include <linux/cleanup.h>
+#include <linux/intel_vsec.h>
 #include <linux/platform_device.h>
+#include <linux/uuid.h>
 
 struct telem_endpoint;
 
+DEFINE_FREE(pmc_acpi_free, void *, if (_T) ACPI_FREE(_T))
+
 #define SLP_S0_RES_COUNTER_MASK			GENMASK(31, 0)
 
 #define PMC_BASE_ADDR_DEFAULT			0xFE000000
@@ -562,6 +567,8 @@ int pmc_core_pmt_get_blk_sub_req(struct pmc_dev *pmcdev, struct pmc *pmc,
 extern const struct file_operations pmc_core_substate_req_regs_fops;
 extern const struct file_operations pmc_core_substate_blk_req_fops;
 
+extern const guid_t intel_vsec_guid;
+
 #define pmc_for_each_mode(mode, pmc)						\
 	for (unsigned int __i = 0, __cond;					\
 	     __cond = __i < (pmc)->num_lpm_modes,				\
@@ -583,4 +590,13 @@ static const struct file_operations __name ## _fops = {			\
 	.release	= single_release,				\
 }
 
+struct intel_vsec_header;
+union acpi_object;
+
+/* Avoid checkpatch warning */
+typedef u32 (*acpi_disc_t)[INTEL_VSEC_ACPI_DISC_DWORDS];
+
+acpi_disc_t pmc_parse_telem_dsd(union acpi_object *obj,
+				struct intel_vsec_header *header);
+union acpi_object *pmc_find_telem_guid(union acpi_object *dsd);
 #endif /* PMC_CORE_H */
diff --git a/drivers/platform/x86/intel/pmc/pwrm_telemetry.c b/drivers/platform/x86/intel/pmc/pwrm_telemetry.c
new file mode 100644
index 000000000000..70fdc79b48a2
--- /dev/null
+++ b/drivers/platform/x86/intel/pmc/pwrm_telemetry.c
@@ -0,0 +1,216 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Intel PMC PWRM ACPI driver
+ *
+ * Copyright (C) 2025, Intel Corporation
+ */
+
+#include <linux/acpi.h>
+#include <linux/bits.h>
+#include <linux/bitfield.h>
+#include <linux/cleanup.h>
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/intel_vsec.h>
+#include <linux/limits.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/resource.h>
+#include <linux/slab.h>
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+#include "core.h"
+
+#define ENTRY_LEN		5
+
+/* DWORD2 */
+#define DVSEC_ID_MASK		GENMASK(15, 0)
+#define NUM_ENTRIES_MASK	GENMASK(23, 16)
+#define ENTRY_SIZE_MASK		GENMASK(31, 24)
+
+/* DWORD3 */
+#define TBIR_MASK		GENMASK(2, 0)
+#define DISC_TBL_OFF_MASK	GENMASK(31, 3)
+
+const guid_t intel_vsec_guid =
+	GUID_INIT(0x294903fb, 0x634d, 0x4fc7, 0xaf, 0x1f, 0x0f, 0xb9,
+		  0x56, 0xb0, 0x4f, 0xc1);
+
+static bool is_valid_entry(union acpi_object *pkg)
+{
+	int i;
+
+	if (!pkg || pkg->type != ACPI_TYPE_PACKAGE || pkg->package.count != ENTRY_LEN)
+		return false;
+
+	if (pkg->package.elements[0].type != ACPI_TYPE_STRING)
+		return false;
+
+	for (i = 1; i < ENTRY_LEN; i++)
+		if (pkg->package.elements[i].type != ACPI_TYPE_INTEGER)
+			return false;
+
+	return true;
+}
+
+acpi_disc_t pmc_parse_telem_dsd(union acpi_object *obj,
+			struct intel_vsec_header *header)
+{
+	union acpi_object *vsec_pkg;
+	union acpi_object *disc_pkg;
+	u64 hdr0, hdr1;
+	int num_regions;
+	int i;
+
+	if (!header)
+		return ERR_PTR(-EINVAL);
+
+	if (!obj || obj->type != ACPI_TYPE_PACKAGE || obj->package.count != 2)
+		return ERR_PTR(-EINVAL);
+
+	/* First Package is DVSEC info */
+	vsec_pkg = &obj->package.elements[0];
+	if (!is_valid_entry(vsec_pkg))
+		return ERR_PTR(-EINVAL);
+
+	hdr0 = vsec_pkg->package.elements[3].integer.value;
+	hdr1 = vsec_pkg->package.elements[4].integer.value;
+
+	header->id          = FIELD_GET(DVSEC_ID_MASK, hdr0);
+	header->num_entries = FIELD_GET(NUM_ENTRIES_MASK, hdr0);
+	header->entry_size  = FIELD_GET(ENTRY_SIZE_MASK, hdr0);
+	header->tbir        = FIELD_GET(TBIR_MASK, hdr1);
+	header->offset      = FIELD_GET(DISC_TBL_OFF_MASK, hdr1);
+
+	/* Second Package contains the discovery tables */
+	disc_pkg = &obj->package.elements[1];
+	if (disc_pkg->type != ACPI_TYPE_PACKAGE || disc_pkg->package.count < 1)
+		return ERR_PTR(-EINVAL);
+
+	num_regions = disc_pkg->package.count;
+	if (header->num_entries != num_regions)
+		return ERR_PTR(-EINVAL);
+
+	acpi_disc_t disc __free(kfree) = kmalloc_array(num_regions, sizeof(*disc),
+						      GFP_KERNEL);
+	if (!disc)
+		return ERR_PTR(-ENOMEM);
+
+	for (i = 0; i < num_regions; i++) {
+		union acpi_object *pkg;
+		u64 value;
+		int j;
+
+		pkg = &disc_pkg->package.elements[i];
+		if (!is_valid_entry(pkg))
+			return ERR_PTR(-EINVAL);
+
+		/* Element 0 is a descriptive string; DWORD values start at index 1. */
+		for (j = 1; j < ENTRY_LEN; j++) {
+			value = pkg->package.elements[j].integer.value;
+			if (value > U32_MAX)
+				return ERR_PTR(-ERANGE);
+
+			disc[i][j - 1] = value;
+		}
+	}
+
+	return no_free_ptr(disc);
+}
+EXPORT_SYMBOL_NS_GPL(pmc_parse_telem_dsd, "INTEL_PMC_CORE");
+
+union acpi_object *pmc_find_telem_guid(union acpi_object *dsd)
+{
+	int i;
+
+	if (!dsd || dsd->type != ACPI_TYPE_PACKAGE)
+		return NULL;
+
+	for (i = 0; i + 1 < dsd->package.count; i += 2) {
+		union acpi_object *uuid_obj, *data_obj;
+		guid_t uuid;
+
+		uuid_obj = &dsd->package.elements[i];
+		data_obj = &dsd->package.elements[i + 1];
+
+		if (uuid_obj->type != ACPI_TYPE_BUFFER ||
+		    uuid_obj->buffer.length != 16)
+			continue;
+
+		memcpy(&uuid, uuid_obj->buffer.pointer, 16);
+		if (guid_equal(&uuid, &intel_vsec_guid))
+			return data_obj;
+	}
+
+	return NULL;
+}
+EXPORT_SYMBOL_NS_GPL(pmc_find_telem_guid, "INTEL_PMC_CORE");
+
+static int pmc_pwrm_acpi_probe(struct platform_device *pdev)
+{
+	struct intel_vsec_header header;
+	struct intel_vsec_header *headers[2] = { &header, NULL };
+	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct intel_vsec_platform_info info = { };
+	struct device *dev = &pdev->dev;
+	union acpi_object *dsd;
+	struct resource *res;
+	acpi_handle handle;
+	acpi_status status;
+
+	handle = ACPI_HANDLE(&pdev->dev);
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_object(handle, "_DSD", NULL, &buf);
+	if (ACPI_FAILURE(status)) {
+		return dev_err_probe(dev, -ENODEV, "Could not evaluate _DSD: %s\n",
+				     acpi_format_exception(status));
+	}
+
+	void *dsd_buf __free(pmc_acpi_free) = buf.pointer;
+
+	dsd = pmc_find_telem_guid(dsd_buf);
+	if (!dsd)
+		return -ENODEV;
+
+	acpi_disc_t acpi_disc __free(kfree) = pmc_parse_telem_dsd(dsd, &header);
+	if (IS_ERR(acpi_disc))
+		return PTR_ERR(acpi_disc);
+
+	res = platform_get_resource(pdev, IORESOURCE_MEM, header.tbir);
+	if (!res)
+		return -EINVAL;
+
+	info.headers = headers;
+	info.caps = VSEC_CAP_TELEMETRY;
+	info.acpi_disc = acpi_disc;
+	info.src = INTEL_VSEC_DISC_ACPI;
+	info.base_addr = res->start;
+
+	return intel_vsec_register(&pdev->dev, &info);
+}
+
+static const struct acpi_device_id pmc_pwrm_acpi_ids[] = {
+	{ "INTC1122", 0 }, /* Nova Lake */
+	{ "INTC1129", 0 }, /* Nova Lake */
+	{ }
+};
+MODULE_DEVICE_TABLE(acpi, pmc_pwrm_acpi_ids);
+
+static struct platform_driver pmc_pwrm_acpi_driver = {
+	.probe = pmc_pwrm_acpi_probe,
+	.driver = {
+		.name = "intel_pmc_pwrm_acpi",
+		.acpi_match_table = ACPI_PTR(pmc_pwrm_acpi_ids),
+	},
+};
+module_platform_driver(pmc_pwrm_acpi_driver);
+
+MODULE_AUTHOR("David E. Box <david.e.box@linux.intel.com>");
+MODULE_DESCRIPTION("Intel PMC PWRM ACPI driver");
+MODULE_LICENSE("GPL");
+MODULE_IMPORT_NS("INTEL_VSEC");
-- 
2.43.0

[PATCH v5 09/16] platform/x86/intel/pmc/ssram: Rename probe and PCI ID table for consistency

[David E. Box]

Rename intel_pmc_ssram_telemetry_probe() to pmc_ssram_telemetry_probe() and
intel_pmc_ssram_telemetry_pci_ids[] to pmc_ssram_telemetry_pci_ids[],
updating the MODULE_DEVICE_TABLE() and pci_driver wiring accordingly.

This aligns the symbol names with the driver filename and module name,
reduces redundant intel_ prefixes, and improves readability. No functional
behavior changes are intended.

Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmc/ssram_telemetry.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 6f6e83e70fc5..1deb4d71da3f 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -149,7 +149,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
 }
 EXPORT_SYMBOL_GPL(pmc_ssram_telemetry_get_pmc_info);
 
-static int intel_pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
+static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 {
 	int ret;
 
@@ -183,7 +183,7 @@ static int intel_pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct
 	return ret;
 }
 
-static const struct pci_device_id intel_pmc_ssram_telemetry_pci_ids[] = {
+static const struct pci_device_id pmc_ssram_telemetry_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_MTL_SOCM) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCS) },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCM) },
@@ -193,14 +193,14 @@ static const struct pci_device_id intel_pmc_ssram_telemetry_pci_ids[] = {
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_WCL_PCDN) },
 	{ }
 };
-MODULE_DEVICE_TABLE(pci, intel_pmc_ssram_telemetry_pci_ids);
+MODULE_DEVICE_TABLE(pci, pmc_ssram_telemetry_pci_ids);
 
-static struct pci_driver intel_pmc_ssram_telemetry_driver = {
+static struct pci_driver pmc_ssram_telemetry_driver = {
 	.name = "intel_pmc_ssram_telemetry",
-	.id_table = intel_pmc_ssram_telemetry_pci_ids,
-	.probe = intel_pmc_ssram_telemetry_probe,
+	.id_table = pmc_ssram_telemetry_pci_ids,
+	.probe = pmc_ssram_telemetry_probe,
 };
-module_pci_driver(intel_pmc_ssram_telemetry_driver);
+module_pci_driver(pmc_ssram_telemetry_driver);
 
 MODULE_IMPORT_NS("INTEL_VSEC");
 MODULE_AUTHOR("Xi Pardee <xi.pardee@intel.com>");
-- 
2.43.0

[PATCH v5 10/16] platform/x86/intel/pmc/ssram: Use fixed-size static pmc array

[David E. Box]

From: Xi Pardee <xi.pardee@linux.intel.com>

Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
static array, eliminating per-probe allocation overhead and simplifying
lifetime management.

Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
check devid availability and tighten input bounds checking. Drop
null-pointer checks now that the storage is static.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 changes:
  - Replaced hardcoded array size [3] with MAX_NUM_PMC constant

 drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 1deb4d71da3f..4bfe60ee55ca 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -24,7 +24,7 @@
 
 DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
 
-static struct pmc_ssram_telemetry *pmc_ssram_telems;
+static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
 static bool device_probed;
 
 static int
@@ -140,7 +140,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
 	if (pmc_idx >= MAX_NUM_PMC)
 		return -EINVAL;
 
-	if (!pmc_ssram_telems || !pmc_ssram_telems[pmc_idx].devid)
+	if (!pmc_ssram_telems[pmc_idx].devid)
 		return -ENODEV;
 
 	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
@@ -153,12 +153,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 {
 	int ret;
 
-	pmc_ssram_telems = devm_kzalloc(&pcidev->dev, sizeof(*pmc_ssram_telems) * MAX_NUM_PMC,
-					GFP_KERNEL);
-	if (!pmc_ssram_telems) {
-		ret = -ENOMEM;
-		goto probe_finish;
-	}
 
 	ret = pcim_enable_device(pcidev);
 	if (ret) {
-- 
2.43.0

[PATCH v5 11/16] platform/x86/intel/pmc/ssram: Refactor DEVID/PWRMBASE extraction into helper

[David E. Box]

Move DEVID/PWRMBASE extraction into pmc_ssram_get_devid_pwrmbase().

This is a preparatory refactor to place functionality in a common helper
for reuse by a subsequent patch. Additionally add missing bits.h
include and define SSRAM_BASE_ADDR_MASK for the address extraction mask.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 changes:
  - Added missing <linux/bits.h> include for GENMASK_ULL() used in get_base()
  - Defined SSRAM_BASE_ADDR_MASK macro to replace magic mask constant
    GENMASK_ULL(63, 3)

 .../platform/x86/intel/pmc/ssram_telemetry.c  | 33 ++++++++++++-------
 1 file changed, 21 insertions(+), 12 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 4bfe60ee55ca..779e84c724ac 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2023, Intel Corporation.
  */
 
+#include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/intel_vsec.h>
 #include <linux/pci.h>
@@ -21,12 +22,30 @@
 #define SSRAM_PCH_OFFSET	0x60
 #define SSRAM_IOE_OFFSET	0x68
 #define SSRAM_DEVID_OFFSET	0x70
+#define SSRAM_BASE_ADDR_MASK	GENMASK_ULL(63, 3)
 
 DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
 
 static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
 static bool device_probed;
 
+static inline u64 get_base(void __iomem *addr, u32 offset)
+{
+	return lo_hi_readq(addr + offset) & SSRAM_BASE_ADDR_MASK;
+}
+
+static void pmc_ssram_get_devid_pwrmbase(void __iomem *ssram, unsigned int pmc_idx)
+{
+	u64 pwrm_base;
+	u16 devid;
+
+	pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
+	devid = readw(ssram + SSRAM_DEVID_OFFSET);
+
+	pmc_ssram_telems[pmc_idx].devid = devid;
+	pmc_ssram_telems[pmc_idx].base_addr = pwrm_base;
+}
+
 static int
 pmc_ssram_telemetry_add_pmt(struct pci_dev *pcidev, u64 ssram_base, void __iomem *ssram)
 {
@@ -63,18 +82,12 @@ pmc_ssram_telemetry_add_pmt(struct pci_dev *pcidev, u64 ssram_base, void __iomem
 	return intel_vsec_register(&pcidev->dev, &info);
 }
 
-static inline u64 get_base(void __iomem *addr, u32 offset)
-{
-	return lo_hi_readq(addr + offset) & GENMASK_ULL(63, 3);
-}
-
 static int
 pmc_ssram_telemetry_get_pmc(struct pci_dev *pcidev, unsigned int pmc_idx, u32 offset)
 {
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *tmp_ssram = NULL;
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *ssram = NULL;
-	u64 ssram_base, pwrm_base;
-	u16 devid;
+	u64 ssram_base;
 
 	ssram_base = pci_resource_start(pcidev, 0);
 	tmp_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
@@ -99,11 +112,7 @@ pmc_ssram_telemetry_get_pmc(struct pci_dev *pcidev, unsigned int pmc_idx, u32 of
 		ssram = no_free_ptr(tmp_ssram);
 	}
 
-	pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
-	devid = readw(ssram + SSRAM_DEVID_OFFSET);
-
-	pmc_ssram_telems[pmc_idx].devid = devid;
-	pmc_ssram_telems[pmc_idx].base_addr = pwrm_base;
+	pmc_ssram_get_devid_pwrmbase(ssram, pmc_idx);
 
 	/* Find and register and PMC telemetry entries */
 	return pmc_ssram_telemetry_add_pmt(pcidev, ssram_base, ssram);
-- 
2.43.0

[PATCH v5 12/16] platform/x86/intel/pmc/ssram: Add PCI platform data

[David E. Box]

Add per-device platform data for SSRAM telemetry PCI IDs and route probe
through a method selector driven by id->driver_data.

This is a preparatory refactor for follow-on discovery methods while
preserving current behavior: all supported IDs continue to use the PCI
initialization path.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 changes:
  - Added missing <linux/device.h> include for dev_dbg() usage in probe

 .../platform/x86/intel/pmc/ssram_telemetry.c  | 70 +++++++++++++++----
 1 file changed, 56 insertions(+), 14 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 779e84c724ac..6917a10cbc80 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -7,6 +7,7 @@
 
 #include <linux/bits.h>
 #include <linux/cleanup.h>
+#include <linux/device.h>
 #include <linux/intel_vsec.h>
 #include <linux/pci.h>
 #include <linux/types.h>
@@ -26,6 +27,18 @@
 
 DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
 
+enum resource_method {
+	RES_METHOD_PCI,
+};
+
+struct ssram_type {
+	enum resource_method method;
+};
+
+static const struct ssram_type pci_main = {
+	.method = RES_METHOD_PCI,
+};
+
 static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
 static bool device_probed;
 
@@ -83,7 +96,7 @@ pmc_ssram_telemetry_add_pmt(struct pci_dev *pcidev, u64 ssram_base, void __iomem
 }
 
 static int
-pmc_ssram_telemetry_get_pmc(struct pci_dev *pcidev, unsigned int pmc_idx, u32 offset)
+pmc_ssram_telemetry_get_pmc_pci(struct pci_dev *pcidev, unsigned int pmc_idx, u32 offset)
 {
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *tmp_ssram = NULL;
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *ssram = NULL;
@@ -118,6 +131,20 @@ pmc_ssram_telemetry_get_pmc(struct pci_dev *pcidev, unsigned int pmc_idx, u32 of
 	return pmc_ssram_telemetry_add_pmt(pcidev, ssram_base, ssram);
 }
 
+static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
+{
+	int ret;
+
+	ret = pmc_ssram_telemetry_get_pmc_pci(pcidev, PMC_IDX_MAIN, 0);
+	if (ret)
+		return ret;
+
+	pmc_ssram_telemetry_get_pmc_pci(pcidev, PMC_IDX_IOE, SSRAM_IOE_OFFSET);
+	pmc_ssram_telemetry_get_pmc_pci(pcidev, PMC_IDX_PCH, SSRAM_PCH_OFFSET);
+
+	return ret;
+}
+
 /**
  * pmc_ssram_telemetry_get_pmc_info() - Get a PMC devid and base_addr information
  * @pmc_idx:               Index of the PMC
@@ -160,8 +187,18 @@ EXPORT_SYMBOL_GPL(pmc_ssram_telemetry_get_pmc_info);
 
 static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_device_id *id)
 {
+	const struct ssram_type *ssram_type;
+	enum resource_method method;
 	int ret;
 
+	ssram_type = (const struct ssram_type *)id->driver_data;
+	if (!ssram_type) {
+		dev_dbg(&pcidev->dev, "missing driver data\n");
+		ret = -EINVAL;
+		goto probe_finish;
+	}
+
+	method = ssram_type->method;
 
 	ret = pcim_enable_device(pcidev);
 	if (ret) {
@@ -169,12 +206,10 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 		goto probe_finish;
 	}
 
-	ret = pmc_ssram_telemetry_get_pmc(pcidev, PMC_IDX_MAIN, 0);
-	if (ret)
-		goto probe_finish;
-
-	pmc_ssram_telemetry_get_pmc(pcidev, PMC_IDX_IOE, SSRAM_IOE_OFFSET);
-	pmc_ssram_telemetry_get_pmc(pcidev, PMC_IDX_PCH, SSRAM_PCH_OFFSET);
+	if (method == RES_METHOD_PCI)
+		ret = pmc_ssram_telemetry_pci_init(pcidev);
+	else
+		ret = -EINVAL;
 
 probe_finish:
 	/*
@@ -187,13 +222,20 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 }
 
 static const struct pci_device_id pmc_ssram_telemetry_pci_ids[] = {
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_MTL_SOCM) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCS) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCM) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_LNL_SOCM) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_PTL_PCDH) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_PTL_PCDP) },
-	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_WCL_PCDN) },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_MTL_SOCM),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCS),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_ARL_SOCM),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_LNL_SOCM),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_PTL_PCDH),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_PTL_PCDP),
+		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_WCL_PCDN),
+		.driver_data = (kernel_ulong_t)&pci_main },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, pmc_ssram_telemetry_pci_ids);
-- 
2.43.0

[PATCH v5 13/16] platform/x86/intel/pmc/ssram: Refactor memory barrier for reentrant probe

[David E. Box]

From: Xi Pardee <xi.pardee@linux.intel.com>

Previously, a single global 'device_probed' flag with memory barriers was
used to prevent callers from reading PMC info before probe completion.  The
write barrier in probe ensured all data, devid and base_addr, was visible
before signaling completion, and the read barrier in callers ensured they
checked the flag before reading data.

A following commit will make probe reentrant, requiring that a different
synchronization flag be used since a single global flag cannot coordinate
multiple concurrent probes.

Switch to per-index devid publication. Each probe instance writes base_addr
first, then a write barrier ensures visibility before devid is written as
the completion signal. Callers check devid first, then use a read barrier
before reading base_addr. This per-index approach allows multiple probes to
work independently while maintaining the same memory ordering guarantees.

Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 changes:
  - Expanded commit message to explain synchronization rationale
  - Remove unused probe_finish label associated with the old global flag

 .../platform/x86/intel/pmc/ssram_telemetry.c  | 40 ++++++++-----------
 1 file changed, 16 insertions(+), 24 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 6917a10cbc80..597bfb7ad822 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -40,7 +40,6 @@ static const struct ssram_type pci_main = {
 };
 
 static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
-static bool device_probed;
 
 static inline u64 get_base(void __iomem *addr, u32 offset)
 {
@@ -55,8 +54,13 @@ static void pmc_ssram_get_devid_pwrmbase(void __iomem *ssram, unsigned int pmc_i
 	pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
 	devid = readw(ssram + SSRAM_DEVID_OFFSET);
 
-	pmc_ssram_telems[pmc_idx].devid = devid;
 	pmc_ssram_telems[pmc_idx].base_addr = pwrm_base;
+	/*
+	 * Memory barrier is used to ensure the correct write order between base_addr
+	 * and devid.
+	 */
+	smp_wmb();
+	pmc_ssram_telems[pmc_idx].devid = devid;
 }
 
 static int
@@ -154,32 +158,28 @@ static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
  * * 0           - Success
  * * -EAGAIN     - Probe function has not finished yet. Try again.
  * * -EINVAL     - Invalid pmc_idx
- * * -ENODEV     - PMC device is not available
  */
 int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
 				     struct pmc_ssram_telemetry *pmc_ssram_telemetry)
 {
+	if (pmc_idx >= MAX_NUM_PMC)
+		return -EINVAL;
+
 	/*
 	 * PMCs are discovered in probe function. If this function is called before
-	 * probe function complete, the result would be invalid. Use device_probed
-	 * variable to avoid this case. Return -EAGAIN to inform the consumer to call
+	 * probe function complete, the result would be invalid. Use devid to avoid
+	 * this case. Return -EAGAIN to inform the consumer to call
 	 * again later.
 	 */
-	if (!device_probed)
+	if (!pmc_ssram_telems[pmc_idx].devid)
 		return -EAGAIN;
 
+	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
 	/*
 	 * Memory barrier is used to ensure the correct read order between
-	 * device_probed variable and PMC info.
+	 * devid variable and base_addr.
 	 */
 	smp_rmb();
-	if (pmc_idx >= MAX_NUM_PMC)
-		return -EINVAL;
-
-	if (!pmc_ssram_telems[pmc_idx].devid)
-		return -ENODEV;
-
-	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
 	pmc_ssram_telemetry->base_addr = pmc_ssram_telems[pmc_idx].base_addr;
 	return 0;
 }
@@ -194,8 +194,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 	ssram_type = (const struct ssram_type *)id->driver_data;
 	if (!ssram_type) {
 		dev_dbg(&pcidev->dev, "missing driver data\n");
-		ret = -EINVAL;
-		goto probe_finish;
+		return -EINVAL;
 	}
 
 	method = ssram_type->method;
@@ -203,7 +202,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 	ret = pcim_enable_device(pcidev);
 	if (ret) {
 		dev_dbg(&pcidev->dev, "failed to enable PMC SSRAM device\n");
-		goto probe_finish;
+		return ret;
 	}
 
 	if (method == RES_METHOD_PCI)
@@ -211,13 +210,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 	else
 		ret = -EINVAL;
 
-probe_finish:
-	/*
-	 * Memory barrier is used to ensure the correct write order between PMC info
-	 * and device_probed variable.
-	 */
-	smp_wmb();
-	device_probed = true;
 	return ret;
 }
 
-- 
2.43.0

[PATCH v5 14/16] platform/x86/intel/pmc/ssram: Add ACPI discovery scaffolding

[David E. Box]

Prepare the SSRAM telemetry driver for ACPI-based discovery by adding the
common initialization path and selection framework needed for both PCI and
ACPI resource discovery.

At this stage, existing supported devices continue to use the PCI path.
This change lays the groundwork for follow-on patches that wire platform
IDs to the ACPI policy path.

Assisted-by: GitHub-Copilot:claude-opus-4.7
Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 changes:
  - Fix dsd_buf leak by moving the __free(pmc_acpi_free)
    declaration after acpi_evaluate_object() populates buf.pointer,
    and switched pmc_find_telem_guid(buf.pointer) to operate on
    dsd_buf so cleanup releases the actual allocation.
  - Split acpi_handle declaration from ACPI_HANDLE() assignment
    and placed the assignment immediately before the !handle
    check (Ilpo).
  - Reordered local variables in pmc_ssram_telemetry_acpi_init()
    in reverse-xmas-tree order (Ilpo).

V4 - Replaced local raw ACPI discovery pointer type u32 (*)[4] with
     acpi_disc_t in SSRAM ACPI initialization path.

V3 - No changes

V2 changes:
  - Fixed cleanup patterns using __free() attributes
  - Addressed Ilpo's recommendations for safer cleanup.h patterns

 .../platform/x86/intel/pmc/ssram_telemetry.c  | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 597bfb7ad822..ac330f7df649 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2023, Intel Corporation.
  */
 
+#include <linux/acpi.h>
 #include <linux/bits.h>
 #include <linux/cleanup.h>
 #include <linux/device.h>
@@ -29,14 +30,17 @@ DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
 
 enum resource_method {
 	RES_METHOD_PCI,
+	RES_METHOD_ACPI,
 };
 
 struct ssram_type {
 	enum resource_method method;
+	enum pmc_index p_index;
 };
 
 static const struct ssram_type pci_main = {
 	.method = RES_METHOD_PCI,
+	.p_index = PMC_IDX_MAIN,
 };
 
 static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
@@ -149,6 +153,69 @@ static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
 	return ret;
 }
 
+static int pmc_ssram_telemetry_get_pmc_acpi(struct pci_dev *pcidev,  unsigned int pmc_idx)
+{
+	u64 ssram_base;
+
+	ssram_base = pci_resource_start(pcidev, 0);
+	if (!ssram_base)
+		return -ENODEV;
+
+	void __iomem __free(pmc_ssram_telemetry_iounmap) *ssram =
+		ioremap(ssram_base, SSRAM_HDR_SIZE);
+	if (!ssram)
+		return -ENOMEM;
+
+	pmc_ssram_get_devid_pwrmbase(ssram, pmc_idx);
+
+	return 0;
+}
+
+static int pmc_ssram_telemetry_acpi_init(struct pci_dev *pcidev,
+					 enum pmc_index index)
+{
+	struct intel_vsec_header header;
+	struct intel_vsec_header *headers[2] = { &header, NULL };
+	struct acpi_buffer buf = { ACPI_ALLOCATE_BUFFER, NULL };
+	struct intel_vsec_platform_info info = { };
+	union acpi_object *dsd;
+	acpi_handle handle;
+	acpi_status status;
+	int ret;
+
+	handle = ACPI_HANDLE(&pcidev->dev);
+	if (!handle)
+		return -ENODEV;
+
+	status = acpi_evaluate_object(handle, "_DSD", NULL, &buf);
+	if (ACPI_FAILURE(status))
+		return -ENODEV;
+
+	void *dsd_buf __free(pmc_acpi_free) = buf.pointer;
+
+	dsd = pmc_find_telem_guid(dsd_buf);
+	if (!dsd)
+		return -ENODEV;
+
+	acpi_disc_t disc __free(kfree) = pmc_parse_telem_dsd(dsd, &header);
+	if (IS_ERR(disc))
+		return PTR_ERR(disc);
+
+	info.headers = headers;
+	info.caps = VSEC_CAP_TELEMETRY;
+	info.acpi_disc = disc;
+	info.src = INTEL_VSEC_DISC_ACPI;
+
+	/* This is an ACPI companion device. PCI BAR will be used for base addr. */
+	info.base_addr = 0;
+
+	ret = intel_vsec_register(&pcidev->dev, &info);
+	if (ret)
+		return ret;
+
+	return pmc_ssram_telemetry_get_pmc_acpi(pcidev, index);
+}
+
 /**
  * pmc_ssram_telemetry_get_pmc_info() - Get a PMC devid and base_addr information
  * @pmc_idx:               Index of the PMC
@@ -189,6 +256,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 {
 	const struct ssram_type *ssram_type;
 	enum resource_method method;
+	enum pmc_index index;
 	int ret;
 
 	ssram_type = (const struct ssram_type *)id->driver_data;
@@ -198,6 +266,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 	}
 
 	method = ssram_type->method;
+	index = ssram_type->p_index;
 
 	ret = pcim_enable_device(pcidev);
 	if (ret) {
@@ -207,6 +276,8 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
 
 	if (method == RES_METHOD_PCI)
 		ret = pmc_ssram_telemetry_pci_init(pcidev);
+	else if (method == RES_METHOD_ACPI)
+		ret = pmc_ssram_telemetry_acpi_init(pcidev, index);
 	else
 		ret = -EINVAL;
 
@@ -239,6 +310,7 @@ static struct pci_driver pmc_ssram_telemetry_driver = {
 };
 module_pci_driver(pmc_ssram_telemetry_driver);
 
+MODULE_IMPORT_NS("INTEL_PMC_CORE");
 MODULE_IMPORT_NS("INTEL_VSEC");
 MODULE_AUTHOR("Xi Pardee <xi.pardee@intel.com>");
 MODULE_DESCRIPTION("Intel PMC SSRAM Telemetry driver");
-- 
2.43.0

[PATCH v5 15/16] platform/x86/intel/pmc/ssram: Make PMT registration optional

[David E. Box]

The SSRAM telemetry driver extracts essential PMC device ID and power
management base address information that intel_pmc_core depends on for core
functionality. If PMT registration failure prevents this critical data from
being available, intel_pmc_core operation would break entirely. Therefore,
PMT registration failures must not block access to this data.

Change the behavior to log a warning when PMT registration fails but
continue with successful driver initialization, ensuring the primary
telemetry data remains accessible to dependent drivers.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 changes:
  - Dropped the standalone cleanup-pattern patch from this refreshed
    series retaining the simpler ssram pointer flow requested in review.
  - Folded PMT-registration-optional handling onto that simpler flow
    with no intended functional change.

V2 changes:
  - Update commit message for clarity
  - Also apply the PCI telemetry path

 drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index ac330f7df649..7bb699116bd0 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -109,6 +109,7 @@ pmc_ssram_telemetry_get_pmc_pci(struct pci_dev *pcidev, unsigned int pmc_idx, u3
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *tmp_ssram = NULL;
 	void __iomem __free(pmc_ssram_telemetry_iounmap) *ssram = NULL;
 	u64 ssram_base;
+	int ret;
 
 	ssram_base = pci_resource_start(pcidev, 0);
 	tmp_ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
@@ -128,7 +129,6 @@ pmc_ssram_telemetry_get_pmc_pci(struct pci_dev *pcidev, unsigned int pmc_idx, u3
 		ssram = ioremap(ssram_base, SSRAM_HDR_SIZE);
 		if (!ssram)
 			return -ENOMEM;
-
 	} else {
 		ssram = no_free_ptr(tmp_ssram);
 	}
@@ -136,7 +136,11 @@ pmc_ssram_telemetry_get_pmc_pci(struct pci_dev *pcidev, unsigned int pmc_idx, u3
 	pmc_ssram_get_devid_pwrmbase(ssram, pmc_idx);
 
 	/* Find and register and PMC telemetry entries */
-	return pmc_ssram_telemetry_add_pmt(pcidev, ssram_base, ssram);
+	ret = pmc_ssram_telemetry_add_pmt(pcidev, ssram_base, ssram);
+	if (ret)
+		dev_warn(&pcidev->dev, "could not register PMT\n");
+
+	return 0;
 }
 
 static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
@@ -211,7 +215,7 @@ static int pmc_ssram_telemetry_acpi_init(struct pci_dev *pcidev,
 
 	ret = intel_vsec_register(&pcidev->dev, &info);
 	if (ret)
-		return ret;
+		dev_warn(&pcidev->dev, "could not register PMT\n");
 
 	return pmc_ssram_telemetry_get_pmc_acpi(pcidev, index);
 }
-- 
2.43.0

[PATCH v5 16/16] platform/x86/intel/pmc: Add NVL PCI IDs for SSRAM telemetry discovery

[David E. Box]

Add Nova Lake S PMC device IDs to enable binding of the SSRAM telemetry
driver on NVL platforms, and map them to the ACPI-based discovery policy.

Signed-off-by: David E. Box <david.e.box@linux.intel.com>
---
V5 - No changes

V4 - No changes

V3 - No changes

V2 - No changes

 drivers/platform/x86/intel/pmc/core.h            |  5 +++++
 drivers/platform/x86/intel/pmc/ssram_telemetry.c | 16 ++++++++++++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
index f458eb908c07..eb2e1030dbf3 100644
--- a/drivers/platform/x86/intel/pmc/core.h
+++ b/drivers/platform/x86/intel/pmc/core.h
@@ -334,6 +334,11 @@ enum ppfear_regs {
 #define PMC_DEVID_MTL_IOEP	0x7ecf
 #define PMC_DEVID_MTL_IOEM	0x7ebf
 
+/* NVL */
+#define PMC_DEVID_NVL_PCDH	0xd37e
+#define PMC_DEVID_NVL_PCDS	0xd47e
+#define PMC_DEVID_NVL_PCHS	0x6e27
+
 extern const char *pmc_lpm_modes[];
 
 struct pmc_bit_map {
diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
index 7bb699116bd0..3ca577fb0eb8 100644
--- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
+++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
@@ -43,6 +43,16 @@ static const struct ssram_type pci_main = {
 	.p_index = PMC_IDX_MAIN,
 };
 
+static const struct ssram_type acpi_main = {
+	.method = RES_METHOD_ACPI,
+	.p_index = PMC_IDX_MAIN,
+};
+
+static const struct ssram_type acpi_pch = {
+	.method = RES_METHOD_ACPI,
+	.p_index = PMC_IDX_PCH,
+};
+
 static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
 
 static inline u64 get_base(void __iomem *addr, u32 offset)
@@ -303,6 +313,12 @@ static const struct pci_device_id pmc_ssram_telemetry_pci_ids[] = {
 		.driver_data = (kernel_ulong_t)&pci_main },
 	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_WCL_PCDN),
 		.driver_data = (kernel_ulong_t)&pci_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_NVL_PCDH),
+		.driver_data = (kernel_ulong_t)&acpi_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_NVL_PCDS),
+		.driver_data = (kernel_ulong_t)&acpi_main },
+	{ PCI_DEVICE(PCI_VENDOR_ID_INTEL, PMC_DEVID_NVL_PCHS),
+		.driver_data = (kernel_ulong_t)&acpi_pch },
 	{ }
 };
 MODULE_DEVICE_TABLE(pci, pmc_ssram_telemetry_pci_ids);
-- 
2.43.0

Re: [PATCH v5 16/16] platform/x86/intel/pmc: Add NVL PCI IDs for SSRAM telemetry discovery

[Ilpo Järvinen]

On Thu, 21 May 2026, David E. Box wrote:

> Add Nova Lake S PMC device IDs to enable binding of the SSRAM telemetry
> driver on NVL platforms, and map them to the ACPI-based discovery policy.
> 
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V5 - No changes
> 
> V4 - No changes
> 
> V3 - No changes
> 
> V2 - No changes
> 
>  drivers/platform/x86/intel/pmc/core.h            |  5 +++++
>  drivers/platform/x86/intel/pmc/ssram_telemetry.c | 16 ++++++++++++++++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/platform/x86/intel/pmc/core.h b/drivers/platform/x86/intel/pmc/core.h
> index f458eb908c07..eb2e1030dbf3 100644
> --- a/drivers/platform/x86/intel/pmc/core.h
> +++ b/drivers/platform/x86/intel/pmc/core.h
> @@ -334,6 +334,11 @@ enum ppfear_regs {
>  #define PMC_DEVID_MTL_IOEP	0x7ecf
>  #define PMC_DEVID_MTL_IOEM	0x7ebf
>  
> +/* NVL */
> +#define PMC_DEVID_NVL_PCDH	0xd37e
> +#define PMC_DEVID_NVL_PCDS	0xd47e
> +#define PMC_DEVID_NVL_PCHS	0x6e27

These seem to already be in pdx86 repo (from Xi Pardee's change).

-- 
 i.

Re: [PATCH v5 05/16] platform/x86/intel/pmt: Cache the telemetry discovery header

[Ilpo Järvinen]

On Thu, 21 May 2026, David E. Box wrote:

> pmt_telem_header_decode() only needs the discovery header dwords, but it
> currently decodes them by reading directly from entry->disc_table.
> 
> Cache the discovery header in intel_pmt_entry when the device is created
> and have telemetry decode use the cached values instead of performing MMIO
> reads at decode time.
> 
> The DVSEC discovery resource for a namespace is sized by its per-entry
> entry_size (in dwords), which can be less than the 4-dword cache (e.g.
> telemetry uses entry_size = 3, i.e. 12 bytes). Cap the memcpy_fromio()
> to resource_size(disc_res) so the new cache does not read past the
> mapped region. Any unread dwords stay zero from the zero-initialized
> allocation of the containing struct.

sashiko seems to not agree with it being always zero-initialized and 
to me it looks there's some merit to that because of pmt_telem_probe() 
code reusing entry:

        for (i = 0; i < intel_vsec_dev->num_resources; i++) {
                struct intel_pmt_entry *entry = &priv->entry[priv->num_entries];

                mutex_lock(&ep_lock);
                ret = intel_pmt_dev_create(entry, &pmt_telem_ns, intel_vsec_dev, i);
                mutex_unlock(&ep_lock);
                if (ret < 0)
                        goto abort_probe;
                if (ret)
                        continue;

                priv->num_entries++;

With the subsequent change to u32 headers[PMT_DISC_HEADER_DWORDS];
in the next patch, the array will not be in zero-initialized state at 
all AFAICT.

> This keeps the telemetry header decode path independent of how the
> discovery data is backed and avoids baking a direct MMIO assumption into
> the feature-specific decode logic.
> 
> Assisted-by: GitHub-Copilot:claude-opus-4.7
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V5 changes:
>   - Cap memcpy_fromio() of the cached discovery header to
>     resource_size(disc_res) so the newly introduced cache does not
>     over-read namespaces whose DVSEC entry_size is smaller than the
>     cache (e.g. telemetry has entry_size = 3, 12 bytes).
> 
> V4 - No changes
> 
> V3 changes:
>   - New patch split out from PMT header-fetch rework to cache discovery
>     header data before downstream decode/population.
>   - Added to carry the post-v3 bug fix while preserving the original series
>     ordering intent.
> 
>  drivers/platform/x86/intel/pmt/class.c     | 11 +++++++++++
>  drivers/platform/x86/intel/pmt/class.h     |  1 +
>  drivers/platform/x86/intel/pmt/telemetry.c | 12 ++++++------
>  3 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 7da8279b54f8..246e11837800 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -383,6 +383,17 @@ int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespa
>  	if (IS_ERR(entry->disc_table))
>  		return PTR_ERR(entry->disc_table);
>  
> +	/*
> +	 * The mapped discovery resource may be smaller than disc_header (its
> +	 * size is the namespace's DVSEC entry_size in dwords, which can be
> +	 * less than 4). Cap the copy to the actual resource size to avoid
> +	 * reading past the mapped region; any unread dwords stay zero from
> +	 * the zero-initialized allocation of the containing struct.
> +	 */
> +	memcpy_fromio(entry->disc_header, entry->disc_table,
> +		      min_t(size_t, sizeof(entry->disc_header),

You forgot to add include for min_t(). Please add it before module.h (even 
if the includes are not fully in alphabetical order atm).

> +			    resource_size(disc_res)));
> +
>  	if (ns->pmt_pre_decode) {
>  		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
>  		if (ret)
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index 8a0db0ef58c1..84202fc7920c 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -44,6 +44,7 @@ struct intel_pmt_entry {
>  	struct telem_endpoint	*ep;
>  	struct pci_dev		*pcidev;
>  	struct intel_pmt_header	header;
> +	u32			disc_header[4];
>  	struct bin_attribute	pmt_bin_attr;
>  	const struct attribute_group *attr_grp;
>  	struct kobject		*kobj;
> diff --git a/drivers/platform/x86/intel/pmt/telemetry.c b/drivers/platform/x86/intel/pmt/telemetry.c
> index d22f633638be..953f35b6daec 100644
> --- a/drivers/platform/x86/intel/pmt/telemetry.c
> +++ b/drivers/platform/x86/intel/pmt/telemetry.c
> @@ -72,16 +72,16 @@ static bool pmt_telem_region_overlaps(struct device *dev, u32 guid, u32 type)
>  static int pmt_telem_header_decode(struct intel_pmt_entry *entry,
>  				   struct device *dev)
>  {
> -	void __iomem *disc_table = entry->disc_table;
>  	struct intel_pmt_header *header = &entry->header;
> +	u32 *disc_header = entry->disc_header;
>  
> -	header->access_type = TELEM_ACCESS(readl(disc_table));
> -	header->guid = readl(disc_table + TELEM_GUID_OFFSET);
> -	header->base_offset = readl(disc_table + TELEM_BASE_OFFSET);
> +	header->access_type = TELEM_ACCESS(disc_header[0]);
> +	header->guid = disc_header[1];
> +	header->base_offset = disc_header[2];
>  
>  	/* Size is measured in DWORDS, but accessor returns bytes */
> -	header->size = TELEM_SIZE(readl(disc_table));
> -	header->telem_type = TELEM_TYPE(readl(entry->disc_table));
> +	header->size = TELEM_SIZE(disc_header[0]);
> +	header->telem_type = TELEM_TYPE(disc_header[0]);
>  
>  	return 0;
>  }
> 

-- 
 i.

Re: [PATCH v5 06/16] platform/x86/intel/pmt: Unify header fetch and add ACPI source

[Ilpo Järvinen]

On Thu, 21 May 2026, David E. Box wrote:

> Allow the PMT class to read discovery headers from either PCI MMIO or
> ACPI-provided entries, depending on the discovery source. The new
> source-aware fetch helper retrieves the first two QWORDs for both paths
> while keeping the mapped discovery table available for users such as
> crashlog.
> 
> Split intel_pmt_populate_entry() into source-specific resolvers:
>   - pmt_resolve_access_pci(): handles both ACCESS_LOCAL and ACCESS_BARID
>     for PCI-backed devices and sets entry->pcidev. Same existing
>     functionality.
>   - pmt_resolve_access_acpi(): handles only ACCESS_BARID for ACPI-backed
>     devices, rejecting ACCESS_LOCAL which has no valid semantics without
>     a physical discovery resource.
> 
> Also, when copying discovery headers, bind the copy size to the canonical
> discovery header definition instead of relying on separate literals.
> 
> This maintains existing PCI behavior and makes no functional changes
> for PCI devices.
> 
> Assisted-by: GitHub-Copilot:claude-opus-4.7
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V5 changes:
>   - Cap memcpy_fromio() in pmt_get_headers() PCI branch to
>     resource_size() of the discovery resource, matching the cap
>     added in the cache-introducing patch.
>   - Documented in the ACPI branch that disc_table is intentionally
>     NULL on that source, so consumers that dereference disc_table
>     must only be wired to INTEL_VSEC_DISC_PCI namespaces.

Please rebase this on top of the most recent for-next code as it seems to 
have developed a context conflict with a recently applied change.

> V4 changes:
>   - Added discovery header width macro, INTEL_VSEC_ACPI_DISC_DWORDS, in
>     shared intel_vsec header and in definitions.
>   - Aliased to PMT_DISC_HEADER_DWORDS and converted PMT discovery header
>     arrays to use it.
>   - Replaced literal header copy sizes with entry->disc_header size in
>     pmt_get_headers() for PCI and ACPI paths.
> 
> V3 changes:
>   - Folded the header fetch rework back into intel_pmt_dev_create() after
>     dropping the previous common header decode helper patch
>   - Cleaned up line wrapping/indentation
> 
> V2 changes:
>   - In pmt_resolve_access_acpi(), moved dev_err() call to single line
>     instead of split across two lines
>   - Restructured error handling in intel_pmt_populate_entry(), moving error
>     returns from after switch/case into each case statement for better
>     readability
>   - Addressed Ilpo's feedback on error message formatting and error
>     handling patterns
> 
>  drivers/platform/x86/intel/pmt/class.c | 157 +++++++++++++++++++++----
>  drivers/platform/x86/intel/pmt/class.h |   3 +-
>  include/linux/intel_vsec.h             |   5 +-
>  3 files changed, 142 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> index 246e11837800..1a77709edc6a 100644
> --- a/drivers/platform/x86/intel/pmt/class.c
> +++ b/drivers/platform/x86/intel/pmt/class.c
> @@ -204,9 +204,9 @@ struct class intel_pmt_class = {
>  };
>  EXPORT_SYMBOL_GPL(intel_pmt_class);
>  
> -static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> -				    struct intel_vsec_device *ivdev,
> -				    int idx)
> +static int pmt_resolve_access_pci(struct intel_pmt_entry *entry,
> +				  struct intel_vsec_device *ivdev,
> +				  int idx)
>  {
>  	struct pci_dev *pci_dev = to_pci_dev(ivdev->dev);
>  	struct device *dev = &ivdev->auxdev.dev;
> @@ -286,6 +286,81 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
>  	}
>  
>  	entry->pcidev = pci_dev;
> +
> +	return 0;
> +}
> +
> +static int pmt_resolve_access_acpi(struct intel_pmt_entry *entry,
> +				   struct intel_vsec_device *ivdev)
> +{
> +	struct pci_dev *pci_dev = NULL;
> +	struct device *dev = &ivdev->auxdev.dev;
> +	struct intel_pmt_header *header = &entry->header;
> +	u8 bir;
> +
> +	if (dev_is_pci(ivdev->dev))
> +		pci_dev = to_pci_dev(ivdev->dev);
> +
> +	/*
> +	 * The base offset should always be 8 byte aligned.
> +	 *
> +	 * For non-local access types the lower 3 bits of base offset
> +	 * contains the index of the base address register where the
> +	 * telemetry can be found.
> +	 */
> +	bir = GET_BIR(header->base_offset);
> +
> +	switch (header->access_type) {
> +	case ACCESS_BARID:
> +		/* ACPI platform drivers use base_addr */
> +		if (ivdev->base_addr) {
> +			entry->base_addr = ivdev->base_addr +
> +					   GET_ADDRESS(header->base_offset);
> +			break;
> +		}
> +
> +		/* If base_addr is not provided, then this is an ACPI companion device */
> +		if (!pci_dev) {
> +			dev_err(dev, "ACCESS_BARID requires PCI BAR resources or base_addr\n");
> +			return -EINVAL;
> +		}
> +
> +		entry->base_addr = pci_resource_start(pci_dev, bir) +
> +			GET_ADDRESS(header->base_offset);
> +		break;
> +	default:
> +		dev_err(dev, "Unsupported access type %d for ACPI based PMT\n",
> +			header->access_type);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> +				    struct intel_vsec_device *ivdev,
> +				    int idx)
> +{
> +	struct intel_pmt_header *header = &entry->header;
> +	struct device *dev = &ivdev->auxdev.dev;
> +	int ret;
> +
> +	switch (ivdev->src) {
> +	case INTEL_VSEC_DISC_PCI:
> +		ret = pmt_resolve_access_pci(entry, ivdev, idx);
> +		if (ret)
> +			return ret;
> +		break;
> +	case INTEL_VSEC_DISC_ACPI:
> +		ret = pmt_resolve_access_acpi(entry, ivdev);
> +		if (ret)
> +			return ret;
> +		break;
> +	default:
> +		dev_err(dev, "Unknown discovery source: %d\n", ivdev->src);
> +		return -EINVAL;
> +	}
> +
>  	entry->guid = header->guid;
>  	entry->size = header->size;
>  	entry->cb = ivdev->priv_data;
> @@ -370,29 +445,71 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
>  	return ret;
>  }
>  
> +static int pmt_get_headers(struct intel_vsec_device *ivdev, int idx,
> +			   struct intel_pmt_entry *entry,
> +			   u32 headers[PMT_DISC_HEADER_DWORDS])
> +{
> +	struct device *dev = &ivdev->auxdev.dev;
> +	size_t header_bytes = sizeof(entry->disc_header);
> +
> +	switch (ivdev->src) {
> +	case INTEL_VSEC_DISC_PCI: {
> +		struct resource *disc_res = &ivdev->resource[idx];
> +		void __iomem *disc_table;
> +
> +		disc_table = devm_ioremap_resource(dev, disc_res);
> +		if (IS_ERR(disc_table))
> +			return PTR_ERR(disc_table);
> +
> +		/*
> +		 * The mapped resource is sized by the namespace's DVSEC
> +		 * entry_size (in dwords), which can be less than
> +		 * PMT_DISC_HEADER_DWORDS (e.g. telemetry uses entry_size = 3,
> +		 * 12 bytes). Cap the copy to resource_size() to avoid reading
> +		 * past the mapped region; any unread dwords stay zero from
> +		 * the zero-initialized allocation of the containing struct.
> +		 */
> +		memcpy_fromio(headers, disc_table,
> +			      min_t(size_t, header_bytes,
> +				    resource_size(disc_res)));
> +		memcpy(entry->disc_header, headers, header_bytes);
> +
> +		/* Used by crashlog driver */
> +		entry->disc_table = disc_table;
> +
> +		return 0;
> +	}
> +	case INTEL_VSEC_DISC_ACPI: {
> +		memcpy(headers, &ivdev->acpi_disc[idx][0], header_bytes);
> +		memcpy(entry->disc_header, headers, header_bytes);
> +		/*
> +		 * No MMIO mapping exists on the ACPI source path; the cached
> +		 * headers are the only view of the discovery record. Consumers
> +		 * that dereference disc_table (e.g. crashlog) must therefore
> +		 * only be wired to namespaces backed by INTEL_VSEC_DISC_PCI.
> +		 */
> +		entry->disc_table = NULL;
> +
> +		return 0;
> +	}
> +	default:
> +		dev_err(dev, "Unknown discovery source type: %d\n", ivdev->src);
> +		break;
> +	}
> +
> +	return -EINVAL;
> +}
> +
>  int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespace *ns,
>  			 struct intel_vsec_device *intel_vsec_dev, int idx)
>  {
>  	struct device *dev = &intel_vsec_dev->auxdev.dev;
> -	struct resource	*disc_res;
> +	u32 headers[PMT_DISC_HEADER_DWORDS];
>  	int ret;
>  
> -	disc_res = &intel_vsec_dev->resource[idx];
> -
> -	entry->disc_table = devm_ioremap_resource(dev, disc_res);
> -	if (IS_ERR(entry->disc_table))
> -		return PTR_ERR(entry->disc_table);
> -
> -	/*
> -	 * The mapped discovery resource may be smaller than disc_header (its
> -	 * size is the namespace's DVSEC entry_size in dwords, which can be
> -	 * less than 4). Cap the copy to the actual resource size to avoid
> -	 * reading past the mapped region; any unread dwords stay zero from
> -	 * the zero-initialized allocation of the containing struct.
> -	 */
> -	memcpy_fromio(entry->disc_header, entry->disc_table,
> -		      min_t(size_t, sizeof(entry->disc_header),
> -			    resource_size(disc_res)));
> +	ret = pmt_get_headers(intel_vsec_dev, idx, entry, headers);

Why do you pass headers to it as this function seems to not need them?

(This could be a leftover from the common decoding approach which is no 
longer pursued, I don't remember how it worked here).

Please also note the sashiko's uninitialized headers comment (which 
changes assumptions made in patch 5) which I also mentioned in patch 5 
comments. But it looks to me pmt_get_headers() could copy directly to 
->disc_header and avoid that problem altogether (the entry reuse still 
looks a problem though which can leave pseudogarbage into the tail of 
->disc_header).

-- 
 i.

> +	if (ret)
> +		return ret;
>  
>  	if (ns->pmt_pre_decode) {
>  		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
> diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> index 84202fc7920c..950fa4ee300d 100644
> --- a/drivers/platform/x86/intel/pmt/class.h
> +++ b/drivers/platform/x86/intel/pmt/class.h
> @@ -18,6 +18,7 @@
>  /* PMT discovery base address/offset register layout */
>  #define GET_BIR(v)		((v) & GENMASK(2, 0))
>  #define GET_ADDRESS(v)		((v) & GENMASK(31, 3))
> +#define PMT_DISC_HEADER_DWORDS INTEL_VSEC_ACPI_DISC_DWORDS
>  
>  struct device;
>  struct pci_dev;
> @@ -44,7 +45,7 @@ struct intel_pmt_entry {
>  	struct telem_endpoint	*ep;
>  	struct pci_dev		*pcidev;
>  	struct intel_pmt_header	header;
> -	u32			disc_header[4];
> +	u32			disc_header[PMT_DISC_HEADER_DWORDS];
>  	struct bin_attribute	pmt_bin_attr;
>  	const struct attribute_group *attr_grp;
>  	struct kobject		*kobj;
> diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> index 1fe5665a9d02..4c58a7f5031e 100644
> --- a/include/linux/intel_vsec.h
> +++ b/include/linux/intel_vsec.h
> @@ -28,6 +28,7 @@
>  #define INTEL_DVSEC_TABLE_BAR(x)	((x) & GENMASK(2, 0))
>  #define INTEL_DVSEC_TABLE_OFFSET(x)	((x) & GENMASK(31, 3))
>  #define TABLE_OFFSET_SHIFT		3
> +#define INTEL_VSEC_ACPI_DISC_DWORDS	4
>  
>  struct device;
>  struct pci_dev;
> @@ -122,7 +123,7 @@ struct intel_vsec_platform_info {
>  	struct device *parent;
>  	struct intel_vsec_header **headers;
>  	const struct vsec_feature_dependency *deps;
> -	u32 (*acpi_disc)[4];
> +	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
>  	enum intel_vsec_disc_source src;
>  	void *priv_data;
>  	unsigned long caps;
> @@ -154,7 +155,7 @@ struct intel_vsec_device {
>  	struct auxiliary_device auxdev;
>  	struct device *dev;
>  	struct resource *resource;
> -	u32 (*acpi_disc)[4];
> +	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
>  	enum intel_vsec_disc_source src;
>  	struct ida *ida;
>  	int num_resources;
> 

Re: [PATCH v5 10/16] platform/x86/intel/pmc/ssram: Use fixed-size static pmc array

[Ilpo Järvinen]

[-- Attachment #1: Type: text/plain, Size: 2715 bytes --]

On Thu, 21 May 2026, David E. Box wrote:

> From: Xi Pardee <xi.pardee@linux.intel.com>
> 
> Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
> static array, eliminating per-probe allocation overhead and simplifying
> lifetime management.
> 
> Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
> check devid availability and tighten input bounds checking. Drop
> null-pointer checks now that the storage is static.
> 
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> ---

Sashiko has noted some corner case issues related probe/init paths and 
missing cleanup which may have some merit, especially if this series makes 
the issue worse (for patches 10-14 but all those seem to fundamendally 
relate to same reprobe corner case and leaks of stale setups through 
static vars).

It has some false positives too, such as not realizing smp_*mb() implies 
a compiler barrier so those READ/WRITE_ONCE() requests look bogus to me.

-- 
 i.

> V5 - No changes
> 
> V4 - No changes
> 
> V3 - No changes
> 
> V2 changes:
>   - Replaced hardcoded array size [3] with MAX_NUM_PMC constant
> 
>  drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 ++--------
>  1 file changed, 2 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> index 1deb4d71da3f..4bfe60ee55ca 100644
> --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> @@ -24,7 +24,7 @@
>  
>  DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
>  
> -static struct pmc_ssram_telemetry *pmc_ssram_telems;
> +static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
>  static bool device_probed;
>  
>  static int
> @@ -140,7 +140,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
>  	if (pmc_idx >= MAX_NUM_PMC)
>  		return -EINVAL;
>  
> -	if (!pmc_ssram_telems || !pmc_ssram_telems[pmc_idx].devid)
> +	if (!pmc_ssram_telems[pmc_idx].devid)
>  		return -ENODEV;
>  
>  	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
> @@ -153,12 +153,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  {
>  	int ret;
>  
> -	pmc_ssram_telems = devm_kzalloc(&pcidev->dev, sizeof(*pmc_ssram_telems) * MAX_NUM_PMC,
> -					GFP_KERNEL);
> -	if (!pmc_ssram_telems) {
> -		ret = -ENOMEM;
> -		goto probe_finish;
> -	}
>  
>  	ret = pcim_enable_device(pcidev);
>  	if (ret) {
> 

Re: [PATCH v5 10/16] platform/x86/intel/pmc/ssram: Use fixed-size static pmc array

[David Box]

On Fri, May 22, 2026 at 01:37:07PM +0300, Ilpo Järvinen wrote:
> On Thu, 21 May 2026, David E. Box wrote:
> 
> > From: Xi Pardee <xi.pardee@linux.intel.com>
> > 
> > Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
> > static array, eliminating per-probe allocation overhead and simplifying
> > lifetime management.
> > 
> > Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
> > check devid availability and tighten input bounds checking. Drop
> > null-pointer checks now that the storage is static.
> > 
> > Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> 
> Sashiko has noted some corner case issues related probe/init paths and 
> missing cleanup which may have some merit, especially if this series makes 
> the issue worse (for patches 10-14 but all those seem to fundamendally 
> relate to same reprobe corner case and leaks of stale setups through 
> static vars).
> 
> It has some false positives too, such as not realizing smp_*mb() implies 
> a compiler barrier so those READ/WRITE_ONCE() requests look bogus to me.

Okay thanks. This will be the first time I try it myself.

David

> 
> -- 
>  i.
> 
> > V5 - No changes
> > 
> > V4 - No changes
> > 
> > V3 - No changes
> > 
> > V2 changes:
> >   - Replaced hardcoded array size [3] with MAX_NUM_PMC constant
> > 
> >  drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 ++--------
> >  1 file changed, 2 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > index 1deb4d71da3f..4bfe60ee55ca 100644
> > --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > @@ -24,7 +24,7 @@
> >  
> >  DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
> >  
> > -static struct pmc_ssram_telemetry *pmc_ssram_telems;
> > +static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
> >  static bool device_probed;
> >  
> >  static int
> > @@ -140,7 +140,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
> >  	if (pmc_idx >= MAX_NUM_PMC)
> >  		return -EINVAL;
> >  
> > -	if (!pmc_ssram_telems || !pmc_ssram_telems[pmc_idx].devid)
> > +	if (!pmc_ssram_telems[pmc_idx].devid)
> >  		return -ENODEV;
> >  
> >  	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
> > @@ -153,12 +153,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
> >  {
> >  	int ret;
> >  
> > -	pmc_ssram_telems = devm_kzalloc(&pcidev->dev, sizeof(*pmc_ssram_telems) * MAX_NUM_PMC,
> > -					GFP_KERNEL);
> > -	if (!pmc_ssram_telems) {
> > -		ret = -ENOMEM;
> > -		goto probe_finish;
> > -	}
> >  
> >  	ret = pcim_enable_device(pcidev);
> >  	if (ret) {
> > 

Re: [PATCH v5 06/16] platform/x86/intel/pmt: Unify header fetch and add ACPI source

[David Box]

On Fri, May 22, 2026 at 01:21:29PM +0300, Ilpo Järvinen wrote:
> On Thu, 21 May 2026, David E. Box wrote:
> 
> > Allow the PMT class to read discovery headers from either PCI MMIO or
> > ACPI-provided entries, depending on the discovery source. The new
> > source-aware fetch helper retrieves the first two QWORDs for both paths
> > while keeping the mapped discovery table available for users such as
> > crashlog.
> > 
> > Split intel_pmt_populate_entry() into source-specific resolvers:
> >   - pmt_resolve_access_pci(): handles both ACCESS_LOCAL and ACCESS_BARID
> >     for PCI-backed devices and sets entry->pcidev. Same existing
> >     functionality.
> >   - pmt_resolve_access_acpi(): handles only ACCESS_BARID for ACPI-backed
> >     devices, rejecting ACCESS_LOCAL which has no valid semantics without
> >     a physical discovery resource.
> > 
> > Also, when copying discovery headers, bind the copy size to the canonical
> > discovery header definition instead of relying on separate literals.
> > 
> > This maintains existing PCI behavior and makes no functional changes
> > for PCI devices.
> > 
> > Assisted-by: GitHub-Copilot:claude-opus-4.7
> > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > ---
> > V5 changes:
> >   - Cap memcpy_fromio() in pmt_get_headers() PCI branch to
> >     resource_size() of the discovery resource, matching the cap
> >     added in the cache-introducing patch.
> >   - Documented in the ACPI branch that disc_table is intentionally
> >     NULL on that source, so consumers that dereference disc_table
> >     must only be wired to INTEL_VSEC_DISC_PCI namespaces.
> 
> Please rebase this on top of the most recent for-next code as it seems to 
> have developed a context conflict with a recently applied change.
> 
> > V4 changes:
> >   - Added discovery header width macro, INTEL_VSEC_ACPI_DISC_DWORDS, in
> >     shared intel_vsec header and in definitions.
> >   - Aliased to PMT_DISC_HEADER_DWORDS and converted PMT discovery header
> >     arrays to use it.
> >   - Replaced literal header copy sizes with entry->disc_header size in
> >     pmt_get_headers() for PCI and ACPI paths.
> > 
> > V3 changes:
> >   - Folded the header fetch rework back into intel_pmt_dev_create() after
> >     dropping the previous common header decode helper patch
> >   - Cleaned up line wrapping/indentation
> > 
> > V2 changes:
> >   - In pmt_resolve_access_acpi(), moved dev_err() call to single line
> >     instead of split across two lines
> >   - Restructured error handling in intel_pmt_populate_entry(), moving error
> >     returns from after switch/case into each case statement for better
> >     readability
> >   - Addressed Ilpo's feedback on error message formatting and error
> >     handling patterns
> > 
> >  drivers/platform/x86/intel/pmt/class.c | 157 +++++++++++++++++++++----
> >  drivers/platform/x86/intel/pmt/class.h |   3 +-
> >  include/linux/intel_vsec.h             |   5 +-
> >  3 files changed, 142 insertions(+), 23 deletions(-)
> > 
> > diff --git a/drivers/platform/x86/intel/pmt/class.c b/drivers/platform/x86/intel/pmt/class.c
> > index 246e11837800..1a77709edc6a 100644
> > --- a/drivers/platform/x86/intel/pmt/class.c
> > +++ b/drivers/platform/x86/intel/pmt/class.c
> > @@ -204,9 +204,9 @@ struct class intel_pmt_class = {
> >  };
> >  EXPORT_SYMBOL_GPL(intel_pmt_class);
> >  
> > -static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> > -				    struct intel_vsec_device *ivdev,
> > -				    int idx)
> > +static int pmt_resolve_access_pci(struct intel_pmt_entry *entry,
> > +				  struct intel_vsec_device *ivdev,
> > +				  int idx)
> >  {
> >  	struct pci_dev *pci_dev = to_pci_dev(ivdev->dev);
> >  	struct device *dev = &ivdev->auxdev.dev;
> > @@ -286,6 +286,81 @@ static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> >  	}
> >  
> >  	entry->pcidev = pci_dev;
> > +
> > +	return 0;
> > +}
> > +
> > +static int pmt_resolve_access_acpi(struct intel_pmt_entry *entry,
> > +				   struct intel_vsec_device *ivdev)
> > +{
> > +	struct pci_dev *pci_dev = NULL;
> > +	struct device *dev = &ivdev->auxdev.dev;
> > +	struct intel_pmt_header *header = &entry->header;
> > +	u8 bir;
> > +
> > +	if (dev_is_pci(ivdev->dev))
> > +		pci_dev = to_pci_dev(ivdev->dev);
> > +
> > +	/*
> > +	 * The base offset should always be 8 byte aligned.
> > +	 *
> > +	 * For non-local access types the lower 3 bits of base offset
> > +	 * contains the index of the base address register where the
> > +	 * telemetry can be found.
> > +	 */
> > +	bir = GET_BIR(header->base_offset);
> > +
> > +	switch (header->access_type) {
> > +	case ACCESS_BARID:
> > +		/* ACPI platform drivers use base_addr */
> > +		if (ivdev->base_addr) {
> > +			entry->base_addr = ivdev->base_addr +
> > +					   GET_ADDRESS(header->base_offset);
> > +			break;
> > +		}
> > +
> > +		/* If base_addr is not provided, then this is an ACPI companion device */
> > +		if (!pci_dev) {
> > +			dev_err(dev, "ACCESS_BARID requires PCI BAR resources or base_addr\n");
> > +			return -EINVAL;
> > +		}
> > +
> > +		entry->base_addr = pci_resource_start(pci_dev, bir) +
> > +			GET_ADDRESS(header->base_offset);
> > +		break;
> > +	default:
> > +		dev_err(dev, "Unsupported access type %d for ACPI based PMT\n",
> > +			header->access_type);
> > +		return -EINVAL;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int intel_pmt_populate_entry(struct intel_pmt_entry *entry,
> > +				    struct intel_vsec_device *ivdev,
> > +				    int idx)
> > +{
> > +	struct intel_pmt_header *header = &entry->header;
> > +	struct device *dev = &ivdev->auxdev.dev;
> > +	int ret;
> > +
> > +	switch (ivdev->src) {
> > +	case INTEL_VSEC_DISC_PCI:
> > +		ret = pmt_resolve_access_pci(entry, ivdev, idx);
> > +		if (ret)
> > +			return ret;
> > +		break;
> > +	case INTEL_VSEC_DISC_ACPI:
> > +		ret = pmt_resolve_access_acpi(entry, ivdev);
> > +		if (ret)
> > +			return ret;
> > +		break;
> > +	default:
> > +		dev_err(dev, "Unknown discovery source: %d\n", ivdev->src);
> > +		return -EINVAL;
> > +	}
> > +
> >  	entry->guid = header->guid;
> >  	entry->size = header->size;
> >  	entry->cb = ivdev->priv_data;
> > @@ -370,29 +445,71 @@ static int intel_pmt_dev_register(struct intel_pmt_entry *entry,
> >  	return ret;
> >  }
> >  
> > +static int pmt_get_headers(struct intel_vsec_device *ivdev, int idx,
> > +			   struct intel_pmt_entry *entry,
> > +			   u32 headers[PMT_DISC_HEADER_DWORDS])
> > +{
> > +	struct device *dev = &ivdev->auxdev.dev;
> > +	size_t header_bytes = sizeof(entry->disc_header);
> > +
> > +	switch (ivdev->src) {
> > +	case INTEL_VSEC_DISC_PCI: {
> > +		struct resource *disc_res = &ivdev->resource[idx];
> > +		void __iomem *disc_table;
> > +
> > +		disc_table = devm_ioremap_resource(dev, disc_res);
> > +		if (IS_ERR(disc_table))
> > +			return PTR_ERR(disc_table);
> > +
> > +		/*
> > +		 * The mapped resource is sized by the namespace's DVSEC
> > +		 * entry_size (in dwords), which can be less than
> > +		 * PMT_DISC_HEADER_DWORDS (e.g. telemetry uses entry_size = 3,
> > +		 * 12 bytes). Cap the copy to resource_size() to avoid reading
> > +		 * past the mapped region; any unread dwords stay zero from
> > +		 * the zero-initialized allocation of the containing struct.
> > +		 */
> > +		memcpy_fromio(headers, disc_table,
> > +			      min_t(size_t, header_bytes,
> > +				    resource_size(disc_res)));
> > +		memcpy(entry->disc_header, headers, header_bytes);
> > +
> > +		/* Used by crashlog driver */
> > +		entry->disc_table = disc_table;
> > +
> > +		return 0;
> > +	}
> > +	case INTEL_VSEC_DISC_ACPI: {
> > +		memcpy(headers, &ivdev->acpi_disc[idx][0], header_bytes);
> > +		memcpy(entry->disc_header, headers, header_bytes);
> > +		/*
> > +		 * No MMIO mapping exists on the ACPI source path; the cached
> > +		 * headers are the only view of the discovery record. Consumers
> > +		 * that dereference disc_table (e.g. crashlog) must therefore
> > +		 * only be wired to namespaces backed by INTEL_VSEC_DISC_PCI.
> > +		 */
> > +		entry->disc_table = NULL;
> > +
> > +		return 0;
> > +	}
> > +	default:
> > +		dev_err(dev, "Unknown discovery source type: %d\n", ivdev->src);
> > +		break;
> > +	}
> > +
> > +	return -EINVAL;
> > +}
> > +
> >  int intel_pmt_dev_create(struct intel_pmt_entry *entry, struct intel_pmt_namespace *ns,
> >  			 struct intel_vsec_device *intel_vsec_dev, int idx)
> >  {
> >  	struct device *dev = &intel_vsec_dev->auxdev.dev;
> > -	struct resource	*disc_res;
> > +	u32 headers[PMT_DISC_HEADER_DWORDS];
> >  	int ret;
> >  
> > -	disc_res = &intel_vsec_dev->resource[idx];
> > -
> > -	entry->disc_table = devm_ioremap_resource(dev, disc_res);
> > -	if (IS_ERR(entry->disc_table))
> > -		return PTR_ERR(entry->disc_table);
> > -
> > -	/*
> > -	 * The mapped discovery resource may be smaller than disc_header (its
> > -	 * size is the namespace's DVSEC entry_size in dwords, which can be
> > -	 * less than 4). Cap the copy to the actual resource size to avoid
> > -	 * reading past the mapped region; any unread dwords stay zero from
> > -	 * the zero-initialized allocation of the containing struct.
> > -	 */
> > -	memcpy_fromio(entry->disc_header, entry->disc_table,
> > -		      min_t(size_t, sizeof(entry->disc_header),
> > -			    resource_size(disc_res)));
> > +	ret = pmt_get_headers(intel_vsec_dev, idx, entry, headers);
> 
> Why do you pass headers to it as this function seems to not need them?
> 
> (This could be a leftover from the common decoding approach which is no 
> longer pursued, I don't remember how it worked here).
> 
> Please also note the sashiko's uninitialized headers comment (which 
> changes assumptions made in patch 5) which I also mentioned in patch 5 
> comments. But it looks to me pmt_get_headers() could copy directly to 
> ->disc_header and avoid that problem altogether (the entry reuse still 
> looks a problem though which can leave pseudogarbage into the tail of 
> ->disc_header).

Too many concurrent projects and AI reliance. I dropped the unneeded header
and added a zero memset to just avoid any dependency on the caller doing a
zero init. Thanks.

David

> 
> -- 
>  i.
> 
> > +	if (ret)
> > +		return ret;
> >  
> >  	if (ns->pmt_pre_decode) {
> >  		ret = ns->pmt_pre_decode(intel_vsec_dev, entry);
> > diff --git a/drivers/platform/x86/intel/pmt/class.h b/drivers/platform/x86/intel/pmt/class.h
> > index 84202fc7920c..950fa4ee300d 100644
> > --- a/drivers/platform/x86/intel/pmt/class.h
> > +++ b/drivers/platform/x86/intel/pmt/class.h
> > @@ -18,6 +18,7 @@
> >  /* PMT discovery base address/offset register layout */
> >  #define GET_BIR(v)		((v) & GENMASK(2, 0))
> >  #define GET_ADDRESS(v)		((v) & GENMASK(31, 3))
> > +#define PMT_DISC_HEADER_DWORDS INTEL_VSEC_ACPI_DISC_DWORDS
> >  
> >  struct device;
> >  struct pci_dev;
> > @@ -44,7 +45,7 @@ struct intel_pmt_entry {
> >  	struct telem_endpoint	*ep;
> >  	struct pci_dev		*pcidev;
> >  	struct intel_pmt_header	header;
> > -	u32			disc_header[4];
> > +	u32			disc_header[PMT_DISC_HEADER_DWORDS];
> >  	struct bin_attribute	pmt_bin_attr;
> >  	const struct attribute_group *attr_grp;
> >  	struct kobject		*kobj;
> > diff --git a/include/linux/intel_vsec.h b/include/linux/intel_vsec.h
> > index 1fe5665a9d02..4c58a7f5031e 100644
> > --- a/include/linux/intel_vsec.h
> > +++ b/include/linux/intel_vsec.h
> > @@ -28,6 +28,7 @@
> >  #define INTEL_DVSEC_TABLE_BAR(x)	((x) & GENMASK(2, 0))
> >  #define INTEL_DVSEC_TABLE_OFFSET(x)	((x) & GENMASK(31, 3))
> >  #define TABLE_OFFSET_SHIFT		3
> > +#define INTEL_VSEC_ACPI_DISC_DWORDS	4
> >  
> >  struct device;
> >  struct pci_dev;
> > @@ -122,7 +123,7 @@ struct intel_vsec_platform_info {
> >  	struct device *parent;
> >  	struct intel_vsec_header **headers;
> >  	const struct vsec_feature_dependency *deps;
> > -	u32 (*acpi_disc)[4];
> > +	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
> >  	enum intel_vsec_disc_source src;
> >  	void *priv_data;
> >  	unsigned long caps;
> > @@ -154,7 +155,7 @@ struct intel_vsec_device {
> >  	struct auxiliary_device auxdev;
> >  	struct device *dev;
> >  	struct resource *resource;
> > -	u32 (*acpi_disc)[4];
> > +	u32 (*acpi_disc)[INTEL_VSEC_ACPI_DISC_DWORDS];
> >  	enum intel_vsec_disc_source src;
> >  	struct ida *ida;
> >  	int num_resources;
> > 

Re: [PATCH v5 10/16] platform/x86/intel/pmc/ssram: Use fixed-size static pmc array

[David Box]

On Fri, May 22, 2026 at 12:49:43PM -0700, David Box wrote:
> On Fri, May 22, 2026 at 01:37:07PM +0300, Ilpo Järvinen wrote:
> > On Thu, 21 May 2026, David E. Box wrote:
> > 
> > > From: Xi Pardee <xi.pardee@linux.intel.com>
> > > 
> > > Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
> > > static array, eliminating per-probe allocation overhead and simplifying
> > > lifetime management.
> > > 
> > > Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
> > > check devid availability and tighten input bounds checking. Drop
> > > null-pointer checks now that the storage is static.
> > > 
> > > Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> > > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > ---
> > 
> > Sashiko has noted some corner case issues related probe/init paths and 
> > missing cleanup which may have some merit, especially if this series makes 
> > the issue worse (for patches 10-14 but all those seem to fundamendally 
> > relate to same reprobe corner case and leaks of stale setups through 
> > static vars).
> > 
> > It has some false positives too, such as not realizing smp_*mb() implies 
> > a compiler barrier so those READ/WRITE_ONCE() requests look bogus to me.
> 
> Okay thanks. This will be the first time I try it myself.
> 

I couldn't get Sashiko to rerun with my Claude setup, but I did manually use
Claude to find what I think is the same issue.

On Fri, May 22, 2026 at 12:49:43PM -0700, David Box wrote:
> On Fri, May 22, 2026 at 01:37:07PM +0300, Ilpo Järvinen wrote:
> > On Thu, 21 May 2026, David E. Box wrote:
> > 
> > > From: Xi Pardee <xi.pardee@linux.intel.com>
> > > 
> > > Switch pmc_ssram_telems from a devm-allocated pointer to a fixed-size
> > > static array, eliminating per-probe allocation overhead and simplifying
> > > lifetime management.
> > > 
> > > Correspondingly simplify pmc_ssram_telemetry_get_pmc_info() validation to
> > > check devid availability and tighten input bounds checking. Drop
> > > null-pointer checks now that the storage is static.
> > > 
> > > Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> > > Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> > > Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > > ---
> > 
> > Sashiko has noted some corner case issues related probe/init paths and 
> > missing cleanup which may have some merit, especially if this series makes 
> > the issue worse (for patches 10-14 but all those seem to fundamendally 
> > relate to same reprobe corner case and leaks of stale setups through 
> > static vars).
> > 
> > It has some false positives too, such as not realizing smp_*mb() implies 
> > a compiler barrier so those READ/WRITE_ONCE() requests look bogus to me.
> 
> Okay thanks. This will be the first time I try it myself.
> 

I couldn't get Sashiko to rerun with my Claude setup, but I did manually use
Claude to find what I think is the same issue.

First, I realize the commit message doesn't capture why we eliminated the devm
allocation. Up to this point the driver probes all PMCs from a single PCI
device probe, but the following patches move toward probing once per device.
That means a per-device allocated PMC tracker no longer fits the later model.
I'll update the commit message with that information.

The bug Claude found in this patch is that the original devm allocation made
pmc_ssram_telems freshly zeroed on every probe attempt, while switching to a
static array means it's only zeroed at module load time.  If we fail and have to
reprobe the data is stale. I fixed this temporarily by zeroing the array at the
top of probe. It will be removed in the next patch when we start probing per
device.

David

> 
> > 
> > -- 
> >  i.
> > 
> > > V5 - No changes
> > > 
> > > V4 - No changes
> > > 
> > > V3 - No changes
> > > 
> > > V2 changes:
> > >   - Replaced hardcoded array size [3] with MAX_NUM_PMC constant
> > > 
> > >  drivers/platform/x86/intel/pmc/ssram_telemetry.c | 10 ++--------
> > >  1 file changed, 2 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > > index 1deb4d71da3f..4bfe60ee55ca 100644
> > > --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > > +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> > > @@ -24,7 +24,7 @@
> > >  
> > >  DEFINE_FREE(pmc_ssram_telemetry_iounmap, void __iomem *, if (_T) iounmap(_T))
> > >  
> > > -static struct pmc_ssram_telemetry *pmc_ssram_telems;
> > > +static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
> > >  static bool device_probed;
> > >  
> > >  static int
> > > @@ -140,7 +140,7 @@ int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
> > >  	if (pmc_idx >= MAX_NUM_PMC)
> > >  		return -EINVAL;
> > >  
> > > -	if (!pmc_ssram_telems || !pmc_ssram_telems[pmc_idx].devid)
> > > +	if (!pmc_ssram_telems[pmc_idx].devid)
> > >  		return -ENODEV;
> > >  
> > >  	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
> > > @@ -153,12 +153,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
> > >  {
> > >  	int ret;
> > >  
> > > -	pmc_ssram_telems = devm_kzalloc(&pcidev->dev, sizeof(*pmc_ssram_telems) * MAX_NUM_PMC,
> > > -					GFP_KERNEL);
> > > -	if (!pmc_ssram_telems) {
> > > -		ret = -ENOMEM;
> > > -		goto probe_finish;
> > > -	}
> > >  
> > >  	ret = pcim_enable_device(pcidev);
> > >  	if (ret) {
> > > 
> 
> 

Re: [PATCH v5 13/16] platform/x86/intel/pmc/ssram: Refactor memory barrier for reentrant probe

[David Box]

Hi Ilpo,

On Thu, May 21, 2026 at 07:21:43PM -0700, David E. Box wrote:
> From: Xi Pardee <xi.pardee@linux.intel.com>
> 
> Previously, a single global 'device_probed' flag with memory barriers was
> used to prevent callers from reading PMC info before probe completion.  The
> write barrier in probe ensured all data, devid and base_addr, was visible
> before signaling completion, and the read barrier in callers ensured they
> checked the flag before reading data.
> 
> A following commit will make probe reentrant, requiring that a different
> synchronization flag be used since a single global flag cannot coordinate
> multiple concurrent probes.
> 
> Switch to per-index devid publication. Each probe instance writes base_addr
> first, then a write barrier ensures visibility before devid is written as
> the completion signal. Callers check devid first, then use a read barrier
> before reading base_addr. This per-index approach allows multiple probes to
> work independently while maintaining the same memory ordering guarantees.
> 
> Signed-off-by: Xi Pardee <xi.pardee@linux.intel.com>
> Signed-off-by: David E. Box <david.e.box@linux.intel.com>
> ---
> V5 - No changes
> 
> V4 - No changes
> 
> V3 - No changes
> 
> V2 changes:
>   - Expanded commit message to explain synchronization rationale
>   - Remove unused probe_finish label associated with the old global flag
> 
>  .../platform/x86/intel/pmc/ssram_telemetry.c  | 40 ++++++++-----------
>  1 file changed, 16 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/platform/x86/intel/pmc/ssram_telemetry.c b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> index 6917a10cbc80..597bfb7ad822 100644
> --- a/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> +++ b/drivers/platform/x86/intel/pmc/ssram_telemetry.c
> @@ -40,7 +40,6 @@ static const struct ssram_type pci_main = {
>  };
>  
>  static struct pmc_ssram_telemetry pmc_ssram_telems[MAX_NUM_PMC];
> -static bool device_probed;
>  
>  static inline u64 get_base(void __iomem *addr, u32 offset)
>  {
> @@ -55,8 +54,13 @@ static void pmc_ssram_get_devid_pwrmbase(void __iomem *ssram, unsigned int pmc_i
>  	pwrm_base = get_base(ssram, SSRAM_PWRM_OFFSET);
>  	devid = readw(ssram + SSRAM_DEVID_OFFSET);
>  
> -	pmc_ssram_telems[pmc_idx].devid = devid;
>  	pmc_ssram_telems[pmc_idx].base_addr = pwrm_base;
> +	/*
> +	 * Memory barrier is used to ensure the correct write order between base_addr
> +	 * and devid.
> +	 */
> +	smp_wmb();
> +	pmc_ssram_telems[pmc_idx].devid = devid;

Claude also flagged this area, and I suspect this is the same class of issue
Sashiko found.

I tried to cheat, overloading devid as both payloaad and probe state. But if a
reprobe fails after old values were published, the old devid/base address can
remain visible. In practice those values describe static hardware (the hidden
PMC DID and PWRM base), so the stale payload itself is not the most concerning
part.

...

>  }
>  
>  static int
> @@ -154,32 +158,28 @@ static int pmc_ssram_telemetry_pci_init(struct pci_dev *pcidev)
>   * * 0           - Success
>   * * -EAGAIN     - Probe function has not finished yet. Try again.
>   * * -EINVAL     - Invalid pmc_idx
> - * * -ENODEV     - PMC device is not available

The more important issue is the error semantics here.

With device_probed removed, devid == 0 becomes the only indication that PMC info
is not ready. That loses the old distinction between,

  - probe has not completed yet: -EAGAIN
  - probe completed/failed and no PMC info is available: -ENODEV

So if the SSRAM probe fails before devid is published and does not later reprobe
successfully, pmc_core keeps seeing -EAGAIN and can keep deferring indefinitely
instead of falling back to its own legacy init path.

The fix is a bit more involved because the driver has to handle two models, the
legacy path where one PCI probe discovers all PMC indices, and the
newer Nova Lake direction where each probe instance owns a single PMC index.

I added explicit per-index state and kept the devid and base_addr values staged
until before the final return from probe. The state distinguishs between
in-progress, present, and unavailable without overloading devid.

David


>   */
>  int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
>  				     struct pmc_ssram_telemetry *pmc_ssram_telemetry)
>  {
> +	if (pmc_idx >= MAX_NUM_PMC)
> +		return -EINVAL;
> +
>  	/*
>  	 * PMCs are discovered in probe function. If this function is called before
> -	 * probe function complete, the result would be invalid. Use device_probed
> -	 * variable to avoid this case. Return -EAGAIN to inform the consumer to call
> +	 * probe function complete, the result would be invalid. Use devid to avoid
> +	 * this case. Return -EAGAIN to inform the consumer to call
>  	 * again later.
>  	 */
> -	if (!device_probed)
> +	if (!pmc_ssram_telems[pmc_idx].devid)
>  		return -EAGAIN;
>  
> +	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
>  	/*
>  	 * Memory barrier is used to ensure the correct read order between
> -	 * device_probed variable and PMC info.
> +	 * devid variable and base_addr.
>  	 */
>  	smp_rmb();
> -	if (pmc_idx >= MAX_NUM_PMC)
> -		return -EINVAL;
> -
> -	if (!pmc_ssram_telems[pmc_idx].devid)
> -		return -ENODEV;
> -
> -	pmc_ssram_telemetry->devid = pmc_ssram_telems[pmc_idx].devid;
>  	pmc_ssram_telemetry->base_addr = pmc_ssram_telems[pmc_idx].base_addr;
>  	return 0;
>  }
> @@ -194,8 +194,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  	ssram_type = (const struct ssram_type *)id->driver_data;
>  	if (!ssram_type) {
>  		dev_dbg(&pcidev->dev, "missing driver data\n");
> -		ret = -EINVAL;
> -		goto probe_finish;
> +		return -EINVAL;
>  	}
>  
>  	method = ssram_type->method;
> @@ -203,7 +202,7 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  	ret = pcim_enable_device(pcidev);
>  	if (ret) {
>  		dev_dbg(&pcidev->dev, "failed to enable PMC SSRAM device\n");
> -		goto probe_finish;
> +		return ret;
>  	}
>  
>  	if (method == RES_METHOD_PCI)
> @@ -211,13 +210,6 @@ static int pmc_ssram_telemetry_probe(struct pci_dev *pcidev, const struct pci_de
>  	else
>  		ret = -EINVAL;
>  
> -probe_finish:
> -	/*
> -	 * Memory barrier is used to ensure the correct write order between PMC info
> -	 * and device_probed variable.
> -	 */
> -	smp_wmb();
> -	device_probed = true;
>  	return ret;
>  }
>  
> -- 
> 2.43.0
>