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 v4 02/15] ACPI: CPPC: Validate _CPC entry and control semantics
Date: Wed, 26 Aug 2026 07:30:06 +0100 [thread overview]
Message-ID: <20260826063019.670240-3-christian.loehle@arm.com> (raw)
In-Reply-To: <20260826063019.670240-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 | 116 +++++++++++++++++++++++++++++++++++----
1 file changed, 104 insertions(+), 12 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 3b8cdf88e31d..e803fe9327ca 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;
@@ -1217,11 +1300,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 = ®_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 = ®_res->cpc_entry.reg;
+ if (IS_NULL_REG(reg))
+ return -EOPNOTSUPP;
+
size = GET_BIT_WIDTH(reg);
if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
@@ -1364,7 +1454,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(®->cpc_entry.reg))) {
pr_debug("CPC register is not supported\n");
@@ -1415,7 +1507,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(®->cpc_entry.reg)) {
+ if (!cpc_is_writable(reg)) {
pr_debug("CPC register is not supported\n");
return -EOPNOTSUPP;
}
@@ -1588,7 +1680,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;
}
@@ -1791,13 +1883,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
next prev parent reply other threads:[~2026-08-26 6:30 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-26 6:30 [PATCH v4 00/15] ACPI: CPPC: Fix register access and lifetime bugs Christian Loehle
2026-08-26 6:30 ` [PATCH v4 01/15] ACPI: CPPC: Validate the _CPC package header Christian Loehle
2026-08-26 6:30 ` Christian Loehle [this message]
2026-08-26 6:30 ` [PATCH v4 03/15] ACPI: CPPC: Propagate performance-control write errors Christian Loehle
2026-08-26 6:30 ` [PATCH v4 04/15] ACPI: CPPC: Use 64-bit masks for register fields Christian Loehle
2026-08-26 6:30 ` [PATCH v4 05/15] ACPI: CPPC: Serialize PCC single-register payload updates Christian Loehle
2026-08-26 6:30 ` [PATCH v4 06/15] ACPI: CPPC: Serialize PCC EPP " Christian Loehle
2026-08-26 6:30 ` [PATCH v4 07/15] ACPI: CPPC: Release CPC descriptors through kobject Christian Loehle
2026-08-26 6:30 ` [PATCH v4 08/15] ACPI: CPPC: Release PCC data after probe failures Christian Loehle
2026-08-26 6:30 ` [PATCH v4 09/15] ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW Christian Loehle
2026-08-26 6:30 ` [PATCH v4 10/15] ACPI: CPPC: Reject direct reads of write-only controls Christian Loehle
2026-08-26 6:30 ` [PATCH v4 11/15] ACPI: CPPC: Validate and access PCC register layouts Christian Loehle
2026-08-26 6:30 ` [PATCH v4 12/15] ACPI: CPPC: Validate SystemIO " Christian Loehle
2026-08-26 6:30 ` [PATCH v4 13/15] ACPI: CPPC: Validate PCC overlaps across processors Christian Loehle
2026-08-26 6:30 ` [PATCH v4 14/15] ACPI: CPPC: Validate SystemIO " Christian Loehle
2026-08-26 6:30 ` [PATCH v4 15/15] ACPI: CPPC: Clear Performance Limited without a stale read Christian Loehle
2026-08-26 13:59 ` Sumit Gupta
2026-08-26 14:49 ` Christian Loehle
2026-08-26 14:17 ` [PATCH v4 00/15] ACPI: CPPC: Fix register access and lifetime bugs 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=20260826063019.670240-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox