[PATCH v6 10/15] ACPI: CPPC: Reject direct reads of write-only controls
From: Christian Loehle
Date: Sun Aug 30 2026 - 08:02:36 EST
Between _CPC revision 3 and revision 4, Desired Performance changed from
Read/Write to Write. Revision 4 also added the write-only OSPM Nominal
Performance control. ACPI 6.6 section 4.6.3 says reads of write-only bit
positions produce undefined results.
The public Desired Performance helper already rejects revision-4 readback,
but the common register accessor still permits either write-only control to
be read. Reject both centrally so new callers cannot consume undefined
values.
A partial SystemMemory field still makes cpc_write() read its complete
access unit to preserve bits outside the field. MASK_VAL_WRITE() replaces
every bit of the field being written, so that field's undefined readback is
not propagated. However, another writer sharing the access unit would
preserve and replay the write-only field when performing its own RMW.
Reject such pairs; a partial write-only field may share its access unit
only with disjoint read-only fields. The descriptor lock serializes
supported RMW, and the preceding validation rejects unsafe cross-descriptor
partial writers.
Keep an invalid but locatable write-only SystemMemory descriptor
represented during overlap validation, but mark it unreadable and
unwritable. Otherwise a neighbouring RMW field could evade validation and
replay undefined readback into the hidden control. Conservatively claim the
larger of the declared access unit and logical field span. Check if the
writer access unit covers the write-only field so harmless asymmetric
geometries are not rejected. Preserve a decoded access-unit claim even when
the malformed Bit Width is zero, and use that claimed range when testing
whether another writer's access overlaps the hidden field.
A retained range-only descriptor cannot issue a transaction over that whole
conservative claim. Keep the full-width ownership rule for accessible
descriptors, but reject a full-width writer against a range-only descriptor
only when its access covers the hidden logical field. Likewise, count only
accessible descriptors as writers in the generic writer-conflict rules;
the dedicated write-only-field check still protects a retained descriptor
from an accessible writer's RMW.
Mark an inaccessible OSPM Nominal Performance control unsupported because
it is optional. Do the same for inaccessible Desired Performance while
parsing, then let the post-parse control check accept it only for immutable
autonomous selection. This preserves the autonomous-only exception without
accepting an unusable Desired control in non-autonomous mode.
Do not advertise fast switching or zero transition latency unless Desired
Performance remains writable. An inaccessible descriptor retained only for
overlap validation still carries its original address-space identity, but
cannot service a performance request.
Fixes: 71e1815113f7 ("ACPI: CPPC: Add support for CPPC v4")
Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
Link: https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@xxxxxxx>
---
drivers/acpi/cppc_acpi.c | 153 +++++++++++++++++++++++++++++++++++----
include/acpi/cppc_acpi.h | 2 +
2 files changed, 141 insertions(+), 14 deletions(-)
diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 058f4c71d981..bb83b5c9ab31 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -171,7 +171,14 @@ static struct cpc_sysmem_node *cpc_sysmem_next(struct cpc_sysmem_node *node,
static bool cpc_is_writable(const struct cpc_register_resource *cpc)
{
return cpc->type == ACPI_TYPE_BUFFER &&
- !IS_NULL_REG(&cpc->cpc_entry.reg);
+ !IS_NULL_REG(&cpc->cpc_entry.reg) &&
+ !cpc->cpc_entry.write_unsupported;
+}
+
+static bool cpc_is_readable(const struct cpc_register_resource *cpc)
+{
+ return cpc->type != ACPI_TYPE_BUFFER ||
+ !cpc->cpc_entry.read_unsupported;
}
static bool cpc_entry_present(const struct cpc_register_resource *cpc)
@@ -313,6 +320,22 @@ static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg)
return width / 8;
}
+static u64 cpc_sysmem_field_size(const struct cpc_reg *gas)
+{
+ return DIV_ROUND_UP((u64)gas->bit_offset + gas->bit_width, 8);
+}
+
+static u64 cpc_sysmem_claim_size(const struct cpc_register_resource *reg)
+{
+ const struct cpc_reg *gas = ®->cpc_entry.reg;
+ u64 access_size = cpc_sysmem_access_size(reg);
+
+ if (!gas->bit_width)
+ return access_size;
+
+ return max(access_size, cpc_sysmem_field_size(gas));
+}
+
static bool cpc_reg_access_aligned(const struct cpc_reg *reg, u64 access_size)
{
/* x86 MMIO and port-I/O accessors support unaligned addresses. */
@@ -324,8 +347,8 @@ static bool cpc_sysmem_access_units_overlap(const struct cpc_register_resource *
{
const struct cpc_reg *a_gas = &a->cpc_entry.reg;
const struct cpc_reg *b_gas = &b->cpc_entry.reg;
- u64 a_size = cpc_sysmem_access_size(a);
- u64 b_size = cpc_sysmem_access_size(b);
+ u64 a_size = cpc_sysmem_claim_size(a);
+ u64 b_size = cpc_sysmem_claim_size(b);
/* Keep the conservative locking path for malformed access widths. */
if (!a_size || !b_size)
@@ -355,6 +378,21 @@ static bool cpc_reg_is_writable(unsigned int reg_idx)
}
}
+static bool cpc_reg_is_write_only(const struct cpc_desc *cpc_desc,
+ unsigned int reg_idx)
+{
+ return cpc_desc->version >= CPPC_V4_REV &&
+ (reg_idx == DESIRED_PERF || reg_idx == OSPM_NOMINAL_PERF);
+}
+
+static void cpc_disable_reg(struct cpc_desc *cpc_desc, unsigned int reg_idx)
+{
+ struct cpc_register_resource *reg = &cpc_desc->cpc_regs[reg_idx];
+
+ reg->type = ACPI_TYPE_INTEGER;
+ reg->cpc_entry.int_value = 0;
+}
+
static bool cpc_sysmem_reg_needs_rmw(const struct cpc_register_resource *reg)
{
const struct cpc_reg *gas = ®->cpc_entry.reg;
@@ -363,7 +401,7 @@ static bool cpc_sysmem_reg_needs_rmw(const struct cpc_register_resource *reg)
return gas->bit_offset || gas->bit_width != access_size * 8;
}
-static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
+static int cpc_validate_sysmem_reg(struct cpc_desc *cpc_desc,
const struct cpc_reg *gas,
unsigned int reg_idx)
{
@@ -388,6 +426,23 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
return 0;
invalid:
+ access_size = 0;
+ if (access_width == 8 || access_width == 16 ||
+ access_width == 32 || access_width == 64)
+ access_size = access_width / 8;
+ if (gas->bit_width)
+ access_size = max(access_size, cpc_sysmem_field_size(gas));
+ if (cpc_reg_is_write_only(cpc_desc, reg_idx) && gas->address &&
+ access_size && gas->address <= U64_MAX - (access_size - 1)) {
+ struct cpc_register_resource *reg = &cpc_desc->cpc_regs[reg_idx];
+
+ pr_warn("CPU%d: _CPC v%d register %u is inaccessible; keeping its range reserved\n",
+ cpc_desc->cpu_id, cpc_desc->version, reg_idx);
+ reg->cpc_entry.read_unsupported = true;
+ reg->cpc_entry.write_unsupported = true;
+ return 0;
+ }
+
pr_debug("CPU:%d invalid SystemMemory GAS for _CPC register %u\n",
cpc_desc->cpu_id, reg_idx);
return -EINVAL;
@@ -445,6 +500,39 @@ static bool cpc_sysmem_fields_overlap(const struct cpc_register_resource *a,
!cpc_bit_position_before(&b_end, &a_start);
}
+static bool cpc_sysmem_access_overlaps_field(const struct cpc_register_resource *access,
+ const struct cpc_register_resource *field)
+{
+ const struct cpc_reg *access_gas = &access->cpc_entry.reg;
+ const struct cpc_reg *field_gas = &field->cpc_entry.reg;
+ u64 access_last;
+ u64 field_start;
+ u64 field_last;
+
+ if (!field_gas->bit_width)
+ return cpc_sysmem_access_units_overlap(access, field);
+
+ access_last = access_gas->address +
+ cpc_sysmem_access_size(access) - 1;
+ field_start = field_gas->address + field_gas->bit_offset / 8;
+ field_last = field_gas->address +
+ (field_gas->bit_offset + field_gas->bit_width - 1) / 8;
+
+ return access_gas->address <= field_last && field_start <= access_last;
+}
+
+static bool cpc_sysmem_full_width_conflicts(const struct cpc_register_resource *writer,
+ const struct cpc_register_resource *other)
+{
+ /*
+ * An accessible descriptor may issue a transaction over its complete
+ * access unit. A retained range-only descriptor cannot; protect only
+ * its logical field from the full-width writer.
+ */
+ return cpc_is_readable(other) || cpc_is_writable(other) ||
+ cpc_sysmem_access_overlaps_field(writer, other);
+}
+
static bool cpc_same_sysmem_register(unsigned int a_idx,
const struct cpc_register_resource *a,
unsigned int b_idx,
@@ -474,8 +562,8 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
!cpc_sysmem_access_units_overlap(a, b))
return 0;
- a_writable = cpc_reg_is_writable(a_idx);
- b_writable = cpc_reg_is_writable(b_idx);
+ a_writable = cpc_reg_is_writable(a_idx) && cpc_is_writable(a);
+ b_writable = cpc_reg_is_writable(b_idx) && cpc_is_writable(b);
if (!a_writable && !b_writable)
return 0;
@@ -498,8 +586,10 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
}
/* A full-width writable register owns its complete access unit. */
- if ((a_writable && !cpc_sysmem_reg_needs_rmw(a)) ||
- (b_writable && !cpc_sysmem_reg_needs_rmw(b)) ||
+ if ((a_writable && !cpc_sysmem_reg_needs_rmw(a) &&
+ cpc_sysmem_full_width_conflicts(a, b)) ||
+ (b_writable && !cpc_sysmem_reg_needs_rmw(b) &&
+ cpc_sysmem_full_width_conflicts(b, a)) ||
(a_writable && b_writable && cpc_sysmem_fields_overlap(a, b)))
goto conflict;
@@ -507,6 +597,18 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
if (a_desc != b_desc && a_writable && b_writable)
goto conflict;
+ /*
+ * RMW of either writer preserves the other field. If that field is
+ * write-only, its readback is undefined and cannot safely be replayed.
+ */
+ if ((cpc_reg_is_write_only(a_desc, a_idx) && b_writable &&
+ cpc_sysmem_reg_needs_rmw(b) &&
+ cpc_sysmem_access_overlaps_field(b, a)) ||
+ (cpc_reg_is_write_only(b_desc, b_idx) && a_writable &&
+ cpc_sysmem_reg_needs_rmw(a) &&
+ cpc_sysmem_access_overlaps_field(a, b)))
+ goto conflict;
+
return 0;
conflict:
@@ -589,7 +691,7 @@ static int cpc_register_sysmem_desc(struct cpc_desc *cpc_desc)
continue;
node = &cpc_desc->sysmem_nodes[i];
- size = cpc_sysmem_access_size(reg);
+ size = cpc_sysmem_claim_size(reg);
node->start = reg->cpc_entry.reg.address;
node->last = node->start + size - 1;
node->desc = cpc_desc;
@@ -970,7 +1072,7 @@ bool cppc_allow_fast_switch(const struct cpumask *cpus)
min_reg = &cpc_ptr->cpc_regs[MIN_PERF];
max_reg = &cpc_ptr->cpc_regs[MAX_PERF];
- if (!CPC_SUPPORTED(desired_reg) ||
+ if (!cpc_is_writable(desired_reg) ||
(!CPC_IN_SYSTEM_MEMORY(desired_reg) &&
!CPC_IN_SYSTEM_IO(desired_reg)) ||
(CPC_SUPPORTED(min_reg) &&
@@ -1370,6 +1472,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
goto out_free;
}
+ cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_BUFFER;
+ memcpy(&cpc_ptr->cpc_regs[i - 2].cpc_entry.reg, gas_t,
+ sizeof(*gas_t));
+
/*
* The PCC Subspace index is encoded inside
* the CPC table entries. The same PCC index
@@ -1396,10 +1502,24 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
size_t access_width;
err = cpc_validate_sysmem_reg(cpc_ptr, gas_t, i - 2);
+ if (err && (i - 2 == DESIRED_PERF ||
+ i - 2 == OSPM_NOMINAL_PERF)) {
+ const char *name = i - 2 == DESIRED_PERF ?
+ "Desired Performance" :
+ "OSPM Nominal Performance";
+
+ pr_warn("CPU%d: disabling inaccessible %s register\n",
+ pr->id, name);
+ cpc_disable_reg(cpc_ptr, i - 2);
+ continue;
+ }
if (err) {
ret = err;
goto out_free;
}
+ if (!cpc_is_readable(&cpc_ptr->cpc_regs[i - 2]) &&
+ !cpc_is_writable(&cpc_ptr->cpc_regs[i - 2]))
+ continue;
if (!osc_cpc_flexible_adr_space_confirmed) {
pr_debug("Flexible address space capability not supported\n");
@@ -1451,10 +1571,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
goto out_free;
}
}
-
- cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_BUFFER;
- memcpy(&cpc_ptr->cpc_regs[i - 2].cpc_entry.reg, gas_t,
- sizeof(*gas_t));
} else if (cpc_obj->type == ACPI_TYPE_PACKAGE && (i - 2) == RESOURCE_PRIORITY) {
/*
* ACPI 6.6, s8.4.6.1.2.7 defines Resource Priority as a
@@ -1794,6 +1910,10 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
}
if (reg->bit_offset || reg->bit_width != size) {
+ /*
+ * MASK_VAL_WRITE() discards the field's old bits, so undefined
+ * readback from a write-only field is not propagated.
+ */
switch (size) {
case 8:
prev_val = readb_relaxed(vaddr);
@@ -1885,6 +2005,8 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val)
pr_debug("No CPC descriptor for CPU:%d\n", cpu);
return -ENODEV;
}
+ if (cpc_reg_is_write_only(cpc_desc, reg_idx))
+ return -EOPNOTSUPP;
reg = &cpc_desc->cpc_regs[reg_idx];
@@ -2975,6 +3097,9 @@ int cppc_get_transition_latency(int cpu_num)
return -ENODATA;
desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
+ if (!cpc_is_writable(desired_reg))
+ return -ENODATA;
+
if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg))
return 0;
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 168e3143e6ae..4e5f59bc95f8 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -72,6 +72,8 @@ struct cpc_register_resource {
struct {
struct cpc_reg reg;
bool use_rmw_lock;
+ bool read_unsupported;
+ bool write_unsupported;
};
u64 int_value;
} cpc_entry;
--
2.34.1