linux-kernel - Re: [PATCH] selftests: cpufreq: Check cpuinfo_cur

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:   Thu, 13 Jul 2017 14:25:37 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Leonard Crestez <leonard.crestez@....com>
Cc:     Shuah Khan <shuah@...nel.org>, linux-kselftest@...r.kernel.org,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Octavian Purdila <octavian.purdila@....com>,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] selftests: cpufreq: Check cpuinfo_cur_freq set as
 expected

On 12-07-17, 14:29, Leonard Crestez wrote:
> This checks that the cpufreq driver actually sets the requested
> frequency.
> 
> Signed-off-by: Leonard Crestez <leonard.crestez@....com>
> 
> ---
> 
> I've been looking at using kselftests for imx. This patch exposes an
> issue with the imx6 cpufreq driver on imx6sx where frequencies are set
> incorrectly because of clk mishandling. This is already caught by some
> internal test scripts which also run against upstream but it's nice to
> make this visible through kselftest.

Sure, thanks for that.

> I'm not sure it's correct to check that frequency matches exactly,
> perhaps something like a 5% tolerance should be included for complex
> drivers where the target freq is only a "hint"?

We can do better, see below..

> I checked intel_pstate
> but it doesn't even seem to expose an userspace governor for manual
> frequency selection anyway.

Sure, and so that wouldn't be affected by this.

> Unfortunately cpufreq selftests don't seem to have a clear idea of
> "pass" or "fail" results.

Yeah, I had this test setup for a while and just pushed it through.
Over that many tests aren't really tests but just looking out for
crashes, etc. Never got a chance to improve it :(

> This patch will just print some TAP-like
> "ok" and "not ok" lines but failures are not actually propagated upwards
> in a well-defined way.

That would be fine for now.

> Have you considered what it would take to TAP-ify the output of cpufreq
> tests? Output is very complex so perhaps it might make sense to adopt some
> sort of subtest syntax for kselftest, something like this:

Not yet :(

> diff --git a/tools/testing/selftests/cpufreq/cpufreq.sh b/tools/testing/selftests/cpufreq/cpufreq.sh
> index 1ed3832..323b5bb 100755
> --- a/tools/testing/selftests/cpufreq/cpufreq.sh
> +++ b/tools/testing/selftests/cpufreq/cpufreq.sh
> @@ -151,6 +151,14 @@ test_all_frequencies()
>  	# Set all frequencies one-by-one
>  	for freq in $freqs; do
>  		set_cpu_frequency $1 $freq
> +
> +		local cur_freq
> +		cur_freq=`cat $CPUFREQROOT/$1/cpuinfo_cur_freq`

Yes, we want to verify if freq change happened or not, but may be only
reading scaling_cur_freq would be enough for now?

And that wouldn't be a problem for X86 (which Rafael mentioned) as
well IIUC.

-- 
viresh