Create Intel PMC SSRAM Telemetry driver

PATCH In review Xi Pardee 2 participants 7 messages Apr 25, 2025 – Apr 29, 2025

Subsystems (3)

x86 platform drivers Intel PMC core driver Intel vendor specific extended capabilities driver

lore

Share & tools

lore

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #0 lore raw

v5 cover Create Intel PMC SSRAM Telemetry driver

This patch series removes the SSRAM support from Intel PMC Core driver and creates a separate PCI driver for SSRAM device. The new Intel PMC SSRAM driver provides the following functionalities:

1. Search and store the PMC information in a structure, including PWRMBASE address and devid for each available PMC. Then Intel PMC Core driver achieves the PMC information using the API provided by the new driver.
2. Search and register Intel Platform Monitoring Techology telemetry regions so they would by available for read through sysfs and Intel PMT API. Intel PMC Core driver can achieve Low Power Mode requirement information from a telemetry region registered by the new driver.

The above functionalities was previously handled by Intel PMC Core driver. Intel PMC Core driver returns -EPROBE_DEFER when trying to read data from a telem region that is not available yet. This setup may result in an infinite loop of .probe() calls as Intel PMC Core driver creates child devices. Creating a separate PCI driver avoids the infinite loop possibility.

v5->v4:
- Remove unneeded changes in pmc_core_ssram_get_lpm_reqs()
- Replace mutex_init() with devm_mutex_init()

V4->v3:
- Remove the first six patches that are accepted.
- Add a patch to change the return type of intel_vsec_registeri() from void to int.
- Change the return type of pmc_ssram_telemetry_get_pmc() so the probe function of Intel PMC SSRAM Telemetry driver will fail if intel_vsec_register() fails. The previous implementation hides the failure.
- Use __free(pci_dev_put) in core.c.
- Move pmc_ssram_telemetry_get_pmc_info() function comment to c file from h file.
- Add a patch to improve pmc_core_get_lpm_req() suggested by comment from last version.

v3->v2:
- Add memory barriers to the new driver to ensure write/read order of device_probed variable.
- Minor grammar changes: add needed white space and end of life new line.
- Add patch to move error handling to init function.
- Remove patch to enable SSRAM support of LNL platforms. This patch will be included in a separate series.

v2->v1:
- Rearrange and restructure patches completely based on feedback from v1

Xi Pardee (5):
platform/x86:intel/vsec: Change return type of intel_vsec_register platform/x86:intel/pmc: Create Intel PMC SSRAM Telemetry driver platform/x86:intel/pmc: Use devm for mutex_init platform/x86:intel/pmc: Move error handling to init function platform/x86:intel/pmc: Improve pmc_core_get_lpm_req()

 drivers/platform/x86/intel/pmc/Kconfig        |   4 +
 drivers/platform/x86/intel/pmc/Makefile       |   8 +-
 drivers/platform/x86/intel/pmc/core.c         | 189 +++++++++++-------
 drivers/platform/x86/intel/pmc/core.h         |   7 -
 .../platform/x86/intel/pmc/ssram_telemetry.c  | 147 +++++++++-----
 .../platform/x86/intel/pmc/ssram_telemetry.h  |  24 +++
 drivers/platform/x86/intel/vsec.c             |   9 +-
 include/linux/intel_vsec.h                    |   5 +-
 8 files changed, 260 insertions(+), 133 deletions(-)
 create mode 100644 drivers/platform/x86/intel/pmc/ssram_telemetry.h

Signature · 1 line

2.43.0

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #1 lore raw

PATCH v5 1/5 platform/x86:intel/vsec: Change return type of intel_vsec_register

Change return type of intel_vsec_register() to int. The current implementation does not indicate if the register fail or not. Change to return error code if it fails or if INTEL_VSEC config is not set. This is a preparation step to introduce a new SSRAM Telemetry driver that will be using this API.

Signed-off-by Xi Pardee <[email protected]>

---
 drivers/platform/x86/intel/vsec.c | 9 ++++++---
 include/linux/intel_vsec.h        | 5 +++--
 2 files changed, 9 insertions(+), 5 deletions(-)

drivers/platform/x86/intel/vsec.c +6 -3

index db3c031d1757..055ca9f48fb4 100644
@@ -332,13 +332,16 @@ static bool intel_vsec_walk_vsec(struct pci_dev *pdev,
 	return have_devices;
 }
 
-void intel_vsec_register(struct pci_dev *pdev,
+int intel_vsec_register(struct pci_dev *pdev,
 			 struct intel_vsec_platform_info *info)
 {
 	if (!pdev || !info || !info->headers)
-		return;
+		return -EINVAL;
 
-	intel_vsec_walk_header(pdev, info);
+	if (!intel_vsec_walk_header(pdev, info))
+		return -ENODEV;
+	else
+		return 0;
 }
 EXPORT_SYMBOL_NS_GPL(intel_vsec_register, "INTEL_VSEC");

include/linux/intel_vsec.h +3 -2

index b94beab64610..bc95821f1bfb 100644
@@ -139,12 +139,13 @@ static inline struct intel_vsec_device *auxdev_to_ivdev(struct auxiliary_device
 }
 
 #if IS_ENABLED(CONFIG_INTEL_VSEC)
-void intel_vsec_register(struct pci_dev *pdev,
+int intel_vsec_register(struct pci_dev *pdev,
 			 struct intel_vsec_platform_info *info);
 #else
-static inline void intel_vsec_register(struct pci_dev *pdev,
+static inline int intel_vsec_register(struct pci_dev *pdev,
 				       struct intel_vsec_platform_info *info)
 {
+	return -ENODEV;
 }
 #endif
 #endif

Signature · 1 line

2.43.0

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #2 lore raw

PATCH v5 2/5 platform/x86:intel/pmc: Create Intel PMC SSRAM Telemetry driver

In reply to Xi Pardee

Convert ssram device related functionalities to a new driver named Intel PMC SSRAM Telemetry driver. Modify PMC Core driver to use API exported by the driver to discover and achieve devid and PWRMBASE address information for each available PMC. PMC Core driver needs to get PCI device when reading from telemetry regions.

The new SSRAM driver binds to the SSRAM device and provides the following functionalities:
1. Look for and register telemetry regions available in SSRAM device.
2. Provide devid and PWRMBASE address information for the corresponding PMCs.

Signed-off-by Xi Pardee <[email protected]>

---
 drivers/platform/x86/intel/pmc/Kconfig        |   4 +
 drivers/platform/x86/intel/pmc/Makefile       |   8 +-
 drivers/platform/x86/intel/pmc/core.c         |  73 ++++++---
 drivers/platform/x86/intel/pmc/core.h         |   7 -
 .../platform/x86/intel/pmc/ssram_telemetry.c  | 147 ++++++++++++------
 .../platform/x86/intel/pmc/ssram_telemetry.h  |  24 +++
 6 files changed, 185 insertions(+), 78 deletions(-)
 create mode 100644 drivers/platform/x86/intel/pmc/ssram_telemetry.h

drivers/platform/x86/intel/pmc/Kconfig +4 -0

index d2f651fbec2c..c6ef0bcf76af 100644
@@ -8,6 +8,7 @@ config INTEL_PMC_CORE
 	depends on PCI
 	depends on ACPI
 	depends on INTEL_PMT_TELEMETRY
+	select INTEL_PMC_SSRAM_TELEMETRY
 	help
 	  The Intel Platform Controller Hub for Intel Core SoCs provides access
 	  to Power Management Controller registers via various interfaces. This

@@ -24,3 +25,6 @@ config INTEL_PMC_CORE
 		- SLPS0 Debug registers (Cannonlake/Icelake PCH)
 		- Low Power Mode registers (Tigerlake and beyond)
 		- PMC quirks as needed to enable SLPS0/S0ix
+
+config INTEL_PMC_SSRAM_TELEMETRY
+	tristate

drivers/platform/x86/intel/pmc/Makefile +6 -2

index e842647d3ced..5f68c8503a56 100644
@@ -3,8 +3,12 @@
 # Intel x86 Platform-Specific Drivers
 #
 
-intel_pmc_core-y			:= core.o ssram_telemetry.o spt.o cnp.o \
-					   icl.o tgl.o adl.o mtl.o arl.o lnl.o ptl.o
+intel_pmc_core-y			:= core.o spt.o cnp.o icl.o \
+					   tgl.o adl.o mtl.o arl.o lnl.o ptl.o
 obj-$(CONFIG_INTEL_PMC_CORE)		+= intel_pmc_core.o
 intel_pmc_core_pltdrv-y			:= pltdrv.o
 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

drivers/platform/x86/intel/pmc/core.c +51 ~~-22~~

158 lines not on this page · raw message

drivers/platform/x86/intel/pmc/core.h +0 -7

index c3b07075d017..e136d18b1d38 100644
@@ -413,7 +413,6 @@ struct pmc {
  * struct pmc_dev - pmc device structure
  * @devs:		pointer to an array of pmc pointers
  * @pdev:		pointer to platform_device struct
- * @ssram_pcidev:	pointer to pci device struct for the PMC SSRAM
  * @crystal_freq:	crystal frequency from cpuid
  * @dbgfs_dir:		path to debugfs interface
  * @pmc_xram_read_bit:	flag to indicate whether PMC XRAM shadow registers

@@ -433,7 +432,6 @@ struct pmc_dev {
 	struct pmc *pmcs[MAX_NUM_PMC];
 	struct dentry *dbgfs_dir;
 	struct platform_device *pdev;
-	struct pci_dev *ssram_pcidev;
 	unsigned int crystal_freq;
 	int pmc_xram_read_bit;
 	struct mutex lock; /* generic mutex lock for PMC Core */

@@ -510,12 +508,7 @@ void pmc_core_get_low_power_modes(struct pmc_dev *pmcdev);
 void pmc_core_punit_pmt_init(struct pmc_dev *pmcdev, u32 guid);
 void pmc_core_set_device_d3(unsigned int device);
 
-int pmc_core_ssram_init(struct pmc_dev *pmcdev, int func);
-
 int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info);
-const struct pmc_reg_map *pmc_core_find_regmap(struct pmc_info *list, u16 devid);
-int pmc_core_pmc_add(struct pmc_dev *pmcdev, u64 pwrm_base,
-		     const struct pmc_reg_map *reg_map, unsigned int pmc_index);
 
 extern struct pmc_dev_info spt_pmc_dev;
 extern struct pmc_dev_info cnp_pmc_dev;

drivers/platform/x86/intel/pmc/ssram_telemetry.c +100 ~~-47~~

212 lines not on this page · raw message

drivers/platform/x86/intel/pmc/ssram_telemetry.h new file +24 -0

new file mode 100644
index 000000000000..daf8aeeb2275
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Intel PMC SSRAM Telemetry PCI Driver Header File
+ *
+ * Copyright (c) 2024, Intel Corporation.
+ */
+
+#ifndef PMC_SSRAM_H
+#define PMC_SSRAM_H
+
+/**
+ * struct pmc_ssram_telemetry - Structure to keep pmc info in ssram device
+ * @devid:		device id of the pmc device
+ * @base_addr:		contains PWRM base address
+ */
+struct pmc_ssram_telemetry {
+	u16 devid;
+	u64 base_addr;
+};
+
+int pmc_ssram_telemetry_get_pmc_info(unsigned int pmc_idx,
+				     struct pmc_ssram_telemetry *pmc_ssram_telemetry);
+
+#endif /* PMC_SSRAM_H */

Signature · 1 line

2.43.0

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #3 lore raw

PATCH v5 3/5 platform/x86:intel/pmc: Use devm for mutex_init

In reply to Xi Pardee

Use devm_mutex_init() to avoid accidental resource leak in the future.

Signed-off-by Xi Pardee <[email protected]>

---
 drivers/platform/x86/intel/pmc/core.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/platform/x86/intel/pmc/core.c +3 -2

index 2028a769cddb..db3fccca06fb 100644
@@ -1681,7 +1681,6 @@ static void pmc_core_clean_structure(struct platform_device *pdev)
 		pmt_telem_unregister_endpoint(pmcdev->punit_ep);
 
 	platform_set_drvdata(pdev, NULL);
-	mutex_destroy(&pmcdev->lock);
 }
 
 static int pmc_core_probe(struct platform_device *pdev)

@@ -1726,7 +1725,9 @@ static int pmc_core_probe(struct platform_device *pdev)
 	if (!pmcdev->pkgc_res_cnt)
 		return -ENOMEM;
 
-	mutex_init(&pmcdev->lock);
+	ret = devm_mutex_init(&pdev->dev, &pmcdev->lock);
+	if (ret)
+		return ret;
 
 	if (pmc_dev_info->init)
 		ret = pmc_dev_info->init(pmcdev, pmc_dev_info);

Signature · 1 line

2.43.0

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #4 lore raw

PATCH v5 4/5 platform/x86:intel/pmc: Move error handling to init function

In reply to Xi Pardee

Move error handling code to generic_core_init() function. The previous implementation is that init function called for "full cleanup" function when error occurs which is error prone. The init function should handle the error path itself to improve code maintainability.

Signed-off-by Xi Pardee <[email protected]>

---
 drivers/platform/x86/intel/pmc/core.c | 22 +++++++++++++++++++---
 1 file changed, 19 insertions(+), 3 deletions(-)

drivers/platform/x86/intel/pmc/core.c +19 -3

index db3fccca06fb..93a335b0ea63 100644
@@ -1583,10 +1583,26 @@ int generic_core_init(struct pmc_dev *pmcdev, struct pmc_dev_info *pmc_dev_info)
 	if (pmc_dev_info->dmu_guid)
 		pmc_core_punit_pmt_init(pmcdev, pmc_dev_info->dmu_guid);
 
-	if (ssram)
-		return pmc_core_ssram_get_lpm_reqs(pmcdev, pmc_dev_info->pci_func);
+	if (ssram) {
+		ret = pmc_core_ssram_get_lpm_reqs(pmcdev, pmc_dev_info->pci_func);
+		if (ret)
+			goto unmap_regbase;
+	}
 
 	return 0;
+
+unmap_regbase:
+	for (unsigned int i = 0; i < ARRAY_SIZE(pmcdev->pmcs); ++i) {
+		struct pmc *pmc = pmcdev->pmcs[i];
+
+		if (pmc && pmc->regbase)
+			iounmap(pmc->regbase);
+	}
+
+	if (pmcdev->punit_ep)
+		pmt_telem_unregister_endpoint(pmcdev->punit_ep);
+
+	return ret;
 }
 
 static const struct x86_cpu_id intel_pmc_core_ids[] = {

@@ -1735,7 +1751,7 @@ static int pmc_core_probe(struct platform_device *pdev)
 		ret = generic_core_init(pmcdev, pmc_dev_info);
 
 	if (ret) {
-		pmc_core_clean_structure(pdev);
+		platform_set_drvdata(pdev, NULL);
 		return ret;
 	}

Signature · 1 line

2.43.0

Xi Pardee

author maintainer

2025-04-25 19:53 UTC #5 lore raw

PATCH v5 5/5 platform/x86:intel/pmc: Improve pmc_core_get_lpm_req()

In reply to Xi Pardee

Minor improvements on pmc_core_get_lpm_req().
1. Move the long comment to be above the function
2. Use %pe to print error pointer
3. Remove unneeded devm_kfree call

These changes improves the code maintainability.

Signed-off-by Xi Pardee <[email protected]>

---
 drivers/platform/x86/intel/pmc/core.c | 91 +++++++++++++--------------
 1 file changed, 45 insertions(+), 46 deletions(-)

drivers/platform/x86/intel/pmc/core.c +45 ~~-46~~

index 93a335b0ea63..a32adc53da98 100644
@@ -1355,6 +1355,50 @@ static u32 pmc_core_find_guid(struct pmc_info *list, const struct pmc_reg_map *m
 	return 0;
 }
 
+/*
+ * This function retrieves low power mode requirement data from PMC Low
+ * Power Mode (LPM) table.
+ *
+ * In telemetry space, the LPM table contains a 4 byte header followed
+ * by 8 consecutive mode blocks (one for each LPM mode). Each block
+ * has a 4 byte header followed by a set of registers that describe the
+ * IP state requirements for the given mode. The IP mapping is platform
+ * specific but the same for each block, making for easy analysis.
+ * Platforms only use a subset of the space to track the requirements
+ * for their IPs. Callers provide the requirement registers they use as
+ * a list of indices. Each requirement register is associated with an
+ * IP map that's maintained by the caller.
+ *
+ * Header
+ * +----+----------------------------+----------------------------+
+ * |  0 |      REVISION              |      ENABLED MODES         |
+ * +----+--------------+-------------+-------------+--------------+
+ *
+ * Low Power Mode 0 Block
+ * +----+--------------+-------------+-------------+--------------+
+ * |  1 |     SUB ID   |     SIZE    |   MAJOR     |   MINOR      |
+ * +----+--------------+-------------+-------------+--------------+
+ * |  2 |           LPM0 Requirements 0                           |
+ * +----+---------------------------------------------------------+
+ * |    |                  ...                                    |
+ * +----+---------------------------------------------------------+
+ * | 29 |           LPM0 Requirements 27                          |
+ * +----+---------------------------------------------------------+
+ *
+ * ...
+ *
+ * Low Power Mode 7 Block
+ * +----+--------------+-------------+-------------+--------------+
+ * |    |     SUB ID   |     SIZE    |   MAJOR     |   MINOR      |
+ * +----+--------------+-------------+-------------+--------------+
+ * | 60 |           LPM7 Requirements 0                           |
+ * +----+---------------------------------------------------------+
+ * |    |                  ...                                    |
+ * +----+---------------------------------------------------------+
+ * | 87 |           LPM7 Requirements 27                          |
+ * +----+---------------------------------------------------------+
+ *
+ */
 static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct pci_dev *pcidev)
 {
 	struct telem_endpoint *ep;

@@ -1374,8 +1418,7 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct
 
 	ep = pmt_telem_find_and_register_endpoint(pcidev, guid, 0);
 	if (IS_ERR(ep)) {
-		dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %ld",
-			PTR_ERR(ep));
+		dev_dbg(&pmcdev->pdev->dev, "couldn't get telem endpoint %pe", ep);
 		return -EPROBE_DEFER;
 	}

@@ -1387,49 +1430,6 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct
 		goto unregister_ep;
 	}
 
-	/*
-	 * PMC Low Power Mode (LPM) table
-	 *
-	 * In telemetry space, the LPM table contains a 4 byte header followed
-	 * by 8 consecutive mode blocks (one for each LPM mode). Each block
-	 * has a 4 byte header followed by a set of registers that describe the
-	 * IP state requirements for the given mode. The IP mapping is platform
-	 * specific but the same for each block, making for easy analysis.
-	 * Platforms only use a subset of the space to track the requirements
-	 * for their IPs. Callers provide the requirement registers they use as
-	 * a list of indices. Each requirement register is associated with an
-	 * IP map that's maintained by the caller.
-	 *
-	 * Header
-	 * +----+----------------------------+----------------------------+
-	 * |  0 |      REVISION              |      ENABLED MODES         |
-	 * +----+--------------+-------------+-------------+--------------+
-	 *
-	 * Low Power Mode 0 Block
-	 * +----+--------------+-------------+-------------+--------------+
-	 * |  1 |     SUB ID   |     SIZE    |   MAJOR     |   MINOR      |
-	 * +----+--------------+-------------+-------------+--------------+
-	 * |  2 |           LPM0 Requirements 0                           |
-	 * +----+---------------------------------------------------------+
-	 * |    |                  ...                                    |
-	 * +----+---------------------------------------------------------+
-	 * | 29 |           LPM0 Requirements 27                          |
-	 * +----+---------------------------------------------------------+
-	 *
-	 * ...
-	 *
-	 * Low Power Mode 7 Block
-	 * +----+--------------+-------------+-------------+--------------+
-	 * |    |     SUB ID   |     SIZE    |   MAJOR     |   MINOR      |
-	 * +----+--------------+-------------+-------------+--------------+
-	 * | 60 |           LPM7 Requirements 0                           |
-	 * +----+---------------------------------------------------------+
-	 * |    |                  ...                                    |
-	 * +----+---------------------------------------------------------+
-	 * | 87 |           LPM7 Requirements 27                          |
-	 * +----+---------------------------------------------------------+
-	 *
-	 */
 	mode_offset = LPM_HEADER_OFFSET + LPM_MODE_OFFSET;
 	pmc_for_each_mode(mode, pmcdev) {
 		u32 *req_offset = pmc->lpm_req_regs + (mode * num_maps);

@@ -1442,7 +1442,6 @@ static int pmc_core_get_lpm_req(struct pmc_dev *pmcdev, struct pmc *pmc, struct
 			if (ret) {
 				dev_err(&pmcdev->pdev->dev,
 					"couldn't read Low Power Mode requirements: %d\n", ret);
-				devm_kfree(&pmcdev->pdev->dev, pmc->lpm_req_regs);
 				goto unregister_ep;
 			}
 			++req_offset;

Signature · 1 line

2.43.0

Ilpo Järvinen

maintainer

2025-04-29 15:02 UTC #6 lore raw

In reply to Xi Pardee

Quoted 12 lines · Xi Pardee

This patch series removes the SSRAM support from Intel PMC Core driver and creates a separate PCI driver for SSRAM device. The new Intel PMC SSRAM driver provides the following functionalities:
1. Search and store the PMC information in a structure, including PWRMBASE address and devid for each available PMC. Then Intel PMC Core driver achieves the PMC information using the API provided by the new driver.
2. Search and register Intel Platform Monitoring Techology telemetry regions so they would by available for read through sysfs and Intel PMT API. Intel PMC Core driver can achieve Low Power Mode requirement information from a telemetry region registered by the new driver.

Quoted 1 line

[...]

Thank you for your contribution, it has been applied to my local review-ilpo-next branch. Note it will show up in the public platform-drivers-x86/review-ilpo-next branch only once I've pushed my local branch there, which might take a while.

The list of commits applied:
[1/5] platform/x86:intel/vsec: Change return type of intel_vsec_register

      commit: f6347ba78ad48eef2a79d2063d77a32d015c58af
[2/5] platform/x86:intel/pmc: Create Intel PMC SSRAM Telemetry driver
      commit: f3d766cced899fa626e3f6f6ace43e16061ea19e
[3/5] platform/x86:intel/pmc: Use devm for mutex_init
      commit: 95350cbf5c4b7453d15990ad776b7b101fd6a290
[4/5] platform/x86:intel/pmc: Move error handling to init function
      commit: 75c7b75459efeb5b600d99421f3d08f485d33896
[5/5] platform/x86:intel/pmc: Improve pmc_core_get_lpm_req()
      commit: a946156129a6adb702dff43111c1d8cdab4f9362

--
i.