Linux Power Management development
 help / color / mirror / Atom feed
From: Christian Loehle <christian.loehle@arm.com>
To: "Rafael J . Wysocki" <rafael@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org, Len Brown <lenb@kernel.org>,
	Jie Zhan <zhanjie9@hisilicon.com>,
	Lifeng Zheng <zhenglifeng1@huawei.com>,
	Pierre Gondois <pierre.gondois@arm.com>,
	Sumit Gupta <sumitg@nvidia.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Ionela Voinescu <ionela.voinescu@arm.com>,
	zhongqiu.han@oss.qualcomm.com,
	Christian Loehle <christian.loehle@arm.com>
Subject: [PATCH v6 13/15] ACPI: CPPC: Validate PCC overlaps across processors
Date: Sun, 30 Aug 2026 12:56:42 +0100	[thread overview]
Message-ID: <20260830115644.2056983-14-christian.loehle@arm.com> (raw)
In-Reply-To: <20260830115644.2056983-1-christian.loehle@arm.com>

PCC shared-memory offsets are physical within a subspace, but the existing
overlap check stops at one _CPC package. Two processors can therefore pass
probe with writable ranges which overlap in the same PCC subspace.

This is unsafe in the performance path, where CPUs may stage requests
concurrently while holding the shared side of pcc_lock. Partially
overlapping stores can construct a payload which belongs to neither request
before a doorbell submits it.

Index retained PCC byte ranges by subspace and physical interval. Permit
read-only overlap and exact aliases of the same logical CPPC entry. The
per-subspace payload lock serializes exact writable aliases, including
multi-byte fields copied with byte-oriented I/O. Reject every other overlap
involving a writable entry.

With that cross-processor protection in place, enable zero-offset,
byte-multiple writable PCC fields. Ordinary performance controls remain at
most 32 bits, while Performance Limited, CPPC Enable, and Autonomous
Selection Enable may use byte-multiple fields through 64 bits.

The interval tree keeps registration proportional to real overlaps rather
than to the number of processors. Remove entries when processor teardown
unpublishes its CPC descriptor, including every probe-failure path after
registration.

Fixes: 85b1407bf6d2 ("ACPI / CPPC: Make CPPC ACPI driver aware of PCC subspace IDs")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 193 ++++++++++++++++++++++++++++++++++++---
 include/acpi/cppc_acpi.h |   2 +
 2 files changed, 180 insertions(+), 15 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index beeae0a983b8..cc749a487373 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -113,6 +113,18 @@ struct cpc_sysmem_node {
 	bool registered;
 };
 
+struct cpc_non_mmio_node {
+	struct rb_node rb;
+	u64 subtree_last;
+	u64 start;
+	u64 last;
+	struct cpc_desc *desc;
+	unsigned int reg_idx;
+	u8 space_id;
+	u8 pcc_ss_id;
+	bool registered;
+};
+
 #define CPC_SYSMEM_START(node) ((node)->start)
 #define CPC_SYSMEM_LAST(node) ((node)->last)
 
@@ -123,6 +135,16 @@ INTERVAL_TREE_DEFINE(struct cpc_sysmem_node, rb, u64, subtree_last,
 static struct rb_root_cached cpc_sysmem_tree = RB_ROOT_CACHED;
 static DEFINE_MUTEX(cpc_sysmem_lock);
 
+#define CPC_NON_MMIO_START(node) ((node)->start)
+#define CPC_NON_MMIO_LAST(node) ((node)->last)
+
+INTERVAL_TREE_DEFINE(struct cpc_non_mmio_node, rb, u64, subtree_last,
+		     CPC_NON_MMIO_START, CPC_NON_MMIO_LAST, static inline,
+		     cpc_non_mmio_itree)
+
+static struct rb_root_cached cpc_pcc_trees[MAX_PCC_SUBSPACES];
+static DEFINE_MUTEX(cpc_non_mmio_lock);
+
 static struct cpc_sysmem_node *cpc_sysmem_first(u64 start, u64 last)
 {
 	return cpc_sysmem_itree_iter_first(&cpc_sysmem_tree, start, last);
@@ -284,15 +306,9 @@ show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time);
 
 static bool cpc_pcc_write_supported(const struct cpc_register_resource *reg)
 {
-	switch (GET_BIT_WIDTH(&reg->cpc_entry.reg)) {
-	case 8:
-	case 16:
-	case 32:
-	case 64:
-		return true;
-	default:
-		return false;
-	}
+	unsigned int width = reg->cpc_entry.reg.bit_width;
+
+	return width && width <= 32 && !(width % 8);
 }
 
 /* Shift and apply the mask for CPC reads/writes */
@@ -629,6 +645,150 @@ static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
 	return 0;
 }
 
+static struct rb_root_cached *cpc_non_mmio_tree(u8 space_id, u8 pcc_ss_id)
+{
+	if (space_id == ACPI_ADR_SPACE_PLATFORM_COMM)
+		return &cpc_pcc_trees[pcc_ss_id];
+	return NULL;
+}
+
+static int cpc_validate_non_mmio_pair(const struct cpc_non_mmio_node *a,
+				      const struct cpc_non_mmio_node *b)
+{
+	bool a_writable = cpc_reg_is_writable(a->reg_idx);
+	bool b_writable = cpc_reg_is_writable(b->reg_idx);
+	const char *name;
+
+	if (!a_writable && !b_writable)
+		return 0;
+
+	if (a->reg_idx == b->reg_idx && a->start == b->start &&
+	    a->last == b->last)
+		return 0;
+
+	name = "PCC";
+	pr_err("CPU%d: %s _CPC register %u conflicts with CPU%d register %u\n",
+	       a->desc->cpu_id, name, a->reg_idx, b->desc->cpu_id,
+	       b->reg_idx);
+	return -EINVAL;
+}
+
+static void cpc_unregister_non_mmio_desc_locked(struct cpc_desc *cpc_desc)
+{
+	unsigned int i;
+
+	if (!cpc_desc->non_mmio_nodes)
+		return;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		struct cpc_non_mmio_node *node = &cpc_desc->non_mmio_nodes[i];
+		struct rb_root_cached *tree;
+
+		if (!node->registered)
+			continue;
+
+		tree = cpc_non_mmio_tree(node->space_id, node->pcc_ss_id);
+		cpc_non_mmio_itree_remove(node, tree);
+	}
+
+	kfree(cpc_desc->non_mmio_nodes);
+	cpc_desc->non_mmio_nodes = NULL;
+}
+
+static int cpc_register_non_mmio_desc(struct cpc_desc *cpc_desc,
+				      int pcc_ss_id)
+{
+	unsigned int nr_regs = cpc_desc->num_entries - 2;
+	unsigned int i;
+	int ret = 0;
+	bool found = false;
+
+	for (i = 0; i < nr_regs; i++) {
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
+		u8 space_id;
+
+		if (!CPC_SUPPORTED(reg) || reg->type != ACPI_TYPE_BUFFER)
+			continue;
+		space_id = reg->cpc_entry.reg.space_id;
+		if (space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
+			found = true;
+			break;
+		}
+	}
+	if (!found)
+		return 0;
+
+	cpc_desc->non_mmio_nodes = kcalloc(nr_regs,
+					   sizeof(*cpc_desc->non_mmio_nodes),
+					   GFP_KERNEL);
+	if (!cpc_desc->non_mmio_nodes)
+		return -ENOMEM;
+
+	mutex_lock(&cpc_non_mmio_lock);
+
+	for (i = 0; i < nr_regs; i++) {
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
+		struct cpc_non_mmio_node *match, *node;
+		struct rb_root_cached *tree;
+		u8 space_id;
+		u64 size;
+
+		if (!CPC_SUPPORTED(reg) || reg->type != ACPI_TYPE_BUFFER)
+			continue;
+
+		space_id = reg->cpc_entry.reg.space_id;
+		if (space_id != ACPI_ADR_SPACE_PLATFORM_COMM)
+			continue;
+
+		if (pcc_ss_id < 0) {
+			ret = -EINVAL;
+			goto out_unregister;
+		}
+
+		node = &cpc_desc->non_mmio_nodes[i];
+		size = cpc_non_mmio_access_size(reg);
+		node->start = reg->cpc_entry.reg.address;
+		node->last = node->start + size - 1;
+		node->desc = cpc_desc;
+		node->reg_idx = i;
+		node->space_id = space_id;
+		node->pcc_ss_id = pcc_ss_id;
+		tree = cpc_non_mmio_tree(space_id, node->pcc_ss_id);
+
+		match = cpc_non_mmio_itree_iter_first(tree, node->start,
+						      node->last);
+		while (match) {
+			ret = cpc_validate_non_mmio_pair(node, match);
+			if (ret)
+				goto out_unregister;
+
+			match = cpc_non_mmio_itree_iter_next(match, node->start,
+							     node->last);
+		}
+
+		cpc_non_mmio_itree_insert(node, tree);
+		node->registered = true;
+	}
+
+	mutex_unlock(&cpc_non_mmio_lock);
+	return 0;
+
+out_unregister:
+	cpc_unregister_non_mmio_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_non_mmio_lock);
+	return ret;
+}
+
+static void cpc_unregister_non_mmio_desc(struct cpc_desc *cpc_desc)
+{
+	if (!cpc_desc->non_mmio_nodes)
+		return;
+
+	mutex_lock(&cpc_non_mmio_lock);
+	cpc_unregister_non_mmio_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_non_mmio_lock);
+}
+
 static void cpc_mark_rmw_lock_users(struct cpc_desc *cpc_desc)
 {
 	int i;
@@ -958,6 +1118,7 @@ static void cppc_free_desc(struct cpc_desc *cpc_ptr)
 {
 	unsigned int i;
 
+	cpc_unregister_non_mmio_desc(cpc_ptr);
 	cpc_unregister_sysmem_desc(cpc_ptr);
 
 	for (i = 2; i < cpc_ptr->num_entries; i++) {
@@ -1672,16 +1833,12 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 				bool wide_write = i - 2 == PERF_LIMITED ||
 						  i - 2 == ENABLE ||
 						  i - 2 == AUTO_SEL_ENABLE;
-				bool write_width_supported = gas_t->bit_width == 8 ||
-						     gas_t->bit_width == 16 ||
-						     gas_t->bit_width == 32 ||
-						     gas_t->bit_width == 64;
 
 				if (!gas_t->bit_width || gas_t->bit_width > 64 ||
 				    gas_t->bit_offset || gas_t->bit_width % 8 ||
 				    (cpc_reg_is_writable(i - 2) &&
-				     (!write_width_supported ||
-				      (!wide_write && gas_t->bit_width > 32)))) {
+				     !wide_write &&
+				     gas_t->bit_width > 32)) {
 					unsupported_regs |= BIT(i - 2);
 					continue;
 				}
@@ -1865,6 +2022,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	if (ret)
 		goto out_free;
 
+	ret = cpc_register_non_mmio_desc(cpc_ptr, pcc_subspace_id);
+	if (ret)
+		goto out_free;
+
 	/* Everything looks okay */
 	pr_debug("Parsed CPC struct for CPU: %d\n", pr->id);
 
@@ -1882,6 +2043,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			"acpi_cppc");
 	if (ret) {
 		per_cpu(cpc_desc_ptr, pr->id) = NULL;
+		cpc_unregister_non_mmio_desc(cpc_ptr);
 		cpc_unregister_sysmem_desc(cpc_ptr);
 		kobject_put(&cpc_ptr->kobj);
 		goto out_pcc_put;
@@ -1926,6 +2088,7 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
 	pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
 	per_cpu(cpc_desc_ptr, pr->id) = NULL;
 	kobject_del(&cpc_ptr->kobj);
+	cpc_unregister_non_mmio_desc(cpc_ptr);
 	cpc_unregister_sysmem_desc(cpc_ptr);
 
 	pcc_data_put(pcc_ss_id);
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 4e5f59bc95f8..be22504eddce 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -80,6 +80,7 @@ struct cpc_register_resource {
 };
 
 struct cpc_sysmem_node;
+struct cpc_non_mmio_node;
 
 /* Container to hold the CPC details for each CPU */
 struct cpc_desc {
@@ -93,6 +94,7 @@ struct cpc_desc {
 	struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
 	struct acpi_psd_package domain_info;
 	struct cpc_sysmem_node *sysmem_nodes;
+	struct cpc_non_mmio_node *non_mmio_nodes;
 	struct kobject kobj;
 };
 
-- 
2.34.1


  parent reply	other threads:[~2026-08-30 11:58 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-30 11:56 [PATCH v6 0/15] ACPI: CPPC: Fix register access and lifetime bugs Christian Loehle
2026-08-30 11:56 ` [PATCH v6 01/15] ACPI: CPPC: Validate the _CPC package header Christian Loehle
2026-08-30 11:56 ` [PATCH v6 02/15] ACPI: CPPC: Validate _CPC entry and control semantics Christian Loehle
2026-08-30 13:32   ` Christian Loehle
2026-09-03 19:27     ` Rafael J. Wysocki (Intel)
2026-09-03 19:44       ` Rafael J. Wysocki (Intel)
2026-09-03 20:02         ` Christian Loehle
2026-08-30 11:56 ` [PATCH v6 03/15] ACPI: CPPC: Propagate performance-control write errors Christian Loehle
2026-08-30 11:56 ` [PATCH v6 04/15] ACPI: CPPC: Use 64-bit masks for register fields Christian Loehle
2026-08-30 11:56 ` [PATCH v6 05/15] ACPI: CPPC: Serialize PCC single-register payload updates Christian Loehle
2026-08-30 11:56 ` [PATCH v6 06/15] ACPI: CPPC: Serialize PCC EPP " Christian Loehle
2026-08-30 11:56 ` [PATCH v6 07/15] ACPI: CPPC: Release CPC descriptors through kobject Christian Loehle
2026-08-30 11:56 ` [PATCH v6 08/15] ACPI: CPPC: Release PCC data after probe failures Christian Loehle
2026-08-30 11:56 ` [PATCH v6 09/15] ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW Christian Loehle
2026-08-30 11:56 ` [PATCH v6 10/15] ACPI: CPPC: Reject direct reads of write-only controls Christian Loehle
2026-08-30 11:56 ` [PATCH v6 11/15] ACPI: CPPC: Validate and access PCC register layouts Christian Loehle
2026-08-30 11:56 ` [PATCH v6 12/15] ACPI: CPPC: Validate SystemIO " Christian Loehle
2026-08-30 11:56 ` Christian Loehle [this message]
2026-08-30 11:56 ` [PATCH v6 14/15] ACPI: CPPC: Validate SystemIO overlaps across processors Christian Loehle
2026-08-30 11:56 ` [PATCH v6 15/15] ACPI: CPPC: Clear Performance Limited without a stale read Christian Loehle
2026-09-01  6:24 ` [PATCH v6 0/15] ACPI: CPPC: Fix register access and lifetime bugs Christian Loehle
2026-09-01 20:10   ` Mario Limonciello

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260830115644.2056983-14-christian.loehle@arm.com \
    --to=christian.loehle@arm.com \
    --cc=ionela.voinescu@arm.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pierre.gondois@arm.com \
    --cc=rafael@kernel.org \
    --cc=sudeep.holla@arm.com \
    --cc=sumitg@nvidia.com \
    --cc=viresh.kumar@linaro.org \
    --cc=zhanjie9@hisilicon.com \
    --cc=zhenglifeng1@huawei.com \
    --cc=zhongqiu.han@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox