From: Paolo Bonzini <pbonzini@redhat.com>
To: Babu Moger <babu.moger@amd.com>,
tglx@linutronix.de, mingo@redhat.com, bp@alien8.de
Cc: fenghua.yu@intel.com, tony.luck@intel.com, wanpengli@tencent.com,
kvm@vger.kernel.org, thomas.lendacky@amd.com,
peterz@infradead.org, seanjc@google.com, joro@8bytes.org,
x86@kernel.org, kyung.min.park@intel.com,
linux-kernel@vger.kernel.org, krish.sadhukhan@oracle.com,
hpa@zytor.com, mgross@linux.intel.com, vkuznets@redhat.com,
kim.phillips@amd.com, wei.huang2@amd.com, jmattson@google.com
Subject: Re: [PATCH v4 2/2] KVM: SVM: Add support for Virtual SPEC_CTRL
Date: Thu, 11 Feb 2021 09:56:39 +0100 [thread overview]
Message-ID: <addd9d50-4a2e-b3ae-ff32-5332ab664463@redhat.com> (raw)
In-Reply-To: <161188100955.28787.11816849358413330720.stgit@bmoger-ubuntu>
On 29/01/21 01:43, Babu Moger wrote:
> This support also fixes an issue where a guest may sometimes see an
> inconsistent value for the SPEC_CTRL MSR on processors that support this
> feature. With the current SPEC_CTRL support, the first write to
> SPEC_CTRL is intercepted and the virtualized version of the SPEC_CTRL
> MSR is not updated.
This is a bit ugly, new features should always be enabled manually (AMD
did it right for vVMLOAD/vVMSAVE for example, even though _in theory_
assuming that all hypervisors were intercepting VMLOAD/VMSAVE would have
been fine).
Also regarding nested virtualization:
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 7a605ad8254d..9e51f9e4f631 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -534,6 +534,7 @@ int nested_svm_vmrun(struct vcpu_svm *svm)
> hsave->save.cr3 = vmcb->save.cr3;
> else
> hsave->save.cr3 = kvm_read_cr3(&svm->vcpu);
> + hsave->save.spec_ctrl = vmcb->save.spec_ctrl;
>
> copy_vmcb_control_area(&hsave->control, &vmcb->control);
>
> @@ -675,6 +676,7 @@ int nested_svm_vmexit(struct vcpu_svm *svm)
> kvm_rip_write(&svm->vcpu, hsave->save.rip);
> svm->vmcb->save.dr7 = DR7_FIXED_1;
> svm->vmcb->save.cpl = 0;
> + svm->vmcb->save.spec_ctrl = hsave->save.spec_ctrl;
> svm->vmcb->control.exit_int_info = 0;
>
> vmcb_mark_all_dirty(svm->vmcb);
I think this is incorrect. Since we don't support this feature in the
nested hypervisor, any writes to the SPEC_CTRL MSR while L2 (nested
guest) runs have to be reflected to L1 (nested hypervisor). In other
words, this new field is more like VMLOAD/VMSAVE state, in that it
doesn't change across VMRUN and VMEXIT. These two hunks can be removed.
If we want to do it, exposing this feature to the nested hypervisor will
be a bit complicated, because one has to write host SPEC_CTRL |
vmcb01.GuestSpecCtrl in the host MSR, in order to free the vmcb02
GuestSpecCtrl for the vmcb12 GuestSpecCtrl.
It would also be possible to emulate it on processors that don't have
it. However I'm not sure it's a good idea because of the problem that
you mentioned with running old kernels on new processors.
I have queued the patches with the small fix above. However I plan to
only include them in 5.13 because I have a bunch of other SVM patches,
those have been tested already but I need to send them out for review
before "officially" getting them in kvm.git.
Paolo
next prev parent reply other threads:[~2021-02-11 8:59 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-01-29 0:43 [PATCH v4 0/2] x86: Add the feature Virtual SPEC_CTRL Babu Moger
2021-01-29 0:43 ` [PATCH v4 1/2] x86/cpufeatures: Add the Virtual SPEC_CTRL feature Babu Moger
2021-01-29 0:43 ` [PATCH v4 2/2] KVM: SVM: Add support for Virtual SPEC_CTRL Babu Moger
2021-02-11 8:56 ` Paolo Bonzini [this message]
2021-02-11 22:42 ` Babu Moger
2022-06-04 3:11 ` Jim Mattson
2022-06-13 15:09 ` Tom Lendacky
2022-06-13 19:23 ` Jim Mattson
2022-06-13 20:16 ` Tom Lendacky
2022-06-13 20:28 ` Jim Mattson
2021-02-10 23:40 ` [PATCH v4 0/2] x86: Add the feature " Babu Moger
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=addd9d50-4a2e-b3ae-ff32-5332ab664463@redhat.com \
--to=pbonzini@redhat.com \
--cc=babu.moger@amd.com \
--cc=bp@alien8.de \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kim.phillips@amd.com \
--cc=krish.sadhukhan@oracle.com \
--cc=kvm@vger.kernel.org \
--cc=kyung.min.park@intel.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mgross@linux.intel.com \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=seanjc@google.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=tony.luck@intel.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
--cc=wei.huang2@amd.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.