linux-kernel - Re: [PATCH] Revert "clk: Fix invalid execution of clk_set

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z07AXbQvvZwI8Ki6@hovoldconsulting.com>
Date: Tue, 3 Dec 2024 09:25:01 +0100
From: Johan Hovold <johan@...nel.org>
To: Stephen Boyd <sboyd@...nel.org>, Viresh Kumar <viresh.kumar@...aro.org>,
	Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
Cc: Johan Hovold <johan+linaro@...nel.org>,
	Michael Turquette <mturquette@...libre.com>,
	linux-clk@...r.kernel.org, linux-kernel@...r.kernel.org,
	regressions@...ts.linux.dev, Aishwarya TCV <aishwarya.tcv@....com>,
	Chuan Liu <chuan.liu@...ogic.com>,
	Sudeep Holla <sudeep.holla@....com>, linux-pm@...r.kernel.org
Subject: Re: [PATCH] Revert "clk: Fix invalid execution of clk_set_rate"

[ +CC: Viresh and Sudeep ]

On Mon, Dec 02, 2024 at 05:20:06PM -0800, Stephen Boyd wrote:
> Quoting Johan Hovold (2024-12-02 02:06:21)
> > This reverts commit 25f1c96a0e841013647d788d4598e364e5c2ebb7.
> > 
> > The offending commit results in errors like
> > 
> >         cpu cpu0: _opp_config_clk_single: failed to set clock rate: -22
> > 
> > spamming the logs on the Lenovo ThinkPad X13s and other Qualcomm
> > machines when cpufreq tries to update the CPUFreq HW Engine clocks.
> > 
> > As mentioned in commit 4370232c727b ("cpufreq: qcom-hw: Add CPU clock
> > provider support"):
> > 
> >         [T]he frequency supplied by the driver is the actual frequency
> >         that comes out of the EPSS/OSM block after the DCVS operation.
> >         This frequency is not same as what the CPUFreq framework has set
> >         but it is the one that gets supplied to the CPUs after
> >         throttling by LMh.
> > 
> > which seems to suggest that the driver relies on the previous behaviour
> > of clk_set_rate().
> 
> I don't understand why a clk provider is needed there. Is anyone looking
> into the real problem?

I mentioned this to Mani yesterday, but I'm not sure if he has had time
to look into it yet. And I forgot to CC Viresh who was involved in
implementing this. There is comment of his in the thread where this
feature was added:

	Most likely no one will ever do clk_set_rate() on this new
	clock, which is fine, though OPP core will likely do
	clk_get_rate() here.

which may suggest that some underlying assumption has changed. [1]

There are some more details in that thread that should explain why
things were implemented the way they were:

	https://lore.kernel.org/linux-arm-msm/20221117053145.10409-1-manivannan.sadhasivam@linaro.org/

> > Since this affects many Qualcomm machines, let's revert for now.
> > 
> > Fixes: 25f1c96a0e84 ("clk: Fix invalid execution of clk_set_rate")
> > Reported-by: Aishwarya TCV <aishwarya.tcv@....com>
> > Link: https://lore.kernel.org/all/e2d83e57-ad07-411b-99f6-a4fc3c4534fa@arm.com/
> > Cc: Chuan Liu <chuan.liu@...ogic.com>
> > Cc: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> > Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> > ---
> 
> Applied to clk-fixes

Thanks.

Johan

[1] https://lore.kernel.org/linux-arm-msm/20221118055730.yrzpuih3zfko5c2q@vireshk-i7/