linux-kernel - Re: [PATCH] powercap/rapl: Do not load in virtualized environments

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0iUauCSR9PV8+uXsqowR9WUZh9S6HAU8jV2Yuf1pnJX=g@mail.gmail.com>
Date:	Thu, 19 May 2016 01:47:43 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	Prarit Bhargava <prarit@...hat.com>
Cc:	"Rafael J. Wysocki" <rafael@...nel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
	Jacob Jun Pan <jacob.jun.pan@...el.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>
Subject: Re: [PATCH] powercap/rapl: Do not load in virtualized environments

On Thu, May 19, 2016 at 1:36 AM, Prarit Bhargava <prarit@...hat.com> wrote:
>
>
> On 05/18/2016 07:06 PM, Rafael J. Wysocki wrote:
>> On Wed, May 18, 2016 at 2:38 PM, Prarit Bhargava <prarit@...hat.com> wrote:
>>>
>>>
>>> On 05/17/2016 08:50 PM, Rafael J. Wysocki wrote:
>>>> On Tue, May 17, 2016 at 1:34 PM, Prarit Bhargava <prarit@...hat.com> wrote:
>>>>> intel_rapl is currently not supported in virtualized environments.  When
>>>>> booting the warning message
>>>>>
>>>>> intel_rapl: no valid rapl domains found in package 0
>>>>
>>>> You seem to be saying that this message is problematic for some
>>>> reason, so why is it?
>>>>
>>>
>>> I thought about my previous answer and after thinking about it realized I didn't
>>> give you enough background Rafael.  Virtual environments won't use this feature
>>> as this is meant for restricting power consumption at the HW level.
>>>
>>> So ... here's the situation.  Most CPU features from Intel have a CPU feature
>>> bit (also known in some circles as cpuflags) set for them.  For example MCE has
>>> an mce bit that is exposed in /proc/cpuinfo.  Unfortunately, for Intel RAPL
>>> there is no bit (I don't know if someone dropped the ball or if Intel
>>> intentionally left this feature off ... I've heard both explanations :)).
>>>
>>> In any case the Intel RAPL driver is one of the few cpu based drivers in the
>>> kernel that still does a x86_match_cpu() against supported CPUs.  This means for
>>> virtual cpus which export the host cpu's cpu model number, the intel_rapl driver
>>> will attempt to load for each cpu.
>>>
>>> As a result the message
>>>
>>> intel_rapl: no valid rapl domains found in package 0
>>>
>>> is output as a *visible* error to the user for each virtual core.
>>>
>>> The error is valid for native cpus (although over 100s of systems I can say I've
>>> never seen the warning output on a native cpu) but it is clearly not valid for
>>> virtual cpus *because virtualized systems don't use this feature*.
>>>
>>> The driver shouldn't load on virt systems.  That's the bottom line here, and the
>>> patch prevents that from happening.  Would I prefer that there were some other
>>> mechanism to detect RAPL?  Yep.  I really really would.  But beyond mucking with
>>> MSRs (which is definitely more complicated and awful than this simple check) I
>>> don't see any easier method than the one I've proposed.
>>>
>>> I really don't want to be the one who sets the precedent of abusing x86_hyper in
>>> this way.  I know it isn't the "right" thing to do -- but I honestly do not see
>>> a better or cleaner way out of this.
>>
>> One quite obvious alternative might be to reduce the log level of the
>> message in question, say to pr_debug.
>
> Yeah -- I thought about that too.  But that's really a band-aid, isn't it?  The
> code shouldn't execute on virt.

"It is not necessary to run that code on virt" would be a better way
to put it IMO.

That is a valid point, but then using x86_hyper to avoid that feels
beyond ugly to be honest.