Re: [PATCH v2 2/2] KVM: SVM: Don't set GIF when clearing EFER.SVME

Yosry Ahmed
2025-11-11T00:27:00Z

On Fri, Oct 17, 2025 at 02:56:58PM -0700, Sean Christopherson wrote:
> On Tue, Oct 14, 2025, Jim Mattson wrote:
> > On Tue, Oct 14, 2025 at 3:07 PM Sean Christopherson <seanjc@google.com> wrote:
> > >
> > > On Tue, Oct 14, 2025, Jim Mattson wrote:
> > > > On Tue, Oct 14, 2025 at 2:18 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > >
> > > > > On Tue, Oct 14, 2025, Jim Mattson wrote:
> > > > > > On Mon, Oct 13, 2025 at 3:33 PM Sean Christopherson <seanjc@google.com> wrote:
> > > > > > >
> > > > > > > On Thu, Oct 09, 2025, Jim Mattson wrote:
> > > > > > > > Clearing EFER.SVME is not architected to set GIF.
> > > > > > >
> > > > > > > But it's also not architected to leave GIF set when the guest is running, which
> > > > > > > was the basic gist of the Fixes commit. I suspect that forcing GIF=1 was
> > > > > > > intentional, e.g. so that the guest doesn't end up with GIF=0 after stuffing the
> > > > > > > vCPU into SMM mode, which might actually be invalid.
> > > > > > >
> > > > > > > I think what we actually want is to to set GIF when force-leaving nested. The
> > > > > > > only path where it's not obvious that's "safe" is toggling SMM in
> > > > > > > kvm_vcpu_ioctl_x86_set_vcpu_events(). In every other path, setting GIF is either
> > > > > > > correct/desirable, or irrelevant because the caller immediately and unconditionally
> > > > > > > sets/clears GIF.
> > > > > > >
> > > > > > > diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
> > > > > > > index a6443feab252..3392c7e22cae 100644
> > > > > > > --- a/arch/x86/kvm/svm/nested.c
> > > > > > > +++ b/arch/x86/kvm/svm/nested.c
> > > > > > > @@ -1367,6 +1367,8 @@ void svm_leave_nested(struct kvm_vcpu *vcpu)
> > > > > > > nested_svm_uninit_mmu_context(vcpu);
> > > > > > > vmcb_mark_all_dirty(svm->vmcb);
> > > > > > >
> > > > > > > + svm_set_gif(svm, true);
> > > > > > > +
> > > > > > > if (kvm_apicv_activated(vcpu->kvm))
> > > > > > > kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
> > > > > > > }
> > > > > > >
> > > > > >
> > > > > > This seems dangerously close to KVM making up "hardware" behavior, but
> > > > > > I'm okay with that if you are.
> > > > >
> > > > > Regardless of what KVM does, we're defining hardware behavior, i.e. keeping GIF
> > > > > unchanged defines behavior just as much as setting GIF. The only way to truly
> > > > > avoid defining behavior would be to terminate the VM and completely prevent
> > > > > userspace from accessing its state.
> > > >
> > > > This can't be the only instance of "undefined behavior" that KVM deals
> > > > with.
> > >
> > > Oh, for sure. But unsurprisingly, people only care about cases that actually
> > > matter in practice. E.g. the other one that comes to mind is SHUTDOWN on AMD:
> > >
> > > /*
> > > * VMCB is undefined after a SHUTDOWN intercept. INIT the vCPU to put
> > > * the VMCB in a known good state. Unfortuately, KVM doesn't have
> > > * KVM_MP_STATE_SHUTDOWN and can't add it without potentially breaking
> > > * userspace. At a platform view, INIT is acceptable behavior as
> > > * there exist bare metal platforms that automatically INIT the CPU
> > > * in response to shutdown.
> > > *
> >
> > The behavior of SHUTDOWN while GIF==0 is clearly architected:
> >
> > "If the processor enters the shutdown state (due to a triple fault for
> > instance) while GIF is clear, it can only be restarted by means of a
> > RESET."
> >
> > Doesn't setting GIF in svm_leave_nested() violate this specification?
>
> Probably? But SHUTDOWN also makes the VMCB undefined, so KVM is caught between
> a rock and a hard place. And when using vGIF, I don't see how KVM can do the
> right thing, because the state of GIF at the time of SHUTDOWN is unknown.
>
> And FWIW, if userspace does RESET the guest (which KVM can't detect with 100%
> accuracy), GIF=1 on RESET, so it's kinda sorta right :-)

This thread kinda ended on a cliffhanger.

Sean, is the plan to apply Jim's patch + your diff to svm_set_gif() in
svm_leave_nested()? Or are you waiting for Jim to send a v2?