Re: Should KVM_GUEST stop depending on PARAVIRT?

[Paolo Bonzini <pbonzini@redhat.com>]

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