linux-kernel - Re: [PATCH] PM / OPP: Update voltage in case freq == 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>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 18 Jun 2018 16:21:32 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Waldemar Rymarkiewicz <waldemar.rymarkiewicz@...il.com>
Cc:     Viresh Kumar <vireshk@...nel.org>, Nishanth Menon <nm@...com>,
        Stephen Boyd <sboyd@...nel.org>, linux-pm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PM / OPP: Update voltage in case freq == old_freq

On 14-06-18, 15:56, Waldemar Rymarkiewicz wrote:
> This commit fixes a rare but possible case when the clk rate is updated
> without update of the regulator voltage.
> 
> At boot up, CPUfreq checks if the system is running at the right freq. This
> is a sanity check in case a bootloader set clk rate that is outside of freq
> table present with cpufreq core. In such cases system can be unstable so
> better to change it to a freq that is preset in freq-table.
> 
> The CPUfreq takes next freq that is >= policy->cur and this is our
> target_freq that needs to be set now.
> 
> dev_pm_opp_set_rate(dev, target_freq) checks the target_freq and the
> old_freq (a current rate). If these are equal it returns early. If not,
> it searches for OPP (old_opp) that fits best to old_freq (not listed in
> the table) and updates old_freq (!).
> 
> Here, we can end up with old_freq = old_opp.rate = target_freq, which
> is not handled in _generic_set_opp_regulator(). It's supposed to update
> voltage only when freq > old_freq  || freq > old_freq.
> 
> if (freq > old_freq) {
> 		ret = _set_opp_voltage(dev, reg, new_supply);
> [...]
> if (freq < old_freq) {
> 		ret = _set_opp_voltage(dev, reg, new_supply);
> 		if (ret)
> 
> It results in, no voltage update while clk rate is updated.
> 
> Example:
> freq-table = {
> 	1000MHz   1.15V
> 	 666MHZ   1.10V
> 	 333MHz   1.05V
> }
> boot-up-freq        = 800MHz   # not listed in freq-table
> freq = target_freq  = 1GHz
> old_freq            = 800Mhz
> old_opp = _find_freq_ceil(opp_table, &old_freq);  #(old_freq is modified!)
> old_freq            = 1GHz
> 
> Signed-off-by: Waldemar Rymarkiewicz <waldemar.rymarkiewicz@...il.com>
> ---
>  drivers/opp/core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index ab2f3fe..31ff03d 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -598,7 +598,7 @@ static int _generic_set_opp_regulator(const struct opp_table *opp_table,
>  	}
>  
>  	/* Scaling up? Scale voltage before frequency */
> -	if (freq > old_freq) {
> +	if (freq >= old_freq) {
>  		ret = _set_opp_voltage(dev, reg, new_supply);
>  		if (ret)
>  			goto restore_voltage;

Applied with:

Fixes: 6a0712f6f199 ("PM / OPP: Add dev_pm_opp_set_rate()")
Cc: 4.6+ <stable@...r.kernel.org> # v4.6+

Thanks.

-- 
viresh