linux-kernel - Re: [PATCH V5 2/2] base/drivers/arch_topology: Default dmips-mhz if they are not set in DT

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: <2b050e93-f734-f2f3-ac67-a30d37bf1950@arm.com>
Date:   Mon, 3 Dec 2018 14:46:09 +0100
From:   Dietmar Eggemann <dietmar.eggemann@....com>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>, rjw@...ysocki.net
Cc:     linux-kernel@...r.kernel.org, viresh.kumar@...aro.org,
        Chris Redpath <chris.redpath@...aro.org>,
        Quentin Perret <quentin.perret@...aro.org>,
        Amit Kucheria <amit.kucheria@...aro.org>,
        Nicolas Dechesne <nicolas.dechesne@...aro.org>,
        Niklas Cassel <niklas.cassel@...aro.org>,
        Rob Herring <robh@...nel.org>,
        Quentin Perret <quentin.perret@....com>,
        Rob Herring <robh+dt@...nel.org>,
        Mark Rutland <mark.rutland@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        Li Yang <leoyang.li@....com>, Olof Johansson <olof@...om.net>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Russell King <linux@...linux.org.uk>
Subject: Re: [PATCH V5 2/2] base/drivers/arch_topology: Default dmips-mhz if
 they are not set in DT

Hi Daniel,

+cc: Russell King <linux@...linux.org.uk>

On 11/27/18 2:24 PM, Daniel Lezcano wrote:
> In the case of asymmetric SoC with the same micro-architecture, we
> have a group of CPUs with smaller OPPs than the other group. One
> example is the 96boards dragonboard 820c. There is no dmips/MHz
> difference between both groups, so no need to specify the values in
> the DT. Unfortunately, without these defined, there is no scaling
> capacity computation triggered, so we need to write
> 'capacity-dmips-mhz' for each CPU with the same value in order to
> force the scaled capacity computation.
> 
> In order to fix this situation, allocate 'raw_capacity' so the pointer
> is set and the init_cpu_capacity_callback() function can be called.
> 
> This was tested on db820c:
>   - specified values in the DT (correct results)
>   - partial values defined in the DT (error + fallback to defaults)
>   - no specified values in the DT (correct results)
> 
> correct results are:
>    cat /sys/devices/system/cpu/cpu*/cpu_capacity
>     758
>     758
>    1024
>    1024
> 
>    ... respectively for CPU0, CPU1, CPU2 and CPU3.
> 
> That reflects the capacity for the max frequencies 1593600 and 2150400.

[...]

I'm afraid that this change is incompatible with the still existing 
cpu_efficiency interface we have in Arm32 for A15/A7 systems like Arm TC2:

In case you specify clock-frequency dt properties per cpu for such a 
system, the cpu_capacity values are determined via the cpu_efficiency 
code in arch/arm/kernel/topology.c.

So on Arm TC2 with clock-frequency = <1000000000> [A15] and <800000000> 
[A7] you get:

root@...aro-nano:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
606
1441
1441
606
606

With your patches on top (cpu_capacity functionality in 
drivers/base/arch_topology.c does not have to be switched on by 
specifying capacity-dmips-mhz dt properties anymore) we end up scaling 
by max frequency again:

root@...aro-nano:~# cat /sys/devices/system/cpu/cpu*/cpu_capacity
358
1024
1024
358
358

I tried to remove the cpu_efficiency based API a year ago but Russell 
pointed out that the compatibility has to be maintained for longer:

https://lore.kernel.org/lkml/20171024102718.16113-1-dietmar.eggemann@arm.com/

I assume that the capacity-dmips-mhz dt property is like a switch to 
turn this functionality on for big.Little and so called gold/silver 
platforms, which have cores with the same uArch but in frequency domains 
with different max frequency values.

So what's wrong with specifying capacity-dmips-mhz = <1024> for all 
cores for those gold/silver platforms? I don't expect that there will be 
so many of them. And normal SMP platforms (w/o frequency domains w/o 
different max frequency values) don't have to execute this code.

IMHO, at least we should remove the cpu_efficiency bits before we do 
this change.

[...]