linux-kernel - Re: [PATCH v1] arch_topology: Adjust initial CPU capacities with current freq

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:   Fri, 10 Jan 2020 11:54:48 +0000
From:   Robin Murphy <robin.murphy@....com>
To:     Sudeep Holla <sudeep.holla@....com>,
        Jeffy Chen <jeffy.chen@...k-chips.com>
Cc:     linux-kernel@...r.kernel.org,
        Brian Norris <briannorris@...omium.org>,
        Marc Zyngier <marc.zyngier@....com>,
        Douglas Anderson <dianders@...omium.org>,
        Heiko Stuebner <heiko@...ech.de>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>
Subject: Re: [PATCH v1] arch_topology: Adjust initial CPU capacities with
 current freq

On 2020-01-10 11:37 am, Sudeep Holla wrote:
> On Thu, Jan 09, 2020 at 03:52:14PM +0800, Jeffy Chen wrote:
>> The CPU freqs are not supposed to change before cpufreq policies
>> properly registered, meaning that they should be used to calculate the
>> initial CPU capacities.
>>
>> Doing this helps choosing the best CPU during early boot, especially
>> for the initramfs decompressing.
>>
>> Signed-off-by: Jeffy Chen <jeffy.chen@...k-chips.com>
> 
> [...]
> 
>> @@ -146,10 +153,15 @@ bool __init topology_parse_cpu_capacity(struct device_node *cpu_node, int cpu)
>>   				return false;
>>   			}
>>   		}
>> -		capacity_scale = max(cpu_capacity, capacity_scale);
>>   		raw_capacity[cpu] = cpu_capacity;
>>   		pr_debug("cpu_capacity: %pOF cpu_capacity=%u (raw)\n",
>>   			cpu_node, raw_capacity[cpu]);
>> +
>> +		cpu_clk = of_clk_get(cpu_node, 0);
>> +		if (!PTR_ERR_OR_ZERO(cpu_clk))
>> +			per_cpu(max_freq, cpu) = clk_get_rate(cpu_clk) / 1000;
>> +
>> +		clk_put(cpu_clk);
> 
> I don't like to assume DVFS to be supplied only using 'clk'. So NACK!
> We have other non-clk mechanism for CPU DVFS and this needs to simply
> use cpufreq APIs to get frequency value if required.

...but in this case, as soon as cpufreq is ready the problem is gone 
anyway, because it sees the big cluster's clock rate is way out-of-spec 
and bumps it up to a sane OPP.

It really is unfortunate that so many RK3399 images out there are using 
the broken firmware combination that manages to miss out the boot-time 
PLL setup altogether.

Robin.