Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
From:   "Ghannam, Yazen" <Yazen.Ghannam@amd.com>
To:     Peter Zijlstra <peterz@infradead.org>,
        "Natarajan, Janakarajan" <Janakarajan.Natarajan@amd.com>
CC:     "linux-acpi@vger.kernel.org" <linux-acpi@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-pm@vger.kernel.org" <linux-pm@vger.kernel.org>,
        "devel@acpica.org" <devel@acpica.org>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        Len Brown <lenb@kernel.org>,
        Viresh Kumar <viresh.kumar@linaro.org>,
        Robert Moore <robert.moore@intel.com>,
        Erik Schmauss <erik.schmauss@intel.com>
Subject: RE: [PATCHv3 0/6] CPPC optional registers AMD support
Thread-Topic: [PATCHv3 0/6] CPPC optional registers AMD support
Thread-Index: AQHVN057bBT6zB/KfUWS9uFb7V9o2qbIYcuAgAOQCkA=
Date:   Mon, 15 Jul 2019 17:57:16 +0000
Message-ID: <SN6PR12MB263964D82CED789D32AF58F1F8CF0@SN6PR12MB2639.namprd12.prod.outlook.com>
References: <cover.1562781484.git.Janakarajan.Natarajan@amd.com>
 <20190713104619.GA3496@hirez.programming.kicks-ass.net>
In-Reply-To: <20190713104619.GA3496@hirez.programming.kicks-ass.net>
Accept-Language: en-US
Content-Language: en-US
received-spf: None (protection.outlook.com: amd.com does not designate
 permitted sender hosts)
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 7ae91538-868a-4271-bb19-08d7094ddfd9
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Jul 2019 17:57:16.7745
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: yghannam@amd.com
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN6PR12MB2782
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> -----Original Message-----
> From: Peter Zijlstra <peterz@infradead.org>
> Sent: Saturday, July 13, 2019 5:46 AM
> To: Natarajan, Janakarajan <Janakarajan.Natarajan@amd.com>
> Cc: linux-acpi@vger.kernel.org; linux-kernel@vger.kernel.org; linux-pm@vg=
er.kernel.org; devel@acpica.org; Rafael J . Wysocki
> <rjw@rjwysocki.net>; Len Brown <lenb@kernel.org>; Viresh Kumar <viresh.ku=
mar@linaro.org>; Robert Moore
> <robert.moore@intel.com>; Erik Schmauss <erik.schmauss@intel.com>; Ghanna=
m, Yazen <Yazen.Ghannam@amd.com>
> Subject: Re: [PATCHv3 0/6] CPPC optional registers AMD support
>=20
> On Wed, Jul 10, 2019 at 06:37:09PM +0000, Natarajan, Janakarajan wrote:
> > CPPC (Collaborative Processor Performance Control) offers optional
> > registers which can be used to tune the system based on energy and/or
> > performance requirements.
> >
> > Newer AMD processors (>=3D Family 17h) add support for a subset of thes=
e
> > optional CPPC registers, based on ACPI v6.1.
> >
> > The following are the supported CPPC registers for which sysfs entries
> > are created:
> > * enable                (NEW)
> > * max_perf              (NEW)
> > * min_perf              (NEW)
> > * energy_perf
> > * lowest_perf
> > * nominal_perf
> > * desired_perf          (NEW)
> > * feedback_ctrs
> > * auto_sel_enable       (NEW)
> > * lowest_nonlinear_perf
> >
> > First, update cppc_acpi to create sysfs entries only when the optional
> > registers are known to be supported.
> >
> > Next, a new CPUFreq driver is introduced to enable the OSPM and the use=
rspace
> > to access the newly supported registers through sysfs entries found in
> > /sys/devices/system/cpu/cpu<num>/amd_cpufreq/.
> >
> > This new CPUFreq driver can only be used by providing a module paramete=
r,
> > amd_cpufreq.cppc_enable=3D1.
> >
> > The purpose of exposing the registers via the amd-cpufreq sysfs entries=
 is to
> > allow the userspace to:
> > * Tweak the values to fit its workload.
> > * Apply a profile from AMD's optimization guides.
>=20
> So in general I think it is a huge mistake to expose all that to
> userspace. Before you know it, there's tools that actually rely on it,
> and then inhibit the kernel from doing anything sane with it.
>=20

Okay, makes sense.

Is there any way to expose a sysfs interface and make it explicitly "experi=
mental"? Maybe putting it in Documentation/ABI/testing/?

Or do you think it's just not worth it?

> > Profiles will be documented in the performance/optimization guides.
>=20
> I don't think userspace can really do anything sane with this; it lacks
> much if not all useful information.
>=20
> > Note:
> > * AMD systems will not have a policy applied in the kernel at this time=
.
>=20
> And why the heck not? We're trying to move all cpufreq into the
> scheduler and have only a single governor, namely schedutil -- yes,
> we're still stuck with legacy, and we're still working on performance
> parity in some cases, but I really hope to get rid of all other cpufreq
> governors eventually.
>=20

Because this is new to AMD systems, we didn't want to enforce a default pol=
icy.

We figured that exposing the CPPC interface would be a good way to decouple=
 policy from the kernel and let users experiment/tune their systems, like u=
sing the userspace governor. And if some pattern emerged then we could make=
 that a default policy in the kernel (for AMD or in general).

But you're saying we should focus more on working with the schedutil govern=
or, correct? Do you think there's still a use for a userspace governor?

> And if you look at schedutil (schedutil_cpu_util in specific) then
> you'll see it is already prepared for CPPC and currently only held back
> by the generic cpufreq interface.
>=20
> It currently only sets desired freq, it has information for
> min/guaranteed, and once we get thermal intergrated we might have
> sensible data for max freq too.
>=20

Will do.

> > TODO:
> > * Create a linux userspace tool that will help users generate a CPPC pr=
ofile
> >   for their target workload.
>=20
> Basically a big fat NAK for this approach to cpufreq.
>=20

Is that for exposing the sysfs interface, having a stub driver, or both?

Would it be better to have a cpufreq driver that implements some policy rat=
her than just providing the sysfs interface?

> > * Create a general CPPC policy in the kernel.
>=20
> We already have that, sorta.

Right, but it seems to still be focused on CPU frequency rather than abstra=
ct performance like how CPPC is defined.

This is another reason for exposing the CPPC interface directly. We'll give=
 users the ability to interact with the platform, using CPPC, without havin=
g to follow the CPUFREQ paradigm.

Do you think this is doable? Or should we always have some kernel interacti=
on because of the scheduler, etc.?

Thanks,
Yazen