From: Sean Christopherson <seanjc@google.com>
To: Maxim Levitsky <mlevitsk@redhat.com>
Cc: kvm@vger.kernel.org, Thomas Gleixner <tglx@linutronix.de>,
x86@kernel.org, Borislav Petkov <bp@alien8.de>,
Paolo Bonzini <pbonzini@redhat.com>,
Ingo Molnar <mingo@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Joerg Roedel <joro@8bytes.org>,
Wanpeng Li <wanpengli@tencent.com>,
"H. Peter Anvin" <hpa@zytor.com>,
linux-kernel@vger.kernel.org, Jim Mattson <jmattson@google.com>
Subject: Re: [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration
Date: Thu, 14 Jan 2021 15:58:37 -0800 [thread overview]
Message-ID: <YADarUMsE9uDKxOe@google.com> (raw)
In-Reply-To: <20210114205449.8715-2-mlevitsk@redhat.com>
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
>
next prev parent reply other threads:[~2021-01-14 23:59 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-14 20:54 [PATCH v2 0/3] VMX: more nested fixes Maxim Levitsky
2021-01-14 20:54 ` [PATCH v2 1/3] KVM: nVMX: Always call sync_vmcs02_to_vmcs12_rare on migration Maxim Levitsky
2021-01-14 23:58 ` Sean Christopherson [this message]
2021-01-21 14:59 ` Paolo Bonzini
2021-01-14 20:54 ` [PATCH v2 2/3] KVM: nVMX: add kvm_nested_vmlaunch_resume tracepoint Maxim Levitsky
2021-01-15 0:14 ` Sean Christopherson
2021-01-15 13:48 ` Paolo Bonzini
2021-01-15 16:30 ` Sean Christopherson
2021-01-21 17:00 ` Maxim Levitsky
2021-01-21 16:58 ` Maxim Levitsky
2021-01-21 17:02 ` Maxim Levitsky
2021-01-14 20:54 ` [PATCH v2 3/3] KVM: VMX: read idt_vectoring_info a bit earlier Maxim Levitsky
2021-01-15 0:29 ` Sean Christopherson
2021-01-21 17:04 ` Maxim Levitsky
2021-01-21 15:00 ` [PATCH v2 0/3] VMX: more nested fixes Paolo Bonzini
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YADarUMsE9uDKxOe@google.com \
--to=seanjc@google.com \
--cc=bp@alien8.de \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=mlevitsk@redhat.com \
--cc=pbonzini@redhat.com \
--cc=tglx@linutronix.de \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).