[PATCH v7 04/20] ACPI: CPPC: Serialize PCC single-register payload updates

[Christian Loehle]

The PCC doorbell protocol requires OSPM to confirm ownership of the shared
subspace before placing a command and its payload there.
cppc_set_reg_val_in_pcc() instead modifies the payload before taking
pcc_lock.

A concurrent command can consequently overwrite or consume the staged
value, and OSPM can write the shared region while the platform still owns
it.

Reject a register width which cpc_write() cannot stage before taking the
exclusive lock. Otherwise that operation can steal Phase II from an older
valid performance batch and then abort it for an unrelated error.

Take the PCC write lock first, wait for the previous command to complete,
and keep the lock held while staging the value and submitting CMD_WRITE.
This follows the ownership sequence in ACPI 6.5 Section 14.5 and the
existing contract documented by send_pcc_cmd().

If ownership acquisition or staging fails, abort any older performance
batch before dropping the exclusive lock. This advances its generation and
wakes cppc_set_perf() callers which otherwise wait indefinitely for a
command this path did not submit.

The generation counter is local to each PCC subspace, while error
completion scans the descriptors of every CPU. Match both the subspace and
generation so failure in one subspace cannot poison a request using the
same generation in another.

Fixes: e05c75072c2e ("ACPI: CPPC: Add cppc_set_reg_val()")
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Link: https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
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 | 44 ++++++++++++++++++++++++++++++++--------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 74feb05195a1..42f3714f82ab 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -393,8 +393,8 @@ static int check_pcc_chan(int pcc_ss_id, bool chk_err_bit)
 	return ret;
 }
 
-static void cppc_complete_pcc_write(struct cppc_pcc_data *pcc_ss_data,
-				    int ret)
+static void cppc_complete_pcc_write(int pcc_ss_id,
+				    struct cppc_pcc_data *pcc_ss_data, int ret)
 {
 	int i;
 
@@ -402,7 +402,8 @@ static void cppc_complete_pcc_write(struct cppc_pcc_data *pcc_ss_data,
 		for_each_possible_cpu(i) {
 			struct cpc_desc *desc = per_cpu(cpc_desc_ptr, i);
 
-			if (!desc)
+			if (!desc ||
+			    per_cpu(cpu_pcc_subspace_idx, i) != pcc_ss_id)
 				continue;
 
 			if (desc->write_cmd_id == pcc_ss_data->pcc_write_cnt)
@@ -414,6 +415,18 @@ static void cppc_complete_pcc_write(struct cppc_pcc_data *pcc_ss_data,
 	wake_up_all(&pcc_ss_data->pcc_write_wait_q);
 }
 
+/* The caller must hold pcc_lock for write. */
+static void cppc_abort_pending_pcc_write(int pcc_ss_id,
+					 struct cppc_pcc_data *pcc_ss_data,
+					 int ret)
+{
+	if (!pcc_ss_data->pending_pcc_write_cmd)
+		return;
+
+	pcc_ss_data->pending_pcc_write_cmd = false;
+	cppc_complete_pcc_write(pcc_ss_id, pcc_ss_data, ret);
+}
+
 /*
  * This function transfers the ownership of the PCC to the platform
  * So it must be called while holding write_lock(pcc_lock)
@@ -513,7 +526,7 @@ static int send_pcc_cmd(int pcc_ss_id, u16 cmd)
 
 end:
 	if (cmd == CMD_WRITE)
-		cppc_complete_pcc_write(pcc_ss_data, ret);
+		cppc_complete_pcc_write(pcc_ss_id, pcc_ss_data, ret);
 
 	return ret;
 }
@@ -1520,23 +1533,36 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val)
 static int cppc_set_reg_val_in_pcc(int cpu, struct cpc_register_resource *reg, u64 val)
 {
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cppc_pcc_data *pcc_ss_data = NULL;
+	struct cppc_pcc_data *pcc_ss_data;
 	int ret;
 
 	if (pcc_ss_id < 0) {
 		pr_debug("Invalid pcc_ss_id\n");
 		return -ENODEV;
 	}
-
-	ret = cpc_write(cpu, reg, val);
-	if (ret)
-		return ret;
+	if (!cpc_pcc_write_supported(reg))
+		return -EFAULT;
 
 	pcc_ss_data = pcc_data[pcc_ss_id];
+	if (!pcc_ss_data)
+		return -ENODEV;
 
 	down_write(&pcc_ss_data->pcc_lock);
+
+	ret = check_pcc_chan(pcc_ss_id, false);
+	if (ret)
+		goto out;
+
+	ret = cpc_write(cpu, reg, val);
+	if (ret)
+		goto out;
+
 	/* after writing CPC, transfer the ownership of PCC to platform */
 	ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
+
+out:
+	if (ret)
+		cppc_abort_pending_pcc_write(pcc_ss_id, pcc_ss_data, ret);
 	up_write(&pcc_ss_data->pcc_lock);
 
 	return ret;
-- 
2.34.1