linux-kernel - RE: [PATCH v2 0/7] CPPC optional registers AMD support

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: <SN6PR12MB26396618BAC0F6600D786E08F8250@SN6PR12MB2639.namprd12.prod.outlook.com>
Date:   Wed, 17 Apr 2019 17:28:32 +0000
From:   "Ghannam, Yazen" <Yazen.Ghannam@....com>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        "Natarajan, Janakarajan" <Janakarajan.Natarajan@....com>
CC:     "Natarajan, Janakarajan" <Janakarajan.Natarajan@....com>,
        "linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
        "devel@...ica.org" <devel@...ica.org>,
        "Rafael J . Wysocki" <rjw@...ysocki.net>,
        Len Brown <lenb@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Robert Moore <robert.moore@...el.com>,
        Erik Schmauss <erik.schmauss@...el.com>
Subject: RE: [PATCH v2 0/7] CPPC optional registers AMD support

> -----Original Message-----
> From: Rafael J. Wysocki <rafael@...nel.org>
> Sent: Tuesday, April 16, 2019 4:34 AM
> To: Natarajan, Janakarajan <Janakarajan.Natarajan@....com>
> Cc: Natarajan, Janakarajan <Janakarajan.Natarajan@....com>; linux-acpi@...r.kernel.org; linux-kernel@...r.kernel.org; linux-
> pm@...r.kernel.org; devel@...ica.org; Rafael J . Wysocki <rjw@...ysocki.net>; Len Brown <lenb@...nel.org>; Viresh Kumar
> <viresh.kumar@...aro.org>; Robert Moore <robert.moore@...el.com>; Erik Schmauss <erik.schmauss@...el.com>; Ghannam, Yazen
> <Yazen.Ghannam@....com>
> Subject: Re: [PATCH v2 0/7] CPPC optional registers AMD support
> 
> On Tue, Apr 16, 2019 at 12:35 AM Janakarajan Natarajan <jnataraj@....com> wrote:
> >
> > On 4/4/19 4:25 PM, 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 add support for a subset of these 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
> > >
> > > The CPPC driver is updated to enable the OSPM and the userspace to
> > > access
> > > the newly supported registers.
> > >
> > > The purpose of exposing the registers via the sysfs entries is to allow
> > > the userspace to:
> > > * Tweak the values to fit its workload.
> > > * Apply a profile from AMD's optimization guides.
> > >
> > > Profiles will be documented in the performance/optimization guides.
> > >
> > > Note:
> > > * AMD systems will not have a policy applied in the kernel at this time.
> > > * By default, acpi_cpufreq will still be used.
> > >
> > > TODO:
> > > * Create a linux userspace tool that will help users generate a CPPC
> > > * profile
> > >    for their target workload.
> > > * Create or update a driver to apply a general CPPC policy in the
> > > * kernel.
> > >
> > > v1->v2:
> > > * Add macro to ensure BUFFER only registers have BUFFER type.
> > > * Add support macro to make the right check based on register type.
> > > * Remove support checks for registers which are mandatory.
> >
> >
> > Are there any concerns regarding this patchset?
> 
> Yes, there are.
> 
> Unfortunately, it is generally problematic.
> 
> First off, the behavior of existing sysfs files cannot be changed at
> will, as there may be users of them out there already depending on the
> current behavior.
> 

The intent is to add new sysfs files without changing the existing files. Is that okay?

Or is the addition of new files also not acceptable?

> Second, at least some CPPC control registers are used by cpufreq
> drivers (cppc_cpufreq and intel_pstate), so modifying them behind the
> drivers' backs is not a good idea in general.  For this reason, adding
> new sysfs attributes to allow user space to do that is quite
> questionable.
> 

Yes, good point.

What if a check is added so that writes only succeed if the CPUFREQ governor is set to userspace?

Thanks,
Yazen