[PATCH v6 02/15] ACPI: CPPC: Validate _CPC entry and control semantics

[Christian Loehle]

Writable _CPC controls are Register descriptors encoded as Buffer objects.
Integer entries represent fixed values or unsupported optional registers;
Autonomous Selection Integer 1 is the special immutable form which enables
operation without Desired Performance.

The parser accepts arbitrary object types and cpc_write() assumes that its
argument contains a GAS. Malformed firmware can therefore make it interpret
an Integer union member as a register.

Validate the portion of each encoding consumed by the driver: bound Integer
DWORD forms to 32 bits, and require Buffer entries to start with a complete
Generic Register descriptor with the expected header. Continue tolerating
Integer 0 for an absent optional register and retain type checks in
cpc_write() as defense in depth. Reject an attempt to disable immutable
Autonomous Selection instead of silently applying only the EPP part of the
request.

Capability registers are read into u64 temporaries but exposed through u32
fields. Reject values above U32_MAX instead of allowing them to be
truncated. In particular, a truncated Highest Performance value can become
a zero divisor in the performance-to-frequency conversion. Enforce the
required ordering from Highest through Nominal, Lowest Nonlinear, and
Lowest Performance, and constrain a present Guaranteed Performance to the
inclusive Lowest-to-Nominal range. Also reject reversed frequency anchors
and unequal frequency anchors with identical performance anchors. Those
invalid tuples otherwise make affine-conversion differences wrap or divide
by zero.

Check mandatory object presence separately from the Integer-zero convention
for absent optional fields. ACPI does not reserve zero in the abstract
Lowest Performance scale, so accept a present Lowest Performance DWORD of
zero when distinct frequency anchors provide a usable nonzero physical
minimum. Retain the old rejection when that mapping is unavailable and the
fallback conversion would expose a 0 kHz cpufreq endpoint.

Minimum Performance also defines zero as a real no-limit value, but the
exported cppc_set_perf() interface historically used zero to omit a bound.
Add explicit validity flags so callers can request zero without changing
that legacy convention. Populate the flags when reading the controls and
mark the bounds supplied by amd-pstate explicitly.

Performance Limited is listed as a required Buffer, but the interface does
not depend on it to control performance and the specification permits a
platform with no limiting indication to always report zero. Preserve
the compatibility with firmware that represents that case using a NULL
register descriptor instead of disabling CPPC entirely.

Emit an error when a present _CPC package fails parsing or initialization
so such firmware and resource failures no longer silently suppress cpufreq.
Initialize malformed-package failures to -EINVAL and preserve specific
allocation, mapping, and unsupported-access errors in that diagnostic.

Fixes: 337aadff8e45 ("ACPI: Introduce CPU performance controls using CPPC")
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Link: https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
---
 drivers/acpi/cppc_acpi.c     | 172 ++++++++++++++++++++++++++++++-----
 drivers/cpufreq/amd-pstate.c |  12 ++-
 include/acpi/cppc_acpi.h     |   2 +
 3 files changed, 159 insertions(+), 27 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 3b8cdf88e31d..6f3ffa4a1845 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -129,6 +129,21 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
 				!!(cpc)->cpc_entry.int_value :		\
 				!IS_NULL_REG(&(cpc)->cpc_entry.reg))
 
+static bool cpc_is_writable(const struct cpc_register_resource *cpc)
+{
+	return cpc->type == ACPI_TYPE_BUFFER &&
+	       !IS_NULL_REG(&cpc->cpc_entry.reg);
+}
+
+static bool cpc_entry_present(const struct cpc_register_resource *cpc)
+{
+	if (cpc->type == ACPI_TYPE_INTEGER)
+		return true;
+
+	return cpc->type == ACPI_TYPE_BUFFER &&
+	       !IS_NULL_REG(&cpc->cpc_entry.reg);
+}
+
 /*
  * Each bit indicates the optionality of the register in per-cpu
  * cpc_regs[] with the corresponding index. 0 means mandatory and 1
@@ -142,6 +157,29 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
  */
 #define IS_OPTIONAL_CPC_REG(reg_idx) (REG_OPTIONAL & (1U << (reg_idx)))
 
+static bool cpc_integer_entry_valid(unsigned int reg_idx, u64 value)
+{
+	switch (reg_idx) {
+	case HIGHEST_PERF:
+	case NOMINAL_PERF:
+	case LOW_NON_LINEAR_PERF:
+	case LOWEST_PERF:
+	case CTR_WRAP_TIME:
+	case REFERENCE_PERF:
+	case LOWEST_FREQ:
+	case NOMINAL_FREQ:
+		return value <= U32_MAX;
+	case AUTO_SEL_ENABLE:
+		return value <= 1;
+	case DESIRED_PERF:
+		/* Validated against Autonomous Selection after parsing. */
+		return value == 0;
+	default:
+		/* Tolerate the customary Integer 0 for an absent option. */
+		return value == 0 && IS_OPTIONAL_CPC_REG(reg_idx);
+	}
+}
+
 /*
  * Arbitrary Retries in case the remote processor is slow to respond
  * to PCC commands. Keeping it high enough to cover emulators where
@@ -150,6 +188,8 @@ static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
 #define NUM_RETRIES 500ULL
 
 #define OVER_16BTS_MASK ~0xFFFFULL
+#define CPC_GENERIC_REGISTER_DESCRIPTOR 0x82
+#define CPC_GENERIC_REGISTER_LENGTH (sizeof(struct cpc_reg) - 3)
 
 #define define_one_cppc_ro(_name)		\
 static struct kobj_attribute _name =		\
@@ -773,8 +813,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	acpi_handle handle = pr->handle;
 	unsigned int num_ent, i, cpc_rev;
 	int pcc_subspace_id = -1;
+	bool cpc_present = false;
 	acpi_status status;
 	int ret = -ENODATA;
+	int err;
 
 	if (!osc_sb_cppc2_support_acked) {
 		pr_debug("CPPC v2 _OSC not acked\n");
@@ -791,6 +833,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 		ret = -ENODEV;
 		goto out_buf_free;
 	}
+	cpc_present = true;
+	ret = -EINVAL;
 
 	out_obj = (union acpi_object *) output.pointer;
 	if (out_obj->package.count < 2) {
@@ -871,11 +915,32 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 		cpc_obj = &out_obj->package.elements[i];
 
 		if (cpc_obj->type == ACPI_TYPE_INTEGER)	{
-			cpc_ptr->cpc_regs[i-2].type = ACPI_TYPE_INTEGER;
-			cpc_ptr->cpc_regs[i-2].cpc_entry.int_value = cpc_obj->integer.value;
+			if (!cpc_integer_entry_valid(i - 2,
+						     cpc_obj->integer.value)) {
+				pr_debug("Invalid Integer _CPC register %u for CPU:%d\n",
+					 i - 2, pr->id);
+				ret = -EINVAL;
+				goto out_free;
+			}
+			cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_INTEGER;
+			cpc_ptr->cpc_regs[i - 2].cpc_entry.int_value = cpc_obj->integer.value;
 		} else if (cpc_obj->type == ACPI_TYPE_BUFFER) {
+			if (cpc_obj->buffer.length < sizeof(*gas_t)) {
+				pr_debug("Invalid register descriptor for CPU:%d\n",
+					 pr->id);
+				ret = -EINVAL;
+				goto out_free;
+			}
+
 			gas_t = (struct cpc_reg *)
 				cpc_obj->buffer.pointer;
+			if (gas_t->descriptor != CPC_GENERIC_REGISTER_DESCRIPTOR ||
+			    gas_t->length != CPC_GENERIC_REGISTER_LENGTH) {
+				pr_debug("Invalid register resource for CPU:%d\n",
+					 pr->id);
+				ret = -EINVAL;
+				goto out_free;
+			}
 
 			/*
 			 * The PCC Subspace index is encoded inside
@@ -886,8 +951,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			if (gas_t->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
 				if (pcc_subspace_id < 0) {
 					pcc_subspace_id = gas_t->access_width;
-					if (pcc_data_alloc(pcc_subspace_id))
+					err = pcc_data_alloc(pcc_subspace_id);
+					if (err) {
+						ret = err;
 						goto out_free;
+					}
 				} else if (pcc_subspace_id != gas_t->access_width) {
 					pr_debug("Mismatched PCC ids in _CPC for CPU:%d\n",
 						 pr->id);
@@ -900,14 +968,18 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
 					if (!osc_cpc_flexible_adr_space_confirmed) {
 						pr_debug("Flexible address space capability not supported\n");
+						ret = -EOPNOTSUPP;
 						if (!cpc_supported_by_cpu())
 							goto out_free;
+						ret = -EINVAL;
 					}
 
 					access_width = GET_BIT_WIDTH(gas_t) / 8;
 					addr = ioremap(gas_t->address, access_width);
-					if (!addr)
+					if (!addr) {
+						ret = -ENOMEM;
 						goto out_free;
+					}
 					cpc_ptr->cpc_regs[i-2].sys_mem_vaddr = addr;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
@@ -929,14 +1001,17 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 				}
 				if (!osc_cpc_flexible_adr_space_confirmed) {
 					pr_debug("Flexible address space capability not supported\n");
+					ret = -EOPNOTSUPP;
 					if (!cpc_supported_by_cpu())
 						goto out_free;
+					ret = -EINVAL;
 				}
 			} else {
 				if (gas_t->space_id != ACPI_ADR_SPACE_FIXED_HARDWARE || !cpc_ffh_supported()) {
 					/* Support only PCC, SystemMemory, SystemIO, and FFH type regs. */
 					pr_debug("Unsupported register type (%d) in _CPC\n",
 						 gas_t->space_id);
+					ret = -EOPNOTSUPP;
 					goto out_free;
 				}
 			}
@@ -961,15 +1036,35 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	}
 	per_cpu(cpu_pcc_subspace_idx, pr->id) = pcc_subspace_id;
 
+	/*
+	 * Performance Limited is required by the specification, but tolerate a
+	 * NULL descriptor used by firmware which cannot report limiting events.
+	 * CPPC control does not depend on this status.
+	 */
+	for (i = 0; i < num_ent - 2; i++) {
+		if (i != DESIRED_PERF && i != PERF_LIMITED &&
+		    !IS_OPTIONAL_CPC_REG(i) &&
+		    !cpc_entry_present(&cpc_ptr->cpc_regs[i])) {
+			pr_debug("CPU:%d lacks mandatory _CPC register %u\n",
+				 pr->id, i);
+			ret = -EINVAL;
+			goto out_free;
+		}
+	}
+
 	/*
 	 * In CPPC v1, DESIRED_PERF is mandatory. In CPPC v2, it is optional
 	 * only when AUTO_SEL_ENABLE is supported.
 	 */
-	if (!CPC_SUPPORTED(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
+	if (!cpc_is_writable(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
 	    (!osc_sb_cppc2_support_acked ||
-	     !CPC_SUPPORTED(&cpc_ptr->cpc_regs[AUTO_SEL_ENABLE])))
-		pr_warn("Desired perf. register is mandatory if CPPC v2 is not supported "
-			"or autonomous selection is disabled\n");
+	     cpc_ptr->cpc_regs[AUTO_SEL_ENABLE].type != ACPI_TYPE_INTEGER ||
+	     cpc_ptr->cpc_regs[AUTO_SEL_ENABLE].cpc_entry.int_value != 1)) {
+		pr_debug("CPU:%d lacks a writable Desired Performance register\n",
+			 pr->id);
+		ret = -EINVAL;
+		goto out_free;
+	}
 
 	/*
 	 * Initialize the remaining cpc_regs as unsupported.
@@ -1037,6 +1132,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	kfree(cpc_ptr);
 
 out_buf_free:
+	if (cpc_present)
+		pr_err("CPU%d: failed to initialize _CPC: %d\n", pr->id, ret);
 	kfree(output.pointer);
 	return ret;
 }
@@ -1217,11 +1314,18 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	u64 prev_val;
 	void __iomem *vaddr = NULL;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
-	struct cpc_reg *reg = &reg_res->cpc_entry.reg;
+	struct cpc_reg *reg;
 	struct cpc_desc *cpc_desc;
 	unsigned long flags;
 	bool locked = false;
 
+	if (reg_res->type != ACPI_TYPE_BUFFER)
+		return -EOPNOTSUPP;
+
+	reg = &reg_res->cpc_entry.reg;
+	if (IS_NULL_REG(reg))
+		return -EOPNOTSUPP;
+
 	size = GET_BIT_WIDTH(reg);
 
 	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
@@ -1364,7 +1468,9 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val)
 
 	reg = &cpc_desc->cpc_regs[reg_idx];
 
-	if ((reg->type == ACPI_TYPE_INTEGER && IS_OPTIONAL_CPC_REG(reg_idx) &&
+	/* Desired may be absent for immutable autonomous selection. */
+	if ((reg->type == ACPI_TYPE_INTEGER &&
+	     (IS_OPTIONAL_CPC_REG(reg_idx) || reg_idx == DESIRED_PERF) &&
 	     !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");
@@ -1415,7 +1521,7 @@ static int cppc_set_reg_val(int cpu, enum cppc_regs reg_idx, u64 val)
 	reg = &cpc_desc->cpc_regs[reg_idx];
 
 	/* if a register is writeable, it must be a buffer and not null */
-	if ((reg->type != ACPI_TYPE_BUFFER) || IS_NULL_REG(&reg->cpc_entry.reg)) {
+	if (!cpc_is_writable(reg)) {
 		pr_debug("CPC register is not supported\n");
 		return -EOPNOTSUPP;
 	}
@@ -1505,7 +1611,7 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 	struct cpc_register_resource *highest_reg, *lowest_reg,
 		*lowest_non_linear_reg, *nominal_reg, *reference_reg,
 		*guaranteed_reg, *low_freq_reg = NULL, *nom_freq_reg = NULL;
-	u64 high, low, guaranteed, nom, ref, min_nonlinear,
+	u64 high, low, guaranteed = 0, nom, ref, min_nonlinear,
 	    low_f = 0, nom_f = 0;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpunum);
 	struct cppc_pcc_data *pcc_ss_data = NULL;
@@ -1588,7 +1694,12 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 		goto out_err;
 	perf_caps->lowest_nonlinear_perf = min_nonlinear;
 
-	if (!high || !low || !nom || !ref || !min_nonlinear) {
+	if (!high || !nom || !ref || !min_nonlinear ||
+	    high > U32_MAX || low > U32_MAX || guaranteed > U32_MAX ||
+	    nom > U32_MAX || ref > U32_MAX || min_nonlinear > U32_MAX ||
+	    high < nom || nom < min_nonlinear || min_nonlinear < low ||
+	    (CPC_SUPPORTED(guaranteed_reg) &&
+	     (guaranteed < low || guaranteed > nom))) {
 		ret = -EFAULT;
 		goto out_err;
 	}
@@ -1605,6 +1716,19 @@ int cppc_get_perf_caps(int cpunum, struct cppc_perf_caps *perf_caps)
 		if (ret)
 			goto out_err;
 	}
+	/*
+	 * Require ordered anchors and a nonzero slope when frequencies differ.
+	 * A zero Lowest Performance needs that affine mapping to produce a
+	 * nonzero physical minimum frequency.
+	 */
+	if (low_f > U32_MAX || nom_f > U32_MAX ||
+	    (!low && (!low_f || !nom_f || low_f == nom_f)) ||
+	    (low_f && nom_f &&
+	     (nom_f < low_f || nom < low ||
+	      (nom_f != low_f && nom == low)))) {
+		ret = -EFAULT;
+		goto out_err;
+	}
 
 	perf_caps->lowest_freq = low_f;
 	perf_caps->nominal_freq = nom_f;
@@ -1779,6 +1903,9 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
 
 	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
 	epp_set_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
+	if (!enable && auto_sel_reg->type == ACPI_TYPE_INTEGER &&
+	    auto_sel_reg->cpc_entry.int_value == 1)
+		return -EOPNOTSUPP;
 
 	epp_ffh_sysmem = CPC_SUPPORTED(epp_set_reg) &&
 		(CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
@@ -1791,13 +1918,13 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
 			return -ENODEV;
 		}
 
-		if (CPC_SUPPORTED(auto_sel_reg)) {
+		if (cpc_is_writable(auto_sel_reg)) {
 			ret = cpc_write(cpu, auto_sel_reg, enable);
 			if (ret)
 				return ret;
 		}
 
-		if (CPC_SUPPORTED(epp_set_reg)) {
+		if (cpc_is_writable(epp_set_reg)) {
 			ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
 			if (ret)
 				return ret;
@@ -1996,6 +2123,8 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
 	energy_perf_reg = &cpc_desc->cpc_regs[ENERGY_PERF];
 	auto_sel_reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
+	perf_ctrls->max_perf_valid = false;
+	perf_ctrls->min_perf_valid = false;
 
 	/* Are any of the regs PCC ?*/
 	if (CPC_IN_PCC(min_perf_reg) || CPC_IN_PCC(max_perf_reg) ||
@@ -2020,6 +2149,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 		ret = cpc_read(cpu, max_perf_reg, &max);
 		if (ret)
 			goto out_err;
+		perf_ctrls->max_perf_valid = true;
 	}
 	perf_ctrls->max_perf = max;
 
@@ -2027,6 +2157,7 @@ int cppc_get_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 		ret = cpc_read(cpu, min_perf_reg, &min);
 		if (ret)
 			goto out_err;
+		perf_ctrls->min_perf_valid = true;
 	}
 	perf_ctrls->min_perf = min;
 
@@ -2113,14 +2244,11 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	if (CPC_SUPPORTED(desired_reg))
 		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
 
-	/*
-	 * Only write if min_perf and max_perf not zero. Some drivers pass zero
-	 * value to min and max perf, but they don't mean to set the zero value,
-	 * they just don't want to write to those registers.
-	 */
-	if (perf_ctrls->min_perf && CPC_SUPPORTED(min_perf_reg))
+	if (CPC_SUPPORTED(min_perf_reg) &&
+	    (perf_ctrls->min_perf || perf_ctrls->min_perf_valid))
 		cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
-	if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
+	if (CPC_SUPPORTED(max_perf_reg) &&
+	    (perf_ctrls->max_perf || perf_ctrls->max_perf_valid))
 		cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
 
 	if (regs_in_pcc)
diff --git a/drivers/cpufreq/amd-pstate.c b/drivers/cpufreq/amd-pstate.c
index d4ff8b228f86..63f0ca5f19b3 100644
--- a/drivers/cpufreq/amd-pstate.c
+++ b/drivers/cpufreq/amd-pstate.c
@@ -544,7 +544,13 @@ static int shmem_update_perf(struct cpufreq_policy *policy, u8 min_perf,
 			     u8 des_perf, u8 max_perf, u8 epp, bool fast_switch)
 {
 	struct amd_cpudata *cpudata = policy->driver_data;
-	struct cppc_perf_ctrls perf_ctrls;
+	struct cppc_perf_ctrls perf_ctrls = {
+		.max_perf = max_perf,
+		.min_perf = min_perf,
+		.desired_perf = des_perf,
+		.max_perf_valid = true,
+		.min_perf_valid = true,
+	};
 	u64 value, prev;
 	int ret;
 
@@ -577,10 +583,6 @@ static int shmem_update_perf(struct cpufreq_policy *policy, u8 min_perf,
 	if (value == prev)
 		return 0;
 
-	perf_ctrls.max_perf = max_perf;
-	perf_ctrls.min_perf = min_perf;
-	perf_ctrls.desired_perf = des_perf;
-
 	ret = cppc_set_perf(cpudata->cpu, &perf_ctrls);
 	if (ret)
 		return ret;
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 94a6277edab2..5dcbe65c5ddc 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -141,6 +141,8 @@ struct cppc_perf_ctrls {
 	u32 desired_perf;
 	u32 energy_perf;
 	bool auto_sel;
+	bool max_perf_valid;
+	bool min_perf_valid;
 };
 
 struct cppc_perf_fb_ctrs {
-- 
2.34.1