Re: [PATCH v2 1/2] x86:kvm:hyperv: add KVM_CAP_HYPERV_SYNIC2

From: Paolo Bonzini <pbonzini@redhat.com>
To: "Roman Kagan" <rkagan@virtuozzo.com>,
	"Radim Krčmář" <rkrcmar@redhat.com>,
	kvm@vger.kernel.org, "Evgeny Yakovlev" <eyakovlev@virtuozzo.com>,
	"Denis V . Lunev" <den@openvz.org>,
	"Eduardo Habkost" <ehabkost@redhat.com>,
	"Igor Mammedov" <imammedo@redhat.com>
Subject: Re: [PATCH v2 1/2] x86:kvm:hyperv: add KVM_CAP_HYPERV_SYNIC2
Date: Thu, 22 Jun 2017 14:06:57 +0200	[thread overview]
Message-ID: <bb6c8640-ec8b-db63-9687-50cf1453fb2d@redhat.com> (raw)
In-Reply-To: <20170622051411.GA15650@rkaganip.lan>

On 22/06/2017 07:14, Roman Kagan wrote:
> On Wed, Jun 21, 2017 at 10:09:15PM +0200, Radim Krčmář wrote:
>> 2017-06-16 20:37+0300, Roman Kagan:
>>> There is a flaw in the Hyper-V SynIC implementation in KVM: when message
>>> page or event flags page is enabled by setting the corresponding msr,
>>> KVM zeroes it out.  This is problematic because on migration the
>>> corresponding MSRs are loaded on the destination, so the content of
>>> those pages is lost.
>>>
>>> This went unnoticed so far because the only user of those pages was
>>> in-KVM hyperv synic timers, which could continue working despite that
>>> zeroing.
>>>
>>> Newer QEMU uses those pages for Hyper-V VMBus implementation, and
>>> zeroing them breaks the migration.
>>>
>>> Besides, in newer QEMU the content of those pages is fully managed by
>>> QEMU, so zeroing them is undesirable even when writing the MSRs from the
>>> guest side.
>>>
>>> To support this new scheme, introduce a new capability,
>>> KVM_CAP_HYPERV_SYNIC2, which, when enabled, makes sure that the synic
>>> pages aren't zeroed out in KVM.
>>>
>>> Signed-off-by: Roman Kagan <rkagan@virtuozzo.com>
>>> ---
>>
>> I have changed the subject tags to more common "kvm: x86: hyperv:" and
>> added minimal documentation:
>>
>> diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
>> index 4029943887a3..3f4b1d5f0dce 100644
>> --- a/Documentation/virtual/kvm/api.txt
>> +++ b/Documentation/virtual/kvm/api.txt
>> @@ -4157,3 +4157,10 @@ Currently the following bits are defined for the device_irq_level bitmap:
>>  Future versions of kvm may implement additional events. These will get
>>  indicated by returning a higher number from KVM_CHECK_EXTENSION and will be
>>  listed above.
>> +
>> +8.10 KVM_CAP_HYPERV_SYNIC2
>> +
>> +Architectures: x86
>> +
>> +The only difference from KVM_CAP_HYPERV_SYNIC is that KVM does not clear SynIC
>> +pages when the guest enables them.
>>
>> Please let me know if you'd like to improve it.
> 
> Yes that's perfectly fine, thanks.  Sorry, I should have done it
> myself...

The new capability should also check that args[0] is zero.  That was a
mistake done previously that we shouldn't repeat.

Paolo