[PATCH 00/10] Support partitioned systems

[PATCH 00/10] Support partitioned systems

[Srinivas Pandruvada]

A partitioned system has two PCI device per package compared to only one.
This doesn't change any TPMI functionality. Just while reading, different
MMIO region is used for TPMI instances.

Some patches are not directly related to partitions, but added for easy
application of series.

Patches are applied on top of origin/review-hans branch as of April 23,
2024.

Srinivas Pandruvada (10):
  platform/x86/intel/tpmi: Handle error from tpmi_process_info()
  platform/x86/intel/tpmi: Check major version change for TPMI
    Information
  platform/x86/intel/tpmi: Align comments in kernel-doc
  platform/x86/intel/tpmi: Add additional TPMI header fields
  platform/x86: ISST: Use local variable for auxdev->dev
  platform/x86: ISST: Shorten the assignments for power_domain_info
  platform/x86: ISST: Support partitioned systems
  platform/x86: ISST: Use in_range() to check package ID validity
  platform/x86: ISST: Add dev_fmt
  platform/x86: ISST: Add missing MODULE_DESCRIPTION

 .../intel/speed_select_if/isst_if_common.c    |   1 +
 .../intel/speed_select_if/isst_tpmi_core.c    | 345 +++++++++++++++---
 drivers/platform/x86/intel/tpmi.c             |  39 +-
 include/linux/intel_tpmi.h                    |  12 +-
 4 files changed, 333 insertions(+), 64 deletions(-)

-- 
2.40.1

[PATCH 01/10] platform/x86/intel/tpmi: Handle error from tpmi_process_info()

[Srinivas Pandruvada]

When tpmi_process_info() returns error, fail to load the driver.
This can happen if call to ioremap() returns error.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Cc: stable@vger.kernel.org # v6.3+
---
 drivers/platform/x86/intel/tpmi.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 910df7c654f4..003e765dedea 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -763,8 +763,11 @@ static int intel_vsec_tpmi_init(struct auxiliary_device *auxdev)
 		 * when actual device nodes created outside this
 		 * loop via tpmi_create_devices().
 		 */
-		if (pfs->pfs_header.tpmi_id == TPMI_INFO_ID)
-			tpmi_process_info(tpmi_info, pfs);
+		if (pfs->pfs_header.tpmi_id == TPMI_INFO_ID) {
+			ret = tpmi_process_info(tpmi_info, pfs);
+			if (ret)
+				return ret;
+		}
 
 		if (pfs->pfs_header.tpmi_id == TPMI_CONTROL_ID)
 			tpmi_set_control_base(auxdev, tpmi_info, pfs);
-- 
2.40.1

[PATCH 02/10] platform/x86/intel/tpmi: Check major version change for TPMI Information

[Srinivas Pandruvada]

Check the major version from TPMI information header and fail to load
driver if the version is not supported.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/platform/x86/intel/tpmi.c | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index 003e765dedea..a5bcb77bcb35 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -666,28 +666,37 @@ static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)
 }
 
 #define TPMI_INFO_BUS_INFO_OFFSET	0x08
+#define TPMI_INFO_MAJOR_VERSION		0x00
 
 static int tpmi_process_info(struct intel_tpmi_info *tpmi_info,
 			     struct intel_tpmi_pm_feature *pfs)
 {
 	struct tpmi_info_header header;
 	void __iomem *info_mem;
+	u64 feature_header;
+	int ret = 0;
 
-	info_mem = ioremap(pfs->vsec_offset + TPMI_INFO_BUS_INFO_OFFSET,
-			   pfs->pfs_header.entry_size * sizeof(u32) - TPMI_INFO_BUS_INFO_OFFSET);
+	info_mem = ioremap(pfs->vsec_offset, pfs->pfs_header.entry_size * sizeof(u32));
 	if (!info_mem)
 		return -ENOMEM;
 
-	memcpy_fromio(&header, info_mem, sizeof(header));
+	feature_header = readq(info_mem);
+	if (TPMI_MAJOR_VERSION(feature_header) != TPMI_INFO_MAJOR_VERSION) {
+		ret = -ENODEV;
+		goto error_info_header;
+	}
+
+	memcpy_fromio(&header, info_mem + TPMI_INFO_BUS_INFO_OFFSET, sizeof(header));
 
 	tpmi_info->plat_info.package_id = header.pkg;
 	tpmi_info->plat_info.bus_number = header.bus;
 	tpmi_info->plat_info.device_number = header.dev;
 	tpmi_info->plat_info.function_number = header.fn;
 
+error_info_header:
 	iounmap(info_mem);
 
-	return 0;
+	return ret;
 }
 
 static int tpmi_fetch_pfs_header(struct intel_tpmi_pm_feature *pfs, u64 start, int size)
-- 
2.40.1

[PATCH 03/10] platform/x86/intel/tpmi: Align comments in kernel-doc

[Srinivas Pandruvada]

Align comments in kernel-doc for the struct intel_tpmi_plat_info.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 include/linux/intel_tpmi.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/include/linux/intel_tpmi.h b/include/linux/intel_tpmi.h
index a3529b962be6..685a41dddf82 100644
--- a/include/linux/intel_tpmi.h
+++ b/include/linux/intel_tpmi.h
@@ -27,9 +27,9 @@ enum intel_tpmi_id {
 
 /**
  * struct intel_tpmi_plat_info - Platform information for a TPMI device instance
- * @package_id:	CPU Package id
- * @bus_number:	PCI bus number
- * @device_number: PCI device number
+ * @package_id:      CPU Package id
+ * @bus_number:      PCI bus number
+ * @device_number:   PCI device number
  * @function_number: PCI function number
  *
  * Structure to store platform data for a TPMI device instance. This
-- 
2.40.1

[PATCH 04/10] platform/x86/intel/tpmi: Add additional TPMI header fields

[Srinivas Pandruvada]

TPMI information header added additional fields in version 2. Some of the
reserved fields in version 1 are used to define new fields.

Parse new fields and export as part of platform data. These fields include:
- PCI segment ID
- Partition ID of the package: If a package is represented by more than
  one PCI device, then partition ID along with cdie_mask, describes the
  scope. For example to update get/set properties for a compute die, one
  of the PCI MMIO region is selected from the partition ID.
- cdie_mask: Mask of all compute dies in this partition.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Zhang Rui <rui.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/platform/x86/intel/tpmi.c | 15 ++++++++++++++-
 include/linux/intel_tpmi.h        |  6 ++++++
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/platform/x86/intel/tpmi.c b/drivers/platform/x86/intel/tpmi.c
index a5bcb77bcb35..6c0cbccd80bb 100644
--- a/drivers/platform/x86/intel/tpmi.c
+++ b/drivers/platform/x86/intel/tpmi.c
@@ -128,6 +128,9 @@ struct intel_tpmi_info {
  * @dev:	PCI device number
  * @bus:	PCI bus number
  * @pkg:	CPU Package id
+ * @segment:	PCI segment id
+ * @partition:	Package Partition id
+ * @cdie_mask:	Bitmap of compute dies in the current partition
  * @reserved:	Reserved for future use
  * @lock:	When set to 1 the register is locked and becomes read-only
  *		until next reset. Not for use by the OS driver.
@@ -139,7 +142,10 @@ struct tpmi_info_header {
 	u64 dev:5;
 	u64 bus:8;
 	u64 pkg:8;
-	u64 reserved:39;
+	u64 segment:8;
+	u64 partition:2;
+	u64 cdie_mask:16;
+	u64 reserved:13;
 	u64 lock:1;
 } __packed;
 
@@ -667,6 +673,7 @@ static int tpmi_create_devices(struct intel_tpmi_info *tpmi_info)
 
 #define TPMI_INFO_BUS_INFO_OFFSET	0x08
 #define TPMI_INFO_MAJOR_VERSION		0x00
+#define TPMI_INFO_MINOR_VERSION		0x02
 
 static int tpmi_process_info(struct intel_tpmi_info *tpmi_info,
 			     struct intel_tpmi_pm_feature *pfs)
@@ -693,6 +700,12 @@ static int tpmi_process_info(struct intel_tpmi_info *tpmi_info,
 	tpmi_info->plat_info.device_number = header.dev;
 	tpmi_info->plat_info.function_number = header.fn;
 
+	if (TPMI_MINOR_VERSION(feature_header) >= TPMI_INFO_MINOR_VERSION) {
+		tpmi_info->plat_info.cdie_mask = header.cdie_mask;
+		tpmi_info->plat_info.partition = header.partition;
+		tpmi_info->plat_info.segment = header.segment;
+	}
+
 error_info_header:
 	iounmap(info_mem);
 
diff --git a/include/linux/intel_tpmi.h b/include/linux/intel_tpmi.h
index 685a41dddf82..1e880cb0f454 100644
--- a/include/linux/intel_tpmi.h
+++ b/include/linux/intel_tpmi.h
@@ -27,7 +27,10 @@ enum intel_tpmi_id {
 
 /**
  * struct intel_tpmi_plat_info - Platform information for a TPMI device instance
+ * @cdie_mask:       Mask of all compute dies in the partition
  * @package_id:      CPU Package id
+ * @partition:       Package partition id when multiple VSEC PCI devices per package
+ * @segment:         PCI segment ID
  * @bus_number:      PCI bus number
  * @device_number:   PCI device number
  * @function_number: PCI function number
@@ -36,7 +39,10 @@ enum intel_tpmi_id {
  * struct is used to return data via tpmi_get_platform_data().
  */
 struct intel_tpmi_plat_info {
+	u16 cdie_mask;
 	u8 package_id;
+	u8 partition;
+	u8 segment;
 	u8 bus_number;
 	u8 device_number;
 	u8 function_number;
-- 
2.40.1

[PATCH 05/10] platform/x86: ISST: Use local variable for auxdev->dev

[Srinivas Pandruvada]

Define a local variable for &auxdev->dev and use to shorten length of
lines. No functional change is done.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../intel/speed_select_if/isst_tpmi_core.c    | 27 ++++++++++---------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index 1d918000d72b..4e09a5611aca 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -313,12 +313,11 @@ static int sst_add_perf_profiles(struct auxiliary_device *auxdev,
 				 struct tpmi_per_power_domain_info *pd_info,
 				 int levels)
 {
+	struct device *dev = &auxdev->dev;
 	u64 perf_level_offsets;
 	int i;
 
-	pd_info->perf_levels = devm_kcalloc(&auxdev->dev, levels,
-					    sizeof(struct perf_level),
-					    GFP_KERNEL);
+	pd_info->perf_levels = devm_kcalloc(dev, levels, sizeof(struct perf_level), GFP_KERNEL);
 	if (!pd_info->perf_levels)
 		return 0;
 
@@ -349,6 +348,7 @@ static int sst_add_perf_profiles(struct auxiliary_device *auxdev,
 
 static int sst_main(struct auxiliary_device *auxdev, struct tpmi_per_power_domain_info *pd_info)
 {
+	struct device *dev = &auxdev->dev;
 	int i, mask, levels;
 
 	*((u64 *)&pd_info->sst_header) = readq(pd_info->sst_base);
@@ -359,13 +359,13 @@ static int sst_main(struct auxiliary_device *auxdev, struct tpmi_per_power_domai
 		return -ENODEV;
 
 	if (TPMI_MAJOR_VERSION(pd_info->sst_header.interface_version) != ISST_MAJOR_VERSION) {
-		dev_err(&auxdev->dev, "SST: Unsupported major version:%lx\n",
+		dev_err(dev, "SST: Unsupported major version:%lx\n",
 			TPMI_MAJOR_VERSION(pd_info->sst_header.interface_version));
 		return -ENODEV;
 	}
 
 	if (TPMI_MINOR_VERSION(pd_info->sst_header.interface_version) != ISST_MINOR_VERSION)
-		dev_info(&auxdev->dev, "SST: Ignore: Unsupported minor version:%lx\n",
+		dev_info(dev, "SST: Ignore: Unsupported minor version:%lx\n",
 			 TPMI_MINOR_VERSION(pd_info->sst_header.interface_version));
 
 	/* Read SST CP Header */
@@ -1273,28 +1273,29 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 {
 	bool read_blocked = 0, write_blocked = 0;
 	struct intel_tpmi_plat_info *plat_info;
+	struct device *dev = &auxdev->dev;
 	struct tpmi_sst_struct *tpmi_sst;
 	int i, ret, pkg = 0, inst = 0;
 	int num_resources;
 
 	ret = tpmi_get_feature_status(auxdev, TPMI_ID_SST, &read_blocked, &write_blocked);
 	if (ret)
-		dev_info(&auxdev->dev, "Can't read feature status: ignoring read/write blocked status\n");
+		dev_info(dev, "Can't read feature status: ignoring read/write blocked status\n");
 
 	if (read_blocked) {
-		dev_info(&auxdev->dev, "Firmware has blocked reads, exiting\n");
+		dev_info(dev, "Firmware has blocked reads, exiting\n");
 		return -ENODEV;
 	}
 
 	plat_info = tpmi_get_platform_data(auxdev);
 	if (!plat_info) {
-		dev_err(&auxdev->dev, "No platform info\n");
+		dev_err(dev, "No platform info\n");
 		return -EINVAL;
 	}
 
 	pkg = plat_info->package_id;
 	if (pkg >= topology_max_packages()) {
-		dev_err(&auxdev->dev, "Invalid package id :%x\n", pkg);
+		dev_err(dev, "Invalid package id :%x\n", pkg);
 		return -EINVAL;
 	}
 
@@ -1306,11 +1307,11 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 	if (!num_resources)
 		return -EINVAL;
 
-	tpmi_sst = devm_kzalloc(&auxdev->dev, sizeof(*tpmi_sst), GFP_KERNEL);
+	tpmi_sst = devm_kzalloc(dev, sizeof(*tpmi_sst), GFP_KERNEL);
 	if (!tpmi_sst)
 		return -ENOMEM;
 
-	tpmi_sst->power_domain_info = devm_kcalloc(&auxdev->dev, num_resources,
+	tpmi_sst->power_domain_info = devm_kcalloc(dev, num_resources,
 						   sizeof(*tpmi_sst->power_domain_info),
 						   GFP_KERNEL);
 	if (!tpmi_sst->power_domain_info)
@@ -1331,13 +1332,13 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 		tpmi_sst->power_domain_info[i].power_domain_id = i;
 		tpmi_sst->power_domain_info[i].auxdev = auxdev;
 		tpmi_sst->power_domain_info[i].write_blocked = write_blocked;
-		tpmi_sst->power_domain_info[i].sst_base = devm_ioremap_resource(&auxdev->dev, res);
+		tpmi_sst->power_domain_info[i].sst_base = devm_ioremap_resource(dev, res);
 		if (IS_ERR(tpmi_sst->power_domain_info[i].sst_base))
 			return PTR_ERR(tpmi_sst->power_domain_info[i].sst_base);
 
 		ret = sst_main(auxdev, &tpmi_sst->power_domain_info[i]);
 		if (ret) {
-			devm_iounmap(&auxdev->dev, tpmi_sst->power_domain_info[i].sst_base);
+			devm_iounmap(dev, tpmi_sst->power_domain_info[i].sst_base);
 			tpmi_sst->power_domain_info[i].sst_base =  NULL;
 			continue;
 		}
-- 
2.40.1

[PATCH 06/10] platform/x86: ISST: Shorten the assignments for power_domain_info

[Srinivas Pandruvada]

Instead of long lines for assignment to tpmi_sst->power_domain_info, use
a local variable pd_info and assign later. Also move the assignment
of number of resources after the assignment of pd_info.

No functional change is expected.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../intel/speed_select_if/isst_tpmi_core.c    | 33 +++++++++----------
 1 file changed, 16 insertions(+), 17 deletions(-)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index 4e09a5611aca..49d573fcbd72 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -1271,6 +1271,7 @@ static long isst_if_def_ioctl(struct file *file, unsigned int cmd,
 
 int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 {
+	struct tpmi_per_power_domain_info *pd_info;
 	bool read_blocked = 0, write_blocked = 0;
 	struct intel_tpmi_plat_info *plat_info;
 	struct device *dev = &auxdev->dev;
@@ -1311,35 +1312,31 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 	if (!tpmi_sst)
 		return -ENOMEM;
 
-	tpmi_sst->power_domain_info = devm_kcalloc(dev, num_resources,
-						   sizeof(*tpmi_sst->power_domain_info),
-						   GFP_KERNEL);
-	if (!tpmi_sst->power_domain_info)
+	pd_info = devm_kcalloc(dev, num_resources, sizeof(*pd_info), GFP_KERNEL);
+	if (!pd_info)
 		return -ENOMEM;
 
-	tpmi_sst->number_of_power_domains = num_resources;
-
 	for (i = 0; i < num_resources; ++i) {
 		struct resource *res;
 
 		res = tpmi_get_resource_at_index(auxdev, i);
 		if (!res) {
-			tpmi_sst->power_domain_info[i].sst_base = NULL;
+			pd_info[i].sst_base = NULL;
 			continue;
 		}
 
-		tpmi_sst->power_domain_info[i].package_id = pkg;
-		tpmi_sst->power_domain_info[i].power_domain_id = i;
-		tpmi_sst->power_domain_info[i].auxdev = auxdev;
-		tpmi_sst->power_domain_info[i].write_blocked = write_blocked;
-		tpmi_sst->power_domain_info[i].sst_base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(tpmi_sst->power_domain_info[i].sst_base))
-			return PTR_ERR(tpmi_sst->power_domain_info[i].sst_base);
+		pd_info[i].package_id = pkg;
+		pd_info[i].power_domain_id = i;
+		pd_info[i].auxdev = auxdev;
+		pd_info[i].write_blocked = write_blocked;
+		pd_info[i].sst_base = devm_ioremap_resource(dev, res);
+		if (IS_ERR(pd_info[i].sst_base))
+			return PTR_ERR(pd_info[i].sst_base);
 
-		ret = sst_main(auxdev, &tpmi_sst->power_domain_info[i]);
+		ret = sst_main(auxdev, &pd_info[i]);
 		if (ret) {
-			devm_iounmap(dev, tpmi_sst->power_domain_info[i].sst_base);
-			tpmi_sst->power_domain_info[i].sst_base =  NULL;
+			devm_iounmap(dev, pd_info[i].sst_base);
+			pd_info[i].sst_base = NULL;
 			continue;
 		}
 
@@ -1350,6 +1347,8 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 		return -ENODEV;
 
 	tpmi_sst->package_id = pkg;
+	tpmi_sst->power_domain_info = pd_info;
+	tpmi_sst->number_of_power_domains = num_resources;
 	auxiliary_set_drvdata(auxdev, tpmi_sst);
 
 	mutex_lock(&isst_tpmi_dev_lock);
-- 
2.40.1

[PATCH 07/10] platform/x86: ISST: Support partitioned systems

[Srinivas Pandruvada]

A partitioned system has two different PCI VSEC devices per package.
A non-partitioned device has only one PCI VSEC device per package.
The current implementation only supports non partitioned systems.

Each partition maps a set of power domains. Other than reading from
different MMIO regions, there is no change in the SST functionality.
The scope of SST control is still per power domain. Hence user space
does not need to be aware of existence of partitions.

With partitions, existing per package information defined using struct
tpmi_sst_struct is enhanced to store information for both partitions. A
mapping function map_partition_power_domain_id() is introduced, which
maps to correct partition and index. This mapping function is called
in get_instance() and isst_if_clos_assoc(), before indexing into
tpmi_sst_struct->power_domain_info[].

The TPMI core platform info provides partition id and compute die ID
mask for each partition. Use this information to order power domains,
so that compute dies are presented before IO dies to match hardware
defined compute die ID for each CPU.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Zhang Rui <rui.zhang@intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 .../intel/speed_select_if/isst_tpmi_core.c    | 299 ++++++++++++++++--
 1 file changed, 267 insertions(+), 32 deletions(-)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index 49d573fcbd72..b8da6847622b 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -23,6 +23,7 @@
 #include <linux/fs.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
+#include <linux/minmax.h>
 #include <linux/module.h>
 #include <uapi/linux/isst_if.h>
 
@@ -263,20 +264,33 @@ struct tpmi_per_power_domain_info {
 	bool write_blocked;
 };
 
+/* Supported maximum partitions */
+#define SST_MAX_PARTITIONS	2
+
 /**
  * struct tpmi_sst_struct -	Store sst info for a package
  * @package_id:			Package id for this aux device instance
  * @number_of_power_domains:	Number of power_domains pointed by power_domain_info pointer
  * @power_domain_info:		Pointer to power domains information
+ * @cdie_mask:			Mask of compute dies present in a partition from hardware.
+ *				This mask is not present in the version 1 information header.
+ * @io_dies:			Number of IO dies in a partition. This will be 0 for TPMI
+ *				version 1 information header.
+ * @partition_mask:		Mask of all partitions.
+ * @partition_mask_current:	Current partition mask as some may have been unbound.
  *
  * This structure is used store full SST information for a package.
- * Each package has a unique OOB PCI device, which enumerates TPMI.
- * Each Package will have multiple power_domains.
+ * Each package has one or multiple OOB PCI devices. Each package can contain multiple
+ * power domains.
  */
 struct tpmi_sst_struct {
 	int package_id;
-	int number_of_power_domains;
-	struct tpmi_per_power_domain_info *power_domain_info;
+	struct tpmi_per_power_domain_info *power_domain_info[SST_MAX_PARTITIONS];
+	u16 cdie_mask[SST_MAX_PARTITIONS];
+	u8 number_of_power_domains[SST_MAX_PARTITIONS];
+	u8 io_dies[SST_MAX_PARTITIONS];
+	u8 partition_mask;
+	u8 partition_mask_current;
 };
 
 /**
@@ -387,6 +401,126 @@ static int sst_main(struct auxiliary_device *auxdev, struct tpmi_per_power_domai
 	return 0;
 }
 
+static u8 isst_instance_count(struct tpmi_sst_struct *sst_inst)
+{
+	u8 i, max_part, count = 0;
+
+	/* Partition mask starts from bit 0 and contains 1s only */
+	max_part = hweight8(sst_inst->partition_mask);
+	for (i = 0; i < max_part; i++)
+		count += sst_inst->number_of_power_domains[i];
+
+	return count;
+}
+
+/**
+ * map_cdies() - Map user domain ID to compute domain ID
+ * @sst_inst: TPMI Instance
+ * @id: User domain ID
+ * @partition: Resolved partition
+ *
+ * Helper function to map_partition_power_domain_id() to resolve compute
+ * domain ID and partition. Use hardware provided cdie_mask for a partition
+ * as is to resolve a compute domain ID.
+ *
+ * Return: %-EINVAL on error, otherwise mapped domain ID >= 0.
+ */
+static int map_cdies(struct tpmi_sst_struct *sst_inst, u8 id, u8 *partition)
+{
+	u8 i, max_part;
+
+	max_part = hweight8(sst_inst->partition_mask);
+	for (i = 0; i < max_part; i++) {
+		if (!(sst_inst->cdie_mask[i] & BIT(id)))
+			continue;
+
+		*partition = i;
+		return id - ffs(sst_inst->cdie_mask[i]) + 1;
+	}
+
+	return -EINVAL;
+}
+
+/**
+ * map_partition_power_domain_id() - Map user domain ID to partition domain ID
+ * @sst_inst: TPMI Instance
+ * @id: User domain ID
+ * @partition: Resolved partition
+ *
+ * In a partitioned system a CPU package has two separate MMIO ranges (Under
+ * two PCI devices). But the CPU package compute die/power domain IDs are
+ * unique in a package. User space can get compute die/power domain ID from
+ * CPUID and MSR 0x54 for a CPU. So, those IDs need to be preserved even if
+ * they are present in two different partitions with its own order.
+ *
+ * For example for command ISST_IF_COUNT_TPMI_INSTANCES, the valid_mask
+ * is 111111b for a 4 compute and 2 IO dies system. This is presented as
+ * provided by the hardware in a non-partitioned system with the following
+ * order:
+ *	I1-I0-C3-C2-C1-C0
+ * Here: "C": for compute and "I" for IO die.
+ * Compute dies are always present first in TPMI instances, as they have
+ * to map to the real power domain/die ID of a system. In a non-partitioned
+ * system there is no way to identify compute and IO die boundaries from
+ * this driver without reading each CPU's mapping.
+ *
+ * The same order needs to be preserved, even if those compute dies are
+ * distributed among multiple partitions. For example:
+ * Partition 1 can contain: I1-C1-C0
+ * Partition 2 can contain: I2-C3-C2
+ *
+ * This will require a conversion of user space IDs to the actual index into
+ * array of stored power domains for each partition. For the above example
+ * this function will return partition and index as follows:
+ *
+ * =============	=========	=====	========
+ * User space ID	Partition	Index	Die type
+ * =============	=========	=====	========
+ * 0			0		0	Compute
+ * 1			0		1	Compute
+ * 2			1		0	Compute
+ * 3			1		1	Compute
+ * 4			0		2	IO
+ * 5			1		2	IO
+ * =============	=========	=====	========
+ *
+ * Return: %-EINVAL on error, otherwise mapped domain ID >= 0.
+ */
+static int map_partition_power_domain_id(struct tpmi_sst_struct *sst_inst, u8 id, u8 *partition)
+{
+	u8 i, io_start_id, max_part;
+
+	*partition = 0;
+
+	/* If any PCI device for partition is unbound, treat this as failure */
+	if (sst_inst->partition_mask != sst_inst->partition_mask_current)
+		return -EINVAL;
+
+	max_part = hweight8(sst_inst->partition_mask);
+
+	/* IO Index begin here */
+	io_start_id = fls(sst_inst->cdie_mask[max_part - 1]);
+
+	if (id < io_start_id)
+		return map_cdies(sst_inst, id, partition);
+
+	for (i = 0; i < max_part; i++) {
+		u8 io_id;
+
+		io_id = id - io_start_id;
+		if (io_id < sst_inst->io_dies[i]) {
+			u8 cdie_range;
+
+			cdie_range = fls(sst_inst->cdie_mask[i]) - ffs(sst_inst->cdie_mask[i]) + 1;
+			*partition = i;
+			return cdie_range + io_id;
+		}
+		io_start_id += sst_inst->io_dies[i];
+	}
+
+	return -EINVAL;
+}
+
 /*
  * Map a package and power_domain id to SST information structure unique for a power_domain.
  * The caller should call under isst_tpmi_dev_lock.
@@ -395,6 +529,7 @@ static struct tpmi_per_power_domain_info *get_instance(int pkg_id, int power_dom
 {
 	struct tpmi_per_power_domain_info *power_domain_info;
 	struct tpmi_sst_struct *sst_inst;
+	u8 part;
 
 	if (pkg_id < 0 || pkg_id > isst_common.max_index ||
 	    pkg_id >= topology_max_packages())
@@ -404,10 +539,11 @@ static struct tpmi_per_power_domain_info *get_instance(int pkg_id, int power_dom
 	if (!sst_inst)
 		return NULL;
 
-	if (power_domain_id < 0 || power_domain_id >= sst_inst->number_of_power_domains)
+	power_domain_id = map_partition_power_domain_id(sst_inst, power_domain_id, &part);
+	if (power_domain_id < 0)
 		return NULL;
 
-	power_domain_info = &sst_inst->power_domain_info[power_domain_id];
+	power_domain_info = &sst_inst->power_domain_info[part][power_domain_id];
 
 	if (power_domain_info && !power_domain_info->sst_base)
 		return NULL;
@@ -579,6 +715,7 @@ static long isst_if_clos_assoc(void __user *argp)
 		struct tpmi_sst_struct *sst_inst;
 		int offset, shift, cpu;
 		u64 val, mask, clos;
+		u8 part;
 
 		if (copy_from_user(&clos_assoc, ptr, sizeof(clos_assoc)))
 			return -EFAULT;
@@ -602,10 +739,11 @@ static long isst_if_clos_assoc(void __user *argp)
 
 		sst_inst = isst_common.sst_inst[pkg_id];
 
-		if (clos_assoc.power_domain_id > sst_inst->number_of_power_domains)
+		punit_id = map_partition_power_domain_id(sst_inst, punit_id, &part);
+		if (punit_id < 0)
 			return -EINVAL;
 
-		power_domain_info = &sst_inst->power_domain_info[punit_id];
+		power_domain_info = &sst_inst->power_domain_info[part][punit_id];
 
 		if (assoc_cmds.get_set && power_domain_info->write_blocked)
 			return -EPERM;
@@ -1134,18 +1272,28 @@ static int isst_if_get_tpmi_instance_count(void __user *argp)
 	if (tpmi_inst.socket_id >= topology_max_packages())
 		return -EINVAL;
 
-	tpmi_inst.count = isst_common.sst_inst[tpmi_inst.socket_id]->number_of_power_domains;
-
 	sst_inst = isst_common.sst_inst[tpmi_inst.socket_id];
+
+	tpmi_inst.count = isst_instance_count(sst_inst);
+
 	tpmi_inst.valid_mask = 0;
-	for (i = 0; i < sst_inst->number_of_power_domains; ++i) {
+	for (i = 0; i < tpmi_inst.count; i++) {
 		struct tpmi_per_power_domain_info *pd_info;
+		u8 part;
+		int pd;
+
+		pd = map_partition_power_domain_id(sst_inst, i, &part);
+		if (pd < 0)
+			continue;
 
-		pd_info = &sst_inst->power_domain_info[i];
+		pd_info = &sst_inst->power_domain_info[part][pd];
 		if (pd_info->sst_base)
 			tpmi_inst.valid_mask |= BIT(i);
 	}
 
+	if (!tpmi_inst.valid_mask)
+		tpmi_inst.count = 0;
+
 	if (copy_to_user(argp, &tpmi_inst, sizeof(tpmi_inst)))
 		return -EFAULT;
 
@@ -1276,8 +1424,11 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 	struct intel_tpmi_plat_info *plat_info;
 	struct device *dev = &auxdev->dev;
 	struct tpmi_sst_struct *tpmi_sst;
-	int i, ret, pkg = 0, inst = 0;
-	int num_resources;
+	u8 i, num_resources, io_die_cnt;
+	int ret, pkg = 0, inst = 0;
+	bool first_enum = false;
+	u16 cdie_mask;
+	u8 partition;
 
 	ret = tpmi_get_feature_status(auxdev, TPMI_ID_SST, &read_blocked, &write_blocked);
 	if (ret)
@@ -1300,21 +1451,59 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 		return -EINVAL;
 	}
 
-	if (isst_common.sst_inst[pkg])
-		return -EEXIST;
+	partition = plat_info->partition;
+	if (partition >= SST_MAX_PARTITIONS) {
+		dev_err(&auxdev->dev, "Invalid partition :%x\n", partition);
+		return -EINVAL;
+	}
 
 	num_resources = tpmi_get_resource_count(auxdev);
 
 	if (!num_resources)
 		return -EINVAL;
 
-	tpmi_sst = devm_kzalloc(dev, sizeof(*tpmi_sst), GFP_KERNEL);
-	if (!tpmi_sst)
-		return -ENOMEM;
+	mutex_lock(&isst_tpmi_dev_lock);
+
+	if (isst_common.sst_inst[pkg]) {
+		tpmi_sst = isst_common.sst_inst[pkg];
+	} else {
+		/*
+		 * tpmi_sst instance is for a package. So needs to be
+		 * allocated only once for both partitions. We can't use
+		 * devm_* allocation here as each partition is a
+		 * different device, which can be unbound.
+		 */
+		tpmi_sst = kzalloc(sizeof(*tpmi_sst), GFP_KERNEL);
+		if (!tpmi_sst) {
+			ret = -ENOMEM;
+			goto unlock_exit;
+		}
+		first_enum = true;
+	}
+
+	ret = 0;
 
 	pd_info = devm_kcalloc(dev, num_resources, sizeof(*pd_info), GFP_KERNEL);
-	if (!pd_info)
-		return -ENOMEM;
+	if (!pd_info) {
+		ret = -ENOMEM;
+		goto unlock_free;
+	}
+
+	/* Get the IO die count, if cdie_mask is present */
+	if (plat_info->cdie_mask) {
+		u8 cdie_range;
+
+		cdie_mask = plat_info->cdie_mask;
+		cdie_range = fls(cdie_mask) - ffs(cdie_mask) + 1;
+		io_die_cnt = num_resources - cdie_range;
+	} else {
+		/*
+		 * This is a synthetic mask, careful when assuming that
+		 * they are compute dies only.
+		 */
+		cdie_mask = (1 << num_resources) - 1;
+		io_die_cnt = 0;
+	}
 
 	for (i = 0; i < num_resources; ++i) {
 		struct resource *res;
@@ -1330,11 +1519,20 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 		pd_info[i].auxdev = auxdev;
 		pd_info[i].write_blocked = write_blocked;
 		pd_info[i].sst_base = devm_ioremap_resource(dev, res);
-		if (IS_ERR(pd_info[i].sst_base))
-			return PTR_ERR(pd_info[i].sst_base);
+		if (IS_ERR(pd_info[i].sst_base)) {
+			ret = PTR_ERR(pd_info[i].sst_base);
+			goto unlock_free;
+		}
 
 		ret = sst_main(auxdev, &pd_info[i]);
 		if (ret) {
+			/*
+			 * This entry is not valid, hardware can partially
+			 * populate dies. In this case MMIO will have 0xFFs.
+			 * Also possible some pre-production hardware has
+			 * invalid data. But don't fail and continue to use
+			 * other dies with valid data.
+			 */
 			devm_iounmap(dev, pd_info[i].sst_base);
 			pd_info[i].sst_base = NULL;
 			continue;
@@ -1343,30 +1541,53 @@ int tpmi_sst_dev_add(struct auxiliary_device *auxdev)
 		++inst;
 	}
 
-	if (!inst)
-		return -ENODEV;
+	if (!inst) {
+		ret = -ENODEV;
+		goto unlock_free;
+	}
 
 	tpmi_sst->package_id = pkg;
-	tpmi_sst->power_domain_info = pd_info;
-	tpmi_sst->number_of_power_domains = num_resources;
+
+	tpmi_sst->power_domain_info[partition] = pd_info;
+	tpmi_sst->number_of_power_domains[partition] = num_resources;
+	tpmi_sst->cdie_mask[partition] = cdie_mask;
+	tpmi_sst->io_dies[partition] = io_die_cnt;
+	tpmi_sst->partition_mask |= BIT(partition);
+	tpmi_sst->partition_mask_current |= BIT(partition);
+
 	auxiliary_set_drvdata(auxdev, tpmi_sst);
 
-	mutex_lock(&isst_tpmi_dev_lock);
 	if (isst_common.max_index < pkg)
 		isst_common.max_index = pkg;
 	isst_common.sst_inst[pkg] = tpmi_sst;
+
+unlock_free:
+	if (ret && first_enum)
+		kfree(tpmi_sst);
+unlock_exit:
 	mutex_unlock(&isst_tpmi_dev_lock);
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_add, INTEL_TPMI_SST);
 
 void tpmi_sst_dev_remove(struct auxiliary_device *auxdev)
 {
 	struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev);
+	struct intel_tpmi_plat_info *plat_info;
+
+	plat_info = tpmi_get_platform_data(auxdev);
+	if (!plat_info)
+		return;
 
 	mutex_lock(&isst_tpmi_dev_lock);
-	isst_common.sst_inst[tpmi_sst->package_id] = NULL;
+	tpmi_sst->power_domain_info[plat_info->partition] = NULL;
+	tpmi_sst->partition_mask_current &= ~BIT(plat_info->partition);
+	/* Free the package instance when the all partitions are removed */
+	if (!tpmi_sst->partition_mask_current) {
+		kfree(tpmi_sst);
+		isst_common.sst_inst[tpmi_sst->package_id] = NULL;
+	}
 	mutex_unlock(&isst_tpmi_dev_lock);
 }
 EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_remove, INTEL_TPMI_SST);
@@ -1374,9 +1595,16 @@ EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_remove, INTEL_TPMI_SST);
 void tpmi_sst_dev_suspend(struct auxiliary_device *auxdev)
 {
 	struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev);
-	struct tpmi_per_power_domain_info *power_domain_info = tpmi_sst->power_domain_info;
+	struct tpmi_per_power_domain_info *power_domain_info;
+	struct intel_tpmi_plat_info *plat_info;
 	void __iomem *cp_base;
 
+	plat_info = tpmi_get_platform_data(auxdev);
+	if (!plat_info)
+		return;
+
+	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
+
 	cp_base = power_domain_info->sst_base + power_domain_info->sst_header.cp_offset;
 	power_domain_info->saved_sst_cp_control = readq(cp_base + SST_CP_CONTROL_OFFSET);
 
@@ -1395,9 +1623,16 @@ EXPORT_SYMBOL_NS_GPL(tpmi_sst_dev_suspend, INTEL_TPMI_SST);
 void tpmi_sst_dev_resume(struct auxiliary_device *auxdev)
 {
 	struct tpmi_sst_struct *tpmi_sst = auxiliary_get_drvdata(auxdev);
-	struct tpmi_per_power_domain_info *power_domain_info = tpmi_sst->power_domain_info;
+	struct tpmi_per_power_domain_info *power_domain_info;
+	struct intel_tpmi_plat_info *plat_info;
 	void __iomem *cp_base;
 
+	plat_info = tpmi_get_platform_data(auxdev);
+	if (!plat_info)
+		return;
+
+	power_domain_info = tpmi_sst->power_domain_info[plat_info->partition];
+
 	cp_base = power_domain_info->sst_base + power_domain_info->sst_header.cp_offset;
 	writeq(power_domain_info->saved_sst_cp_control, cp_base + SST_CP_CONTROL_OFFSET);
 
-- 
2.40.1

[PATCH 08/10] platform/x86: ISST: Use in_range() to check package ID validity

[Srinivas Pandruvada]

Use in_range() macro to simplify range check.

No functional impact is expected.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index b8da6847622b..e75fb9eba598 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -531,8 +531,7 @@ static struct tpmi_per_power_domain_info *get_instance(int pkg_id, int power_dom
 	struct tpmi_sst_struct *sst_inst;
 	u8 part;
 
-	if (pkg_id < 0 || pkg_id > isst_common.max_index ||
-	    pkg_id >= topology_max_packages())
+	if (!in_range(pkg_id, 0, topology_max_packages()) || pkg_id > isst_common.max_index)
 		return NULL;
 
 	sst_inst = isst_common.sst_inst[pkg_id];
-- 
2.40.1

[PATCH 09/10] platform/x86: ISST: Add dev_fmt

[Srinivas Pandruvada]

Add dev_fmt for formatting log messages.

No functional impact is expected.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Suggested-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index e75fb9eba598..039333eac71a 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -17,6 +17,8 @@
  * the hardware mapping.
  */
 
+#define dev_fmt(fmt) "tpmi_sst: " fmt
+
 #include <linux/auxiliary_bus.h>
 #include <linux/delay.h>
 #include <linux/intel_tpmi.h>
-- 
2.40.1

[PATCH 10/10] platform/x86: ISST: Add missing MODULE_DESCRIPTION

[Srinivas Pandruvada]

Add missing MODULE_DESCRIPTION() to ISST modules.

Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@linux.intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/platform/x86/intel/speed_select_if/isst_if_common.c | 1 +
 drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c | 1 +
 2 files changed, 2 insertions(+)

diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
index 08df9494603c..88a17be7cb7e 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
@@ -837,4 +837,5 @@ void isst_if_cdev_unregister(int device_type)
 }
 EXPORT_SYMBOL_GPL(isst_if_cdev_unregister);
 
+MODULE_DESCRIPTION("ISST common interface module");
 MODULE_LICENSE("GPL v2");
diff --git a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
index 039333eac71a..6bcbb97b0101 100644
--- a/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
+++ b/drivers/platform/x86/intel/speed_select_if/isst_tpmi_core.c
@@ -1705,4 +1705,5 @@ EXPORT_SYMBOL_NS_GPL(tpmi_sst_exit, INTEL_TPMI_SST);
 MODULE_IMPORT_NS(INTEL_TPMI);
 MODULE_IMPORT_NS(INTEL_TPMI_POWER_DOMAIN);
 
+MODULE_DESCRIPTION("ISST TPMI interface module");
 MODULE_LICENSE("GPL");
-- 
2.40.1

Re: [PATCH 00/10] Support partitioned systems

[Hans de Goede]

Hi,

On 4/23/24 10:46 PM, Srinivas Pandruvada wrote:
> A partitioned system has two PCI device per package compared to only one.
> This doesn't change any TPMI functionality. Just while reading, different
> MMIO region is used for TPMI instances.
> 
> Some patches are not directly related to partitions, but added for easy
> application of series.
> 
> Patches are applied on top of origin/review-hans branch as of April 23,
> 2024.

Thank you for your patch-series, I've applied the series to my
review-hans branch:
https://git.kernel.org/pub/scm/linux/kernel/git/pdx86/platform-drivers-x86.git/log/?h=review-hans

Once I've run some tests on this branch the patches there will be
added to the platform-drivers-x86/for-next branch and eventually
will be included in the pdx86 pull-request to Linus for the next
merge-window.

Regards,

Hans



> 
> Srinivas Pandruvada (10):
>   platform/x86/intel/tpmi: Handle error from tpmi_process_info()
>   platform/x86/intel/tpmi: Check major version change for TPMI
>     Information
>   platform/x86/intel/tpmi: Align comments in kernel-doc
>   platform/x86/intel/tpmi: Add additional TPMI header fields
>   platform/x86: ISST: Use local variable for auxdev->dev
>   platform/x86: ISST: Shorten the assignments for power_domain_info
>   platform/x86: ISST: Support partitioned systems
>   platform/x86: ISST: Use in_range() to check package ID validity
>   platform/x86: ISST: Add dev_fmt
>   platform/x86: ISST: Add missing MODULE_DESCRIPTION
> 
>  .../intel/speed_select_if/isst_if_common.c    |   1 +
>  .../intel/speed_select_if/isst_tpmi_core.c    | 345 +++++++++++++++---
>  drivers/platform/x86/intel/tpmi.c             |  39 +-
>  include/linux/intel_tpmi.h                    |  12 +-
>  4 files changed, 333 insertions(+), 64 deletions(-)
>