[PATCH] cpufreq: scmi: publish perf_ops with release semantics
Zhongqiu Han
zhongqiu.han at oss.qualcomm.com
Mon Sep 21 21:26:11 PDT 2026
Hi Jaidev,
On 9/22/2026 9:19 AM, Jaidev Shastri via B4 Relay wrote:
> From: Jaidev Shastri <jaidevshastri at vt.edu>
>
> scmi_cpufreq_probe() stores the SCMI performance protocol ops pointer to
> the file-scope perf_ops with a plain store and then registers the
> cpufreq driver. The callbacks read perf_ops with plain loads, starting
> with scmi_cpufreq_init() on whichever CPU brings up a policy.
This doesn't appear to be a real issue, similar to the case discussed in
https://lore.kernel.org/all/495a47a3-ac6d-4002-abe3-23b85b2f2034@oss.qualcomm.com/
>
> Keep the pointer in a local, publish it with smp_store_release() and
> read it with smp_load_acquire() in scmi_cpufreq_init(), the first
> callback invoked for a policy.
>
> Found with MBCheck, a static herd7-based memory consistency checker.
>
> Signed-off-by: Jaidev Shastri <jaidevshastri at vt.edu>
> ---
> drivers/cpufreq/scmi-cpufreq.c | 21 +++++++++++++++------
> 1 file changed, 15 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/cpufreq/scmi-cpufreq.c b/drivers/cpufreq/scmi-cpufreq.c
> index 4edb4f7a8..c394f63cc 100644
> --- a/drivers/cpufreq/scmi-cpufreq.c
> +++ b/drivers/cpufreq/scmi-cpufreq.c
> @@ -197,6 +197,8 @@ static int scmi_limit_notify_cb(struct notifier_block *nb, unsigned long event,
>
> static int scmi_cpufreq_init(struct cpufreq_policy *policy)
> {
> + /* Pairs with the smp_store_release() in scmi_cpufreq_probe(). */
> + const struct scmi_perf_proto_ops *ops = smp_load_acquire(&perf_ops);
> int ret, nr_opp, domain;
> unsigned int latency;
> struct device *cpu_dev;
> @@ -252,7 +254,7 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
> */
> nr_opp = dev_pm_opp_get_opp_count(cpu_dev);
> if (nr_opp <= 0) {
> - ret = perf_ops->device_opps_add(ph, cpu_dev, domain);
> + ret = ops->device_opps_add(ph, cpu_dev, domain);
> if (ret) {
> dev_warn(cpu_dev, "failed to add opps to the device\n");
> goto out_free_cpumask;
> @@ -293,14 +295,14 @@ static int scmi_cpufreq_init(struct cpufreq_policy *policy)
> /* SCMI allows DVFS request for any domain from any CPU */
> policy->dvfs_possible_from_any_cpu = true;
>
> - latency = perf_ops->transition_latency_get(ph, domain);
> + latency = ops->transition_latency_get(ph, domain);
> if (!latency)
> latency = CPUFREQ_DEFAULT_TRANSITION_LATENCY_NS;
>
> policy->cpuinfo.transition_latency = latency;
>
> policy->fast_switch_possible =
> - perf_ops->fast_switch_possible(ph, domain);
> + ops->fast_switch_possible(ph, domain);
>
> policy->transition_delay_us =
> scmi_get_rate_limit(domain, policy->fast_switch_possible);
> @@ -443,6 +445,7 @@ static int scmi_cpufreq_probe(struct scmi_device *sdev)
> int ret;
> struct device *dev = &sdev->dev;
> const struct scmi_handle *handle;
> + const struct scmi_perf_proto_ops *ops;
>
> handle = sdev->handle;
>
> @@ -451,9 +454,15 @@ static int scmi_cpufreq_probe(struct scmi_device *sdev)
>
> scmi_cpufreq_driver.driver_data = sdev;
>
> - perf_ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PERF, &ph);
> - if (IS_ERR(perf_ops))
> - return PTR_ERR(perf_ops);
> + ops = handle->devm_protocol_get(sdev, SCMI_PROTOCOL_PERF, &ph);
> + if (IS_ERR(ops))
> + return PTR_ERR(ops);
> + /*
> + * The cpufreq callbacks read perf_ops after cpufreq_register_driver()
> + * below. Publish it with release semantics so that ph and the ops are
> + * visible together.
> + */
> + smp_store_release(&perf_ops, ops);
>
> #ifdef CONFIG_COMMON_CLK
> /* dummy clock provider as needed by OPP if clocks property is used */
>
> ---
> base-commit: 93f51579e7df248780214094418f205253383cc5
> change-id: 20260921-mb-scmi-cpufreq-cabfeb548027
>
> Best regards,
> --
> Jaidev Shastri <jaidevshastri at vt.edu>
>
>
>
--
Thx and BRs,
Zhongqiu Han
More information about the linux-arm-kernel
mailing list