linux-kernel - Re: Re: [PATCH] [CPUFREQ] EXYNOS: bugfix on retrieving old

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

Message-id: <13404.111131334054311065.JavaMail.weblogic@epv6ml08>
Date:	Tue, 10 Apr 2012 10:38:33 +0000 (GMT)
From:	MyungJoo Ham <myungjoo.ham@...sung.com>
To:	Inderpal Singh <inderpal.singh@...aro.org>
Cc:	"cpufreq@...r.kernel.org" <cpufreq@...r.kernel.org>,
	Dave Jones <davej@...hat.com>,
	이재철 <jc.lee@...sung.com>,
	Tushar Behera <tushar.behera@...aro.org>,
	박경민 <kyungmin.park@...sung.com>,
	최찬우 <cw00.choi@...sung.com>,
	"myungjoo.ham@...il.com" <myungjoo.ham@...il.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Re: [PATCH] [CPUFREQ] EXYNOS: bugfix on retrieving old_index from
 freqs.old

Sender : Inderpal Singh<inderpal.singh@...aro.org> Date : 2012-04-10 19:05 (GMT+09:00)
> 
> Hi MyungJoo,

Hi Inderpal,

> 
> On 4 April 2012 15:53, MyungJoo Ham wrote:
> > The policy might have been changed since last call of target().
> > Thus, using cpufreq_frequency_table_target(), which depends on
> > policy to find the correspoding index from a frequency, may return
> > inconsistent index for freqs.old. Thus, old_index should be
> > calculated not based on the current policy.
> >
> > We have been observing such issue when scaling_min/max_freq were
> > updated and sometimes caused system lockups due to incorrectly
> > configured voltages.
> >
> > Signed-off-by: MyungJoo Ham 
> > ---
> >  drivers/cpufreq/exynos-cpufreq.c |   13 +++++++++++--
> >  1 files changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpufreq/exynos-cpufreq.c b/drivers/cpufreq/exynos-cpufreq.c
> > index b243a7e..1577522 100644
> > --- a/drivers/cpufreq/exynos-cpufreq.c
> > +++ b/drivers/cpufreq/exynos-cpufreq.c
> > @@ -62,8 +62,17 @@ static int exynos_target(struct cpufreq_policy *policy,
> >                goto out;
> >        }
> >
> > -       if (cpufreq_frequency_table_target(policy, freq_table,
> > -                                          freqs.old, relation, &old_index)) {
> > +       /*
> > +        * The policy may have been changed so that we cannot get proper
> > +        * old_index with cpufreq_frequency_table_target(). Thus, ignore
> > +        * policy and get the index from the raw frequency table.
> > +        */
> > +       for (old_index = 0;
> > +            freq_table[old_index].frequency != CPUFREQ_TABLE_END;
> > +            old_index++)
> > +               if (freq_table[old_index].frequency == freqs.old)
> > +                       break;
> > +       if (freq_table[old_index].frequency == CPUFREQ_TABLE_END) {
> >                ret = -EINVAL;
> >                goto out;
> >        }
> 
> I also had the same issue and same fix while testing powertop 1.98 as
> it changes the scaling_min/max_freq.
> 
> The only concern I have is when this code gets called for the very
> first time, the freqs.old will be the freq set by bootloader. Now if
> bootloader sets a freq which is not in the freq_table (not sure if its
> practical but theoretically its possible ), this code will error out.

It is not practical (the vendor ships IPL), but theoretically, it's possible
if one hacks the IPL.

However, in such a theoretical scenario, cpufreq won't work anyway even if
we accept out-of-specification freqs.old.

In the case you've mentioned, the bootloader has configured the clock dividers
and sources incorrectly (out of the chip vendor's specification). Then,
the Exynos cpufreq is not compatible with such a bootloader anyway even if
out-of-spec freqs.old is allowed. It is because Exynos cpufreq driver assumes
that the dividers and sources are correctly configured previously; i.e., it
does not reset/set every related divider or source when the frequency has been
updated. Thus, the resulting frequency may be different from the intended
frequency and often the system will suffer from lock ups. We've been observing
lock ups when the bootloader configures some dividers different from what the
kernel expects during suspend-to-ram and wakeup.

Cheers!
MyungJoo.

> 
> 
> > --
> > 1.7.4.1
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe cpufreq" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Thanks,
> Inder
> 
> 
> 
> 

--

MyungJoo Ham (함명주), PHD

System S/W Lab, S/W Platform Team, Software Center
Samsung Electronics
Cell: +82-10-6714-2858