[PATCH v6 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Christian Loehle]

The Performance Limited status bits are sticky and write-zero-to-clear.
ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
accesses.

cppc_set_perf_limited() currently reads the register, computes a new value,
and writes it in a separate transaction. If the platform reports another
excursion between those transactions, the stale write can clear that new
event.

Write zero to the requested bits and one to the other defined status bits
directly. Keep reserved bits zero as required for hardware status registers
by ACPI 6.6 Section 4.6.1. This removes the stale read window.

A partial SystemMemory field would still make the generic writer perform a
read-modify-write to preserve the containing access unit. The
per-descriptor spinlock cannot interlock that RMW with platform updates, so
reject clears of such a field. Keep the descriptor mapped and readable,
because reading the containing access unit once and extracting the field
does not require RMW.

Classify a field as a writer during overlap validation only when its _CPC
semantics permit writes and its validated resource remains writable. This
allows partial Performance Limited fields whose clear path was disabled to
share an access unit with other read-only fields, while still rejecting an
actual writer in that access unit.

Also reject another writable SystemMemory field sharing Performance
Limited's access unit. Its RMW could similarly replay stale status bits,
and an OSPM lock cannot serialize against the platform.

Also reject 64-bit SystemMemory descriptions on 32-bit kernels, where
generic readq()/writeq() may be split into two 32-bit operations and cannot
provide the required portable interlocked access. A naturally aligned
full-width QWord remains supported on 64-bit kernels, where the
architecture provides a native 64-bit MMIO accessor.

Retain any inaccessible Performance Limited descriptor whose conservative
physical range is still locatable, while marking both reads and writes
unsupported. This includes a QWord on a 32-bit kernel. Skip its mapping and
the flexible-address-space capability gate, because Linux will issue no
access, without hiding the asynchronous status range from
neighbouring-writer validation. Both the interval registry and pairwise
overlap test use the larger of the access unit and logical field span, so a
malformed field extending beyond its nominal access unit remains covered.

Performance Limited status is not required for CPPC control. If firmware
describes it without even a locatable physical range, disable that status
register instead of rejecting the processor's otherwise usable _CPC
package. Report reads as unsupported rather than returning a synthetic
zero, and emit a single warning for each nonfatal fallback.

Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
---
 drivers/acpi/cppc_acpi.c | 76 +++++++++++++++++++++++++++++-----------
 1 file changed, 56 insertions(+), 20 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 25bccd4cfb34..a07440ed7f80 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -443,6 +443,22 @@ static int cpc_validate_sysmem_reg(struct cpc_desc *cpc_desc,
 	if (!cpc_reg_access_aligned(gas, access_size))
 		goto invalid;
 
+	if (reg_idx == PERF_LIMITED) {
+		if (access_width == 64 && !IS_ENABLED(CONFIG_64BIT)) {
+			pr_warn("CPU%d: Performance Limited register cannot be accessed atomically; keeping its range reserved\n",
+				cpc_desc->cpu_id);
+			cpc_desc->cpc_regs[reg_idx].cpc_entry.read_unsupported = true;
+			cpc_desc->cpc_regs[reg_idx].cpc_entry.write_unsupported = true;
+			return 0;
+		}
+
+		if (gas->bit_offset || gas->bit_width != access_width) {
+			pr_warn("CPU%d: Performance Limited register cannot be cleared safely; keeping it readable\n",
+				cpc_desc->cpu_id);
+			cpc_desc->cpc_regs[reg_idx].cpc_entry.write_unsupported = true;
+		}
+	}
+
 	return 0;
 
 invalid:
@@ -452,8 +468,9 @@ static int cpc_validate_sysmem_reg(struct cpc_desc *cpc_desc,
 		access_size = access_width / 8;
 	if (gas->bit_width)
 		access_size = max(access_size, cpc_sysmem_field_size(gas));
-	if (cpc_reg_is_write_only(cpc_desc, reg_idx) && gas->address &&
-	    access_size && gas->address <= U64_MAX - (access_size - 1)) {
+	if ((cpc_reg_is_write_only(cpc_desc, reg_idx) ||
+	     reg_idx == PERF_LIMITED) && gas->address && access_size &&
+	    gas->address <= U64_MAX - (access_size - 1)) {
 		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[reg_idx];
 
 		pr_warn("CPU%d: _CPC v%d register %u is inaccessible; keeping its range reserved\n",
@@ -502,6 +519,14 @@ static int cpc_resolve_unsupported(struct cpc_desc *cpc_desc,
 		if (!(unsupported & BIT(i)))
 			continue;
 
+		/* CPPC control does not depend on Performance Limited status. */
+		if (i == PERF_LIMITED) {
+			pr_warn("CPU%d: ignoring inaccessible Performance Limited register\n",
+				cpc_desc->cpu_id);
+			cpc_disable_reg(cpc_desc, i);
+			continue;
+		}
+
 		if (i == DESIRED_PERF && cpc_immutable_autonomous(cpc_desc)) {
 			pr_warn("CPU%d: ignoring inaccessible Desired Performance register in autonomous mode\n",
 				cpc_desc->cpu_id);
@@ -803,7 +828,8 @@ static void cpc_mark_rmw_lock_users(struct cpc_desc *cpc_desc)
 	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
 		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
 
-		if (CPC_SUPPORTED(reg) && CPC_IN_SYSTEM_MEMORY(reg))
+		if (CPC_SUPPORTED(reg) && CPC_IN_SYSTEM_MEMORY(reg) &&
+		    cpc_is_writable(reg))
 			reg->cpc_entry.use_rmw_lock =
 				cpc_sysmem_reg_needs_rmw(reg);
 	}
@@ -905,6 +931,7 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
 	const struct cpc_register_resource *b = &b_desc->cpc_regs[b_idx];
 	bool a_writable, b_writable;
 
+	/* The overlap helper includes each descriptor's conservative claim. */
 	if (!CPC_SUPPORTED(a) || !CPC_IN_SYSTEM_MEMORY(a) ||
 	    !CPC_SUPPORTED(b) || !CPC_IN_SYSTEM_MEMORY(b) ||
 	    !cpc_sysmem_access_units_overlap(a, b))
@@ -933,6 +960,15 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
 		goto conflict;
 	}
 
+	/*
+	 * The platform may set Performance Limited asynchronously.  A write to
+	 * another field in the same access unit could write back stale status
+	 * bits, which an OSPM lock cannot prevent.
+	 */
+	if ((a_idx == PERF_LIMITED && b_writable) ||
+	    (b_idx == PERF_LIMITED && a_writable))
+		goto conflict;
+
 	/* A full-width writable register owns its complete access unit. */
 	if ((a_writable && !cpc_sysmem_reg_needs_rmw(a) &&
 	     cpc_sysmem_full_width_conflicts(a, b)) ||
@@ -2167,6 +2203,9 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
 	struct cpc_reg *reg = &reg_res->cpc_entry.reg;
 
+	if (!cpc_is_readable(reg_res))
+		return -EOPNOTSUPP;
+
 	if (reg_res->type == ACPI_TYPE_INTEGER) {
 		*val = reg_res->cpc_entry.int_value;
 		return 0;
@@ -2268,13 +2307,10 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	unsigned int i;
 	bool locked = false;
 
-	if (reg_res->type != ACPI_TYPE_BUFFER)
+	if (!cpc_is_writable(reg_res))
 		return -EOPNOTSUPP;
 
 	reg = &reg_res->cpc_entry.reg;
-	if (IS_NULL_REG(reg))
-		return -EOPNOTSUPP;
-
 	size = GET_BIT_WIDTH(reg);
 
 	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
@@ -2438,9 +2474,13 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val)
 
 	reg = &cpc_desc->cpc_regs[reg_idx];
 
-	/* Desired may be absent for immutable autonomous selection. */
+	/*
+	 * Desired and Performance Limited may be disabled despite not being
+	 * generally optional.
+	 */
 	if ((reg->type == ACPI_TYPE_INTEGER &&
-	     (IS_OPTIONAL_CPC_REG(reg_idx) || reg_idx == DESIRED_PERF) &&
+	     (IS_OPTIONAL_CPC_REG(reg_idx) || reg_idx == DESIRED_PERF ||
+	      reg_idx == PERF_LIMITED) &&
 	     !reg->cpc_entry.int_value) || (reg->type != ACPI_TYPE_INTEGER &&
 	     IS_NULL_REG(&reg->cpc_entry.reg))) {
 		pr_debug("CPC register is not supported\n");
@@ -3468,9 +3508,6 @@ EXPORT_SYMBOL_GPL(cppc_get_perf_limited);
  */
 int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
 {
-	u64 current_val, new_val;
-	int ret;
-
 	/* Only bits 0 and 1 are valid */
 	if (bits_to_clear & ~CPPC_PERF_LIMITED_MASK)
 		return -EINVAL;
@@ -3478,14 +3515,13 @@ int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
 	if (!bits_to_clear)
 		return 0;
 
-	ret = cppc_get_perf_limited(cpu, &current_val);
-	if (ret)
-		return ret;
-
-	/* Clear the specified bits */
-	new_val = current_val & ~bits_to_clear;
-
-	return cppc_set_reg_val(cpu, PERF_LIMITED, new_val);
+	/*
+	 * Performance Limited is write-zero-to-clear.  Write one to the other
+	 * defined sticky bits so a concurrently reported event is not cleared
+	 * using a value obtained by an earlier, separate read transaction.
+	 */
+	return cppc_set_reg_val(cpu, PERF_LIMITED,
+				CPPC_PERF_LIMITED_MASK & ~bits_to_clear);
 }
 EXPORT_SYMBOL_GPL(cppc_set_perf_limited);
 
-- 
2.34.1