From mboxrd@z Thu Jan 1 00:00:00 1970 From: Paolo Bonzini Subject: Re: Should KVM_GUEST stop depending on PARAVIRT? Date: Wed, 29 Jul 2015 11:24:09 +0200 Message-ID: <55B89BB9.1060102@redhat.com> References: <55B63933.4090702@redhat.com> <55B66F42.3000202@redhat.com> <55B678B5.8040602@redhat.com> <55B75225.8080505@redhat.com> <20150729091920.GK19282@twins.programming.kicks-ass.net> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: KVM list , Arjan van de Ven To: Peter Zijlstra , Andy Lutomirski Return-path: Received: from mx1.redhat.com ([209.132.183.28]:47056 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752650AbbG2JYP (ORCPT ); Wed, 29 Jul 2015 05:24:15 -0400 In-Reply-To: <20150729091920.GK19282@twins.programming.kicks-ass.net> Sender: kvm-owner@vger.kernel.org List-ID: On 29/07/2015 11:19, Peter Zijlstra wrote: > On Tue, Jul 28, 2015 at 05:23:22PM -0700, Andy Lutomirski wrote: >> PeterZ, can we fix this for real instead of relying on >> CONFIG_PARAVIRT=y accidentally turning all msr accesses into "safe" >> accesses? We have the CPUID "hypervisor" bit, but I still don't fully >> understand the problem. > > So a whole bunch of PMU drivers already probe the MSRs at init time > using a combination of rdmsrl_safe() and wrmsrl_safe() to see if: > > 1) its there at all > 2) if its there, it 'works' like it ought to > > See for example: arch/x86/kernel/cpu/perf_event.c:check_hw_exists(). Great. Of course it's even better if you can probe it with CPUID (e.g. aperfmperf in intel_pstate.c), then there's no need for the _safe variant. > I have no problem using the _safe() methods to probe MSRs in init > routines and failing the init of the driver etc. > > What I do object to is sprinkling _safe() all over the driver itself and > creating horrid error paths all over (yes, people have proposed crazy > things like that). > > Another example is the LBR probing in > arch/x86/kernel/cpu/perf_event_intel.c:intel_pmu_init(). This is the one > people wanted to use _safe() crud for all throughout the driver, instead > of simply disabling the LBR at init time. I agree that check_msr is okay. > As to the 'bug' at hand, I really don't see the problem. The RAPL driver > says there's no valid RAPL domains, this is true, there aren't any on > the virtual machine, so what? Well, people have complained about it because it's KERN_ERR. Do you think it is okay to downgrade this (perhaps not even just on VMs) to info? Paolo