From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 88FB93B19D0; Sun, 30 Aug 2026 11:57:55 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=217.140.110.172 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788091077; cv=none; b=swGP9ASwh8Opu96/f+3Fr7qtnW82N+5pPdchfe4H0jU8YmmySVxioUDriwqWyikOIxH4xa3EFzh3t0P/KpDBV/HZPdGAPud9yiGSZprDx6/ll6B057Ux/7XCe8oXl/tTdmoUOfvYxa7Bo9IDJW3YrJTpdF2rQa8Hw22jAs8c300= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788091077; c=relaxed/simple; bh=7zZy5pzFyhXYIy4HaLNuOF5zwJqYKloHSGw24dMLwos=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=q3NSqFcs5OAOv8MNeV04GqRDin/bn2MPL+hnsVarJ8ko31k71uDqHXqcWN12v20/9m8wW3RiMpmCVDbXRF6CSRScQ1i/mTmImwj71OAbznGmu4gaARBojA95aW4ABEvrvwLjoZBIfuU5xSF1+McCT8iMzsTzGTU+1YZpA1m4V/k= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com; spf=pass smtp.mailfrom=arm.com; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b=ZnHibw8k; arc=none smtp.client-ip=217.140.110.172 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=arm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=arm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=arm.com header.i=@arm.com header.b="ZnHibw8k" Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id 3711F1477; Sun, 30 Aug 2026 04:57:51 -0700 (PDT) Received: from e127648.arm.com (unknown [10.57.5.212]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPA id 857663F66F; Sun, 30 Aug 2026 04:57:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=arm.com; s=foss; t=1788091075; bh=7zZy5pzFyhXYIy4HaLNuOF5zwJqYKloHSGw24dMLwos=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=ZnHibw8kTHqLm5g3gUC4fu5B+bEKF452d8Vaa3JncHWXR+pWm4BAZ0Jiq681Rbgbe N9JTPu0yI+OC2t1ucDeTc/i8pP+t6fav3osbZPHfGMbTlf7Or0ghbrYdgs1OPuyWgw feESb3uuyziOWyCaY+GBGh9mcRD9n1pT2sd+HBBc= From: Christian Loehle To: "Rafael J . Wysocki" , Viresh Kumar Cc: linux-pm@vger.kernel.org, linux-acpi@vger.kernel.org, linux-kernel@vger.kernel.org, Len Brown , Jie Zhan , Lifeng Zheng , Pierre Gondois , Sumit Gupta , Sudeep Holla , Ionela Voinescu , zhongqiu.han@oss.qualcomm.com, Christian Loehle , Sashiko Subject: [PATCH v6 10/15] ACPI: CPPC: Reject direct reads of write-only controls Date: Sun, 30 Aug 2026 12:56:39 +0100 Message-Id: <20260830115644.2056983-11-christian.loehle@arm.com> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20260830115644.2056983-1-christian.loehle@arm.com> References: <20260830115644.2056983-1-christian.loehle@arm.com> Precedence: bulk X-Mailing-List: linux-pm@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Between _CPC revision 3 and revision 4, Desired Performance changed from Read/Write to Write. Revision 4 also added the write-only OSPM Nominal Performance control. ACPI 6.6 section 4.6.3 says reads of write-only bit positions produce undefined results. The public Desired Performance helper already rejects revision-4 readback, but the common register accessor still permits either write-only control to be read. Reject both centrally so new callers cannot consume undefined values. A partial SystemMemory field still makes cpc_write() read its complete access unit to preserve bits outside the field. MASK_VAL_WRITE() replaces every bit of the field being written, so that field's undefined readback is not propagated. However, another writer sharing the access unit would preserve and replay the write-only field when performing its own RMW. Reject such pairs; a partial write-only field may share its access unit only with disjoint read-only fields. The descriptor lock serializes supported RMW, and the preceding validation rejects unsafe cross-descriptor partial writers. Keep an invalid but locatable write-only SystemMemory descriptor represented during overlap validation, but mark it unreadable and unwritable. Otherwise a neighbouring RMW field could evade validation and replay undefined readback into the hidden control. Conservatively claim the larger of the declared access unit and logical field span. Check if the writer access unit covers the write-only field so harmless asymmetric geometries are not rejected. Preserve a decoded access-unit claim even when the malformed Bit Width is zero, and use that claimed range when testing whether another writer's access overlaps the hidden field. A retained range-only descriptor cannot issue a transaction over that whole conservative claim. Keep the full-width ownership rule for accessible descriptors, but reject a full-width writer against a range-only descriptor only when its access covers the hidden logical field. Likewise, count only accessible descriptors as writers in the generic writer-conflict rules; the dedicated write-only-field check still protects a retained descriptor from an accessible writer's RMW. Mark an inaccessible OSPM Nominal Performance control unsupported because it is optional. Do the same for inaccessible Desired Performance while parsing, then let the post-parse control check accept it only for immutable autonomous selection. This preserves the autonomous-only exception without accepting an unusable Desired control in non-autonomous mode. Do not advertise fast switching or zero transition latency unless Desired Performance remains writable. An inaccessible descriptor retained only for overlap validation still carries its original address-space identity, but cannot service a performance request. Fixes: 71e1815113f7 ("ACPI: CPPC: Add support for CPPC v4") Reported-by: Sashiko 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 --- drivers/acpi/cppc_acpi.c | 153 +++++++++++++++++++++++++++++++++++---- include/acpi/cppc_acpi.h | 2 + 2 files changed, 141 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 058f4c71d981..bb83b5c9ab31 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -171,7 +171,14 @@ static struct cpc_sysmem_node *cpc_sysmem_next(struct cpc_sysmem_node *node, static bool cpc_is_writable(const struct cpc_register_resource *cpc) { return cpc->type == ACPI_TYPE_BUFFER && - !IS_NULL_REG(&cpc->cpc_entry.reg); + !IS_NULL_REG(&cpc->cpc_entry.reg) && + !cpc->cpc_entry.write_unsupported; +} + +static bool cpc_is_readable(const struct cpc_register_resource *cpc) +{ + return cpc->type != ACPI_TYPE_BUFFER || + !cpc->cpc_entry.read_unsupported; } static bool cpc_entry_present(const struct cpc_register_resource *cpc) @@ -313,6 +320,22 @@ static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg) return width / 8; } +static u64 cpc_sysmem_field_size(const struct cpc_reg *gas) +{ + return DIV_ROUND_UP((u64)gas->bit_offset + gas->bit_width, 8); +} + +static u64 cpc_sysmem_claim_size(const struct cpc_register_resource *reg) +{ + const struct cpc_reg *gas = ®->cpc_entry.reg; + u64 access_size = cpc_sysmem_access_size(reg); + + if (!gas->bit_width) + return access_size; + + return max(access_size, cpc_sysmem_field_size(gas)); +} + static bool cpc_reg_access_aligned(const struct cpc_reg *reg, u64 access_size) { /* x86 MMIO and port-I/O accessors support unaligned addresses. */ @@ -324,8 +347,8 @@ static bool cpc_sysmem_access_units_overlap(const struct cpc_register_resource * { const struct cpc_reg *a_gas = &a->cpc_entry.reg; const struct cpc_reg *b_gas = &b->cpc_entry.reg; - u64 a_size = cpc_sysmem_access_size(a); - u64 b_size = cpc_sysmem_access_size(b); + u64 a_size = cpc_sysmem_claim_size(a); + u64 b_size = cpc_sysmem_claim_size(b); /* Keep the conservative locking path for malformed access widths. */ if (!a_size || !b_size) @@ -355,6 +378,21 @@ static bool cpc_reg_is_writable(unsigned int reg_idx) } } +static bool cpc_reg_is_write_only(const struct cpc_desc *cpc_desc, + unsigned int reg_idx) +{ + return cpc_desc->version >= CPPC_V4_REV && + (reg_idx == DESIRED_PERF || reg_idx == OSPM_NOMINAL_PERF); +} + +static void cpc_disable_reg(struct cpc_desc *cpc_desc, unsigned int reg_idx) +{ + struct cpc_register_resource *reg = &cpc_desc->cpc_regs[reg_idx]; + + reg->type = ACPI_TYPE_INTEGER; + reg->cpc_entry.int_value = 0; +} + static bool cpc_sysmem_reg_needs_rmw(const struct cpc_register_resource *reg) { const struct cpc_reg *gas = ®->cpc_entry.reg; @@ -363,7 +401,7 @@ static bool cpc_sysmem_reg_needs_rmw(const struct cpc_register_resource *reg) return gas->bit_offset || gas->bit_width != access_size * 8; } -static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc, +static int cpc_validate_sysmem_reg(struct cpc_desc *cpc_desc, const struct cpc_reg *gas, unsigned int reg_idx) { @@ -388,6 +426,23 @@ static int cpc_validate_sysmem_reg(const struct cpc_desc *cpc_desc, return 0; invalid: + access_size = 0; + if (access_width == 8 || access_width == 16 || + access_width == 32 || access_width == 64) + access_size = access_width / 8; + if (gas->bit_width) + access_size = max(access_size, cpc_sysmem_field_size(gas)); + if (cpc_reg_is_write_only(cpc_desc, reg_idx) && gas->address && + access_size && gas->address <= U64_MAX - (access_size - 1)) { + struct cpc_register_resource *reg = &cpc_desc->cpc_regs[reg_idx]; + + pr_warn("CPU%d: _CPC v%d register %u is inaccessible; keeping its range reserved\n", + cpc_desc->cpu_id, cpc_desc->version, reg_idx); + reg->cpc_entry.read_unsupported = true; + reg->cpc_entry.write_unsupported = true; + return 0; + } + pr_debug("CPU:%d invalid SystemMemory GAS for _CPC register %u\n", cpc_desc->cpu_id, reg_idx); return -EINVAL; @@ -445,6 +500,39 @@ static bool cpc_sysmem_fields_overlap(const struct cpc_register_resource *a, !cpc_bit_position_before(&b_end, &a_start); } +static bool cpc_sysmem_access_overlaps_field(const struct cpc_register_resource *access, + const struct cpc_register_resource *field) +{ + const struct cpc_reg *access_gas = &access->cpc_entry.reg; + const struct cpc_reg *field_gas = &field->cpc_entry.reg; + u64 access_last; + u64 field_start; + u64 field_last; + + if (!field_gas->bit_width) + return cpc_sysmem_access_units_overlap(access, field); + + access_last = access_gas->address + + cpc_sysmem_access_size(access) - 1; + field_start = field_gas->address + field_gas->bit_offset / 8; + field_last = field_gas->address + + (field_gas->bit_offset + field_gas->bit_width - 1) / 8; + + return access_gas->address <= field_last && field_start <= access_last; +} + +static bool cpc_sysmem_full_width_conflicts(const struct cpc_register_resource *writer, + const struct cpc_register_resource *other) +{ + /* + * An accessible descriptor may issue a transaction over its complete + * access unit. A retained range-only descriptor cannot; protect only + * its logical field from the full-width writer. + */ + return cpc_is_readable(other) || cpc_is_writable(other) || + cpc_sysmem_access_overlaps_field(writer, other); +} + static bool cpc_same_sysmem_register(unsigned int a_idx, const struct cpc_register_resource *a, unsigned int b_idx, @@ -474,8 +562,8 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc, !cpc_sysmem_access_units_overlap(a, b)) return 0; - a_writable = cpc_reg_is_writable(a_idx); - b_writable = cpc_reg_is_writable(b_idx); + a_writable = cpc_reg_is_writable(a_idx) && cpc_is_writable(a); + b_writable = cpc_reg_is_writable(b_idx) && cpc_is_writable(b); if (!a_writable && !b_writable) return 0; @@ -498,8 +586,10 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc, } /* A full-width writable register owns its complete access unit. */ - if ((a_writable && !cpc_sysmem_reg_needs_rmw(a)) || - (b_writable && !cpc_sysmem_reg_needs_rmw(b)) || + if ((a_writable && !cpc_sysmem_reg_needs_rmw(a) && + cpc_sysmem_full_width_conflicts(a, b)) || + (b_writable && !cpc_sysmem_reg_needs_rmw(b) && + cpc_sysmem_full_width_conflicts(b, a)) || (a_writable && b_writable && cpc_sysmem_fields_overlap(a, b))) goto conflict; @@ -507,6 +597,18 @@ static int cpc_validate_sysmem_pair(const struct cpc_desc *a_desc, if (a_desc != b_desc && a_writable && b_writable) goto conflict; + /* + * RMW of either writer preserves the other field. If that field is + * write-only, its readback is undefined and cannot safely be replayed. + */ + if ((cpc_reg_is_write_only(a_desc, a_idx) && b_writable && + cpc_sysmem_reg_needs_rmw(b) && + cpc_sysmem_access_overlaps_field(b, a)) || + (cpc_reg_is_write_only(b_desc, b_idx) && a_writable && + cpc_sysmem_reg_needs_rmw(a) && + cpc_sysmem_access_overlaps_field(a, b))) + goto conflict; + return 0; conflict: @@ -589,7 +691,7 @@ static int cpc_register_sysmem_desc(struct cpc_desc *cpc_desc) continue; node = &cpc_desc->sysmem_nodes[i]; - size = cpc_sysmem_access_size(reg); + size = cpc_sysmem_claim_size(reg); node->start = reg->cpc_entry.reg.address; node->last = node->start + size - 1; node->desc = cpc_desc; @@ -970,7 +1072,7 @@ bool cppc_allow_fast_switch(const struct cpumask *cpus) min_reg = &cpc_ptr->cpc_regs[MIN_PERF]; max_reg = &cpc_ptr->cpc_regs[MAX_PERF]; - if (!CPC_SUPPORTED(desired_reg) || + if (!cpc_is_writable(desired_reg) || (!CPC_IN_SYSTEM_MEMORY(desired_reg) && !CPC_IN_SYSTEM_IO(desired_reg)) || (CPC_SUPPORTED(min_reg) && @@ -1370,6 +1472,10 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) goto out_free; } + cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_BUFFER; + memcpy(&cpc_ptr->cpc_regs[i - 2].cpc_entry.reg, gas_t, + sizeof(*gas_t)); + /* * The PCC Subspace index is encoded inside * the CPC table entries. The same PCC index @@ -1396,10 +1502,24 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) size_t access_width; err = cpc_validate_sysmem_reg(cpc_ptr, gas_t, i - 2); + if (err && (i - 2 == DESIRED_PERF || + i - 2 == OSPM_NOMINAL_PERF)) { + const char *name = i - 2 == DESIRED_PERF ? + "Desired Performance" : + "OSPM Nominal Performance"; + + pr_warn("CPU%d: disabling inaccessible %s register\n", + pr->id, name); + cpc_disable_reg(cpc_ptr, i - 2); + continue; + } if (err) { ret = err; goto out_free; } + if (!cpc_is_readable(&cpc_ptr->cpc_regs[i - 2]) && + !cpc_is_writable(&cpc_ptr->cpc_regs[i - 2])) + continue; if (!osc_cpc_flexible_adr_space_confirmed) { pr_debug("Flexible address space capability not supported\n"); @@ -1451,10 +1571,6 @@ int acpi_cppc_processor_probe(struct acpi_processor *pr) goto out_free; } } - - cpc_ptr->cpc_regs[i - 2].type = ACPI_TYPE_BUFFER; - memcpy(&cpc_ptr->cpc_regs[i - 2].cpc_entry.reg, gas_t, - sizeof(*gas_t)); } else if (cpc_obj->type == ACPI_TYPE_PACKAGE && (i - 2) == RESOURCE_PRIORITY) { /* * ACPI 6.6, s8.4.6.1.2.7 defines Resource Priority as a @@ -1794,6 +1910,10 @@ static int cpc_write(int cpu, struct cpc_register_resource *reg_res, u64 val) } if (reg->bit_offset || reg->bit_width != size) { + /* + * MASK_VAL_WRITE() discards the field's old bits, so undefined + * readback from a write-only field is not propagated. + */ switch (size) { case 8: prev_val = readb_relaxed(vaddr); @@ -1885,6 +2005,8 @@ static int cppc_get_reg_val(int cpu, enum cppc_regs reg_idx, u64 *val) pr_debug("No CPC descriptor for CPU:%d\n", cpu); return -ENODEV; } + if (cpc_reg_is_write_only(cpc_desc, reg_idx)) + return -EOPNOTSUPP; reg = &cpc_desc->cpc_regs[reg_idx]; @@ -2975,6 +3097,9 @@ int cppc_get_transition_latency(int cpu_num) return -ENODATA; desired_reg = &cpc_desc->cpc_regs[DESIRED_PERF]; + if (!cpc_is_writable(desired_reg)) + return -ENODATA; + if (CPC_IN_SYSTEM_MEMORY(desired_reg) || CPC_IN_SYSTEM_IO(desired_reg)) return 0; diff --git a/include/acpi/cppc_acpi.h b/include/acpi/cppc_acpi.h index 168e3143e6ae..4e5f59bc95f8 100644 --- a/include/acpi/cppc_acpi.h +++ b/include/acpi/cppc_acpi.h @@ -72,6 +72,8 @@ struct cpc_register_resource { struct { struct cpc_reg reg; bool use_rmw_lock; + bool read_unsupported; + bool write_unsupported; }; u64 int_value; } cpc_entry; -- 2.34.1