From 27954e29d7de66e65ef1be4ce5a50505066c9bb6 Mon Sep 17 00:00:00 2001 From: Christian Loehle Date: Wed, 16 Sep 2026 17:27:51 +0100 Subject: [PATCH] ACPI: CPPC: Release CPC descriptors through kobject 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. The frequency-invariance teardown can run after processor removal has unpublished a descriptor. Remember which PCC work items were initialized so policy exit drains them without looking up that descriptor. Also make the counter-transport query tolerate a missing descriptor. Fixes: 158c998ea44b ("ACPI / CPPC: add sysfs support to compute delivered performance") Reported-by: Sashiko Link: https://sashiko.dev/#/patchset/20260724134251.1632824-1-christian.loehle%40arm.com Signed-off-by: Christian Loehle Tested-by: Sumit Gupta Link: https://patch.msgid.link/20260916162805.1039247-7-christian.loehle@arm.com Signed-off-by: Rafael J. Wysocki --- drivers/acpi/cppc_acpi.c | 52 +++++++++++++++++++++------------- drivers/cpufreq/cppc_cpufreq.c | 15 +++++----- 2 files changed, 39 insertions(+), 28 deletions(-) diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c index 2198376a6a9c..0f0cce383007 100644 --- a/drivers/acpi/cppc_acpi.c +++ b/drivers/acpi/cppc_acpi.c @@ -373,7 +373,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, }; @@ -1178,21 +1198,14 @@ 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); return 0; out_free: - /* 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: if (cpc_present) @@ -1211,10 +1224,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--; @@ -1225,20 +1242,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); @@ -1822,6 +1831,9 @@ bool cppc_perf_ctrs_in_pcc_cpu(unsigned int cpu) { struct cpc_desc *cpc_desc = per_cpu(cpc_desc_ptr, cpu); + if (!cpc_desc) + return false; + return CPC_IN_PCC(&cpc_desc->cpc_regs[DELIVERED_CTR]) || CPC_IN_PCC(&cpc_desc->cpc_regs[REFERENCE_CTR]) || CPC_IN_PCC(&cpc_desc->cpc_regs[CTR_WRAP_TIME]); diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c index 80893844353c..af46f913e907 100644 --- a/drivers/cpufreq/cppc_cpufreq.c +++ b/drivers/cpufreq/cppc_cpufreq.c @@ -41,6 +41,7 @@ MODULE_PARM_DESC(fie_disabled, "Disable Frequency Invariance Engine (FIE)"); /* Frequency invariance support */ struct cppc_freq_invariance { int cpu; + bool pcc_work_initialized; struct irq_work irq_work; struct kthread_work work; struct cppc_perf_fb_ctrs prev_perf_fb_ctrs; @@ -163,6 +164,7 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy) if (cppc_perf_ctrs_in_pcc_cpu(cpu)) { kthread_init_work(&cppc_fi->work, cppc_scale_freq_workfn); init_irq_work(&cppc_fi->irq_work, cppc_irq_work); + cppc_fi->pcc_work_initialized = true; sftd = &cppc_sftd_pcc; } @@ -184,12 +186,8 @@ static void cppc_cpufreq_cpu_fie_init(struct cpufreq_policy *policy) } /* - * We free all the resources on policy's removal and not on CPU removal as the - * irq-work are per-cpu and the hotplug core takes care of flushing the pending - * irq-works (hint: smpcfd_dying_cpu()) on CPU hotplug. Even if the kthread-work - * fires on another CPU after the concerned CPU is removed, it won't harm. - * - * We just need to make sure to remove them all on policy->exit(). + * Drain work initialized by this policy even if processor removal has + * already unpublished the CPU's CPC descriptor. */ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy) { @@ -203,11 +201,12 @@ static void cppc_cpufreq_cpu_fie_exit(struct cpufreq_policy *policy) topology_clear_scale_freq_source(SCALE_FREQ_SOURCE_CPPC, policy->related_cpus); for_each_cpu(cpu, policy->related_cpus) { - if (!cppc_perf_ctrs_in_pcc_cpu(cpu)) - continue; cppc_fi = &per_cpu(cppc_freq_inv, cpu); + if (!cppc_fi->pcc_work_initialized) + continue; irq_work_sync(&cppc_fi->irq_work); kthread_cancel_work_sync(&cppc_fi->work); + cppc_fi->pcc_work_initialized = false; } }