All of lore.kernel.org
 help / color / mirror / Atom feed
From: Christian Loehle <christian.loehle@arm.com>
To: "Rafael J . Wysocki" <rafael@kernel.org>,
	Viresh Kumar <viresh.kumar@linaro.org>
Cc: linux-pm@vger.kernel.org, linux-acpi@vger.kernel.org,
	linux-kernel@vger.kernel.org, Len Brown <lenb@kernel.org>,
	Jie Zhan <zhanjie9@hisilicon.com>,
	Lifeng Zheng <zhenglifeng1@huawei.com>,
	Pierre Gondois <pierre.gondois@arm.com>,
	Sumit Gupta <sumitg@nvidia.com>,
	Sudeep Holla <sudeep.holla@arm.com>,
	Ionela Voinescu <ionela.voinescu@arm.com>,
	zhongqiu.han@oss.qualcomm.com,
	Christian Loehle <christian.loehle@arm.com>,
	Sashiko <sashiko-bot@kernel.org>
Subject: [PATCH v3 02/15] ACPI: CPPC: Validate _CPC entry and control semantics
Date: Sun,  9 Aug 2026 07:25:36 +0100	[thread overview]
Message-ID: <20260809062549.1415955-3-christian.loehle@arm.com> (raw)
In-Reply-To: <20260809062549.1415955-1-christian.loehle@arm.com>

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 Linux: 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.

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 and remove the matching runtime rejection.

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
Linux's 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.

Fixes: 337aadff8e45 ("ACPI: Introduce CPU performance controls using CPPC")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 112 +++++++++++++++++++++++++++++++++++----
 1 file changed, 101 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 17d88aae1c3c..19a3a71fee45 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 =		\
@@ -871,11 +911,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
@@ -961,15 +1022,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.
@@ -1027,6 +1108,8 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	return 0;
 
 out_free:
+	pr_err("CPU%d: failed to initialize _CPC: %d\n", pr->id, ret);
+
 	/* Free all the mapped sys mem areas for this CPU */
 	for (i = 2; i < cpc_ptr->num_entries; i++) {
 		void __iomem *addr = cpc_ptr->cpc_regs[i-2].sys_mem_vaddr;
@@ -1201,11 +1284,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) &&
@@ -1399,7 +1489,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;
 	}
@@ -1572,7 +1662,7 @@ 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) {
 		ret = -EFAULT;
 		goto out_err;
 	}
@@ -1751,13 +1841,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;
-- 
2.34.1


  parent reply	other threads:[~2026-08-09  6:26 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-09  6:25 [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs Christian Loehle
2026-08-09  6:25 ` [PATCH v3 01/15] ACPI: CPPC: Validate the _CPC package header Christian Loehle
2026-08-09  6:25 ` Christian Loehle [this message]
2026-08-09  6:25 ` [PATCH v3 03/15] ACPI: CPPC: Propagate performance-control write errors Christian Loehle
2026-08-09  6:25 ` [PATCH v3 04/15] ACPI: CPPC: Use 64-bit masks for register fields Christian Loehle
2026-08-09  6:25 ` [PATCH v3 05/15] ACPI: CPPC: Serialize PCC single-register payload updates Christian Loehle
2026-08-09  6:25 ` [PATCH v3 06/15] ACPI: CPPC: Serialize PCC EPP " Christian Loehle
2026-08-09  6:25 ` [PATCH v3 07/15] ACPI: CPPC: Release CPC descriptors through kobject Christian Loehle
2026-08-09  6:25 ` [PATCH v3 08/15] ACPI: CPPC: Release PCC data after probe failures Christian Loehle
2026-08-09  6:25 ` [PATCH v3 09/15] ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW Christian Loehle
2026-08-09  6:25 ` [PATCH v3 10/15] ACPI: CPPC: Reject reads and RMW of write-only controls Christian Loehle
2026-08-09  6:25 ` [PATCH v3 11/15] ACPI: CPPC: Validate and access PCC register layouts Christian Loehle
2026-08-09  6:25 ` [PATCH v3 12/15] ACPI: CPPC: Validate SystemIO " Christian Loehle
2026-08-09  6:25 ` [PATCH v3 13/15] ACPI: CPPC: Validate PCC overlaps across processors Christian Loehle
2026-08-09  6:25 ` [PATCH v3 14/15] ACPI: CPPC: Validate SystemIO " Christian Loehle
2026-08-09  6:25 ` [PATCH v3 15/15] ACPI: CPPC: Clear Performance Limited without a stale read Christian Loehle
2026-08-09  7:01   ` Christian Loehle
2026-08-25  9:15   ` Sumit Gupta
2026-08-25 10:09     ` Christian Loehle
2026-08-09  7:18 ` [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs Christian Loehle
2026-08-10  5:15   ` Christian Loehle
2026-08-20 10:07 ` Christian Loehle
2026-08-20 10:31   ` Rafael J. Wysocki (Intel)
2026-08-21 14:39     ` Rafael J. Wysocki (Intel)
2026-08-25  6:58   ` Sumit Gupta
2026-08-25  8:25     ` Christian Loehle
2026-08-25 11:48       ` Rafael J. Wysocki (Intel)
2026-08-25 20:53       ` Sumit Gupta

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260809062549.1415955-3-christian.loehle@arm.com \
    --to=christian.loehle@arm.com \
    --cc=ionela.voinescu@arm.com \
    --cc=lenb@kernel.org \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=pierre.gondois@arm.com \
    --cc=rafael@kernel.org \
    --cc=sashiko-bot@kernel.org \
    --cc=sudeep.holla@arm.com \
    --cc=sumitg@nvidia.com \
    --cc=viresh.kumar@linaro.org \
    --cc=zhanjie9@hisilicon.com \
    --cc=zhenglifeng1@huawei.com \
    --cc=zhongqiu.han@oss.qualcomm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.