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

[Roman Kagan <rkagan@virtuozzo.com>]

On Thu, Jun 22, 2017 at 02:06:57PM +0200, Paolo Bonzini wrote:
> 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.

Good point, so that we don't need to add another capability when we
figure out another design flaw ;)

I'll respin the patches then.

Thanks,
Roman.