linux-kernel - Re: [PATCH 2/3] clk: keystone: don't cache clock rate

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] [day] [month] [year] [list]

Message-ID: <DD23HER39PNM.O17TMFNNWD37@ti.com>
Date: Thu, 25 Sep 2025 13:32:37 -0500
From: Randolph Sapp <rs@...com>
To: Maxime Ripard <mripard@...nel.org>, Randolph Sapp <rs@...com>
CC: Kevin Hilman <khilman@...nel.org>, Michael Walle <mwalle@...nel.org>,
        Frank Binns <frank.binns@...tec.com>,
        Matt Coster <matt.coster@...tec.com>,
        Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
        Thomas Zimmermann
	<tzimmermann@...e.de>,
        David Airlie <airlied@...il.com>, Simona Vetter
	<simona@...ll.ch>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski
	<krzk+dt@...nel.org>,
        Conor Dooley <conor+dt@...nel.org>, "Nishanth Menon"
	<nm@...com>,
        Vignesh Raghavendra <vigneshr@...com>,
        Tero Kristo
	<kristo@...nel.org>,
        Santosh Shilimkar <ssantosh@...nel.org>,
        "Michael
 Turquette" <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>, Andrew
 Davis <afd@...com>,
        <dri-devel@...ts.freedesktop.org>, <devicetree@...r.kernel.org>,
        <linux-kernel@...r.kernel.org>, <linux-arm-kernel@...ts.infradead.org>,
        <linux-clk@...r.kernel.org>
Subject: Re: [PATCH 2/3] clk: keystone: don't cache clock rate

On Thu Sep 25, 2025 at 6:43 AM CDT, Maxime Ripard wrote:
> On Wed, Sep 24, 2025 at 09:26:17PM -0500, Randolph Sapp wrote:
>> On Wed Sep 17, 2025 at 10:24 AM CDT, Kevin Hilman wrote:
>> > Michael Walle <mwalle@...nel.org> writes:
>> >
>> >> The TISCI firmware will return 0 if the clock or consumer is not
>> >> enabled although there is a stored value in the firmware. IOW a call to
>> >> set rate will work but at get rate will always return 0 if the clock is
>> >> disabled.
>> >> The clk framework will try to cache the clock rate when it's requested
>> >> by a consumer. If the clock or consumer is not enabled at that point,
>> >> the cached value is 0, which is wrong.
>> >
>> > Hmm, it also seems wrong to me that the clock framework would cache a
>> > clock rate when it's disabled.  On platforms with clocks that may have
>> > shared management (eg. TISCI or other platforms using SCMI) it's
>> > entirely possible that when Linux has disabled a clock, some other
>> > entity may have changed it.
>> >
>> > Could another solution here be to have the clk framework only cache when
>> > clocks are enabled?
>> 
>> So I looked into that. There are still about 34 clock operations that are
>> functionally uncached, but it does seem more logical than treating everything as
>> uncached.
>> 
>> Side note, why would someone even want to read the rate of an unprepared clock?
>> I dumped some debug info for all the clocks tripping this new uncached path.
>> Seems weird that it's even happening this often. Even weirder that it's
>> apparently happening 3 times to cpu0's core clock on the board I'm currently
>> testing.
>
> The short, unsatisfying, answer is that the API explicitly allowed it so far.
>
> It's also somewhat natural when you have a functional rate to set it up
> before enabling it and the logic driven by it so that you would avoid a
> rate change, or something like a cycle where you would enable, shut
> down, reparent, enable the clock again.
>
> In such a case, we would either need the cache, or to read the rate, to
> know if we have to change the clock rate in the first place.
>
> Maxime

Thanks Maxime. I'll take this as a hint to stop digging for the moment. Right
now, treating unprepared clocks as untrusted / uncached makes sense and
shouldn't be too much of a performance issue.

- Randolph