| Message ID | 20200709171507.1819-1-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: fixes for preemption timer migration | expand |
On Thu, Jul 9, 2020 at 10:15 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > Commit 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration", > 2020-06-01) accidentally broke nVMX live migration from older version > by changing the userspace ABI. Restore it and, while at it, ensure > that vmx->nested.has_preemption_timer_deadline is always initialized > according to the KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE flag. > > Cc: Makarand Sonare <makarandsonare@google.com> > Fixes: 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration") > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > --- > arch/x86/include/uapi/asm/kvm.h | 5 +++-- > arch/x86/kvm/vmx/nested.c | 3 ++- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h > index 17c5a038f42d..0780f97c1850 100644 > --- a/arch/x86/include/uapi/asm/kvm.h > +++ b/arch/x86/include/uapi/asm/kvm.h > @@ -408,14 +408,15 @@ struct kvm_vmx_nested_state_data { > }; > > struct kvm_vmx_nested_state_hdr { > - __u32 flags; > __u64 vmxon_pa; > __u64 vmcs12_pa; > - __u64 preemption_timer_deadline; > > struct { > __u16 flags; > } smm; > + > + __u32 flags; > + __u64 preemption_timer_deadline; > }; > > struct kvm_svm_nested_state_data { > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index b26655104d4a..3fc2411edc92 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -6180,7 +6180,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > vmx->nested.has_preemption_timer_deadline = true; > vmx->nested.preemption_timer_deadline = > kvm_state->hdr.vmx.preemption_timer_deadline; > - } > + } else > + vmx->nested.has_preemption_timer_deadline = false; Doesn't the coding standard require braces around the else clause? Reviewed-by: Jim Mattson <jmattson@google.com>
On 09/07/20 19:23, Jim Mattson wrote: > On Thu, Jul 9, 2020 at 10:15 AM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> Commit 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration", >> 2020-06-01) accidentally broke nVMX live migration from older version >> by changing the userspace ABI. Restore it and, while at it, ensure >> that vmx->nested.has_preemption_timer_deadline is always initialized >> according to the KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE flag. >> >> Cc: Makarand Sonare <makarandsonare@google.com> >> Fixes: 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration") >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> arch/x86/include/uapi/asm/kvm.h | 5 +++-- >> arch/x86/kvm/vmx/nested.c | 3 ++- >> 2 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h >> index 17c5a038f42d..0780f97c1850 100644 >> --- a/arch/x86/include/uapi/asm/kvm.h >> +++ b/arch/x86/include/uapi/asm/kvm.h >> @@ -408,14 +408,15 @@ struct kvm_vmx_nested_state_data { >> }; >> >> struct kvm_vmx_nested_state_hdr { >> - __u32 flags; >> __u64 vmxon_pa; >> __u64 vmcs12_pa; >> - __u64 preemption_timer_deadline; >> >> struct { >> __u16 flags; >> } smm; >> + >> + __u32 flags; >> + __u64 preemption_timer_deadline; >> }; >> >> struct kvm_svm_nested_state_data { >> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> index b26655104d4a..3fc2411edc92 100644 >> --- a/arch/x86/kvm/vmx/nested.c >> +++ b/arch/x86/kvm/vmx/nested.c >> @@ -6180,7 +6180,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >> vmx->nested.has_preemption_timer_deadline = true; >> vmx->nested.preemption_timer_deadline = >> kvm_state->hdr.vmx.preemption_timer_deadline; >> - } >> + } else >> + vmx->nested.has_preemption_timer_deadline = false; > > Doesn't the coding standard require braces around the else clause? Yes, indeed. Paolo > Reviewed-by: Jim Mattson <jmattson@google.com> >
Jim Mattson <jmattson@google.com> writes: > On Thu, Jul 9, 2020 at 10:15 AM Paolo Bonzini <pbonzini@redhat.com> wrote: >> >> Commit 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration", >> 2020-06-01) accidentally broke nVMX live migration from older version >> by changing the userspace ABI. Restore it and, while at it, ensure >> that vmx->nested.has_preemption_timer_deadline is always initialized >> according to the KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE flag. >> >> Cc: Makarand Sonare <makarandsonare@google.com> >> Fixes: 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration") >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> --- >> arch/x86/include/uapi/asm/kvm.h | 5 +++-- >> arch/x86/kvm/vmx/nested.c | 3 ++- >> 2 files changed, 5 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h >> index 17c5a038f42d..0780f97c1850 100644 >> --- a/arch/x86/include/uapi/asm/kvm.h >> +++ b/arch/x86/include/uapi/asm/kvm.h >> @@ -408,14 +408,15 @@ struct kvm_vmx_nested_state_data { >> }; >> >> struct kvm_vmx_nested_state_hdr { >> - __u32 flags; >> __u64 vmxon_pa; >> __u64 vmcs12_pa; >> - __u64 preemption_timer_deadline; >> >> struct { >> __u16 flags; >> } smm; >> + >> + __u32 flags; >> + __u64 preemption_timer_deadline; >> }; >> Oops! >> struct kvm_svm_nested_state_data { >> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> index b26655104d4a..3fc2411edc92 100644 >> --- a/arch/x86/kvm/vmx/nested.c >> +++ b/arch/x86/kvm/vmx/nested.c >> @@ -6180,7 +6180,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >> vmx->nested.has_preemption_timer_deadline = true; >> vmx->nested.preemption_timer_deadline = >> kvm_state->hdr.vmx.preemption_timer_deadline; >> - } >> + } else >> + vmx->nested.has_preemption_timer_deadline = false; > > Doesn't the coding standard require braces around the else clause? > I think so... for if/else where at least one of them is multiline. > Reviewed-by: Jim Mattson <jmattson@google.com> Looks good to me, Reviewed-by: Bandan Das <bsd@redhat.com>
diff --git a/arch/x86/include/uapi/asm/kvm.h b/arch/x86/include/uapi/asm/kvm.h index 17c5a038f42d..0780f97c1850 100644 --- a/arch/x86/include/uapi/asm/kvm.h +++ b/arch/x86/include/uapi/asm/kvm.h @@ -408,14 +408,15 @@ struct kvm_vmx_nested_state_data { }; struct kvm_vmx_nested_state_hdr { - __u32 flags; __u64 vmxon_pa; __u64 vmcs12_pa; - __u64 preemption_timer_deadline; struct { __u16 flags; } smm; + + __u32 flags; + __u64 preemption_timer_deadline; }; struct kvm_svm_nested_state_data { diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index b26655104d4a..3fc2411edc92 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -6180,7 +6180,8 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, vmx->nested.has_preemption_timer_deadline = true; vmx->nested.preemption_timer_deadline = kvm_state->hdr.vmx.preemption_timer_deadline; - } + } else + vmx->nested.has_preemption_timer_deadline = false; if (nested_vmx_check_controls(vcpu, vmcs12) || nested_vmx_check_host_state(vcpu, vmcs12) ||
Commit 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration", 2020-06-01) accidentally broke nVMX live migration from older version by changing the userspace ABI. Restore it and, while at it, ensure that vmx->nested.has_preemption_timer_deadline is always initialized according to the KVM_STATE_VMX_PREEMPTION_TIMER_DEADLINE flag. Cc: Makarand Sonare <makarandsonare@google.com> Fixes: 850448f35aaf ("KVM: nVMX: Fix VMX preemption timer migration") Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- arch/x86/include/uapi/asm/kvm.h | 5 +++-- arch/x86/kvm/vmx/nested.c | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-)