linux-kernel - Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20200831112638.v6vyljefggptij2v@vireshk-i7>
Date:   Mon, 31 Aug 2020 16:56:38 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     Ionela Voinescu <ionela.voinescu@....com>
Cc:     Vincent Guittot <vincent.guittot@...aro.org>,
        Ben Segall <bsegall@...gle.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Dietmar Eggemann <dietmar.eggemann@....com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ingo Molnar <mingo@...hat.com>,
        Juri Lelli <juri.lelli@...hat.com>,
        Mel Gorman <mgorman@...e.de>,
        Peter Zijlstra <peterz@...radead.org>,
        "Rafael J. Wysocki" <rafael@...nel.org>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sudeep Holla <sudeep.holla@....com>,
        Will Deacon <will@...nel.org>,
        Peter Puhov <peter.puhov@...aro.org>,
        LAK <linux-arm-kernel@...ts.infradead.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        "open list:THERMAL" <linux-pm@...r.kernel.org>
Subject: Re: [RFC 0/3] cpufreq: cppc: Add support for frequency invariance

On 27-08-20, 12:27, Ionela Voinescu wrote:
> I don't see it as anyone registering for freq invariance, rather the
> freq invariance framework chooses its source of information (AMU, CPPC,
> cpufreq).

Yeah, either way is fine for me.

> > i.e. if CPPC registers for it first then there is no need to check
> > AMUs further (as CPPC will be using AMUs anyway), else we will
>                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Not necessarily. Even if AMUs are present, they are only used for CPPC's
> delivered and reference performance counters if the ACPI _CPC entry
> specifies FFH as method:
> 
>   ResourceTemplate(){Register(FFixedHW, 0x40, 0, 1, 0x4)},
>   ResourceTemplate(){Register(FFixedHW, 0x40, 0, 0, 0x4)},

Right.

> While I understand your point (accessing AMUs through CPPC's read
> functions to implement invariance) I don't think it's worth tying the
> two together.
> 
> I see the two functionalities as independent:
>  - frequency invariance with whichever source of information is valid
>    (AMUs, cpufreq, etc) is separate from
>  - CPPC's delivered and reference performance counters, which currently
>    are used in cpufreq's .get() function.
> 
> Therefore, taking each of the scenarios one by one:
>  - All CPUs support AMUs: the freq invariance initialisation code will
>    find AMUs valid and it will use them to set the scale factor;
>    completely independently, if the FFH method is specified for CPPC's
>    delivered and reference performance counters, it will also use
>    AMUs, even if, let's say, invariance is disabled.
> 
>  - None of the CPUs support AMUs, but the _CPC entry specifies some
>    platform specific counters for delivered and reference performance.
>    With the current mainline code neither cpufreq or counter based
>    invariance is supported, but the CPPC counters can be used in the
>    cppc_cpufreq driver for the .get() function.
> 
>    But with the above new functionality we can detect that AMUs are not
>    supported and expose the CPPC counters to replace them in
>    implementing invariance.
> 
>  - Mixed scenarios are also supported if we play our cards right and
>    implement the above per-cpu.
> 
> 
> I'm thinking that having some well defined invariance sources might work
> well: it will simplify the init function (go through all registered
> sources and choose (per-cpu) the one that's valid) and allow for
> otherwise generic invariance support. Something like:
> 
> enum freq_inv_source {INV_CPUFREQ, INV_AMU_COUNTERS, INV_CPPC_COUNTERS};
> 
> struct freq_inv_source {
> 	enum freq_inv_source source;
> 	bool (*valid)(int cpu);
> 	u64 (*read_corecnt)(int cpu);
> 	u64 (*read_constcnt)(int cpu);
> 	u64 (*max_rate)(int cpu);
> 	u64 (*ref_rate)(int cpu);
> }
> 
> I am in the middle of unifying AMU counter and cpufreq invariance through
> something like this, so if you like the idea and you don't think I'm
> stepping too much on your toes with this, I can consider the usecase in
> my (what should be) generic support. So in the end this might end up
> being just a matter of adding a new invariance source (CPPC counters).

Okay, if you have already started working on that, no issues from my
side. I can just get the relevant stuff from CPPC added once you
provide that layer..

> My only worry is that while I know how a cpufreq source behaves and how
> AMU counters behave, I'm not entirely sure what to expect from CPPC

Neither do I :)

> counters: if they are always appropriate for updates on the tick (not
> blocking),

The update stuff may sleep here and so I had to do stuff in the
irq-work handler in my patch.

> if they both stop during idle, if there is save/restore
> functionality before/after idle, etc.

This I will check.

-- 
viresh