* [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits
@ 2020-12-03 15:15 Paolo Bonzini
2020-12-04 17:12 ` Sean Christopherson
2020-12-08 9:23 ` Denis V. Lunev
0 siblings, 2 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-12-03 15:15 UTC (permalink / raw)
To: linux-kernel, kvm; +Cc: stable, Denis V . Lunev
Until commit e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP",
2018-05-17), KVM was testing both Intel and AMD CPUID bits before allowing the
guest to write MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD. Testing only Intel bits
on VMX processors, or only AMD bits on SVM processors, fails if the guests are
created with the "opposite" vendor as the host.
While at it, also tweak the host CPU check to use the vendor-agnostic feature bit
X86_FEATURE_IBPB, since we only care about the availability of the MSR on the host
here and not about specific CPUID bits.
Fixes: e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP")
Cc: stable@vger.kernel.org
Reported-by: Denis V. Lunev <den@openvz.org>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
arch/x86/kvm/svm/svm.c | 3 ++-
arch/x86/kvm/vmx/vmx.c | 10 +++++++---
2 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 62390fbc9233..0b4aa60b2754 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2686,12 +2686,13 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
break;
case MSR_IA32_PRED_CMD:
if (!msr->host_initiated &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
!guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
return 1;
if (data & ~PRED_CMD_IBPB)
return 1;
- if (!boot_cpu_has(X86_FEATURE_AMD_IBPB))
+ if (!boot_cpu_has(X86_FEATURE_IBPB))
return 1;
if (!data)
break;
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index c3441e7e5a87..b74d2105ced7 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -2028,7 +2028,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
break;
case MSR_IA32_SPEC_CTRL:
if (!msr_info->host_initiated &&
- !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
+ !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
return 1;
if (kvm_spec_ctrl_test_value(data))
@@ -2063,12 +2066,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
goto find_uret_msr;
case MSR_IA32_PRED_CMD:
if (!msr_info->host_initiated &&
- !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
+ !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
return 1;
if (data & ~PRED_CMD_IBPB)
return 1;
- if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
+ if (!boot_cpu_has(X86_FEATURE_IBPB))
return 1;
if (!data)
break;
--
2.26.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits
2020-12-03 15:15 [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits Paolo Bonzini
@ 2020-12-04 17:12 ` Sean Christopherson
2020-12-06 10:16 ` Paolo Bonzini
2020-12-08 9:23 ` Denis V. Lunev
1 sibling, 1 reply; 4+ messages in thread
From: Sean Christopherson @ 2020-12-04 17:12 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: linux-kernel, kvm, stable, Denis V . Lunev
On Thu, Dec 03, 2020, Paolo Bonzini wrote:
> Until commit e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP",
> 2018-05-17), KVM was testing both Intel and AMD CPUID bits before allowing the
> guest to write MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD. Testing only Intel bits
> on VMX processors, or only AMD bits on SVM processors, fails if the guests are
> created with the "opposite" vendor as the host.
>
> While at it, also tweak the host CPU check to use the vendor-agnostic feature bit
> X86_FEATURE_IBPB, since we only care about the availability of the MSR on the host
> here and not about specific CPUID bits.
>
> Fixes: e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP")
> Cc: stable@vger.kernel.org
> Reported-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/svm/svm.c | 3 ++-
> arch/x86/kvm/vmx/vmx.c | 10 +++++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 62390fbc9233..0b4aa60b2754 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2686,12 +2686,13 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> break;
> case MSR_IA32_PRED_CMD:
> if (!msr->host_initiated &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
> return 1;
>
> if (data & ~PRED_CMD_IBPB)
> return 1;
> - if (!boot_cpu_has(X86_FEATURE_AMD_IBPB))
> + if (!boot_cpu_has(X86_FEATURE_IBPB))
> return 1;
> if (!data)
> break;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index c3441e7e5a87..b74d2105ced7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2028,7 +2028,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
Doesn't vmx_get_msr() also need to be changed for SPEC_CTRL? Assuming that's
the case, adding helpers in cpuid.h to detect guest support for SPEC_CTRL (and
maybe for PRED_CMD?) would be helpful. It'd reduce duplicate code and document
that KVM allows cross-vendor emulation. The condition for SPEC_CTRL support is
especially long.
> break;
> case MSR_IA32_SPEC_CTRL:
> if (!msr_info->host_initiated &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
Whitespace is messed up.
ERROR: code indent should use tabs where possible
#54: FILE: arch/x86/kvm/vmx/vmx.c:2031:
+ !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&$
WARNING: please, no spaces at the start of a line
#54: FILE: arch/x86/kvm/vmx/vmx.c:2031:
+ !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&$
ERROR: code indent should use tabs where possible
#55: FILE: arch/x86/kvm/vmx/vmx.c:2032:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) &&$
WARNING: please, no spaces at the start of a line
#55: FILE: arch/x86/kvm/vmx/vmx.c:2032:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) &&$
ERROR: code indent should use tabs where possible
#56: FILE: arch/x86/kvm/vmx/vmx.c:2033:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&$
WARNING: please, no spaces at the start of a line
#56: FILE: arch/x86/kvm/vmx/vmx.c:2033:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&$
ERROR: code indent should use tabs where possible
#57: FILE: arch/x86/kvm/vmx/vmx.c:2034:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))$
WARNING: please, no spaces at the start of a line
#57: FILE: arch/x86/kvm/vmx/vmx.c:2034:
+ !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))$
> return 1;
>
> if (kvm_spec_ctrl_test_value(data))
> @@ -2063,12 +2066,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> goto find_uret_msr;
> case MSR_IA32_PRED_CMD:
> if (!msr_info->host_initiated &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
> return 1;
>
> if (data & ~PRED_CMD_IBPB)
> return 1;
> - if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
> + if (!boot_cpu_has(X86_FEATURE_IBPB))
> return 1;
> if (!data)
> break;
> --
> 2.26.2
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits
2020-12-04 17:12 ` Sean Christopherson
@ 2020-12-06 10:16 ` Paolo Bonzini
0 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2020-12-06 10:16 UTC (permalink / raw)
To: Sean Christopherson; +Cc: linux-kernel, kvm, stable, Denis V . Lunev
On 04/12/20 18:12, Sean Christopherson wrote:
> Assuming that's
> the case, adding helpers in cpuid.h to detect guest support for SPEC_CTRL (and
> maybe for PRED_CMD?) would be helpful. It'd reduce duplicate code and document
> that KVM allows cross-vendor emulation. The condition for SPEC_CTRL support is
> especially long.
That's a great idea nevertheless.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits
2020-12-03 15:15 [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits Paolo Bonzini
2020-12-04 17:12 ` Sean Christopherson
@ 2020-12-08 9:23 ` Denis V. Lunev
1 sibling, 0 replies; 4+ messages in thread
From: Denis V. Lunev @ 2020-12-08 9:23 UTC (permalink / raw)
To: Paolo Bonzini, linux-kernel, kvm; +Cc: stable
On 12/3/20 6:15 PM, Paolo Bonzini wrote:
> Until commit e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP",
> 2018-05-17), KVM was testing both Intel and AMD CPUID bits before allowing the
> guest to write MSR_IA32_SPEC_CTRL and MSR_IA32_PRED_CMD. Testing only Intel bits
> on VMX processors, or only AMD bits on SVM processors, fails if the guests are
> created with the "opposite" vendor as the host.
>
> While at it, also tweak the host CPU check to use the vendor-agnostic feature bit
> X86_FEATURE_IBPB, since we only care about the availability of the MSR on the host
> here and not about specific CPUID bits.
>
> Fixes: e7c587da1252 ("x86/speculation: Use synthetic bits for IBRS/IBPB/STIBP")
> Cc: stable@vger.kernel.org
> Reported-by: Denis V. Lunev <den@openvz.org>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> arch/x86/kvm/svm/svm.c | 3 ++-
> arch/x86/kvm/vmx/vmx.c | 10 +++++++---
> 2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 62390fbc9233..0b4aa60b2754 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2686,12 +2686,13 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
> break;
> case MSR_IA32_PRED_CMD:
> if (!msr->host_initiated &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
> return 1;
>
> if (data & ~PRED_CMD_IBPB)
> return 1;
> - if (!boot_cpu_has(X86_FEATURE_AMD_IBPB))
> + if (!boot_cpu_has(X86_FEATURE_IBPB))
> return 1;
> if (!data)
> break;
> diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
> index c3441e7e5a87..b74d2105ced7 100644
> --- a/arch/x86/kvm/vmx/vmx.c
> +++ b/arch/x86/kvm/vmx/vmx.c
> @@ -2028,7 +2028,10 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> break;
> case MSR_IA32_SPEC_CTRL:
> if (!msr_info->host_initiated &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_STIBP) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBRS) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_SSBD))
> return 1;
>
> if (kvm_spec_ctrl_test_value(data))
> @@ -2063,12 +2066,13 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> goto find_uret_msr;
> case MSR_IA32_PRED_CMD:
> if (!msr_info->host_initiated &&
> - !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL))
> + !guest_cpuid_has(vcpu, X86_FEATURE_SPEC_CTRL) &&
> + !guest_cpuid_has(vcpu, X86_FEATURE_AMD_IBPB))
> return 1;
>
> if (data & ~PRED_CMD_IBPB)
> return 1;
> - if (!boot_cpu_has(X86_FEATURE_SPEC_CTRL))
> + if (!boot_cpu_has(X86_FEATURE_IBPB))
> return 1;
> if (!data)
> break;
Reviewed-by: Denis V. Lunev <den@openvz.org>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2020-12-08 9:24 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-03 15:15 [PATCH] KVM: x86: reinstate vendor-agnostic check on SPEC_CTRL cpuid bits Paolo Bonzini
2020-12-04 17:12 ` Sean Christopherson
2020-12-06 10:16 ` Paolo Bonzini
2020-12-08 9:23 ` Denis V. Lunev
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.