Re: [PATCH] KVM: SVM: Clear VMCB save area instead of entire VMCB on shutdown intercept

[Sean Christopherson <seanjc@google.com>]

On Tue, Aug 25, 2026, Shivansh Dhiman wrote:

> On 24-08-26 19:21, sashiko-bot@kernel.org wrote:
> >> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> >> --- a/arch/x86/kvm/svm/svm.c
> >> +++ b/arch/x86/kvm/svm/svm.c
> > [ ... ]
> >> @@ -2164,18 +2164,23 @@ static int shutdown_interception(struct kvm_vcpu *vcpu)
> > [ ... ]
> >>  	if (!is_sev_es_guest(vcpu)) {
> >> -		clear_page(svm->vmcb);
> >> +		struct vmcb_control_area *control = &svm->vmcb->control;
> >> +
> >> +		memset(&svm->vmcb->save, 0, sizeof(svm->vmcb->save));
> >> +		control->int_ctl &= ~GENMASK(15, 0);
> >> +		control->int_state = 0;
> > 
> > [Severity: High]
> > Since clear_page() was replaced with targeted clearing, do we also need to
> > clear control->event_inj and control->event_inj_err here?
> > 
> > According to the AMD APM, hardware does not clear the EVENTINJ field in the
> > VMCB on a VM exit, which leaves the VALID bit set if an exit occurs during
> > event delivery (such as a triple fault leading to SVM_EXIT_SHUTDOWN).
> > 
> > If the EVENTINJ field is left intact across this shutdown reset, will KVM
> > execute the subsequent VMRUN with the stale EVENTINJ_VALID flag still set?
> > 
> > Could this cause KVM to spuriously inject the old fault into the newly
> > reset real-mode vCPU on its first instruction, leading to an immediate
> > crash or an infinite triple-fault loop?
> The latest APM says otherwise. The #VMEXIT sequence in APM ends with "clear
> EVENTINJ field in VMCB". Hardware clears the field on every exit, so there
> is nothing stale to carry across the shutdown reset and the scenario can't
> arise.

But that seemingly unconditional statement has hidden clauses[*]:

 : >> The second half isn't. Clearing EVENTINJ is not part of injection, it's part of
 : >> #VMEXIT, and it's unconditional.
 : > So that doesn't mesh with the above comment, which says:
 : >
 : >   Hardware clears EVENTINJ field when it injects an event.
 : >
 : > And it begs the question of how this patch is at all useful.  Because all this
 : > fancy new paranoia is clearly generating #VMEXITs, and if #VMEXIT unconditionally
 : > clears control->event_inj, I don't see how control->event_inj can be non-zero if
 : > KVM attempted VMRUN.
 : >
 : > I.e. either this is all broken, or the APM is buggy.
 : 
 : My wording in the comment is misleading. The clear is part of the #VMEXIT path
 : out of guest mode, where it is indeed unconditional. However, VMRUN can
 : terminate even before the guest mode is ever entered. With ESMTP, the hardware
 : can give out a garden variety of exits at the sync point. In this case we
 : get a VMEXIT that implies the VMRUN never actually ran any guest code then
 : EVENTINJ won't be cleared. Since the exit code isn't a reliable discriminator,
 : a non-zero EVENTINJ is the only way to tell.

Ignoring that IMO the APM needs to be updated to clarify exactly when EVENTINJ
is cleared and when it isn't, I don't see any point in not manually clearing the
field on SHUTDOWN.  Common sense would say that it's unnecessary as SHUTDOWN can
only occur if VMRUN gets into the guest, but the cost is completely neglible and
there is zero chance EVENTINJ *needs* to be retained, unlike say the PML index.

We can certainly add a comment saying it's paranoid, but I do think we should
manually clear EVENTINJ and friend, e.g.

	/*
	 * EVENTINJ is cleared on #VMEXIT, but only if VMRUN fully entered the
	 * guest.  Manually clear the fields even though it should be redundant,
	 * as there is no downside to doing so.
	 */
	control->event_inj = 0;
	control->event_inj_err = 0;

[*] https://lore.kernel.org/all/d946de080e3dd34844a665c167179e4910fdc9d4.1789399214.git.prsampat@amd.com