[PATCH] KVM: SVM: don't apply host security mitigations to the guest

[PATCH] KVM: SVM: don't apply host security mitigations to the guest

[Fares Mehanna]

Support of virtual SPEC_CTRL caused a new behavior in KVM which made host
security mitigations applying by default to the guests.

We noticed a regression after applying the patch, primarily because of the
enablement of SSBD on AMD Milan.

This patch keeps the new behavior of applying host security mitigations to
the guests, but adds an option to disable it so the guests are free to pick
their own security mitigations.

Fixes: d00b99c514b3 ("KVM: SVM: Add support for Virtual SPEC_CTRL")
Signed-off-by: Fares Mehanna <faresx@amazon.de>
Reviewed-by: Benjamin Serebrin <serebrin@amazon.com>
Reviewed-by: Filippo Sironi <sironi@amazon.de>
---
 arch/x86/kvm/svm/svm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index f3813dbacb9f..c6ea24685301 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -225,6 +225,10 @@ module_param(avic, bool, 0444);
 bool __read_mostly dump_invalid_vmcb;
 module_param(dump_invalid_vmcb, bool, 0644);
 
+/* enable/disable applying host security mitigations on the guest */
+static bool host_mitigations_on_guest = true;
+module_param(host_mitigations_on_guest, bool, 0444);
+
 
 bool intercept_smi = true;
 module_param(intercept_smi, bool, 0444);
@@ -4000,6 +4004,12 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 	 */
 	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
 		x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
+	else if (!host_mitigations_on_guest)
+		/*
+		 * Clear the host MSR before vm-enter to avoid applying host
+		 * security mitigations to the guest.
+		 */
+		x86_spec_ctrl_set_guest(0, 0);
 
 	svm_vcpu_enter_exit(vcpu);
 
@@ -4025,7 +4035,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
 	if (!sev_es_guest(vcpu->kvm))
 		reload_tss(vcpu);
 
-	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
+	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL) || !host_mitigations_on_guest)
 		x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);
 
 	if (!sev_es_guest(vcpu->kvm)) {
-- 
2.37.1




Amazon Development Center Germany GmbH
Krausenstr. 38
10117 Berlin
Geschaeftsfuehrung: Christian Schlaeger, Jonathan Weiss
Eingetragen am Amtsgericht Charlottenburg unter HRB 149173 B
Sitz: Berlin
Ust-ID: DE 289 237 879

Re: [PATCH] KVM: SVM: don't apply host security mitigations to the guest

[Tom Lendacky]

On 9/12/22 06:58, Fares Mehanna wrote:
> Support of virtual SPEC_CTRL caused a new behavior in KVM which made host
> security mitigations applying by default to the guests.

Maybe expand on this to say that the effective mitigation is the host 
SPEC_CTRL value or'd with guest SPEC_CTRL value.

> 
> We noticed a regression after applying the patch, primarily because of the
> enablement of SSBD on AMD Milan.
> 
> This patch keeps the new behavior of applying host security mitigations to
> the guests, but adds an option to disable it so the guests are free to pick
> their own security mitigations.
> 
> Fixes: d00b99c514b3 ("KVM: SVM: Add support for Virtual SPEC_CTRL")
> Signed-off-by: Fares Mehanna <faresx@amazon.de>
> Reviewed-by: Benjamin Serebrin <serebrin@amazon.com>
> Reviewed-by: Filippo Sironi <sironi@amazon.de>
> ---
>   arch/x86/kvm/svm/svm.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index f3813dbacb9f..c6ea24685301 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -225,6 +225,10 @@ module_param(avic, bool, 0444);
>   bool __read_mostly dump_invalid_vmcb;
>   module_param(dump_invalid_vmcb, bool, 0644);
>   
> +/* enable/disable applying host security mitigations on the guest */
> +static bool host_mitigations_on_guest = true;
> +module_param(host_mitigations_on_guest, bool, 0444);
> +
>   
>   bool intercept_smi = true;
>   module_param(intercept_smi, bool, 0444);
> @@ -4000,6 +4004,12 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>   	 */
>   	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
>   		x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
> +	else if (!host_mitigations_on_guest)
> +		/*
> +		 * Clear the host MSR before vm-enter to avoid applying host
> +		 * security mitigations to the guest.
> +		 */
> +		x86_spec_ctrl_set_guest(0, 0);

If X86_FEATURE_V_SPEC_CTRL is active, won't svm->spec_ctrl and 
svm->virt_spec_ctrl always be zero, in which case you can do the if 
statement similar to the below one? Maybe just add a comment that those 
values will be zero in the case of X86_FEATURE_V_SPEC_CTRL, thus 
eliminating the host security mitigation effect.

Thanks,
Tom

>   
>   	svm_vcpu_enter_exit(vcpu);
>   
> @@ -4025,7 +4035,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
>   	if (!sev_es_guest(vcpu->kvm))
>   		reload_tss(vcpu);
>   
> -	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
> +	if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL) || !host_mitigations_on_guest)
>   		x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);
>   
>   	if (!sev_es_guest(vcpu->kvm)) {

Re: [PATCH] KVM: SVM: don't apply host security mitigations to the guest

[Jim Mattson]

On Thu, Sep 29, 2022 at 6:47 AM Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> On 9/12/22 06:58, Fares Mehanna wrote:
> > Support of virtual SPEC_CTRL caused a new behavior in KVM which made host
> > security mitigations applying by default to the guests.
>
> Maybe expand on this to say that the effective mitigation is the host
> SPEC_CTRL value or'd with guest SPEC_CTRL value.
>
> >
> > We noticed a regression after applying the patch, primarily because of the
> > enablement of SSBD on AMD Milan.
> >
> > This patch keeps the new behavior of applying host security mitigations to
> > the guests, but adds an option to disable it so the guests are free to pick
> > their own security mitigations.
> >
> > Fixes: d00b99c514b3 ("KVM: SVM: Add support for Virtual SPEC_CTRL")
> > Signed-off-by: Fares Mehanna <faresx@amazon.de>
> > Reviewed-by: Benjamin Serebrin <serebrin@amazon.com>
> > Reviewed-by: Filippo Sironi <sironi@amazon.de>
> > ---
> >   arch/x86/kvm/svm/svm.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> > index f3813dbacb9f..c6ea24685301 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -225,6 +225,10 @@ module_param(avic, bool, 0444);
> >   bool __read_mostly dump_invalid_vmcb;
> >   module_param(dump_invalid_vmcb, bool, 0644);
> >
> > +/* enable/disable applying host security mitigations on the guest */
> > +static bool host_mitigations_on_guest = true;
> > +module_param(host_mitigations_on_guest, bool, 0444);
> > +
> >
> >   bool intercept_smi = true;
> >   module_param(intercept_smi, bool, 0444);
> > @@ -4000,6 +4004,12 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >        */
> >       if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
> >               x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl);
> > +     else if (!host_mitigations_on_guest)
> > +             /*
> > +              * Clear the host MSR before vm-enter to avoid applying host
> > +              * security mitigations to the guest.
> > +              */
> > +             x86_spec_ctrl_set_guest(0, 0);
>
> If X86_FEATURE_V_SPEC_CTRL is active, won't svm->spec_ctrl and
> svm->virt_spec_ctrl always be zero, in which case you can do the if
> statement similar to the below one? Maybe just add a comment that those
> values will be zero in the case of X86_FEATURE_V_SPEC_CTRL, thus
> eliminating the host security mitigation effect.
>
> Thanks,
> Tom
>
> >
> >       svm_vcpu_enter_exit(vcpu);
> >
> > @@ -4025,7 +4035,7 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct kvm_vcpu *vcpu)
> >       if (!sev_es_guest(vcpu->kvm))
> >               reload_tss(vcpu);
> >
> > -     if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL))
> > +     if (!static_cpu_has(X86_FEATURE_V_SPEC_CTRL) || !host_mitigations_on_guest)
> >               x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl);

This is much too late to restore the host's value of IA32_SPEC_CTRL on
hosts that are using RETBLEED_MITIGATION_UNRET. The host's
IA32_SPEC_CTRL value should be restored prior to the UNTRAIN_RET in
vmenter.S.

> >       if (!sev_es_guest(vcpu->kvm)) {