[PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Christian Loehle]

First of all, sorry this got so out of hand, initially this was just
trying to fix some relatively simple issues found by sashiko in an
earlier (unrelated) series.
But with me touching more and more code and going through rounds of AI
review that kept finding more and more pre-existing issues I've arrived
at this.

This series fixes correctness and robustness issues found while reviewing
the CPPC control path. They affect malformed _CPC handling, error
propagation, PCC ownership and cleanup, CPC object lifetime, register field
access, cross-processor aliases, and Performance Limited clearing.

Series structure
================

Patches 1-8 are deliberately small, independently useful fixes. They
validate the _CPC encoding consumed by cppc-acpi, propagate control-write
errors, serialize PCC payload updates, correct 64-bit field masks, and fix
descriptor and PCC lifetime handling.

Patches 9-15 are the register-layout hardening portion. Geometry validation
is more substantial because safe RMW and alias handling depend on the
physical access unit, not merely on a logical _CPC entry or _PSD domain.
These patches normalize and validate each supported address space before
building probe-only physical interval registries. Keeping this work in the
same posting gives the complete safety boundary and a single base for
review, while each transport and bug retains its own Fixes provenance.
Feel free to treat the two parts as independent series, I didn't split it
because they're all technically fixes and to get Sashiko review for the
whole lot.

No interval lookup is added to the scheduler hot path. Full-width
SystemMemory writes remain lockless. RMW locking remains necessary only for
a partial field, where we must preserve the other bits in its access
unit. The existing per-descriptor raw lock continues to cover disjoint
partial fields within one _CPC package; probe rejects cross-descriptor
layouts that it cannot protect.

Parsing and control semantics
=============================

The parser now validates the package header before indexing it, bounds the
BYTE and DWORD Integer forms before conversion, and validates the Generic
Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
package count, but additional trailing package elements are ignored because
doing so is safe and preserves compatibility with padded firmware. The parser
likewise tolerates trailing ResourceTemplate data instead of imposing a new
EndTag compatibility requirement.

Writable controls must be Buffer-encoded registers. Minimum and Maximum
Performance are checked as the pair required by ACPI 6.6 Sections
8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
Integer-zero convention for absent optional fields, so Lowest Performance
may retain the valid abstract value zero.

Performance Limited is one deliberate compatibility exception. ACPI lists
it as required, but permits a platform with no limiting indication to
always return zero, and deployed firmware represents that case with a NULL
descriptor. CPPC control does not depend on this status register, so we
continue to accept that encoding. A present _CPC package which otherwise
fails parsing or initialization now emits an error instead of silently
preventing cpufreq registration.

Compound performance and EPP updates propagate errors and perform every
fallible non-PCC write before modifying the PCC payload. Updates across
address spaces cannot be atomic, but a known non-PCC failure can no longer
commit only the PCC portion or leave an unsent value for a later command.

SystemMemory locking and support boundary
=========================================

A partial SystemMemory field requires RMW to preserve the rest of its
access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
a per-_CPC lock and noted that a global lock would be needed if physical
registers were shared between packages.

ACPI does not make _PSD a physical-register ownership boundary. Rather than
put a global raw lock or lookup into the scheduler path, this series makes
the cheaper per-descriptor model's assumptions enforceable at probe.

Supported SystemMemory layouts are:

  - naturally aligned 8-, 16-, 32-, and 64-bit access units;
  - lockless full-width controls;
  - read-only aliases;
  - exact full-width writable aliases, including 64-bit aliases on 64-bit
    kernels;
  - disjoint partial writers within one descriptor, serialized by its
    rmw_lock; and
  - a partial writer sharing an access unit with a disjoint read-only
    field.

Probe rejects overlapping logical fields involving a writer, another field
inside a full-width writable access unit, cross-descriptor partial writers,
unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
These layouts were not safely supported by the old per-descriptor lock or
generic writeq(); rejecting them turns possible corruption into a visible
probe failure rather than removing working support.

PCC access and locking
======================

The PCC protocol requires OSPM to acquire the subspace before changing its
command or payload. Single-register and EPP updates now hold pcc_lock
across ownership acquisition, payload staging, and command submission.

ACPI 6.6's implementation example places a mandatory 32-bit Delivered
Performance Counter at unaligned PCC offset 0x116. Performance controls may
also use byte-multiple widths such as 24 bits. PCC therefore uses
byte-oriented I/O with explicit little-endian encoding for zero-offset,
byte-multiple fields from 8 through 64 bits. A short per-subspace payload
lock protects concurrent aliased copies made under the shared side of
pcc_lock; it does not replace the protocol ownership lock.

Bit-level PCC fields require RMW and remain unsupported. An unsupported
optional field is marked absent, but a present inaccessible CPPC Enable
fails probe because OSPM must write it before using CPPC. Thus the
ACPI-legal one-bit CPPC Enable used by the specification example is a
documented kernel limitation. The old accessor could not program it
correctly either, so an explicit error is safer than silently proceeding
without enabling CPPC.

Every retained PCC field is bounds checked against the shared-memory
region. A subspace-keyed interval registry permits read-only overlap and
exact same-control aliases while rejecting every other writable overlap
across processors.

SystemIO support boundary
=========================

SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
at or below port 0xffff, including legacy Access Size zero when Bit Width
supplies the size. Partial fields never worked because the driver neither
shifted them nor preserved adjacent bits, so they now fail visibly instead
of being misprogrammed.

On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
disabled according to the affected control's semantics. Runtime accessors
also return -EOPNOTSUPP rather than treating an I/O port as a
physical-memory address. A global port interval registry rejects
cross-processor writable overlap.

Write-only and Performance Limited controls
===========================================

Between _CPC revisions 3 and 4, Desired Performance changed from
Read/Write to Write, and revision 4 added write-only OSPM Nominal
Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
are undefined. Explicit reads of both controls are rejected, as are
SystemMemory layouts which would implicitly read them for RMW. Full-width
writes remain supported.

Performance Limited is sticky, write-zero-to-clear, and requires
interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
read and write could clear a new event reported between transactions. The
clear path now writes zero only to requested status bits and one to the
other defined bits. Partial SystemMemory forms cannot be used because a
spinlock cannot interlock an enclosing RMW with platform updates. QWord
forms cannot be used on 32-bit kernels, where the MMIO accessor may be
split into two 32-bit operations; naturally aligned, full-width QWords
remain supported on 64-bit kernels. Since CPPC control does not depend on
Performance Limited status, an unusable description disables that status
register instead of rejecting the processor's otherwise usable _CPC.

Lifetime and cleanup
====================

CPC descriptors are released through their kobject callback, keeping their
storage and mappings alive for outstanding sysfs references. Every PCC
allocation, reference, and acquired channel is unwound on probe failure,
and the per-CPU PCC index is initialized before every early return. PCC
allocation uses a separate temporary result, so its success cannot turn a
later parse failure into a successful probe return.

Changes since v2
================

  - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
    package elements while still rejecting any count that could cause an
    out-of-bounds walk.
  - Made patch 10 independently preserve immutable-autonomous setups whose
    inaccessible Desired Performance register requires RMW, rather than
    relying on patch 11 to restore that exception.
    
Sashiko v2 review not addressed 
===============================

  - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
    nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
    entry.

Deferred follow-up work
=======================

Sashiko also identified a broader pre-existing lifetime question which this
series does not attempt to solve. In-kernel accessors read the per-CPU
cpc_desc_ptr without acquiring a reference, while processor teardown can
unpublish and eventually release the descriptor and its PCC data. The kobject
change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
would not protect a caller which already holds a stale pointer. Closing this
properly requires defining the kernel accessor lifetime contract and then
using CPU-hotplug serialization / safe referencing across all callers,
therefore will be handled by a follow-up.

ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
implemented by the old whole-value accessors. Supporting these descriptions
requires transport-specific field extraction and an RMW operation which obeys
PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
descriptors would silently program the wrong value. Therefore continue to
disable optional inaccessible fields where safe and reject a present
inaccessible CPPC Enable control.
Full support, if even needed, belongs in a separate follow-up.

The review additionally suggested validating the complete AML
ResourceTemplate, including its EndTag. We currently validate the Register
descriptor we consume and tolerate trailing firmware data. I don't really
see the point of ever doing this, but definitely not in this series,
where I'm trying to guarantee that no reasonably working platform is
regressing.

Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
reviewing:

  https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com

Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:

  https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com

Patches 1 and 10 address findings from the v2 review:

  https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com

The series is based on Rafael's bleeding-edge
77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
the base-commit specified below is linux-next for Sashiko review.
It applies cleanly on either.

Christian Loehle (15):
  ACPI: CPPC: Validate the _CPC package header
  ACPI: CPPC: Validate _CPC entry and control semantics
  ACPI: CPPC: Propagate performance-control write errors
  ACPI: CPPC: Use 64-bit masks for register fields
  ACPI: CPPC: Serialize PCC single-register payload updates
  ACPI: CPPC: Serialize PCC EPP payload updates
  ACPI: CPPC: Release CPC descriptors through kobject
  ACPI: CPPC: Release PCC data after probe failures
  ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
  ACPI: CPPC: Reject reads and RMW of write-only controls
  ACPI: CPPC: Validate and access PCC register layouts
  ACPI: CPPC: Validate SystemIO register layouts
  ACPI: CPPC: Validate PCC overlaps across processors
  ACPI: CPPC: Validate SystemIO overlaps across processors
  ACPI: CPPC: Clear Performance Limited without a stale read

 drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
 include/acpi/cppc_acpi.h |    7 +-
 2 files changed, 1116 insertions(+), 189 deletions(-)

base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
-- 
2.34.1

[PATCH v3 01/15] ACPI: CPPC: Validate the _CPC package header

[Christian Loehle]

The _CPC NumEntries field includes every package element, including
NumEntries and Revision. acpi_cppc_processor_probe() nevertheless reads
those first two elements before checking that they exist and trusts
NumEntries when walking the remaining elements.

Reject packages with fewer than two elements and NumEntries values which
exceed the package count before accessing Revision or iterating over
register descriptors. Although the specification defines NumEntries as the
number of package elements, tolerate additional trailing elements because
ignoring them is safe and avoids rejecting padded firmware.

Revision is specified as a BYTE, but the parser assigns its 64-bit AML
Integer to an unsigned int before validating it. Reject values above U8_MAX
before conversion so, for example, 0x100000004 cannot truncate to revision
4.

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
Link: https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 2f7c09552566..17d88aae1c3c 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -793,6 +793,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	}
 
 	out_obj = (union acpi_object *) output.pointer;
+	if (out_obj->package.count < 2) {
+		pr_debug("Unexpected _CPC package count (%u) for CPU:%d\n",
+			 out_obj->package.count, pr->id);
+		goto out_buf_free;
+	}
 
 	cpc_ptr = kzalloc_obj(struct cpc_desc);
 	if (!cpc_ptr) {
@@ -803,12 +808,15 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	/* First entry is NumEntries. */
 	cpc_obj = &out_obj->package.elements[0];
 	if (cpc_obj->type == ACPI_TYPE_INTEGER)	{
-		num_ent = cpc_obj->integer.value;
-		if (num_ent <= 1) {
-			pr_debug("Unexpected _CPC NumEntries value (%d) for CPU:%d\n",
-				 num_ent, pr->id);
+		if (cpc_obj->integer.value < 2 ||
+		    cpc_obj->integer.value > out_obj->package.count) {
+			pr_debug("Invalid _CPC NumEntries (%llu) for package count (%u) on CPU:%d\n",
+				 cpc_obj->integer.value, out_obj->package.count,
+				 pr->id);
 			goto out_free;
 		}
+
+		num_ent = cpc_obj->integer.value;
 	} else {
 		pr_debug("Unexpected _CPC NumEntries entry type (%d) for CPU:%d\n",
 			 cpc_obj->type, pr->id);
@@ -818,6 +826,12 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	/* Second entry should be revision. */
 	cpc_obj = &out_obj->package.elements[1];
 	if (cpc_obj->type == ACPI_TYPE_INTEGER)	{
+		if (cpc_obj->integer.value > U8_MAX) {
+			pr_debug("Invalid _CPC Revision (%llu) for CPU:%d\n",
+				 cpc_obj->integer.value, pr->id);
+			ret = -EINVAL;
+			goto out_free;
+		}
 		cpc_rev = cpc_obj->integer.value;
 	} else {
 		pr_debug("Unexpected _CPC Revision entry type (%d) for CPU:%d\n",
-- 
2.34.1

[PATCH v3 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 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

[PATCH v3 03/15] ACPI: CPPC: Propagate performance-control write errors

[Christian Loehle]

cppc_set_perf() can skip malformed controls, discard cpc_write() failures,
and report success without programming the requested performance tuple.

Return every control-write error to the caller. For mixed PCC and non-PCC
performance controls, complete all requested non-PCC writes before taking
PCC ownership or changing its payload. A non-PCC failure therefore cannot
submit only the PCC portion of a request. Once ownership is held, stage
only PCC controls and mark the command pending after successful staging.

If PCC staging fails while another CPU has already staged a request, take
the exclusive PCC lock and abort the pending batch before returning. This
advances the write generation and wakes Phase-II waiters which would
otherwise wait indefinitely for a doorbell that no CPU will ring.

Cross-address-space updates cannot be atomic, but this ordering ensures a
known non-PCC failure never commits the PCC portion by itself.

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

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 19a3a71fee45..e511cf8987a1 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -373,13 +373,45 @@ 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)
+{
+	int i;
+
+	if (unlikely(ret)) {
+		for_each_possible_cpu(i) {
+			struct cpc_desc *desc = per_cpu(cpc_desc_ptr, i);
+
+			if (!desc)
+				continue;
+
+			if (desc->write_cmd_id == pcc_ss_data->pcc_write_cnt)
+				desc->write_cmd_status = ret;
+		}
+	}
+
+	pcc_ss_data->pcc_write_cnt++;
+	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(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_data, ret);
+}
+
 /*
  * This function transfers the ownership of the PCC to the platform
  * So it must be called while holding write_lock(pcc_lock)
  */
 static int send_pcc_cmd(int pcc_ss_id, u16 cmd)
 {
-	int ret = -EIO, i;
+	int ret = -EIO;
 	struct cppc_pcc_data *pcc_ss_data = pcc_data[pcc_ss_id];
 	struct acpi_pcct_shared_memory __iomem *generic_comm_base =
 					pcc_ss_data->pcc_channel->shmem;
@@ -471,21 +503,8 @@ static int send_pcc_cmd(int pcc_ss_id, u16 cmd)
 		mbox_client_txdone(pcc_ss_data->pcc_channel->mchan, ret);
 
 end:
-	if (cmd == CMD_WRITE) {
-		if (unlikely(ret)) {
-			for_each_possible_cpu(i) {
-				struct cpc_desc *desc = per_cpu(cpc_desc_ptr, i);
-
-				if (!desc)
-					continue;
-
-				if (desc->write_cmd_id == pcc_ss_data->pcc_write_cnt)
-					desc->write_cmd_status = ret;
-			}
-		}
-		pcc_ss_data->pcc_write_cnt++;
-		wake_up_all(&pcc_ss_data->pcc_write_wait_q);
-	}
+	if (cmd == CMD_WRITE)
+		cppc_complete_pcc_write(pcc_ss_data, ret);
 
 	return ret;
 }
@@ -2116,7 +2135,7 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	struct cpc_register_resource *desired_reg, *min_perf_reg, *max_perf_reg;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
 	struct cppc_pcc_data *pcc_ss_data = NULL;
-	bool regs_in_pcc;
+	bool desired_pcc, min_pcc, max_pcc, regs_in_pcc;
 	int ret = 0;
 
 	if (!cpc_desc) {
@@ -2127,8 +2146,29 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 	desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF];
 	min_perf_reg = &cpc_desc->cpc_regs[MIN_PERF];
 	max_perf_reg = &cpc_desc->cpc_regs[MAX_PERF];
-	regs_in_pcc = CPC_IN_PCC(desired_reg) || CPC_IN_PCC(min_perf_reg) ||
-		      CPC_IN_PCC(max_perf_reg);
+	desired_pcc = cpc_is_writable(desired_reg) && CPC_IN_PCC(desired_reg);
+	min_pcc = perf_ctrls->min_perf && cpc_is_writable(min_perf_reg) &&
+		  CPC_IN_PCC(min_perf_reg);
+	max_pcc = perf_ctrls->max_perf && cpc_is_writable(max_perf_reg) &&
+		  CPC_IN_PCC(max_perf_reg);
+	regs_in_pcc = desired_pcc || min_pcc || max_pcc;
+
+	/* Do not stage PCC data if a fallible non-PCC write has failed. */
+	if (cpc_is_writable(desired_reg) && !desired_pcc) {
+		ret = cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+		if (ret)
+			return ret;
+	}
+	if (perf_ctrls->min_perf && cpc_is_writable(min_perf_reg) && !min_pcc) {
+		ret = cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
+		if (ret)
+			return ret;
+	}
+	if (perf_ctrls->max_perf && cpc_is_writable(max_perf_reg) && !max_pcc) {
+		ret = cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
+		if (ret)
+			return ret;
+	}
 
 	/*
 	 * This is Phase-I where we want to write to CPC registers
@@ -2151,30 +2191,37 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 				return ret;
 			}
 		}
-		/*
-		 * Update the pending_write to make sure a PCC CMD_READ will not
-		 * arrive and steal the channel during the switch to write lock
-		 */
-		pcc_ss_data->pending_pcc_write_cmd = true;
-		cpc_desc->write_cmd_id = pcc_ss_data->pcc_write_cnt;
-		cpc_desc->write_cmd_status = 0;
 	}
 
-	if (CPC_SUPPORTED(desired_reg))
-		cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+	if (desired_pcc) {
+		ret = cpc_write(cpu, desired_reg, perf_ctrls->desired_perf);
+		if (ret)
+			goto out_pcc_read_unlock;
+	}
 
 	/*
 	 * 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))
-		cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
-	if (perf_ctrls->max_perf && CPC_SUPPORTED(max_perf_reg))
-		cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
+	if (min_pcc) {
+		ret = cpc_write(cpu, min_perf_reg, perf_ctrls->min_perf);
+		if (ret)
+			goto out_pcc_read_unlock;
+	}
+	if (max_pcc) {
+		ret = cpc_write(cpu, max_perf_reg, perf_ctrls->max_perf);
+		if (ret)
+			goto out_pcc_read_unlock;
+	}
 
-	if (regs_in_pcc)
+	if (regs_in_pcc) {
+		/* Block a PCC read until the staged payload has been submitted. */
+		pcc_ss_data->pending_pcc_write_cmd = true;
+		cpc_desc->write_cmd_id = pcc_ss_data->pcc_write_cnt;
+		cpc_desc->write_cmd_status = 0;
 		up_read(&pcc_ss_data->pcc_lock);	/* END Phase-I */
+	}
 	/*
 	 * This is Phase-II where we transfer the ownership of PCC to Platform
 	 *
@@ -2233,9 +2280,17 @@ int cppc_set_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls)
 				   cpc_desc->write_cmd_id != pcc_ss_data->pcc_write_cnt);
 
 		/* send_pcc_cmd updates the status in case of failure */
-		ret = cpc_desc->write_cmd_status;
+		if (!ret)
+			ret = cpc_desc->write_cmd_status;
 	}
 	return ret;
+
+out_pcc_read_unlock:
+	up_read(&pcc_ss_data->pcc_lock);
+	down_write(&pcc_ss_data->pcc_lock);
+	cppc_abort_pending_pcc_write(pcc_ss_data, ret);
+	up_write(&pcc_ss_data->pcc_lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(cppc_set_perf);
 
-- 
2.34.1

[PATCH v3 04/15] ACPI: CPPC: Use 64-bit masks for register fields

[Christian Loehle]

MASK_VAL_READ() and MASK_VAL_WRITE() operate on u64 values but construct
their masks with GENMASK(), whose type is unsigned long. On 32-bit kernels
a field wider than 32 bits therefore produces an invalid shift or loses
its upper bits.

The Generic Address Structure permits QWord access units and the CPPC
accessors implement 64-bit MMIO reads and writes. Use GENMASK_ULL() in both
directions so the mask matches the value and supported access width on
every architecture.

Fixes: 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage")
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 | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index e511cf8987a1..8da853e42de0 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -235,10 +235,10 @@ show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time);
 
 /* Shift and apply the mask for CPC reads/writes */
 #define MASK_VAL_READ(reg, val) (((val) >> (reg)->bit_offset) &				\
-					GENMASK(((reg)->bit_width) - 1, 0))
+					GENMASK_ULL(((reg)->bit_width) - 1, 0))
 #define MASK_VAL_WRITE(reg, prev_val, val)						\
-	((((val) & GENMASK(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) |		\
-	((prev_val) & ~(GENMASK(((reg)->bit_width) - 1, 0) << (reg)->bit_offset)))	\
+	((((val) & GENMASK_ULL(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) |	\
+	((prev_val) & ~(GENMASK_ULL(((reg)->bit_width) - 1, 0) << (reg)->bit_offset))) \
 
 static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg)
 {
-- 
2.34.1

[PATCH v3 05/15] 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.

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.

Fixes: e05c75072c2e ("ACPI: CPPC: Add cppc_set_reg_val()")
Reported-by: Sashiko <sashiko-bot@kernel.org>
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@arm.com>
---
 drivers/acpi/cppc_acpi.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 8da853e42de0..3e726da8d564 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1473,7 +1473,7 @@ 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) {
@@ -1481,15 +1481,26 @@ static int cppc_set_reg_val_in_pcc(int cpu, struct cpc_register_resource *reg, u
 		return -ENODEV;
 	}
 
-	ret = cpc_write(cpu, reg, val);
-	if (ret)
-		return ret;
-
 	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_data, ret);
 	up_write(&pcc_ss_data->pcc_lock);
 
 	return ret;
-- 
2.34.1

[PATCH v3 06/15] ACPI: CPPC: Serialize PCC EPP payload updates

[Christian Loehle]

cppc_set_epp_perf() stages Autonomous Selection and Energy Performance
Preference in the PCC shared region before taking pcc_lock. The platform
may still own the subspace, or a concurrent command may consume or
overwrite only part of the new payload.

Take the PCC write lock and wait for OSPM ownership before staging either
control. Keep the lock held until the complete payload has been submitted
with CMD_WRITE, so firmware cannot observe a mixed transaction.

For a mixed PCC/non-PCC description, complete every fallible non-PCC write
before staging PCC data. Cross-address-space updates cannot be atomic, but
a non-PCC failure can no longer leave an unsent value in shared memory for
a later PCC command to consume.

Classify every probe-validated writable control as either PCC or non-PCC.
This covers SystemIO along with FFH and SystemMemory and avoids repeating a
flexible-address-space _OSC decision that probe has already made.

If ownership acquisition or PCC staging fails, abort any older pending
performance batch before releasing the exclusive lock so its Phase-II
waiters receive the error instead of sleeping indefinitely.

This follows the PCC ownership sequence in ACPI 6.5 Section 14.5.

Fixes: 7bc1fcd39901 ("ACPI: CPPC: Add AMD pstate energy performance preference cppc control")
Reported-by: Sashiko <sashiko-bot@kernel.org>
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@arm.com>
---
 drivers/acpi/cppc_acpi.c | 74 ++++++++++++++++++++++++----------------
 1 file changed, 44 insertions(+), 30 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 3e726da8d564..2b5da5c39cdd 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -1848,8 +1848,10 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable)
 	struct cpc_register_resource *auto_sel_reg;
 	struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu);
 	struct cppc_pcc_data *pcc_ss_data = NULL;
-	bool autosel_ffh_sysmem;
-	bool epp_ffh_sysmem;
+	bool auto_sel_pcc;
+	bool auto_sel_non_pcc;
+	bool epp_pcc;
+	bool epp_non_pcc;
 	int ret;
 
 	if (!cpc_desc) {
@@ -1860,52 +1862,64 @@ 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];
 
-	epp_ffh_sysmem = CPC_SUPPORTED(epp_set_reg) &&
-		(CPC_IN_FFH(epp_set_reg) || CPC_IN_SYSTEM_MEMORY(epp_set_reg));
-	autosel_ffh_sysmem = CPC_SUPPORTED(auto_sel_reg) &&
-		(CPC_IN_FFH(auto_sel_reg) || CPC_IN_SYSTEM_MEMORY(auto_sel_reg));
+	auto_sel_pcc = cpc_is_writable(auto_sel_reg) &&
+		CPC_IN_PCC(auto_sel_reg);
+	epp_pcc = cpc_is_writable(epp_set_reg) && CPC_IN_PCC(epp_set_reg);
+	auto_sel_non_pcc = cpc_is_writable(auto_sel_reg) && !auto_sel_pcc;
+	epp_non_pcc = cpc_is_writable(epp_set_reg) && !epp_pcc;
 
-	if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_reg)) {
+	/* Complete fallible non-PCC writes before staging PCC data. */
+	if (auto_sel_non_pcc) {
+		ret = cpc_write(cpu, auto_sel_reg, enable);
+		if (ret)
+			return ret;
+	}
+	if (epp_non_pcc) {
+		ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
+		if (ret)
+			return ret;
+	}
+
+	if (epp_pcc || auto_sel_pcc) {
 		if (pcc_ss_id < 0) {
 			pr_debug("Invalid pcc_ss_id for CPU:%d\n", cpu);
 			return -ENODEV;
 		}
 
-		if (cpc_is_writable(auto_sel_reg)) {
+		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_unlock;
+
+		if (auto_sel_pcc) {
 			ret = cpc_write(cpu, auto_sel_reg, enable);
 			if (ret)
-				return ret;
+				goto out_unlock;
 		}
 
-		if (cpc_is_writable(epp_set_reg)) {
+		if (epp_pcc) {
 			ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf);
 			if (ret)
-				return ret;
+				goto out_unlock;
 		}
 
-		pcc_ss_data = pcc_data[pcc_ss_id];
-
-		down_write(&pcc_ss_data->pcc_lock);
 		/* after writing CPC, transfer the ownership of PCC to platform */
 		ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE);
-		up_write(&pcc_ss_data->pcc_lock);
-	} else if (osc_cpc_flexible_adr_space_confirmed &&
-		   (epp_ffh_sysmem || autosel_ffh_sysmem)) {
-		if (autosel_ffh_sysmem) {
-			ret = cpc_write(cpu, auto_sel_reg, enable);
-			if (ret)
-				return ret;
-		}
 
-		if (epp_ffh_sysmem) {
-			ret = cpc_write(cpu, epp_set_reg,
-					perf_ctrls->energy_perf);
-			if (ret)
-				return ret;
-		}
+out_unlock:
+		if (ret)
+			cppc_abort_pending_pcc_write(pcc_ss_data, ret);
+		up_write(&pcc_ss_data->pcc_lock);
+	} else if (epp_non_pcc || auto_sel_non_pcc) {
+		ret = 0;
 	} else {
-		ret = -ENOTSUPP;
-		pr_debug("_CPC in PCC/FFH/SystemMemory are not supported\n");
+		ret = -EOPNOTSUPP;
+		pr_debug("No writable EPP controls for CPU:%d\n", cpu);
 	}
 
 	return ret;
-- 
2.34.1

[PATCH v3 07/15] ACPI: CPPC: Release CPC descriptors through kobject

[Christian Loehle]

The CPC descriptor embeds the kobject used for its sysfs directory, but
it has no release callback and processor exit frees the descriptor
immediately after kobject_put(). It also unmaps register resources and
releases PCC state before sysfs removal has drained active attribute
callbacks.

Provide a release callback which unmaps and frees the descriptor, and use
the same cleanup helper before kobject initialization. Once initialized,
let kobject_put() own descriptor cleanup so its error path cannot free the
object twice.

On processor exit, first unpublish the per-CPU descriptor and remove its
sysfs directory, then release PCC state and drop the kobject reference.
This keeps all resources valid while sysfs callbacks are active and also
works with delayed kobject release.

Fixes: 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance")
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 | 50 +++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 21 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 2b5da5c39cdd..826e27f61530 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -337,7 +337,27 @@ static struct attribute *cppc_attrs[] = {
 };
 ATTRIBUTE_GROUPS(cppc);
 
+static void cppc_free_desc(struct cpc_desc *cpc_ptr)
+{
+	unsigned int i;
+
+	for (i = 2; i < cpc_ptr->num_entries; i++) {
+		void __iomem *addr = cpc_ptr->cpc_regs[i - 2].sys_mem_vaddr;
+
+		if (addr)
+			iounmap(addr);
+	}
+
+	kfree(cpc_ptr);
+}
+
+static void cppc_kobj_release(struct kobject *kobj)
+{
+	cppc_free_desc(to_cpc_desc(kobj));
+}
+
 static const struct kobj_type cppc_ktype = {
+	.release = cppc_kobj_release,
 	.sysfs_ops = &kobj_sysfs_ops,
 	.default_groups = cppc_groups,
 };
@@ -1120,7 +1140,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	if (ret) {
 		per_cpu(cpc_desc_ptr, pr->id) = NULL;
 		kobject_put(&cpc_ptr->kobj);
-		goto out_free;
+		goto out_buf_free;
 	}
 
 	kfree(output.pointer);
@@ -1128,15 +1148,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 
 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;
-
-		if (addr)
-			iounmap(addr);
-	}
-	kfree(cpc_ptr);
+	cppc_free_desc(cpc_ptr);
 
 out_buf_free:
 	kfree(output.pointer);
@@ -1153,10 +1165,14 @@ EXPORT_SYMBOL_GPL(acpi_cppc_processor_probe);
 void acpi_cppc_processor_exit(struct acpi_processor *pr)
 {
 	struct cpc_desc *cpc_ptr;
-	unsigned int i;
-	void __iomem *addr;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
 
+	cpc_ptr = per_cpu(cpc_desc_ptr, pr->id);
+	if (cpc_ptr) {
+		per_cpu(cpc_desc_ptr, pr->id) = NULL;
+		kobject_del(&cpc_ptr->kobj);
+	}
+
 	if (pcc_ss_id >= 0 && pcc_data[pcc_ss_id]) {
 		if (pcc_data[pcc_ss_id]->pcc_channel_acquired) {
 			pcc_data[pcc_ss_id]->refcount--;
@@ -1167,20 +1183,12 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
 			}
 		}
 	}
+	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
 
-	cpc_ptr = per_cpu(cpc_desc_ptr, pr->id);
 	if (!cpc_ptr)
 		return;
 
-	/* Free all the mapped sys mem areas for this CPU */
-	for (i = 2; i < cpc_ptr->num_entries; i++) {
-		addr = cpc_ptr->cpc_regs[i-2].sys_mem_vaddr;
-		if (addr)
-			iounmap(addr);
-	}
-
 	kobject_put(&cpc_ptr->kobj);
-	kfree(cpc_ptr);
 }
 EXPORT_SYMBOL_GPL(acpi_cppc_processor_exit);
 
-- 
2.34.1

[PATCH v3 08/15] ACPI: CPPC: Release PCC data after probe failures

[Christian Loehle]

pcc_data_alloc() takes a per-processor reference while parsing the first
PCC register. Every later probe failure currently frees the CPC descriptor
without dropping that reference. Depending on where probe fails, this
leaks an unacquired PCC object, an acquired mailbox channel, or an extra
reference to a shared channel.

Factor the teardown into pcc_data_put() so it handles both acquired and
unacquired state, and invoke it on every failure after allocation.

Do not store the temporary pcc_data_alloc() result in the eventual probe
return value. A successful allocation must not make a later parsing failure
run cleanup and then return success.

The per-CPU PCC subspace index is zero-initialized. If probe returns before
assigning it, a later processor exit can consequently drop the reference
for subspace 0 even though this CPU never acquired one. Initialize the
index to -1 before any probe return and only release it from exit after a
CPC descriptor has been published.

Fixes: 85b1407bf6d2 ("ACPI / CPPC: Make CPPC ACPI driver aware of PCC subspace IDs")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 61 +++++++++++++++++++++++++++-------------
 1 file changed, 42 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 826e27f61530..ef1f716860b3 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -807,6 +807,24 @@ static int pcc_data_alloc(int pcc_ss_id)
 	return 0;
 }
 
+static void pcc_data_put(int pcc_ss_id)
+{
+	struct cppc_pcc_data *data;
+
+	if (pcc_ss_id < 0 || pcc_ss_id >= MAX_PCC_SUBSPACES)
+		return;
+
+	data = pcc_data[pcc_ss_id];
+	if (!data || --data->refcount)
+		return;
+
+	if (data->pcc_channel_acquired)
+		pcc_mbox_free_channel(data->pcc_channel);
+
+	kfree(data);
+	pcc_data[pcc_ss_id] = NULL;
+}
+
 /*
  * An example CPC table looks like the following.
  *
@@ -852,8 +870,12 @@ 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 pcc_data_ref = false;
 	acpi_status status;
 	int ret = -ENODATA;
+	int err;
+
+	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
 
 	if (!osc_sb_cppc2_support_acked) {
 		pr_debug("CPPC v2 _OSC not acked\n");
@@ -986,8 +1008,12 @@ 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;
+					}
+					pcc_data_ref = true;
 				} else if (pcc_subspace_id != gas_t->access_width) {
 					pr_debug("Mismatched PCC ids in _CPC for CPU:%d\n",
 						 pr->id);
@@ -1140,7 +1166,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	if (ret) {
 		per_cpu(cpc_desc_ptr, pr->id) = NULL;
 		kobject_put(&cpc_ptr->kobj);
-		goto out_buf_free;
+		goto out_pcc_put;
 	}
 
 	kfree(output.pointer);
@@ -1150,6 +1176,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	pr_err("CPU%d: failed to initialize _CPC: %d\n", pr->id, ret);
 	cppc_free_desc(cpc_ptr);
 
+out_pcc_put:
+	if (pcc_data_ref)
+		pcc_data_put(pcc_subspace_id);
+	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
+
 out_buf_free:
 	kfree(output.pointer);
 	return ret;
@@ -1165,28 +1196,20 @@ EXPORT_SYMBOL_GPL(acpi_cppc_processor_probe);
 void acpi_cppc_processor_exit(struct acpi_processor *pr)
 {
 	struct cpc_desc *cpc_ptr;
-	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
+	int pcc_ss_id;
 
 	cpc_ptr = per_cpu(cpc_desc_ptr, pr->id);
-	if (cpc_ptr) {
-		per_cpu(cpc_desc_ptr, pr->id) = NULL;
-		kobject_del(&cpc_ptr->kobj);
+	if (!cpc_ptr) {
+		per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
+		return;
 	}
 
-	if (pcc_ss_id >= 0 && pcc_data[pcc_ss_id]) {
-		if (pcc_data[pcc_ss_id]->pcc_channel_acquired) {
-			pcc_data[pcc_ss_id]->refcount--;
-			if (!pcc_data[pcc_ss_id]->refcount) {
-				pcc_mbox_free_channel(pcc_data[pcc_ss_id]->pcc_channel);
-				kfree(pcc_data[pcc_ss_id]);
-				pcc_data[pcc_ss_id] = NULL;
-			}
-		}
-	}
-	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
+	pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
+	per_cpu(cpc_desc_ptr, pr->id) = NULL;
+	kobject_del(&cpc_ptr->kobj);
 
-	if (!cpc_ptr)
-		return;
+	pcc_data_put(pcc_ss_id);
+	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
 
 	kobject_put(&cpc_ptr->kobj);
 }
-- 
2.34.1

[PATCH v3 09/15] ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW

[Christian Loehle]

A SystemMemory CPPC field narrower than its access unit needs a
read-modify-write operation. The existing per-CPC-descriptor lock
serializes such fields within one _CPC package, but not access units
described by different processors. Concurrent writers can therefore lose
updates.

The ACPI specification does not make _PSD a physical register-ownership
boundary. It can legally describe shared or packed registers across
processors and performance domains. Supporting every such layout would
require locks keyed by physical access unit.

Retain the deliberately cheaper per-descriptor locking model and enforce
its assumptions at probe instead. Reject overlapping access units from
different descriptors when both contain writable partial fields,
including exact partial aliases. Such layouts were never safely writable
by Linux, so this turns possible corruption into a probe error rather than
removing working support.

Within one descriptor, disjoint partial writable fields remain supported
and share its raw spinlock. Across descriptors, exact full-width writable
aliases remain supported and lockless when the architecture provides a
native access of the declared width. Reject 64-bit writable aliases on
32-bit kernels because generic writeq() may use two 32-bit stores and is
not a portable atomicity guarantee. Read-only 64-bit aliases remain
supported on every architecture.

Read-only overlaps, and a partial writable field sharing an access unit
with a disjoint read-only field, also remain supported. Reject overlapping
logical fields when either is writable and reject any other field sharing
a full-width writable access unit.

Validate the GAS geometry and require naturally aligned 8-, 16-, 32-, or
64-bit SystemMemory access units. The driver uses direct MMIO accessors and
cannot portably execute unaligned accesses. Keep an interval tree solely
for scalable probe-time overlap validation; the full-width scheduler path
does no lookup and takes no lock.

Keep validation errors separate from the probe return value. A successful
SystemMemory geometry check must not overwrite the pending negative error,
because a later mapping or address-space failure still needs to make probe
fail.

Fixes: 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage")
Reported-by: Sashiko <sashiko-bot@kernel.org>
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@arm.com>
---
 drivers/acpi/cppc_acpi.c | 348 ++++++++++++++++++++++++++++++++++-----
 include/acpi/cppc_acpi.h |   5 +-
 2 files changed, 315 insertions(+), 38 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index ef1f716860b3..8967ffdac76f 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -34,8 +34,11 @@
 #define pr_fmt(fmt)	"ACPI CPPC: " fmt
 
 #include <linux/delay.h>
+#include <linux/interval_tree_generic.h>
 #include <linux/iopoll.h>
 #include <linux/ktime.h>
+#include <linux/mutex.h>
+#include <linux/rbtree.h>
 #include <linux/rwsem.h>
 #include <linux/wait.h>
 #include <linux/topology.h>
@@ -93,6 +96,37 @@ static DEFINE_PER_CPU(int, cpu_pcc_subspace_idx);
  */
 static DEFINE_PER_CPU(struct cpc_desc *, cpc_desc_ptr);
 
+struct cpc_sysmem_node {
+	struct rb_node rb;
+	u64 subtree_last;
+	u64 start;
+	u64 last;
+	struct cpc_desc *desc;
+	unsigned int reg_idx;
+	bool registered;
+};
+
+#define CPC_SYSMEM_START(node) ((node)->start)
+#define CPC_SYSMEM_LAST(node) ((node)->last)
+
+INTERVAL_TREE_DEFINE(struct cpc_sysmem_node, rb, u64, subtree_last,
+		     CPC_SYSMEM_START, CPC_SYSMEM_LAST, static inline,
+		     cpc_sysmem_itree)
+
+static struct rb_root_cached cpc_sysmem_tree = RB_ROOT_CACHED;
+static DEFINE_MUTEX(cpc_sysmem_lock);
+
+static struct cpc_sysmem_node *cpc_sysmem_first(u64 start, u64 last)
+{
+	return cpc_sysmem_itree_iter_first(&cpc_sysmem_tree, start, last);
+}
+
+static struct cpc_sysmem_node *cpc_sysmem_next(struct cpc_sysmem_node *node,
+					       u64 start, u64 last)
+{
+	return cpc_sysmem_itree_iter_next(node, start, last);
+}
+
 /* pcc mapped address + header size + offset within PCC subspace */
 #define GET_PCC_VADDR(offs, pcc_ss_id) (pcc_data[pcc_ss_id]->pcc_channel->shmem + \
 						0x8 + (offs))
@@ -240,15 +274,20 @@ show_cppc_data(cppc_get_perf_ctrs, cppc_perf_fb_ctrs, wraparound_time);
 	((((val) & GENMASK_ULL(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) |	\
 	((prev_val) & ~(GENMASK_ULL(((reg)->bit_width) - 1, 0) << (reg)->bit_offset))) \
 
-static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg)
+static unsigned int cpc_reg_access_width(const struct cpc_reg *reg)
 {
-	const struct cpc_reg *gas = &reg->cpc_entry.reg;
-	unsigned int width;
-
-	if (gas->access_width > 4)
+	if (reg->access_width > 4)
 		return 0;
 
-	width = GET_BIT_WIDTH(gas);
+	if (reg->access_width)
+		return 8U << (reg->access_width - 1);
+
+	return reg->bit_width;
+}
+
+static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg)
+{
+	unsigned int width = cpc_reg_access_width(&reg->cpc_entry.reg);
 
 	if (width != 8 && width != 16 && width != 32 && width != 64)
 		return 0;
@@ -274,36 +313,260 @@ static bool cpc_sysmem_access_units_overlap(const struct cpc_register_resource *
 	return a_gas->address - b_gas->address < b_size;
 }
 
+static bool cpc_reg_is_writable(unsigned int reg_idx)
+{
+	switch (reg_idx) {
+	case DESIRED_PERF:
+	case MIN_PERF:
+	case MAX_PERF:
+	case PERF_REDUC_TOLERANCE:
+	case TIME_WINDOW:
+	case PERF_LIMITED:
+	case ENABLE:
+	case AUTO_SEL_ENABLE:
+	case AUTO_ACT_WINDOW:
+	case ENERGY_PERF:
+	case OSPM_NOMINAL_PERF:
+		return true;
+	default:
+		return false;
+	}
+}
+
+static bool cpc_sysmem_reg_needs_rmw(const struct cpc_register_resource *reg)
+{
+	const struct cpc_reg *gas = &reg->cpc_entry.reg;
+	u64 access_size = cpc_sysmem_access_size(reg);
+
+	return gas->bit_offset || gas->bit_width != access_size * 8;
+}
+
+static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
+				   const struct cpc_reg *gas,
+				   unsigned int reg_idx)
+{
+	unsigned int access_width = cpc_reg_access_width(gas);
+	u64 access_size;
+
+	if (access_width != 8 && access_width != 16 &&
+	    access_width != 32 && access_width != 64)
+		goto invalid;
+
+	if (!gas->bit_width || gas->bit_width > access_width ||
+	    gas->bit_offset >= access_width ||
+	    gas->bit_width > access_width - gas->bit_offset)
+		goto invalid;
+
+	access_size = access_width / 8;
+	if (!gas->address || gas->address > U64_MAX - (access_size - 1))
+		goto invalid;
+	if (gas->address & (access_size - 1))
+		goto invalid;
+
+	return 0;
+
+invalid:
+	pr_debug("CPU:%d invalid SystemMemory GAS for _CPC register %u\n",
+		 cpc_desc->cpu_id, reg_idx);
+	return -EINVAL;
+}
+
 static void cpc_mark_rmw_lock_users(struct cpc_desc *cpc_desc)
 {
-	int i, j;
+	int i;
 
 	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
-		struct cpc_register_resource *a = &cpc_desc->cpc_regs[i];
-		struct cpc_reg *gas;
-		u64 access_size;
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
 
-		if (!CPC_SUPPORTED(a) || !CPC_IN_SYSTEM_MEMORY(a))
-			continue;
+		if (CPC_SUPPORTED(reg) && CPC_IN_SYSTEM_MEMORY(reg))
+			reg->cpc_entry.use_rmw_lock =
+				cpc_sysmem_reg_needs_rmw(reg);
+	}
+}
 
-		gas = &a->cpc_entry.reg;
-		access_size = cpc_sysmem_access_size(a);
-		if (gas->bit_offset || !access_size ||
-		    gas->bit_width != access_size * 8)
-			a->cpc_entry.use_rmw_lock = true;
+struct cpc_bit_position {
+	u64 byte;
+	u8 bit;
+};
 
-		for (j = i + 1; j < cpc_desc->num_entries - 2; j++) {
-			struct cpc_register_resource *b = &cpc_desc->cpc_regs[j];
+static bool cpc_bit_position_before(const struct cpc_bit_position *a,
+				    const struct cpc_bit_position *b)
+{
+	return a->byte < b->byte || (a->byte == b->byte && a->bit < b->bit);
+}
 
-			if (!CPC_SUPPORTED(b) || !CPC_IN_SYSTEM_MEMORY(b))
-				continue;
-			if (!cpc_sysmem_access_units_overlap(a, b))
-				continue;
+static bool cpc_sysmem_fields_overlap(const struct cpc_register_resource *a,
+				      const struct cpc_register_resource *b)
+{
+	const struct cpc_reg *a_gas = &a->cpc_entry.reg;
+	const struct cpc_reg *b_gas = &b->cpc_entry.reg;
+	unsigned int a_last_bit = a_gas->bit_offset + a_gas->bit_width - 1;
+	unsigned int b_last_bit = b_gas->bit_offset + b_gas->bit_width - 1;
+	struct cpc_bit_position a_start = {
+		.byte = a_gas->address + a_gas->bit_offset / 8,
+		.bit = a_gas->bit_offset % 8,
+	};
+	struct cpc_bit_position a_end = {
+		.byte = a_gas->address + a_last_bit / 8,
+		.bit = a_last_bit % 8,
+	};
+	struct cpc_bit_position b_start = {
+		.byte = b_gas->address + b_gas->bit_offset / 8,
+		.bit = b_gas->bit_offset % 8,
+	};
+	struct cpc_bit_position b_end = {
+		.byte = b_gas->address + b_last_bit / 8,
+		.bit = b_last_bit % 8,
+	};
+
+	return !cpc_bit_position_before(&a_end, &b_start) &&
+	       !cpc_bit_position_before(&b_end, &a_start);
+}
+
+static bool cpc_same_sysmem_register(unsigned int a_idx,
+				     const struct cpc_register_resource *a,
+				     unsigned int b_idx,
+				     const struct cpc_register_resource *b)
+{
+	const struct cpc_reg *a_gas = &a->cpc_entry.reg;
+	const struct cpc_reg *b_gas = &b->cpc_entry.reg;
+
+	return a_idx == b_idx &&
+	       a_gas->address == b_gas->address &&
+	       a_gas->bit_width == b_gas->bit_width &&
+	       a_gas->bit_offset == b_gas->bit_offset &&
+	       cpc_reg_access_width(a_gas) == cpc_reg_access_width(b_gas);
+}
 
-			a->cpc_entry.use_rmw_lock = true;
-			b->cpc_entry.use_rmw_lock = true;
+static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc,
+				    unsigned int a_idx,
+				    const struct cpc_desc *b_desc,
+				    unsigned int b_idx)
+{
+	const struct cpc_register_resource *a = &a_desc->cpc_regs[a_idx];
+	const struct cpc_register_resource *b = &b_desc->cpc_regs[b_idx];
+	bool a_writable, b_writable;
+
+	if (!CPC_SUPPORTED(a) || !CPC_IN_SYSTEM_MEMORY(a) ||
+	    !CPC_SUPPORTED(b) || !CPC_IN_SYSTEM_MEMORY(b) ||
+	    !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);
+	if (!a_writable && !b_writable)
+		return 0;
+
+	if (cpc_same_sysmem_register(a_idx, a, b_idx, b)) {
+		/*
+		 * Cross-CPU partial writes were never safely serialized, and a
+		 * 64-bit MMIO write may be split on 32-bit kernels.
+		 */
+		if (!a_writable ||
+		    (!cpc_sysmem_reg_needs_rmw(a) &&
+		     (cpc_sysmem_access_size(a) < sizeof(u64) ||
+		      IS_ENABLED(CONFIG_64BIT))))
+			return 0;
+		goto conflict;
+	}
+
+	/*
+	 * A full-width writable register owns its access unit.  It cannot
+	 * coexist with another logical field because it has no bits to
+	 * preserve, even when that other field is read-only.
+	 */
+	if ((a_writable && !cpc_sysmem_reg_needs_rmw(a)) ||
+	    (b_writable && !cpc_sysmem_reg_needs_rmw(b)) ||
+	    cpc_sysmem_fields_overlap(a, b) ||
+	    (a_desc != b_desc && a_writable && b_writable))
+		goto conflict;
+
+	return 0;
+
+conflict:
+	pr_err("CPU%d: SystemMemory _CPC register %u conflicts with CPU%d register %u\n",
+	       a_desc->cpu_id, a_idx, b_desc->cpu_id, b_idx);
+	return -EINVAL;
+}
+
+static void cpc_unregister_sysmem_desc_locked(struct cpc_desc *cpc_desc)
+{
+	unsigned int i;
+
+	if (!cpc_desc->sysmem_nodes)
+		return;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		struct cpc_sysmem_node *node = &cpc_desc->sysmem_nodes[i];
+
+		if (node->registered)
+			cpc_sysmem_itree_remove(node, &cpc_sysmem_tree);
+	}
+
+	kfree(cpc_desc->sysmem_nodes);
+	cpc_desc->sysmem_nodes = NULL;
+}
+
+static int cpc_register_sysmem_desc(struct cpc_desc *cpc_desc)
+{
+	unsigned int nr_regs = cpc_desc->num_entries - 2;
+	unsigned int i;
+	int ret = 0;
+
+	cpc_desc->sysmem_nodes = kcalloc(nr_regs,
+					 sizeof(*cpc_desc->sysmem_nodes),
+					 GFP_KERNEL);
+	if (!cpc_desc->sysmem_nodes)
+		return -ENOMEM;
+
+	mutex_lock(&cpc_sysmem_lock);
+
+	for (i = 0; i < nr_regs; i++) {
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
+		struct cpc_sysmem_node *match, *node;
+		u64 size;
+
+		if (!CPC_SUPPORTED(reg) || !CPC_IN_SYSTEM_MEMORY(reg))
+			continue;
+
+		node = &cpc_desc->sysmem_nodes[i];
+		size = cpc_sysmem_access_size(reg);
+		node->start = reg->cpc_entry.reg.address;
+		node->last = node->start + size - 1;
+		node->desc = cpc_desc;
+		node->reg_idx = i;
+
+		match = cpc_sysmem_first(node->start, node->last);
+		while (match) {
+			ret = cpc_validate_sysmem_pair(cpc_desc, i, match->desc,
+						       match->reg_idx);
+			if (ret)
+				goto out_unregister;
+
+			match = cpc_sysmem_next(match, node->start, node->last);
 		}
+
+		cpc_sysmem_itree_insert(node, &cpc_sysmem_tree);
+		node->registered = true;
 	}
+
+	mutex_unlock(&cpc_sysmem_lock);
+	return 0;
+
+out_unregister:
+	cpc_unregister_sysmem_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_sysmem_lock);
+	return ret;
+}
+
+static void cpc_unregister_sysmem_desc(struct cpc_desc *cpc_desc)
+{
+	if (!cpc_desc->sysmem_nodes)
+		return;
+
+	mutex_lock(&cpc_sysmem_lock);
+	cpc_unregister_sysmem_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_sysmem_lock);
 }
 
 static ssize_t show_feedback_ctrs(struct kobject *kobj,
@@ -341,6 +604,8 @@ static void cppc_free_desc(struct cpc_desc *cpc_ptr)
 {
 	unsigned int i;
 
+	cpc_unregister_sysmem_desc(cpc_ptr);
+
 	for (i = 2; i < cpc_ptr->num_entries; i++) {
 		void __iomem *addr = cpc_ptr->cpc_regs[i - 2].sys_mem_vaddr;
 
@@ -905,6 +1170,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 		ret = -ENOMEM;
 		goto out_buf_free;
 	}
+	cpc_ptr->cpu_id = pr->id;
 
 	/* First entry is NumEntries. */
 	cpc_obj = &out_obj->package.elements[0];
@@ -1020,21 +1286,28 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					goto out_free;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
-				if (gas_t->address) {
+				if (!IS_NULL_REG(gas_t)) {
 					void __iomem *addr;
 					size_t access_width;
 
+					err = cpc_validate_sysmem_reg(cpc_ptr, gas_t, i - 2);
+					if (err) {
+						ret = err;
+						goto out_free;
+					}
+
 					if (!osc_cpc_flexible_adr_space_confirmed) {
 						pr_debug("Flexible address space capability not supported\n");
 						if (!cpc_supported_by_cpu())
 							goto out_free;
 					}
 
-					access_width = GET_BIT_WIDTH(gas_t) / 8;
+					access_width = cpc_reg_access_width(gas_t);
+					access_width /= 8;
 					addr = ioremap(gas_t->address, access_width);
 					if (!addr)
 						goto out_free;
-					cpc_ptr->cpc_regs[i-2].sys_mem_vaddr = addr;
+					cpc_ptr->cpc_regs[i - 2].sys_mem_vaddr = addr;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 				if (gas_t->access_width < 1 || gas_t->access_width > 3) {
@@ -1067,8 +1340,9 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 				}
 			}
 
-			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));
+			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
@@ -1128,8 +1402,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	}
 
 
-	/* Store CPU Logical ID */
-	cpc_ptr->cpu_id = pr->id;
 	cpc_mark_rmw_lock_users(cpc_ptr);
 	raw_spin_lock_init(&cpc_ptr->rmw_lock);
 
@@ -1138,6 +1410,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	if (ret)
 		goto out_free;
 
+	ret = cpc_register_sysmem_desc(cpc_ptr);
+	if (ret)
+		goto out_free;
+
 	/* Register PCC channel once for all PCC subspace ID. */
 	if (pcc_subspace_id >= 0 && !pcc_data[pcc_subspace_id]->pcc_channel_acquired) {
 		ret = register_pcc_channel(pcc_subspace_id);
@@ -1165,6 +1441,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			"acpi_cppc");
 	if (ret) {
 		per_cpu(cpc_desc_ptr, pr->id) = NULL;
+		cpc_unregister_sysmem_desc(cpc_ptr);
 		kobject_put(&cpc_ptr->kobj);
 		goto out_pcc_put;
 	}
@@ -1207,6 +1484,7 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
 	pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
 	per_cpu(cpc_desc_ptr, pr->id) = NULL;
 	kobject_del(&cpc_ptr->kobj);
+	cpc_unregister_sysmem_desc(cpc_ptr);
 
 	pcc_data_put(pcc_ss_id);
 	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
@@ -1378,11 +1656,7 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 				val, size);
 
 	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
-		/*
-		 * The _CPC layout is immutable after probe. The precomputed flag
-		 * retains serialization for partial fields or overlapping access
-		 * units; standalone full-width registers avoid the lock.
-		 */
+		/* Partial fields use their per-CPU descriptor lock. */
 		locked = reg_res->cpc_entry.use_rmw_lock;
 		if (locked) {
 			cpc_desc = per_cpu(cpc_desc_ptr, cpu);
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index 19830146c644..c4f47c312054 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -77,6 +77,8 @@ struct cpc_register_resource {
 	} cpc_entry;
 };
 
+struct cpc_sysmem_node;
+
 /* Container to hold the CPC details for each CPU */
 struct cpc_desc {
 	int num_entries;
@@ -84,10 +86,11 @@ struct cpc_desc {
 	int cpu_id;
 	int write_cmd_status;
 	int write_cmd_id;
-	/* Lock used for RMW operations in cpc_write() */
+	/* Serialize partial SystemMemory writes within this descriptor. */
 	raw_spinlock_t rmw_lock;
 	struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
 	struct acpi_psd_package domain_info;
+	struct cpc_sysmem_node *sysmem_nodes;
 	struct kobject kobj;
 };
 
-- 
2.34.1

[PATCH v3 10/15] ACPI: CPPC: Reject reads and RMW of write-only controls

[Christian Loehle]

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 bypass the revision
rule.

A partial SystemMemory field would also make cpc_write() read its complete
access unit to preserve bits outside the field. Reject revision-4
descriptions of either write-only control when their geometry requires RMW.
A full-width description remains supported and is written without a
preceding read.

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.

Fixes: 71e1815113f7 ("ACPI: CPPC: Add support for CPPC v4")
Reported-by: Sashiko <sashiko-bot@kernel.org>
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@arm.com>
---
 drivers/acpi/cppc_acpi.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 8967ffdac76f..26e4c83dc2df 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -333,6 +333,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 = &reg->cpc_entry.reg;
@@ -363,6 +378,17 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
 	if (gas->address & (access_size - 1))
 		goto invalid;
 
+	if (cpc_reg_is_write_only(cpc_desc, reg_idx) &&
+	    (gas->bit_offset || gas->bit_width != access_width)) {
+		const char *name = reg_idx == DESIRED_PERF ?
+				   "Desired Performance" :
+				   "OSPM Nominal Performance";
+
+		pr_err("CPU%d: _CPC v%d %s register requires unsupported read-modify-write\n",
+		       cpc_desc->cpu_id, cpc_desc->version, name);
+		return -EINVAL;
+	}
+
 	return 0;
 
 invalid:
@@ -1291,6 +1317,17 @@ 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;
@@ -1759,6 +1796,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];
 
-- 
2.34.1

[PATCH v3 11/15] ACPI: CPPC: Validate and access PCC register layouts

[Christian Loehle]

PCC GAS Access Size identifies the subspace, so the accessors use Bit Width
as the field size. Their typed readw/readl/readq operations require natural
alignment and support only 8, 16, 32, or 64 bits. This rejects valid
firmware: ACPI 6.6 shows a mandatory 32-bit Delivered Performance Counter
at unaligned offset 0x116, and performance controls may use byte-multiple
widths such as 24 bits.

Support zero-offset, byte-multiple fields up to 64 bits with byte-oriented
I/O and explicit little-endian encoding. A short per-subspace raw spinlock
serializes aliased payload copies performed concurrently under the shared
side of pcc_lock. Fields with a nonzero bit offset or non-byte width still
need unsupported RMW semantics and are rejected or disabled.

Defer that decision until the entire package is known. A provided but
inaccessible CPPC Enable control fails probe because OSPM must write it
before using CPPC. An inaccessible Desired control can be ignored only for
immutable autonomous operation. Inaccessible Minimum or Maximum controls
fail probe, and the post-sanitization check enforces the specification
requirement that both controls exist or neither does. Other inaccessible
optional controls are marked absent; mandatory controls fail.

Keep the eventual probe result negative while temporary validation helpers
succeed. This prevents a later malformed entry, unsupported address space,
or mapping failure from freeing the descriptor and nevertheless returning
success.

Apply the same control-specific policy to unsupported SystemMemory
geometry. Once the PCC channel is available, verify every retained field
fits after the eight-byte header and within shared memory, and reject
overlapping logical ranges when either is writable.

This deliberately does not implement bit-level PCC RMW. In particular, the
one-bit CPPC Enable field in the specification example now produces an
explicit probe failure instead of being silently treated as absent.

Fixes: 85b1407bf6d2 ("ACPI / CPPC: Make CPPC ACPI driver aware of PCC subspace IDs")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 305 ++++++++++++++++++++++++++++++++-------
 1 file changed, 255 insertions(+), 50 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 26e4c83dc2df..03100fc9b993 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -73,6 +73,8 @@ struct cppc_pcc_data {
 	 *	Take write_lock for all purposes which gives exclusive access
 	 */
 	struct rw_semaphore pcc_lock;
+	/* Serialize byte-oriented accesses to aliased PCC payload fields. */
+	raw_spinlock_t payload_lock;
 
 	/* Wait queue for CPUs whose requests were batched */
 	wait_queue_head_t pcc_write_wait_q;
@@ -127,9 +129,11 @@ static struct cpc_sysmem_node *cpc_sysmem_next(struct cpc_sysmem_node *node,
 	return cpc_sysmem_itree_iter_next(node, start, last);
 }
 
+#define CPC_PCC_HEADER_SIZE	0x8
+
 /* pcc mapped address + header size + offset within PCC subspace */
 #define GET_PCC_VADDR(offs, pcc_ss_id) (pcc_data[pcc_ss_id]->pcc_channel->shmem + \
-						0x8 + (offs))
+						CPC_PCC_HEADER_SIZE + (offs))
 
 /* Check if a CPC register is in PCC */
 #define CPC_IN_PCC(cpc) ((cpc)->type == ACPI_TYPE_BUFFER &&		\
@@ -397,6 +401,170 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
 	return -EINVAL;
 }
 
+static bool cpc_immutable_autonomous(const struct cpc_desc *cpc_desc)
+{
+	const struct cpc_register_resource *reg;
+
+	reg = &cpc_desc->cpc_regs[AUTO_SEL_ENABLE];
+	return osc_sb_cppc2_support_acked && reg->type == ACPI_TYPE_INTEGER &&
+	       reg->cpc_entry.int_value == 1;
+}
+
+static int cpc_resolve_unsupported(struct cpc_desc *cpc_desc,
+				   u32 unsupported)
+{
+	unsigned int i;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		if (!(unsupported & BIT(i)))
+			continue;
+
+		if (i == DESIRED_PERF && cpc_immutable_autonomous(cpc_desc)) {
+			pr_warn("CPU%d: ignoring inaccessible Desired Performance register in autonomous mode\n",
+				cpc_desc->cpu_id);
+			cpc_disable_reg(cpc_desc, i);
+			continue;
+		}
+
+		/* A present Enable control must be usable to enter CPPC mode. */
+		if (i == ENABLE || i == MIN_PERF || i == MAX_PERF ||
+		    !IS_OPTIONAL_CPC_REG(i)) {
+			pr_err("CPU%d: cannot access _CPC register %u\n",
+			       cpc_desc->cpu_id, i);
+			return -EINVAL;
+		}
+
+		pr_warn("CPU%d: ignoring inaccessible optional _CPC register %u\n",
+			cpc_desc->cpu_id, i);
+		cpc_disable_reg(cpc_desc, i);
+	}
+
+	return 0;
+}
+
+static int cpc_validate_required_controls(struct cpc_desc *cpc_desc)
+{
+	bool have_min, have_max;
+	unsigned int i;
+
+	/*
+	 * 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 < cpc_desc->num_entries - 2; i++) {
+		if (i != DESIRED_PERF && i != PERF_LIMITED &&
+		    !IS_OPTIONAL_CPC_REG(i) &&
+		    !cpc_entry_present(&cpc_desc->cpc_regs[i])) {
+			pr_debug("CPU:%d lacks mandatory _CPC register %u\n",
+				 cpc_desc->cpu_id, i);
+			return -EINVAL;
+		}
+	}
+
+	/* Desired may be absent only for immutable autonomous operation. */
+	if (!cpc_is_writable(&cpc_desc->cpc_regs[DESIRED_PERF]) &&
+	    !cpc_immutable_autonomous(cpc_desc)) {
+		pr_debug("CPU:%d lacks a writable Desired Performance register\n",
+			 cpc_desc->cpu_id);
+		return -EINVAL;
+	}
+
+	have_min = cpc_is_writable(&cpc_desc->cpc_regs[MIN_PERF]);
+	have_max = cpc_is_writable(&cpc_desc->cpc_regs[MAX_PERF]);
+	if (have_min != have_max) {
+		pr_err("CPU%d: _CPC must provide both Minimum and Maximum Performance or neither\n",
+		       cpc_desc->cpu_id);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int cpc_validate_pcc_bounds(struct cpc_desc *cpc_desc,
+				   struct cppc_pcc_data *data,
+				   u32 *unsupported)
+{
+	u64 shmem_size = data->pcc_channel->shmem_size;
+	unsigned int i;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
+		struct cpc_reg *gas;
+		u64 access_size;
+
+		if ((*unsupported & BIT(i)) || !CPC_SUPPORTED(reg) ||
+		    !CPC_IN_PCC(reg))
+			continue;
+
+		gas = &reg->cpc_entry.reg;
+		access_size = gas->bit_width / 8;
+		if (shmem_size >= CPC_PCC_HEADER_SIZE &&
+		    gas->address <= shmem_size - CPC_PCC_HEADER_SIZE &&
+		    access_size <= shmem_size - CPC_PCC_HEADER_SIZE - gas->address)
+			continue;
+
+		pr_debug("CPU%d: _CPC register %u exceeds the PCC shared region\n",
+			 cpc_desc->cpu_id, i);
+		*unsupported |= BIT(i);
+	}
+
+	return 0;
+}
+
+static u64 cpc_non_mmio_access_size(const struct cpc_register_resource *reg)
+{
+	const struct cpc_reg *gas = &reg->cpc_entry.reg;
+
+	if (gas->space_id == ACPI_ADR_SPACE_PLATFORM_COMM)
+		return gas->bit_width / 8;
+
+	return cpc_reg_access_width(gas) / 8;
+}
+
+static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
+					  u8 space_id, const char *name)
+{
+	unsigned int i, j;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		struct cpc_register_resource *a = &cpc_desc->cpc_regs[i];
+		const struct cpc_reg *a_gas;
+		u64 a_size;
+
+		if (!CPC_SUPPORTED(a) || a->type != ACPI_TYPE_BUFFER ||
+		    a->cpc_entry.reg.space_id != space_id)
+			continue;
+
+		a_gas = &a->cpc_entry.reg;
+		a_size = cpc_non_mmio_access_size(a);
+		for (j = i + 1; j < cpc_desc->num_entries - 2; j++) {
+			struct cpc_register_resource *b = &cpc_desc->cpc_regs[j];
+			const struct cpc_reg *b_gas;
+			u64 b_size;
+
+			if (!CPC_SUPPORTED(b) || b->type != ACPI_TYPE_BUFFER ||
+			    b->cpc_entry.reg.space_id != space_id)
+				continue;
+
+			b_gas = &b->cpc_entry.reg;
+			b_size = cpc_non_mmio_access_size(b);
+			if (!cpc_reg_is_writable(i) && !cpc_reg_is_writable(j))
+				continue;
+			if (a_gas->address < b_gas->address ?
+			    b_gas->address - a_gas->address >= a_size :
+			    a_gas->address - b_gas->address >= b_size)
+				continue;
+
+			pr_err("CPU%d: overlapping writable %s _CPC registers %u and %u\n",
+			       cpc_desc->cpu_id, name, i, j);
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
 static void cpc_mark_rmw_lock_users(struct cpc_desc *cpc_desc)
 {
 	int i;
@@ -1092,6 +1260,7 @@ static int pcc_data_alloc(int pcc_ss_id)
 		pcc_data[pcc_ss_id] = kzalloc_obj(struct cppc_pcc_data);
 		if (!pcc_data[pcc_ss_id])
 			return -ENOMEM;
+		raw_spin_lock_init(&pcc_data[pcc_ss_id]->payload_lock);
 		pcc_data[pcc_ss_id]->refcount++;
 	}
 
@@ -1160,10 +1329,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	struct device *cpu_dev;
 	acpi_handle handle = pr->handle;
 	unsigned int num_ent, i, cpc_rev;
+	u32 unsupported_regs = 0;
 	int pcc_subspace_id = -1;
 	bool pcc_data_ref = false;
 	acpi_status status;
-	int ret = -ENODATA;
+	int ret = -EINVAL;
 	int err;
 
 	per_cpu(cpu_pcc_subspace_idx, pr->id) = -1;
@@ -1291,6 +1461,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
@@ -1298,6 +1472,12 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			 * so extract it only once.
 			 */
 			if (gas_t->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
+				if (!gas_t->bit_width || gas_t->bit_width > 64 ||
+				    gas_t->bit_offset || gas_t->bit_width % 8) {
+					unsupported_regs |= BIT(i - 2);
+					continue;
+				}
+
 				if (pcc_subspace_id < 0) {
 					pcc_subspace_id = gas_t->access_width;
 					err = pcc_data_alloc(pcc_subspace_id);
@@ -1309,6 +1489,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 				} else if (pcc_subspace_id != gas_t->access_width) {
 					pr_debug("Mismatched PCC ids in _CPC for CPU:%d\n",
 						 pr->id);
+					ret = -EINVAL;
 					goto out_free;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY) {
@@ -1316,21 +1497,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					void __iomem *addr;
 					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;
-					}
+					err = cpc_validate_sysmem_reg(cpc_ptr, gas_t,
+								      i - 2);
 					if (err) {
-						ret = err;
-						goto out_free;
+						unsupported_regs |= BIT(i - 2);
+						continue;
 					}
 
 					if (!osc_cpc_flexible_adr_space_confirmed) {
@@ -1342,8 +1513,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					access_width = cpc_reg_access_width(gas_t);
 					access_width /= 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) {
@@ -1376,10 +1549,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
@@ -1398,35 +1567,14 @@ 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;
-		}
-	}
+	ret = cpc_resolve_unsupported(cpc_ptr, unsupported_regs);
+	if (ret)
+		goto out_free;
+	unsupported_regs = 0;
 
-	/*
-	 * In CPPC v1, DESIRED_PERF is mandatory. In CPPC v2, it is optional
-	 * only when AUTO_SEL_ENABLE is supported.
-	 */
-	if (!cpc_is_writable(&cpc_ptr->cpc_regs[DESIRED_PERF]) &&
-	    (!osc_sb_cppc2_support_acked ||
-	     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;
+	ret = cpc_validate_required_controls(cpc_ptr);
+	if (ret)
 		goto out_free;
-	}
 
 	/*
 	 * Initialize the remaining cpc_regs as unsupported.
@@ -1460,6 +1608,27 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 		init_rwsem(&pcc_data[pcc_subspace_id]->pcc_lock);
 		init_waitqueue_head(&pcc_data[pcc_subspace_id]->pcc_write_wait_q);
 	}
+	if (pcc_subspace_id >= 0) {
+		ret = cpc_validate_pcc_bounds(cpc_ptr,
+					      pcc_data[pcc_subspace_id],
+					      &unsupported_regs);
+		if (ret)
+			goto out_free;
+
+		ret = cpc_resolve_unsupported(cpc_ptr, unsupported_regs);
+		if (ret)
+			goto out_free;
+	}
+
+	ret = cpc_validate_non_mmio_overlaps(cpc_ptr,
+					     ACPI_ADR_SPACE_PLATFORM_COMM,
+					     "PCC");
+	if (ret)
+		goto out_free;
+
+	ret = cpc_validate_required_controls(cpc_ptr);
+	if (ret)
+		goto out_free;
 
 	/* Everything looks okay */
 	pr_debug("Parsed CPC struct for CPU: %d\n", pr->id);
@@ -1569,6 +1738,9 @@ int __weak cpc_write_ffh(int cpunum, struct cpc_reg *reg, u64 val)
 static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 {
 	void __iomem *vaddr = NULL;
+	unsigned long flags;
+	u8 buf[sizeof(*val)];
+	unsigned int i;
 	int size;
 	int pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, cpu);
 	struct cpc_reg *reg = &reg_res->cpc_entry.reg;
@@ -1596,13 +1768,29 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 
 		*val = val_u32;
 		return 0;
-	} else if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >= 0) {
+	} else if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
+		if (pcc_ss_id < 0 || !pcc_data[pcc_ss_id])
+			return -ENODEV;
+
 		/*
 		 * For registers in PCC space, the register size is determined
 		 * by the bit width field; the access size is used to indicate
 		 * the PCC subspace id.
 		 */
 		vaddr = GET_PCC_VADDR(reg->address, pcc_ss_id);
+		size = reg->bit_width / 8;
+		if (!size || size > sizeof(buf) || reg->bit_width % 8)
+			return -EFAULT;
+
+		raw_spin_lock_irqsave(&pcc_data[pcc_ss_id]->payload_lock, flags);
+		memcpy_fromio(buf, vaddr, size);
+		raw_spin_unlock_irqrestore(&pcc_data[pcc_ss_id]->payload_lock,
+					   flags);
+
+		*val = 0;
+		for (i = 0; i < size; i++)
+			*val |= (u64)buf[i] << (i * 8);
+		return 0;
 	}
 	else if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		vaddr = reg_res->sys_mem_vaddr;
@@ -1652,6 +1840,8 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 	struct cpc_reg *reg;
 	struct cpc_desc *cpc_desc;
 	unsigned long flags;
+	u8 buf[sizeof(val)];
+	unsigned int i;
 	bool locked = false;
 
 	if (reg_res->type != ACPI_TYPE_BUFFER)
@@ -1676,13 +1866,28 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 		}
 
 		return 0;
-	} else if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id >= 0) {
+	} else if (reg->space_id == ACPI_ADR_SPACE_PLATFORM_COMM) {
+		if (pcc_ss_id < 0 || !pcc_data[pcc_ss_id])
+			return -ENODEV;
+
 		/*
 		 * For registers in PCC space, the register size is determined
 		 * by the bit width field; the access size is used to indicate
 		 * the PCC subspace id.
 		 */
 		vaddr = GET_PCC_VADDR(reg->address, pcc_ss_id);
+		size = reg->bit_width / 8;
+		if (!size || size > sizeof(buf) || reg->bit_width % 8)
+			return -EFAULT;
+
+		for (i = 0; i < size; i++)
+			buf[i] = val >> (i * 8);
+
+		raw_spin_lock_irqsave(&pcc_data[pcc_ss_id]->payload_lock, flags);
+		memcpy_toio(vaddr, buf, size);
+		raw_spin_unlock_irqrestore(&pcc_data[pcc_ss_id]->payload_lock,
+					   flags);
+		return 0;
 	}
 	else if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
 		vaddr = reg_res->sys_mem_vaddr;
-- 
2.34.1

[PATCH v3 12/15] ACPI: CPPC: Validate SystemIO register layouts

[Christian Loehle]

cpc_read() and cpc_write() access SystemIO registers using the complete GAS
access width. They do not apply Bit Offset or preserve bits outside the
described field. Accepting a partial register therefore reads the wrong
value and can clobber adjacent fields on write.

Only retain full-width 8-, 16-, and 32-bit entries with Bit Offset zero
whose complete access lies in the 16-bit I/O port space. Keep accepting
Access Size zero when Bit Width supplies one of those sizes.

When CONFIG_HAS_IOPORT is disabled, mark SystemIO layouts inaccessible
during probe. Also return -EOPNOTSUPP explicitly in cpc_read() and
cpc_write() so a SystemIO entry can never fall through and treat its port
number as a physical-memory address.

Resolve inaccessible entries using the control-specific policy established
for PCC: optional fields can be disabled, while mandatory or semantically
required controls fail probe. Reject overlapping logical port ranges when
either entry is writable; read-only overlaps remain allowed.

These rejected partial forms are permitted by ACPI, but never worked with
the existing whole-width Linux accessor. Implementing them would require
field-aware I/O and appropriate RMW serialization.

Fixes: a2c8f92bea5f ("ACPI: CPPC: Implement support for SystemIO registers")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 53 ++++++++++++++++++++++++++--------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 03100fc9b993..9c3d278243ca 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -225,7 +225,6 @@ static bool cpc_integer_entry_valid(unsigned int reg_idx, u64 value)
  */
 #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)
 
@@ -1520,21 +1519,28 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					cpc_ptr->cpc_regs[i - 2].sys_mem_vaddr = addr;
 				}
 			} else if (gas_t->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
-				if (gas_t->access_width < 1 || gas_t->access_width > 3) {
-					/*
-					 * 1 = 8-bit, 2 = 16-bit, and 3 = 32-bit.
-					 * SystemIO doesn't implement 64-bit
-					 * registers.
-					 */
-					pr_debug("Invalid access width %d for SystemIO register in _CPC\n",
-						 gas_t->access_width);
-					goto out_free;
+				u64 access_size;
+				const char *reason = "uses unsupported SystemIO geometry";
+				unsigned int access_width;
+				bool unsupported;
+
+				access_width = cpc_reg_access_width(gas_t);
+				unsupported = !IS_ENABLED(CONFIG_HAS_IOPORT) ||
+					      (access_width != 8 &&
+					      access_width != 16 &&
+					      access_width != 32);
+				if (!unsupported) {
+					access_size = access_width / 8;
+					unsupported = gas_t->bit_offset ||
+						gas_t->bit_width != access_width ||
+						gas_t->address >
+						U16_MAX - (access_size - 1);
 				}
-				if (gas_t->address & OVER_16BTS_MASK) {
-					/* SystemIO registers use 16-bit integer addresses */
-					pr_debug("Invalid IO port %llu for SystemIO register in _CPC\n",
-						 gas_t->address);
-					goto out_free;
+				if (unsupported) {
+					pr_debug("CPU%d: _CPC register %u %s\n",
+						 pr->id, i - 2, reason);
+					unsupported_regs |= BIT(i - 2);
+					continue;
 				}
 				if (!osc_cpc_flexible_adr_space_confirmed) {
 					pr_debug("Flexible address space capability not supported\n");
@@ -1625,6 +1631,11 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 					     "PCC");
 	if (ret)
 		goto out_free;
+	ret = cpc_validate_non_mmio_overlaps(cpc_ptr,
+					     ACPI_ADR_SPACE_SYSTEM_IO,
+					     "SystemIO");
+	if (ret)
+		goto out_free;
 
 	ret = cpc_validate_required_controls(cpc_ptr);
 	if (ret)
@@ -1753,11 +1764,13 @@ static int cpc_read(int cpu, struct cpc_register_resource *reg_res, u64 *val)
 	*val = 0;
 	size = GET_BIT_WIDTH(reg);
 
-	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
-	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		u32 val_u32;
 		acpi_status status;
 
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -EOPNOTSUPP;
+
 		status = acpi_os_read_port((acpi_io_address)reg->address,
 					   &val_u32, size);
 		if (ACPI_FAILURE(status)) {
@@ -1853,10 +1866,12 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val)
 
 	size = GET_BIT_WIDTH(reg);
 
-	if (IS_ENABLED(CONFIG_HAS_IOPORT) &&
-	    reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
+	if (reg->space_id == ACPI_ADR_SPACE_SYSTEM_IO) {
 		acpi_status status;
 
+		if (!IS_ENABLED(CONFIG_HAS_IOPORT))
+			return -EOPNOTSUPP;
+
 		status = acpi_os_write_port((acpi_io_address)reg->address,
 					    (u32)val, size);
 		if (ACPI_FAILURE(status)) {
-- 
2.34.1

[PATCH v3 13/15] ACPI: CPPC: Validate PCC overlaps across processors

[Christian Loehle]

PCC shared-memory offsets are physical within a subspace, but the existing
overlap check stops at one _CPC package. Two processors can therefore pass
probe with writable ranges which overlap in the same PCC subspace.

This is unsafe in the performance path, where CPUs may stage requests
concurrently while holding the shared side of pcc_lock. Partially
overlapping stores can construct a payload which belongs to neither request
before a doorbell submits it.

Index retained PCC byte ranges by subspace and physical interval. Permit
read-only overlap and exact aliases of the same logical CPPC entry. The
per-subspace payload lock serializes exact writable aliases, including
multi-byte fields copied with byte-oriented I/O. Reject every other overlap
involving a writable entry.

The interval tree keeps registration proportional to real overlaps rather
than to the number of processors. Remove entries when processor teardown
unpublishes its CPC descriptor, including every probe-failure path after
registration.

Fixes: 85b1407bf6d2 ("ACPI / CPPC: Make CPPC ACPI driver aware of PCC subspace IDs")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 157 +++++++++++++++++++++++++++++++++++++++
 include/acpi/cppc_acpi.h |   2 +
 2 files changed, 159 insertions(+)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 9c3d278243ca..a7141f4d9d0f 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -108,6 +108,18 @@ struct cpc_sysmem_node {
 	bool registered;
 };
 
+struct cpc_non_mmio_node {
+	struct rb_node rb;
+	u64 subtree_last;
+	u64 start;
+	u64 last;
+	struct cpc_desc *desc;
+	unsigned int reg_idx;
+	u8 space_id;
+	u8 pcc_ss_id;
+	bool registered;
+};
+
 #define CPC_SYSMEM_START(node) ((node)->start)
 #define CPC_SYSMEM_LAST(node) ((node)->last)
 
@@ -118,6 +130,16 @@ INTERVAL_TREE_DEFINE(struct cpc_sysmem_node, rb, u64, subtree_last,
 static struct rb_root_cached cpc_sysmem_tree = RB_ROOT_CACHED;
 static DEFINE_MUTEX(cpc_sysmem_lock);
 
+#define CPC_NON_MMIO_START(node) ((node)->start)
+#define CPC_NON_MMIO_LAST(node) ((node)->last)
+
+INTERVAL_TREE_DEFINE(struct cpc_non_mmio_node, rb, u64, subtree_last,
+		     CPC_NON_MMIO_START, CPC_NON_MMIO_LAST, static inline,
+		     cpc_non_mmio_itree)
+
+static struct rb_root_cached cpc_pcc_trees[MAX_PCC_SUBSPACES];
+static DEFINE_MUTEX(cpc_non_mmio_lock);
+
 static struct cpc_sysmem_node *cpc_sysmem_first(u64 start, u64 last)
 {
 	return cpc_sysmem_itree_iter_first(&cpc_sysmem_tree, start, last);
@@ -564,6 +586,134 @@ static int cpc_validate_non_mmio_overlaps(struct cpc_desc *cpc_desc,
 	return 0;
 }
 
+static struct rb_root_cached *cpc_non_mmio_tree(u8 space_id, u8 pcc_ss_id)
+{
+	if (space_id == ACPI_ADR_SPACE_PLATFORM_COMM)
+		return &cpc_pcc_trees[pcc_ss_id];
+	return NULL;
+}
+
+static int cpc_validate_non_mmio_pair(const struct cpc_non_mmio_node *a,
+				      const struct cpc_non_mmio_node *b)
+{
+	bool a_writable = cpc_reg_is_writable(a->reg_idx);
+	bool b_writable = cpc_reg_is_writable(b->reg_idx);
+	const char *name;
+
+	if (!a_writable && !b_writable)
+		return 0;
+
+	if (a->reg_idx == b->reg_idx && a->start == b->start &&
+	    a->last == b->last)
+		return 0;
+
+	name = "PCC";
+	pr_err("CPU%d: %s _CPC register %u conflicts with CPU%d register %u\n",
+	       a->desc->cpu_id, name, a->reg_idx, b->desc->cpu_id,
+	       b->reg_idx);
+	return -EINVAL;
+}
+
+static void cpc_unregister_non_mmio_desc_locked(struct cpc_desc *cpc_desc)
+{
+	unsigned int i;
+
+	if (!cpc_desc->non_mmio_nodes)
+		return;
+
+	for (i = 0; i < cpc_desc->num_entries - 2; i++) {
+		struct cpc_non_mmio_node *node = &cpc_desc->non_mmio_nodes[i];
+		struct rb_root_cached *tree;
+
+		if (!node->registered)
+			continue;
+
+		tree = cpc_non_mmio_tree(node->space_id, node->pcc_ss_id);
+		cpc_non_mmio_itree_remove(node, tree);
+	}
+
+	kfree(cpc_desc->non_mmio_nodes);
+	cpc_desc->non_mmio_nodes = NULL;
+}
+
+static int cpc_register_non_mmio_desc(struct cpc_desc *cpc_desc,
+				      int pcc_ss_id)
+{
+	unsigned int nr_regs = cpc_desc->num_entries - 2;
+	unsigned int i;
+	int ret = 0;
+
+	cpc_desc->non_mmio_nodes = kcalloc(nr_regs,
+					   sizeof(*cpc_desc->non_mmio_nodes),
+					   GFP_KERNEL);
+	if (!cpc_desc->non_mmio_nodes)
+		return -ENOMEM;
+
+	mutex_lock(&cpc_non_mmio_lock);
+
+	for (i = 0; i < nr_regs; i++) {
+		struct cpc_register_resource *reg = &cpc_desc->cpc_regs[i];
+		struct cpc_non_mmio_node *match, *node;
+		struct rb_root_cached *tree;
+		u8 space_id;
+		u64 size;
+
+		if (!CPC_SUPPORTED(reg) || reg->type != ACPI_TYPE_BUFFER)
+			continue;
+
+		space_id = reg->cpc_entry.reg.space_id;
+		if (space_id != ACPI_ADR_SPACE_PLATFORM_COMM)
+			continue;
+
+		if (pcc_ss_id < 0) {
+			ret = -EINVAL;
+			goto out_unregister;
+		}
+
+		node = &cpc_desc->non_mmio_nodes[i];
+		size = cpc_non_mmio_access_size(reg);
+		node->start = reg->cpc_entry.reg.address;
+		node->last = node->start + size - 1;
+		node->desc = cpc_desc;
+		node->reg_idx = i;
+		node->space_id = space_id;
+		node->pcc_ss_id = pcc_ss_id;
+		tree = cpc_non_mmio_tree(space_id, node->pcc_ss_id);
+
+		match = cpc_non_mmio_itree_iter_first(tree, node->start,
+						      node->last);
+		while (match) {
+			ret = cpc_validate_non_mmio_pair(node, match);
+			if (ret)
+				goto out_unregister;
+
+			match = cpc_non_mmio_itree_iter_next(match, node->start,
+							     node->last);
+		}
+
+		cpc_non_mmio_itree_insert(node, tree);
+		node->registered = true;
+	}
+
+	mutex_unlock(&cpc_non_mmio_lock);
+	return 0;
+
+out_unregister:
+	cpc_unregister_non_mmio_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_non_mmio_lock);
+	return ret;
+}
+
+static void cpc_unregister_non_mmio_desc(struct cpc_desc *cpc_desc)
+{
+	if (!cpc_desc->non_mmio_nodes)
+		return;
+
+	mutex_lock(&cpc_non_mmio_lock);
+	cpc_unregister_non_mmio_desc_locked(cpc_desc);
+	mutex_unlock(&cpc_non_mmio_lock);
+}
+
 static void cpc_mark_rmw_lock_users(struct cpc_desc *cpc_desc)
 {
 	int i;
@@ -797,6 +947,7 @@ static void cppc_free_desc(struct cpc_desc *cpc_ptr)
 {
 	unsigned int i;
 
+	cpc_unregister_non_mmio_desc(cpc_ptr);
 	cpc_unregister_sysmem_desc(cpc_ptr);
 
 	for (i = 2; i < cpc_ptr->num_entries; i++) {
@@ -1641,6 +1792,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 	if (ret)
 		goto out_free;
 
+	ret = cpc_register_non_mmio_desc(cpc_ptr, pcc_subspace_id);
+	if (ret)
+		goto out_free;
+
 	/* Everything looks okay */
 	pr_debug("Parsed CPC struct for CPU: %d\n", pr->id);
 
@@ -1658,6 +1813,7 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr)
 			"acpi_cppc");
 	if (ret) {
 		per_cpu(cpc_desc_ptr, pr->id) = NULL;
+		cpc_unregister_non_mmio_desc(cpc_ptr);
 		cpc_unregister_sysmem_desc(cpc_ptr);
 		kobject_put(&cpc_ptr->kobj);
 		goto out_pcc_put;
@@ -1701,6 +1857,7 @@ void acpi_cppc_processor_exit(struct acpi_processor *pr)
 	pcc_ss_id = per_cpu(cpu_pcc_subspace_idx, pr->id);
 	per_cpu(cpc_desc_ptr, pr->id) = NULL;
 	kobject_del(&cpc_ptr->kobj);
+	cpc_unregister_non_mmio_desc(cpc_ptr);
 	cpc_unregister_sysmem_desc(cpc_ptr);
 
 	pcc_data_put(pcc_ss_id);
diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h
index c4f47c312054..dbcd25878ab6 100644
--- a/include/acpi/cppc_acpi.h
+++ b/include/acpi/cppc_acpi.h
@@ -78,6 +78,7 @@ struct cpc_register_resource {
 };
 
 struct cpc_sysmem_node;
+struct cpc_non_mmio_node;
 
 /* Container to hold the CPC details for each CPU */
 struct cpc_desc {
@@ -91,6 +92,7 @@ struct cpc_desc {
 	struct cpc_register_resource cpc_regs[MAX_CPC_REG_ENT];
 	struct acpi_psd_package domain_info;
 	struct cpc_sysmem_node *sysmem_nodes;
+	struct cpc_non_mmio_node *non_mmio_nodes;
 	struct kobject kobj;
 };
 
-- 
2.34.1

[PATCH v3 14/15] ACPI: CPPC: Validate SystemIO overlaps across processors

[Christian Loehle]

System I/O port space is global, but the overlap check only compares
registers within one _CPC package.  Whole-width accesses from different
processors can therefore target partially overlapping port ranges and
clobber each other.

Extend the physical interval registry to SystemIO.  Permit read-only
overlap and exact full-width aliases of the same logical CPPC entry.
Reject every other cross-processor overlap involving a writable entry.

Fixes: a2c8f92bea5f ("ACPI: CPPC: Implement support for SystemIO registers")
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index a7141f4d9d0f..67d7f81a21b7 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -138,6 +138,7 @@ INTERVAL_TREE_DEFINE(struct cpc_non_mmio_node, rb, u64, subtree_last,
 		     cpc_non_mmio_itree)
 
 static struct rb_root_cached cpc_pcc_trees[MAX_PCC_SUBSPACES];
+static struct rb_root_cached cpc_sysio_tree = RB_ROOT_CACHED;
 static DEFINE_MUTEX(cpc_non_mmio_lock);
 
 static struct cpc_sysmem_node *cpc_sysmem_first(u64 start, u64 last)
@@ -590,6 +591,8 @@ static struct rb_root_cached *cpc_non_mmio_tree(u8 space_id, u8 pcc_ss_id)
 {
 	if (space_id == ACPI_ADR_SPACE_PLATFORM_COMM)
 		return &cpc_pcc_trees[pcc_ss_id];
+	if (space_id == ACPI_ADR_SPACE_SYSTEM_IO)
+		return &cpc_sysio_tree;
 	return NULL;
 }
 
@@ -607,7 +610,8 @@ static int cpc_validate_non_mmio_pair(const struct cpc_non_mmio_node *a,
 	    a->last == b->last)
 		return 0;
 
-	name = "PCC";
+	name = a->space_id == ACPI_ADR_SPACE_PLATFORM_COMM ?
+	       "PCC" : "SystemIO";
 	pr_err("CPU%d: %s _CPC register %u conflicts with CPU%d register %u\n",
 	       a->desc->cpu_id, name, a->reg_idx, b->desc->cpu_id,
 	       b->reg_idx);
@@ -662,10 +666,11 @@ static int cpc_register_non_mmio_desc(struct cpc_desc *cpc_desc,
 			continue;
 
 		space_id = reg->cpc_entry.reg.space_id;
-		if (space_id != ACPI_ADR_SPACE_PLATFORM_COMM)
+		if (space_id != ACPI_ADR_SPACE_PLATFORM_COMM &&
+		    space_id != ACPI_ADR_SPACE_SYSTEM_IO)
 			continue;
 
-		if (pcc_ss_id < 0) {
+		if (space_id == ACPI_ADR_SPACE_PLATFORM_COMM && pcc_ss_id < 0) {
 			ret = -EINVAL;
 			goto out_unregister;
 		}
@@ -677,7 +682,8 @@ static int cpc_register_non_mmio_desc(struct cpc_desc *cpc_desc,
 		node->desc = cpc_desc;
 		node->reg_idx = i;
 		node->space_id = space_id;
-		node->pcc_ss_id = pcc_ss_id;
+		node->pcc_ss_id = space_id == ACPI_ADR_SPACE_PLATFORM_COMM ?
+				      pcc_ss_id : 0;
 		tree = cpc_non_mmio_tree(space_id, node->pcc_ss_id);
 
 		match = cpc_non_mmio_itree_iter_first(tree, node->start,
-- 
2.34.1

[PATCH v3 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Christian Loehle]

The Performance Limited status bits are sticky and write-zero-to-clear.
ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
accesses.

cppc_set_perf_limited() currently reads the register, computes a new value,
and writes it in a separate transaction. If the platform reports another
excursion between those transactions, the stale write can clear that new
event.

Write zero to the requested bits and one to the other defined status bits
directly. Keep reserved bits zero as required for hardware status registers
by ACPI 6.6 Section 4.6.1. This removes the stale read window.

Reject SystemMemory descriptions which require read-modify-write to
preserve the containing access unit, because the per-descriptor spinlock
cannot interlock that RMW with platform updates. Also reject 64-bit
SystemMemory descriptions on 32-bit kernels, where generic readq()/writeq()
may be split into two 32-bit operations and cannot provide the required
portable interlocked access. A naturally aligned full-width QWord remains
supported on 64-bit kernels, where the architecture provides a native
64-bit MMIO accessor.

Performance Limited status is not required for CPPC control. If firmware
describes it using an access that Linux cannot interlock safely, disable
that status register instead of rejecting the processor's otherwise usable
_CPC package.

Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
Signed-off-by: Christian Loehle <christian.loehle@arm.com>
---
 drivers/acpi/cppc_acpi.c | 33 ++++++++++++++++++++++-----------
 1 file changed, 22 insertions(+), 11 deletions(-)

diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
index 67d7f81a21b7..9afc285c9314 100644
--- a/drivers/acpi/cppc_acpi.c
+++ b/drivers/acpi/cppc_acpi.c
@@ -414,6 +414,13 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
 		       cpc_desc->cpu_id, cpc_desc->version, name);
 		return -EINVAL;
 	}
+	if (reg_idx == PERF_LIMITED &&
+	    (gas->bit_offset || gas->bit_width != access_width ||
+	     (access_width == 64 && !IS_ENABLED(CONFIG_64BIT)))) {
+		pr_err("CPU%d: Performance Limited register cannot use an interlocked SystemMemory access\n",
+		       cpc_desc->cpu_id);
+		return -EINVAL;
+	}
 
 	return 0;
 
@@ -441,6 +448,14 @@ static int cpc_resolve_unsupported(struct cpc_desc *cpc_desc,
 		if (!(unsupported & BIT(i)))
 			continue;
 
+		/* CPPC control does not depend on Performance Limited status. */
+		if (i == PERF_LIMITED) {
+			pr_warn("CPU%d: ignoring inaccessible Performance Limited register\n",
+				cpc_desc->cpu_id);
+			cpc_disable_reg(cpc_desc, i);
+			continue;
+		}
+
 		if (i == DESIRED_PERF && cpc_immutable_autonomous(cpc_desc)) {
 			pr_warn("CPU%d: ignoring inaccessible Desired Performance register in autonomous mode\n",
 				cpc_desc->cpu_id);
@@ -3081,9 +3096,6 @@ EXPORT_SYMBOL_GPL(cppc_get_perf_limited);
  */
 int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
 {
-	u64 current_val, new_val;
-	int ret;
-
 	/* Only bits 0 and 1 are valid */
 	if (bits_to_clear & ~CPPC_PERF_LIMITED_MASK)
 		return -EINVAL;
@@ -3091,14 +3103,13 @@ int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
 	if (!bits_to_clear)
 		return 0;
 
-	ret = cppc_get_perf_limited(cpu, &current_val);
-	if (ret)
-		return ret;
-
-	/* Clear the specified bits */
-	new_val = current_val & ~bits_to_clear;
-
-	return cppc_set_reg_val(cpu, PERF_LIMITED, new_val);
+	/*
+	 * Performance Limited is write-zero-to-clear.  Write one to the other
+	 * defined sticky bits so a concurrently reported event is not cleared
+	 * using a value obtained by an earlier, separate read transaction.
+	 */
+	return cppc_set_reg_val(cpu, PERF_LIMITED,
+				CPPC_PERF_LIMITED_MASK & ~bits_to_clear);
 }
 EXPORT_SYMBOL_GPL(cppc_set_perf_limited);
 
-- 
2.34.1

Re: [PATCH v3 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Christian Loehle]

On 8/9/26 07:25, Christian Loehle wrote:
> The Performance Limited status bits are sticky and write-zero-to-clear.
> ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
> accesses.
> 
> cppc_set_perf_limited() currently reads the register, computes a new value,
> and writes it in a separate transaction. If the platform reports another
> excursion between those transactions, the stale write can clear that new
> event.
> 
> Write zero to the requested bits and one to the other defined status bits
> directly. Keep reserved bits zero as required for hardware status registers
> by ACPI 6.6 Section 4.6.1. This removes the stale read window.
> 
> Reject SystemMemory descriptions which require read-modify-write to
> preserve the containing access unit, because the per-descriptor spinlock
> cannot interlock that RMW with platform updates. Also reject 64-bit
> SystemMemory descriptions on 32-bit kernels, where generic readq()/writeq()
> may be split into two 32-bit operations and cannot provide the required
> portable interlocked access. A naturally aligned full-width QWord remains
> supported on 64-bit kernels, where the architecture provides a native
> 64-bit MMIO accessor.
> 
> Performance Limited status is not required for CPPC control. If firmware
> describes it using an access that Linux cannot interlock safely, disable
> that status register instead of rejecting the processor's otherwise usable
> _CPC package.
> 
> Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>  drivers/acpi/cppc_acpi.c | 33 ++++++++++++++++++++++-----------
>  1 file changed, 22 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index 67d7f81a21b7..9afc285c9314 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -414,6 +414,13 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc,
>  		       cpc_desc->cpu_id, cpc_desc->version, name);
>  		return -EINVAL;
>  	}
> +	if (reg_idx == PERF_LIMITED &&
> +	    (gas->bit_offset || gas->bit_width != access_width ||
> +	     (access_width == 64 && !IS_ENABLED(CONFIG_64BIT)))) {
> +		pr_err("CPU%d: Performance Limited register cannot use an interlocked SystemMemory access\n",
> +		       cpc_desc->cpu_id);
> +		return -EINVAL;
> +	}
>  
>  	return 0;
>  
> @@ -441,6 +448,14 @@ static int cpc_resolve_unsupported(struct cpc_desc *cpc_desc,
>  		if (!(unsupported & BIT(i)))
>  			continue;
>  
> +		/* CPPC control does not depend on Performance Limited status. */
> +		if (i == PERF_LIMITED) {
> +			pr_warn("CPU%d: ignoring inaccessible Performance Limited register\n",
> +				cpc_desc->cpu_id);
> +			cpc_disable_reg(cpc_desc, i);
> +			continue;
> +		}
> +
>  		if (i == DESIRED_PERF && cpc_immutable_autonomous(cpc_desc)) {
>  			pr_warn("CPU%d: ignoring inaccessible Desired Performance register in autonomous mode\n",
>  				cpc_desc->cpu_id);
> @@ -3081,9 +3096,6 @@ EXPORT_SYMBOL_GPL(cppc_get_perf_limited);
>   */
>  int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
>  {
> -	u64 current_val, new_val;
> -	int ret;
> -
>  	/* Only bits 0 and 1 are valid */
>  	if (bits_to_clear & ~CPPC_PERF_LIMITED_MASK)
>  		return -EINVAL;
> @@ -3091,14 +3103,13 @@ int cppc_set_perf_limited(int cpu, u64 bits_to_clear)
>  	if (!bits_to_clear)
>  		return 0;
>  
> -	ret = cppc_get_perf_limited(cpu, &current_val);
> -	if (ret)
> -		return ret;
> -
> -	/* Clear the specified bits */
> -	new_val = current_val & ~bits_to_clear;
> -
> -	return cppc_set_reg_val(cpu, PERF_LIMITED, new_val);
> +	/*
> +	 * Performance Limited is write-zero-to-clear.  Write one to the other
> +	 * defined sticky bits so a concurrently reported event is not cleared
> +	 * using a value obtained by an earlier, separate read transaction.
> +	 */
> +	return cppc_set_reg_val(cpu, PERF_LIMITED,
> +				CPPC_PERF_LIMITED_MASK & ~bits_to_clear);

Sashiko review:
"Does writing 0 to the reserved bits unconditionally clear them in this
write-zero-to-clear register?
Because CPPC_PERF_LIMITED_MASK masks off the upper reserved bits, the bitwise
AND results in 0 being written to bits 2-63. In a write-zero-to-clear (W0C)
register, 0 is the active clearing value, which could result in silent data loss
for sticky events in reserved bits defined by future ACPI versions.
While keeping reserved bits zero is the historical rule for standard
write-one-to-clear status registers, doing so here converts a small race
condition in the previous RMW logic into an unconditional clearing of all
reserved bits.
To safely preserve reserved bits in a W0C register, should this write 1s to the
reserved bits instead (for example, by simply writing ~bits_to_clear)?"

I won't adopt that proposal because ACPI 6.6 §4.6.1 explicitly requires
to write zero to reserved bits in status registers. We must not write 1s here.

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Christian Loehle]

On 8/9/26 07:25, Christian Loehle wrote:
> First of all, sorry this got so out of hand, initially this was just
> trying to fix some relatively simple issues found by sashiko in an
> earlier (unrelated) series.
> But with me touching more and more code and going through rounds of AI
> review that kept finding more and more pre-existing issues I've arrived
> at this.
> 
> This series fixes correctness and robustness issues found while reviewing
> the CPPC control path. They affect malformed _CPC handling, error
> propagation, PCC ownership and cleanup, CPC object lifetime, register field
> access, cross-processor aliases, and Performance Limited clearing.
> 
> Series structure
> ================
> 
> Patches 1-8 are deliberately small, independently useful fixes. They
> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
> descriptor and PCC lifetime handling.
> 
> Patches 9-15 are the register-layout hardening portion. Geometry validation
> is more substantial because safe RMW and alias handling depend on the
> physical access unit, not merely on a logical _CPC entry or _PSD domain.
> These patches normalize and validate each supported address space before
> building probe-only physical interval registries. Keeping this work in the
> same posting gives the complete safety boundary and a single base for
> review, while each transport and bug retains its own Fixes provenance.
> Feel free to treat the two parts as independent series, I didn't split it
> because they're all technically fixes and to get Sashiko review for the
> whole lot.
> 
> No interval lookup is added to the scheduler hot path. Full-width
> SystemMemory writes remain lockless. RMW locking remains necessary only for
> a partial field, where we must preserve the other bits in its access
> unit. The existing per-descriptor raw lock continues to cover disjoint
> partial fields within one _CPC package; probe rejects cross-descriptor
> layouts that it cannot protect.
> 
> Parsing and control semantics
> =============================
> 
> The parser now validates the package header before indexing it, bounds the
> BYTE and DWORD Integer forms before conversion, and validates the Generic
> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
> package count, but additional trailing package elements are ignored because
> doing so is safe and preserves compatibility with padded firmware. The parser
> likewise tolerates trailing ResourceTemplate data instead of imposing a new
> EndTag compatibility requirement.
> 
> Writable controls must be Buffer-encoded registers. Minimum and Maximum
> Performance are checked as the pair required by ACPI 6.6 Sections
> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
> Integer-zero convention for absent optional fields, so Lowest Performance
> may retain the valid abstract value zero.
> 
> Performance Limited is one deliberate compatibility exception. ACPI lists
> it as required, but permits a platform with no limiting indication to
> always return zero, and deployed firmware represents that case with a NULL
> descriptor. CPPC control does not depend on this status register, so we
> continue to accept that encoding. A present _CPC package which otherwise
> fails parsing or initialization now emits an error instead of silently
> preventing cpufreq registration.
> 
> Compound performance and EPP updates propagate errors and perform every
> fallible non-PCC write before modifying the PCC payload. Updates across
> address spaces cannot be atomic, but a known non-PCC failure can no longer
> commit only the PCC portion or leave an unsent value for a later command.
> 
> SystemMemory locking and support boundary
> =========================================
> 
> A partial SystemMemory field requires RMW to preserve the rest of its
> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
> a per-_CPC lock and noted that a global lock would be needed if physical
> registers were shared between packages.
> 
> ACPI does not make _PSD a physical-register ownership boundary. Rather than
> put a global raw lock or lookup into the scheduler path, this series makes
> the cheaper per-descriptor model's assumptions enforceable at probe.
> 
> Supported SystemMemory layouts are:
> 
>   - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>   - lockless full-width controls;
>   - read-only aliases;
>   - exact full-width writable aliases, including 64-bit aliases on 64-bit
>     kernels;
>   - disjoint partial writers within one descriptor, serialized by its
>     rmw_lock; and
>   - a partial writer sharing an access unit with a disjoint read-only
>     field.
> 
> Probe rejects overlapping logical fields involving a writer, another field
> inside a full-width writable access unit, cross-descriptor partial writers,
> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
> These layouts were not safely supported by the old per-descriptor lock or
> generic writeq(); rejecting them turns possible corruption into a visible
> probe failure rather than removing working support.
> 
> PCC access and locking
> ======================
> 
> The PCC protocol requires OSPM to acquire the subspace before changing its
> command or payload. Single-register and EPP updates now hold pcc_lock
> across ownership acquisition, payload staging, and command submission.
> 
> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
> Performance Counter at unaligned PCC offset 0x116. Performance controls may
> also use byte-multiple widths such as 24 bits. PCC therefore uses
> byte-oriented I/O with explicit little-endian encoding for zero-offset,
> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
> lock protects concurrent aliased copies made under the shared side of
> pcc_lock; it does not replace the protocol ownership lock.
> 
> Bit-level PCC fields require RMW and remain unsupported. An unsupported
> optional field is marked absent, but a present inaccessible CPPC Enable
> fails probe because OSPM must write it before using CPPC. Thus the
> ACPI-legal one-bit CPPC Enable used by the specification example is a
> documented kernel limitation. The old accessor could not program it
> correctly either, so an explicit error is safer than silently proceeding
> without enabling CPPC.
> 
> Every retained PCC field is bounds checked against the shared-memory
> region. A subspace-keyed interval registry permits read-only overlap and
> exact same-control aliases while rejecting every other writable overlap
> across processors.
> 
> SystemIO support boundary
> =========================
> 
> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
> at or below port 0xffff, including legacy Access Size zero when Bit Width
> supplies the size. Partial fields never worked because the driver neither
> shifted them nor preserved adjacent bits, so they now fail visibly instead
> of being misprogrammed.
> 
> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
> disabled according to the affected control's semantics. Runtime accessors
> also return -EOPNOTSUPP rather than treating an I/O port as a
> physical-memory address. A global port interval registry rejects
> cross-processor writable overlap.
> 
> Write-only and Performance Limited controls
> ===========================================
> 
> Between _CPC revisions 3 and 4, Desired Performance changed from
> Read/Write to Write, and revision 4 added write-only OSPM Nominal
> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
> are undefined. Explicit reads of both controls are rejected, as are
> SystemMemory layouts which would implicitly read them for RMW. Full-width
> writes remain supported.
> 
> Performance Limited is sticky, write-zero-to-clear, and requires
> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
> read and write could clear a new event reported between transactions. The
> clear path now writes zero only to requested status bits and one to the
> other defined bits. Partial SystemMemory forms cannot be used because a
> spinlock cannot interlock an enclosing RMW with platform updates. QWord
> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
> split into two 32-bit operations; naturally aligned, full-width QWords
> remain supported on 64-bit kernels. Since CPPC control does not depend on
> Performance Limited status, an unusable description disables that status
> register instead of rejecting the processor's otherwise usable _CPC.
> 
> Lifetime and cleanup
> ====================
> 
> CPC descriptors are released through their kobject callback, keeping their
> storage and mappings alive for outstanding sysfs references. Every PCC
> allocation, reference, and acquired channel is unwound on probe failure,
> and the per-CPU PCC index is initialized before every early return. PCC
> allocation uses a separate temporary result, so its success cannot turn a
> later parse failure into a successful probe return.
> 
> Changes since v2
> ================
> 
>   - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>     package elements while still rejecting any count that could cause an
>     out-of-bounds walk.
>   - Made patch 10 independently preserve immutable-autonomous setups whose
>     inaccessible Desired Performance register requires RMW, rather than
>     relying on patch 11 to restore that exception.
>     
> Sashiko v2 review not addressed 
> ===============================
> 
>   - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>     nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>     entry.
> 
> Deferred follow-up work
> =======================
> 
> Sashiko also identified a broader pre-existing lifetime question which this
> series does not attempt to solve. In-kernel accessors read the per-CPU
> cpc_desc_ptr without acquiring a reference, while processor teardown can
> unpublish and eventually release the descriptor and its PCC data. The kobject
> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
> would not protect a caller which already holds a stale pointer. Closing this
> properly requires defining the kernel accessor lifetime contract and then
> using CPU-hotplug serialization / safe referencing across all callers,
> therefore will be handled by a follow-up.
> 
> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
> implemented by the old whole-value accessors. Supporting these descriptions
> requires transport-specific field extraction and an RMW operation which obeys
> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
> descriptors would silently program the wrong value. Therefore continue to
> disable optional inaccessible fields where safe and reject a present
> inaccessible CPPC Enable control.
> Full support, if even needed, belongs in a separate follow-up.
> 
> The review additionally suggested validating the complete AML
> ResourceTemplate, including its EndTag. We currently validate the Register
> descriptor we consume and tolerate trailing firmware data. I don't really
> see the point of ever doing this, but definitely not in this series,
> where I'm trying to guarantee that no reasonably working platform is
> regressing.
> 
> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
> reviewing:
> 
>   https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
> 
> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
> 
>   https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> 
> Patches 1 and 10 address findings from the v2 review:
> 
>   https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
> 
> The series is based on Rafael's bleeding-edge
> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
> the base-commit specified below is linux-next for Sashiko review.
> It applies cleanly on either.
> 
> Christian Loehle (15):
>   ACPI: CPPC: Validate the _CPC package header
>   ACPI: CPPC: Validate _CPC entry and control semantics
>   ACPI: CPPC: Propagate performance-control write errors
>   ACPI: CPPC: Use 64-bit masks for register fields
>   ACPI: CPPC: Serialize PCC single-register payload updates
>   ACPI: CPPC: Serialize PCC EPP payload updates
>   ACPI: CPPC: Release CPC descriptors through kobject
>   ACPI: CPPC: Release PCC data after probe failures
>   ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>   ACPI: CPPC: Reject reads and RMW of write-only controls
>   ACPI: CPPC: Validate and access PCC register layouts
>   ACPI: CPPC: Validate SystemIO register layouts
>   ACPI: CPPC: Validate PCC overlaps across processors
>   ACPI: CPPC: Validate SystemIO overlaps across processors
>   ACPI: CPPC: Clear Performance Limited without a stale read
> 
>  drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>  include/acpi/cppc_acpi.h |    7 +-
>  2 files changed, 1116 insertions(+), 189 deletions(-)
> 
> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928

Looks like Sashiko is happy with this now, so human-reviewers, feel free
to have a go!

Also I'm afraid this won't be the last series, Sashiko found 20 pre-existing
issues again, I haven't looked at all of them in detail but the ones I have
look legitimate to me and some are at least non-trivial to fix.
(Some are also issues fixed by $SUBJECT but reviewed in an earlier patch.)
I didn't see anything super urgent in it though so I might just let things
settle before posting that.

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Christian Loehle]

On 8/9/26 08:18, Christian Loehle wrote:
> On 8/9/26 07:25, Christian Loehle wrote:
>> First of all, sorry this got so out of hand, initially this was just
>> trying to fix some relatively simple issues found by sashiko in an
>> earlier (unrelated) series.
>> But with me touching more and more code and going through rounds of AI
>> review that kept finding more and more pre-existing issues I've arrived
>> at this.
>>
>> This series fixes correctness and robustness issues found while reviewing
>> the CPPC control path. They affect malformed _CPC handling, error
>> propagation, PCC ownership and cleanup, CPC object lifetime, register field
>> access, cross-processor aliases, and Performance Limited clearing.
>>
>> Series structure
>> ================
>>
>> Patches 1-8 are deliberately small, independently useful fixes. They
>> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
>> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
>> descriptor and PCC lifetime handling.
>>
>> Patches 9-15 are the register-layout hardening portion. Geometry validation
>> is more substantial because safe RMW and alias handling depend on the
>> physical access unit, not merely on a logical _CPC entry or _PSD domain.
>> These patches normalize and validate each supported address space before
>> building probe-only physical interval registries. Keeping this work in the
>> same posting gives the complete safety boundary and a single base for
>> review, while each transport and bug retains its own Fixes provenance.
>> Feel free to treat the two parts as independent series, I didn't split it
>> because they're all technically fixes and to get Sashiko review for the
>> whole lot.
>>
>> No interval lookup is added to the scheduler hot path. Full-width
>> SystemMemory writes remain lockless. RMW locking remains necessary only for
>> a partial field, where we must preserve the other bits in its access
>> unit. The existing per-descriptor raw lock continues to cover disjoint
>> partial fields within one _CPC package; probe rejects cross-descriptor
>> layouts that it cannot protect.
>>
>> Parsing and control semantics
>> =============================
>>
>> The parser now validates the package header before indexing it, bounds the
>> BYTE and DWORD Integer forms before conversion, and validates the Generic
>> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
>> package count, but additional trailing package elements are ignored because
>> doing so is safe and preserves compatibility with padded firmware. The parser
>> likewise tolerates trailing ResourceTemplate data instead of imposing a new
>> EndTag compatibility requirement.
>>
>> Writable controls must be Buffer-encoded registers. Minimum and Maximum
>> Performance are checked as the pair required by ACPI 6.6 Sections
>> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
>> Integer-zero convention for absent optional fields, so Lowest Performance
>> may retain the valid abstract value zero.
>>
>> Performance Limited is one deliberate compatibility exception. ACPI lists
>> it as required, but permits a platform with no limiting indication to
>> always return zero, and deployed firmware represents that case with a NULL
>> descriptor. CPPC control does not depend on this status register, so we
>> continue to accept that encoding. A present _CPC package which otherwise
>> fails parsing or initialization now emits an error instead of silently
>> preventing cpufreq registration.
>>
>> Compound performance and EPP updates propagate errors and perform every
>> fallible non-PCC write before modifying the PCC payload. Updates across
>> address spaces cannot be atomic, but a known non-PCC failure can no longer
>> commit only the PCC portion or leave an unsent value for a later command.
>>
>> SystemMemory locking and support boundary
>> =========================================
>>
>> A partial SystemMemory field requires RMW to preserve the rest of its
>> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
>> a per-_CPC lock and noted that a global lock would be needed if physical
>> registers were shared between packages.
>>
>> ACPI does not make _PSD a physical-register ownership boundary. Rather than
>> put a global raw lock or lookup into the scheduler path, this series makes
>> the cheaper per-descriptor model's assumptions enforceable at probe.
>>
>> Supported SystemMemory layouts are:
>>
>>   - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>>   - lockless full-width controls;
>>   - read-only aliases;
>>   - exact full-width writable aliases, including 64-bit aliases on 64-bit
>>     kernels;
>>   - disjoint partial writers within one descriptor, serialized by its
>>     rmw_lock; and
>>   - a partial writer sharing an access unit with a disjoint read-only
>>     field.
>>
>> Probe rejects overlapping logical fields involving a writer, another field
>> inside a full-width writable access unit, cross-descriptor partial writers,
>> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
>> These layouts were not safely supported by the old per-descriptor lock or
>> generic writeq(); rejecting them turns possible corruption into a visible
>> probe failure rather than removing working support.
>>
>> PCC access and locking
>> ======================
>>
>> The PCC protocol requires OSPM to acquire the subspace before changing its
>> command or payload. Single-register and EPP updates now hold pcc_lock
>> across ownership acquisition, payload staging, and command submission.
>>
>> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
>> Performance Counter at unaligned PCC offset 0x116. Performance controls may
>> also use byte-multiple widths such as 24 bits. PCC therefore uses
>> byte-oriented I/O with explicit little-endian encoding for zero-offset,
>> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
>> lock protects concurrent aliased copies made under the shared side of
>> pcc_lock; it does not replace the protocol ownership lock.
>>
>> Bit-level PCC fields require RMW and remain unsupported. An unsupported
>> optional field is marked absent, but a present inaccessible CPPC Enable
>> fails probe because OSPM must write it before using CPPC. Thus the
>> ACPI-legal one-bit CPPC Enable used by the specification example is a
>> documented kernel limitation. The old accessor could not program it
>> correctly either, so an explicit error is safer than silently proceeding
>> without enabling CPPC.
>>
>> Every retained PCC field is bounds checked against the shared-memory
>> region. A subspace-keyed interval registry permits read-only overlap and
>> exact same-control aliases while rejecting every other writable overlap
>> across processors.
>>
>> SystemIO support boundary
>> =========================
>>
>> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
>> at or below port 0xffff, including legacy Access Size zero when Bit Width
>> supplies the size. Partial fields never worked because the driver neither
>> shifted them nor preserved adjacent bits, so they now fail visibly instead
>> of being misprogrammed.
>>
>> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
>> disabled according to the affected control's semantics. Runtime accessors
>> also return -EOPNOTSUPP rather than treating an I/O port as a
>> physical-memory address. A global port interval registry rejects
>> cross-processor writable overlap.
>>
>> Write-only and Performance Limited controls
>> ===========================================
>>
>> Between _CPC revisions 3 and 4, Desired Performance changed from
>> Read/Write to Write, and revision 4 added write-only OSPM Nominal
>> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
>> are undefined. Explicit reads of both controls are rejected, as are
>> SystemMemory layouts which would implicitly read them for RMW. Full-width
>> writes remain supported.
>>
>> Performance Limited is sticky, write-zero-to-clear, and requires
>> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
>> read and write could clear a new event reported between transactions. The
>> clear path now writes zero only to requested status bits and one to the
>> other defined bits. Partial SystemMemory forms cannot be used because a
>> spinlock cannot interlock an enclosing RMW with platform updates. QWord
>> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
>> split into two 32-bit operations; naturally aligned, full-width QWords
>> remain supported on 64-bit kernels. Since CPPC control does not depend on
>> Performance Limited status, an unusable description disables that status
>> register instead of rejecting the processor's otherwise usable _CPC.
>>
>> Lifetime and cleanup
>> ====================
>>
>> CPC descriptors are released through their kobject callback, keeping their
>> storage and mappings alive for outstanding sysfs references. Every PCC
>> allocation, reference, and acquired channel is unwound on probe failure,
>> and the per-CPU PCC index is initialized before every early return. PCC
>> allocation uses a separate temporary result, so its success cannot turn a
>> later parse failure into a successful probe return.
>>
>> Changes since v2
>> ================
>>
>>   - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>>     package elements while still rejecting any count that could cause an
>>     out-of-bounds walk.
>>   - Made patch 10 independently preserve immutable-autonomous setups whose
>>     inaccessible Desired Performance register requires RMW, rather than
>>     relying on patch 11 to restore that exception.
>>     
>> Sashiko v2 review not addressed 
>> ===============================
>>
>>   - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>>     nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>>     entry.
>>
>> Deferred follow-up work
>> =======================
>>
>> Sashiko also identified a broader pre-existing lifetime question which this
>> series does not attempt to solve. In-kernel accessors read the per-CPU
>> cpc_desc_ptr without acquiring a reference, while processor teardown can
>> unpublish and eventually release the descriptor and its PCC data. The kobject
>> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
>> would not protect a caller which already holds a stale pointer. Closing this
>> properly requires defining the kernel accessor lifetime contract and then
>> using CPU-hotplug serialization / safe referencing across all callers,
>> therefore will be handled by a follow-up.
>>
>> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
>> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
>> implemented by the old whole-value accessors. Supporting these descriptions
>> requires transport-specific field extraction and an RMW operation which obeys
>> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
>> descriptors would silently program the wrong value. Therefore continue to
>> disable optional inaccessible fields where safe and reject a present
>> inaccessible CPPC Enable control.
>> Full support, if even needed, belongs in a separate follow-up.
>>
>> The review additionally suggested validating the complete AML
>> ResourceTemplate, including its EndTag. We currently validate the Register
>> descriptor we consume and tolerate trailing firmware data. I don't really
>> see the point of ever doing this, but definitely not in this series,
>> where I'm trying to guarantee that no reasonably working platform is
>> regressing.
>>
>> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
>> reviewing:
>>
>>   https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
>>
>> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
>>
>>   https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
>>
>> Patches 1 and 10 address findings from the v2 review:
>>
>>   https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
>>
>> The series is based on Rafael's bleeding-edge
>> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
>> the base-commit specified below is linux-next for Sashiko review.
>> It applies cleanly on either.
>>
>> Christian Loehle (15):
>>   ACPI: CPPC: Validate the _CPC package header
>>   ACPI: CPPC: Validate _CPC entry and control semantics
>>   ACPI: CPPC: Propagate performance-control write errors
>>   ACPI: CPPC: Use 64-bit masks for register fields
>>   ACPI: CPPC: Serialize PCC single-register payload updates
>>   ACPI: CPPC: Serialize PCC EPP payload updates
>>   ACPI: CPPC: Release CPC descriptors through kobject
>>   ACPI: CPPC: Release PCC data after probe failures
>>   ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>>   ACPI: CPPC: Reject reads and RMW of write-only controls
>>   ACPI: CPPC: Validate and access PCC register layouts
>>   ACPI: CPPC: Validate SystemIO register layouts
>>   ACPI: CPPC: Validate PCC overlaps across processors
>>   ACPI: CPPC: Validate SystemIO overlaps across processors
>>   ACPI: CPPC: Clear Performance Limited without a stale read
>>
>>  drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>>  include/acpi/cppc_acpi.h |    7 +-
>>  2 files changed, 1116 insertions(+), 189 deletions(-)
>>
>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> 
> Looks like Sashiko is happy with this now, so human-reviewers, feel free
> to have a go!
> 
> Also I'm afraid this won't be the last series, Sashiko found 20 pre-existing
> issues again, I haven't looked at all of them in detail but the ones I have
> look legitimate to me and some are at least non-trivial to fix.
> (Some are also issues fixed by $SUBJECT but reviewed in an earlier patch.)
> I didn't see anything super urgent in it though so I might just let things
> settle before posting that.

Okay after having a more detailed look, it's not as bad as I thought, there's
basically only the one case mentioned above which is lifetime wrt
acpi_cppc_processor_exit() teardown which is currently unprotected against kernel
users, so mostly the driver unbind. AFAICS it's fixable with RCU, refcount and
some cleanup work, so definitely non-trivial.
All other findings either fall into the other two mentioned ones or are already
fixed by later patches of the series AFAICT.

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Christian Loehle]

On 8/9/26 07:25, Christian Loehle wrote:
> First of all, sorry this got so out of hand, initially this was just
> trying to fix some relatively simple issues found by sashiko in an
> earlier (unrelated) series.
> But with me touching more and more code and going through rounds of AI
> review that kept finding more and more pre-existing issues I've arrived
> at this.
> 
> This series fixes correctness and robustness issues found while reviewing
> the CPPC control path. They affect malformed _CPC handling, error
> propagation, PCC ownership and cleanup, CPC object lifetime, register field
> access, cross-processor aliases, and Performance Limited clearing.
> 
> Series structure
> ================
> 
> Patches 1-8 are deliberately small, independently useful fixes. They
> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
> descriptor and PCC lifetime handling.
> 
> Patches 9-15 are the register-layout hardening portion. Geometry validation
> is more substantial because safe RMW and alias handling depend on the
> physical access unit, not merely on a logical _CPC entry or _PSD domain.
> These patches normalize and validate each supported address space before
> building probe-only physical interval registries. Keeping this work in the
> same posting gives the complete safety boundary and a single base for
> review, while each transport and bug retains its own Fixes provenance.
> Feel free to treat the two parts as independent series, I didn't split it
> because they're all technically fixes and to get Sashiko review for the
> whole lot.
> 
> No interval lookup is added to the scheduler hot path. Full-width
> SystemMemory writes remain lockless. RMW locking remains necessary only for
> a partial field, where we must preserve the other bits in its access
> unit. The existing per-descriptor raw lock continues to cover disjoint
> partial fields within one _CPC package; probe rejects cross-descriptor
> layouts that it cannot protect.
> 
> Parsing and control semantics
> =============================
> 
> The parser now validates the package header before indexing it, bounds the
> BYTE and DWORD Integer forms before conversion, and validates the Generic
> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
> package count, but additional trailing package elements are ignored because
> doing so is safe and preserves compatibility with padded firmware. The parser
> likewise tolerates trailing ResourceTemplate data instead of imposing a new
> EndTag compatibility requirement.
> 
> Writable controls must be Buffer-encoded registers. Minimum and Maximum
> Performance are checked as the pair required by ACPI 6.6 Sections
> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
> Integer-zero convention for absent optional fields, so Lowest Performance
> may retain the valid abstract value zero.
> 
> Performance Limited is one deliberate compatibility exception. ACPI lists
> it as required, but permits a platform with no limiting indication to
> always return zero, and deployed firmware represents that case with a NULL
> descriptor. CPPC control does not depend on this status register, so we
> continue to accept that encoding. A present _CPC package which otherwise
> fails parsing or initialization now emits an error instead of silently
> preventing cpufreq registration.
> 
> Compound performance and EPP updates propagate errors and perform every
> fallible non-PCC write before modifying the PCC payload. Updates across
> address spaces cannot be atomic, but a known non-PCC failure can no longer
> commit only the PCC portion or leave an unsent value for a later command.
> 
> SystemMemory locking and support boundary
> =========================================
> 
> A partial SystemMemory field requires RMW to preserve the rest of its
> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
> a per-_CPC lock and noted that a global lock would be needed if physical
> registers were shared between packages.
> 
> ACPI does not make _PSD a physical-register ownership boundary. Rather than
> put a global raw lock or lookup into the scheduler path, this series makes
> the cheaper per-descriptor model's assumptions enforceable at probe.
> 
> Supported SystemMemory layouts are:
> 
>   - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>   - lockless full-width controls;
>   - read-only aliases;
>   - exact full-width writable aliases, including 64-bit aliases on 64-bit
>     kernels;
>   - disjoint partial writers within one descriptor, serialized by its
>     rmw_lock; and
>   - a partial writer sharing an access unit with a disjoint read-only
>     field.
> 
> Probe rejects overlapping logical fields involving a writer, another field
> inside a full-width writable access unit, cross-descriptor partial writers,
> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
> These layouts were not safely supported by the old per-descriptor lock or
> generic writeq(); rejecting them turns possible corruption into a visible
> probe failure rather than removing working support.
> 
> PCC access and locking
> ======================
> 
> The PCC protocol requires OSPM to acquire the subspace before changing its
> command or payload. Single-register and EPP updates now hold pcc_lock
> across ownership acquisition, payload staging, and command submission.
> 
> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
> Performance Counter at unaligned PCC offset 0x116. Performance controls may
> also use byte-multiple widths such as 24 bits. PCC therefore uses
> byte-oriented I/O with explicit little-endian encoding for zero-offset,
> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
> lock protects concurrent aliased copies made under the shared side of
> pcc_lock; it does not replace the protocol ownership lock.
> 
> Bit-level PCC fields require RMW and remain unsupported. An unsupported
> optional field is marked absent, but a present inaccessible CPPC Enable
> fails probe because OSPM must write it before using CPPC. Thus the
> ACPI-legal one-bit CPPC Enable used by the specification example is a
> documented kernel limitation. The old accessor could not program it
> correctly either, so an explicit error is safer than silently proceeding
> without enabling CPPC.
> 
> Every retained PCC field is bounds checked against the shared-memory
> region. A subspace-keyed interval registry permits read-only overlap and
> exact same-control aliases while rejecting every other writable overlap
> across processors.
> 
> SystemIO support boundary
> =========================
> 
> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
> at or below port 0xffff, including legacy Access Size zero when Bit Width
> supplies the size. Partial fields never worked because the driver neither
> shifted them nor preserved adjacent bits, so they now fail visibly instead
> of being misprogrammed.
> 
> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
> disabled according to the affected control's semantics. Runtime accessors
> also return -EOPNOTSUPP rather than treating an I/O port as a
> physical-memory address. A global port interval registry rejects
> cross-processor writable overlap.
> 
> Write-only and Performance Limited controls
> ===========================================
> 
> Between _CPC revisions 3 and 4, Desired Performance changed from
> Read/Write to Write, and revision 4 added write-only OSPM Nominal
> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
> are undefined. Explicit reads of both controls are rejected, as are
> SystemMemory layouts which would implicitly read them for RMW. Full-width
> writes remain supported.
> 
> Performance Limited is sticky, write-zero-to-clear, and requires
> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
> read and write could clear a new event reported between transactions. The
> clear path now writes zero only to requested status bits and one to the
> other defined bits. Partial SystemMemory forms cannot be used because a
> spinlock cannot interlock an enclosing RMW with platform updates. QWord
> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
> split into two 32-bit operations; naturally aligned, full-width QWords
> remain supported on 64-bit kernels. Since CPPC control does not depend on
> Performance Limited status, an unusable description disables that status
> register instead of rejecting the processor's otherwise usable _CPC.
> 
> Lifetime and cleanup
> ====================
> 
> CPC descriptors are released through their kobject callback, keeping their
> storage and mappings alive for outstanding sysfs references. Every PCC
> allocation, reference, and acquired channel is unwound on probe failure,
> and the per-CPU PCC index is initialized before every early return. PCC
> allocation uses a separate temporary result, so its success cannot turn a
> later parse failure into a successful probe return.
> 
> Changes since v2
> ================
> 
>   - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>     package elements while still rejecting any count that could cause an
>     out-of-bounds walk.
>   - Made patch 10 independently preserve immutable-autonomous setups whose
>     inaccessible Desired Performance register requires RMW, rather than
>     relying on patch 11 to restore that exception.
>     
> Sashiko v2 review not addressed 
> ===============================
> 
>   - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>     nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>     entry.
> 
> Deferred follow-up work
> =======================
> 
> Sashiko also identified a broader pre-existing lifetime question which this
> series does not attempt to solve. In-kernel accessors read the per-CPU
> cpc_desc_ptr without acquiring a reference, while processor teardown can
> unpublish and eventually release the descriptor and its PCC data. The kobject
> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
> would not protect a caller which already holds a stale pointer. Closing this
> properly requires defining the kernel accessor lifetime contract and then
> using CPU-hotplug serialization / safe referencing across all callers,
> therefore will be handled by a follow-up.
> 
> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
> implemented by the old whole-value accessors. Supporting these descriptions
> requires transport-specific field extraction and an RMW operation which obeys
> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
> descriptors would silently program the wrong value. Therefore continue to
> disable optional inaccessible fields where safe and reject a present
> inaccessible CPPC Enable control.
> Full support, if even needed, belongs in a separate follow-up.
> 
> The review additionally suggested validating the complete AML
> ResourceTemplate, including its EndTag. We currently validate the Register
> descriptor we consume and tolerate trailing firmware data. I don't really
> see the point of ever doing this, but definitely not in this series,
> where I'm trying to guarantee that no reasonably working platform is
> regressing.
> 
> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
> reviewing:
> 
>   https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
> 
> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
> 
>   https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> 
> Patches 1 and 10 address findings from the v2 review:
> 
>   https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
> 
> The series is based on Rafael's bleeding-edge
> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
> the base-commit specified below is linux-next for Sashiko review.
> It applies cleanly on either.
> 
> Christian Loehle (15):
>   ACPI: CPPC: Validate the _CPC package header
>   ACPI: CPPC: Validate _CPC entry and control semantics
>   ACPI: CPPC: Propagate performance-control write errors
>   ACPI: CPPC: Use 64-bit masks for register fields
>   ACPI: CPPC: Serialize PCC single-register payload updates
>   ACPI: CPPC: Serialize PCC EPP payload updates
>   ACPI: CPPC: Release CPC descriptors through kobject
>   ACPI: CPPC: Release PCC data after probe failures
>   ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>   ACPI: CPPC: Reject reads and RMW of write-only controls
>   ACPI: CPPC: Validate and access PCC register layouts
>   ACPI: CPPC: Validate SystemIO register layouts
>   ACPI: CPPC: Validate PCC overlaps across processors
>   ACPI: CPPC: Validate SystemIO overlaps across processors
>   ACPI: CPPC: Clear Performance Limited without a stale read
> 
>  drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>  include/acpi/cppc_acpi.h |    7 +-
>  2 files changed, 1116 insertions(+), 189 deletions(-)
> 
> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928

Gentle ping on this in particular to the CCs not involved in the merge window.
Even just a Tested-by: that the new CPC validation didn't break your platform would
be appreciated!

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Rafael J. Wysocki (Intel)]

On Thu, Aug 20, 2026 at 12:07 PM Christian Loehle
<christian.loehle@arm.com> wrote:
>
> On 8/9/26 07:25, Christian Loehle wrote:
> > First of all, sorry this got so out of hand, initially this was just
> > trying to fix some relatively simple issues found by sashiko in an
> > earlier (unrelated) series.
> > But with me touching more and more code and going through rounds of AI
> > review that kept finding more and more pre-existing issues I've arrived
> > at this.
> >
> > This series fixes correctness and robustness issues found while reviewing
> > the CPPC control path. They affect malformed _CPC handling, error
> > propagation, PCC ownership and cleanup, CPC object lifetime, register field
> > access, cross-processor aliases, and Performance Limited clearing.
> >
> > Series structure
> > ================
> >
> > Patches 1-8 are deliberately small, independently useful fixes. They
> > validate the _CPC encoding consumed by cppc-acpi, propagate control-write
> > errors, serialize PCC payload updates, correct 64-bit field masks, and fix
> > descriptor and PCC lifetime handling.
> >
> > Patches 9-15 are the register-layout hardening portion. Geometry validation
> > is more substantial because safe RMW and alias handling depend on the
> > physical access unit, not merely on a logical _CPC entry or _PSD domain.
> > These patches normalize and validate each supported address space before
> > building probe-only physical interval registries. Keeping this work in the
> > same posting gives the complete safety boundary and a single base for
> > review, while each transport and bug retains its own Fixes provenance.
> > Feel free to treat the two parts as independent series, I didn't split it
> > because they're all technically fixes and to get Sashiko review for the
> > whole lot.
> >
> > No interval lookup is added to the scheduler hot path. Full-width
> > SystemMemory writes remain lockless. RMW locking remains necessary only for
> > a partial field, where we must preserve the other bits in its access
> > unit. The existing per-descriptor raw lock continues to cover disjoint
> > partial fields within one _CPC package; probe rejects cross-descriptor
> > layouts that it cannot protect.
> >
> > Parsing and control semantics
> > =============================
> >
> > The parser now validates the package header before indexing it, bounds the
> > BYTE and DWORD Integer forms before conversion, and validates the Generic
> > Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
> > package count, but additional trailing package elements are ignored because
> > doing so is safe and preserves compatibility with padded firmware. The parser
> > likewise tolerates trailing ResourceTemplate data instead of imposing a new
> > EndTag compatibility requirement.
> >
> > Writable controls must be Buffer-encoded registers. Minimum and Maximum
> > Performance are checked as the pair required by ACPI 6.6 Sections
> > 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
> > Integer-zero convention for absent optional fields, so Lowest Performance
> > may retain the valid abstract value zero.
> >
> > Performance Limited is one deliberate compatibility exception. ACPI lists
> > it as required, but permits a platform with no limiting indication to
> > always return zero, and deployed firmware represents that case with a NULL
> > descriptor. CPPC control does not depend on this status register, so we
> > continue to accept that encoding. A present _CPC package which otherwise
> > fails parsing or initialization now emits an error instead of silently
> > preventing cpufreq registration.
> >
> > Compound performance and EPP updates propagate errors and perform every
> > fallible non-PCC write before modifying the PCC payload. Updates across
> > address spaces cannot be atomic, but a known non-PCC failure can no longer
> > commit only the PCC portion or leave an unsent value for a later command.
> >
> > SystemMemory locking and support boundary
> > =========================================
> >
> > A partial SystemMemory field requires RMW to preserve the rest of its
> > access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
> > a per-_CPC lock and noted that a global lock would be needed if physical
> > registers were shared between packages.
> >
> > ACPI does not make _PSD a physical-register ownership boundary. Rather than
> > put a global raw lock or lookup into the scheduler path, this series makes
> > the cheaper per-descriptor model's assumptions enforceable at probe.
> >
> > Supported SystemMemory layouts are:
> >
> >   - naturally aligned 8-, 16-, 32-, and 64-bit access units;
> >   - lockless full-width controls;
> >   - read-only aliases;
> >   - exact full-width writable aliases, including 64-bit aliases on 64-bit
> >     kernels;
> >   - disjoint partial writers within one descriptor, serialized by its
> >     rmw_lock; and
> >   - a partial writer sharing an access unit with a disjoint read-only
> >     field.
> >
> > Probe rejects overlapping logical fields involving a writer, another field
> > inside a full-width writable access unit, cross-descriptor partial writers,
> > unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
> > These layouts were not safely supported by the old per-descriptor lock or
> > generic writeq(); rejecting them turns possible corruption into a visible
> > probe failure rather than removing working support.
> >
> > PCC access and locking
> > ======================
> >
> > The PCC protocol requires OSPM to acquire the subspace before changing its
> > command or payload. Single-register and EPP updates now hold pcc_lock
> > across ownership acquisition, payload staging, and command submission.
> >
> > ACPI 6.6's implementation example places a mandatory 32-bit Delivered
> > Performance Counter at unaligned PCC offset 0x116. Performance controls may
> > also use byte-multiple widths such as 24 bits. PCC therefore uses
> > byte-oriented I/O with explicit little-endian encoding for zero-offset,
> > byte-multiple fields from 8 through 64 bits. A short per-subspace payload
> > lock protects concurrent aliased copies made under the shared side of
> > pcc_lock; it does not replace the protocol ownership lock.
> >
> > Bit-level PCC fields require RMW and remain unsupported. An unsupported
> > optional field is marked absent, but a present inaccessible CPPC Enable
> > fails probe because OSPM must write it before using CPPC. Thus the
> > ACPI-legal one-bit CPPC Enable used by the specification example is a
> > documented kernel limitation. The old accessor could not program it
> > correctly either, so an explicit error is safer than silently proceeding
> > without enabling CPPC.
> >
> > Every retained PCC field is bounds checked against the shared-memory
> > region. A subspace-keyed interval registry permits read-only overlap and
> > exact same-control aliases while rejecting every other writable overlap
> > across processors.
> >
> > SystemIO support boundary
> > =========================
> >
> > SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
> > at or below port 0xffff, including legacy Access Size zero when Bit Width
> > supplies the size. Partial fields never worked because the driver neither
> > shifted them nor preserved adjacent bits, so they now fail visibly instead
> > of being misprogrammed.
> >
> > On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
> > disabled according to the affected control's semantics. Runtime accessors
> > also return -EOPNOTSUPP rather than treating an I/O port as a
> > physical-memory address. A global port interval registry rejects
> > cross-processor writable overlap.
> >
> > Write-only and Performance Limited controls
> > ===========================================
> >
> > Between _CPC revisions 3 and 4, Desired Performance changed from
> > Read/Write to Write, and revision 4 added write-only OSPM Nominal
> > Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
> > are undefined. Explicit reads of both controls are rejected, as are
> > SystemMemory layouts which would implicitly read them for RMW. Full-width
> > writes remain supported.
> >
> > Performance Limited is sticky, write-zero-to-clear, and requires
> > interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
> > read and write could clear a new event reported between transactions. The
> > clear path now writes zero only to requested status bits and one to the
> > other defined bits. Partial SystemMemory forms cannot be used because a
> > spinlock cannot interlock an enclosing RMW with platform updates. QWord
> > forms cannot be used on 32-bit kernels, where the MMIO accessor may be
> > split into two 32-bit operations; naturally aligned, full-width QWords
> > remain supported on 64-bit kernels. Since CPPC control does not depend on
> > Performance Limited status, an unusable description disables that status
> > register instead of rejecting the processor's otherwise usable _CPC.
> >
> > Lifetime and cleanup
> > ====================
> >
> > CPC descriptors are released through their kobject callback, keeping their
> > storage and mappings alive for outstanding sysfs references. Every PCC
> > allocation, reference, and acquired channel is unwound on probe failure,
> > and the per-CPU PCC index is initialized before every early return. PCC
> > allocation uses a separate temporary result, so its success cannot turn a
> > later parse failure into a successful probe return.
> >
> > Changes since v2
> > ================
> >
> >   - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
> >     package elements while still rejecting any count that could cause an
> >     out-of-bounds walk.
> >   - Made patch 10 independently preserve immutable-autonomous setups whose
> >     inaccessible Desired Performance register requires RMW, rather than
> >     relying on patch 11 to restore that exception.
> >
> > Sashiko v2 review not addressed
> > ===============================
> >
> >   - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
> >     nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
> >     entry.
> >
> > Deferred follow-up work
> > =======================
> >
> > Sashiko also identified a broader pre-existing lifetime question which this
> > series does not attempt to solve. In-kernel accessors read the per-CPU
> > cpc_desc_ptr without acquiring a reference, while processor teardown can
> > unpublish and eventually release the descriptor and its PCC data. The kobject
> > change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
> > would not protect a caller which already holds a stale pointer. Closing this
> > properly requires defining the kernel accessor lifetime contract and then
> > using CPU-hotplug serialization / safe referencing across all callers,
> > therefore will be handled by a follow-up.
> >
> > ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
> > particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
> > implemented by the old whole-value accessors. Supporting these descriptions
> > requires transport-specific field extraction and an RMW operation which obeys
> > PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
> > descriptors would silently program the wrong value. Therefore continue to
> > disable optional inaccessible fields where safe and reject a present
> > inaccessible CPPC Enable control.
> > Full support, if even needed, belongs in a separate follow-up.
> >
> > The review additionally suggested validating the complete AML
> > ResourceTemplate, including its EndTag. We currently validate the Register
> > descriptor we consume and tolerate trailing firmware data. I don't really
> > see the point of ever doing this, but definitely not in this series,
> > where I'm trying to guarantee that no reasonably working platform is
> > regressing.
> >
> > Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
> > reviewing:
> >
> >   https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
> >
> > Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
> >
> >   https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> >
> > Patches 1 and 10 address findings from the v2 review:
> >
> >   https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
> >
> > The series is based on Rafael's bleeding-edge
> > 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
> > the base-commit specified below is linux-next for Sashiko review.
> > It applies cleanly on either.
> >
> > Christian Loehle (15):
> >   ACPI: CPPC: Validate the _CPC package header
> >   ACPI: CPPC: Validate _CPC entry and control semantics
> >   ACPI: CPPC: Propagate performance-control write errors
> >   ACPI: CPPC: Use 64-bit masks for register fields
> >   ACPI: CPPC: Serialize PCC single-register payload updates
> >   ACPI: CPPC: Serialize PCC EPP payload updates
> >   ACPI: CPPC: Release CPC descriptors through kobject
> >   ACPI: CPPC: Release PCC data after probe failures
> >   ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
> >   ACPI: CPPC: Reject reads and RMW of write-only controls
> >   ACPI: CPPC: Validate and access PCC register layouts
> >   ACPI: CPPC: Validate SystemIO register layouts
> >   ACPI: CPPC: Validate PCC overlaps across processors
> >   ACPI: CPPC: Validate SystemIO overlaps across processors
> >   ACPI: CPPC: Clear Performance Limited without a stale read
> >
> >  drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
> >  include/acpi/cppc_acpi.h |    7 +-
> >  2 files changed, 1116 insertions(+), 189 deletions(-)
> >
> > base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
>
> Gentle ping on this in particular to the CCs not involved in the merge window.
> Even just a Tested-by: that the new CPC validation didn't break your platform would
> be appreciated!

Yes, please.

My current plan is to apply the first part of the series for 7.3-rc1
tomorrow and queue up the rest of the series for 7.4 after the end of
the merge window.

Also, the other CPPC-related pending material is gated on this, so it
won't get in before this series has been applied completely.

Thanks!

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Rafael J. Wysocki (Intel)]

On Thu, Aug 20, 2026 at 12:31 PM Rafael J. Wysocki (Intel)
<rafael@kernel.org> wrote:
>
> On Thu, Aug 20, 2026 at 12:07 PM Christian Loehle
> <christian.loehle@arm.com> wrote:
> >
> > On 8/9/26 07:25, Christian Loehle wrote:
> > > First of all, sorry this got so out of hand, initially this was just
> > > trying to fix some relatively simple issues found by sashiko in an
> > > earlier (unrelated) series.
> > > But with me touching more and more code and going through rounds of AI
> > > review that kept finding more and more pre-existing issues I've arrived
> > > at this.
> > >
> > > This series fixes correctness and robustness issues found while reviewing
> > > the CPPC control path. They affect malformed _CPC handling, error
> > > propagation, PCC ownership and cleanup, CPC object lifetime, register field
> > > access, cross-processor aliases, and Performance Limited clearing.
> > >
> > > Series structure
> > > ================
> > >
> > > Patches 1-8 are deliberately small, independently useful fixes. They
> > > validate the _CPC encoding consumed by cppc-acpi, propagate control-write
> > > errors, serialize PCC payload updates, correct 64-bit field masks, and fix
> > > descriptor and PCC lifetime handling.
> > >
> > > Patches 9-15 are the register-layout hardening portion. Geometry validation
> > > is more substantial because safe RMW and alias handling depend on the
> > > physical access unit, not merely on a logical _CPC entry or _PSD domain.
> > > These patches normalize and validate each supported address space before
> > > building probe-only physical interval registries. Keeping this work in the
> > > same posting gives the complete safety boundary and a single base for
> > > review, while each transport and bug retains its own Fixes provenance.
> > > Feel free to treat the two parts as independent series, I didn't split it
> > > because they're all technically fixes and to get Sashiko review for the
> > > whole lot.
> > >
> > > No interval lookup is added to the scheduler hot path. Full-width
> > > SystemMemory writes remain lockless. RMW locking remains necessary only for
> > > a partial field, where we must preserve the other bits in its access
> > > unit. The existing per-descriptor raw lock continues to cover disjoint
> > > partial fields within one _CPC package; probe rejects cross-descriptor
> > > layouts that it cannot protect.
> > >
> > > Parsing and control semantics
> > > =============================
> > >
> > > The parser now validates the package header before indexing it, bounds the
> > > BYTE and DWORD Integer forms before conversion, and validates the Generic
> > > Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
> > > package count, but additional trailing package elements are ignored because
> > > doing so is safe and preserves compatibility with padded firmware. The parser
> > > likewise tolerates trailing ResourceTemplate data instead of imposing a new
> > > EndTag compatibility requirement.
> > >
> > > Writable controls must be Buffer-encoded registers. Minimum and Maximum
> > > Performance are checked as the pair required by ACPI 6.6 Sections
> > > 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
> > > Integer-zero convention for absent optional fields, so Lowest Performance
> > > may retain the valid abstract value zero.
> > >
> > > Performance Limited is one deliberate compatibility exception. ACPI lists
> > > it as required, but permits a platform with no limiting indication to
> > > always return zero, and deployed firmware represents that case with a NULL
> > > descriptor. CPPC control does not depend on this status register, so we
> > > continue to accept that encoding. A present _CPC package which otherwise
> > > fails parsing or initialization now emits an error instead of silently
> > > preventing cpufreq registration.
> > >
> > > Compound performance and EPP updates propagate errors and perform every
> > > fallible non-PCC write before modifying the PCC payload. Updates across
> > > address spaces cannot be atomic, but a known non-PCC failure can no longer
> > > commit only the PCC portion or leave an unsent value for a later command.
> > >
> > > SystemMemory locking and support boundary
> > > =========================================
> > >
> > > A partial SystemMemory field requires RMW to preserve the rest of its
> > > access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
> > > a per-_CPC lock and noted that a global lock would be needed if physical
> > > registers were shared between packages.
> > >
> > > ACPI does not make _PSD a physical-register ownership boundary. Rather than
> > > put a global raw lock or lookup into the scheduler path, this series makes
> > > the cheaper per-descriptor model's assumptions enforceable at probe.
> > >
> > > Supported SystemMemory layouts are:
> > >
> > >   - naturally aligned 8-, 16-, 32-, and 64-bit access units;
> > >   - lockless full-width controls;
> > >   - read-only aliases;
> > >   - exact full-width writable aliases, including 64-bit aliases on 64-bit
> > >     kernels;
> > >   - disjoint partial writers within one descriptor, serialized by its
> > >     rmw_lock; and
> > >   - a partial writer sharing an access unit with a disjoint read-only
> > >     field.
> > >
> > > Probe rejects overlapping logical fields involving a writer, another field
> > > inside a full-width writable access unit, cross-descriptor partial writers,
> > > unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
> > > These layouts were not safely supported by the old per-descriptor lock or
> > > generic writeq(); rejecting them turns possible corruption into a visible
> > > probe failure rather than removing working support.
> > >
> > > PCC access and locking
> > > ======================
> > >
> > > The PCC protocol requires OSPM to acquire the subspace before changing its
> > > command or payload. Single-register and EPP updates now hold pcc_lock
> > > across ownership acquisition, payload staging, and command submission.
> > >
> > > ACPI 6.6's implementation example places a mandatory 32-bit Delivered
> > > Performance Counter at unaligned PCC offset 0x116. Performance controls may
> > > also use byte-multiple widths such as 24 bits. PCC therefore uses
> > > byte-oriented I/O with explicit little-endian encoding for zero-offset,
> > > byte-multiple fields from 8 through 64 bits. A short per-subspace payload
> > > lock protects concurrent aliased copies made under the shared side of
> > > pcc_lock; it does not replace the protocol ownership lock.
> > >
> > > Bit-level PCC fields require RMW and remain unsupported. An unsupported
> > > optional field is marked absent, but a present inaccessible CPPC Enable
> > > fails probe because OSPM must write it before using CPPC. Thus the
> > > ACPI-legal one-bit CPPC Enable used by the specification example is a
> > > documented kernel limitation. The old accessor could not program it
> > > correctly either, so an explicit error is safer than silently proceeding
> > > without enabling CPPC.
> > >
> > > Every retained PCC field is bounds checked against the shared-memory
> > > region. A subspace-keyed interval registry permits read-only overlap and
> > > exact same-control aliases while rejecting every other writable overlap
> > > across processors.
> > >
> > > SystemIO support boundary
> > > =========================
> > >
> > > SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
> > > at or below port 0xffff, including legacy Access Size zero when Bit Width
> > > supplies the size. Partial fields never worked because the driver neither
> > > shifted them nor preserved adjacent bits, so they now fail visibly instead
> > > of being misprogrammed.
> > >
> > > On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
> > > disabled according to the affected control's semantics. Runtime accessors
> > > also return -EOPNOTSUPP rather than treating an I/O port as a
> > > physical-memory address. A global port interval registry rejects
> > > cross-processor writable overlap.
> > >
> > > Write-only and Performance Limited controls
> > > ===========================================
> > >
> > > Between _CPC revisions 3 and 4, Desired Performance changed from
> > > Read/Write to Write, and revision 4 added write-only OSPM Nominal
> > > Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
> > > are undefined. Explicit reads of both controls are rejected, as are
> > > SystemMemory layouts which would implicitly read them for RMW. Full-width
> > > writes remain supported.
> > >
> > > Performance Limited is sticky, write-zero-to-clear, and requires
> > > interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
> > > read and write could clear a new event reported between transactions. The
> > > clear path now writes zero only to requested status bits and one to the
> > > other defined bits. Partial SystemMemory forms cannot be used because a
> > > spinlock cannot interlock an enclosing RMW with platform updates. QWord
> > > forms cannot be used on 32-bit kernels, where the MMIO accessor may be
> > > split into two 32-bit operations; naturally aligned, full-width QWords
> > > remain supported on 64-bit kernels. Since CPPC control does not depend on
> > > Performance Limited status, an unusable description disables that status
> > > register instead of rejecting the processor's otherwise usable _CPC.
> > >
> > > Lifetime and cleanup
> > > ====================
> > >
> > > CPC descriptors are released through their kobject callback, keeping their
> > > storage and mappings alive for outstanding sysfs references. Every PCC
> > > allocation, reference, and acquired channel is unwound on probe failure,
> > > and the per-CPU PCC index is initialized before every early return. PCC
> > > allocation uses a separate temporary result, so its success cannot turn a
> > > later parse failure into a successful probe return.
> > >
> > > Changes since v2
> > > ================
> > >
> > >   - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
> > >     package elements while still rejecting any count that could cause an
> > >     out-of-bounds walk.
> > >   - Made patch 10 independently preserve immutable-autonomous setups whose
> > >     inaccessible Desired Performance register requires RMW, rather than
> > >     relying on patch 11 to restore that exception.
> > >
> > > Sashiko v2 review not addressed
> > > ===============================
> > >
> > >   - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
> > >     nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
> > >     entry.
> > >
> > > Deferred follow-up work
> > > =======================
> > >
> > > Sashiko also identified a broader pre-existing lifetime question which this
> > > series does not attempt to solve. In-kernel accessors read the per-CPU
> > > cpc_desc_ptr without acquiring a reference, while processor teardown can
> > > unpublish and eventually release the descriptor and its PCC data. The kobject
> > > change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
> > > would not protect a caller which already holds a stale pointer. Closing this
> > > properly requires defining the kernel accessor lifetime contract and then
> > > using CPU-hotplug serialization / safe referencing across all callers,
> > > therefore will be handled by a follow-up.
> > >
> > > ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
> > > particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
> > > implemented by the old whole-value accessors. Supporting these descriptions
> > > requires transport-specific field extraction and an RMW operation which obeys
> > > PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
> > > descriptors would silently program the wrong value. Therefore continue to
> > > disable optional inaccessible fields where safe and reject a present
> > > inaccessible CPPC Enable control.
> > > Full support, if even needed, belongs in a separate follow-up.
> > >
> > > The review additionally suggested validating the complete AML
> > > ResourceTemplate, including its EndTag. We currently validate the Register
> > > descriptor we consume and tolerate trailing firmware data. I don't really
> > > see the point of ever doing this, but definitely not in this series,
> > > where I'm trying to guarantee that no reasonably working platform is
> > > regressing.
> > >
> > > Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
> > > reviewing:
> > >
> > >   https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
> > >
> > > Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
> > >
> > >   https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> > >
> > > Patches 1 and 10 address findings from the v2 review:
> > >
> > >   https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
> > >
> > > The series is based on Rafael's bleeding-edge
> > > 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
> > > the base-commit specified below is linux-next for Sashiko review.
> > > It applies cleanly on either.
> > >
> > > Christian Loehle (15):
> > >   ACPI: CPPC: Validate the _CPC package header
> > >   ACPI: CPPC: Validate _CPC entry and control semantics
> > >   ACPI: CPPC: Propagate performance-control write errors
> > >   ACPI: CPPC: Use 64-bit masks for register fields
> > >   ACPI: CPPC: Serialize PCC single-register payload updates
> > >   ACPI: CPPC: Serialize PCC EPP payload updates
> > >   ACPI: CPPC: Release CPC descriptors through kobject
> > >   ACPI: CPPC: Release PCC data after probe failures
> > >   ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
> > >   ACPI: CPPC: Reject reads and RMW of write-only controls
> > >   ACPI: CPPC: Validate and access PCC register layouts
> > >   ACPI: CPPC: Validate SystemIO register layouts
> > >   ACPI: CPPC: Validate PCC overlaps across processors
> > >   ACPI: CPPC: Validate SystemIO overlaps across processors
> > >   ACPI: CPPC: Clear Performance Limited without a stale read
> > >
> > >  drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
> > >  include/acpi/cppc_acpi.h |    7 +-
> > >  2 files changed, 1116 insertions(+), 189 deletions(-)
> > >
> > > base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> >
> > Gentle ping on this in particular to the CCs not involved in the merge window.
> > Even just a Tested-by: that the new CPC validation didn't break your platform would
> > be appreciated!
>
> Yes, please.
>
> My current plan is to apply the first part of the series for 7.3-rc1
> tomorrow and queue up the rest of the series for 7.4 after the end of
> the merge window.

So I've gone ahead and applied the whole lot.

I plan to send a pull request with it by the end of the next week.

> Also, the other CPPC-related pending material is gated on this, so it
> won't get in before this series has been applied completely.

So this means until 7.3-rc1 is out if all goes well.

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Sumit Gupta]

Hi Christian,

Sorry for late reply.


On 20/08/26 15:37, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> On 8/9/26 07:25, Christian Loehle wrote:
>> First of all, sorry this got so out of hand, initially this was just
>> trying to fix some relatively simple issues found by sashiko in an
>> earlier (unrelated) series.
>> But with me touching more and more code and going through rounds of AI
>> review that kept finding more and more pre-existing issues I've arrived
>> at this.
>>
>> This series fixes correctness and robustness issues found while reviewing
>> the CPPC control path. They affect malformed _CPC handling, error
>> propagation, PCC ownership and cleanup, CPC object lifetime, register field
>> access, cross-processor aliases, and Performance Limited clearing.
>>
>> Series structure
>> ================
>>
>> Patches 1-8 are deliberately small, independently useful fixes. They
>> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
>> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
>> descriptor and PCC lifetime handling.
>>
>> Patches 9-15 are the register-layout hardening portion. Geometry validation
>> is more substantial because safe RMW and alias handling depend on the
>> physical access unit, not merely on a logical _CPC entry or _PSD domain.
>> These patches normalize and validate each supported address space before
>> building probe-only physical interval registries. Keeping this work in the
>> same posting gives the complete safety boundary and a single base for
>> review, while each transport and bug retains its own Fixes provenance.
>> Feel free to treat the two parts as independent series, I didn't split it
>> because they're all technically fixes and to get Sashiko review for the
>> whole lot.
>>
>> No interval lookup is added to the scheduler hot path. Full-width
>> SystemMemory writes remain lockless. RMW locking remains necessary only for
>> a partial field, where we must preserve the other bits in its access
>> unit. The existing per-descriptor raw lock continues to cover disjoint
>> partial fields within one _CPC package; probe rejects cross-descriptor
>> layouts that it cannot protect.
>>
>> Parsing and control semantics
>> =============================
>>
>> The parser now validates the package header before indexing it, bounds the
>> BYTE and DWORD Integer forms before conversion, and validates the Generic
>> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
>> package count, but additional trailing package elements are ignored because
>> doing so is safe and preserves compatibility with padded firmware. The parser
>> likewise tolerates trailing ResourceTemplate data instead of imposing a new
>> EndTag compatibility requirement.
>>
>> Writable controls must be Buffer-encoded registers. Minimum and Maximum
>> Performance are checked as the pair required by ACPI 6.6 Sections
>> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
>> Integer-zero convention for absent optional fields, so Lowest Performance
>> may retain the valid abstract value zero.
>>
>> Performance Limited is one deliberate compatibility exception. ACPI lists
>> it as required, but permits a platform with no limiting indication to
>> always return zero, and deployed firmware represents that case with a NULL
>> descriptor. CPPC control does not depend on this status register, so we
>> continue to accept that encoding. A present _CPC package which otherwise
>> fails parsing or initialization now emits an error instead of silently
>> preventing cpufreq registration.
>>
>> Compound performance and EPP updates propagate errors and perform every
>> fallible non-PCC write before modifying the PCC payload. Updates across
>> address spaces cannot be atomic, but a known non-PCC failure can no longer
>> commit only the PCC portion or leave an unsent value for a later command.
>>
>> SystemMemory locking and support boundary
>> =========================================
>>
>> A partial SystemMemory field requires RMW to preserve the rest of its
>> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
>> a per-_CPC lock and noted that a global lock would be needed if physical
>> registers were shared between packages.
>>
>> ACPI does not make _PSD a physical-register ownership boundary. Rather than
>> put a global raw lock or lookup into the scheduler path, this series makes
>> the cheaper per-descriptor model's assumptions enforceable at probe.
>>
>> Supported SystemMemory layouts are:
>>
>>    - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>>    - lockless full-width controls;
>>    - read-only aliases;
>>    - exact full-width writable aliases, including 64-bit aliases on 64-bit
>>      kernels;
>>    - disjoint partial writers within one descriptor, serialized by its
>>      rmw_lock; and
>>    - a partial writer sharing an access unit with a disjoint read-only
>>      field.
>>
>> Probe rejects overlapping logical fields involving a writer, another field
>> inside a full-width writable access unit, cross-descriptor partial writers,
>> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
>> These layouts were not safely supported by the old per-descriptor lock or
>> generic writeq(); rejecting them turns possible corruption into a visible
>> probe failure rather than removing working support.
>>
>> PCC access and locking
>> ======================
>>
>> The PCC protocol requires OSPM to acquire the subspace before changing its
>> command or payload. Single-register and EPP updates now hold pcc_lock
>> across ownership acquisition, payload staging, and command submission.
>>
>> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
>> Performance Counter at unaligned PCC offset 0x116. Performance controls may
>> also use byte-multiple widths such as 24 bits. PCC therefore uses
>> byte-oriented I/O with explicit little-endian encoding for zero-offset,
>> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
>> lock protects concurrent aliased copies made under the shared side of
>> pcc_lock; it does not replace the protocol ownership lock.
>>
>> Bit-level PCC fields require RMW and remain unsupported. An unsupported
>> optional field is marked absent, but a present inaccessible CPPC Enable
>> fails probe because OSPM must write it before using CPPC. Thus the
>> ACPI-legal one-bit CPPC Enable used by the specification example is a
>> documented kernel limitation. The old accessor could not program it
>> correctly either, so an explicit error is safer than silently proceeding
>> without enabling CPPC.
>>
>> Every retained PCC field is bounds checked against the shared-memory
>> region. A subspace-keyed interval registry permits read-only overlap and
>> exact same-control aliases while rejecting every other writable overlap
>> across processors.
>>
>> SystemIO support boundary
>> =========================
>>
>> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
>> at or below port 0xffff, including legacy Access Size zero when Bit Width
>> supplies the size. Partial fields never worked because the driver neither
>> shifted them nor preserved adjacent bits, so they now fail visibly instead
>> of being misprogrammed.
>>
>> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
>> disabled according to the affected control's semantics. Runtime accessors
>> also return -EOPNOTSUPP rather than treating an I/O port as a
>> physical-memory address. A global port interval registry rejects
>> cross-processor writable overlap.
>>
>> Write-only and Performance Limited controls
>> ===========================================
>>
>> Between _CPC revisions 3 and 4, Desired Performance changed from
>> Read/Write to Write, and revision 4 added write-only OSPM Nominal
>> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
>> are undefined. Explicit reads of both controls are rejected, as are
>> SystemMemory layouts which would implicitly read them for RMW. Full-width
>> writes remain supported.
>>
>> Performance Limited is sticky, write-zero-to-clear, and requires
>> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
>> read and write could clear a new event reported between transactions. The
>> clear path now writes zero only to requested status bits and one to the
>> other defined bits. Partial SystemMemory forms cannot be used because a
>> spinlock cannot interlock an enclosing RMW with platform updates. QWord
>> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
>> split into two 32-bit operations; naturally aligned, full-width QWords
>> remain supported on 64-bit kernels. Since CPPC control does not depend on
>> Performance Limited status, an unusable description disables that status
>> register instead of rejecting the processor's otherwise usable _CPC.
>>
>> Lifetime and cleanup
>> ====================
>>
>> CPC descriptors are released through their kobject callback, keeping their
>> storage and mappings alive for outstanding sysfs references. Every PCC
>> allocation, reference, and acquired channel is unwound on probe failure,
>> and the per-CPU PCC index is initialized before every early return. PCC
>> allocation uses a separate temporary result, so its success cannot turn a
>> later parse failure into a successful probe return.
>>
>> Changes since v2
>> ================
>>
>>    - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>>      package elements while still rejecting any count that could cause an
>>      out-of-bounds walk.
>>    - Made patch 10 independently preserve immutable-autonomous setups whose
>>      inaccessible Desired Performance register requires RMW, rather than
>>      relying on patch 11 to restore that exception.
>>
>> Sashiko v2 review not addressed
>> ===============================
>>
>>    - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>>      nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>>      entry.
>>
>> Deferred follow-up work
>> =======================
>>
>> Sashiko also identified a broader pre-existing lifetime question which this
>> series does not attempt to solve. In-kernel accessors read the per-CPU
>> cpc_desc_ptr without acquiring a reference, while processor teardown can
>> unpublish and eventually release the descriptor and its PCC data. The kobject
>> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
>> would not protect a caller which already holds a stale pointer. Closing this
>> properly requires defining the kernel accessor lifetime contract and then
>> using CPU-hotplug serialization / safe referencing across all callers,
>> therefore will be handled by a follow-up.
>>
>> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
>> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
>> implemented by the old whole-value accessors. Supporting these descriptions
>> requires transport-specific field extraction and an RMW operation which obeys
>> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
>> descriptors would silently program the wrong value. Therefore continue to
>> disable optional inaccessible fields where safe and reject a present
>> inaccessible CPPC Enable control.
>> Full support, if even needed, belongs in a separate follow-up.
>>
>> The review additionally suggested validating the complete AML
>> ResourceTemplate, including its EndTag. We currently validate the Register
>> descriptor we consume and tolerate trailing firmware data. I don't really
>> see the point of ever doing this, but definitely not in this series,
>> where I'm trying to guarantee that no reasonably working platform is
>> regressing.
>>
>> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
>> reviewing:
>>
>>    https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
>>
>> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
>>
>>    https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
>>
>> Patches 1 and 10 address findings from the v2 review:
>>
>>    https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
>>
>> The series is based on Rafael's bleeding-edge
>> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
>> the base-commit specified below is linux-next for Sashiko review.
>> It applies cleanly on either.
>>
>> Christian Loehle (15):
>>    ACPI: CPPC: Validate the _CPC package header
>>    ACPI: CPPC: Validate _CPC entry and control semantics
>>    ACPI: CPPC: Propagate performance-control write errors
>>    ACPI: CPPC: Use 64-bit masks for register fields
>>    ACPI: CPPC: Serialize PCC single-register payload updates
>>    ACPI: CPPC: Serialize PCC EPP payload updates
>>    ACPI: CPPC: Release CPC descriptors through kobject
>>    ACPI: CPPC: Release PCC data after probe failures
>>    ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>>    ACPI: CPPC: Reject reads and RMW of write-only controls
>>    ACPI: CPPC: Validate and access PCC register layouts
>>    ACPI: CPPC: Validate SystemIO register layouts
>>    ACPI: CPPC: Validate PCC overlaps across processors
>>    ACPI: CPPC: Validate SystemIO overlaps across processors
>>    ACPI: CPPC: Clear Performance Limited without a stale read
>>
>>   drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>>   include/acpi/cppc_acpi.h |    7 +-
>>   2 files changed, 1116 insertions(+), 189 deletions(-)
>>
>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> Gentle ping on this in particular to the CCs not involved in the merge window.
> Even just a Tested-by: that the new CPC validation didn't break your platform would
> be appreciated!

Tested the series on linux-next-20260824.

With firmware reporting _CPC revision 3, every CPU logs:
   ACPI CPPC: CPU0: Performance Limited register cannot use an 
interlocked SystemMemory access
   ACPI CPPC: CPU0: ignoring inaccessible Performance Limited register

cppc_cpufreq still probes and frequency scaling works, though
perf_limited interface reads 0 rather than reporting the
register as unsupported.

With revision 4, every CPU logs:
   ACPI CPPC: CPU0: _CPC v4 Desired Performance register requires 
unsupported read-modify-write
   ACPI CPPC: CPU0: Performance Limited register cannot use an 
interlocked SystemMemory access
   ACPI CPPC: CPU0: _CPC v4 OSPM Nominal Performance register requires 
unsupported read-modify-write
   ACPI CPPC: CPU0: cannot access _CPC register 5
   ACPI CPPC: CPU0: failed to initialize _CPC: -22

Here _CPC initialization fails on every CPU, so cppc_cpufreq does not
register.

Thanks,
Sumit

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Christian Loehle]

On 8/25/26 07:58, Sumit Gupta wrote:
> Hi Christian,
> 
> Sorry for late reply.
> 
> 
> On 20/08/26 15:37, Christian Loehle wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> On 8/9/26 07:25, Christian Loehle wrote:
>>> First of all, sorry this got so out of hand, initially this was just
>>> trying to fix some relatively simple issues found by sashiko in an
>>> earlier (unrelated) series.
>>> But with me touching more and more code and going through rounds of AI
>>> review that kept finding more and more pre-existing issues I've arrived
>>> at this.
>>>
>>> This series fixes correctness and robustness issues found while reviewing
>>> the CPPC control path. They affect malformed _CPC handling, error
>>> propagation, PCC ownership and cleanup, CPC object lifetime, register field
>>> access, cross-processor aliases, and Performance Limited clearing.
>>>
>>> Series structure
>>> ================
>>>
>>> Patches 1-8 are deliberately small, independently useful fixes. They
>>> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
>>> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
>>> descriptor and PCC lifetime handling.
>>>
>>> Patches 9-15 are the register-layout hardening portion. Geometry validation
>>> is more substantial because safe RMW and alias handling depend on the
>>> physical access unit, not merely on a logical _CPC entry or _PSD domain.
>>> These patches normalize and validate each supported address space before
>>> building probe-only physical interval registries. Keeping this work in the
>>> same posting gives the complete safety boundary and a single base for
>>> review, while each transport and bug retains its own Fixes provenance.
>>> Feel free to treat the two parts as independent series, I didn't split it
>>> because they're all technically fixes and to get Sashiko review for the
>>> whole lot.
>>>
>>> No interval lookup is added to the scheduler hot path. Full-width
>>> SystemMemory writes remain lockless. RMW locking remains necessary only for
>>> a partial field, where we must preserve the other bits in its access
>>> unit. The existing per-descriptor raw lock continues to cover disjoint
>>> partial fields within one _CPC package; probe rejects cross-descriptor
>>> layouts that it cannot protect.
>>>
>>> Parsing and control semantics
>>> =============================
>>>
>>> The parser now validates the package header before indexing it, bounds the
>>> BYTE and DWORD Integer forms before conversion, and validates the Generic
>>> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
>>> package count, but additional trailing package elements are ignored because
>>> doing so is safe and preserves compatibility with padded firmware. The parser
>>> likewise tolerates trailing ResourceTemplate data instead of imposing a new
>>> EndTag compatibility requirement.
>>>
>>> Writable controls must be Buffer-encoded registers. Minimum and Maximum
>>> Performance are checked as the pair required by ACPI 6.6 Sections
>>> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
>>> Integer-zero convention for absent optional fields, so Lowest Performance
>>> may retain the valid abstract value zero.
>>>
>>> Performance Limited is one deliberate compatibility exception. ACPI lists
>>> it as required, but permits a platform with no limiting indication to
>>> always return zero, and deployed firmware represents that case with a NULL
>>> descriptor. CPPC control does not depend on this status register, so we
>>> continue to accept that encoding. A present _CPC package which otherwise
>>> fails parsing or initialization now emits an error instead of silently
>>> preventing cpufreq registration.
>>>
>>> Compound performance and EPP updates propagate errors and perform every
>>> fallible non-PCC write before modifying the PCC payload. Updates across
>>> address spaces cannot be atomic, but a known non-PCC failure can no longer
>>> commit only the PCC portion or leave an unsent value for a later command.
>>>
>>> SystemMemory locking and support boundary
>>> =========================================
>>>
>>> A partial SystemMemory field requires RMW to preserve the rest of its
>>> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
>>> a per-_CPC lock and noted that a global lock would be needed if physical
>>> registers were shared between packages.
>>>
>>> ACPI does not make _PSD a physical-register ownership boundary. Rather than
>>> put a global raw lock or lookup into the scheduler path, this series makes
>>> the cheaper per-descriptor model's assumptions enforceable at probe.
>>>
>>> Supported SystemMemory layouts are:
>>>
>>>    - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>>>    - lockless full-width controls;
>>>    - read-only aliases;
>>>    - exact full-width writable aliases, including 64-bit aliases on 64-bit
>>>      kernels;
>>>    - disjoint partial writers within one descriptor, serialized by its
>>>      rmw_lock; and
>>>    - a partial writer sharing an access unit with a disjoint read-only
>>>      field.
>>>
>>> Probe rejects overlapping logical fields involving a writer, another field
>>> inside a full-width writable access unit, cross-descriptor partial writers,
>>> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
>>> These layouts were not safely supported by the old per-descriptor lock or
>>> generic writeq(); rejecting them turns possible corruption into a visible
>>> probe failure rather than removing working support.
>>>
>>> PCC access and locking
>>> ======================
>>>
>>> The PCC protocol requires OSPM to acquire the subspace before changing its
>>> command or payload. Single-register and EPP updates now hold pcc_lock
>>> across ownership acquisition, payload staging, and command submission.
>>>
>>> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
>>> Performance Counter at unaligned PCC offset 0x116. Performance controls may
>>> also use byte-multiple widths such as 24 bits. PCC therefore uses
>>> byte-oriented I/O with explicit little-endian encoding for zero-offset,
>>> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
>>> lock protects concurrent aliased copies made under the shared side of
>>> pcc_lock; it does not replace the protocol ownership lock.
>>>
>>> Bit-level PCC fields require RMW and remain unsupported. An unsupported
>>> optional field is marked absent, but a present inaccessible CPPC Enable
>>> fails probe because OSPM must write it before using CPPC. Thus the
>>> ACPI-legal one-bit CPPC Enable used by the specification example is a
>>> documented kernel limitation. The old accessor could not program it
>>> correctly either, so an explicit error is safer than silently proceeding
>>> without enabling CPPC.
>>>
>>> Every retained PCC field is bounds checked against the shared-memory
>>> region. A subspace-keyed interval registry permits read-only overlap and
>>> exact same-control aliases while rejecting every other writable overlap
>>> across processors.
>>>
>>> SystemIO support boundary
>>> =========================
>>>
>>> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
>>> at or below port 0xffff, including legacy Access Size zero when Bit Width
>>> supplies the size. Partial fields never worked because the driver neither
>>> shifted them nor preserved adjacent bits, so they now fail visibly instead
>>> of being misprogrammed.
>>>
>>> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
>>> disabled according to the affected control's semantics. Runtime accessors
>>> also return -EOPNOTSUPP rather than treating an I/O port as a
>>> physical-memory address. A global port interval registry rejects
>>> cross-processor writable overlap.
>>>
>>> Write-only and Performance Limited controls
>>> ===========================================
>>>
>>> Between _CPC revisions 3 and 4, Desired Performance changed from
>>> Read/Write to Write, and revision 4 added write-only OSPM Nominal
>>> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
>>> are undefined. Explicit reads of both controls are rejected, as are
>>> SystemMemory layouts which would implicitly read them for RMW. Full-width
>>> writes remain supported.
>>>
>>> Performance Limited is sticky, write-zero-to-clear, and requires
>>> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
>>> read and write could clear a new event reported between transactions. The
>>> clear path now writes zero only to requested status bits and one to the
>>> other defined bits. Partial SystemMemory forms cannot be used because a
>>> spinlock cannot interlock an enclosing RMW with platform updates. QWord
>>> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
>>> split into two 32-bit operations; naturally aligned, full-width QWords
>>> remain supported on 64-bit kernels. Since CPPC control does not depend on
>>> Performance Limited status, an unusable description disables that status
>>> register instead of rejecting the processor's otherwise usable _CPC.
>>>
>>> Lifetime and cleanup
>>> ====================
>>>
>>> CPC descriptors are released through their kobject callback, keeping their
>>> storage and mappings alive for outstanding sysfs references. Every PCC
>>> allocation, reference, and acquired channel is unwound on probe failure,
>>> and the per-CPU PCC index is initialized before every early return. PCC
>>> allocation uses a separate temporary result, so its success cannot turn a
>>> later parse failure into a successful probe return.
>>>
>>> Changes since v2
>>> ================
>>>
>>>    - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>>>      package elements while still rejecting any count that could cause an
>>>      out-of-bounds walk.
>>>    - Made patch 10 independently preserve immutable-autonomous setups whose
>>>      inaccessible Desired Performance register requires RMW, rather than
>>>      relying on patch 11 to restore that exception.
>>>
>>> Sashiko v2 review not addressed
>>> ===============================
>>>
>>>    - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>>>      nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>>>      entry.
>>>
>>> Deferred follow-up work
>>> =======================
>>>
>>> Sashiko also identified a broader pre-existing lifetime question which this
>>> series does not attempt to solve. In-kernel accessors read the per-CPU
>>> cpc_desc_ptr without acquiring a reference, while processor teardown can
>>> unpublish and eventually release the descriptor and its PCC data. The kobject
>>> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
>>> would not protect a caller which already holds a stale pointer. Closing this
>>> properly requires defining the kernel accessor lifetime contract and then
>>> using CPU-hotplug serialization / safe referencing across all callers,
>>> therefore will be handled by a follow-up.
>>>
>>> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
>>> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
>>> implemented by the old whole-value accessors. Supporting these descriptions
>>> requires transport-specific field extraction and an RMW operation which obeys
>>> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
>>> descriptors would silently program the wrong value. Therefore continue to
>>> disable optional inaccessible fields where safe and reject a present
>>> inaccessible CPPC Enable control.
>>> Full support, if even needed, belongs in a separate follow-up.
>>>
>>> The review additionally suggested validating the complete AML
>>> ResourceTemplate, including its EndTag. We currently validate the Register
>>> descriptor we consume and tolerate trailing firmware data. I don't really
>>> see the point of ever doing this, but definitely not in this series,
>>> where I'm trying to guarantee that no reasonably working platform is
>>> regressing.
>>>
>>> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
>>> reviewing:
>>>
>>>    https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
>>>
>>> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
>>>
>>>    https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
>>>
>>> Patches 1 and 10 address findings from the v2 review:
>>>
>>>    https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
>>>
>>> The series is based on Rafael's bleeding-edge
>>> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
>>> the base-commit specified below is linux-next for Sashiko review.
>>> It applies cleanly on either.
>>>
>>> Christian Loehle (15):
>>>    ACPI: CPPC: Validate the _CPC package header
>>>    ACPI: CPPC: Validate _CPC entry and control semantics
>>>    ACPI: CPPC: Propagate performance-control write errors
>>>    ACPI: CPPC: Use 64-bit masks for register fields
>>>    ACPI: CPPC: Serialize PCC single-register payload updates
>>>    ACPI: CPPC: Serialize PCC EPP payload updates
>>>    ACPI: CPPC: Release CPC descriptors through kobject
>>>    ACPI: CPPC: Release PCC data after probe failures
>>>    ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>>>    ACPI: CPPC: Reject reads and RMW of write-only controls
>>>    ACPI: CPPC: Validate and access PCC register layouts
>>>    ACPI: CPPC: Validate SystemIO register layouts
>>>    ACPI: CPPC: Validate PCC overlaps across processors
>>>    ACPI: CPPC: Validate SystemIO overlaps across processors
>>>    ACPI: CPPC: Clear Performance Limited without a stale read
>>>
>>>   drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>>>   include/acpi/cppc_acpi.h |    7 +-
>>>   2 files changed, 1116 insertions(+), 189 deletions(-)
>>>
>>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
>> Gentle ping on this in particular to the CCs not involved in the merge window.
>> Even just a Tested-by: that the new CPC validation didn't break your platform would
>> be appreciated!
> 
> Tested the series on linux-next-20260824.

Thanks for testing, can you dump the CPC descriptors for both revisions?

> 
> With firmware reporting _CPC revision 3, every CPU logs:
>   ACPI CPPC: CPU0: Performance Limited register cannot use an interlocked SystemMemory access
>   ACPI CPPC: CPU0: ignoring inaccessible Performance Limited register

Assuming the detection here this is expected.

> 
> cppc_cpufreq still probes and frequency scaling works, though
> perf_limited interface reads 0 rather than reporting the
> register as unsupported.

Ack, I'll fix the getter.

> 
> With revision 4, every CPU logs:
>   ACPI CPPC: CPU0: _CPC v4 Desired Performance register requires unsupported read-modify-write
>   ACPI CPPC: CPU0: Performance Limited register cannot use an interlocked SystemMemory access
>   ACPI CPPC: CPU0: _CPC v4 OSPM Nominal Performance register requires unsupported read-modify-write
>   ACPI CPPC: CPU0: cannot access _CPC register 5
>   ACPI CPPC: CPU0: failed to initialize _CPC: -22
> 
> Here _CPC initialization fails on every CPU, so cppc_cpufreq does not
> register.

It would be nice to have your CPC descriptors here, but in case the detection isn't wrong we
have:
Desired (5), Performance Limited (13), and OSPM Nominal (21) inaccessible
I'm assuming this platform supports both 0 and 1 for AUTO_SEL_ENABLE.
We can disable Performance Limited then and OSPM Nominal but Desired we do need
(unless the kernel forces AUTO_SEL_ENABLE=1?)
We clearly cannot do a RMW on a write-only register (CPCv4), the only solution which
may be possible is that we write all registers that lie in Desired access all the time,
but that really depends on what registers those are. In case these registers are
from other policies this would equate into a global lock, which clearly isn't what
we would want.
Any suggestion how this platform is supposed to work here?

> 
> Thanks,
> Sumit
> 
> 

Re: [PATCH v3 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Sumit Gupta]

On 09/08/26 11:55, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> The Performance Limited status bits are sticky and write-zero-to-clear.
> ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
> accesses.
>
> cppc_set_perf_limited() currently reads the register, computes a new value,
> and writes it in a separate transaction. If the platform reports another
> excursion between those transactions, the stale write can clear that new
> event.
>
> Write zero to the requested bits and one to the other defined status bits
> directly. Keep reserved bits zero as required for hardware status registers
> by ACPI 6.6 Section 4.6.1. This removes the stale read window.
>
> Reject SystemMemory descriptions which require read-modify-write to
> preserve the containing access unit, because the per-descriptor spinlock
> cannot interlock that RMW with platform updates. Also reject 64-bit
> SystemMemory descriptions on 32-bit kernels, where generic readq()/writeq()
> may be split into two 32-bit operations and cannot provide the required
> portable interlocked access. A naturally aligned full-width QWord remains
> supported on 64-bit kernels, where the architecture provides a native
> 64-bit MMIO accessor.
>
> Performance Limited status is not required for CPPC control. If firmware
> describes it using an access that Linux cannot interlock safely, disable
> that status register instead of rejecting the processor's otherwise usable
> _CPC package.
>
> Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
> ---
>   drivers/acpi/cppc_acpi.c | 33 ++++++++++++++++++++++-----------
>   1 file changed, 22 insertions(+), 11 deletions(-)

On my test platform Performance Limited is a 2-bit field at offset 0 with
Access Size 3 (DWord), so bit_width (2) != access_width (32). This is a
valid GAS description. The mismatch only means that the generic writer
would use RMW.

I agree with removing the stale read. However, even if writes are rejected
for this partial width layout, reads do not require RMW. Could the register
remain readable instead of being disabled completely?

If the register must be disabled, the current fallback is reported
incorrectly. Performance Limited is mandatory in _CPC. cpc_disable_reg()
replaces it with an Integer 0, but cppc_get_reg_val() only recognizes
that as unsupported for registers in REG_OPTIONAL, which does not
include PERF_LIMITED. Reads therefore succeed with zero while writes
return -EOPNOTSUPP, so userspace sees "never limited" instead of
"<unsupported>". This hides real events on a platform that can signal
them.

Also, pr_err followed by pr_warn emits two messages per CPU for this
nonfatal fallback. Could these messages be consolidated?

Thanks,
Sumit
....

Re: [PATCH v3 15/15] ACPI: CPPC: Clear Performance Limited without a stale read

[Christian Loehle]

On 8/25/26 10:15, Sumit Gupta wrote:
> 
> On 09/08/26 11:55, Christian Loehle wrote:
>> External email: Use caution opening links or attachments
>>
>>
>> The Performance Limited status bits are sticky and write-zero-to-clear.
>> ACPI 6.6 Section 8.4.6.1.3.2 also requires both entities to use interlocked
>> accesses.
>>
>> cppc_set_perf_limited() currently reads the register, computes a new value,
>> and writes it in a separate transaction. If the platform reports another
>> excursion between those transactions, the stale write can clear that new
>> event.
>>
>> Write zero to the requested bits and one to the other defined status bits
>> directly. Keep reserved bits zero as required for hardware status registers
>> by ACPI 6.6 Section 4.6.1. This removes the stale read window.
>>
>> Reject SystemMemory descriptions which require read-modify-write to
>> preserve the containing access unit, because the per-descriptor spinlock
>> cannot interlock that RMW with platform updates. Also reject 64-bit
>> SystemMemory descriptions on 32-bit kernels, where generic readq()/writeq()
>> may be split into two 32-bit operations and cannot provide the required
>> portable interlocked access. A naturally aligned full-width QWord remains
>> supported on 64-bit kernels, where the architecture provides a native
>> 64-bit MMIO accessor.
>>
>> Performance Limited status is not required for CPPC control. If firmware
>> describes it using an access that Linux cannot interlock safely, disable
>> that status register instead of rejecting the processor's otherwise usable
>> _CPC package.
>>
>> Fixes: 13c45a26635f ("ACPI: CPPC: add APIs and sysfs interface for perf_limited")
>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>> Link: https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
>> Signed-off-by: Christian Loehle <christian.loehle@arm.com>
>> ---
>>   drivers/acpi/cppc_acpi.c | 33 ++++++++++++++++++++++-----------
>>   1 file changed, 22 insertions(+), 11 deletions(-)
> 
> On my test platform Performance Limited is a 2-bit field at offset 0 with
> Access Size 3 (DWord), so bit_width (2) != access_width (32). This is a
> valid GAS description. The mismatch only means that the generic writer
> would use RMW.
> 
> I agree with removing the stale read. However, even if writes are rejected
> for this partial width layout, reads do not require RMW. Could the register
> remain readable instead of being disabled completely?

Ack, I can turn this into read-only if not writable by the kernel.

> 
> If the register must be disabled, the current fallback is reported
> incorrectly. Performance Limited is mandatory in _CPC. cpc_disable_reg()
> replaces it with an Integer 0, but cppc_get_reg_val() only recognizes
> that as unsupported for registers in REG_OPTIONAL, which does not
> include PERF_LIMITED. Reads therefore succeed with zero while writes
> return -EOPNOTSUPP, so userspace sees "never limited" instead of
> "<unsupported>". This hides real events on a platform that can signal
> them.

Ack, I've fixed the getter.

> 
> Also, pr_err followed by pr_warn emits two messages per CPU for this
> nonfatal fallback. Could these messages be consolidated?

Ack

> 
> Thanks,
> Sumit
> ....
> 
> 

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Rafael J. Wysocki (Intel)]

On Tue, Aug 25, 2026 at 10:25 AM Christian Loehle
<christian.loehle@arm.com> wrote:
>

[cut]

> >>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
> >> Gentle ping on this in particular to the CCs not involved in the merge window.
> >> Even just a Tested-by: that the new CPC validation didn't break your platform would
> >> be appreciated!
> >
> > Tested the series on linux-next-20260824.
>
> Thanks for testing, can you dump the CPC descriptors for both revisions?
>
> >
> > With firmware reporting _CPC revision 3, every CPU logs:
> >   ACPI CPPC: CPU0: Performance Limited register cannot use an interlocked SystemMemory access
> >   ACPI CPPC: CPU0: ignoring inaccessible Performance Limited register
>
> Assuming the detection here this is expected.
>
> >
> > cppc_cpufreq still probes and frequency scaling works, though
> > perf_limited interface reads 0 rather than reporting the
> > register as unsupported.
>
> Ack, I'll fix the getter.

This means though that it will miss the 7.3 merge window mark and it
will become 7.4 material.

Re: [PATCH v3 00/15] ACPI: CPPC: Fix register access and lifetime bugs

[Sumit Gupta]

On 25/08/26 13:55, Christian Loehle wrote:
> External email: Use caution opening links or attachments
>
>
> On 8/25/26 07:58, Sumit Gupta wrote:
>> Hi Christian,
>>
>> Sorry for late reply.
>>
>>
>> On 20/08/26 15:37, Christian Loehle wrote:
>>> External email: Use caution opening links or attachments
>>>
>>>
>>> On 8/9/26 07:25, Christian Loehle wrote:
>>>> First of all, sorry this got so out of hand, initially this was just
>>>> trying to fix some relatively simple issues found by sashiko in an
>>>> earlier (unrelated) series.
>>>> But with me touching more and more code and going through rounds of AI
>>>> review that kept finding more and more pre-existing issues I've arrived
>>>> at this.
>>>>
>>>> This series fixes correctness and robustness issues found while reviewing
>>>> the CPPC control path. They affect malformed _CPC handling, error
>>>> propagation, PCC ownership and cleanup, CPC object lifetime, register field
>>>> access, cross-processor aliases, and Performance Limited clearing.
>>>>
>>>> Series structure
>>>> ================
>>>>
>>>> Patches 1-8 are deliberately small, independently useful fixes. They
>>>> validate the _CPC encoding consumed by cppc-acpi, propagate control-write
>>>> errors, serialize PCC payload updates, correct 64-bit field masks, and fix
>>>> descriptor and PCC lifetime handling.
>>>>
>>>> Patches 9-15 are the register-layout hardening portion. Geometry validation
>>>> is more substantial because safe RMW and alias handling depend on the
>>>> physical access unit, not merely on a logical _CPC entry or _PSD domain.
>>>> These patches normalize and validate each supported address space before
>>>> building probe-only physical interval registries. Keeping this work in the
>>>> same posting gives the complete safety boundary and a single base for
>>>> review, while each transport and bug retains its own Fixes provenance.
>>>> Feel free to treat the two parts as independent series, I didn't split it
>>>> because they're all technically fixes and to get Sashiko review for the
>>>> whole lot.
>>>>
>>>> No interval lookup is added to the scheduler hot path. Full-width
>>>> SystemMemory writes remain lockless. RMW locking remains necessary only for
>>>> a partial field, where we must preserve the other bits in its access
>>>> unit. The existing per-descriptor raw lock continues to cover disjoint
>>>> partial fields within one _CPC package; probe rejects cross-descriptor
>>>> layouts that it cannot protect.
>>>>
>>>> Parsing and control semantics
>>>> =============================
>>>>
>>>> The parser now validates the package header before indexing it, bounds the
>>>> BYTE and DWORD Integer forms before conversion, and validates the Generic
>>>> Register descriptor consumed by cppc-acpi. NumEntries may not exceed the AML
>>>> package count, but additional trailing package elements are ignored because
>>>> doing so is safe and preserves compatibility with padded firmware. The parser
>>>> likewise tolerates trailing ResourceTemplate data instead of imposing a new
>>>> EndTag compatibility requirement.
>>>>
>>>> Writable controls must be Buffer-encoded registers. Minimum and Maximum
>>>> Performance are checked as the pair required by ACPI 6.6 Sections
>>>> 8.4.6.1.2.1 and 8.4.6.1.2.2. Object presence is kept separate from the
>>>> Integer-zero convention for absent optional fields, so Lowest Performance
>>>> may retain the valid abstract value zero.
>>>>
>>>> Performance Limited is one deliberate compatibility exception. ACPI lists
>>>> it as required, but permits a platform with no limiting indication to
>>>> always return zero, and deployed firmware represents that case with a NULL
>>>> descriptor. CPPC control does not depend on this status register, so we
>>>> continue to accept that encoding. A present _CPC package which otherwise
>>>> fails parsing or initialization now emits an error instead of silently
>>>> preventing cpufreq registration.
>>>>
>>>> Compound performance and EPP updates propagate errors and perform every
>>>> fallible non-PCC write before modifying the PCC payload. Updates across
>>>> address spaces cannot be atomic, but a known non-PCC failure can no longer
>>>> commit only the PCC portion or leave an unsent value for a later command.
>>>>
>>>> SystemMemory locking and support boundary
>>>> =========================================
>>>>
>>>> A partial SystemMemory field requires RMW to preserve the rest of its
>>>> access unit. Commit 60949b7b8054 ("ACPI: CPPC: Fix MASK_VAL() usage") used
>>>> a per-_CPC lock and noted that a global lock would be needed if physical
>>>> registers were shared between packages.
>>>>
>>>> ACPI does not make _PSD a physical-register ownership boundary. Rather than
>>>> put a global raw lock or lookup into the scheduler path, this series makes
>>>> the cheaper per-descriptor model's assumptions enforceable at probe.
>>>>
>>>> Supported SystemMemory layouts are:
>>>>
>>>>     - naturally aligned 8-, 16-, 32-, and 64-bit access units;
>>>>     - lockless full-width controls;
>>>>     - read-only aliases;
>>>>     - exact full-width writable aliases, including 64-bit aliases on 64-bit
>>>>       kernels;
>>>>     - disjoint partial writers within one descriptor, serialized by its
>>>>       rmw_lock; and
>>>>     - a partial writer sharing an access unit with a disjoint read-only
>>>>       field.
>>>>
>>>> Probe rejects overlapping logical fields involving a writer, another field
>>>> inside a full-width writable access unit, cross-descriptor partial writers,
>>>> unaligned accesses, and exact writable 64-bit aliases on 32-bit kernels.
>>>> These layouts were not safely supported by the old per-descriptor lock or
>>>> generic writeq(); rejecting them turns possible corruption into a visible
>>>> probe failure rather than removing working support.
>>>>
>>>> PCC access and locking
>>>> ======================
>>>>
>>>> The PCC protocol requires OSPM to acquire the subspace before changing its
>>>> command or payload. Single-register and EPP updates now hold pcc_lock
>>>> across ownership acquisition, payload staging, and command submission.
>>>>
>>>> ACPI 6.6's implementation example places a mandatory 32-bit Delivered
>>>> Performance Counter at unaligned PCC offset 0x116. Performance controls may
>>>> also use byte-multiple widths such as 24 bits. PCC therefore uses
>>>> byte-oriented I/O with explicit little-endian encoding for zero-offset,
>>>> byte-multiple fields from 8 through 64 bits. A short per-subspace payload
>>>> lock protects concurrent aliased copies made under the shared side of
>>>> pcc_lock; it does not replace the protocol ownership lock.
>>>>
>>>> Bit-level PCC fields require RMW and remain unsupported. An unsupported
>>>> optional field is marked absent, but a present inaccessible CPPC Enable
>>>> fails probe because OSPM must write it before using CPPC. Thus the
>>>> ACPI-legal one-bit CPPC Enable used by the specification example is a
>>>> documented kernel limitation. The old accessor could not program it
>>>> correctly either, so an explicit error is safer than silently proceeding
>>>> without enabling CPPC.
>>>>
>>>> Every retained PCC field is bounds checked against the shared-memory
>>>> region. A subspace-keyed interval registry permits read-only overlap and
>>>> exact same-control aliases while rejecting every other writable overlap
>>>> across processors.
>>>>
>>>> SystemIO support boundary
>>>> =========================
>>>>
>>>> SystemIO supports Bit Offset zero, full 8-, 16-, or 32-bit accesses ending
>>>> at or below port 0xffff, including legacy Access Size zero when Bit Width
>>>> supplies the size. Partial fields never worked because the driver neither
>>>> shifted them nor preserved adjacent bits, so they now fail visibly instead
>>>> of being misprogrammed.
>>>>
>>>> On kernels without CONFIG_HAS_IOPORT, SystemIO entries are rejected or
>>>> disabled according to the affected control's semantics. Runtime accessors
>>>> also return -EOPNOTSUPP rather than treating an I/O port as a
>>>> physical-memory address. A global port interval registry rejects
>>>> cross-processor writable overlap.
>>>>
>>>> Write-only and Performance Limited controls
>>>> ===========================================
>>>>
>>>> Between _CPC revisions 3 and 4, Desired Performance changed from
>>>> Read/Write to Write, and revision 4 added write-only OSPM Nominal
>>>> Performance. ACPI 6.6 Section 4.6.3 says reads from write-only positions
>>>> are undefined. Explicit reads of both controls are rejected, as are
>>>> SystemMemory layouts which would implicitly read them for RMW. Full-width
>>>> writes remain supported.
>>>>
>>>> Performance Limited is sticky, write-zero-to-clear, and requires
>>>> interlocked accesses under ACPI 6.6 Section 8.4.6.1.3.2. The old separate
>>>> read and write could clear a new event reported between transactions. The
>>>> clear path now writes zero only to requested status bits and one to the
>>>> other defined bits. Partial SystemMemory forms cannot be used because a
>>>> spinlock cannot interlock an enclosing RMW with platform updates. QWord
>>>> forms cannot be used on 32-bit kernels, where the MMIO accessor may be
>>>> split into two 32-bit operations; naturally aligned, full-width QWords
>>>> remain supported on 64-bit kernels. Since CPPC control does not depend on
>>>> Performance Limited status, an unusable description disables that status
>>>> register instead of rejecting the processor's otherwise usable _CPC.
>>>>
>>>> Lifetime and cleanup
>>>> ====================
>>>>
>>>> CPC descriptors are released through their kobject callback, keeping their
>>>> storage and mappings alive for outstanding sysfs references. Every PCC
>>>> allocation, reference, and acquired channel is unwound on probe failure,
>>>> and the per-CPU PCC index is initialized before every early return. PCC
>>>> allocation uses a separate temporary result, so its success cannot turn a
>>>> later parse failure into a successful probe return.
>>>>
>>>> Changes since v2
>>>> ================
>>>>
>>>>     - Relaxed the exact NumEntries/package-count match to tolerate safe trailing
>>>>       package elements while still rejecting any count that could cause an
>>>>       out-of-bounds walk.
>>>>     - Made patch 10 independently preserve immutable-autonomous setups whose
>>>>       inaccessible Desired Performance register requires RMW, rather than
>>>>       relying on patch 11 to restore that exception.
>>>>
>>>> Sashiko v2 review not addressed
>>>> ===============================
>>>>
>>>>     - Kept Guaranteed Performance Buffer-only. The suggestion was to accept a
>>>>       nonzero Integer, but ACPI 6.6 Table 8.23 permits only a Buffer for this
>>>>       entry.
>>>>
>>>> Deferred follow-up work
>>>> =======================
>>>>
>>>> Sashiko also identified a broader pre-existing lifetime question which this
>>>> series does not attempt to solve. In-kernel accessors read the per-CPU
>>>> cpc_desc_ptr without acquiring a reference, while processor teardown can
>>>> unpublish and eventually release the descriptor and its PCC data. The kobject
>>>> change here fixes the concrete sysfs lifetime bug, but a NULL pcc_data check
>>>> would not protect a caller which already holds a stale pointer. Closing this
>>>> properly requires defining the kernel accessor lifetime contract and then
>>>> using CPU-hotplug serialization / safe referencing across all callers,
>>>> therefore will be handled by a follow-up.
>>>>
>>>> ACPI-legal bit-level PCC and SystemIO fields also remain unsupported. In
>>>> particular, the ACPI example's one-bit PCC CPPC Enable register cannot be
>>>> implemented by the old whole-value accessors. Supporting these descriptions
>>>> requires transport-specific field extraction and an RMW operation which obeys
>>>> PCC ownership or safely preserves adjacent SystemIO bits, just accepting the
>>>> descriptors would silently program the wrong value. Therefore continue to
>>>> disable optional inaccessible fields where safe and reject a present
>>>> inaccessible CPPC Enable control.
>>>> Full support, if even needed, belongs in a separate follow-up.
>>>>
>>>> The review additionally suggested validating the complete AML
>>>> ResourceTemplate, including its EndTag. We currently validate the Register
>>>> descriptor we consume and tolerate trailing firmware data. I don't really
>>>> see the point of ever doing this, but definitely not in this series,
>>>> where I'm trying to guarantee that no reasonably working platform is
>>>> regressing.
>>>>
>>>> Patches 1, 2, 4-7, and 9 address findings reported by Sashiko while
>>>> reviewing:
>>>>
>>>>     https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com
>>>>
>>>> Patches 3, 5, 6, 9, 10, and 15 address findings from the follow-up review:
>>>>
>>>>     https://sashiko.dev/#/patchset/20260807111303.1062391-1-christian.loehle%40arm.com
>>>>
>>>> Patches 1 and 10 address findings from the v2 review:
>>>>
>>>>     https://sashiko.dev/#/patchset/20260808082644.1251332-1-christian.loehle%40arm.com
>>>>
>>>> The series is based on Rafael's bleeding-edge
>>>> 77acf59d7cf1 ("Merge branch 'acpi-cppc' into bleeding-edge")
>>>> the base-commit specified below is linux-next for Sashiko review.
>>>> It applies cleanly on either.
>>>>
>>>> Christian Loehle (15):
>>>>     ACPI: CPPC: Validate the _CPC package header
>>>>     ACPI: CPPC: Validate _CPC entry and control semantics
>>>>     ACPI: CPPC: Propagate performance-control write errors
>>>>     ACPI: CPPC: Use 64-bit masks for register fields
>>>>     ACPI: CPPC: Serialize PCC single-register payload updates
>>>>     ACPI: CPPC: Serialize PCC EPP payload updates
>>>>     ACPI: CPPC: Release CPC descriptors through kobject
>>>>     ACPI: CPPC: Release PCC data after probe failures
>>>>     ACPI: CPPC: Reject unsafe cross-CPU SystemMemory RMW
>>>>     ACPI: CPPC: Reject reads and RMW of write-only controls
>>>>     ACPI: CPPC: Validate and access PCC register layouts
>>>>     ACPI: CPPC: Validate SystemIO register layouts
>>>>     ACPI: CPPC: Validate PCC overlaps across processors
>>>>     ACPI: CPPC: Validate SystemIO overlaps across processors
>>>>     ACPI: CPPC: Clear Performance Limited without a stale read
>>>>
>>>>    drivers/acpi/cppc_acpi.c | 1298 ++++++++++++++++++++++++++++++++------
>>>>    include/acpi/cppc_acpi.h |    7 +-
>>>>    2 files changed, 1116 insertions(+), 189 deletions(-)
>>>>
>>>> base-commit: ea2bff00da89d7767d677bb68470130ba96f4928
>>> Gentle ping on this in particular to the CCs not involved in the merge window.
>>> Even just a Tested-by: that the new CPC validation didn't break your platform would
>>> be appreciated!
>> Tested the series on linux-next-20260824.
> Thanks for testing, can you dump the CPC descriptors for both revisions?
>
>> With firmware reporting _CPC revision 3, every CPU logs:
>>    ACPI CPPC: CPU0: Performance Limited register cannot use an interlocked SystemMemory access
>>    ACPI CPPC: CPU0: ignoring inaccessible Performance Limited register
> Assuming the detection here this is expected.
>
>> cppc_cpufreq still probes and frequency scaling works, though
>> perf_limited interface reads 0 rather than reporting the
>> register as unsupported.
> Ack, I'll fix the getter.
>
>> With revision 4, every CPU logs:
>>    ACPI CPPC: CPU0: _CPC v4 Desired Performance register requires unsupported read-modify-write
>>    ACPI CPPC: CPU0: Performance Limited register cannot use an interlocked SystemMemory access
>>    ACPI CPPC: CPU0: _CPC v4 OSPM Nominal Performance register requires unsupported read-modify-write
>>    ACPI CPPC: CPU0: cannot access _CPC register 5
>>    ACPI CPPC: CPU0: failed to initialize _CPC: -22
>>
>> Here _CPC initialization fails on every CPU, so cppc_cpufreq does not
>> register.
> It would be nice to have your CPC descriptors here, but in case the detection isn't wrong we
> have:
> Desired (5), Performance Limited (13), and OSPM Nominal (21) inaccessible

The detection is right. All three are SystemMemory with Bit Offset 0 and
Access Size 3, so each is a partial field inside a 32-bit unit:

   Desired              9 bits
   Performance Limited  2 bits
   OSPM Nominal         9 bits

Revision 3 describes identically and OSPM Nominal is absent there.


> I'm assuming this platform supports both 0 and 1 for AUTO_SEL_ENABLE.

Yes. Autonomous Selection is a writable 1-bit register, not a fixed
Integer 1. It reads zero, so nothing is forcing autonomous mode.


> We can disable Performance Limited then and OSPM Nominal but Desired we do need
> (unless the kernel forces AUTO_SEL_ENABLE=1?)
> We clearly cannot do a RMW on a write-only register (CPCv4), the only solution which
> may be possible is that we write all registers that lie in Desired access all the time,
> but that really depends on what registers those are. In case these registers are
> from other policies this would equate into a global lock, which clearly isn't what
> we would want.

Every SystemMemory register in the package occupies its own DWord, 8 bytes
apart. No other _CPC described field shares these access units, on this
processor or any other, so no cross-descriptor serialization is needed.
Fields described outside _CPC are not visible here.


> Any suggestion how this platform is supposed to work here?

Where the access unit is not shared, could the read be kept,
or the full unit written instead?
Both write only controls are 9 bits at Bit Offset 0, so the driver
takes bits 8:0 from the new value and only bits 31:9 from the read.
Those upper bits lie outside the register, so nothing undefined is
used or written back.

Thanks,
Sumit