diff mbox series

[v2,1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration

Message ID 20210114205449.8715-2-mlevitsk@redhat.com (mailing list archive)
State New, archived
Headers show
Series VMX: more nested fixes | expand

Commit Message

Maxim Levitsky Jan. 14, 2021, 8:54 p.m. UTC
Even when we are outside the nested guest, some vmcs02 fields
are not in sync vs vmcs12.

However during the migration, the vmcs12 has to be up to date
to be able to load it later after the migration.

To fix that, call that function.

Fixes: 7952d769c29ca ("KVM: nVMX: Sync rarely accessed guest fields only when needed")

Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
---
 arch/x86/kvm/vmx/nested.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

Comments

Sean Christopherson Jan. 14, 2021, 11:58 p.m. UTC | #1
Subject is confusing, and technically wrong.  Confusing because there is no call
to sync_vmcs02_to_vmcs12_rare().  Technically wrong because sync_...() won't be
called if need_sync_vmcs02_to_vmcs12_rare==false.

Maybe something like?

KVM: nVMX: Sync unsync'd vmcs02 state to vmcs12 on migration

On Thu, Jan 14, 2021, Maxim Levitsky wrote:
> Even when we are outside the nested guest, some vmcs02 fields
> are not in sync vs vmcs12.

s/are not/may not be

It'd be helpful to provide more details in the changelog, e.g. for those not in
the loop, it's not obvious that KVM intentionally keeps those fields unsync'd,
even across nested VM-Exit.

> However during the migration, the vmcs12 has to be up to date
> to be able to load it later after the migration.
> 
> To fix that, call that function.

s/that function/copy_vmcs02_to_vmcs12_rare().  Characters are cheap, and it's
jarring to have to jump back all the way back to the subject.  And, as mentioned
above, this doesn't actually call sync_ directly.

For the code,

Reviewed-by: Sean Christopherson <seanjc@google.com> 

> 
> Fixes: 7952d769c29ca ("KVM: nVMX: Sync rarely accessed guest fields only when needed")
> 
> Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> ---
>  arch/x86/kvm/vmx/nested.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 0fbb46990dfce..776688f9d1017 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
> @@ -6077,11 +6077,14 @@ static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
>  	if (is_guest_mode(vcpu)) {
>  		sync_vmcs02_to_vmcs12(vcpu, vmcs12);
>  		sync_vmcs02_to_vmcs12_rare(vcpu, vmcs12);
> -	} else if (!vmx->nested.need_vmcs12_to_shadow_sync) {
> -		if (vmx->nested.hv_evmcs)
> -			copy_enlightened_to_vmcs12(vmx);
> -		else if (enable_shadow_vmcs)
> -			copy_shadow_to_vmcs12(vmx);
> +	} else  {
> +		copy_vmcs02_to_vmcs12_rare(vcpu, get_vmcs12(vcpu));
> +		if (!vmx->nested.need_vmcs12_to_shadow_sync) {
> +			if (vmx->nested.hv_evmcs)
> +				copy_enlightened_to_vmcs12(vmx);
> +			else if (enable_shadow_vmcs)
> +				copy_shadow_to_vmcs12(vmx);
> +		}
>  	}
>  
>  	BUILD_BUG_ON(sizeof(user_vmx_nested_state->vmcs12) < VMCS12_SIZE);
> -- 
> 2.26.2
>
Paolo Bonzini Jan. 21, 2021, 2:59 p.m. UTC | #2
On 15/01/21 00:58, Sean Christopherson wrote:
> Reviewed-by: Sean Christopherson<seanjc@google.com>  

New commit message:

KVM: nVMX: Sync unsync'd vmcs02 state to vmcs12 on migration

Even when we are outside the nested guest, some vmcs02 fields
may not be in sync vs vmcs12.  This is intentional, even across
nested VM-exit, because the sync can be delayed until the nested
hypervisor performs a VMCLEAR or a VMREAD/VMWRITE that affects those
rarely accessed fields.

However, during KVM_GET_NESTED_STATE, the vmcs12 has to be up to date to
be able to restore it.  To fix that, call copy_vmcs02_to_vmcs12_rare()
before the vmcs12 contents are copied to userspace.

Paolo
diff mbox series

Patch

diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 0fbb46990dfce..776688f9d1017 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -6077,11 +6077,14 @@  static int vmx_get_nested_state(struct kvm_vcpu *vcpu,
 	if (is_guest_mode(vcpu)) {
 		sync_vmcs02_to_vmcs12(vcpu, vmcs12);
 		sync_vmcs02_to_vmcs12_rare(vcpu, vmcs12);
-	} else if (!vmx->nested.need_vmcs12_to_shadow_sync) {
-		if (vmx->nested.hv_evmcs)
-			copy_enlightened_to_vmcs12(vmx);
-		else if (enable_shadow_vmcs)
-			copy_shadow_to_vmcs12(vmx);
+	} else  {
+		copy_vmcs02_to_vmcs12_rare(vcpu, get_vmcs12(vcpu));
+		if (!vmx->nested.need_vmcs12_to_shadow_sync) {
+			if (vmx->nested.hv_evmcs)
+				copy_enlightened_to_vmcs12(vmx);
+			else if (enable_shadow_vmcs)
+				copy_shadow_to_vmcs12(vmx);
+		}
 	}
 
 	BUILD_BUG_ON(sizeof(user_vmx_nested_state->vmcs12) < VMCS12_SIZE);