[PATCH v6 13/15] ACPI: CPPC: Validate PCC overlaps across processors

[Christian Loehle]

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@xxxxxxx>
---
 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