Re: [Qemu-devel] [PATCHv4 3/4] cpuid: disable pv eoi for 1.1 and older compat types

[Michael S. Tsirkin]

On Wed, Aug 29, 2012 at 06:59:03AM -0300, Marcelo Tosatti wrote:
> On Wed, Aug 29, 2012 at 01:21:13AM +0300, Michael S. Tsirkin wrote:
> > On Tue, Aug 28, 2012 at 07:02:42PM -0300, Eduardo Habkost wrote:
> > > On Wed, Aug 29, 2012 at 12:35:28AM +0300, Michael S. Tsirkin wrote:
> > > > On Tue, Aug 28, 2012 at 04:13:38PM -0300, Eduardo Habkost wrote:
> > > > > On Tue, Aug 28, 2012 at 08:43:52PM +0300, Michael S. Tsirkin wrote:
> > > > > > In preparation for adding PV EOI support, disable PV EOI by default 
> > > > > > for
> > > > > > 1.1 and older machine types, to avoid CPUID changing during 
> > > > > > migration.
> > > > > > 
> > > > > > PV EOI can still be enabled/disabled by specifying it explicitly.
> > > > > >     Enable for 1.1
> > > > > >     -M pc-1.1 -cpu kvm64,+kvm_pv_eoi
> > > > > >     Disable for 1.2
> > > > > >     -M pc-1.2 -cpu kvm64,-kvm_pv_eoi
> > > > > > 
> > > > > 
> > > > > What about users that are already running "qemu-1.1 -M pc-1.1" on a 
> > > > > host
> > > > > kernel that supports PV EOI already? They would get PV EOI disabled 
> > > > > when
> > > > > migrating to a destination running "qemu-1.2 -M pc-1.1".
> > > > > 
> > > > > (On the other hand, people running "qemu-1.1 -M pc-1.1" on a host 
> > > > > kernel
> > > > > supporting PV EOI already have migration broken, so there's not much 
> > > > > we
> > > > > can do for them)
> > > > 
> > > > Exactly.
> > > > 
> > > > Talked to Gleb, long term I think we should rework code to make
> > > > it forward-compatible wrt adding new MSRs:
> > > > - source gets list of MSRs to be migrated from KVM and simply sends 
> > > > them all
> > > > - send all MSRS in key/value format
> > > > - destination gets list of MSRs to be migrated from KVM and
> > > >   only restores the supported ones
> > > 
> > > As far as I understand the migration code requirements/expectations, if
> > > the origin is sending some data, it is because it is part of the
> > > guest-visible machine state that must be kept while migrating. Because
> > > of that, the destination is not allowed to drop anything it doesn't know
> > > about.
> > 
> > 
> > We have a ton of code that reads in values then just
> > ignores them, for compat with old qemu.
> > This will be exactly such a case:
> > we don't drop anything - protocol does not
> > support this. We read and simply do not tell kvm
> > about it. We also have tons of code that sends
> > useless values again for compatibility.
> > 
> > > At the same time, if it's not part of guest-visible machine
> > > state, it doesn't have to be sent by the migration origin.
> > > 
> > 
> > False, we often send internal device state which is not
> > directly guest visible.
> > 
> > > On the other hand, a mode of operation that doesn't require updating
> > > QEMU every time there's a new bit of guest-visible state to be migrated
> > > would be nice (just like the "-cpu host" mode, that doesn't require
> > > updating QEMU for every new CPU feature, is nice for some use cases). I
> > > just don't know how to make work with the current migration protocol.
> > > 
> > 
> > I don't understand. What is the problem with the proposal?
> > What will not work with our protocol?
> > Can you give an example please?
> > 
> > 
> > > > Too late for 1.2?
> > > 
> > > Absolutely (in my opinion).
> 
> My concern here is that you are introducing infrastructure by using
> _one example_, claiming it to solve a general problem, without the
> appropriate amount of energy spent in this solution.

What are you talking about? This new idea of mine?
Do you just assume this? Why? I can give lots of
past examples where it would have worked and
gave us forward compatibility.
For example, APF.
In another example, kvm clock.


> Using old machines is supposed to be supported on older guests.

Sorry, what does this mean exactly?

> Michael, not migrating the PVEOI MSR actually crashes the guest?


Not at all. migration currently simply disables PV EOI, so
each EOI triggers exits after migration.

-- 
MST