Re: [PATCH 2/2] KVM: SVM: Raise #UD if VMMCALL instruction is not intercepted

[Yosry Ahmed]

On Tue, Jan 06, 2026 at 03:38:57PM -0800, Sean Christopherson wrote:
> On Tue, Jan 06, 2026, Yosry Ahmed wrote:
> > On Tue, Jan 06, 2026 at 10:29:59AM -0800, Sean Christopherson wrote:
> > > > +static int vmmcall_interception(struct kvm_vcpu *vcpu)
> > > > +{
> > > > +	/*
> > > > +	 * If VMMCALL from L2 is not intercepted by L1, the instruction raises a
> > > > +	 * #UD exception
> > > > +	 */
> > > 
> > > Mentioning L2 and L1 is confusing.  It reads like arbitrary KVM behavior.  And
> > > IMO the most notable thing is what's missing: an intercept check.  _That_ is
> > > worth commenting, e.g.
> > > 
> > > 	/*
> > > 	 * VMMCALL #UDs if it's not intercepted, and KVM reaches this point if
> > > 	 * and only if the VMCALL intercept is not set in vmcb12.
> > 
> > Nit: VMMCALL
> > 
> > > 	 */
> > > 
> > 
> > Would it be too paranoid to WARN if the L1 intercept is set here?
> 
> Yes.  At some point we have to rely on not being completely inept :-D, and more
> importantly this is something that should be trivial easy to validate via tests.
> 
> My hesitation for such a check is that adding a WARN here begs the question of
> what makes _this_ particular handler special, i.e. why doesn't every other handler
> also check that an exit shouldn't have been routed to L1?  At that point we'd be
> replicating much of the routing logic into every exit handler.

I briefly thought about this, and I thought since we're explicitly
calling out the dependency on L1 not intercepting this here, a WARN
would make sense. Anyway, your argument make sense. I was going to
suggested adding a WARN in svm_invoke_exit_handler() instead, something
like:

WARN_ON_ONCE(vmcb12_is_intercept(&svm->nested.ctl, exit_code));

But this doesn't work for all cases (e.g. SVM_EXIT_MSR, SVM_EXIT_IOIO,
etc). We can exclude these cases, but this point maintaining the WARN
becomes a burden in itself. I give up :)

> 
> And it _still_ wouldn't guarantee correctness, e.g. wouldn't detect the case where
> KVM incorrectly forwarded a VMMCALL to L1, i.e. we still need the aforementioned
> tests, and so I see the WARN as an overall net-negative.
> 
> > WARN_ON_ONCE(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_VMMCALL));