| Message ID | 20210511111956.1555830-6-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Fixes for nested state migration when eVMCS is in use | expand |
On Tue, May 11, 2021, Vitaly Kuznetsov wrote: > When nested state migration happens during L1's execution, it > is incorrect to modify eVMCS as it is L1 who 'owns' it at the moment. > At lease genuine Hyper-v seems to not be very happy when 'clean fields' > data changes underneath it. > > 'Clean fields' data is used in KVM twice: by copy_enlightened_to_vmcs12() > and prepare_vmcs02_rare() so we can reset it from prepare_vmcs02() instead. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- > arch/x86/kvm/vmx/nested.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 3257a2291693..1661e2e19560 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -2090,9 +2090,6 @@ void nested_sync_vmcs12_to_shadow(struct kvm_vcpu *vcpu) > > if (vmx->nested.hv_evmcs) { > copy_vmcs12_to_enlightened(vmx); > - /* All fields are clean */ > - vmx->nested.hv_evmcs->hv_clean_fields |= > - HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; > } else { > copy_vmcs12_to_shadow(vmx); > } Looks like this makes both paths single lines, i.e. the curly braces can be dropped. > @@ -2636,6 +2633,12 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, > > kvm_rsp_write(vcpu, vmcs12->guest_rsp); > kvm_rip_write(vcpu, vmcs12->guest_rip); > + > + /* Mark all fields as clean so L1 hypervisor can set what's dirty */ > + if (hv_evmcs) > + vmx->nested.hv_evmcs->hv_clean_fields |= > + HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; > + > return 0; > } > > -- > 2.30.2 >
Sean Christopherson <seanjc@google.com> writes: > On Tue, May 11, 2021, Vitaly Kuznetsov wrote: >> When nested state migration happens during L1's execution, it >> is incorrect to modify eVMCS as it is L1 who 'owns' it at the moment. >> At lease genuine Hyper-v seems to not be very happy when 'clean fields' >> data changes underneath it. >> >> 'Clean fields' data is used in KVM twice: by copy_enlightened_to_vmcs12() >> and prepare_vmcs02_rare() so we can reset it from prepare_vmcs02() instead. >> >> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> >> --- >> arch/x86/kvm/vmx/nested.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >> index 3257a2291693..1661e2e19560 100644 >> --- a/arch/x86/kvm/vmx/nested.c >> +++ b/arch/x86/kvm/vmx/nested.c >> @@ -2090,9 +2090,6 @@ void nested_sync_vmcs12_to_shadow(struct kvm_vcpu *vcpu) >> >> if (vmx->nested.hv_evmcs) { >> copy_vmcs12_to_enlightened(vmx); >> - /* All fields are clean */ >> - vmx->nested.hv_evmcs->hv_clean_fields |= >> - HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; >> } else { >> copy_vmcs12_to_shadow(vmx); >> } > > Looks like this makes both paths single lines, i.e. the curly braces can be > dropped. > Yea, I've noticed that but I wasn't exactly sure the benefit of removing one line here overwhelms the pain of needing an extra step while doing code archeology with 'git blame' :-) Will drop on v2. >> @@ -2636,6 +2633,12 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, >> >> kvm_rsp_write(vcpu, vmcs12->guest_rsp); >> kvm_rip_write(vcpu, vmcs12->guest_rip); >> + >> + /* Mark all fields as clean so L1 hypervisor can set what's dirty */ >> + if (hv_evmcs) >> + vmx->nested.hv_evmcs->hv_clean_fields |= >> + HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; >> + >> return 0; >> } >> >> -- >> 2.30.2 >> >
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 3257a2291693..1661e2e19560 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -2090,9 +2090,6 @@ void nested_sync_vmcs12_to_shadow(struct kvm_vcpu *vcpu) if (vmx->nested.hv_evmcs) { copy_vmcs12_to_enlightened(vmx); - /* All fields are clean */ - vmx->nested.hv_evmcs->hv_clean_fields |= - HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; } else { copy_vmcs12_to_shadow(vmx); } @@ -2636,6 +2633,12 @@ static int prepare_vmcs02(struct kvm_vcpu *vcpu, struct vmcs12 *vmcs12, kvm_rsp_write(vcpu, vmcs12->guest_rsp); kvm_rip_write(vcpu, vmcs12->guest_rip); + + /* Mark all fields as clean so L1 hypervisor can set what's dirty */ + if (hv_evmcs) + vmx->nested.hv_evmcs->hv_clean_fields |= + HV_VMX_ENLIGHTENED_CLEAN_FIELD_ALL; + return 0; }
When nested state migration happens during L1's execution, it is incorrect to modify eVMCS as it is L1 who 'owns' it at the moment. At lease genuine Hyper-v seems to not be very happy when 'clean fields' data changes underneath it. 'Clean fields' data is used in KVM twice: by copy_enlightened_to_vmcs12() and prepare_vmcs02_rare() so we can reset it from prepare_vmcs02() instead. Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/vmx/nested.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)