All of lore.kernel.org
 help / color / mirror / Atom feed
From: Vitaly Kuznetsov <vkuznets@redhat.com>
To: Sean Christopherson <seanjc@google.com>,
	Kevin Cheng <chengkev@google.com>
Cc: pbonzini@redhat.com, kvm@vger.kernel.org,
	linux-kernel@vger.kernel.org, yosry@kernel.org
Subject: Re: [PATCH V4 4/4] KVM: SVM: Raise #UD if VMMCALL instruction is not intercepted
Date: Tue, 03 Mar 2026 10:36:23 +0100	[thread overview]
Message-ID: <87h5qxfe5k.fsf@redhat.com> (raw)
In-Reply-To: <aaZF43PdvrZvIaXn@google.com>

Sean Christopherson <seanjc@google.com> writes:

> +Vitaly
>
> On Sat, Feb 28, 2026, Kevin Cheng wrote:
>> The AMD APM states that if VMMCALL instruction is not intercepted, the
>> instruction raises a #UD exception.
>> 
>> Create a vmmcall exit handler that generates a #UD if a VMMCALL exit
>> from L2 is being handled by L0, which means that L1 did not intercept
>> the VMMCALL instruction. The exception to this is if the exiting
>> instruction was for Hyper-V L2 TLB flush hypercalls as they are handled
>> by L0.
>
> *sigh*
>
> Except this changelog doesn't capture *any* of the subtlety.  And were it not for
> an internal bug discussion, I would have literally no clue WTF is going on.
>
> There's not generic missed #UD bug, because this code in recalc_intercepts()
> effectively disables the VMMCALL intercept in vmcb02 if the intercept isn't set
> in vmcb12.
>
> 	/*
> 	 * We want to see VMMCALLs from a nested guest only when Hyper-V L2 TLB
> 	 * flush feature is enabled.
> 	 */
> 	if (!nested_svm_l2_tlb_flush_enabled(&svm->vcpu))
> 		vmcb_clr_intercept(c, INTERCEPT_VMMCALL);
>
> I.e. the only bug *knowingly* being fixed, maybe, is an edge case where Hyper-V
> TLB flushes are enabled for L2 and the hypercall is something other than one of
> the blessed Hyper-V hypercalls.  But in that case, it's not at all clear to me
> that synthesizing a #UD into L2 is correct.  I can't find anything in the TLFS
> (not surprising), so I guess anything goes?
>
> Vitaly,
>
> The scenario in question is where HV_X64_NESTED_DIRECT_FLUSH is enabled, L1 doesn't
> intercept VMMCALL, and L2 executes VMMCALL with something other than one of the
> Hyper-V TLB flush hypercalls.  The proposed change is to synthesize #UD (which
> is what happens if HV_X64_NESTED_DIRECT_FLUSH isn't enable).  Does that sound
> sane?  Should KVM instead return an error.

I think this does sound sane. In the situation, when the hypercall
issued by L2 is not a TLB flush hypercall, I believe the behavior should
be exactly the same whether HV_X64_NESTED_DIRECT_FLUSH is enabled or
not.

Also, I'm tempted to say that L1 not intercepting VMMCALL and at the
same time using extended features like HV_X64_NESTED_DIRECT_FLUSH can be
an unsupported combo and we can just refuse to run L2 or crash L1 for
misbehaving but I'm afraid this can backfire. E.g. when Hyper-V is
shutting down or in some other 'special' situation.

>
> As for bugs that are *unknowingly* being fixed, intercepting VMMCALL and manually
> injecting a #UD effectively fixes a bad interaction with KVM's asinine
> KVM_X86_QUIRK_FIX_HYPERCALL_INSN.  If KVM doesn't intercept VMMCALL while L2
> is active (L1 doesn't wants to intercept VMMCALL and the Hyper-V L2 TLB flush
> hypercall is disabled), then L2 will hang on the VMMCALL as KVM will intercept
> the #UD, then "emulate" VMMCALL by trying to fixup the opcode and restarting the
> instruction.
>
> That can be "fixed" by disabling the quirk, or by hacking the fixup like so:
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index db3f393192d9..3f6d9950f8f8 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -10506,17 +10506,22 @@ static int emulator_fix_hypercall(struct x86_emulate_ctxt *ctxt)
>          * If the quirk is disabled, synthesize a #UD and let the guest pick up
>          * the pieces.
>          */
> -       if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_FIX_HYPERCALL_INSN)) {
> -               ctxt->exception.error_code_valid = false;
> -               ctxt->exception.vector = UD_VECTOR;
> -               ctxt->have_exception = true;
> -               return X86EMUL_PROPAGATE_FAULT;
> -       }
> +       if (!kvm_check_has_quirk(vcpu->kvm, KVM_X86_QUIRK_FIX_HYPERCALL_INSN))
> +               goto inject_ud;
>  
>         kvm_x86_call(patch_hypercall)(vcpu, instruction);
>  
> +       if (is_guest_mode(vcpu) && !memcmp(instruction, ctxt->fetch.data, 3))
> +               goto inject_ud;
> +
>         return emulator_write_emulated(ctxt, rip, instruction, 3,
>                 &ctxt->exception);
> +
> +inject_ud:
> +       ctxt->exception.error_code_valid = false;
> +       ctxt->exception.vector = UD_VECTOR;
> +       ctxt->have_exception = true;
> +       return X86EMUL_PROPAGATE_FAULT;
>  }
>  
>  static int dm_request_for_irq_injection(struct kvm_vcpu *vcpu)
> --
>
> But that's extremely convoluted for no purpose that I can see.  Not intercepting
> VMMCALL requires _more_ code and is overall more complex.
>
> So unless I'm missing something, I'm going to tack on this to fix the L2 infinite
> loop, and then figure out what to do about Hyper-V, pending Vitaly's input.
>
> diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> index 45d1496031a7..a55af647649c 100644
> --- a/arch/x86/kvm/svm/nested.c
> +++ b/arch/x86/kvm/svm/nested.c
> @@ -156,13 +156,6 @@ void recalc_intercepts(struct vcpu_svm *svm)
>                         vmcb_clr_intercept(c, INTERCEPT_VINTR);
>         }
>  
> -       /*
> -        * We want to see VMMCALLs from a nested guest only when Hyper-V L2 TLB
> -        * flush feature is enabled.
> -        */
> -       if (!nested_svm_l2_tlb_flush_enabled(&svm->vcpu))
> -               vmcb_clr_intercept(c, INTERCEPT_VMMCALL);
> -
>         for (i = 0; i < MAX_INTERCEPT; i++)
>                 c->intercepts[i] |= g->intercepts[i];
>  
>

-- 
Vitaly


  reply	other threads:[~2026-03-03  9:36 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-28  3:33 [PATCH V4 0/4] Align SVM with APM defined behaviors Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 1/4] KVM: SVM: Move STGI and CLGI intercept handling Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 2/4] KVM: SVM: Inject #UD for INVLPGA if EFER.SVME=0 Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 3/4] KVM: SVM: Recalc instructions intercepts when EFER.SVME is toggled Kevin Cheng
2026-02-28  3:33 ` [PATCH V4 4/4] KVM: SVM: Raise #UD if VMMCALL instruction is not intercepted Kevin Cheng
2026-03-03  2:22   ` Sean Christopherson
2026-03-03  9:36     ` Vitaly Kuznetsov [this message]
2026-03-02 16:21 ` [PATCH V4 0/4] Align SVM with APM defined behaviors Yosry Ahmed
2026-03-02 18:32   ` Sean Christopherson
2026-03-02 23:17     ` Sean Christopherson
2026-03-03  0:34       ` Sean Christopherson
2026-03-03 21:56         ` Kevin Cheng
2026-03-03 22:08           ` Sean Christopherson
2026-03-03 22:27             ` Kevin Cheng
2026-03-03 21:52       ` Kevin Cheng
2026-03-03 21:48   ` Kevin Cheng
2026-03-05 17:08 ` Sean Christopherson

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=87h5qxfe5k.fsf@redhat.com \
    --to=vkuznets@redhat.com \
    --cc=chengkev@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pbonzini@redhat.com \
    --cc=seanjc@google.com \
    --cc=yosry@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.