Re: [PATCH v3 2/2] cpufreq: qcom: Add cpufreq scaling support for Qualcomm Shikra SoC

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Invalid `kfree()` on a `devm_kzalloc`-managed interior pointer in `qcom_cpufreq_hw_cpu_exit()` leads to a guaranteed SLUB BUG and kernel panic.
- [High] Boost/Turbo frequency is lost and incorrectly reported as invalid (~0u) when the hardware frequency LUT is completely populated.
- [High] Memory and OPP entries are permanently leaked in the error paths of `qcom_cpufreq_hw_cpu_init()`.
- [High] An unbalanced `disable_irq_nosync()` during CPU offline leaves the LMh hardware throttle IRQ permanently disabled.
- [Medium] Unhandled `request_threaded_irq()` failure leaves `throttle_irq` populated, leading to invalid IRQ operations and kernel warnings.
--

commit 3d8cb988016155d4dc9ae011b0b91c02adb8ac0d
Author: Taniya Das <taniya.das@oss.qualcomm.com>

cpufreq: qcom: Add cpufreq scaling support for Qualcomm Shikra SoC

This commit introduces cpufreq scaling support for the Qualcomm Shikra SoC by
adding the shikra_epss_soc_data structure. It dynamically limits the frequency
lookup table based on the new lut_max_entries field, replacing the hardcoded
LUT_MAX_ENTRIES limit to prevent out-of-bounds reads.

> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c b/drivers/cpufreq/qcom-cpufreq-hw.c
> index ea9a20d27b8fd..3d5a865fb8a35 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
[ ... ]
> @@ -236,7 +238,7 @@ static int qcom_cpufreq_hw_read_lut(struct device *cpu_dev,
>  		icc_scaling_enabled = false;
>  	}
>  
> -	for (i = 0; i < LUT_MAX_ENTRIES; i++) {
> +	for (i = 0; i < soc_data->lut_max_entries; i++) {
>  		data = readl_relaxed(drv_data->base + soc_data->reg_freq_lut +
>  				      i * soc_data->lut_row_size);
>  		src = FIELD_GET(LUT_SRC, data);

[Severity: High]
This is a pre-existing issue, but if the hardware frequency lookup table is
completely populated up to soc_data->lut_max_entries with unique
frequencies, doesn't the loop terminate without processing a duplicate
entry? 

If so, it appears the fix-up block is bypassed, leaving the highest
performance state marked as CPUFREQ_ENTRY_INVALID (~0u). Could this cause
qcom_cpufreq_get_freq() to return ~0u and potentially break the scaling
governors?

[ ... ]

[Severity: High]
This isn't a bug introduced by this patch, but looking at
qcom_cpufreq_hw_cpu_init(), if steps fail after policy->freq_table is
allocated via kzalloc_objs (such as a failure in qcom_cpufreq_hw_lmh_init),
the function directly returns an error code. 

Since the cpufreq core doesn't invoke the exit() callback on an init()
failure, does this permanently leak the dynamically allocated memory and opp
entries?

[Severity: Critical]
This is a pre-existing issue, but in qcom_cpufreq_hw_cpu_exit(), the code
calls kfree(data) on policy->driver_data. 

Given that qcom_cpufreq.data was originally allocated using devm_kzalloc()
in qcom_cpufreq_hw_driver_probe(), won't passing this interior pointer
directly to kfree() cause a slab allocator bug and kernel panic?

[Severity: High]
This is a pre-existing issue, but when hardware throttling occurs,
qcom_lmh_dcvs_handle_irq() calls disable_irq_nosync() and schedules
throttle_work to re-enable it. 

If the cpu is offlined before the work executes,
qcom_cpufreq_hw_cpu_offline() cancels the work and calls
disable_irq_nosync() again. 

Will this bring the irq disable depth to 2, so that when the cpu is brought
back online, qcom_cpufreq_ready() calling enable_irq() only reduces the
depth to 1 and leaves the thermal throttling interrupt permanently disabled?

[Severity: Medium]
This isn't a bug introduced by this patch, but in qcom_cpufreq_hw_lmh_init(),
if request_threaded_irq() fails, it logs an error and returns 0. 

However, it doesn't seem to invalidate data->throttle_irq. Because it is
still populated with a valid irq number, will subsequent cpufreq callbacks
like qcom_cpufreq_ready() and qcom_cpufreq_hw_lmh_exit() erroneously attempt
to call enable_irq() and free_irq() on it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260521-shikra-cpufreq-scaling-v3-0-883c13d1e514@oss.qualcomm.com?part=2