| Message ID | 20170621200915.GD27032@potion (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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... Thanks, Roman.
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
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.
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.