Messages in this thread | | | Subject | Re: [PATCH] x86/rapl: Do not load in a guest | From | Paolo Bonzini <> | Date | Fri, 4 Dec 2015 11:41:03 +0100 |
| |
On 04/12/2015 11:19, Borislav Petkov wrote: > + Paolo. > > On Fri, Dec 04, 2015 at 09:28:23AM +0100, Ingo Molnar wrote: >>>> So when a hypervisor starts supporting RAPL we'll disable the driver erroneously? >>>> >>>> Isn't there any better method to detect RAPL support? >>>> >>>> So in particular in drivers/powercap/intel_rapl.c there's an enumerated list of >>>> CPU models, which is used via a x86_match_cpu() call. That's still not ideal (it >>>> does not work on hypervisors for example), but even better would be to detect RAPL >>>> support in some other fashion, that does not rely on us statically enumerating CPU >>>> models that support it. >>> >>> RAPL isn't enumerated, the best we could do is attempt to write to one >>> of the writable MSRs and see if that 'works'. >> >> Hm, bad - writing to MSRs like that is generally dangerous. >> >> So we should at least provide a central 'is RAPL available' call instead of >> spreading multiple X86_FEATURE_HYPERVISOR checks. > > Well, looks like someone dropped the ball at the CPUID registrar.
Yup, this is an issue with RAPL.
> And since there's no CPUID bit, I don't see any other way to detect the > RAPL presence. Poking at MSRs is a bad idea. > > I wonder if we could go and allocate a bit in the kvm-emulated CPUID > leafs which says whether RAPL is supported or not.
No, please don't. Why do you need a wrmsr instead of a rdmsr? If there's no RAPL domains, the device doesn't load. On hypervisors, reading random MSRs is generally safe.
Paolo
> Then we can go and check for that leaf on baremetal - if it is not > there, we do the vendor + fms check and if it is there, we know we're in > a guest and whether the guest supports it or not. > > Dunno. > > On the one hand, it looks like a bit too much to me. > > On the other, it could be useful for other future feature checks where > we want baremetal and kvm to be synchronized wrt features and a single > method to be used by the kernel for checking features presence works > both on baremetal and virt. > > Just a thought, anyway... > > hpa, thoughts? >
| |