Re: [PATCH v2 0/2] kvm: x86: Emulate MSR_PLATFORM_INFO

[Gleb Natapov <gleb@redhat.com>]

On Wed, Jun 19, 2013 at 01:50:45PM -0400, Bandan Das wrote:
> Gleb Natapov <gleb@redhat.com> writes:
> 
> > On Tue, Jun 18, 2013 at 11:29:27AM -0400, Bandan Das wrote:
> >> Gleb Natapov <gleb@redhat.com> writes:
> >> 
> >> > On Tue, Jun 18, 2013 at 04:05:08PM +0200, Paolo Bonzini wrote:
> >> >> Il 05/06/2013 10:42, Gleb Natapov ha scritto:
> >> >> >> > These patches add an emulated MSR_PLATFORM_INFO that kvm guests
> >> >> >> > can read as described in section 14.3.2.4 of the Intel SDM. 
> >> >> >> > The relevant changes and details are in [2/2]; [1/2] makes vendor_intel
> >> >> >> > generic. There are atleat two known applications that fail to run because
> >> >> >> > of this MSR missing - Sandra and vTune.
> >> >> > So I really want Intel opinion on this. Right now it is impossible to
> >> >> > implement the MSR correctly in the face of migration (may be with tsc
> >> >> > scaling it will be possible) and while it is unimplemented if application
> >> >> > tries to use it it fails, but if we will implement it application will
> >> >> > just produce incorrect result without any means for user to detect it.
> >> >> 
> >> >> Jun, ping?  (Perhaps Gleb you want to ask a more specific question though).
> >> >> 
> >> >> I don't think this is much different from any other RDTSC usage in
> >> >> applications (they will typically do their calibration manually, and do
> >> >> it just once).  I'm applying it to queue.
> >> >> 
> >> > And we do not support application that uses RDTSC directly! If we could
> >> > catch those it would be good from support point of view, so the way
> >> > MSR_PLATFORM_INFO behaves now it better then proposed alternative.
> >> 
> >> If support is the issue, can't we have a flag that disables this by
> >> default and users who want to take the plunge (and be responsible
> >> for the consequences) can enable it to read platform_info ?
> >> 
> > We have it already :) ignore_msrs. If it is set unimplemented MSRs will
> > not inject #GP, but return zero value instead. Zero it as incorrect as
> > anything else in the case of migration.
> 
> But does the intended purpose of ignore_msrs fit here ?  Even if we return
> 0 after a migration, there's no guarantee that the application will
> give correct results based on the old value of the MSR it read.
> 
I am not sure I understand. I am saying that since we cannot have
correct implementation of the MSR, any implementation that does not
crash a guest is as good as any other.

> I agree with you on the potential problems but I think we are completely
> ignoring the "non-migration" use case. These users will probably benefit 
> from a correct value of (virtual) msr_platform_info. And it appears, the 
> easiest way to give both options to the user is using a new module_param, 
> say ignore_platform_info.
> 
Migration is important part of virtualization. PMU emulation currently
is in demo status because migration is not implemented yet, but at least
it is possible to implement it.

> Scenarios :
> 1. ignore_platform_info = 0 (Default). Inject #GP if application tries to
> read this MSR.
> 2. ignore_platform_info = 1. User wants to read the calculated value, her
> environment doesn't require migration.
> 3. ignore_msrs = 1. If this is set, we always return 0 and application will
> hopefully resort to a workaround.
> 
Module flag is global for all VMs on a host. Implementing it like this
will guaranty that the feature will not be used in production ever.
ignore_msrs exists only for developers to quickly check if a problem goes
away if some MSR does not #GP, never as a real solution.

To make it somewhat useful the flag should be per-vm and exposed to all
layers up to a management. Management is the one who enables it per VM
basis and guaranties that VM with the feature enabled is never live
migrated. Frankly IMO it will be another management knob that will never
be set.

--
			Gleb.