From 76e6914a843ed2bc2bea152c30cc7dd445d02276 Mon Sep 17 00:00:00 2001 From: Christian Loehle Date: Wed, 16 Sep 2026 17:27:50 +0100 Subject: [PATCH] ACPI: CPPC: Serialize PCC EPP payload updates 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. Reject an unsupported PCC width before any non-PCC control is changed. Classify every writable control as either PCC or non-PCC. For SystemIO, require port-I/O support and an access within the port range. Require natural alignment on non-x86 platforms, whose port-I/O accessors use MMIO; x86 port instructions support unaligned addresses. Also require a field covering the complete access unit. cpc_write() performs a raw whole-port write, so any other geometry could update the wrong bits or corrupt an adjacent field. 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 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 Tested-by: Sumit Gupta Link: https://patch.msgid.link/20260916162805.1039247-6-christian.loehle@arm.com Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 104 +++++++++++++++++++++++++++------------ 1 file changed, 72 insertions(+), 32 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 42f3714f82ab..2198376a6a9c 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -260,6 +260,22 @@ static bool cpc_pcc_write_supported(const struct cpc_register_resource *reg) ((((val) & GENMASK(((reg)->bit_width) - 1, 0)) << (reg)->bit_offset) | \ ((prev_val) & ~(GENMASK(((reg)->bit_width) - 1, 0) << (reg)->bit_offset))) \ +static bool +cpc_system_io_write_supported(const struct cpc_register_resource *reg) +{ + const struct cpc_reg *gas = ®->cpc_entry.reg; + unsigned int access_width = GET_BIT_WIDTH(gas); + u64 access_size; + + if (!IS_ENABLED(CONFIG_HAS_IOPORT) || !CPC_IN_SYSTEM_IO(reg) || + (access_width != 8 && access_width != 16 && access_width != 32)) + return false; + + access_size = access_width / 8; + return !gas->bit_offset && gas->bit_width == access_width && + gas->address <= U16_MAX - (access_size - 1) && + (IS_ENABLED(CONFIG_X86) || IS_ALIGNED(gas->address, access_size)); +} static u64 cpc_sysmem_access_size(const struct cpc_register_resource *reg) { const struct cpc_reg *gas = ®->cpc_entry.reg; @@ -1947,8 +1963,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) { @@ -1962,52 +1980,74 @@ int cppc_set_epp_perf(int cpu, struct cppc_perf_ctrls *perf_ctrls, bool enable) auto_sel_reg->cpc_entry.int_value == 1) return -EOPNOTSUPP; - 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); + if ((auto_sel_pcc && !cpc_pcc_write_supported(auto_sel_reg)) || + (epp_pcc && !cpc_pcc_write_supported(epp_set_reg))) + return -EFAULT; + if ((cpc_is_writable(auto_sel_reg) && + CPC_IN_SYSTEM_IO(auto_sel_reg) && + !cpc_system_io_write_supported(auto_sel_reg)) || + (cpc_is_writable(epp_set_reg) && CPC_IN_SYSTEM_IO(epp_set_reg) && + !cpc_system_io_write_supported(epp_set_reg))) + return -EOPNOTSUPP; - if (CPC_IN_PCC(epp_set_reg) || CPC_IN_PCC(auto_sel_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; + + /* 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)) { - ret = cpc_write(cpu, auto_sel_reg, enable); - if (ret) - return ret; - } - - if (cpc_is_writable(epp_set_reg)) { - ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf); - 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); - /* 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 = 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 (epp_ffh_sysmem) { - ret = cpc_write(cpu, epp_set_reg, - perf_ctrls->energy_perf); + if (epp_pcc) { + ret = cpc_write(cpu, epp_set_reg, perf_ctrls->energy_perf); if (ret) - return ret; + goto out_unlock; } + + /* after writing CPC, transfer the ownership of PCC to platform */ + ret = send_pcc_cmd(pcc_ss_id, CMD_WRITE); + +out_unlock: + if (ret) + cppc_abort_pending_pcc_write(pcc_ss_id, 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;