linux-kernel - Re: [RFC PATCH 4/9] opp: core: Don't warn if required OPP device does not exist

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: <20211012092603.lkmhhjoo5v67wh44@vireshk-i7>
Date:   Tue, 12 Oct 2021 14:56:03 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Hector Martin <marcan@...can.st>
Cc:     Sibi Sankar <sibis@...eaurora.org>,
        Saravana Kannan <saravanak@...gle.com>,
        linux-arm-kernel@...ts.infradead.org,
        Alyssa Rosenzweig <alyssa@...enzweig.io>,
        Sven Peter <sven@...npeter.dev>, Marc Zyngier <maz@...nel.org>,
        Mark Kettenis <mark.kettenis@...all.nl>,
        Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@...onical.com>,
        Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Catalin Marinas <catalin.marinas@....com>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Kevin Hilman <khilman@...nel.org>,
        Ulf Hansson <ulf.hansson@...aro.org>,
        linux-clk@...r.kernel.org, devicetree@...r.kernel.org,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 4/9] opp: core: Don't warn if required OPP device
 does not exist

On 12-10-21, 14:57, Hector Martin wrote:
> On 12/10/2021 14.51, Viresh Kumar wrote:
> > On 12-10-21, 14:34, Hector Martin wrote:
> > > The table *is* assigned to a genpd (the memory controller), it's just that
> > > that genpd isn't actually a parent of the CPU device. Without the patch you
> > > end up with:
> > > 
> > > [    3.040060] cpu cpu4: Failed to set performance rate of cpu4: 0 (-19)
> > > [    3.042881] cpu cpu4: Failed to set required opps: -19
> > > [    3.045508] cpufreq: __target_index: Failed to change cpu frequency: -19
> > 
> > Hmm, Saravana and Sibi were working on a similar problem earlier and decided to
> > solve this using devfreq instead. Don't remember the exact series which got
> > merged for this, Sibi ?
> > 
> > If this part fails, how do you actually set the performance state of the memory
> > controller's genpd ?
> 
> The clock controller has the genpd as an actual power-domain parent, so it
> does it instead. From patch #7:
> 
> > +	if (cluster->has_pd)
> > +		dev_pm_genpd_set_performance_state(cluster->dev,
> > +						   dev_pm_opp_get_required_pstate(opp, 0));
> > +
> 
> This is arguably not entirely representative of how the hardware works,
> since technically the cluster switching couldn't care less what the memory
> controller is doing; it's a soft dependency, states that should be switched
> together but are not interdependent (in fact, the clock code does this
> unconditionally after the CPU p-state change, regardless of whether we're
> shifting up or down; this is, FWIW, the same order macOS uses, and it
> clearly doesn't matter which way you do it).

Yeah, I understand what you are doing. But the current patch is
incorrect in the sense that it can cause a bug on other platforms. To
make this work, you should rather set this genpd as parent of CPU
devices (which are doing anyway since you are updating them with CPU's
DVFS). With that the clk driver won't be required to do the magic
behind the scene.

-- 
viresh