| Message ID | 20181205191956.31480-1-ehabkost@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | kvm: x86: Report STIBP on GET_SUPPORTED_CPUID | expand |
On Wed, Dec 5, 2018 at 11:19 AM Eduardo Habkost <ehabkost@redhat.com> wrote: > > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > to the guest, which makes STIBP available to guests. This was implemented > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > MSR_IA32_SPEC_CTRL"). > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > STIBP can be enabled in CPUID. Fix that by updating > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Jim Mattson <jmattson@google.com>
On Wed, Dec 05, 2018 at 05:19:56PM -0200, Eduardo Habkost wrote: > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > to the guest, which makes STIBP available to guests. This was implemented > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > MSR_IA32_SPEC_CTRL"). > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > STIBP can be enabled in CPUID. Fix that by updating Ooops! > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. Shouldn't there also be a patch in QEMU to use it? (aka, +stibp). Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thank you! > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > arch/x86/kvm/cpuid.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 7bcfa61375c0..cc6dd65828a4 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -381,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > /* cpuid 0x80000008.ebx */ > const u32 kvm_cpuid_8000_0008_ebx_x86_features = > F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | > - F(AMD_SSB_NO); > + F(AMD_SSB_NO) | F(AMD_STIBP); > > /* cpuid 0xC0000001.edx */ > const u32 kvm_cpuid_C000_0001_edx_x86_features = > @@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > /* cpuid 7.0.edx*/ > const u32 kvm_cpuid_7_0_edx_x86_features = > F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) | > - F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES); > + F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP); > > /* all calls to cpuid_count() should be made on the same cpu */ > get_cpu(); > -- > 2.18.0.rc1.1.g3f1ff2140 >
On Wed, Dec 5, 2018 at 2:02 PM Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote: > > On Wed, Dec 05, 2018 at 05:19:56PM -0200, Eduardo Habkost wrote: > > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > > to the guest, which makes STIBP available to guests. This was implemented > > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > > MSR_IA32_SPEC_CTRL"). > > > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > > STIBP can be enabled in CPUID. Fix that by updating > > Ooops! > > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. > > Shouldn't there also be a patch in QEMU to use it? (aka, +stibp). I don't see how that's relevant here. GET_SUPPORTED_CPUID should report all of the supported CPUID features regardless of what particular consumers choose to do with them.
On Wed, Dec 05, 2018 at 05:02:06PM -0500, Konrad Rzeszutek Wilk wrote: > On Wed, Dec 05, 2018 at 05:19:56PM -0200, Eduardo Habkost wrote: > > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > > to the guest, which makes STIBP available to guests. This was implemented > > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > > MSR_IA32_SPEC_CTRL"). > > > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > > STIBP can be enabled in CPUID. Fix that by updating > > Ooops! > > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. > > Shouldn't there also be a patch in QEMU to use it? (aka, +stibp). I will submit the QEMU patch soon. A patch exists on some downstream QEMU distributions, already, but it was never merged upstream because GET_SUPPORTED_CPUID never supported STIBP in the upstream kernel. (And because in the end it was not used for mitigating Spectre) > > Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Thanks!
On 05/12/18 20:19, Eduardo Habkost wrote: > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > to the guest, which makes STIBP available to guests. This was implemented > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > MSR_IA32_SPEC_CTRL"). > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > STIBP can be enabled in CPUID. Fix that by updating > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > --- > arch/x86/kvm/cpuid.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c > index 7bcfa61375c0..cc6dd65828a4 100644 > --- a/arch/x86/kvm/cpuid.c > +++ b/arch/x86/kvm/cpuid.c > @@ -381,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > /* cpuid 0x80000008.ebx */ > const u32 kvm_cpuid_8000_0008_ebx_x86_features = > F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | > - F(AMD_SSB_NO); > + F(AMD_SSB_NO) | F(AMD_STIBP); > > /* cpuid 0xC0000001.edx */ > const u32 kvm_cpuid_C000_0001_edx_x86_features = > @@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, > /* cpuid 7.0.edx*/ > const u32 kvm_cpuid_7_0_edx_x86_features = > F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) | > - F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES); > + F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP); > > /* all calls to cpuid_count() should be made on the same cpu */ > get_cpu(); > Queued, thanks. Paolo
On Fri, Dec 14, 2018 at 2:55 AM Paolo Bonzini <pbonzini@redhat.com> wrote: > > On 05/12/18 20:19, Eduardo Habkost wrote: > > Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL > > to the guest, which makes STIBP available to guests. This was implemented > > by commits d28b387fb74d ("KVM/VMX: Allow direct access to > > MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to > > MSR_IA32_SPEC_CTRL"). > > > > However, we never updated GET_SUPPORTED_CPUID to let userspace know that > > STIBP can be enabled in CPUID. Fix that by updating > > kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. > > > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> > > ... > Queued, thanks. > > Paolo On second thought, I believe this is premature. KVM does not currently support Intel's STIBP. From volume 4 of the SDM, "Prevents indirect branch predictions on *all* logical processors on the core from being controlled by any sibling logical processor in the same core." (emphasis mine) In particular, if two virtual HT siblings are running on different physical cores, and one of them sets IA32_SPEC_CTRL.STIBP, KVM must intercept the MSR write, track down the sibling vCPU thread, and ensure that IA32_SPEC_CTRL.STIBP is set on its logical processor. Moreover, whenever a vCPU thread migrates to a new logical processor, IA32_SPEC_CTRL.STIBP on the logical processor must be set to the logical or of the vCPU thread's own IA32_SPEC_CTRL.STIBP value and its sibling vCPU thread's IA32_SPEC_CTRL.STIBP value. Note that this implies that IA32_SPEC_CTRL cannot be a pass-through MSR.
diff --git a/arch/x86/kvm/cpuid.c b/arch/x86/kvm/cpuid.c index 7bcfa61375c0..cc6dd65828a4 100644 --- a/arch/x86/kvm/cpuid.c +++ b/arch/x86/kvm/cpuid.c @@ -381,7 +381,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, /* cpuid 0x80000008.ebx */ const u32 kvm_cpuid_8000_0008_ebx_x86_features = F(AMD_IBPB) | F(AMD_IBRS) | F(AMD_SSBD) | F(VIRT_SSBD) | - F(AMD_SSB_NO); + F(AMD_SSB_NO) | F(AMD_STIBP); /* cpuid 0xC0000001.edx */ const u32 kvm_cpuid_C000_0001_edx_x86_features = @@ -411,7 +411,7 @@ static inline int __do_cpuid_ent(struct kvm_cpuid_entry2 *entry, u32 function, /* cpuid 7.0.edx*/ const u32 kvm_cpuid_7_0_edx_x86_features = F(AVX512_4VNNIW) | F(AVX512_4FMAPS) | F(SPEC_CTRL) | - F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES); + F(SPEC_CTRL_SSBD) | F(ARCH_CAPABILITIES) | F(INTEL_STIBP); /* all calls to cpuid_count() should be made on the same cpu */ get_cpu();
Months ago, we have added code to allow direct access to MSR_IA32_SPEC_CTRL to the guest, which makes STIBP available to guests. This was implemented by commits d28b387fb74d ("KVM/VMX: Allow direct access to MSR_IA32_SPEC_CTRL") and b2ac58f90540 ("KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL"). However, we never updated GET_SUPPORTED_CPUID to let userspace know that STIBP can be enabled in CPUID. Fix that by updating kvm_cpuid_8000_0008_ebx_x86_features and kvm_cpuid_7_0_edx_x86_features. Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> --- arch/x86/kvm/cpuid.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)