linux-kernel - Re: [PATCH] ARM: Don't ever downscale loops_per

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]

Date:	Wed, 14 May 2014 14:42:55 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Nicolas Pitre <nicolas.pitre@...aro.org>
Cc:	Stephen Warren <swarren@...dotorg.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Viresh Kumar <viresh.kumar@...aro.org>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	Will Deacon <will.deacon@....com>,
	John Stultz <john.stultz@...aro.org>,
	David Riley <davidriley@...omium.org>,
	"olof@...om.net" <olof@...om.net>,
	Sonny Rao <sonnyrao@...omium.org>,
	Santosh Shilimkar <santosh.shilimkar@...com>,
	Shawn Guo <shawn.guo@...aro.org>,
	Stephen Boyd <sboyd@...eaurora.org>,
	Marc Zyngier <marc.zyngier@....com>,
	Stephen Warren <swarren@...dia.com>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	"linux-arm-kernel@...ts.infradead.org" 
	<linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] ARM: Don't ever downscale loops_per_jiffy in SMP systems#

Hi,

On Tue, May 13, 2014 at 4:29 PM, Nicolas Pitre <nicolas.pitre@...aro.org> wrote:
> On Tue, 13 May 2014, Nicolas Pitre wrote:
>
>> On Tue, 13 May 2014, Stephen Warren wrote:
>>
>> > On 05/13/2014 03:50 PM, Doug Anderson wrote:
>> > ...
>> > > ...but then I found the true problem shows up when we transition
>> > > between very low frequencies on exynos, like between 200MHz and
>> > > 300MHz.  While transitioning between frequencies the system
>> > > temporarily bumps over to the "switcher" PLL running at 800MHz while
>> > > waiting for the main PLL to stabilize.  No CPUFREQ notification is
>> > > sent for that.  That means there's a period of time when we're running
>> > > at 800MHz but loops_per_jiffy is calibrated at between 200MHz and
>> > > 300MHz.
>> > >
>> > >
>> > > I'm welcome to any suggestions for how to address this.  It sorta
>> > > feels like it would be a common thing to have a temporary PLL during
>> > > the transition, ...
>> >
>> > We definitely do that on Tegra for some cpufreq transitions.
>>
>> Ouch...  If this is a common strategy to use a third frequency during a
>> transition phase, especially if that frequency is way off (800MHz vs
>> 200-300MHz) then it is something the cpufreq layer must capture and
>> advertise.
>
> Of course if only the loops_per_jiffy scaling does care about frequency
> changes these days, and if in those cases udelay() can instead be moved
> to a timer source on those hick-up prone platforms, then all this is
> fairly theoretical and may not be worth pursuing.

Yup, I think I'm going to abandon this.  If anyone ever runs into this
problem later and can't use a bloody timer, hopefully they'll stumble
on this thread and at least end up with a good starting point.

For the record, the fix I made locally (which actually worked) was:

1. Add a new "temp" field to cpufreq_freqs:

--- a/include/linux/cpufreq.h
+++ b/include/linux/cpufreq.h
@@ -133,6 +133,7 @@ struct cpufreq_freqs {
        unsigned int cpu;       /* cpu nr */
        unsigned int old;
        unsigned int new;
+       unsigned int temp;
        u8 flags;               /* flags of cpufreq_driver, see below. */
 };

...depending on whether all users of this structure zero-initialize it
(I didn't check) you might also need to add a flag to say whether the
temp field is actually valid.


2. Set the "temp" to the switching PLL frequency in the cpufreq driver.


3. Change the ARM cpufreq_callback() to take the temp freq into account:

+       if (val == CPUFREQ_PRECHANGE) {
+               old_freq = freq->old;
+               new_freq = max(freq->new, freq->temp);
+       } else if (val == CPUFREQ_POSTCHANGE) {
+               old_freq = max(freq->old, freq->temp);
+               new_freq = freq->new;
+       }

...then use old_freq and new_freq in the place of freq->old and freq->new



-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/