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

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.

Reject SystemMemory descriptions which require read-modify-write to
preserve the containing access unit, because the per-descriptor spinlock
cannot interlock that RMW with platform updates. 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.

Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
Signed-off-by: Christian Loehle <[email protected]>
---
 drivers/acpi/cppc_acpi.c | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 621b190aea3c..4f55607fc116 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -406,6 +406,13 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
 		       cpc_desc->cpu_id, cpc_desc->version, name);
 		return -EINVAL;
 	}
+	if (reg_idx == PERF_LIMITED &&
+	    (gas->bit_offset || gas->bit_width != access_width ||
+	     (access_width == 64 && !IS_ENABLED(CONFIG_64BIT)))) {
+		pr_err("CPU%d: Performance Limited register cannot use an interlocked SystemMemory access\n",
+		       cpc_desc->cpu_id);
+		return -EINVAL;
+	}
 
 	return 0;
 
@@ -3054,9 +3061,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;
@@ -3064,14 +3068,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