[PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock and DEV are not available

Kevin Cheng posted 5 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock and DEV are not available
Posted by Kevin Cheng 3 weeks, 6 days ago
The AMD APM states that STGI causes a #UD if SVM is not enabled and
neither SVM Lock nor the device exclusion vector (DEV) are supported.
Fix the STGI exit handler by injecting #UD when these conditions are
met.

Signed-off-by: Kevin Cheng <chengkev@google.com>
---
 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 6373a25d85479..557c84a060fc6 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2271,8 +2271,18 @@ static int stgi_interception(struct kvm_vcpu *vcpu)
 {
 	int ret;
 
-	if (nested_svm_check_permissions(vcpu))
+	if ((!(vcpu->arch.efer & EFER_SVME) &&
+	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) &&
+	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT)) ||
+	    !is_paging(vcpu)) {
+		kvm_queue_exception(vcpu, UD_VECTOR);
+		return 1;
+	}
+
+	if (to_svm(vcpu)->vmcb->save.cpl) {
+		kvm_inject_gp(vcpu, 0);
 		return 1;
+	}
 
 	ret = kvm_skip_emulated_instruction(vcpu);
 	svm_set_gif(to_svm(vcpu), true);
-- 
2.52.0.457.g6b5491de43-goog
Re: [PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock and DEV are not available
Posted by Yosry Ahmed 3 weeks, 5 days ago
On Mon, Jan 12, 2026 at 05:45:32PM +0000, Kevin Cheng wrote:
> The AMD APM states that STGI causes a #UD if SVM is not enabled and
> neither SVM Lock nor the device exclusion vector (DEV) are supported.

Might be useful to also mention the following part "Support for DEV is
part of the SKINIT architecture".

> Fix the STGI exit handler by injecting #UD when these conditions are
> met.
> 
> Signed-off-by: Kevin Cheng <chengkev@google.com>
> ---
>  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 6373a25d85479..557c84a060fc6 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2271,8 +2271,18 @@ static int stgi_interception(struct kvm_vcpu *vcpu)
>  {
>  	int ret;
>  
> -	if (nested_svm_check_permissions(vcpu))
> +	if ((!(vcpu->arch.efer & EFER_SVME) &&
> +	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) &&
> +	     !guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT)) ||
> +	    !is_paging(vcpu)) {
> +		kvm_queue_exception(vcpu, UD_VECTOR);
> +		return 1;
> +	}
> +
> +	if (to_svm(vcpu)->vmcb->save.cpl) {
> +		kvm_inject_gp(vcpu, 0);
>  		return 1;
> +	}

Not a big fan of open-coding nested_svm_check_permissions() here. The
checks could get out of sync.

How about refactoring nested_svm_check_permissions() like so:

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index f295a41ec659..7f53c54b9d39 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1520,9 +1520,10 @@ int nested_svm_exit_handled(struct vcpu_svm *svm)
        return vmexit;
 }

-int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
+static int __nested_svm_check_permissions(struct kvm_vcpu *vcpu,
+                                         bool insn_allowed)
 {
-       if (!(vcpu->arch.efer & EFER_SVME) || !is_paging(vcpu)) {
+       if (!insn_allowed || !is_paging(vcpu)) {
                kvm_queue_exception(vcpu, UD_VECTOR);
                return 1;
        }
@@ -1535,6 +1536,11 @@ int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
        return 0;
 }

+int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
+{
+       return __nested_svm_check_permissions(vcpu, vcpu->arch.efer & EFER_SVME);
+}
+
 static bool nested_svm_is_exception_vmexit(struct kvm_vcpu *vcpu, u8 vector,
                                           u32 error_code)
 {
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 7041498a8091..6340c4ce323c 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -2333,9 +2333,19 @@ void svm_set_gif(struct vcpu_svm *svm, bool value)

 static int stgi_interception(struct kvm_vcpu *vcpu)
 {
+       bool insn_allowed;
        int ret;

-       if (nested_svm_check_permissions(vcpu))
+       /*
+        * According to the APM, STGI is allowed even with SVM disabled if SVM
+        * Lock or device exclusion vector (DEV) are supported. DEV is part of
+        * the SKINIT architecture.
+        */
+       insn_allowed = (vcpu->arch.efer & EFER_SVME) ||
+               guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) ||
+               guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT);
+
+       if (__nested_svm_check_permissions(vcpu, insn_allowed))
                return 1;

        ret = kvm_skip_emulated_instruction(vcpu);

---

We may also want to rename nested_svm_check_permissions() to
nested_svm_insn_check_permissions() or something. Sean, WDYT?

>  
>  	ret = kvm_skip_emulated_instruction(vcpu);
>  	svm_set_gif(to_svm(vcpu), true);
> -- 
> 2.52.0.457.g6b5491de43-goog
>
Re: [PATCH V2 2/5] KVM: SVM: Inject #UD for STGI if EFER.SVME=0 and SVM Lock and DEV are not available
Posted by Kevin Cheng 2 weeks, 3 days ago
On Mon, Jan 12, 2026 at 3:50 PM Yosry Ahmed <yosry.ahmed@linux.dev> wrote:
>
> On Mon, Jan 12, 2026 at 05:45:32PM +0000, Kevin Cheng wrote:
> > The AMD APM states that STGI causes a #UD if SVM is not enabled and
> > neither SVM Lock nor the device exclusion vector (DEV) are supported.
>
> Might be useful to also mention the following part "Support for DEV is
> part of the SKINIT architecture".
>
> > Fix the STGI exit handler by injecting #UD when these conditions are
> > met.
> >
> > Signed-off-by: Kevin Cheng <chengkev@google.com>
> > ---
> >  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 6373a25d85479..557c84a060fc6 100644
> > --- a/arch/x86/kvm/svm/svm.c
> > +++ b/arch/x86/kvm/svm/svm.c
> > @@ -2271,8 +2271,18 @@ static int stgi_interception(struct kvm_vcpu *vcpu)
> >  {
> >       int ret;
> >
> > -     if (nested_svm_check_permissions(vcpu))
> > +     if ((!(vcpu->arch.efer & EFER_SVME) &&
> > +          !guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) &&
> > +          !guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT)) ||
> > +         !is_paging(vcpu)) {
> > +             kvm_queue_exception(vcpu, UD_VECTOR);
> > +             return 1;
> > +     }
> > +
> > +     if (to_svm(vcpu)->vmcb->save.cpl) {
> > +             kvm_inject_gp(vcpu, 0);
> >               return 1;
> > +     }
>
> Not a big fan of open-coding nested_svm_check_permissions() here. The
> checks could get out of sync.
>
> How about refactoring nested_svm_check_permissions() like so:
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index f295a41ec659..7f53c54b9d39 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -1520,9 +1520,10 @@ int nested_svm_exit_handled(struct vcpu_svm *svm)
>         return vmexit;
>  }
>
> -int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
> +static int __nested_svm_check_permissions(struct kvm_vcpu *vcpu,
> +                                         bool insn_allowed)
>  {
> -       if (!(vcpu->arch.efer & EFER_SVME) || !is_paging(vcpu)) {
> +       if (!insn_allowed || !is_paging(vcpu)) {
>                 kvm_queue_exception(vcpu, UD_VECTOR);
>                 return 1;
>         }
> @@ -1535,6 +1536,11 @@ int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
>         return 0;
>  }
>
> +int nested_svm_check_permissions(struct kvm_vcpu *vcpu)
> +{
> +       return __nested_svm_check_permissions(vcpu, vcpu->arch.efer & EFER_SVME);
> +}
> +
>  static bool nested_svm_is_exception_vmexit(struct kvm_vcpu *vcpu, u8 vector,
>                                            u32 error_code)
>  {
> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 7041498a8091..6340c4ce323c 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -2333,9 +2333,19 @@ void svm_set_gif(struct vcpu_svm *svm, bool value)
>
>  static int stgi_interception(struct kvm_vcpu *vcpu)
>  {
> +       bool insn_allowed;
>         int ret;
>
> -       if (nested_svm_check_permissions(vcpu))
> +       /*
> +        * According to the APM, STGI is allowed even with SVM disabled if SVM
> +        * Lock or device exclusion vector (DEV) are supported. DEV is part of
> +        * the SKINIT architecture.
> +        */
> +       insn_allowed = (vcpu->arch.efer & EFER_SVME) ||
> +               guest_cpu_cap_has(vcpu, X86_FEATURE_SVML) ||
> +               guest_cpu_cap_has(vcpu, X86_FEATURE_SKINIT);
> +
> +       if (__nested_svm_check_permissions(vcpu, insn_allowed))
>                 return 1;
>
>         ret = kvm_skip_emulated_instruction(vcpu);
>
> ---
>
> We may also want to rename nested_svm_check_permissions() to
> nested_svm_insn_check_permissions() or something. Sean, WDYT?

I just sent out v3 without the rename for now. Sean, if you prefer
nested_svm_insn_check_permissions over nested_svm_check_permissions
let me know and I can change along with any final revisions.