linux-kernel - Re: [PATCH] cpufreq: CPPC: use 10ms delay instead of 2us to avoid high error

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: <41ae3b9e-c774-5061-b045-d5ec2a658880@arm.com>
Date:   Wed, 19 Apr 2023 18:52:47 +0200
From:   Pierre Gondois <pierre.gondois@....com>
To:     Yang Shi <yang@...amperecomputing.com>
Cc:     viresh.kumar@...aro.org, scott@...amperecomputing.com,
        linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        "Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH] cpufreq: CPPC: use 10ms delay instead of 2us to avoid
 high error

> 
> Just 2 other comments:
> a-
> It might be interesting to change the order in which cpc registers are read
> just to see if it has an impact, but even if it has, I m not sure how this
> could be exploitable.
> Just in case, I mean doing that, but I think that b. might be better to try.
> 
> diff --git a/drivers/acpi/cppc_acpi.c b/drivers/acpi/cppc_acpi.c
> index c51d3ccb4cca..479b55006020 100644
> --- a/drivers/acpi/cppc_acpi.c
> +++ b/drivers/acpi/cppc_acpi.c
> @@ -1350,8 +1350,8 @@ int cppc_get_perf_ctrs(int cpunum, struct cppc_perf_fb_ctrs *perf_fb_ctrs)
>                   }
>           }
>    
> -       cpc_read(cpunum, delivered_reg, &delivered);
>           cpc_read(cpunum, reference_reg, &reference);
> +       cpc_read(cpunum, delivered_reg, &delivered);
>           cpc_read(cpunum, ref_perf_reg, &ref_perf);
>    
>           /*
> 
> b-
> In the trace that you shared, the cpc_read() calls in the fist
> cppc_get_perf_ctrs() calls seem to always take a bit more time than in the
> second cppc_get_perf_ctrs() call.
> Would it be possible to collect traces similar as above with 3 or 4 calls to
> cppc_get_perf_ctrs() instead of 2 ? It would allow to check whether in the first
> call, accessing the cpc registers takes more time than in the following calls,
> due to cache misses or other reasons.
> Ideally statistics on the result would be the best, or if you have a trace.dat
> to share containing a trace with multiple cppc_cpufreq_get_rate() calls.
> 
> Example of code where we do 4 calls to cppc_get_perf_ctrs():
> 
> diff --git a/drivers/cpufreq/cppc_cpufreq.c b/drivers/cpufreq/cppc_cpufreq.c
> index 022e3555407c..6370f2f0bdad 100644
> --- a/drivers/cpufreq/cppc_cpufreq.c
> +++ b/drivers/cpufreq/cppc_cpufreq.c
> @@ -853,6 +853,20 @@ static unsigned int cppc_cpufreq_get_rate(unsigned int cpu)
>    
>           udelay(2); /* 2usec delay between sampling */
>    
> +       ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
> +       if (ret)
> +               return ret;
> +
> +       udelay(2); /* 2usec delay between sampling */
> +
> +       /* Do a third call. */
> +       ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
> +       if (ret)
> +               return ret;
> +
> +       udelay(2); /* 2usec delay between sampling */
> +
> +       /* Do a fourth call. */
>           ret = cppc_get_perf_ctrs(cpu, &fb_ctrs_t1);
>           if (ret)
>                   return ret;

And also, if the cpc_read() calls in the third/fourth call are actually faster,
would it be possible to check whether the computed frequency is more accurate
(i.e. no over/undershoot) ?